diff --git a/TODO.md b/TODO.md index 22b7755..1da4feb 100644 --- a/TODO.md +++ b/TODO.md @@ -99,26 +99,26 @@ **Fix:** Extract `FilePersistenceUtil` or base class -### 2. Persist nicknames on change -`NickServiceImpl.java:77` mutates only in-memory map; `save()` only on shutdown → nicknames lost on crash (async save after mutation). +### 2. Persist nicknames on change ✅ +`NickServiceImpl` now saves asynchronously (`FoliaUtil.runAsync → save()`) on every `setNickname`/`resetNickname` — nicknames survive crashes. ### 3. DIP: Service Locator Anti-Pattern `ServiceRegistry.get(Xxx.class)` used everywhere instead of constructor DI. Big refactor. -### 4. DRY: PM Send Logic Duplicated -`MsgCommand` + `ReplyCommand` still have near-identical send logic. +### 4. DRY: PM Send Logic ✅ +PM cooldown + actual send centralized in `PrivateMessageServiceImpl.sendPrivateMessage`; both `/msg` and `/reply` delegate to it. -### 5. Fix cooldown map collision -`PlayerServiceImpl.java:80-82` — all non-global types (local chat + RP `/me /do /try /roll`) share one map → cross-blocking cooldowns. Split per-type maps. +### 5. Fix cooldown map collision ✅ +`PlayerServiceImpl` now keeps a separate `ConcurrentHashMap` per chat type (`global`, `local`, `rp_me`, `rp_do`, `rp_try`, `pm`) — no more cross-blocking. Covered by `PlayerServiceCooldownTest`. -### 6. Thread-safe flood/spam deques -`FloodFilter.java:32`, `SpamFilter.java:30` use plain `ArrayDeque` written from async threads, read from main → use synchronized/thread-safe structures. +### 6. Thread-safe flood/spam deques ✅ +`FloodFilter`/`SpamFilter` now use `ConcurrentLinkedDeque` instead of plain `ArrayDeque`. ### 7. Tests Only 11 test files for 155 main files. Very low coverage (JaCoCo wired, CI uploads report). -### 8. No PM cooldown -`chat.pm.cooldown: 2` in config.yml is never read → `/msg` spam unthrottled. +### 8. No PM cooldown ✅ +`chat.pm.cooldown` (default 2s) is now enforced in `PrivateMessageServiceImpl.sendPrivateMessage`, honoring `chat.bypass.cooldown`. --- diff --git a/src/main/java/com/loki/lochat/core/filter/filters/FloodFilter.java b/src/main/java/com/loki/lochat/core/filter/filters/FloodFilter.java index f96e365..bf338ae 100644 --- a/src/main/java/com/loki/lochat/core/filter/filters/FloodFilter.java +++ b/src/main/java/com/loki/lochat/core/filter/filters/FloodFilter.java @@ -5,18 +5,17 @@ import com.loki.lochat.core.filter.FilterResult; import org.bukkit.entity.Player; -import java.util.ArrayDeque; -import java.util.Deque; import java.util.Map; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentLinkedDeque; public class FloodFilter { private static final String FLOOD_BLOCK_MESSAGE = "&#CF6679Не флудите!"; private final int maxMessages; private final int timePeriod; - private final Map> floodTracker = new ConcurrentHashMap<>(); + private final Map> floodTracker = new ConcurrentHashMap<>(); public FloodFilter(FiltersConfig filters) { this.maxMessages = filters.getFloodMaxMessages(); @@ -31,8 +30,8 @@ public class FloodFilter { long now = System.currentTimeMillis(); long windowMs = timePeriod * 1000L; - Deque timestamps = floodTracker.computeIfAbsent( - player.getUniqueId(), k -> new ArrayDeque<>()); + ConcurrentLinkedDeque timestamps = floodTracker.computeIfAbsent( + player.getUniqueId(), k -> new ConcurrentLinkedDeque<>()); // Удаляем старые timestamp'ы while (!timestamps.isEmpty() && now - timestamps.peekFirst() > windowMs) { diff --git a/src/main/java/com/loki/lochat/core/filter/filters/SpamFilter.java b/src/main/java/com/loki/lochat/core/filter/filters/SpamFilter.java index 1d70c3a..be6ecf3 100644 --- a/src/main/java/com/loki/lochat/core/filter/filters/SpamFilter.java +++ b/src/main/java/com/loki/lochat/core/filter/filters/SpamFilter.java @@ -5,17 +5,16 @@ import com.loki.lochat.core.filter.FilterResult; import org.bukkit.entity.Player; -import java.util.ArrayDeque; -import java.util.Deque; import java.util.Map; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentLinkedDeque; public class SpamFilter { private final int maxSimilar; private final int threshold; private final String blockMessage; - private final Map> spamTracker = new ConcurrentHashMap<>(); + private final Map> spamTracker = new ConcurrentHashMap<>(); public SpamFilter(FiltersConfig filters) { this(filters.getSpamMaxSimilarMessages(), filters.getSpamSimilarityThreshold(), @@ -33,8 +32,8 @@ public class SpamFilter { return FilterResult.ok(message); } - Deque history = spamTracker.computeIfAbsent( - player.getUniqueId(), k -> new ArrayDeque<>()); + ConcurrentLinkedDeque history = spamTracker.computeIfAbsent( + player.getUniqueId(), k -> new ConcurrentLinkedDeque<>()); // Считаем похожие сообщения long similarCount = history.stream() diff --git a/src/main/java/com/loki/lochat/core/registry/ServiceRegistry.java b/src/main/java/com/loki/lochat/core/registry/ServiceRegistry.java index 9c231d3..882a8c3 100644 --- a/src/main/java/com/loki/lochat/core/registry/ServiceRegistry.java +++ b/src/main/java/com/loki/lochat/core/registry/ServiceRegistry.java @@ -52,7 +52,7 @@ public class ServiceRegistry { register(ChatService.class, new ChatServiceImpl(plugin, messageService)); - PrivateMessageServiceImpl pmService = new PrivateMessageServiceImpl((LoChat) plugin); + PrivateMessageServiceImpl pmService = new PrivateMessageServiceImpl((LoChat) plugin, playerService); MessagingService messagingService = ServiceFactory.createMessagingService(plugin, messageConfig, pmService); pmService.init(messagingService); register(PrivateMessageService.class, pmService); diff --git a/src/main/java/com/loki/lochat/core/service/NickServiceImpl.java b/src/main/java/com/loki/lochat/core/service/NickServiceImpl.java index c167bf0..8419b61 100644 --- a/src/main/java/com/loki/lochat/core/service/NickServiceImpl.java +++ b/src/main/java/com/loki/lochat/core/service/NickServiceImpl.java @@ -4,6 +4,7 @@ import com.loki.lochat.api.service.NickService; import com.loki.lochat.config.RatConfig; import com.loki.lochat.utils.format.ChatFormatter; import com.loki.lochat.utils.persistence.FilePersistence; +import com.loki.lochat.utils.platform.FoliaUtil; import net.kyori.adventure.text.Component; import net.kyori.adventure.text.serializer.plain.PlainTextComponentSerializer; @@ -76,6 +77,9 @@ public class NickServiceImpl implements NickService { // Устанавливаем ник nicknames.put(player, nickname); + // Персистим сразу — не теряем ники при падении сервера + FoliaUtil.runAsync(plugin, this::save); + // Обновляем display Player onlinePlayer = Bukkit.getPlayer(player); if (onlinePlayer != null) { @@ -89,6 +93,9 @@ public class NickServiceImpl implements NickService { public void resetNickname(UUID player) { nicknames.remove(player); + // Персистим сразу + FoliaUtil.runAsync(plugin, this::save); + // Сбрасываем display Player onlinePlayer = Bukkit.getPlayer(player); if (onlinePlayer != null) { diff --git a/src/main/java/com/loki/lochat/core/service/PlayerServiceImpl.java b/src/main/java/com/loki/lochat/core/service/PlayerServiceImpl.java index 1d42491..1963974 100644 --- a/src/main/java/com/loki/lochat/core/service/PlayerServiceImpl.java +++ b/src/main/java/com/loki/lochat/core/service/PlayerServiceImpl.java @@ -20,9 +20,8 @@ public class PlayerServiceImpl implements PlayerService { private final JavaPlugin plugin; - // Cooldown state - private final Map globalCooldowns = new ConcurrentHashMap<>(); - private final Map localCooldowns = new ConcurrentHashMap<>(); + // Cooldown state — separate map per chat type (global, local, rp_me, rp_do, rp_try, pm...) + private final Map> cooldowns = new ConcurrentHashMap<>(); // Statistics state private final Map playerMessages = new ConcurrentHashMap<>(); @@ -74,12 +73,11 @@ public class PlayerServiceImpl implements PlayerService { @Override public void removeCooldown(UUID player) { - globalCooldowns.remove(player); - localCooldowns.remove(player); + cooldowns.values().forEach(map -> map.remove(player)); } private Map getCooldownMap(String type) { - return "global".equals(type) ? globalCooldowns : localCooldowns; + return cooldowns.computeIfAbsent(type, k -> new ConcurrentHashMap<>()); } // ========== Statistics Implementation ========== diff --git a/src/main/java/com/loki/lochat/core/service/messaging/PrivateMessageServiceImpl.java b/src/main/java/com/loki/lochat/core/service/messaging/PrivateMessageServiceImpl.java index f4ecf4e..6826663 100644 --- a/src/main/java/com/loki/lochat/core/service/messaging/PrivateMessageServiceImpl.java +++ b/src/main/java/com/loki/lochat/core/service/messaging/PrivateMessageServiceImpl.java @@ -2,6 +2,7 @@ package com.loki.lochat.core.service.messaging; import com.loki.lochat.LoChat; import com.loki.lochat.api.service.MessagingService; +import com.loki.lochat.api.service.PlayerService; import com.loki.lochat.api.service.pm.PrivateMessageService; import com.loki.lochat.utils.format.ChatFormatter; import com.loki.lochat.utils.player.PlayerUtil; @@ -23,10 +24,12 @@ public class PrivateMessageServiceImpl implements PrivateMessageService { private final Map lastConversation = new ConcurrentHashMap<>(); private final LoChat plugin; + private final PlayerService playerService; private MessagingService messagingService; - public PrivateMessageServiceImpl(LoChat plugin) { + public PrivateMessageServiceImpl(LoChat plugin, PlayerService playerService) { this.plugin = plugin; + this.playerService = playerService; } public void init(MessagingService messagingService) { @@ -56,6 +59,19 @@ public class PrivateMessageServiceImpl implements PrivateMessageService { @Override public void sendPrivateMessage(CommandSender sender, Player target, String message) { Player playerSender = (Player) sender; + + int cooldown = plugin.getConfig().getInt("chat.pm.cooldown", 2); + if (cooldown > 0 && !playerSender.hasPermission("chat.bypass.cooldown") + && playerService.isOnCooldown(playerSender.getUniqueId(), "pm", cooldown)) { + int remaining = playerService.getRemainingCooldown( + playerSender.getUniqueId(), "pm", cooldown); + String cooldownMessage = plugin.getConfigManager().getMessagesConfig() + .getCooldownMessage().replace("{remaining}", String.valueOf(remaining)); + sender.sendMessage(ChatFormatter.parse(cooldownMessage)); + return; + } + playerService.setCooldown(playerSender.getUniqueId(), "pm"); + sender.sendMessage(ChatFormatter.formatPmSentNew( plugin.getMessageConfig().getPmFormatSent(), playerSender, target, message)); target.sendMessage(ChatFormatter.formatPmReceivedNew( diff --git a/src/test/java/com/loki/lochat/core/service/PlayerServiceCooldownTest.java b/src/test/java/com/loki/lochat/core/service/PlayerServiceCooldownTest.java new file mode 100644 index 0000000..1f62034 --- /dev/null +++ b/src/test/java/com/loki/lochat/core/service/PlayerServiceCooldownTest.java @@ -0,0 +1,67 @@ +package com.loki.lochat.core.service; + +import com.loki.lochat.utils.persistence.FilePersistence; + +import org.bukkit.configuration.file.YamlConfiguration; +import org.bukkit.plugin.java.JavaPlugin; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; +import org.mockito.Mockito; + +import java.io.File; +import java.util.UUID; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +class PlayerServiceCooldownTest { + + @Test + void cooldown_typesAreIndependent() { + try (MockedStatic fp = Mockito.mockStatic(FilePersistence.class)) { + JavaPlugin plugin = mock(JavaPlugin.class); + fp.when(() -> FilePersistence.getFile(plugin, "data/statistics.yml")) + .thenReturn(new File("/tmp/nonexistent-statistics.yml")); + fp.when(() -> FilePersistence.loadYaml(plugin, "data/statistics.yml")) + .thenReturn(new YamlConfiguration()); + PlayerServiceImpl service = new PlayerServiceImpl(plugin); + + UUID player = UUID.randomUUID(); + service.setCooldown(player, "rp_me"); + service.setCooldown(player, "local"); + + // rp_me на кулдауне + assertTrue(service.isOnCooldown(player, "rp_me", 5)); + // local на кулдауне + assertTrue(service.isOnCooldown(player, "local", 5)); + // rp_try НЕ на кулдауне (разные типы не блокируют друг друга) + assertFalse(service.isOnCooldown(player, "rp_try", 5)); + // global НЕ на кулдауне + assertFalse(service.isOnCooldown(player, "global", 5)); + // pm НЕ на кулдауне + assertFalse(service.isOnCooldown(player, "pm", 5)); + } + } + + @Test + void removeCooldown_clearsAllTypes() { + try (MockedStatic fp = Mockito.mockStatic(FilePersistence.class)) { + JavaPlugin plugin = mock(JavaPlugin.class); + fp.when(() -> FilePersistence.getFile(plugin, "data/statistics.yml")) + .thenReturn(new File("/tmp/nonexistent-statistics.yml")); + fp.when(() -> FilePersistence.loadYaml(plugin, "data/statistics.yml")) + .thenReturn(new YamlConfiguration()); + PlayerServiceImpl service = new PlayerServiceImpl(plugin); + + UUID player = UUID.randomUUID(); + service.setCooldown(player, "global"); + service.setCooldown(player, "rp_do"); + service.removeCooldown(player); + + assertFalse(service.isOnCooldown(player, "global", 5)); + assertFalse(service.isOnCooldown(player, "rp_do", 5)); + } + } +} \ No newline at end of file