Skip to content
Open
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
package com.eternalcode.core.feature.msg;

import com.eternalcode.core.feature.msg.toggle.MsgState;
import com.eternalcode.core.feature.msg.toggle.MsgToggleRepository;
import com.eternalcode.core.injector.annotations.Inject;
import com.eternalcode.core.injector.annotations.component.Controller;
import com.eternalcode.core.placeholder.PlaceholderRegistry;
import com.eternalcode.core.placeholder.PlaceholderReplacer;
import com.eternalcode.core.placeholder.cache.AsyncPlaceholderCacheRegistry;
import com.eternalcode.core.placeholder.cache.AsyncPlaceholderCached;
import com.eternalcode.core.publish.Subscribe;
import com.eternalcode.core.publish.event.EternalInitializeEvent;
import com.eternalcode.core.translation.Translation;
import com.eternalcode.core.translation.TranslationManager;
import java.time.Duration;
import java.util.UUID;
import java.util.function.Function;

@Controller
public class MsgPlaceholderSetup {

public static final String MSG_STATE_CACHE_KEY = "msg_state";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tutaj mozna byloby rozwazyc wrzucenie dodatkowych stalych globalnych na sama mozliwosc kontrolowania centralnia czas cache zamiast overidowac w metodzie rejeestracji


private final MsgService msgService;
private final MsgToggleRepository msgToggleRepository;
private final TranslationManager translationManager;
private final AsyncPlaceholderCacheRegistry cacheRegistry;

@Inject
MsgPlaceholderSetup(
MsgService msgService,
MsgToggleRepository msgToggleRepository,
TranslationManager translationManager,
AsyncPlaceholderCacheRegistry cacheRegistry
) {
this.msgService = msgService;
this.msgToggleRepository = msgToggleRepository;
this.translationManager = translationManager;
this.cacheRegistry = cacheRegistry;
}

@Subscribe(EternalInitializeEvent.class)
void setUpPlaceholders(PlaceholderRegistry placeholderRegistry) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proponowalbym poprawic nazewnictwo metody obecnie jest to ogolnikowe a nie jasne ze chodzi o rejestracje placeholderow, czyli jezeli jest kompozycja na registry dajemy setUpPlaceholders -> registerMessagePlaceholders, lub samo registryPlaceholders ale jezeli wciagamy "msg" to Message

Translation translation = this.translationManager.getMessages();

AsyncPlaceholderCached<MsgState> stateCache = this.cacheRegistry.register(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Narazie wystarczy ale w przyszlosci jezeli bedziesz planowac w roadmapie dalej zasilac w pelni asynchroniczne placeholdery to wypadaloby zwracac Cfa i opakowywac go w calosci

MSG_STATE_CACHE_KEY,
this.msgToggleRepository::getPrivateChatState,
Duration.ofMinutes(10)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move to private static variable, maybe?

);

placeholderRegistry.registerPlaceholder(PlaceholderReplacer.of(
"socialspy_status",
player -> String.valueOf(this.msgService.isSpy(player.getUniqueId()))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

microoptimizations: use Boolean#toString instead of String.valueOf

Suggested change
player -> String.valueOf(this.msgService.isSpy(player.getUniqueId()))
player -> Boolean.toString(this.msgService.isSpy(player.getUniqueId()))

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obecny auto boxing zamienic na Boolean#toString intuicyjniej bardziej i wyizolowac do prywatnej metody cos w stylu:

private String getSocialSpyStatus(UUID uuid) {
    return Boolean.toString(msgService.isSpy(uuid));
}

));

placeholderRegistry.registerPlaceholder(PlaceholderReplacer.of(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Takie wyrazenia w scope mozna byloby wyizolowac do prywatnej metody przykladowo

private String socialSpyStatusFormatted(UUID uuid, Translation translation) {
    return msgService.isSpy(uuid)
        ? translation.msg().placeholders().socialSpyEnabled()
        : translation.msg().placeholders().socialSpyDisabled();
}

"socialspy_status_formatted",
player -> {
UUID uuid = player.getUniqueId();
return this.msgService.isSpy(uuid)
? translation.msg().placeholders().socialSpyEnabled()
: translation.msg().placeholders().socialSpyDisabled();
}
));

placeholderRegistry.registerPlaceholder(PlaceholderReplacer.of(
"msg_status",
player -> this.formatMsgState(
player.getUniqueId(),
stateCache,
translation,
state -> state.name().toLowerCase()
)
));

placeholderRegistry.registerPlaceholder(PlaceholderReplacer.of(
"msg_status_formatted",
player -> this.formatMsgState(
player.getUniqueId(),
stateCache,
translation,
state -> state == MsgState.ENABLED
? translation.msg().placeholders().msgEnabled()
: translation.msg().placeholders().msgDisabled()
)
));
}

private String formatMsgState(
UUID uuid,
AsyncPlaceholderCached<MsgState> stateCache,
Translation translation,
Function<MsgState, String> formatter
) {
MsgState state = stateCache.getCached(uuid);

if (state == null) {
return translation.msg().placeholders().loading();
}

return formatter.apply(state);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,22 @@ public class ENMsgMessages extends OkaeriConfig implements MsgMessages {
public Notice socialSpyEnable = Notice.chat("<green>► <white>SocialSpy has been {STATE}<white>!");
public Notice socialSpyDisable = Notice.chat("<red>► <white>SocialSpy has been {STATE}<white>!");

@Comment("# Formatowanie placeholderów")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gemini is right 👍

public ENPlaceholders placeholders = new ENPlaceholders();

@Getter
@Accessors(fluent = true)
public static class ENPlaceholders extends OkaeriConfig implements MsgMessages.Placeholders {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section is too general; you should move it to the feature-related section. Creating such a section makes it similar to PlayerSection/ItemSection in the past. Currently, it only has a few messages, but over time, it will become very messy. Alternatively, I suggest you move it to the placeholders.yml file, but remember to also divide it into feature-related sections.

private String loading = "<yellow>Loading...";

private String msgEnabled = "<green>Enabled";
private String msgDisabled = "<red>Disabled";
private String socialSpyEnabled = "<green>Enabled";
private String socialSpyDisabled = "<red>Disabled";
}

public ENPlaceholders placeholders() {
return this.placeholders;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,16 @@ public interface MsgMessages {
Notice otherMessagesDisabled();
Notice otherMessagesEnabled();

Placeholders placeholders();

interface Placeholders {
String loading();

String msgEnabled();
String msgDisabled();

String socialSpyEnabled();
String socialSpyDisabled();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,21 @@ public class PLMsgMessages extends OkaeriConfig implements MsgMessages {
public Notice otherMessagesDisabled = Notice.chat("<green>► <white>Wiadomości prywatne zostały <red>wyłączone <white>dla gracza <green>{PLAYER}<white>!");
public Notice otherMessagesEnabled = Notice.chat("<green>► <white>Wiadomości prywatne zostały <green>włączone <white>dla gracza <green>{PLAYER}<white>!");

@Comment("# Formatowanie placeholderów")
public PLPlaceholders placeholders = new PLPlaceholders();

@Getter
@Accessors(fluent = true)
public static class PLPlaceholders extends OkaeriConfig implements MsgMessages.Placeholders {
private String loading = "<yellow>Ładowanie...";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private String loading = "<yellow>Ładowanie...";
private String loading = "<yellow>Wczytywanie...";

Wczytywanie (w kontekście danych) tutaj brzmi lepiej, ładowanie bardziej się kojarzy z jakimiś intensywnymi zadaniami


private String msgEnabled = "<green>Włączone";
private String msgDisabled = "<red>Wyłączone";
private String socialSpyEnabled = "<green>Włączony";
private String socialSpyDisabled = "<red>Wyłączony";
}

public PLPlaceholders placeholders() {
return this.placeholders;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import java.util.UUID;
import java.util.concurrent.CompletableFuture;

interface MsgToggleRepository {
public interface MsgToggleRepository {

CompletableFuture<MsgState> getPrivateChatState(UUID uuid);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,41 +1,42 @@
package com.eternalcode.core.feature.msg.toggle;

import com.eternalcode.core.feature.msg.MsgPlaceholderSetup;
import com.eternalcode.core.injector.annotations.Inject;
import com.eternalcode.core.injector.annotations.component.Service;
import com.eternalcode.core.placeholder.cache.AsyncPlaceholderCacheRegistry;
import com.eternalcode.core.placeholder.cache.AsyncPlaceholderCached;
import java.util.UUID;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Consumer;

@Service
class MsgToggleServiceImpl implements MsgToggleService {
Copy link
Member

@noyzys noyzys Oct 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rozbilbym to na kilka innych komponentow z ich odpowiedzialnoscia z kosztem korzysci,

  • MessageStateCache -> operacja na cache, izolacja logiki cache,
  • MessageStateToggleCalculator -> logika biznesowa od samego toggle w zamian za kalkulacje i invertowanie,
  • MessageStateUpdateCoordinator -> koordynacja samym update cos ala transaction-like,
  • MessageToggleServiceImpl -> Orchestracja uzyskujemy cienka warstwe ktora koordynuje tylko komponenty

Wszystko wchodzi do glownego MsgToggleService. Wyzej zaznaczylem Message ale to wiadomo o co raczej chodzi

    @Override
    public CompletableFuture<Void> setState(UUID playerId, MsgState state) {
        return updateCoordinator.updateState(playerId, state);
    }

// i tak dalej:
MsgState newState = toggleCalculator.calculateToggledState(currentState);


private final MsgToggleRepository msgToggleRepository;
private final ConcurrentHashMap<UUID, MsgState> cachedToggleStates;
private final AsyncPlaceholderCacheRegistry cacheRegistry;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

słabe rozwiązanie, najlepiej jakby tego cache nie było w repo + nie było tutaj nic placeholderach, bo to tworzy dziwne zależności. Raczej placeholdery widzą API serisów i tyle. Serwisy nie powinny widzieć placeholders


@Inject
MsgToggleServiceImpl(MsgToggleRepository msgToggleRepository) {
this.cachedToggleStates = new ConcurrentHashMap<>();
MsgToggleServiceImpl(MsgToggleRepository msgToggleRepository, AsyncPlaceholderCacheRegistry cacheRegistry) {
this.msgToggleRepository = msgToggleRepository;

this.cacheRegistry = cacheRegistry;
}


@Override
public CompletableFuture<MsgState> getState(UUID playerUniqueId) {
if (this.cachedToggleStates.containsKey(playerUniqueId)) {
return CompletableFuture.completedFuture(this.cachedToggleStates.get(playerUniqueId));
}

return this.msgToggleRepository.getPrivateChatState(playerUniqueId);
return this.msgToggleRepository.getPrivateChatState(playerUniqueId)
.thenApply(state -> {
this.withCache(cache -> cache.update(playerUniqueId, state));
return state;
});
}

@Override
public CompletableFuture<Void> setState(UUID playerUniqueId, MsgState state) {
this.cachedToggleStates.put(playerUniqueId, state);
this.withCache(cache -> cache.update(playerUniqueId, state));

return this.msgToggleRepository.setPrivateChatState(playerUniqueId, state)
.exceptionally(throwable -> {
this.cachedToggleStates.remove(playerUniqueId);
this.withCache(cache -> cache.invalidate(playerUniqueId));
return null;
});
}
Expand All @@ -48,4 +49,9 @@ public CompletableFuture<MsgState> toggleState(UUID playerUniqueId) {
.thenApply(aVoid -> newState);
});
}

private void withCache(Consumer<AsyncPlaceholderCached<MsgState>> action) {
this.cacheRegistry.<MsgState>get(MsgPlaceholderSetup.MSG_STATE_CACHE_KEY)
.ifPresent(action);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package com.eternalcode.core.placeholder.cache;

import com.eternalcode.core.injector.annotations.Inject;
import com.eternalcode.core.injector.annotations.component.Controller;
import com.eternalcode.core.publish.Subscribe;
import com.eternalcode.core.publish.event.EternalReloadEvent;
import com.eternalcode.core.publish.event.EternalShutdownEvent;
import org.bukkit.event.EventHandler;
import org.bukkit.event.EventPriority;
import org.bukkit.event.Listener;
import org.bukkit.event.player.PlayerQuitEvent;

@Controller
class AsyncPlaceholderCacheController implements Listener {

private final AsyncPlaceholderCacheRegistry cacheRegistry;

@Inject
AsyncPlaceholderCacheController(AsyncPlaceholderCacheRegistry cacheRegistry) {
this.cacheRegistry = cacheRegistry;
}

@EventHandler(priority = EventPriority.MONITOR)
void onPlayerQuit(PlayerQuitEvent event) {
this.cacheRegistry.invalidatePlayer(event.getPlayer().getUniqueId());
}

@Subscribe
void onDisable(EternalShutdownEvent event) {
this.cacheRegistry.invalidateAll();
}

@Subscribe
void onReload(EternalReloadEvent event) {
this.cacheRegistry.invalidateAll();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package com.eternalcode.core.placeholder.cache;

import com.eternalcode.core.injector.annotations.Inject;
import com.eternalcode.core.injector.annotations.component.Service;
import java.time.Duration;
import java.util.Map;
import java.util.Optional;
import java.util.UUID;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Function;

@Service
public class AsyncPlaceholderCacheRegistry {

private static final Duration DEFAULT_EXPIRE_DURATION = Duration.ofMinutes(30);

private final Map<String, AsyncPlaceholderCached<?>> caches = new ConcurrentHashMap<>();

@Inject
public AsyncPlaceholderCacheRegistry() {
}

public <T> AsyncPlaceholderCached<T> register(String key, Function<UUID, CompletableFuture<T>> loader) {
Copy link
Member

@noyzys noyzys Oct 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moze byc taka sytuacja ze key juz istnieje w mapie ale zostanie nadpisany bez ostrzezenia to mamy niestety utrate cache,
Minusem troche moze byc to ze nie ma konfiguracji loadera z timeoutem,
Null-check dla key i loader i ewentualne rzucenie wyjatku kiedy klucz juz istnieje na runtime, przekazywanie timeoutu loadera do PlaceholderCached to samo w metodach nizej.
Wlasciwa walidacja do logiki tutaj bylaby bardzo potrzebna jedna metoda zalatwiasz cala walidacje przykladowo:

private <T> void validateRegistrationParameters(String key, Function<UUID, CompletableFuture<T>> loader, Duration expireAfterWrite) {
    if (key == null || key.trim().isEmpty()) {
        throw new IllegalArgumentException("Cache key cannot be null or empty");
    }

    if (loader == null) {
        throw new IllegalArgumentException("Loader function cannot be null");
    }

    if (expireAfterWrite == null) {
        throw new IllegalArgumentException("Expire duration cannot be null");
    }

    if (expireAfterWrite.isNegative() || expireAfterWrite.isZero()) {
        throw new IllegalArgumentException("Expire duration must be positive");
    }
}

Wszystko tutaj mozna rozbudowac i rozszerzyc na jakas konfiguracje jezeli potrzeba w obecnym stanie jezeli nie - zostawiamy i tyle

return this.register(key, loader, DEFAULT_EXPIRE_DURATION);
}

public <T> AsyncPlaceholderCached<T> register(String key, Function<UUID, CompletableFuture<T>> loader, Duration expireAfterWrite) {
AsyncPlaceholderCached<T> cache = new AsyncPlaceholderCached<>(loader, expireAfterWrite);
this.caches.put(key, cache);
return cache;
}

@SuppressWarnings("unchecked")
public <T> Optional<AsyncPlaceholderCached<T>> get(String key) {
return Optional.ofNullable((AsyncPlaceholderCached<T>) this.caches.get(key));
}

public void invalidatePlayer(UUID uuid) {
this.caches.values().forEach(cache -> cache.invalidate(uuid));
}

public void invalidateAll() {
this.caches.values().forEach(AsyncPlaceholderCached::clear);
}

public void unregister(String key) {
AsyncPlaceholderCached<?> cache = this.caches.remove(key);
if (cache != null) {
cache.clear();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package com.eternalcode.core.placeholder.cache;

import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
import java.time.Duration;
import java.util.Map;
import java.util.UUID;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Function;

public class AsyncPlaceholderCached<T> {

private final Cache<UUID, T> cache;
private final Function<UUID, CompletableFuture<T>> loader;
private final Map<UUID, CompletableFuture<T>> loading = new ConcurrentHashMap<>();

public AsyncPlaceholderCached(Function<UUID, CompletableFuture<T>> loader, Duration expireAfterWrite) {
this.loader = loader;
this.cache = CacheBuilder.newBuilder()
.expireAfterWrite(expireAfterWrite)
.build();
}

public T getCached(UUID uuid) {
Copy link
Member

@noyzys noyzys Oct 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return null - jak value nie jest jeszcze dostepna to wymaga obslugi null majac ze soba ryzuko rzucenia NPE.
Obecnie nie zwraca pelnej informacji o tym ze loading odbywa sie cyklu async.

  • Proponowalbym zwracac CFA dlaczego lepiej? - bo zwracamy CFA zamiast null od razu wiemy ze score jest cyklem async,
  • -Mozna dorzucic null check na UUID uzyskujemy safe API
  • Izolacja logiki ladowania do loadAsync - czystsza seperacja wlasciwej odpowiedzialnosci.
  • Obslugujemy async ladowanie danych z pola loader i aktualizujemy cache,
  • Zapobiegamy wielokrotnemu ruszaniu loadera dla tego samego UUID
  • Wazne zeby logika bla non-blocking przy Paper API nie blokujac tickow serwera
  • Ogolnie to najlepiej opakowac takie logiki w jakis error handling dla spokoju i wytrwalosci

Finalnie by to wjechalo tak:

    public CompletableFuture<T> getCached(UUID uuid) {
        // ewentualny null check dla UUID
        T cached = cache.getIfPresent(uuid);
        if (cached != null) {
            return CompletableFuture.completedFuture(cached);
        }

        return loading.computeIfAbsent(uuid, this::loadAsync);
    }

    private CompletableFuture<T> loadAsync(UUID uuid) {
        return loader.apply(uuid).whenComplete((value, throwable) -> {
            if (value != null) {
                cache.put(uuid, value);
            }

            loading.remove(uuid); 
        });
    }

T cached = this.cache.getIfPresent(uuid);
if (cached != null) {
return cached;
}

this.loading.computeIfAbsent(uuid, key ->
this.loader.apply(key).whenComplete((value, throwable) -> {
if (value != null) {
this.cache.put(key, value);
}
this.loading.remove(key);
})
);

return null;
}
public void update(UUID uuid, T value) {
this.cache.put(uuid, value);
}

public void invalidate(UUID uuid) {
this.cache.invalidate(uuid);
}

public void clear() {
this.cache.invalidateAll();
}

public boolean contains(UUID uuid) {
return this.cache.getIfPresent(uuid) != null;
}
}
Loading