From a4c45e126e92f1e1a2f86869db072cb0594901cc Mon Sep 17 00:00:00 2001 From: ljacqu Date: Mon, 30 Nov 2015 21:09:52 +0100 Subject: [PATCH] Start refactoring of command handling (work in progress) Preparation: - Remove unused API - Move some logic from "data classes" elsewhere --- src/main/java/fr/xephi/authme/AuthMe.java | 7 +- .../authme/command/CommandDescription.java | 211 +++--------------- .../xephi/authme/command/CommandHandler.java | 107 ++++++--- .../fr/xephi/authme/command/CommandParts.java | 11 - .../fr/xephi/authme/command/CommandUtils.java | 21 ++ .../authme/command/help/HelpPrinter.java | 9 +- .../authme/command/help/HelpSyntaxHelper.java | 2 +- 7 files changed, 140 insertions(+), 228 deletions(-) create mode 100644 src/main/java/fr/xephi/authme/command/CommandUtils.java diff --git a/src/main/java/fr/xephi/authme/AuthMe.java b/src/main/java/fr/xephi/authme/AuthMe.java index d6427ae1b..715bebc8f 100644 --- a/src/main/java/fr/xephi/authme/AuthMe.java +++ b/src/main/java/fr/xephi/authme/AuthMe.java @@ -24,6 +24,7 @@ import fr.xephi.authme.settings.*; import fr.xephi.authme.util.GeoLiteAPI; import fr.xephi.authme.util.StringUtils; import fr.xephi.authme.util.Utils; +import fr.xephi.authme.util.Wrapper; import net.minelink.ctplus.CombatTagPlus; import org.apache.logging.log4j.LogManager; import org.bukkit.Bukkit; @@ -72,6 +73,7 @@ public class AuthMe extends JavaPlugin { private static AuthMe plugin; private static Server server; + private static Wrapper wrapper = Wrapper.getInstance(); public Management management; public NewAPI api; @@ -942,9 +944,10 @@ public class AuthMe extends JavaPlugin { public boolean onCommand(CommandSender sender, Command cmd, String commandLabel, String[] args) { // Get the command handler, and make sure it's valid - CommandHandler commandHandler = this.getCommandHandler(); - if (commandHandler == null) + if (commandHandler == null) { + wrapper.getLogger().warning("AuthMe command handler is not available"); return false; + } // Handle the command, return the result return commandHandler.onCommand(sender, cmd, commandLabel, args); diff --git a/src/main/java/fr/xephi/authme/command/CommandDescription.java b/src/main/java/fr/xephi/authme/command/CommandDescription.java index 887650a54..76948aeb0 100644 --- a/src/main/java/fr/xephi/authme/command/CommandDescription.java +++ b/src/main/java/fr/xephi/authme/command/CommandDescription.java @@ -25,7 +25,7 @@ public class CommandDescription { * Defines the labels to execute the command. For example, if labels are "register" and "r" and the parent is * the command for "/authme", then both "/authme register" and "/authme r" will be handled by this command. */ - private List labels; + private List labels = new ArrayList<>(); // TODO remove field initialization /** * Command description. */ @@ -49,11 +49,11 @@ public class CommandDescription { /** * The arguments the command takes. */ - private List arguments; + private List arguments = new ArrayList<>(); // TODO remove field initialization /** * Defines whether there is an argument maximum or not. */ - private boolean noArgumentMaximum; + private boolean noArgumentMaximum = false; // TODO remove field initialization /** * Defines the command permissions. */ @@ -70,7 +70,8 @@ public class CommandDescription { */ @Deprecated public CommandDescription(ExecutableCommand executableCommand, List labels, String description, String detailedDescription, CommandDescription parent) { - this(executableCommand, labels, description, detailedDescription, parent, null); + this(executableCommand, labels, description, detailedDescription, parent, + new ArrayList()); } /** @@ -90,7 +91,7 @@ public class CommandDescription { this.description = description; this.detailedDescription = detailedDescription; setParent(parent); - setArguments(arguments); + this.arguments = arguments; } /** @@ -122,25 +123,6 @@ public class CommandDescription { } } - /** - * Check whether two labels are equal to each other. - * - * @param commandLabel The first command label. - * @param otherCommandLabel The other command label. - * - * @return True if the labels are equal to each other. - */ - private static boolean commandLabelEquals(String commandLabel, String otherCommandLabel) { - // Trim the command labels from unwanted whitespaces - commandLabel = commandLabel.trim(); - otherCommandLabel = otherCommandLabel.trim(); - - // Check whether the the two command labels are equal (case insensitive) - return (commandLabel.equalsIgnoreCase(otherCommandLabel)); - } - - - /** * Get the label most similar to the reference. The first label will be returned if no reference was supplied. * @@ -192,12 +174,11 @@ public class CommandDescription { * @return True if this command label equals to the param command. */ public boolean hasLabel(String commandLabel) { - // Check whether any command matches with the argument - for (String entry : this.labels) - if (commandLabelEquals(entry, commandLabel)) + for (String label : this.labels) { + if (label.equalsIgnoreCase(commandLabel)) { return true; - - // No match found, return false + } + } return false; } @@ -215,48 +196,16 @@ public class CommandDescription { return false; // Get the parent count + //getParent() = getParent().getParentCount() + 1 String element = commandReference.get(getParentCount()); // Check whether this command description has this command label - return hasLabel(element); - } - - /** - * Get the absolute command label, without a starting slash. - * - * @return The absolute label - */ - public String getAbsoluteLabel() { - return getAbsoluteLabel(false); - } - - /** - * Get the absolute command label. - * - * @param includeSlash boolean - * - * @return Absolute command label. - */ - public String getAbsoluteLabel(boolean includeSlash) { - return getAbsoluteLabel(includeSlash, null); - } - - /** - * Get the absolute command label. - * - * @param includeSlash - * @param reference - * - * @return Absolute command label. - */ - public String getAbsoluteLabel(boolean includeSlash, CommandParts reference) { - // Get the command reference, and make sure it is valid - CommandParts out = getCommandReference(reference); - if (out == null) - return ""; - - // Return the result - return (includeSlash ? "/" : "") + out.toString(); + for (String label : labels) { + if (label.equalsIgnoreCase(element)) { + return true; + } + } + return false; } /** @@ -463,7 +412,7 @@ public class CommandDescription { */ public boolean isChild(CommandDescription commandDescription) { // Make sure the description is valid - if (commandDescription == null) // TODO: After builder, commandDescription == null -> never + if (commandDescription == null) return false; // Check whether this child exists, return the result @@ -482,10 +431,6 @@ public class CommandDescription { if (argument == null) return false; - // Make sure the argument isn't added already - if (hasArgument(argument)) - return true; - // Add the argument, return the result return this.arguments.add(argument); } @@ -499,81 +444,17 @@ public class CommandDescription { return this.arguments; } - /** - * Set the arguments of this command. - * - * @param arguments New command arguments. Null to clear the list of arguments. - */ - public void setArguments(List arguments) { - // Convert null into an empty argument list - if (arguments == null) { - // Note ljacqu 20151128: Temporary workaround to avoid null pointer exception. Soon we won't need setters - // on the main class (-> complete instantiation via Builder) - // TODO Remove this method once unused - this.arguments = new ArrayList<>(); - } else { - this.arguments = arguments; - } - } - - /** - * Check whether an argument exists. - * - * @param argument The argument to check for. - * - * @return True if this argument already exists, false otherwise. - */ - public boolean hasArgument(CommandArgumentDescription argument) { - return argument != null && arguments.contains(argument); - } - /** * Check whether this command has any arguments. * * @return True if this command has any arguments. */ public boolean hasArguments() { - return !arguments.isEmpty(); + return !getArguments().isEmpty(); } - /** - * The minimum number of arguments required for this command. - * - * @return The minimum number of required arguments. - */ - public int getMinimumArguments() { - // Get the number of required and optional arguments - int requiredArguments = 0; - int optionalArgument = 0; - - // Loop through each argument - for (CommandArgumentDescription argument : this.arguments) { - // Check whether the command is optional - if (!argument.isOptional()) { - requiredArguments += optionalArgument + 1; - optionalArgument = 0; - - } else - optionalArgument++; - } - - // Return the number of required arguments - return requiredArguments; - } - - /** - * Get the maximum number of arguments. - * - * @return The maximum number of arguments. A negative number will be returned if there's no maximum. - */ - public int getMaximumArguments() { - // Check whether there is a maximum set - if (this.noArgumentMaximum) - // TODO ljacqu 20151128: Magic number - return -1; - - // Return the maximum based on the registered arguments - return this.arguments.size(); + public boolean hasMaximumArguments() { + return !noArgumentMaximum; // TODO ljacqu 20151130 Change variable name } /** @@ -582,16 +463,7 @@ public class CommandDescription { * @return Command description. */ public String getDescription() { - return hasDescription() ? this.description : this.detailedDescription; - } - - /** - * Check whether this command has any description. - * - * @return True if this command has any description. - */ - public boolean hasDescription() { - return !StringUtils.isEmpty(description); + return description; } /** @@ -600,7 +472,7 @@ public class CommandDescription { * @return Command detailed description. */ public String getDetailedDescription() { - return !StringUtils.isEmpty(detailedDescription) ? this.detailedDescription : this.description; + return detailedDescription; } /** @@ -616,12 +488,13 @@ public class CommandDescription { return null; // Check whether this description is for the last element in the command reference, if so return the current command - if (queryReference.getCount() <= getParentCount() + 1) + if (queryReference.getCount() <= getParentCount() + 1) { return new FoundCommandResult( this, getCommandReference(queryReference), new CommandParts(), queryReference); + } // Get the new command reference and arguments CommandParts newReference = new CommandParts(queryReference.getRange(0, getParentCount() + 1)); @@ -665,28 +538,6 @@ public class CommandDescription { return null; } - /** - * Check whether there's any command description that matches the specified command reference. - * - * @param commandReference The command reference. - * - * @return True if so, false otherwise. - */ - public boolean hasSuitableCommand(CommandParts commandReference) { - return findCommand(commandReference) != null; - } - - /** - * Check if the remaining command reference elements are suitable with arguments of the current command description. - * - * @param commandReference The command reference. - * - * @return True if the arguments are suitable, false otherwise. - */ - public boolean hasSuitableArguments(CommandParts commandReference) { - return getSuitableArgumentsDifference(commandReference) == 0; - } - /** * Check if the remaining command reference elements are suitable with arguments of the current command description, * and get the difference in argument count. @@ -705,16 +556,18 @@ public class CommandDescription { int remainingElementCount = commandReference.getCount() - getParentCount() - 1; // Check if there are too few arguments - if (getMinimumArguments() > remainingElementCount) { - return Math.abs(getMinimumArguments() - remainingElementCount); + int minArguments = CommandUtils.getMinNumberOfArguments(this); + if (minArguments > remainingElementCount) { + return Math.abs(minArguments - remainingElementCount); } // Check if there are too many arguments - if (getMaximumArguments() < remainingElementCount && getMaximumArguments() >= 0) { - return Math.abs(remainingElementCount - getMaximumArguments()); + int maxArguments = CommandUtils.getMaxNumberOfArguments(this); + if (maxArguments >= 0 && maxArguments < remainingElementCount) { + return Math.abs(remainingElementCount - maxArguments); } - // The arguments seem to be EQUALS, return the result + // The argument count is the same return 0; } diff --git a/src/main/java/fr/xephi/authme/command/CommandHandler.java b/src/main/java/fr/xephi/authme/command/CommandHandler.java index 5290507ac..6a6a53207 100644 --- a/src/main/java/fr/xephi/authme/command/CommandHandler.java +++ b/src/main/java/fr/xephi/authme/command/CommandHandler.java @@ -2,11 +2,12 @@ package fr.xephi.authme.command; import fr.xephi.authme.AuthMe; import fr.xephi.authme.command.help.HelpProvider; +import fr.xephi.authme.util.StringUtils; import org.bukkit.ChatColor; +import org.bukkit.command.Command; import org.bukkit.command.CommandSender; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; /** @@ -15,6 +16,18 @@ import java.util.List; */ public class CommandHandler { + /** + * The threshold for assuming an existing command. If the difference is below this value, we assume + * that the user meant the similar command and we will run it. + */ + private static final double ASSUME_COMMAND_THRESHOLD = 0.12; + + /** + * The threshold for suggesting a similar command. If the difference is below this value, we will + * ask the player whether he meant the similar command. + */ + private static final double SUGGEST_COMMAND_THRESHOLD = 0.75; + /** * Process a command. * @@ -26,12 +39,11 @@ public class CommandHandler { * @return True if the command was executed, false otherwise. */ // TODO ljacqu 20151129: Rename onCommand() method to something not suggesting it is auto-invoked by an event - public boolean onCommand(CommandSender sender, org.bukkit.command.Command bukkitCommand, String bukkitCommandLabel, String[] bukkitArgs) { - // Process the arguments - List args = processArguments(bukkitArgs); + public boolean onCommand(CommandSender sender, Command bukkitCommand, String bukkitCommandLabel, String[] bukkitArgs) { + List commandArgs = skipEmptyArguments(bukkitArgs); - // Create a command reference, and make sure at least one command part is available - CommandParts commandReference = new CommandParts(bukkitCommandLabel, args); + // Make sure the command isn't empty (does this happen?) + CommandParts commandReference = new CommandParts(bukkitCommandLabel, commandArgs); if (commandReference.getCount() == 0) return false; @@ -47,12 +59,12 @@ public class CommandHandler { // Make sure the difference between the command reference and the actual command isn't too big final double commandDifference = result.getDifference(); - if (commandDifference > 0.12) { + if (commandDifference > ASSUME_COMMAND_THRESHOLD) { // Show the unknown command warning sender.sendMessage(ChatColor.DARK_RED + "Unknown command!"); // Show a command suggestion if available and the difference isn't too big - if (commandDifference < 0.75) + if (commandDifference < SUGGEST_COMMAND_THRESHOLD) if (result.getCommandDescription() != null) sender.sendMessage(ChatColor.YELLOW + "Did you mean " + ChatColor.GOLD + "/" + result.getCommandDescription().getCommandReference(commandReference) + ChatColor.YELLOW + "?"); @@ -113,33 +125,29 @@ public class CommandHandler { } /** - * Process the command arguments, and return them as an array list. + * Skips all entries of the given array that are simply whitespace. * - * @param args The command arguments to process. - * - * @return The processed command arguments. + * @param args The array to process + * @return List of the items that are not empty */ - private List processArguments(String[] args) { - // Convert the array into a list of arguments - List arguments = new ArrayList<>(Arrays.asList(args)); - - /// Remove all empty arguments - for (int i = 0; i < arguments.size(); i++) { - // Get the argument value - final String arg = arguments.get(i); - - // Check whether the argument value is empty - if (arg.trim().length() == 0) { - // Remove the current argument - arguments.remove(i); - - // Decrease the index by one, continue to the next argument - i--; + private static List skipEmptyArguments(String[] args) { + List cleanArguments = new ArrayList<>(args.length); + for (String argument : args) { + if (!StringUtils.isEmpty(argument)) { + cleanArguments.add(argument); } } + return cleanArguments; + } - // Return the argument - return arguments; + + private static CommandDescription mapToBase(String commandLabel) { + for (CommandDescription command : CommandInitializer.getBaseCommands()) { + if (command.getLabels().contains(commandLabel)) { + return command; + } + } + return null; } /** @@ -170,4 +178,43 @@ public class CommandHandler { // No applicable command description found, return false return null; } + + /** + * Find the best suitable command for the specified reference. + * + * @param commandParts The query reference to find a command for. + * + * @return The command found, or null. + */ + public CommandDescription findCommand(List commandParts) { + // Make sure the command reference is valid + if (commandParts.isEmpty()) { + return null; + } + + // TODO ljacqu 20151129: Since we only use .contains() on the CommandDescription#labels after init, change + // the type to set for faster lookup + Iterable commandsToScan = CommandInitializer.getBaseCommands(); + CommandDescription result = null; + for (String label : commandParts) { + result = findLabel(label, commandsToScan); + if (result == null) { + return null; + } + commandsToScan = result.getChildren(); + } + return result; + } + + private static CommandDescription findLabel(String label, Iterable commands) { + if (commands == null) { + return null; + } + for (CommandDescription command : commands) { + if (command.getLabels().contains(label)) { + return command; + } + } + return null; + } } diff --git a/src/main/java/fr/xephi/authme/command/CommandParts.java b/src/main/java/fr/xephi/authme/command/CommandParts.java index 1c677f225..35e895746 100644 --- a/src/main/java/fr/xephi/authme/command/CommandParts.java +++ b/src/main/java/fr/xephi/authme/command/CommandParts.java @@ -162,17 +162,6 @@ public class CommandParts { return elements; } - /** - * Get the difference value between two references. This won't do a full compare, just the last reference parts instead. - * - * @param other The other reference. - * - * @return The result from zero to above. A negative number will be returned on error. - */ - public double getDifference(CommandParts other) { - return getDifference(other, false); - } - /** * Get the difference value between two references. * diff --git a/src/main/java/fr/xephi/authme/command/CommandUtils.java b/src/main/java/fr/xephi/authme/command/CommandUtils.java new file mode 100644 index 000000000..f8e78c46e --- /dev/null +++ b/src/main/java/fr/xephi/authme/command/CommandUtils.java @@ -0,0 +1,21 @@ +package fr.xephi.authme.command; + +public final class CommandUtils { + + public static int getMinNumberOfArguments(CommandDescription command) { + int mandatoryArguments = 0; + for (CommandArgumentDescription argument : command.getArguments()) { + if (!argument.isOptional()) { + ++mandatoryArguments; + } + } + return mandatoryArguments; + } + + public static int getMaxNumberOfArguments(CommandDescription command) { + return command.hasMaximumArguments() + ? command.getArguments().size() + : -1; + } + +} diff --git a/src/main/java/fr/xephi/authme/command/help/HelpPrinter.java b/src/main/java/fr/xephi/authme/command/help/HelpPrinter.java index d67647621..a3244c914 100644 --- a/src/main/java/fr/xephi/authme/command/help/HelpPrinter.java +++ b/src/main/java/fr/xephi/authme/command/help/HelpPrinter.java @@ -5,6 +5,7 @@ import fr.xephi.authme.command.CommandArgumentDescription; import fr.xephi.authme.command.CommandDescription; import fr.xephi.authme.command.CommandParts; import fr.xephi.authme.command.CommandPermissions; +import fr.xephi.authme.command.CommandUtils; import fr.xephi.authme.permission.PermissionNode; import fr.xephi.authme.util.StringUtils; import org.bukkit.ChatColor; @@ -39,9 +40,7 @@ public class HelpPrinter { * @param command The command to print the description help for. */ public static void printCommandDescription(CommandSender sender, CommandDescription command) { - // Print the regular description, if available - if (command.hasDescription()) - sender.sendMessage(ChatColor.GOLD + "Short Description: " + ChatColor.WHITE + command.getDescription()); + sender.sendMessage(ChatColor.GOLD + "Short Description: " + ChatColor.WHITE + command.getDescription()); // Print the detailed description, if available if (!StringUtils.isEmpty(command.getDetailedDescription())) { @@ -59,7 +58,7 @@ public class HelpPrinter { @SuppressWarnings("StringConcatenationInsideStringBufferAppend") public static void printArguments(CommandSender sender, CommandDescription command) { // Make sure there are any commands to print - if (!command.hasArguments() && command.getMaximumArguments() >= 0) + if (!command.hasArguments()) return; // Print the header @@ -80,7 +79,7 @@ public class HelpPrinter { } // Show the unlimited arguments argument - if (command.getMaximumArguments() < 0) + if (!command.hasMaximumArguments()) sender.sendMessage(" " + ChatColor.YELLOW + ChatColor.ITALIC + "... : " + ChatColor.WHITE + "Any additional arguments." + ChatColor.GRAY + ChatColor.ITALIC + " (Optional)"); } diff --git a/src/main/java/fr/xephi/authme/command/help/HelpSyntaxHelper.java b/src/main/java/fr/xephi/authme/command/help/HelpSyntaxHelper.java index 90a9c7716..024914c9a 100644 --- a/src/main/java/fr/xephi/authme/command/help/HelpSyntaxHelper.java +++ b/src/main/java/fr/xephi/authme/command/help/HelpSyntaxHelper.java @@ -62,7 +62,7 @@ public final class HelpSyntaxHelper { } // Add some dots if the command allows unlimited arguments - if (commandDescription.getMaximumArguments() < 0) { + if (!commandDescription.hasMaximumArguments()) { sb.append(ChatColor.ITALIC).append(" ..."); }