fix: require marketblock.admin for /marketblock

src/main/java/net/tfminecraft/marketblock/manager/CommandManager.java

@@ -16,12 +16,20 @@
 
 public class CommandManager implements CommandExecutor {
 
+    public static final String ADMIN_PERMISSION = "marketblock.admin";
+
     public String cmd1 = "marketblock";
 
     @Override
     public boolean onCommand(CommandSender sender, Command command, String label, String[] args) {
         if (!command.getName().equalsIgnoreCase(cmd1)) return true;
 
+        // plugin.yml also guards the command, but every subcommand here edits live trades.
+        if (!sender.hasPermission(ADMIN_PERMISSION)) {
+            sender.sendMessage("§cYou do not have permission to use this command.");
+            return true;
+        }
+
         if (args.length == 0) {
             sender.sendMessage("§cUsage: /marketblock add OR /marketblock delete <id>");
             return true;
@@ -52,7 +60,7 @@ public boolean onCommand(CommandSender sender, Command command, String label, St
 
             MarketblockConversation convo = new MarketblockConversation(player, item);
             ConversationManager.startConversation(player, convo);
-            player.sendMessage("§aPlease enter the trade ID in chat:");
+            player.sendMessage("§aPlease enter the trade ID in chat, or type §ecancel§a to stop:");
             return true;
         }
 

src/main/java/net/tfminecraft/marketblock/manager/commands/ChatListener.java

@@ -1,13 +1,17 @@
 package net.tfminecraft.marketblock.manager.commands;
 
+import org.bukkit.Bukkit;
 import org.bukkit.entity.Player;
 import org.bukkit.event.EventHandler;
 import org.bukkit.event.Listener;
 import org.bukkit.event.player.AsyncPlayerChatEvent;
+import org.bukkit.event.player.PlayerQuitEvent;
 
 import net.tfminecraft.tlibs.TLibs;
+import net.tfminecraft.marketblock.MarketBlock;
 import net.tfminecraft.marketblock.loader.CategoryLoader;
 import net.tfminecraft.marketblock.loader.TradeLoader;
+import net.tfminecraft.marketblock.manager.CommandManager;
 import net.tfminecraft.marketblock.trade.Category;
 import net.tfminecraft.marketblock.trade.Trade;
 
@@ -22,9 +26,35 @@ public void onPlayerChat(AsyncPlayerChatEvent event) {
 
         if (convo == null) return;
 
-        event.setCancelled(true);
+        answerIfCurrent(event, player, convo);
+    }
+
+    /**
+     * One answer at a time per conversation. A late message for a conversation that has already
+     * been cancelled, finished or replaced is not an answer, so it goes to chat as normal.
+     */
+    @SuppressWarnings("deprecation")
+    void answerIfCurrent(AsyncPlayerChatEvent event, Player player, MarketblockConversation convo) {
         String message = event.getMessage();
+        synchronized (convo) {
+            if (ConversationManager.getConversation(player) != convo) return;
+            // Permission may have been removed since /marketblock add; let the message reach chat.
+            if (!player.hasPermission(CommandManager.ADMIN_PERMISSION)) {
+                ConversationManager.finishConversation(player, convo);
+                return;
+            }
+            event.setCancelled(true);
+            if (TradeInput.isCancel(message)) {
+                if (ConversationManager.finishConversation(player, convo)) {
+                    player.sendMessage("§eTrade creation cancelled.");
+                }
+                return;
+            }
+            answer(player, convo, message);
+        }
+    }
 
+    private void answer(Player player, MarketblockConversation convo, String message) {
         switch (convo.getStep()) {
             case 0 -> {
                 if (TradeLoader.getTradeById(message) != null) {
@@ -37,15 +67,15 @@ public void onPlayerChat(AsyncPlayerChatEvent event) {
                 player.sendMessage("§aEnter the demand limit:");
             }
             case 1 -> {
-                try {
-                    double demandLimit = Double.parseDouble(message);
-                    convo.setDemandLimit(demandLimit);
-                    player.sendMessage("§aDemand limit set to: " + demandLimit);
-                    convo.nextStep();
-                    player.sendMessage("§aEnter the group:");
-                } catch (NumberFormatException e) {
-                    player.sendMessage("§cInvalid number. Please enter a valid demand limit.");
+                Double demandLimit = TradeInput.demandLimit(message);
+                if (demandLimit == null) {
+                    player.sendMessage("§cInvalid number. Enter a demand limit of at least 1.");
+                    return;
                 }
+                convo.setDemandLimit(demandLimit);
+                player.sendMessage("§aDemand limit set to: " + demandLimit);
+                convo.nextStep();
+                player.sendMessage("§aEnter the group:");
             }
             case 2 -> {
                 try {
@@ -59,42 +89,50 @@ public void onPlayerChat(AsyncPlayerChatEvent event) {
                 }
             }
             case 3 -> {
-                try {
-                    double priceChange = Double.parseDouble(message);
-                    convo.setPriceChange(priceChange);
-                    player.sendMessage("§aPrice change set to: " + priceChange);
-                    convo.nextStep();
-                    player.sendMessage("§aEnter the resting price:");
-                } catch (NumberFormatException e) {
-                    player.sendMessage("§cInvalid number. Please enter a valid price change.");
+                Double priceChange = TradeInput.priceChange(message);
+                if (priceChange == null) {
+                    player.sendMessage("§cInvalid number. Enter a price change of 0 or more.");
+                    return;
                 }
+                convo.setPriceChange(priceChange);
+                player.sendMessage("§aPrice change set to: " + priceChange);
+                convo.nextStep();
+                player.sendMessage("§aEnter the resting price:");
             }
             case 4 -> {
-                try {
-                    double restingPrice = Double.parseDouble(message);
-                    convo.setRestingPrice(restingPrice);
-                    player.sendMessage("§aResting price set to: " + restingPrice);
-                    convo.nextStep();
-                    player.sendMessage("§aEnter the category:");
-                } catch (NumberFormatException e) {
-                    player.sendMessage("§cInvalid number. Please enter a valid resting price.");
+                Double restingPrice = TradeInput.restingPrice(message);
+                if (restingPrice == null) {
+                    player.sendMessage("§cInvalid number. Enter a resting price above 0.");
+                    return;
                 }
+                convo.setRestingPrice(restingPrice);
+                player.sendMessage("§aResting price set to: " + restingPrice);
+                convo.nextStep();
+                player.sendMessage("§aEnter the category:");
             }
             case 5 -> {
+                // Done! Claim the conversation first, so a quit that got there first stops the save.
+                if (!ConversationManager.finishConversation(player, convo)) return;
                 Category cat = CategoryLoader.getByString(message);
                 if (cat == null || cat.getId().equalsIgnoreCase("unknown")) {
                     player.sendMessage("§cWarning, no category found, default selected");
                 }
                 convo.setCategory(cat);
                 player.sendMessage("§aCategory set to: " + cat.getId());
-                // Done!
-                ConversationManager.endConversation(player);
-                if (!saveTrade(player, convo)) return;
-                player.sendMessage("§aTrade successfully created!");
+                // Chat arrives on an async thread; trades and trades.yml belong to the main thread.
+                Bukkit.getScheduler().runTask(MarketBlock.plugin, () -> {
+                    if (!saveTrade(player, convo)) return;
+                    player.sendMessage("§aTrade successfully created!");
+                });
             }
         }
     }
 
+    @EventHandler
+    public void onQuit(PlayerQuitEvent event) {
+        ConversationManager.endConversation(event.getPlayer());
+    }
+
     private boolean saveTrade(Player p, MarketblockConversation convo) {
         String path = TLibs.getItemAPI().getChecker().getAsStringPath(convo.getItem());
         if(path == null) {

src/main/java/net/tfminecraft/marketblock/manager/commands/ConversationManager.java

@@ -2,12 +2,14 @@
 
 import org.bukkit.entity.Player;
 
-import java.util.HashMap;
+import java.util.Map;
 import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
 
 public class ConversationManager {
 
-    private static final HashMap<UUID, MarketblockConversation> conversations = new HashMap<>();
+    // Read from the async chat thread and written from the main thread.
+    private static final Map<UUID, MarketblockConversation> conversations = new ConcurrentHashMap<>();
 
     public static void startConversation(Player player, MarketblockConversation convo) {
         conversations.put(player.getUniqueId(), convo);
@@ -20,5 +22,13 @@ public static MarketblockConversation getConversation(Player player) {
     public static void endConversation(Player player) {
         conversations.remove(player.getUniqueId());
     }
+
+    /**
+     * Ends this exact conversation and reports whether this call did so. Only one caller can win,
+     * so a trade is saved only if its final answer arrived before the player quit or cancelled.
+     */
+    public static boolean finishConversation(Player player, MarketblockConversation convo) {
+        return conversations.remove(player.getUniqueId(), convo);
+    }
 }
 

src/main/java/net/tfminecraft/marketblock/manager/commands/TabCompletion.java

@@ -1,6 +1,7 @@
 package net.tfminecraft.marketblock.manager.commands;
 
 import net.tfminecraft.marketblock.loader.TradeLoader;
+import net.tfminecraft.marketblock.manager.CommandManager;
 import org.bukkit.command.Command;
 import org.bukkit.command.CommandSender;
 import org.bukkit.command.TabCompleter;
@@ -15,6 +16,7 @@ public List<String> onTabComplete(CommandSender sender, Command command, String
         List<String> completions = new ArrayList<>();
 
         if (!command.getName().equalsIgnoreCase("marketblock")) return completions;
+        if (!sender.hasPermission(CommandManager.ADMIN_PERMISSION)) return completions;
 
         if (args.length == 1) {
             String prefix = args[0].toLowerCase();

src/main/java/net/tfminecraft/marketblock/manager/commands/TradeInput.java

@@ -0,0 +1,45 @@
+package net.tfminecraft.marketblock.manager.commands;
+
+/**
+ * Parses the numbers typed during /marketblock add. Each method returns null when the text is not
+ * a usable value, so a typo or a value such as NaN or Infinity cannot reach a saved trade.
+ */
+public final class TradeInput {
+
+    public static final String CANCEL = "cancel";
+
+    private TradeInput() {
+    }
+
+    public static boolean isCancel(String message) {
+        return message != null && message.trim().equalsIgnoreCase(CANCEL);
+    }
+
+    /** Demand limit: a finite number of at least 1, the lowest a trade allows. */
+    public static Double demandLimit(String message) {
+        Double value = finite(message);
+        return value != null && value >= 1 ? value : null;
+    }
+
+    /** Price change: a finite number of at least 0. */
+    public static Double priceChange(String message) {
+        Double value = finite(message);
+        return value != null && value >= 0 ? value : null;
+    }
+
+    /** Resting price: a finite number above 0. */
+    public static Double restingPrice(String message) {
+        Double value = finite(message);
+        return value != null && value > 0 ? value : null;
+    }
+
+    private static Double finite(String message) {
+        if (message == null) return null;
+        try {
+            double value = Double.parseDouble(message.trim());
+            return Double.isFinite(value) ? value : null;
+        } catch (NumberFormatException e) {
+            return null;
+        }
+    }
+}

src/main/resources/plugin.yml

@@ -9,5 +9,11 @@ softdepend: [Cooking]
 
 commands:
   marketblock:
-    useage: /<command>
-    description: MarketBlock Command
+    usage: /<command>
+    description: MarketBlock Command
+    permission: marketblock.admin
+
+permissions:
+  marketblock.admin:
+    description: Add, delete, reset and reload market trades
+    default: op

src/test/java/net/tfminecraft/marketblock/manager/CommandManagerTest.java

@@ -0,0 +1,89 @@
+package net.tfminecraft.marketblock.manager;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.InputStreamReader;
+import java.io.Reader;
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+
+import org.bukkit.command.Command;
+import org.bukkit.command.CommandSender;
+import org.bukkit.configuration.file.YamlConfiguration;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+
+import net.tfminecraft.marketblock.manager.commands.ConversationManager;
+import net.tfminecraft.marketblock.manager.commands.TabCompletion;
+
+class CommandManagerTest {
+    private final CommandManager commands = new CommandManager();
+    private final Command command = new Command("marketblock") {
+        @Override
+        public boolean execute(CommandSender sender, String label, String[] args) {
+            return false;
+        }
+    };
+    private FakePlayer tester;
+
+    @AfterEach
+    void endConversation() {
+        if (tester != null) ConversationManager.endConversation(tester.player);
+    }
+
+    @Test
+    void aPlayerWithoutPermissionCannotStartAddingATrade() {
+        tester = new FakePlayer(false);
+
+        assertTrue(commands.onCommand(tester.player, command, "marketblock", new String[] { "add" }));
+
+        assertNull(ConversationManager.getConversation(tester.player));
+        assertEquals("§cYou do not have permission to use this command.", tester.lastMessage());
+    }
+
+    @Test
+    void everySubcommandIsRefusedWithoutPermission() {
+        // reload would reach JavaPlugin.getPlugin and throw here if the check did not stop it first.
+        for (String sub : List.of("add", "delete", "reset", "resetall", "reload")) {
+            tester = new FakePlayer(false);
+
+            commands.onCommand(tester.player, command, "marketblock", new String[] { sub, "stone" });
+
+            assertEquals(List.of("§cYou do not have permission to use this command."), tester.messages, sub);
+        }
+    }
+
+    @Test
+    void anAdminGetsPastThePermissionCheck() {
+        tester = new FakePlayer(true);
+
+        commands.onCommand(tester.player, command, "marketblock", new String[] { "add" });
+
+        assertEquals("§cYou must hold an item in your hand to add a trade.", tester.lastMessage());
+    }
+
+    @Test
+    void tabCompletionIsEmptyWithoutPermission() {
+        TabCompletion completion = new TabCompletion();
+
+        assertTrue(completion.onTabComplete(new FakePlayer(false).player, command, "marketblock", new String[] { "" }).isEmpty());
+        assertEquals(List.of("add", "delete", "reload", "reset", "resetall"),
+                completion.onTabComplete(new FakePlayer(true).player, command, "marketblock", new String[] { "" }));
+    }
+
+    @Test
+    void pluginYmlRequiresTheAdminPermission() throws Exception {
+        YamlConfiguration yaml;
+        try (Reader reader = new InputStreamReader(
+                getClass().getClassLoader().getResourceAsStream("plugin.yml"), StandardCharsets.UTF_8)) {
+            yaml = YamlConfiguration.loadConfiguration(reader);
+        }
+
+        assertEquals(CommandManager.ADMIN_PERMISSION, yaml.getString("commands.marketblock.permission"));
+        assertNotNull(yaml.getConfigurationSection("permissions." + CommandManager.ADMIN_PERMISSION));
+        assertEquals("op", yaml.getString("permissions." + CommandManager.ADMIN_PERMISSION + ".default"));
+    }
+}