From da0c5d1ea2a082deafa763f515bfa2bca7da376a Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sun, 29 Nov 2015 12:51:11 +0100 Subject: [PATCH 01/10] Split command management into initializer and handler - Create Initializer class that only initializes AuthMe commands - Move remaining method to CommandHandler - Deprecate constructors on CommandDescription in favor of the builder - Various cleanups and comments --- src/main/java/fr/xephi/authme/AuthMe.java | 3 +- .../command/CommandArgumentDescription.java | 9 +- .../authme/command/CommandDescription.java | 24 +- .../xephi/authme/command/CommandHandler.java | 103 +++----- ...ndManager.java => CommandInitializer.java} | 223 ++++++++---------- .../authme/command/help/HelpProvider.java | 4 +- .../authme/permission/PermissionsManager.java | 1 + .../authme/command/CommandManagerTest.java | 32 +-- 8 files changed, 170 insertions(+), 229 deletions(-) rename src/main/java/fr/xephi/authme/command/{CommandManager.java => CommandInitializer.java} (82%) diff --git a/src/main/java/fr/xephi/authme/AuthMe.java b/src/main/java/fr/xephi/authme/AuthMe.java index ab54071a1..d6427ae1b 100644 --- a/src/main/java/fr/xephi/authme/AuthMe.java +++ b/src/main/java/fr/xephi/authme/AuthMe.java @@ -418,8 +418,7 @@ public class AuthMe extends JavaPlugin { * Set up the command handler. */ private void setupCommandHandler() { - this.commandHandler = new CommandHandler(false); - this.commandHandler.init(); + this.commandHandler = new CommandHandler(); } /** diff --git a/src/main/java/fr/xephi/authme/command/CommandArgumentDescription.java b/src/main/java/fr/xephi/authme/command/CommandArgumentDescription.java index 0440ddc0f..5a9cd9d7c 100644 --- a/src/main/java/fr/xephi/authme/command/CommandArgumentDescription.java +++ b/src/main/java/fr/xephi/authme/command/CommandArgumentDescription.java @@ -1,6 +1,7 @@ package fr.xephi.authme.command; /** + * Wrapper for the description of a command argument. */ public class CommandArgumentDescription { @@ -9,15 +10,15 @@ public class CommandArgumentDescription { /** * Argument label (one-word description of the argument). */ - private String label; + private final String label; /** * Argument description. */ - private String description; + private final String description; /** * Defines whether the argument is optional. */ - private boolean isOptional = false; + private final boolean isOptional; /** * Constructor. @@ -51,7 +52,7 @@ public class CommandArgumentDescription { } /** - * Check whether the argument is optional. + * Return whether the argument is optional. * * @return True if the argument is optional, false otherwise. */ diff --git a/src/main/java/fr/xephi/authme/command/CommandDescription.java b/src/main/java/fr/xephi/authme/command/CommandDescription.java index 3a5342461..887650a54 100644 --- a/src/main/java/fr/xephi/authme/command/CommandDescription.java +++ b/src/main/java/fr/xephi/authme/command/CommandDescription.java @@ -15,9 +15,9 @@ import java.util.List; * Command description - defines which labels ("names") will lead to a command and points to the * {@link ExecutableCommand} implementation that executes the logic of the command. * - * CommandDescription is built hierarchically and have one parent or {@code null} for base commands (main commands - * such as /authme) and may have multiple children extending the mapping of the parent: e.g. if /authme has a child - * whose label is "register", then "/authme register" is the command that the child defines. + * CommandDescription instances are built hierarchically and have one parent or {@code null} for base commands + * (main commands such as /authme) and may have multiple children extending the mapping of the parent: e.g. if + * /authme has a child whose label is "register", then "/authme register" is the command that the child defines. */ public class CommandDescription { @@ -68,6 +68,7 @@ public class CommandDescription { * @param detailedDescription Detailed comment description. * @param parent Parent command. */ + @Deprecated public CommandDescription(ExecutableCommand executableCommand, List labels, String description, String detailedDescription, CommandDescription parent) { this(executableCommand, labels, description, detailedDescription, parent, null); } @@ -82,6 +83,7 @@ public class CommandDescription { * @param parent Parent command. * @param arguments Command arguments. */ + @Deprecated public CommandDescription(ExecutableCommand executableCommand, List labels, String description, String detailedDescription, CommandDescription parent, List arguments) { setExecutableCommand(executableCommand); this.labels = labels; @@ -739,6 +741,9 @@ public class CommandDescription { return new Builder(); } + /** + * Builder for initializing CommandDescription objects. + */ public static final class Builder { private List labels; private String description; @@ -750,7 +755,8 @@ public class CommandDescription { private CommandPermissions permissions; /** - * Build a CommandDescription from the builder. + * Build a CommandDescription from the builder or throw an exception if mandatory + * fields have not been set. * * @return The generated CommandDescription object */ @@ -796,6 +802,16 @@ public class CommandDescription { return this; } + /** + * Add an argument that the command description requires. This method can be called multiples times to add + * multiple arguments. + * + * @param label The label of the argument (single word name of the argument) + * @param description The description of the argument + * @param isOptional True if the argument is option, false if it is mandatory + * + * @return The builder + */ public Builder withArgument(String label, String description, boolean isOptional) { arguments.add(new CommandArgumentDescription(label, description, isOptional)); return this; diff --git a/src/main/java/fr/xephi/authme/command/CommandHandler.java b/src/main/java/fr/xephi/authme/command/CommandHandler.java index d06c1b876..5290507ac 100644 --- a/src/main/java/fr/xephi/authme/command/CommandHandler.java +++ b/src/main/java/fr/xephi/authme/command/CommandHandler.java @@ -10,78 +10,11 @@ import java.util.Arrays; import java.util.List; /** + * The AuthMe command handler, responsible for mapping incoming commands to the correct {@link CommandDescription} + * or to display help messages for unknown invocations. */ public class CommandHandler { - /** - * The command manager instance. - */ - private CommandManager commandManager; - - /** - * Constructor. - * - * @param init True to immediately initialize. - */ - public CommandHandler(boolean init) { - // Initialize - if (init) - init(); - } - - /** - * Initialize the command handler. - * - * @return True if succeed, false on failure. True will also be returned if the command handler was already - * initialized. - */ - public boolean init() { - // Make sure the handler isn't initialized already - if (isInit()) - return true; - - // Initialize the command manager - this.commandManager = new CommandManager(false); - this.commandManager.registerCommands(); - - // Return the result - return true; - } - - /** - * Check whether the command handler is initialized. - * - * @return True if the command handler is initialized. - */ - public boolean isInit() { - return this.commandManager != null; - } - - /** - * Destroy the command handler. - * - * @return True if the command handler was destroyed successfully, false otherwise. True will also be returned if - * the command handler wasn't initialized. - */ - public boolean destroy() { - // Make sure the command handler is initialized - if (!isInit()) - return true; - - // Unset the command manager - this.commandManager = null; - return true; - } - - /** - * Get the command manager. - * - * @return Command manager instance. - */ - public CommandManager getCommandManager() { - return this.commandManager; - } - /** * Process a command. * @@ -92,6 +25,7 @@ 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); @@ -102,7 +36,7 @@ public class CommandHandler { return false; // Get a suitable command for this reference, and make sure it isn't null - FoundCommandResult result = this.commandManager.findCommand(commandReference); + FoundCommandResult result = findCommand(commandReference); if (result == null) { sender.sendMessage(ChatColor.DARK_RED + "Failed to parse " + AuthMe.getPluginName() + " command!"); return false; @@ -207,4 +141,33 @@ public class CommandHandler { // Return the argument return arguments; } + + /** + * Find the best suitable command for the specified reference. + * + * @param queryReference The query reference to find a command for. + * + * @return The command found, or null. + */ + public FoundCommandResult findCommand(CommandParts queryReference) { + // Make sure the command reference is valid + if (queryReference.getCount() <= 0) + return null; + + // TODO ljacqu 20151129: If base commands are only used in here (or in the future CommandHandler after changes), + // it might make sense to make the CommandInitializer package-private and to return its result into this class + // instead of regularly fetching the list of base commands from the other class. + for (CommandDescription commandDescription : CommandInitializer.getBaseCommands()) { + // Check whether there's a command description available for the + // current command + if (!commandDescription.isSuitableLabel(queryReference)) + continue; + + // Find the command reference, return the result + return commandDescription.findCommand(queryReference); + } + + // No applicable command description found, return false + return null; + } } diff --git a/src/main/java/fr/xephi/authme/command/CommandManager.java b/src/main/java/fr/xephi/authme/command/CommandInitializer.java similarity index 82% rename from src/main/java/fr/xephi/authme/command/CommandManager.java rename to src/main/java/fr/xephi/authme/command/CommandInitializer.java index cc335561c..fd0c4e786 100644 --- a/src/main/java/fr/xephi/authme/command/CommandManager.java +++ b/src/main/java/fr/xephi/authme/command/CommandInitializer.java @@ -1,7 +1,26 @@ package fr.xephi.authme.command; import fr.xephi.authme.command.executable.HelpCommand; -import fr.xephi.authme.command.executable.authme.*; +import fr.xephi.authme.command.executable.authme.AccountsCommand; +import fr.xephi.authme.command.executable.authme.AuthMeCommand; +import fr.xephi.authme.command.executable.authme.ChangePasswordCommand; +import fr.xephi.authme.command.executable.authme.FirstSpawnCommand; +import fr.xephi.authme.command.executable.authme.ForceLoginCommand; +import fr.xephi.authme.command.executable.authme.GetEmailCommand; +import fr.xephi.authme.command.executable.authme.GetIpCommand; +import fr.xephi.authme.command.executable.authme.LastLoginCommand; +import fr.xephi.authme.command.executable.authme.PurgeBannedPlayersCommand; +import fr.xephi.authme.command.executable.authme.PurgeCommand; +import fr.xephi.authme.command.executable.authme.PurgeLastPositionCommand; +import fr.xephi.authme.command.executable.authme.RegisterCommand; +import fr.xephi.authme.command.executable.authme.ReloadCommand; +import fr.xephi.authme.command.executable.authme.SetEmailCommand; +import fr.xephi.authme.command.executable.authme.SetFirstSpawnCommand; +import fr.xephi.authme.command.executable.authme.SetSpawnCommand; +import fr.xephi.authme.command.executable.authme.SpawnCommand; +import fr.xephi.authme.command.executable.authme.SwitchAntiBotCommand; +import fr.xephi.authme.command.executable.authme.UnregisterCommand; +import fr.xephi.authme.command.executable.authme.VersionCommand; import fr.xephi.authme.command.executable.captcha.CaptchaCommand; import fr.xephi.authme.command.executable.converter.ConverterCommand; import fr.xephi.authme.command.executable.email.AddEmailCommand; @@ -11,6 +30,7 @@ import fr.xephi.authme.command.executable.login.LoginCommand; import fr.xephi.authme.command.executable.logout.LogoutCommand; import fr.xephi.authme.permission.AdminPermission; import fr.xephi.authme.permission.UserPermission; +import fr.xephi.authme.util.Wrapper; import java.util.ArrayList; import java.util.Arrays; @@ -20,132 +40,125 @@ import static fr.xephi.authme.command.CommandPermissions.DefaultPermission.ALLOW import static fr.xephi.authme.command.CommandPermissions.DefaultPermission.OP_ONLY; /** + * Initializes all available AuthMe commands. */ -public class CommandManager { +public final class CommandInitializer { - /** - * The list of commandDescriptions. - */ - private final List commandDescriptions = new ArrayList<>(); + private static List baseCommands; - /** - * Constructor. - * - * @param registerCommands True to register the commands, false otherwise. - */ - public CommandManager(boolean registerCommands) { - // Register the commands - if (registerCommands) - registerCommands(); + private CommandInitializer() { + // Helper class } - /** - * Register all commands. - */ - public void registerCommands() { + public static List getBaseCommands() { + if (baseCommands == null) { + Wrapper.getInstance().getLogger().info("Initializing AuthMe commands"); + initializeCommands(); + } + return baseCommands; + } + + private static void initializeCommands() { // Create a list of help command labels final List helpCommandLabels = Arrays.asList("help", "hlp", "h", "sos", "?"); - ExecutableCommand helpCommandExecutable = new HelpCommand(); + final ExecutableCommand helpCommandExecutable = new HelpCommand(); // Register the base AuthMe Reloaded command - CommandDescription authMeBaseCommand = CommandDescription.builder() - .executableCommand(new AuthMeCommand()) + final CommandDescription AUTHME_BASE = CommandDescription.builder() .labels("authme") .description("Main command") .detailedDescription("The main AuthMeReloaded command. The root for all admin commands.") - .parent(null) + .executableCommand(new AuthMeCommand()) .build(); // Register the help command - CommandDescription authMeHelpCommand = CommandDescription.builder() - .executableCommand(helpCommandExecutable) + CommandDescription.builder() + .parent(AUTHME_BASE) .labels(helpCommandLabels) .description("View help") .detailedDescription("View detailed help pages about AuthMeReloaded commands.") - .parent(authMeBaseCommand) .withArgument("query", "The command or query to view help for.", true) + .executableCommand(helpCommandExecutable) .build(); // Register the register command - CommandDescription registerCommand = CommandDescription.builder() - .executableCommand(new RegisterCommand()) + CommandDescription.builder() + .parent(AUTHME_BASE) .labels("register", "reg", "r") .description("Register a player") .detailedDescription("Register the specified player with the specified password.") - .parent(authMeBaseCommand) - .permissions(OP_ONLY, UserPermission.REGISTER) .withArgument("player", "Player name", false) .withArgument("password", "Password", false) + .permissions(OP_ONLY, UserPermission.REGISTER) + .executableCommand(new RegisterCommand()) .build(); // Register the unregister command - CommandDescription unregisterCommand = CommandDescription.builder() - .executableCommand(new UnregisterCommand()) + CommandDescription.builder() + .parent(AUTHME_BASE) .labels("unregister", "unreg", "unr") .description("Unregister a player") .detailedDescription("Unregister the specified player.") - .parent(authMeBaseCommand) - .permissions(OP_ONLY, UserPermission.UNREGISTER) .withArgument("player", "Player name", false) + .permissions(OP_ONLY, UserPermission.UNREGISTER) + .executableCommand(new UnregisterCommand()) .build(); // Register the forcelogin command - CommandDescription forceLoginCommand = CommandDescription.builder() - .executableCommand(new ForceLoginCommand()) + CommandDescription.builder() + .parent(AUTHME_BASE) .labels("forcelogin", "login") .description("Enforce login player") .detailedDescription("Enforce the specified player to login.") - .parent(authMeBaseCommand) - .permissions(OP_ONLY, UserPermission.CAN_LOGIN_BE_FORCED) .withArgument("player", "Online player name", true) + .permissions(OP_ONLY, UserPermission.CAN_LOGIN_BE_FORCED) + .executableCommand(new ForceLoginCommand()) .build(); // Register the changepassword command - CommandDescription changePasswordCommand = CommandDescription.builder() - .executableCommand(new ChangePasswordCommand()) + CommandDescription.builder() + .parent(AUTHME_BASE) .labels("password", "changepassword", "changepass", "cp") .description("Change a player's password") .detailedDescription("Change the password of a player.") - .parent(authMeBaseCommand) - .permissions(OP_ONLY, UserPermission.CHANGE_PASSWORD) .withArgument("player", "Player name", false) .withArgument("pwd", "New password", false) + .permissions(OP_ONLY, UserPermission.CHANGE_PASSWORD) + .executableCommand(new ChangePasswordCommand()) .build(); // Register the last login command - CommandDescription lastLoginCommand = CommandDescription.builder() - .executableCommand(new LastLoginCommand()) + CommandDescription.builder() + .parent(AUTHME_BASE) .labels("lastlogin", "ll") .description("Player's last login") .detailedDescription("View the date of the specified players last login.") - .parent(authMeBaseCommand) - .permissions(OP_ONLY, AdminPermission.LAST_LOGIN) .withArgument("player", "Player name", true) + .permissions(OP_ONLY, AdminPermission.LAST_LOGIN) + .executableCommand(new LastLoginCommand()) .build(); // Register the accounts command - CommandDescription accountsCommand = CommandDescription.builder() - .executableCommand(new AccountsCommand()) + CommandDescription.builder() + .parent(AUTHME_BASE) .labels("accounts", "account") .description("Display player accounts") .detailedDescription("Display all accounts of a player by his player name or IP.") - .parent(authMeBaseCommand) - .permissions(OP_ONLY, AdminPermission.ACCOUNTS) .withArgument("player", "Player name or IP", true) + .permissions(OP_ONLY, AdminPermission.ACCOUNTS) + .executableCommand(new AccountsCommand()) .build(); // Register the getemail command - CommandDescription getEmailCommand = new CommandDescription(new GetEmailCommand(), new ArrayList() { - - { - add("getemail"); - add("getmail"); - add("email"); - add("mail"); - } - }, "Display player's email", "Display the email address of the specified player if set.", authMeBaseCommand); - getEmailCommand.setCommandPermissions(AdminPermission.GET_EMAIL, OP_ONLY); - getEmailCommand.addArgument(new CommandArgumentDescription("player", "Player name", true)); + CommandDescription.builder() + .parent(AUTHME_BASE) + .labels("getemail", "getmail", "email", "mail") + .description("Display player's email") + .detailedDescription("Display the email address of the specified player if set.") + .withArgument("player", "Player name", true) + .permissions(OP_ONLY, AdminPermission.GET_EMAIL) + .executableCommand(new GetEmailCommand()) + .build(); // Register the setemail command CommandDescription setEmailCommand = new CommandDescription(new SetEmailCommand(), new ArrayList() { @@ -156,7 +169,7 @@ public class CommandManager { add("setemail"); add("setmail"); } - }, "Change player's email", "Change the email address of the specified player.", authMeBaseCommand); + }, "Change player's email", "Change the email address of the specified player.", AUTHME_BASE); setEmailCommand.setCommandPermissions(AdminPermission.CHANGE_EMAIL, OP_ONLY); setEmailCommand.addArgument(new CommandArgumentDescription("player", "Player name", false)); setEmailCommand.addArgument(new CommandArgumentDescription("email", "Player email", false)); @@ -168,7 +181,7 @@ public class CommandManager { add("getip"); add("ip"); } - }, "Get player's IP", "Get the IP address of the specified online player.", authMeBaseCommand); + }, "Get player's IP", "Get the IP address of the specified online player.", AUTHME_BASE); getIpCommand.setCommandPermissions(AdminPermission.GET_IP, OP_ONLY); getIpCommand.addArgument(new CommandArgumentDescription("player", "Online player name", true)); @@ -179,7 +192,7 @@ public class CommandManager { add("spawn"); add("home"); } - }, "Teleport to spawn", "Teleport to the spawn.", authMeBaseCommand); + }, "Teleport to spawn", "Teleport to the spawn.", AUTHME_BASE); spawnCommand.setCommandPermissions(AdminPermission.SPAWN, OP_ONLY); // Register the setspawn command @@ -189,7 +202,7 @@ public class CommandManager { add("setspawn"); add("chgspawn"); } - }, "Change the spawn", "Change the player's spawn to your current position.", authMeBaseCommand); + }, "Change the spawn", "Change the player's spawn to your current position.", AUTHME_BASE); setSpawnCommand.setCommandPermissions(AdminPermission.SET_SPAWN, OP_ONLY); // Register the firstspawn command @@ -199,7 +212,7 @@ public class CommandManager { add("firstspawn"); add("firsthome"); } - }, "Teleport to first spawn", "Teleport to the first spawn.", authMeBaseCommand); + }, "Teleport to first spawn", "Teleport to the first spawn.", AUTHME_BASE); firstSpawnCommand.setCommandPermissions(AdminPermission.FIRST_SPAWN, OP_ONLY); // Register the setfirstspawn command @@ -209,7 +222,7 @@ public class CommandManager { add("setfirstspawn"); add("chgfirstspawn"); } - }, "Change the first spawn", "Change the first player's spawn to your current position.", authMeBaseCommand); + }, "Change the first spawn", "Change the first player's spawn to your current position.", AUTHME_BASE); setFirstSpawnCommand.setCommandPermissions(AdminPermission.SET_FIRST_SPAWN, OP_ONLY); // Register the purge command @@ -219,7 +232,7 @@ public class CommandManager { add("purge"); add("delete"); } - }, "Purge old data", "Purge old AuthMeReloaded data longer than the specified amount of days ago.", authMeBaseCommand); + }, "Purge old data", "Purge old AuthMeReloaded data longer than the specified amount of days ago.", AUTHME_BASE); purgeCommand.setCommandPermissions(AdminPermission.PURGE, OP_ONLY); purgeCommand.addArgument(new CommandArgumentDescription("days", "Number of days", false)); @@ -234,7 +247,7 @@ public class CommandManager { add("resetlastposition"); add("resetlastpos"); } - }, "Purge player's last position", "Purge the last know position of the specified player.", authMeBaseCommand); + }, "Purge player's last position", "Purge the last know position of the specified player.", AUTHME_BASE); purgeLastPositionCommand.setCommandPermissions(AdminPermission.PURGE_LAST_POSITION, OP_ONLY); purgeLastPositionCommand.addArgument(new CommandArgumentDescription("player", "Player name", true)); @@ -247,7 +260,7 @@ public class CommandManager { add("deletebannedplayers"); add("deletebannedplayer"); } - }, "Purge banned palyers data", "Purge all AuthMeReloaded data for banned players.", authMeBaseCommand); + }, "Purge banned palyers data", "Purge all AuthMeReloaded data for banned players.", AUTHME_BASE); purgeBannedPlayersCommand.setCommandPermissions(AdminPermission.PURGE_BANNED_PLAYERS, OP_ONLY); // Register the switchantibot command @@ -258,7 +271,7 @@ public class CommandManager { add("toggleantibot"); add("antibot"); } - }, "Switch AntiBot mode", "Switch or toggle the AntiBot mode to the specified state.", authMeBaseCommand); + }, "Switch AntiBot mode", "Switch or toggle the AntiBot mode to the specified state.", AUTHME_BASE); switchAntiBotCommand.setCommandPermissions(AdminPermission.SWITCH_ANTIBOT, OP_ONLY); switchAntiBotCommand.addArgument(new CommandArgumentDescription("mode", "ON / OFF", true)); @@ -282,7 +295,7 @@ public class CommandManager { add("reload"); add("rld"); } - }, "Reload plugin", "Reload the AuthMeReloaded plugin.", authMeBaseCommand); + }, "Reload plugin", "Reload the AuthMeReloaded plugin.", AUTHME_BASE); reloadCommand.setCommandPermissions(AdminPermission.RELOAD, OP_ONLY); // Register the version command @@ -292,7 +305,7 @@ public class CommandManager { .description("Version info") .detailedDescription("Show detailed information about the installed AuthMeReloaded version, and shows the " + "developers, contributors, license and other information.") - .parent(authMeBaseCommand) + .parent(AUTHME_BASE) .build(); // Register the base login command @@ -461,59 +474,15 @@ public class CommandManager { converterHelpCommand.addArgument(new CommandArgumentDescription("query", "The command or query to view help for.", true)); // Add the base commands to the commands array - this.commandDescriptions.add(authMeBaseCommand); - this.commandDescriptions.add(loginBaseCommand); - this.commandDescriptions.add(logoutBaseCommand); - this.commandDescriptions.add(registerBaseCommand); - this.commandDescriptions.add(unregisterBaseCommand); - this.commandDescriptions.add(changePasswordBaseCommand); - this.commandDescriptions.add(emailBaseCommand); - this.commandDescriptions.add(captchaBaseCommand); - this.commandDescriptions.add(converterBaseCommand); - } - - /** - * Get the list of command descriptions - * - * @return List of command descriptions. - */ - public List getCommandDescriptions() { - return this.commandDescriptions; - } - - /** - * Get the number of command description count. - * - * @return Command description count. - */ - public int getCommandDescriptionCount() { - return this.getCommandDescriptions().size(); - } - - /** - * Find the best suitable command for the specified reference. - * - * @param queryReference The query reference to find a command for. - * - * @return The command found, or null. - */ - public FoundCommandResult findCommand(CommandParts queryReference) { - // Make sure the command reference is valid - if (queryReference.getCount() <= 0) - return null; - - // Get the base command description - for (CommandDescription commandDescription : this.commandDescriptions) { - // Check whether there's a command description available for the - // current command - if (!commandDescription.isSuitableLabel(queryReference)) - continue; - - // Find the command reference, return the result - return commandDescription.findCommand(queryReference); - } - - // No applicable command description found, return false - return null; + baseCommands = Arrays.asList( + AUTHME_BASE, + loginBaseCommand, + logoutBaseCommand, + registerBaseCommand, + unregisterBaseCommand, + changePasswordBaseCommand, + emailBaseCommand, + captchaBaseCommand, + converterBaseCommand); } } diff --git a/src/main/java/fr/xephi/authme/command/help/HelpProvider.java b/src/main/java/fr/xephi/authme/command/help/HelpProvider.java index ce4e83173..b53bba6ed 100644 --- a/src/main/java/fr/xephi/authme/command/help/HelpProvider.java +++ b/src/main/java/fr/xephi/authme/command/help/HelpProvider.java @@ -37,9 +37,9 @@ public class HelpProvider { */ public static void showHelp(CommandSender sender, CommandParts reference, CommandParts helpQuery, boolean showCommand, boolean showDescription, boolean showArguments, boolean showPermissions, boolean showAlternatives, boolean showCommands) { // Find the command for this help query, one with and one without a prefixed base command - FoundCommandResult result = AuthMe.getInstance().getCommandHandler().getCommandManager().findCommand(new CommandParts(helpQuery.getList())); + FoundCommandResult result = AuthMe.getInstance().getCommandHandler().findCommand(new CommandParts(helpQuery.getList())); CommandParts commandReferenceOther = new CommandParts(reference.get(0), helpQuery.getList()); - FoundCommandResult resultOther = AuthMe.getInstance().getCommandHandler().getCommandManager().findCommand(commandReferenceOther); + FoundCommandResult resultOther = AuthMe.getInstance().getCommandHandler().findCommand(commandReferenceOther); if (resultOther != null) { if (result == null) result = resultOther; diff --git a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java index 1cff42a5c..04f08434f 100644 --- a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java +++ b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java @@ -260,6 +260,7 @@ public class PermissionsManager { * * @param event Event instance. */ + // TODO ljacqu 20151129: Misleading name since onPluginEnable is a typical event-based method name public void onPluginEnable(PluginEnableEvent event) { // Get the plugin and it's name Plugin plugin = event.getPlugin(); diff --git a/src/test/java/fr/xephi/authme/command/CommandManagerTest.java b/src/test/java/fr/xephi/authme/command/CommandManagerTest.java index c15b203d7..ff6a1a382 100644 --- a/src/test/java/fr/xephi/authme/command/CommandManagerTest.java +++ b/src/test/java/fr/xephi/authme/command/CommandManagerTest.java @@ -18,7 +18,7 @@ import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; /** - * Test for {@link CommandManager}, especially to guarantee the integrity of the defined commands. + * Test for {@link CommandInitializer} to guarantee the integrity of the defined commands. */ public class CommandManagerTest { @@ -28,24 +28,20 @@ public class CommandManagerTest { */ private static int MAX_ALLOWED_DEPTH = 1; - private static CommandManager manager; + private static List commands; @BeforeClass public static void initializeCommandManager() { - manager = new CommandManager(true); + commands = CommandInitializer.getBaseCommands(); } @Test public void shouldInitializeCommands() { - // given/when - int commandCount = manager.getCommandDescriptionCount(); - List commands = manager.getCommandDescriptions(); - - // then + // given/when/then // It obviously doesn't make sense to test much of the concrete data // that is being initialized; we just want to guarantee with this test // that data is indeed being initialized and we take a few "probes" - assertThat(commandCount, equalTo(9)); + assertThat(commands.size(), equalTo(9)); assertThat(commandsIncludeLabel(commands, "authme"), equalTo(true)); assertThat(commandsIncludeLabel(commands, "register"), equalTo(true)); assertThat(commandsIncludeLabel(commands, "help"), equalTo(false)); @@ -62,7 +58,7 @@ public class CommandManagerTest { }; // when/then - walkThroughCommands(manager.getCommandDescriptions(), descriptionTester); + walkThroughCommands(commands, descriptionTester); } /** Ensure that all children of a command stored the parent. */ @@ -84,7 +80,7 @@ public class CommandManagerTest { }; // when/then - walkThroughCommands(manager.getCommandDescriptions(), connectionTester); + walkThroughCommands(commands, connectionTester); } @Test @@ -105,7 +101,7 @@ public class CommandManagerTest { }; // when/then - walkThroughCommands(manager.getCommandDescriptions(), uniqueMappingTester); + walkThroughCommands(commands, uniqueMappingTester); } /** @@ -132,7 +128,7 @@ public class CommandManagerTest { }; // when/then - walkThroughCommands(manager.getCommandDescriptions(), descriptionTester); + walkThroughCommands(commands, descriptionTester); } /** @@ -143,7 +139,6 @@ public class CommandManagerTest { public void shouldNotHaveMultipleInstancesOfSameExecutableCommandSubType() { // given final Map, ExecutableCommand> implementations = new HashMap<>(); - CommandManager manager = new CommandManager(true); BiConsumer descriptionTester = new BiConsumer() { @Override public void accept(CommandDescription command, int depth) { @@ -160,10 +155,7 @@ public class CommandManagerTest { } }; - // when - List commands = manager.getCommandDescriptions(); - - // then + // when/then walkThroughCommands(commands, descriptionTester); } @@ -186,7 +178,7 @@ public class CommandManagerTest { }; // when/then - walkThroughCommands(manager.getCommandDescriptions(), argumentOrderTester); + walkThroughCommands(commands, argumentOrderTester); } /** @@ -209,7 +201,7 @@ public class CommandManagerTest { }; // when/then - walkThroughCommands(manager.getCommandDescriptions(), noArgumentForParentChecker); + walkThroughCommands(commands, noArgumentForParentChecker); } From a4c45e126e92f1e1a2f86869db072cb0594901cc Mon Sep 17 00:00:00 2001 From: ljacqu Date: Mon, 30 Nov 2015 21:09:52 +0100 Subject: [PATCH 02/10] 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(" ..."); } From 44eef346b90bc32521082cf2e3bfe4971c662ef0 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Mon, 30 Nov 2015 21:18:58 +0100 Subject: [PATCH 03/10] Add test to verify the format of command labels - Fix test class throwing NPE when run isolation -> attempts to get Logger from Wrapper --- ...rTest.java => CommandInitializerTest.java} | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) rename src/test/java/fr/xephi/authme/command/{CommandManagerTest.java => CommandInitializerTest.java} (91%) diff --git a/src/test/java/fr/xephi/authme/command/CommandManagerTest.java b/src/test/java/fr/xephi/authme/command/CommandInitializerTest.java similarity index 91% rename from src/test/java/fr/xephi/authme/command/CommandManagerTest.java rename to src/test/java/fr/xephi/authme/command/CommandInitializerTest.java index ff6a1a382..2cc424c01 100644 --- a/src/test/java/fr/xephi/authme/command/CommandManagerTest.java +++ b/src/test/java/fr/xephi/authme/command/CommandInitializerTest.java @@ -1,6 +1,7 @@ package fr.xephi.authme.command; import fr.xephi.authme.util.StringUtils; +import fr.xephi.authme.util.WrapperMock; import org.junit.BeforeClass; import org.junit.Test; @@ -10,6 +11,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.regex.Pattern; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.not; @@ -20,7 +22,7 @@ import static org.junit.Assert.fail; /** * Test for {@link CommandInitializer} to guarantee the integrity of the defined commands. */ -public class CommandManagerTest { +public class CommandInitializerTest { /** * Defines the maximum allowed depths for nesting CommandDescription instances. @@ -32,6 +34,7 @@ public class CommandManagerTest { @BeforeClass public static void initializeCommandManager() { + WrapperMock.createInstance(); commands = CommandInitializer.getBaseCommands(); } @@ -83,6 +86,27 @@ public class CommandManagerTest { walkThroughCommands(commands, connectionTester); } + @Test + public void shouldUseProperLowerCaseLabels() { + // given + final Pattern invalidPattern = Pattern.compile("\\s"); + BiConsumer labelFormatTester = new BiConsumer() { + @Override + public void accept(CommandDescription command, int depth) { + for (String label : command.getLabels()) { + if (!label.equals(label.toLowerCase())) { + fail("Label '" + label + "' should be lowercase"); + } else if (invalidPattern.matcher(label).matches()) { + fail("Label '" + label + "' has whitespace"); + } + } + } + }; + + // when/then + walkThroughCommands(commands, labelFormatTester); + } + @Test public void shouldNotDefineSameLabelTwice() { // given From 54d8ede5bce24ef7955453ffdd822ebba64158df Mon Sep 17 00:00:00 2001 From: ljacqu Date: Tue, 1 Dec 2015 20:44:44 +0100 Subject: [PATCH 04/10] Add changes missed during merge --- .../java/fr/xephi/authme/Log4JFilter.java | 18 ++++++-------- .../authme/command/CommandInitializer.java | 24 +++++++------------ .../java/fr/xephi/authme/Log4JFilterTest.java | 2 -- 3 files changed, 15 insertions(+), 29 deletions(-) diff --git a/src/main/java/fr/xephi/authme/Log4JFilter.java b/src/main/java/fr/xephi/authme/Log4JFilter.java index 581699f08..e0eac14e0 100644 --- a/src/main/java/fr/xephi/authme/Log4JFilter.java +++ b/src/main/java/fr/xephi/authme/Log4JFilter.java @@ -3,6 +3,7 @@ package fr.xephi.authme; import fr.xephi.authme.util.StringUtils; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.Marker; +import org.apache.logging.log4j.core.Filter; import org.apache.logging.log4j.core.LogEvent; import org.apache.logging.log4j.core.Logger; import org.apache.logging.log4j.message.Message; @@ -11,9 +12,8 @@ import org.apache.logging.log4j.message.Message; * Implements a filter for Log4j to skip sensitive AuthMe commands. * * @author Xephi59 - * @version $Revision: 1.0 $ */ -public class Log4JFilter implements org.apache.logging.log4j.core.Filter { +public class Log4JFilter implements Filter { /** * List of commands (lower-case) to skip. @@ -30,8 +30,7 @@ public class Log4JFilter implements org.apache.logging.log4j.core.Filter { /** * Validates a Message instance and returns the {@link Result} value - * depending depending on whether the message contains sensitive AuthMe - * data. + * depending on whether the message contains sensitive AuthMe data. * * @param message the Message object to verify * @@ -46,7 +45,7 @@ public class Log4JFilter implements org.apache.logging.log4j.core.Filter { /** * Validates a message and returns the {@link Result} value depending - * depending on whether the message contains sensitive AuthMe data. + * on whether the message contains sensitive AuthMe data. * * @param message the message to verify * @@ -74,14 +73,12 @@ public class Log4JFilter implements org.apache.logging.log4j.core.Filter { } @Override - public Result filter(Logger arg0, Level arg1, Marker arg2, String message, - Object... arg4) { + public Result filter(Logger arg0, Level arg1, Marker arg2, String message, Object... arg4) { return validateMessage(message); } @Override - public Result filter(Logger arg0, Level arg1, Marker arg2, Object message, - Throwable arg4) { + public Result filter(Logger arg0, Level arg1, Marker arg2, Object message, Throwable arg4) { if (message == null) { return Result.NEUTRAL; } @@ -89,8 +86,7 @@ public class Log4JFilter implements org.apache.logging.log4j.core.Filter { } @Override - public Result filter(Logger arg0, Level arg1, Marker arg2, Message message, - Throwable arg4) { + public Result filter(Logger arg0, Level arg1, Marker arg2, Message message, Throwable arg4) { return validateMessage(message); } diff --git a/src/main/java/fr/xephi/authme/command/CommandInitializer.java b/src/main/java/fr/xephi/authme/command/CommandInitializer.java index 132c416e0..04658f5ce 100644 --- a/src/main/java/fr/xephi/authme/command/CommandInitializer.java +++ b/src/main/java/fr/xephi/authme/command/CommandInitializer.java @@ -87,11 +87,9 @@ public final class CommandInitializer { .labels("register", "reg", "r") .description("Register a player") .detailedDescription("Register the specified player with the specified password.") - .parent(authMeBaseCommand) - .permissions(OP_ONLY, PlayerPermission.REGISTER) .withArgument("player", "Player name", false) .withArgument("password", "Password", false) - .permissions(OP_ONLY, UserPermission.REGISTER) + .permissions(OP_ONLY, PlayerPermission.REGISTER) .executableCommand(new RegisterCommand()) .build(); @@ -101,10 +99,8 @@ public final class CommandInitializer { .labels("unregister", "unreg", "unr") .description("Unregister a player") .detailedDescription("Unregister the specified player.") - .parent(authMeBaseCommand) - .permissions(OP_ONLY, PlayerPermission.UNREGISTER) .withArgument("player", "Player name", false) - .permissions(OP_ONLY, UserPermission.UNREGISTER) + .permissions(OP_ONLY, PlayerPermission.UNREGISTER) .executableCommand(new UnregisterCommand()) .build(); @@ -114,10 +110,8 @@ public final class CommandInitializer { .labels("forcelogin", "login") .description("Enforce login player") .detailedDescription("Enforce the specified player to login.") - .parent(authMeBaseCommand) - .permissions(OP_ONLY, PlayerPermission.CAN_LOGIN_BE_FORCED) .withArgument("player", "Online player name", true) - .permissions(OP_ONLY, UserPermission.CAN_LOGIN_BE_FORCED) + .permissions(OP_ONLY, PlayerPermission.CAN_LOGIN_BE_FORCED) .executableCommand(new ForceLoginCommand()) .build(); @@ -127,11 +121,9 @@ public final class CommandInitializer { .labels("password", "changepassword", "changepass", "cp") .description("Change a player's password") .detailedDescription("Change the password of a player.") - .parent(authMeBaseCommand) - .permissions(OP_ONLY, PlayerPermission.CHANGE_PASSWORD) .withArgument("player", "Player name", false) .withArgument("pwd", "New password", false) - .permissions(OP_ONLY, UserPermission.CHANGE_PASSWORD) + .permissions(OP_ONLY, PlayerPermission.CHANGE_PASSWORD) .executableCommand(new ChangePasswordCommand()) .build(); @@ -158,23 +150,23 @@ public final class CommandInitializer { .build(); // Register the getemail command - CommandDescription getEmailCommand = CommandDescription.builder() - .executableCommand(new GetEmailCommand()) + CommandDescription.builder() + .parent(AUTHME_BASE) .labels("getemail", "getmail", "email", "mail") .description("Display player's email") .detailedDescription("Display the email address of the specified player if set.") - .parent(authMeBaseCommand) .permissions(OP_ONLY, AdminPermission.GET_EMAIL) .withArgument("player", "Player name", true) + .executableCommand(new GetEmailCommand()) .build(); // Register the setemail command CommandDescription setEmailCommand = CommandDescription.builder() .executableCommand(new SetEmailCommand()) + .parent(AUTHME_BASE) .labels("chgemail", "chgmail", "setemail", "setmail") .description("Change player's email") .detailedDescription("Change the email address of the specified player.") - .parent(authMeBaseCommand) .permissions(OP_ONLY, AdminPermission.CHANGE_EMAIL) .withArgument("player", "Player name", false) .withArgument("email", "Player email", false) diff --git a/src/test/java/fr/xephi/authme/Log4JFilterTest.java b/src/test/java/fr/xephi/authme/Log4JFilterTest.java index f8d185d34..466bc0da2 100644 --- a/src/test/java/fr/xephi/authme/Log4JFilterTest.java +++ b/src/test/java/fr/xephi/authme/Log4JFilterTest.java @@ -12,8 +12,6 @@ import org.mockito.Mockito; /** * Test for {@link Log4JFilter}. - * @author Gabriele - * @version $Revision: 1.0 $ */ public class Log4JFilterTest { From 1ca6bcffe1fa94399edebab74a4114746ab14e72 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Wed, 2 Dec 2015 22:13:43 +0100 Subject: [PATCH 05/10] Make AuthMe.management private; create test for CaptchaCommand --- src/main/java/fr/xephi/authme/AuthMe.java | 2 +- src/main/java/fr/xephi/authme/api/API.java | 2 +- src/main/java/fr/xephi/authme/api/NewAPI.java | 8 +-- .../executable/authme/ForceLoginCommand.java | 2 +- .../executable/captcha/CaptchaCommand.java | 6 +- .../unregister/UnregisterCommand.java | 2 +- .../authme/listener/AuthMePlayerListener.java | 4 +- .../authme/process/join/AsynchronousJoin.java | 2 +- .../captcha/CaptchaCommandTest.java | 71 +++++++++++++++++++ 9 files changed, 86 insertions(+), 13 deletions(-) create mode 100644 src/test/java/fr/xephi/authme/command/executable/captcha/CaptchaCommandTest.java diff --git a/src/main/java/fr/xephi/authme/AuthMe.java b/src/main/java/fr/xephi/authme/AuthMe.java index c1d4a4d69..9fb59f289 100644 --- a/src/main/java/fr/xephi/authme/AuthMe.java +++ b/src/main/java/fr/xephi/authme/AuthMe.java @@ -73,7 +73,7 @@ public class AuthMe extends JavaPlugin { private static Server server; private static Wrapper wrapper = Wrapper.getInstance(); - public Management management; + private Management management; public NewAPI api; public SendMailSSL mail; public DataManager dataManager; diff --git a/src/main/java/fr/xephi/authme/api/API.java b/src/main/java/fr/xephi/authme/api/API.java index e849e7950..1f1ac69d8 100644 --- a/src/main/java/fr/xephi/authme/api/API.java +++ b/src/main/java/fr/xephi/authme/api/API.java @@ -170,7 +170,7 @@ public class API { */ @Deprecated public static void forceLogin(Player player) { - instance.management.performLogin(player, "dontneed", true); + instance.getManagement().performLogin(player, "dontneed", true); } /** diff --git a/src/main/java/fr/xephi/authme/api/NewAPI.java b/src/main/java/fr/xephi/authme/api/NewAPI.java index cfe4508f0..908f75df0 100644 --- a/src/main/java/fr/xephi/authme/api/NewAPI.java +++ b/src/main/java/fr/xephi/authme/api/NewAPI.java @@ -170,7 +170,7 @@ public class NewAPI { * @param player * player */ public void forceLogin(Player player) { - plugin.management.performLogin(player, "dontneed", true); + plugin.getManagement().performLogin(player, "dontneed", true); } /** @@ -179,7 +179,7 @@ public class NewAPI { * @param player * player */ public void forceLogout(Player player) { - plugin.management.performLogout(player); + plugin.getManagement().performLogout(player); } /** @@ -189,7 +189,7 @@ public class NewAPI { * @param password String */ public void forceRegister(Player player, String password) { - plugin.management.performRegister(player, password, null); + plugin.getManagement().performRegister(player, password, null); } /** @@ -198,6 +198,6 @@ public class NewAPI { * @param player * player */ public void forceUnregister(Player player) { - plugin.management.performUnregister(player, "", true); + plugin.getManagement().performUnregister(player, "", true); } } diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/ForceLoginCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/ForceLoginCommand.java index b5fa337eb..f6f3efef5 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/ForceLoginCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/ForceLoginCommand.java @@ -34,7 +34,7 @@ public class ForceLoginCommand extends ExecutableCommand { sender.sendMessage("You cannot force login for the player " + playerName + "!"); return true; } - plugin.management.performLogin(player, "dontneed", true); + plugin.getManagement().performLogin(player, "dontneed", true); sender.sendMessage("Force Login for " + playerName + " performed!"); } catch (Exception e) { sender.sendMessage("An error occurred while trying to get that player!"); diff --git a/src/main/java/fr/xephi/authme/command/executable/captcha/CaptchaCommand.java b/src/main/java/fr/xephi/authme/command/executable/captcha/CaptchaCommand.java index 4fa2bbef9..696773537 100644 --- a/src/main/java/fr/xephi/authme/command/executable/captcha/CaptchaCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/captcha/CaptchaCommand.java @@ -8,6 +8,7 @@ import fr.xephi.authme.security.RandomString; import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.output.Messages; import fr.xephi.authme.settings.Settings; +import fr.xephi.authme.util.Wrapper; import org.bukkit.command.CommandSender; import org.bukkit.entity.Player; @@ -30,10 +31,11 @@ public class CaptchaCommand extends ExecutableCommand { String captcha = commandArguments.get(0); // AuthMe plugin instance - final AuthMe plugin = AuthMe.getInstance(); + final Wrapper wrapper = Wrapper.getInstance(); + final AuthMe plugin = wrapper.getAuthMe(); // Messages instance - final Messages m = plugin.getMessages(); + final Messages m = wrapper.getMessages(); // Command logic if (PlayerCache.getInstance().isAuthenticated(playerNameLowerCase)) { diff --git a/src/main/java/fr/xephi/authme/command/executable/unregister/UnregisterCommand.java b/src/main/java/fr/xephi/authme/command/executable/unregister/UnregisterCommand.java index 22ef35661..8ff40ed3d 100644 --- a/src/main/java/fr/xephi/authme/command/executable/unregister/UnregisterCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/unregister/UnregisterCommand.java @@ -49,7 +49,7 @@ public class UnregisterCommand extends ExecutableCommand { } // Unregister the player - plugin.management.performUnregister(player, playerPass, false); + plugin.getManagement().performUnregister(player, playerPass, false); return true; } } diff --git a/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java b/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java index 773ed8d5d..52fc51ab9 100644 --- a/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java +++ b/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java @@ -333,7 +333,7 @@ public class AuthMePlayerListener implements Listener { event.setQuitMessage(null); } - plugin.management.performQuit(player, false); + plugin.getManagement().performQuit(player, false); } @EventHandler(ignoreCancelled = true, priority = EventPriority.MONITOR) @@ -349,7 +349,7 @@ public class AuthMePlayerListener implements Listener { } Player player = event.getPlayer(); - plugin.management.performQuit(player, true); + plugin.getManagement().performQuit(player, true); } @EventHandler(ignoreCancelled = true, priority = EventPriority.HIGHEST) diff --git a/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java b/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java index 3160ddcc4..724c334fc 100644 --- a/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java +++ b/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java @@ -238,7 +238,7 @@ public class AsynchronousJoin { PlayerCache.getInstance().removePlayer(name); if (auth != null && auth.getIp().equals(ip)) { m.send(player, MessageKey.SESSION_RECONNECTION); - plugin.management.performLogin(player, "dontneed", true); + plugin.getManagement().performLogin(player, "dontneed", true); return; } else if (Settings.sessionExpireOnIpChange) { m.send(player, MessageKey.SESSION_EXPIRED); diff --git a/src/test/java/fr/xephi/authme/command/executable/captcha/CaptchaCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/captcha/CaptchaCommandTest.java new file mode 100644 index 000000000..36169db05 --- /dev/null +++ b/src/test/java/fr/xephi/authme/command/executable/captcha/CaptchaCommandTest.java @@ -0,0 +1,71 @@ +package fr.xephi.authme.command.executable.captcha; + +import fr.xephi.authme.AuthMe; +import fr.xephi.authme.command.CommandParts; +import fr.xephi.authme.command.ExecutableCommand; +import fr.xephi.authme.output.MessageKey; +import fr.xephi.authme.output.Messages; +import fr.xephi.authme.settings.Settings; +import fr.xephi.authme.util.WrapperMock; +import org.bukkit.command.BlockCommandSender; +import org.bukkit.command.CommandSender; +import org.bukkit.entity.Player; +import org.junit.Before; +import org.junit.Ignore; +import org.junit.Test; +import org.mockito.Mockito; + +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThat; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Test for {@link CaptchaCommand}. + */ +public class CaptchaCommandTest { + + private WrapperMock wrapperMock; + + @Before + public void setUpWrapperMock() { + wrapperMock = WrapperMock.createInstance(); + Settings.useCaptcha = true; + } + + @Test + public void shouldRejectNonPlayerSender() { + // given + CommandSender sender = Mockito.mock(BlockCommandSender.class); + ExecutableCommand command = new CaptchaCommand(); + + // when + boolean result = command.executeCommand(sender, new CommandParts(), new CommandParts()); + + // then + assertThat(result, equalTo(true)); + assertThat(wrapperMock.wasMockCalled(AuthMe.class), equalTo(false)); + assertThat(wrapperMock.wasMockCalled(Messages.class), equalTo(false)); + } + + @Test + @Ignore + public void shouldRejectIfCaptchaIsNotUsed() { + // given + Player player = mockPlayerWithName("testplayer"); + ExecutableCommand command = new CaptchaCommand(); + + // when + boolean result = command.executeCommand(player, new CommandParts(), new CommandParts()); + + // then + assertThat(result, equalTo(true)); + verify(wrapperMock.getMessages()).send(player, MessageKey.USAGE_LOGIN); + } + + private static Player mockPlayerWithName(String name) { + Player player = Mockito.mock(Player.class); + when(player.getName()).thenReturn(name); + return player; + } +} From c78e12de046c331bc3947ed335a195d10d505183 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Fri, 4 Dec 2015 21:33:50 +0100 Subject: [PATCH 06/10] Refactorings - prepare to remove FoundCommandResult - Reduce tight coupling by passing list of available commands as param to CommandHandler - Split command processing method into several smaller ones - Remove unfinished logic for unlimited command arguments (reason: was not implemented and not used, probably needs another way to handle it once the need for it arises) - Create test for CommandHandler --- src/main/java/fr/xephi/authme/AuthMe.java | 11 +- .../authme/command/CommandDescription.java | 32 +--- .../xephi/authme/command/CommandHandler.java | 171 ++++++++++-------- .../fr/xephi/authme/command/CommandUtils.java | 4 +- .../authme/command/FoundCommandResult.java | 16 +- .../authme/command/help/HelpPrinter.java | 5 - .../authme/command/help/HelpSyntaxHelper.java | 5 - .../authme/command/CommandHandlerTest.java | 115 ++++++++++++ .../command/help/HelpSyntaxHelperTest.java | 37 +--- 9 files changed, 223 insertions(+), 173 deletions(-) create mode 100644 src/test/java/fr/xephi/authme/command/CommandHandlerTest.java diff --git a/src/main/java/fr/xephi/authme/AuthMe.java b/src/main/java/fr/xephi/authme/AuthMe.java index 9fb59f289..655a1f33b 100644 --- a/src/main/java/fr/xephi/authme/AuthMe.java +++ b/src/main/java/fr/xephi/authme/AuthMe.java @@ -10,6 +10,7 @@ import fr.xephi.authme.cache.backup.JsonCache; import fr.xephi.authme.cache.limbo.LimboCache; import fr.xephi.authme.cache.limbo.LimboPlayer; import fr.xephi.authme.command.CommandHandler; +import fr.xephi.authme.command.CommandInitializer; import fr.xephi.authme.converter.Converter; import fr.xephi.authme.converter.ForceFlatToSqlite; import fr.xephi.authme.datasource.*; @@ -433,7 +434,7 @@ public class AuthMe extends JavaPlugin { * Set up the command handler. */ private void setupCommandHandler() { - this.commandHandler = new CommandHandler(); + this.commandHandler = new CommandHandler(CommandInitializer.getBaseCommands()); } /** @@ -956,14 +957,14 @@ public class AuthMe extends JavaPlugin { @Override public boolean onCommand(CommandSender sender, Command cmd, String commandLabel, String[] args) { - // Get the command handler, and make sure it's valid + // Make sure the command handler has been initialized if (commandHandler == null) { - wrapper.getLogger().warning("AuthMe command handler is not available"); + wrapper.getLogger().severe("AuthMe command handler is not available"); return false; } - // Handle the command, return the result - return commandHandler.onCommand(sender, cmd, commandLabel, args); + // Handle the command + return commandHandler.processCommand(sender, 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 34a265ee8..faeb0d746 100644 --- a/src/main/java/fr/xephi/authme/command/CommandDescription.java +++ b/src/main/java/fr/xephi/authme/command/CommandDescription.java @@ -50,10 +50,6 @@ public class CommandDescription { * The arguments the command takes. */ private List arguments = new ArrayList<>(); // TODO remove field initialization - /** - * Defines whether there is an argument maximum or not. - */ - private boolean noArgumentMaximum = false; // TODO remove field initialization /** * Defines the command permissions. */ @@ -106,15 +102,13 @@ public class CommandDescription { */ private CommandDescription(List labels, String description, String detailedDescription, ExecutableCommand executableCommand, CommandDescription parent, - List arguments, boolean noArgumentMaximum, - CommandPermissions permissions) { + List arguments, CommandPermissions permissions) { this.labels = labels; this.description = description; this.detailedDescription = detailedDescription; this.executableCommand = executableCommand; this.parent = parent; this.arguments = arguments; - this.noArgumentMaximum = noArgumentMaximum; this.permissions = permissions; if (parent != null) { @@ -279,15 +273,6 @@ public class CommandDescription { this.executableCommand = executableCommand; } - /** - * Check whether this command is executable, based on the assigned executable command. - * - * @return True if this command is executable. - */ - public boolean isExecutable() { - return this.executableCommand != null; - } - /** * Execute the command, if possible. * @@ -298,10 +283,6 @@ public class CommandDescription { * @return True on success, false on failure. */ public boolean execute(CommandSender sender, CommandParts commandReference, CommandParts commandArguments) { - // Make sure the command is executable - if (!isExecutable()) - return false; - // Execute the command, return the result return getExecutableCommand().executeCommand(sender, commandReference, commandArguments); } @@ -453,10 +434,6 @@ public class CommandDescription { return !getArguments().isEmpty(); } - public boolean hasMaximumArguments() { - return !noArgumentMaximum; // TODO ljacqu 20151130 Change variable name - } - /** * Get the command description. * @@ -604,7 +581,6 @@ public class CommandDescription { private ExecutableCommand executableCommand; private CommandDescription parent; private List arguments = new ArrayList<>(); - private boolean noArgumentMaximum; private CommandPermissions permissions; /** @@ -621,7 +597,6 @@ public class CommandDescription { getOrThrow(executableCommand, "executableCommand"), firstNonNull(parent, null), arguments, - noArgumentMaximum, firstNonNull(permissions, null) ); } @@ -670,11 +645,6 @@ public class CommandDescription { return this; } - public CommandBuilder noArgumentMaximum(boolean noArgumentMaximum) { - this.noArgumentMaximum = noArgumentMaximum; - return this; - } - public CommandBuilder permissions(CommandPermissions.DefaultPermission defaultPermission, PermissionNode... permissionNodes) { this.permissions = new CommandPermissions(asMutableList(permissionNodes), defaultPermission); diff --git a/src/main/java/fr/xephi/authme/command/CommandHandler.java b/src/main/java/fr/xephi/authme/command/CommandHandler.java index 6a6a53207..653a47646 100644 --- a/src/main/java/fr/xephi/authme/command/CommandHandler.java +++ b/src/main/java/fr/xephi/authme/command/CommandHandler.java @@ -4,7 +4,6 @@ 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; @@ -28,104 +27,70 @@ public class CommandHandler { */ private static final double SUGGEST_COMMAND_THRESHOLD = 0.75; + private List commands; + /** - * Process a command. + * Create a command handler. + * + * @param commands The collection of available AuthMe commands + */ + public CommandHandler(List commands) { + this.commands = commands; + } + + /** + * Map a command that was invoked to the proper {@link CommandDescription} or return a useful error + * message upon failure. * * @param sender The command sender (Bukkit). - * @param bukkitCommand The command (Bukkit). * @param bukkitCommandLabel The command label (Bukkit). * @param bukkitArgs The command arguments (Bukkit). * * @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, Command bukkitCommand, String bukkitCommandLabel, String[] bukkitArgs) { + public boolean processCommand(CommandSender sender, String bukkitCommandLabel, String[] bukkitArgs) { List commandArgs = skipEmptyArguments(bukkitArgs); + // Add the Bukkit command label to the front so we get something like [authme, register, pass, passConfirm] + commandArgs.add(0, bukkitCommandLabel); - // Make sure the command isn't empty (does this happen?) - CommandParts commandReference = new CommandParts(bukkitCommandLabel, commandArgs); - if (commandReference.getCount() == 0) - return false; + // TODO: remove commandParts + CommandParts commandReference = new CommandParts(commandArgs); // Get a suitable command for this reference, and make sure it isn't null FoundCommandResult result = findCommand(commandReference); if (result == null) { + // TODO ljacqu 20151204: Log more information to the console (bukkitCommandLabel) sender.sendMessage(ChatColor.DARK_RED + "Failed to parse " + AuthMe.getPluginName() + " command!"); return false; } - // Get the base command - String baseCommand = commandReference.get(0); + + String baseCommand = commandArgs.get(0); // Make sure the difference between the command reference and the actual command isn't too big final double commandDifference = result.getDifference(); - if (commandDifference > ASSUME_COMMAND_THRESHOLD) { - // Show the unknown command warning - sender.sendMessage(ChatColor.DARK_RED + "Unknown command!"); + if (commandDifference <= ASSUME_COMMAND_THRESHOLD) { - // Show a command suggestion if available and the difference isn't too big - if (commandDifference < SUGGEST_COMMAND_THRESHOLD) - if (result.getCommandDescription() != null) - sender.sendMessage(ChatColor.YELLOW + "Did you mean " + ChatColor.GOLD + "/" + result.getCommandDescription().getCommandReference(commandReference) + ChatColor.YELLOW + "?"); + // Show a message when the command handler is assuming a command + if (commandDifference > 0) { + sendCommandAssumptionMessage(sender, result, commandReference); + } - // Show the help command - sender.sendMessage(ChatColor.YELLOW + "Use the command " + ChatColor.GOLD + "/" + baseCommand + " help" + ChatColor.YELLOW + " to view help."); - return true; + if (!result.hasPermission(sender)) { + sender.sendMessage(ChatColor.DARK_RED + "You don't have permission to use this command!"); + } else if (!result.hasProperArguments()) { + sendImproperArgumentsMessage(sender, result, commandReference, baseCommand); + } else { + return result.executeCommand(sender); + } + } else { + sendUnknownCommandMessage(sender, commandDifference, result, baseCommand); } - - // Show a message when the command handler is assuming a command - if (commandDifference > 0) { - // Get the suggested command - CommandParts suggestedCommandParts = new CommandParts(result.getCommandDescription().getCommandReference(commandReference)); - - // Show the suggested command - sender.sendMessage(ChatColor.DARK_RED + "Unknown command, assuming " + ChatColor.GOLD + "/" + suggestedCommandParts + - ChatColor.DARK_RED + "!"); - } - - // Make sure the command is executable - if (!result.isExecutable()) { - // Get the command reference - CommandParts helpCommandReference = new CommandParts(result.getCommandReference().getRange(1)); - - // Show the unknown command warning - sender.sendMessage(ChatColor.DARK_RED + "Invalid command!"); - - // Show the help command - sender.sendMessage(ChatColor.YELLOW + "Use the command " + ChatColor.GOLD + "/" + baseCommand + " help " + helpCommandReference + ChatColor.YELLOW + " to view help."); - return true; - } - - // Make sure the command sender has permission - if (!result.hasPermission(sender)) { - // Show the no permissions warning - sender.sendMessage(ChatColor.DARK_RED + "You don't have permission to use this command!"); - return true; - } - - // Make sure the command sender has permission - if (!result.hasProperArguments()) { - // Get the command and the suggested command reference - CommandParts suggestedCommandReference = new CommandParts(result.getCommandDescription().getCommandReference(commandReference)); - CommandParts helpCommandReference = new CommandParts(suggestedCommandReference.getRange(1)); - - // Show the invalid arguments warning - sender.sendMessage(ChatColor.DARK_RED + "Incorrect command arguments!"); - - // Show the command argument help - HelpProvider.showHelp(sender, commandReference, suggestedCommandReference, true, false, true, false, false, false); - - // Show the command to use for detailed help - sender.sendMessage(ChatColor.GOLD + "Detailed help: " + ChatColor.WHITE + "/" + baseCommand + " help " + helpCommandReference); - return true; - } - - // Execute the command if it's suitable - return result.executeCommand(sender); + return true; } /** - * Skips all entries of the given array that are simply whitespace. + * Skip all entries of the given array that are simply whitespace. * * @param args The array to process * @return List of the items that are not empty @@ -162,10 +127,7 @@ public class CommandHandler { if (queryReference.getCount() <= 0) return null; - // TODO ljacqu 20151129: If base commands are only used in here (or in the future CommandHandler after changes), - // it might make sense to make the CommandInitializer package-private and to return its result into this class - // instead of regularly fetching the list of base commands from the other class. - for (CommandDescription commandDescription : CommandInitializer.getBaseCommands()) { + for (CommandDescription commandDescription : commands) { // Check whether there's a command description available for the // current command if (!commandDescription.isSuitableLabel(queryReference)) @@ -186,7 +148,7 @@ public class CommandHandler { * * @return The command found, or null. */ - public CommandDescription findCommand(List commandParts) { + private CommandDescription findCommand(List commandParts) { // Make sure the command reference is valid if (commandParts.isEmpty()) { return null; @@ -211,10 +173,63 @@ public class CommandHandler { return null; } for (CommandDescription command : commands) { - if (command.getLabels().contains(label)) { + if (command.getLabels().contains(label)) { // TODO ljacqu should be case-insensitive return command; } } return null; } + + /** + * Show an "unknown command" to the user and suggests an existing command if its similarity is within + * the defined threshold. + * + * @param sender The command sender + * @param commandDifference The difference between the invoked command and the existing one + * @param result The command that was found during the mapping process + * @param baseCommand The base command (TODO: This is probably already in FoundCommandResult) + */ + private static void sendUnknownCommandMessage(CommandSender sender, double commandDifference, + FoundCommandResult result, String baseCommand) { + CommandParts commandReference = result.getCommandReference(); + sender.sendMessage(ChatColor.DARK_RED + "Unknown command!"); + + + // Show a command suggestion if available and the difference isn't too big + if (commandDifference < SUGGEST_COMMAND_THRESHOLD && result.getCommandDescription() != null) { + sender.sendMessage(ChatColor.YELLOW + "Did you mean " + ChatColor.GOLD + "/" + + result.getCommandDescription().getCommandReference(commandReference) + ChatColor.YELLOW + "?"); + } + + sender.sendMessage(ChatColor.YELLOW + "Use the command " + ChatColor.GOLD + "/" + baseCommand + " help" + + ChatColor.YELLOW + " to view help."); + } + + private static void sendImproperArgumentsMessage(CommandSender sender, FoundCommandResult result, + CommandParts commandReference, String baseCommand) { + // Get the command and the suggested command reference + CommandParts suggestedCommandReference = + new CommandParts(result.getCommandDescription().getCommandReference(commandReference)); + CommandParts helpCommandReference = new CommandParts(suggestedCommandReference.getRange(1)); + + // Show the invalid arguments warning + sender.sendMessage(ChatColor.DARK_RED + "Incorrect command arguments!"); + + // Show the command argument help + HelpProvider.showHelp(sender, commandReference, suggestedCommandReference, + true, false, true, false, false, false); + + // Show the command to use for detailed help + sender.sendMessage(ChatColor.GOLD + "Detailed help: " + ChatColor.WHITE + "/" + baseCommand + + " help " + helpCommandReference); + } + + private static void sendCommandAssumptionMessage(CommandSender sender, FoundCommandResult result, + CommandParts commandReference) { + CommandParts assumedCommandParts = + new CommandParts(result.getCommandDescription().getCommandReference(commandReference)); + + sender.sendMessage(ChatColor.DARK_RED + "Unknown command, assuming " + ChatColor.GOLD + "/" + + assumedCommandParts + ChatColor.DARK_RED + "!"); + } } diff --git a/src/main/java/fr/xephi/authme/command/CommandUtils.java b/src/main/java/fr/xephi/authme/command/CommandUtils.java index f8e78c46e..6d5e2b551 100644 --- a/src/main/java/fr/xephi/authme/command/CommandUtils.java +++ b/src/main/java/fr/xephi/authme/command/CommandUtils.java @@ -13,9 +13,7 @@ public final class CommandUtils { } public static int getMaxNumberOfArguments(CommandDescription command) { - return command.hasMaximumArguments() - ? command.getArguments().size() - : -1; + return command.getArguments().size(); } } diff --git a/src/main/java/fr/xephi/authme/command/FoundCommandResult.java b/src/main/java/fr/xephi/authme/command/FoundCommandResult.java index d369a8a70..1ee4fbd9d 100644 --- a/src/main/java/fr/xephi/authme/command/FoundCommandResult.java +++ b/src/main/java/fr/xephi/authme/command/FoundCommandResult.java @@ -61,27 +61,13 @@ public class FoundCommandResult { return this.commandDescription; } - /** - * Set the command description. - * - * @param commandDescription The command description. - */ - public void setCommandDescription(CommandDescription commandDescription) { - this.commandDescription = commandDescription; - } - /** * Check whether the command is executable. * * @return True if the command is executable, false otherwise. */ public boolean isExecutable() { - // Make sure the command description is valid - if (this.commandDescription == null) - return false; - - // Check whether the command is executable, return the result - return this.commandDescription.isExecutable(); + return commandDescription != null; } /** 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 a3244c914..7ae189836 100644 --- a/src/main/java/fr/xephi/authme/command/help/HelpPrinter.java +++ b/src/main/java/fr/xephi/authme/command/help/HelpPrinter.java @@ -5,7 +5,6 @@ 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; @@ -77,10 +76,6 @@ public class HelpPrinter { // Print the syntax sender.sendMessage(argString.toString()); } - - // Show the unlimited arguments argument - 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 024914c9a..023e4085b 100644 --- a/src/main/java/fr/xephi/authme/command/help/HelpSyntaxHelper.java +++ b/src/main/java/fr/xephi/authme/command/help/HelpSyntaxHelper.java @@ -61,11 +61,6 @@ public final class HelpSyntaxHelper { sb.append(ChatColor.ITALIC).append(formatArgument(arg)); } - // Add some dots if the command allows unlimited arguments - if (!commandDescription.hasMaximumArguments()) { - sb.append(ChatColor.ITALIC).append(" ..."); - } - // Return the build command syntax return sb.toString(); } diff --git a/src/test/java/fr/xephi/authme/command/CommandHandlerTest.java b/src/test/java/fr/xephi/authme/command/CommandHandlerTest.java new file mode 100644 index 000000000..99358ef55 --- /dev/null +++ b/src/test/java/fr/xephi/authme/command/CommandHandlerTest.java @@ -0,0 +1,115 @@ +package fr.xephi.authme.command; + +import fr.xephi.authme.permission.PlayerPermission; +import fr.xephi.authme.util.WrapperMock; +import org.bukkit.command.CommandSender; +import org.junit.BeforeClass; +import org.junit.Ignore; +import org.junit.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Mockito; + +import java.util.List; + +import static java.util.Arrays.asList; +import static java.util.Collections.singletonList; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.stringContainsInOrder; +import static org.mockito.BDDMockito.given; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +/** + * Test for {@link CommandHandler}. + */ +public class CommandHandlerTest { + + private static List commands; + private static CommandHandler handler; + + @BeforeClass + public static void setUpCommandHandler() { + WrapperMock.createInstance(); + + CommandDescription authMeBase = createCommand(null, null, singletonList("authme")); + createCommand(PlayerPermission.LOGIN, authMeBase, singletonList("login"), newArgument("password", false)); + createCommand(PlayerPermission.LOGIN, authMeBase, asList("register", "reg"), + newArgument("password", false), newArgument("confirmation", false)); + + CommandDescription testBase = createCommand(null, null, singletonList("test"), newArgument("test", true)); + commands = asList(authMeBase, testBase); + handler = new CommandHandler(commands); + } + + @Test + public void shouldForwardCommandToExecutable() { + // given + CommandSender sender = Mockito.mock(CommandSender.class); + given(sender.isOp()).willReturn(true); + String bukkitLabel = "authme"; + String[] args = {"login", "password"}; + + // when + handler.processCommand(sender, bukkitLabel, args); + + // then + final CommandDescription loginCmd = commands.get(0).getChildren().get(0); + verify(sender, never()).sendMessage(anyString()); + verify(loginCmd.getExecutableCommand()).executeCommand( + eq(sender), any(CommandParts.class), any(CommandParts.class)); + } + + @Test + @Ignore // TODO ljacqu Fix test --> command classes too tightly coupled at the moment + public void shouldRejectCommandWithTooManyArguments() { + // given + CommandSender sender = Mockito.mock(CommandSender.class); + given(sender.isOp()).willReturn(true); + String bukkitLabel = "authme"; + String[] args = {"login", "password", "__unneededArgument__"}; + + // when + boolean result = handler.processCommand(sender, bukkitLabel, args); + + // then + assertThat(result, equalTo(true)); + final CommandDescription loginCmd = commands.get(0).getChildren().get(0); + assertSenderGotMessageContaining("help", sender); + verify(loginCmd.getExecutableCommand()).executeCommand( + eq(sender), any(CommandParts.class), any(CommandParts.class)); + } + + private static CommandDescription createCommand(PlayerPermission permission, CommandDescription parent, + List labels, CommandArgumentDescription... arguments) { + CommandDescription.CommandBuilder command = CommandDescription.builder() + .labels(labels) + .parent(parent) + .permissions(CommandPermissions.DefaultPermission.OP_ONLY, permission) + .description("Test") + .detailedDescription("Test command") + .executableCommand(mock(ExecutableCommand.class)); + + if (arguments != null && arguments.length > 0) { + for (CommandArgumentDescription argument : arguments) { + command.withArgument(argument.getLabel(), "Test description", argument.isOptional()); + } + } + + return command.build(); + } + + private static CommandArgumentDescription newArgument(String label, boolean isOptional) { + return new CommandArgumentDescription(label, "Test description", isOptional); + } + + private void assertSenderGotMessageContaining(String text, CommandSender sender) { + ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); + verify(sender).sendMessage(captor.capture()); + assertThat(captor.getValue(), stringContainsInOrder(text)); + } +} diff --git a/src/test/java/fr/xephi/authme/command/help/HelpSyntaxHelperTest.java b/src/test/java/fr/xephi/authme/command/help/HelpSyntaxHelperTest.java index 496dd93fb..f9fa9a6a0 100644 --- a/src/test/java/fr/xephi/authme/command/help/HelpSyntaxHelperTest.java +++ b/src/test/java/fr/xephi/authme/command/help/HelpSyntaxHelperTest.java @@ -27,8 +27,7 @@ public class HelpSyntaxHelperTest { .build(); // when - String result = HelpSyntaxHelper.getCommandSyntax( - description, new CommandParts(), "", false); + String result = HelpSyntaxHelper.getCommandSyntax(description, new CommandParts(), "", false); // then assertThat(result, equalTo(WHITE + "/authme register" + ITALIC + " [name]")); @@ -42,8 +41,7 @@ public class HelpSyntaxHelperTest { .build(); // when - String result = HelpSyntaxHelper.getCommandSyntax( - description, new CommandParts(), null, false); + String result = HelpSyntaxHelper.getCommandSyntax(description, new CommandParts(), null, false); // then assertThat(result, equalTo(WHITE + "/authme register" + ITALIC + " ")); @@ -58,8 +56,7 @@ public class HelpSyntaxHelperTest { .build(); // when - String result = HelpSyntaxHelper.getCommandSyntax( - description, new CommandParts(), "", false); + String result = HelpSyntaxHelper.getCommandSyntax(description, new CommandParts(), "", false); // then assertThat(result, equalTo(WHITE + "/authme register" + ITALIC + " [name]" + ITALIC + " ")); @@ -74,8 +71,7 @@ public class HelpSyntaxHelperTest { .build(); // when - String result = HelpSyntaxHelper.getCommandSyntax( - description, new CommandParts(), "", true); + String result = HelpSyntaxHelper.getCommandSyntax(description, new CommandParts(), "", true); // then assertThat(result, equalTo(WHITE + "/authme " @@ -89,8 +85,7 @@ public class HelpSyntaxHelperTest { CommandDescription description = getDescriptionBuilder().build(); // when - String result = HelpSyntaxHelper.getCommandSyntax( - description, new CommandParts(), null, true); + String result = HelpSyntaxHelper.getCommandSyntax(description, new CommandParts(), null, true); // then assertThat(result, equalTo(WHITE + "/authme " + YELLOW + BOLD + "register" + YELLOW)); @@ -104,32 +99,12 @@ public class HelpSyntaxHelperTest { .build(); // when - String result = HelpSyntaxHelper.getCommandSyntax( - description, new CommandParts(), "alt", false); + String result = HelpSyntaxHelper.getCommandSyntax(description, new CommandParts(), "alt", false); // then assertThat(result, equalTo(WHITE + "/authme alt" + ITALIC + " [name]")); } - @Test - public void shouldHighlightCommandWithAltLabelAndUnlimitedArguments() { - // given - CommandDescription description = getDescriptionBuilder() - .withArgument("name", "", true) - .withArgument("test", "", false) - .noArgumentMaximum(true) - .build(); - - // when - String result = HelpSyntaxHelper.getCommandSyntax( - description, new CommandParts(), "test", true); - - // then - assertThat(result, equalTo(WHITE + "/authme " - + YELLOW + BOLD + "test" - + YELLOW + ITALIC + " [name]" + ITALIC + " " + ITALIC + " ...")); - } - private static CommandDescription.CommandBuilder getDescriptionBuilder() { CommandDescription base = CommandDescription.builder() From 9140ebe60293c517988df4f290696969623befef Mon Sep 17 00:00:00 2001 From: ljacqu Date: Fri, 4 Dec 2015 21:58:16 +0100 Subject: [PATCH 07/10] Change initialized command descriptions to Set - Set is more efficient if it's frequently used for `contains()`, which is what we use it for after initialization --- .../xephi/authme/command/CommandHandler.java | 7 +- .../authme/command/CommandInitializer.java | 108 +++++++++--------- .../authme/command/CommandHandlerTest.java | 30 ++++- .../command/CommandInitializerTest.java | 13 +-- 4 files changed, 86 insertions(+), 72 deletions(-) diff --git a/src/main/java/fr/xephi/authme/command/CommandHandler.java b/src/main/java/fr/xephi/authme/command/CommandHandler.java index 653a47646..bb0b1298c 100644 --- a/src/main/java/fr/xephi/authme/command/CommandHandler.java +++ b/src/main/java/fr/xephi/authme/command/CommandHandler.java @@ -8,6 +8,7 @@ import org.bukkit.command.CommandSender; import java.util.ArrayList; import java.util.List; +import java.util.Set; /** * The AuthMe command handler, responsible for mapping incoming commands to the correct {@link CommandDescription} @@ -27,14 +28,14 @@ public class CommandHandler { */ private static final double SUGGEST_COMMAND_THRESHOLD = 0.75; - private List commands; + private final Set commands; /** * Create a command handler. * * @param commands The collection of available AuthMe commands */ - public CommandHandler(List commands) { + public CommandHandler(Set commands) { this.commands = commands; } @@ -50,7 +51,7 @@ public class CommandHandler { */ public boolean processCommand(CommandSender sender, String bukkitCommandLabel, String[] bukkitArgs) { List commandArgs = skipEmptyArguments(bukkitArgs); - // Add the Bukkit command label to the front so we get something like [authme, register, pass, passConfirm] + // Add the Bukkit command label to the front so we get a list like [authme, register, pass, passConfirm] commandArgs.add(0, bukkitCommandLabel); // TODO: remove commandParts diff --git a/src/main/java/fr/xephi/authme/command/CommandInitializer.java b/src/main/java/fr/xephi/authme/command/CommandInitializer.java index ed618e5de..7fcd0f023 100644 --- a/src/main/java/fr/xephi/authme/command/CommandInitializer.java +++ b/src/main/java/fr/xephi/authme/command/CommandInitializer.java @@ -32,9 +32,7 @@ import fr.xephi.authme.permission.AdminPermission; import fr.xephi.authme.permission.PlayerPermission; import fr.xephi.authme.util.Wrapper; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; +import java.util.*; import static fr.xephi.authme.command.CommandPermissions.DefaultPermission.ALLOWED; import static fr.xephi.authme.command.CommandPermissions.DefaultPermission.OP_ONLY; @@ -44,13 +42,13 @@ import static fr.xephi.authme.command.CommandPermissions.DefaultPermission.OP_ON */ public final class CommandInitializer { - private static List baseCommands; + private static Set baseCommands; private CommandInitializer() { // Helper class } - public static List getBaseCommands() { + public static Set getBaseCommands() { if (baseCommands == null) { Wrapper.getInstance().getLogger().info("Initializing AuthMe commands"); initializeCommands(); @@ -287,17 +285,17 @@ public final class CommandInitializer { reloadCommand.setCommandPermissions(AdminPermission.RELOAD, OP_ONLY); // Register the version command - CommandDescription versionCommand = CommandDescription.builder() - .executableCommand(new VersionCommand()) + CommandDescription.builder() + .parent(AUTHME_BASE) .labels("version", "ver", "v", "about", "info") .description("Version info") .detailedDescription("Show detailed information about the installed AuthMeReloaded version, and shows the " + "developers, contributors, license and other information.") - .parent(AUTHME_BASE) + .executableCommand(new VersionCommand()) .build(); // Register the base login command - CommandDescription loginBaseCommand = CommandDescription.builder() + final CommandDescription LOGIN_BASE = CommandDescription.builder() .executableCommand(new LoginCommand()) .labels("login", "l") .description("Login command") @@ -309,70 +307,72 @@ public final class CommandInitializer { // Register the help command CommandDescription loginHelpCommand = new CommandDescription(helpCommandExecutable, helpCommandLabels, - "View help", "View detailed help pages about AuthMeReloaded login commands.", loginBaseCommand); + "View help", "View detailed help pages about AuthMeReloaded login commands.", LOGIN_BASE); loginHelpCommand.addArgument(new CommandArgumentDescription("query", "The command or query to view help for.", true)); // Register the base logout command - CommandDescription logoutBaseCommand = new CommandDescription(new LogoutCommand(), new ArrayList() { + CommandDescription LOGOUT_BASE = new CommandDescription(new LogoutCommand(), new ArrayList() { { add("logout"); } }, "Logout command", "Command to logout using AuthMeReloaded.", null); - logoutBaseCommand.setCommandPermissions(PlayerPermission.LOGOUT, CommandPermissions.DefaultPermission.ALLOWED); + LOGOUT_BASE.setCommandPermissions(PlayerPermission.LOGOUT, CommandPermissions.DefaultPermission.ALLOWED); // Register the help command CommandDescription logoutHelpCommand = new CommandDescription(helpCommandExecutable, helpCommandLabels, - "View help", "View detailed help pages about AuthMeReloaded logout commands.", logoutBaseCommand); + "View help", "View detailed help pages about AuthMeReloaded logout commands.", LOGOUT_BASE); logoutHelpCommand.addArgument(new CommandArgumentDescription("query", "The command or query to view help for.", true)); // Register the base register command - CommandDescription registerBaseCommand = new CommandDescription(new fr.xephi.authme.command.executable.register.RegisterCommand(), new ArrayList() { - { - add("register"); - add("reg"); - } - }, "Registration command", "Command to register using AuthMeReloaded.", null); - registerBaseCommand.setCommandPermissions(PlayerPermission.REGISTER, CommandPermissions.DefaultPermission.ALLOWED); - registerBaseCommand.addArgument(new CommandArgumentDescription("password", "Password", false)); - registerBaseCommand.addArgument(new CommandArgumentDescription("verifyPassword", "Verify password", false)); + final CommandDescription REGISTER_BASE = CommandDescription.builder() + .parent(null) + .labels("register", "reg") + .description("Registration command") + .detailedDescription("Command to register using AuthMeReloaded.") + .withArgument("password", "Password", false) + .withArgument("verifyPassword", "Verify password", false) + .permissions(ALLOWED, PlayerPermission.REGISTER) + .executableCommand(new fr.xephi.authme.command.executable.register.RegisterCommand()) + .build(); // Register the help command CommandDescription registerHelpCommand = new CommandDescription(helpCommandExecutable, helpCommandLabels, - "View help", "View detailed help pages about AuthMeReloaded register commands.", registerBaseCommand); + "View help", "View detailed help pages about AuthMeReloaded register commands.", REGISTER_BASE); registerHelpCommand.addArgument(new CommandArgumentDescription("query", "The command or query to view help for.", true)); // Register the base unregister command - CommandDescription unregisterBaseCommand = new CommandDescription(new fr.xephi.authme.command.executable.unregister.UnregisterCommand(), new ArrayList() { + CommandDescription UNREGISTER_BASE = new CommandDescription(new fr.xephi.authme.command.executable.unregister.UnregisterCommand(), new ArrayList() { { add("unregister"); add("unreg"); } }, "Unregistration command", "Command to unregister using AuthMeReloaded.", null); - unregisterBaseCommand.setCommandPermissions(PlayerPermission.UNREGISTER, CommandPermissions.DefaultPermission.ALLOWED); - unregisterBaseCommand.addArgument(new CommandArgumentDescription("password", "Password", false)); + UNREGISTER_BASE.setCommandPermissions(PlayerPermission.UNREGISTER, CommandPermissions.DefaultPermission.ALLOWED); + UNREGISTER_BASE.addArgument(new CommandArgumentDescription("password", "Password", false)); // Register the help command - CommandDescription unregisterHelpCommand = new CommandDescription(helpCommandExecutable, helpCommandLabels, "View help", "View detailed help pages about AuthMeReloaded unregister commands.", unregisterBaseCommand); + CommandDescription unregisterHelpCommand = new CommandDescription(helpCommandExecutable, helpCommandLabels, "View help", "View detailed help pages about AuthMeReloaded unregister commands.", UNREGISTER_BASE); unregisterHelpCommand.addArgument(new CommandArgumentDescription("query", "The command or query to view help for.", true)); // Register the base changepassword command - CommandDescription changePasswordBaseCommand = new CommandDescription(new fr.xephi.authme.command.executable.changepassword.ChangePasswordCommand(), new ArrayList() { + final CommandDescription CHANGE_PASSWORD_BASE = new CommandDescription( + new fr.xephi.authme.command.executable.changepassword.ChangePasswordCommand(), new ArrayList() { { add("changepassword"); add("changepass"); } }, "Change password command", "Command to change your password using AuthMeReloaded.", null); - changePasswordBaseCommand.setCommandPermissions(PlayerPermission.CHANGE_PASSWORD, CommandPermissions.DefaultPermission.ALLOWED); - changePasswordBaseCommand.addArgument(new CommandArgumentDescription("password", "Password", false)); - changePasswordBaseCommand.addArgument(new CommandArgumentDescription("verifyPassword", "Verify password", false)); + CHANGE_PASSWORD_BASE.setCommandPermissions(PlayerPermission.CHANGE_PASSWORD, CommandPermissions.DefaultPermission.ALLOWED); + CHANGE_PASSWORD_BASE.addArgument(new CommandArgumentDescription("password", "Password", false)); + CHANGE_PASSWORD_BASE.addArgument(new CommandArgumentDescription("verifyPassword", "Verify password", false)); // Register the help command CommandDescription changePasswordHelpCommand = new CommandDescription(helpCommandExecutable, helpCommandLabels, - "View help", "View detailed help pages about AuthMeReloaded change password commands.", changePasswordBaseCommand); + "View help", "View detailed help pages about AuthMeReloaded change password commands.", CHANGE_PASSWORD_BASE); changePasswordHelpCommand.addArgument(new CommandArgumentDescription("query", "The command or query to view help for.", true)); // Register the base Dungeon Maze command - CommandDescription emailBaseCommand = new CommandDescription(helpCommandExecutable, new ArrayList() { + CommandDescription EMAIL_BASE = new CommandDescription(helpCommandExecutable, new ArrayList() { { add("email"); add("mail"); @@ -381,7 +381,7 @@ public final class CommandInitializer { // Register the help command CommandDescription emailHelpCommand = new CommandDescription(helpCommandExecutable, helpCommandLabels, - "View help", "View detailed help pages about AuthMeReloaded help commands.", emailBaseCommand); + "View help", "View detailed help pages about AuthMeReloaded help commands.", EMAIL_BASE); emailHelpCommand.addArgument(new CommandArgumentDescription("query", "The command or query to view help for.", true)); // Register the add command @@ -391,7 +391,7 @@ public final class CommandInitializer { add("addemail"); add("addmail"); } - }, "Add E-mail", "Add an new E-Mail address to your account.", emailBaseCommand); + }, "Add E-mail", "Add an new E-Mail address to your account.", EMAIL_BASE); addEmailCommand.setCommandPermissions(PlayerPermission.ADD_EMAIL, CommandPermissions.DefaultPermission.ALLOWED); addEmailCommand.addArgument(new CommandArgumentDescription("email", "Email address", false)); addEmailCommand.addArgument(new CommandArgumentDescription("verifyEmail", "Email address verification", false)); @@ -403,7 +403,7 @@ public final class CommandInitializer { add("changeemail"); add("changemail"); } - }, "Change E-mail", "Change an E-Mail address of your account.", emailBaseCommand); + }, "Change E-mail", "Change an E-Mail address of your account.", EMAIL_BASE); changeEmailCommand.setCommandPermissions(PlayerPermission.CHANGE_EMAIL, CommandPermissions.DefaultPermission.ALLOWED); changeEmailCommand.addArgument(new CommandArgumentDescription("oldEmail", "Old email address", false)); changeEmailCommand.addArgument(new CommandArgumentDescription("newEmail", "New email address", false)); @@ -416,51 +416,51 @@ public final class CommandInitializer { add("recoveremail"); add("recovermail"); } - }, "Recover using E-mail", "Recover your account using an E-mail address.", emailBaseCommand); + }, "Recover using E-mail", "Recover your account using an E-mail address.", EMAIL_BASE); recoverEmailCommand.setCommandPermissions(PlayerPermission.RECOVER_EMAIL, CommandPermissions.DefaultPermission.ALLOWED); recoverEmailCommand.addArgument(new CommandArgumentDescription("email", "Email address", false)); // Register the base captcha command - CommandDescription captchaBaseCommand = new CommandDescription(new CaptchaCommand(), new ArrayList() { + CommandDescription CAPTCHA_BASE = new CommandDescription(new CaptchaCommand(), new ArrayList() { { add("captcha"); add("capt"); } }, "Captcha command", "Captcha command for AuthMeReloaded.", null); - captchaBaseCommand.setCommandPermissions(PlayerPermission.CAPTCHA, CommandPermissions.DefaultPermission.ALLOWED); - captchaBaseCommand.addArgument(new CommandArgumentDescription("captcha", "The captcha", false)); + CAPTCHA_BASE.setCommandPermissions(PlayerPermission.CAPTCHA, CommandPermissions.DefaultPermission.ALLOWED); + CAPTCHA_BASE.addArgument(new CommandArgumentDescription("captcha", "The captcha", false)); // Register the help command CommandDescription captchaHelpCommand = new CommandDescription(helpCommandExecutable, helpCommandLabels, - "View help", "View detailed help pages about AuthMeReloaded change captcha commands.", captchaBaseCommand); + "View help", "View detailed help pages about AuthMeReloaded change captcha commands.", CAPTCHA_BASE); captchaHelpCommand.addArgument(new CommandArgumentDescription("query", "The command or query to view help for.", true)); // Register the base converter command - CommandDescription converterBaseCommand = new CommandDescription(new ConverterCommand(), new ArrayList() { + CommandDescription CONVERTER_BASE = new CommandDescription(new ConverterCommand(), new ArrayList() { { add("converter"); add("convert"); add("conv"); } }, "Convert command", "Convert command for AuthMeReloaded.", null); - converterBaseCommand.setCommandPermissions(AdminPermission.CONVERTER, OP_ONLY); - converterBaseCommand.addArgument(new CommandArgumentDescription("job", "Conversion job: flattosql / flattosqlite /| xauth / crazylogin / rakamak / royalauth / vauth / sqltoflat", false)); + CONVERTER_BASE.setCommandPermissions(AdminPermission.CONVERTER, OP_ONLY); + CONVERTER_BASE.addArgument(new CommandArgumentDescription("job", "Conversion job: flattosql / flattosqlite /| xauth / crazylogin / rakamak / royalauth / vauth / sqltoflat", false)); // Register the help command CommandDescription converterHelpCommand = new CommandDescription(helpCommandExecutable, helpCommandLabels, - "View help", "View detailed help pages about AuthMeReloaded change captcha commands.", converterBaseCommand); + "View help", "View detailed help pages about AuthMeReloaded change captcha commands.", CONVERTER_BASE); converterHelpCommand.addArgument(new CommandArgumentDescription("query", "The command or query to view help for.", true)); // Add the base commands to the commands array - baseCommands = Arrays.asList( + baseCommands = new HashSet<>(Arrays.asList( AUTHME_BASE, - loginBaseCommand, - logoutBaseCommand, - registerBaseCommand, - unregisterBaseCommand, - changePasswordBaseCommand, - emailBaseCommand, - captchaBaseCommand, - converterBaseCommand); + LOGIN_BASE, + LOGOUT_BASE, + REGISTER_BASE, + UNREGISTER_BASE, + CHANGE_PASSWORD_BASE, + EMAIL_BASE, + CAPTCHA_BASE, + CONVERTER_BASE)); } } diff --git a/src/test/java/fr/xephi/authme/command/CommandHandlerTest.java b/src/test/java/fr/xephi/authme/command/CommandHandlerTest.java index 99358ef55..9043bd7e1 100644 --- a/src/test/java/fr/xephi/authme/command/CommandHandlerTest.java +++ b/src/test/java/fr/xephi/authme/command/CommandHandlerTest.java @@ -9,7 +9,10 @@ import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.Mockito; +import java.util.Collection; +import java.util.HashSet; import java.util.List; +import java.util.Set; import static java.util.Arrays.asList; import static java.util.Collections.singletonList; @@ -29,7 +32,7 @@ import static org.mockito.Mockito.verify; */ public class CommandHandlerTest { - private static List commands; + private static Set commands; private static CommandHandler handler; @BeforeClass @@ -42,7 +45,7 @@ public class CommandHandlerTest { newArgument("password", false), newArgument("confirmation", false)); CommandDescription testBase = createCommand(null, null, singletonList("test"), newArgument("test", true)); - commands = asList(authMeBase, testBase); + commands = new HashSet<>(asList(authMeBase, testBase)); handler = new CommandHandler(commands); } @@ -58,7 +61,7 @@ public class CommandHandlerTest { handler.processCommand(sender, bukkitLabel, args); // then - final CommandDescription loginCmd = commands.get(0).getChildren().get(0); + final CommandDescription loginCmd = getChildWithLabel("login", getCommandWithLabel("authme", commands)); verify(sender, never()).sendMessage(anyString()); verify(loginCmd.getExecutableCommand()).executeCommand( eq(sender), any(CommandParts.class), any(CommandParts.class)); @@ -78,10 +81,7 @@ public class CommandHandlerTest { // then assertThat(result, equalTo(true)); - final CommandDescription loginCmd = commands.get(0).getChildren().get(0); assertSenderGotMessageContaining("help", sender); - verify(loginCmd.getExecutableCommand()).executeCommand( - eq(sender), any(CommandParts.class), any(CommandParts.class)); } private static CommandDescription createCommand(PlayerPermission permission, CommandDescription parent, @@ -112,4 +112,22 @@ public class CommandHandlerTest { verify(sender).sendMessage(captor.capture()); assertThat(captor.getValue(), stringContainsInOrder(text)); } + + private static CommandDescription getCommandWithLabel(String label, Collection commands) { + for (CommandDescription command : commands) { + if (command.getLabels().contains(label)) { + return command; + } + } + return null; + } + + private static CommandDescription getChildWithLabel(String label, CommandDescription command) { + for (CommandDescription child : command.getChildren()) { + if (child.getLabels().contains(label)) { + return child; + } + } + return null; + } } diff --git a/src/test/java/fr/xephi/authme/command/CommandInitializerTest.java b/src/test/java/fr/xephi/authme/command/CommandInitializerTest.java index 2cc424c01..dcbec20ed 100644 --- a/src/test/java/fr/xephi/authme/command/CommandInitializerTest.java +++ b/src/test/java/fr/xephi/authme/command/CommandInitializerTest.java @@ -5,12 +5,7 @@ import fr.xephi.authme.util.WrapperMock; import org.junit.BeforeClass; import org.junit.Test; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.util.*; import java.util.regex.Pattern; import static org.hamcrest.Matchers.equalTo; @@ -30,7 +25,7 @@ public class CommandInitializerTest { */ private static int MAX_ALLOWED_DEPTH = 1; - private static List commands; + private static Set commands; @BeforeClass public static void initializeCommandManager() { @@ -232,11 +227,11 @@ public class CommandInitializerTest { // ------------ // Helper methods // ------------ - private static void walkThroughCommands(List commands, BiConsumer consumer) { + private static void walkThroughCommands(Collection commands, BiConsumer consumer) { walkThroughCommands(commands, consumer, 0); } - private static void walkThroughCommands(List commands, BiConsumer consumer, int depth) { + private static void walkThroughCommands(Collection commands, BiConsumer consumer, int depth) { for (CommandDescription command : commands) { consumer.accept(command, depth); if (command.hasChildren()) { From 2faad44ffaf0c963dbb3a9cf12acd050b7f47d98 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Fri, 4 Dec 2015 23:19:37 +0100 Subject: [PATCH 08/10] Move most API of CommandParts to util classes Gradual transition to removing CommandParts altogether. - Move logic from CommandParts to utils classes --- .../authme/command/CommandDescription.java | 14 ++- .../fr/xephi/authme/command/CommandParts.java | 73 +------------ .../fr/xephi/authme/command/CommandUtils.java | 39 +++++++ .../authme/command/FoundCommandResult.java | 5 +- .../authme/command/help/HelpProvider.java | 10 +- .../fr/xephi/authme/util/CollectionUtils.java | 47 ++++++++ .../fr/xephi/authme/util/StringUtils.java | 3 +- .../command/CommandInitializerTest.java | 6 +- .../authme/command/CommandPartsTest.java | 3 +- .../captcha/CaptchaCommandTest.java | 6 +- .../ChangePasswordCommandTest.java | 17 +-- .../executable/email/AddEmailCommandTest.java | 9 +- .../email/ChangeEmailCommandTest.java | 9 +- .../email/RecoverEmailCommandTest.java | 4 +- .../executable/login/LoginCommandTest.java | 12 ++- .../executable/logout/LogoutCommandTest.java | 6 +- .../register/RegisterCommandTest.java | 12 ++- .../command/help/HelpSyntaxHelperTest.java | 18 ++-- .../authme/util/CollectionUtilsTest.java | 102 ++++++++++++++++++ 19 files changed, 278 insertions(+), 117 deletions(-) create mode 100644 src/main/java/fr/xephi/authme/util/CollectionUtils.java create mode 100644 src/test/java/fr/xephi/authme/util/CollectionUtilsTest.java diff --git a/src/main/java/fr/xephi/authme/command/CommandDescription.java b/src/main/java/fr/xephi/authme/command/CommandDescription.java index faeb0d746..2911055de 100644 --- a/src/main/java/fr/xephi/authme/command/CommandDescription.java +++ b/src/main/java/fr/xephi/authme/command/CommandDescription.java @@ -1,6 +1,7 @@ package fr.xephi.authme.command; import fr.xephi.authme.permission.PermissionNode; +import fr.xephi.authme.util.CollectionUtils; import fr.xephi.authme.util.StringUtils; import org.bukkit.command.CommandSender; @@ -185,10 +186,6 @@ public class CommandDescription { * @return True if the command reference is suitable to this command label, false otherwise. */ public boolean isSuitableLabel(CommandParts commandReference) { - // Make sure the command reference is valid - if (commandReference.getCount() <= 0) - return false; - // Get the parent count //getParent() = getParent().getParentCount() + 1 String element = commandReference.get(getParentCount()); @@ -252,7 +249,8 @@ public class CommandDescription { CommandParts reference = getCommandReference(other); // Compare the two references, return the result - return reference.getDifference(new CommandParts(other.getRange(0, reference.getCount())), fullCompare); + return CommandUtils.getDifference(reference.getList(), + CollectionUtils.getRange(other.getList(), 0, reference.getList().size()), fullCompare); } /** @@ -469,13 +467,13 @@ public class CommandDescription { return new FoundCommandResult( this, getCommandReference(queryReference), - new CommandParts(), + new CommandParts(new ArrayList()), queryReference); } // Get the new command reference and arguments - CommandParts newReference = new CommandParts(queryReference.getRange(0, getParentCount() + 1)); - CommandParts newArguments = new CommandParts(queryReference.getRange(getParentCount() + 1)); + CommandParts newReference = new CommandParts(CollectionUtils.getRange(queryReference.getList(), 0, getParentCount() + 1)); + CommandParts newArguments = new CommandParts(CollectionUtils.getRange(queryReference.getList(), getParentCount() + 1)); // Handle the child's, if this command has any if (getChildren().size() > 0) { diff --git a/src/main/java/fr/xephi/authme/command/CommandParts.java b/src/main/java/fr/xephi/authme/command/CommandParts.java index 35e895746..58df26011 100644 --- a/src/main/java/fr/xephi/authme/command/CommandParts.java +++ b/src/main/java/fr/xephi/authme/command/CommandParts.java @@ -14,12 +14,6 @@ public class CommandParts { */ private final List parts = new ArrayList<>(); - /** - * Constructor. - */ - public CommandParts() { - } - /** * Constructor. * @@ -47,17 +41,6 @@ public class CommandParts { this.parts.addAll(parts); } - /** - * Constructor. - * - * @param base The base part. - * @param parts The list of additional parts. - */ - public CommandParts(String base, List parts) { - this.parts.add(base); - this.parts.addAll(parts); - } - /** * Get the command parts. * @@ -67,41 +50,6 @@ public class CommandParts { return this.parts; } - /** - * Add a part. - * - * @param part The part to add. - * - * @return The result. - */ - public boolean add(String part) { - return this.parts.add(part); - } - - /** - * Add some parts. - * - * @param parts The parts to add. - * - * @return The result. - */ - public boolean add(List parts) { - return this.parts.addAll(parts); - } - - /** - * Add some parts. - * - * @param parts The parts to add. - * - * @return The result. - */ - public boolean add(String[] parts) { - for (String entry : parts) - add(entry); - return true; - } - /** * Get the number of parts. * @@ -146,6 +94,7 @@ public class CommandParts { * * @return The parts range. Parts that were out of bound are not included. */ + @Deprecated public List getRange(int start, int count) { // Create a new list to put the range into List elements = new ArrayList<>(); @@ -162,27 +111,7 @@ public class CommandParts { return elements; } - /** - * Get the difference value between two references. - * - * @param other The other reference. - * @param fullCompare True to compare the full references as far as the range reaches. - * - * @return The result from zero to above. A negative number will be returned on error. - */ - public double getDifference(CommandParts other, boolean fullCompare) { - // Make sure the other reference is correct - if (other == null) - return -1; - // Get the range to use - int range = Math.min(this.getCount(), other.getCount()); - - // Get and the difference - if (fullCompare) - return StringUtils.getDifference(this.toString(), other.toString()); - return StringUtils.getDifference(this.getRange(range - 1, 1).toString(), other.getRange(range - 1, 1).toString()); - } /** * Convert the parts to a string. diff --git a/src/main/java/fr/xephi/authme/command/CommandUtils.java b/src/main/java/fr/xephi/authme/command/CommandUtils.java index 6d5e2b551..6903ff7a4 100644 --- a/src/main/java/fr/xephi/authme/command/CommandUtils.java +++ b/src/main/java/fr/xephi/authme/command/CommandUtils.java @@ -1,5 +1,11 @@ package fr.xephi.authme.command; +import fr.xephi.authme.util.CollectionUtils; +import fr.xephi.authme.util.StringUtils; + +import java.util.ArrayList; +import java.util.List; + public final class CommandUtils { public static int getMinNumberOfArguments(CommandDescription command) { @@ -16,4 +22,37 @@ public final class CommandUtils { return command.getArguments().size(); } + /** + * Provide a textual representation of a list of labels to show it as a command. For example, a list containing + * the items ["authme", "register", "player"] it will return "authme register player". + * + * @param labels The labels to format + * @return The space-separated labels + */ + public static String labelsToString(Iterable labels) { + return StringUtils.join(" ", labels); + } + + public static double getDifference(List labels1, List labels2, boolean fullCompare) { + // Make sure the other reference is correct + if (labels1 == null || labels2 == null) { + return -1; + } + + // Get the range to use + int range = Math.min(labels1.size(), labels2.size()); + + // Get and the difference + if (fullCompare) { + return StringUtils.getDifference(CommandUtils.labelsToString(labels1), CommandUtils.labelsToString(labels2)); + } + return StringUtils.getDifference( + labelsToString(CollectionUtils.getRange(labels1, range - 1, 1)), + labelsToString(CollectionUtils.getRange(labels2, range - 1, 1))); + } + + + + + } diff --git a/src/main/java/fr/xephi/authme/command/FoundCommandResult.java b/src/main/java/fr/xephi/authme/command/FoundCommandResult.java index 1ee4fbd9d..915516500 100644 --- a/src/main/java/fr/xephi/authme/command/FoundCommandResult.java +++ b/src/main/java/fr/xephi/authme/command/FoundCommandResult.java @@ -139,10 +139,11 @@ public class FoundCommandResult { */ public double getDifference() { // Get the difference through the command found - if (this.commandDescription != null) + if (this.commandDescription != null) { return this.commandDescription.getCommandDifference(this.queryReference); + } // Get the difference from the query reference - return this.queryReference.getDifference(commandReference, true); + return CommandUtils.getDifference(queryReference.getList(), commandReference.getList(), true); } } diff --git a/src/main/java/fr/xephi/authme/command/help/HelpProvider.java b/src/main/java/fr/xephi/authme/command/help/HelpProvider.java index 03dab7b4f..a2f3d1f2c 100644 --- a/src/main/java/fr/xephi/authme/command/help/HelpProvider.java +++ b/src/main/java/fr/xephi/authme/command/help/HelpProvider.java @@ -9,6 +9,9 @@ import fr.xephi.authme.settings.Settings; import org.bukkit.ChatColor; import org.bukkit.command.CommandSender; +import java.util.ArrayList; +import java.util.List; + /** */ public class HelpProvider { @@ -40,7 +43,12 @@ public class HelpProvider { public static void showHelp(CommandSender sender, CommandParts reference, CommandParts helpQuery, boolean showCommand, boolean showDescription, boolean showArguments, boolean showPermissions, boolean showAlternatives, boolean showCommands) { // Find the command for this help query, one with and one without a prefixed base command FoundCommandResult result = AuthMe.getInstance().getCommandHandler().findCommand(new CommandParts(helpQuery.getList())); - CommandParts commandReferenceOther = new CommandParts(reference.get(0), helpQuery.getList()); + + // TODO ljacqu 20151204 Fix me to nicer code + List parts = new ArrayList<>(helpQuery.getList()); + parts.add(0, reference.get(0)); + CommandParts commandReferenceOther = new CommandParts(parts); + FoundCommandResult resultOther = AuthMe.getInstance().getCommandHandler().findCommand(commandReferenceOther); if (resultOther != null) { if (result == null) diff --git a/src/main/java/fr/xephi/authme/util/CollectionUtils.java b/src/main/java/fr/xephi/authme/util/CollectionUtils.java new file mode 100644 index 000000000..4d43757e1 --- /dev/null +++ b/src/main/java/fr/xephi/authme/util/CollectionUtils.java @@ -0,0 +1,47 @@ +package fr.xephi.authme.util; + +import java.util.ArrayList; +import java.util.List; + +/** + * Utils class for collections. + */ +public final class CollectionUtils { + + private CollectionUtils() { + } + + /** + * Get a range from a list based on start and count parameters in a safe way. + * + * @param start The start index + * @param count The number of elements to add + * + * @return The sublist consisting at most of {@code count} elements (less if the parameters + * exceed the size of the list) + */ + public static List getRange(List list, int start, int count) { + if (start >= list.size() || count <= 0) { + return new ArrayList<>(); + } else if (start < 0) { + start = 0; + } + int end = Math.min(list.size(), start + count); + return list.subList(start, end); + } + + /** + * Get all elements from a list starting from the given index. + * + * @param start The start index + * + * @return The sublist of all elements from index {@code start} and on; empty list + * if the start index exceeds the list's size + */ + public static List getRange(List list, int start) { + if (start >= list.size()) { + return new ArrayList<>(); + } + return getRange(list, start, list.size() - start); + } +} diff --git a/src/main/java/fr/xephi/authme/util/StringUtils.java b/src/main/java/fr/xephi/authme/util/StringUtils.java index 74fe04756..2e37bfce0 100644 --- a/src/main/java/fr/xephi/authme/util/StringUtils.java +++ b/src/main/java/fr/xephi/authme/util/StringUtils.java @@ -29,8 +29,9 @@ public final class StringUtils { */ public static double getDifference(String first, String second) { // Make sure the strings are valid. - if (first == null || second == null) + if (first == null || second == null) { return 1.0; + } // Create a string similarity service instance, to allow comparison StringSimilarityService service = new StringSimilarityServiceImpl(new LevenshteinDistanceStrategy()); diff --git a/src/test/java/fr/xephi/authme/command/CommandInitializerTest.java b/src/test/java/fr/xephi/authme/command/CommandInitializerTest.java index dcbec20ed..51a8e3cdc 100644 --- a/src/test/java/fr/xephi/authme/command/CommandInitializerTest.java +++ b/src/test/java/fr/xephi/authme/command/CommandInitializerTest.java @@ -254,12 +254,12 @@ public class CommandInitializerTest { } /** - * Get the absolute label that a command defines. Note: Assumes that only the passed command might have + * Get the absolute binding that a command defines. Note: Assumes that only the passed command can have * multiple labels; only considering the first label for all of the command's parents. * - * @param command The command to verify + * @param command The command to process * - * @return The full command binding + * @return List of all bindings that lead to the command */ private static List getAbsoluteLabels(CommandDescription command) { String parentPath = ""; diff --git a/src/test/java/fr/xephi/authme/command/CommandPartsTest.java b/src/test/java/fr/xephi/authme/command/CommandPartsTest.java index 62d22b5fa..163f52d4c 100644 --- a/src/test/java/fr/xephi/authme/command/CommandPartsTest.java +++ b/src/test/java/fr/xephi/authme/command/CommandPartsTest.java @@ -3,6 +3,7 @@ package fr.xephi.authme.command; import org.junit.Test; import java.util.Arrays; +import java.util.Collections; import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertThat; @@ -27,7 +28,7 @@ public class CommandPartsTest { @Test public void shouldPrintEmptyStringForNoArguments() { // given - CommandParts parts = new CommandParts(); + CommandParts parts = new CommandParts(Collections.EMPTY_LIST); // when String str = parts.toString(); diff --git a/src/test/java/fr/xephi/authme/command/executable/captcha/CaptchaCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/captcha/CaptchaCommandTest.java index 36169db05..6b183e35a 100644 --- a/src/test/java/fr/xephi/authme/command/executable/captcha/CaptchaCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/captcha/CaptchaCommandTest.java @@ -15,6 +15,8 @@ import org.junit.Ignore; import org.junit.Test; import org.mockito.Mockito; +import java.util.Collections; + import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertThat; import static org.mockito.Mockito.verify; @@ -40,7 +42,7 @@ public class CaptchaCommandTest { ExecutableCommand command = new CaptchaCommand(); // when - boolean result = command.executeCommand(sender, new CommandParts(), new CommandParts()); + boolean result = command.executeCommand(sender, new CommandParts(Collections.EMPTY_LIST), new CommandParts(Collections.EMPTY_LIST)); // then assertThat(result, equalTo(true)); @@ -56,7 +58,7 @@ public class CaptchaCommandTest { ExecutableCommand command = new CaptchaCommand(); // when - boolean result = command.executeCommand(player, new CommandParts(), new CommandParts()); + boolean result = command.executeCommand(player, new CommandParts(Collections.EMPTY_LIST), new CommandParts(Collections.EMPTY_LIST)); // then assertThat(result, equalTo(true)); diff --git a/src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java index a81611a5d..9092205aa 100644 --- a/src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java @@ -18,6 +18,7 @@ import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -58,7 +59,7 @@ public class ChangePasswordCommandTest { CommandParts arguments = mock(CommandParts.class); // when - command.executeCommand(sender, new CommandParts(), arguments); + command.executeCommand(sender, newParts(), arguments); // then verify(arguments, never()).get(anyInt()); @@ -72,7 +73,7 @@ public class ChangePasswordCommandTest { ChangePasswordCommand command = new ChangePasswordCommand(); // when - command.executeCommand(sender, new CommandParts(), new CommandParts("pass")); + command.executeCommand(sender, newParts(), new CommandParts("pass")); // then verify(messagesMock).send(sender, MessageKey.NOT_LOGGED_IN); @@ -86,7 +87,7 @@ public class ChangePasswordCommandTest { ChangePasswordCommand command = new ChangePasswordCommand(); // when - command.executeCommand(sender, new CommandParts(), newParts("old123", "!pass")); + command.executeCommand(sender, newParts(), newParts("old123", "!pass")); // then verify(messagesMock).send(sender, MessageKey.PASSWORD_MATCH_ERROR); @@ -101,7 +102,7 @@ public class ChangePasswordCommandTest { ChangePasswordCommand command = new ChangePasswordCommand(); // when - command.executeCommand(sender, new CommandParts(), newParts("old_", "Tester")); + command.executeCommand(sender, newParts(), newParts("old_", "Tester")); // then verify(messagesMock).send(sender, MessageKey.PASSWORD_IS_USERNAME_ERROR); @@ -116,7 +117,7 @@ public class ChangePasswordCommandTest { Settings.passwordMaxLength = 3; // when - command.executeCommand(sender, new CommandParts(), newParts("12", "test")); + command.executeCommand(sender, newParts(), newParts("12", "test")); // then verify(messagesMock).send(sender, MessageKey.INVALID_PASSWORD_LENGTH); @@ -131,7 +132,7 @@ public class ChangePasswordCommandTest { Settings.getPasswordMinLen = 7; // when - command.executeCommand(sender, new CommandParts(), newParts("oldverylongpassword", "tester")); + command.executeCommand(sender, newParts(), newParts("oldverylongpassword", "tester")); // then verify(messagesMock).send(sender, MessageKey.INVALID_PASSWORD_LENGTH); @@ -146,7 +147,7 @@ public class ChangePasswordCommandTest { Settings.unsafePasswords = asList("test", "abc123"); // when - command.executeCommand(sender, new CommandParts(), newParts("oldpw", "abc123")); + command.executeCommand(sender, newParts(), newParts("oldpw", "abc123")); // then verify(messagesMock).send(sender, MessageKey.PASSWORD_UNSAFE_ERROR); @@ -160,7 +161,7 @@ public class ChangePasswordCommandTest { ChangePasswordCommand command = new ChangePasswordCommand(); // when - command.executeCommand(sender, new CommandParts(), newParts("abc123", "abc123")); + command.executeCommand(sender, newParts(), newParts("abc123", "abc123")); // then verify(messagesMock, never()).send(eq(sender), any(MessageKey.class)); diff --git a/src/test/java/fr/xephi/authme/command/executable/email/AddEmailCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/email/AddEmailCommandTest.java index a06cff220..d377fc0f8 100644 --- a/src/test/java/fr/xephi/authme/command/executable/email/AddEmailCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/email/AddEmailCommandTest.java @@ -11,6 +11,7 @@ import org.junit.Before; import org.junit.Test; import org.mockito.Mockito; +import java.util.ArrayList; import java.util.Arrays; import static org.mockito.Mockito.never; @@ -40,7 +41,7 @@ public class AddEmailCommandTest { AddEmailCommand command = new AddEmailCommand(); // when - command.executeCommand(sender, new CommandParts(), new CommandParts()); + command.executeCommand(sender, newParts(), newParts()); // then verify(authMeMock, never()).getManagement(); @@ -53,11 +54,15 @@ public class AddEmailCommandTest { AddEmailCommand command = new AddEmailCommand(); // when - command.executeCommand(sender, new CommandParts(), + command.executeCommand(sender, newParts(), new CommandParts(Arrays.asList("mail@example", "other_example"))); // then verify(authMeMock).getManagement(); verify(managementMock).performAddEmail(sender, "mail@example", "other_example"); } + + private static CommandParts newParts() { + return new CommandParts(new ArrayList()); + } } diff --git a/src/test/java/fr/xephi/authme/command/executable/email/ChangeEmailCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/email/ChangeEmailCommandTest.java index 0f2c05e6e..2dc25c96b 100644 --- a/src/test/java/fr/xephi/authme/command/executable/email/ChangeEmailCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/email/ChangeEmailCommandTest.java @@ -11,6 +11,7 @@ import org.junit.Before; import org.junit.Test; import org.mockito.Mockito; +import java.util.ArrayList; import java.util.Arrays; import static org.mockito.Mockito.never; @@ -40,7 +41,7 @@ public class ChangeEmailCommandTest { ChangeEmailCommand command = new ChangeEmailCommand(); // when - command.executeCommand(sender, new CommandParts(), new CommandParts()); + command.executeCommand(sender, newParts(), newParts()); // then verify(authMeMock, never()).getManagement(); @@ -53,11 +54,15 @@ public class ChangeEmailCommandTest { ChangeEmailCommand command = new ChangeEmailCommand(); // when - command.executeCommand(sender, new CommandParts(), + command.executeCommand(sender, newParts(), new CommandParts(Arrays.asList("new.mail@example.org", "old_mail@example.org"))); // then verify(authMeMock).getManagement(); verify(managementMock).performChangeEmail(sender, "new.mail@example.org", "old_mail@example.org"); } + + private static CommandParts newParts() { + return new CommandParts(new ArrayList()); + } } diff --git a/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java index 9857a2d4c..a1403c5ac 100644 --- a/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java @@ -9,6 +9,8 @@ import org.junit.Ignore; import org.junit.Test; import org.mockito.Mockito; +import java.util.Collections; + /** * Test for {@link RecoverEmailCommand}. */ @@ -27,7 +29,7 @@ public class RecoverEmailCommandTest { RecoverEmailCommand command = new RecoverEmailCommand(); // when - command.executeCommand(sender, new CommandParts(), new CommandParts()); + command.executeCommand(sender, new CommandParts(Collections.EMPTY_LIST), new CommandParts(Collections.EMPTY_LIST)); // then } diff --git a/src/test/java/fr/xephi/authme/command/executable/login/LoginCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/login/LoginCommandTest.java index 6f06a5fff..52a71891b 100644 --- a/src/test/java/fr/xephi/authme/command/executable/login/LoginCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/login/LoginCommandTest.java @@ -11,6 +11,8 @@ import org.junit.Before; import org.junit.Test; import org.mockito.Mockito; +import java.util.ArrayList; + import static org.mockito.Matchers.*; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -38,7 +40,7 @@ public class LoginCommandTest { LoginCommand command = new LoginCommand(); // when - command.executeCommand(sender, new CommandParts(), new CommandParts()); + command.executeCommand(sender, newParts(), newParts()); // then Mockito.verify(managementMock, never()).performLogin(any(Player.class), anyString(), anyBoolean()); @@ -51,7 +53,7 @@ public class LoginCommandTest { LoginCommand command = new LoginCommand(); // when - command.executeCommand(sender, new CommandParts(), new CommandParts("password")); + command.executeCommand(sender, newParts(), new CommandParts("password")); // then Mockito.verify(managementMock).performLogin(eq(sender), eq("password"), eq(false)); @@ -64,11 +66,15 @@ public class LoginCommandTest { LoginCommand command = new LoginCommand(); // when - command.executeCommand(sender, new CommandParts(), new CommandParts()); + command.executeCommand(sender, newParts(), newParts()); // then // TODO ljacqu 20151121: May make sense to handle null password in LoginCommand instead of forwarding the call String password = null; Mockito.verify(managementMock).performLogin(eq(sender), eq(password), eq(false)); } + + private static CommandParts newParts() { + return new CommandParts(new ArrayList()); + } } diff --git a/src/test/java/fr/xephi/authme/command/executable/logout/LogoutCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/logout/LogoutCommandTest.java index c8774978c..65f246397 100644 --- a/src/test/java/fr/xephi/authme/command/executable/logout/LogoutCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/logout/LogoutCommandTest.java @@ -12,6 +12,8 @@ import org.junit.Before; import org.junit.Test; import org.mockito.Mockito; +import java.util.ArrayList; + import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -40,7 +42,7 @@ public class LogoutCommandTest { LogoutCommand command = new LogoutCommand(); // when - command.executeCommand(sender, new CommandParts(), new CommandParts()); + command.executeCommand(sender, new CommandParts(new ArrayList()), new CommandParts(new ArrayList())); // then Mockito.verify(managementMock, never()).performLogout(any(Player.class)); @@ -53,7 +55,7 @@ public class LogoutCommandTest { LogoutCommand command = new LogoutCommand(); // when - command.executeCommand(sender, new CommandParts(), new CommandParts("password")); + command.executeCommand(sender, new CommandParts(new ArrayList()), new CommandParts("password")); // then Mockito.verify(managementMock).performLogout(sender); diff --git a/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java index 02bb65a73..6dd19eb25 100644 --- a/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java @@ -14,6 +14,8 @@ import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.Mockito; +import java.util.ArrayList; + import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertThat; import static org.mockito.Matchers.any; @@ -48,7 +50,7 @@ public class RegisterCommandTest { ArgumentCaptor messageCaptor = ArgumentCaptor.forClass(String.class); // when - command.executeCommand(sender, new CommandParts(), new CommandParts()); + command.executeCommand(sender, newParts(), newParts()); // then verify(sender).sendMessage(messageCaptor.capture()); @@ -63,7 +65,7 @@ public class RegisterCommandTest { RegisterCommand command = new RegisterCommand(); // when - command.executeCommand(sender, new CommandParts(), new CommandParts()); + command.executeCommand(sender, newParts(), newParts()); // then verify(messagesMock).send(sender, MessageKey.USAGE_REGISTER); @@ -77,9 +79,13 @@ public class RegisterCommandTest { RegisterCommand command = new RegisterCommand(); // when - command.executeCommand(sender, new CommandParts(), new CommandParts("password")); + command.executeCommand(sender, newParts(), new CommandParts("password")); // then verify(managementMock).performRegister(sender, "password", ""); } + + private static CommandParts newParts() { + return new CommandParts(new ArrayList()); + } } diff --git a/src/test/java/fr/xephi/authme/command/help/HelpSyntaxHelperTest.java b/src/test/java/fr/xephi/authme/command/help/HelpSyntaxHelperTest.java index f9fa9a6a0..acfe90282 100644 --- a/src/test/java/fr/xephi/authme/command/help/HelpSyntaxHelperTest.java +++ b/src/test/java/fr/xephi/authme/command/help/HelpSyntaxHelperTest.java @@ -7,6 +7,8 @@ import fr.xephi.authme.command.executable.authme.RegisterCommand; import org.junit.Test; import org.mockito.Mockito; +import java.util.ArrayList; + import static org.bukkit.ChatColor.BOLD; import static org.bukkit.ChatColor.ITALIC; import static org.bukkit.ChatColor.WHITE; @@ -27,7 +29,7 @@ public class HelpSyntaxHelperTest { .build(); // when - String result = HelpSyntaxHelper.getCommandSyntax(description, new CommandParts(), "", false); + String result = HelpSyntaxHelper.getCommandSyntax(description, newParts(), "", false); // then assertThat(result, equalTo(WHITE + "/authme register" + ITALIC + " [name]")); @@ -41,7 +43,7 @@ public class HelpSyntaxHelperTest { .build(); // when - String result = HelpSyntaxHelper.getCommandSyntax(description, new CommandParts(), null, false); + String result = HelpSyntaxHelper.getCommandSyntax(description, newParts(), null, false); // then assertThat(result, equalTo(WHITE + "/authme register" + ITALIC + " ")); @@ -56,7 +58,7 @@ public class HelpSyntaxHelperTest { .build(); // when - String result = HelpSyntaxHelper.getCommandSyntax(description, new CommandParts(), "", false); + String result = HelpSyntaxHelper.getCommandSyntax(description, newParts(), "", false); // then assertThat(result, equalTo(WHITE + "/authme register" + ITALIC + " [name]" + ITALIC + " ")); @@ -71,7 +73,7 @@ public class HelpSyntaxHelperTest { .build(); // when - String result = HelpSyntaxHelper.getCommandSyntax(description, new CommandParts(), "", true); + String result = HelpSyntaxHelper.getCommandSyntax(description, newParts(), "", true); // then assertThat(result, equalTo(WHITE + "/authme " @@ -85,7 +87,7 @@ public class HelpSyntaxHelperTest { CommandDescription description = getDescriptionBuilder().build(); // when - String result = HelpSyntaxHelper.getCommandSyntax(description, new CommandParts(), null, true); + String result = HelpSyntaxHelper.getCommandSyntax(description, newParts(), null, true); // then assertThat(result, equalTo(WHITE + "/authme " + YELLOW + BOLD + "register" + YELLOW)); @@ -99,12 +101,16 @@ public class HelpSyntaxHelperTest { .build(); // when - String result = HelpSyntaxHelper.getCommandSyntax(description, new CommandParts(), "alt", false); + String result = HelpSyntaxHelper.getCommandSyntax(description, newParts(), "alt", false); // then assertThat(result, equalTo(WHITE + "/authme alt" + ITALIC + " [name]")); } + private static CommandParts newParts() { + // TODO ljacqu 20151204: Remove this method once CommandParts has been removed + return new CommandParts(new ArrayList()); + } private static CommandDescription.CommandBuilder getDescriptionBuilder() { CommandDescription base = CommandDescription.builder() diff --git a/src/test/java/fr/xephi/authme/util/CollectionUtilsTest.java b/src/test/java/fr/xephi/authme/util/CollectionUtilsTest.java new file mode 100644 index 000000000..f359f0883 --- /dev/null +++ b/src/test/java/fr/xephi/authme/util/CollectionUtilsTest.java @@ -0,0 +1,102 @@ +package fr.xephi.authme.util; + +import org.junit.Test; + +import java.util.Arrays; +import java.util.List; + +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThat; +import static org.hamcrest.Matchers.empty; + +/** + * Test for {@link CollectionUtils}. + */ +public class CollectionUtilsTest { + + @Test + public void shouldGetFullList() { + // given + List list = Arrays.asList("test", "1", "2", "3", "4"); + + // when + List result = CollectionUtils.getRange(list, 0, 24); + + // then + assertThat(result, equalTo(list)); + } + + @Test + public void shouldReturnEmptyListForZeroCount() { + // given + List list = Arrays.asList("test", "1", "2", "3", "4"); + + // when + List result = CollectionUtils.getRange(list, 2, 0); + + // then + assertThat(result, empty()); + } + + + @Test + public void shouldReturnEmptyListForTooHighStart() { + // given + List list = Arrays.asList("test", "1", "2", "3", "4"); + + // when + List result = CollectionUtils.getRange(list, 12, 2); + + // then + assertThat(result, empty()); + } + + @Test + public void shouldReturnSubList() { + // given + List list = Arrays.asList("test", "1", "2", "3", "4"); + + // when + List result = CollectionUtils.getRange(list, 1, 3); + + // then + assertThat(result, contains("1", "2", "3")); + } + + @Test + public void shouldReturnTillEnd() { + // given + List list = Arrays.asList("test", "1", "2", "3", "4"); + + // when + List result = CollectionUtils.getRange(list, 2, 3); + + // then + assertThat(result, contains("2", "3", "4")); + } + + @Test + public void shouldRemoveFirstTwo() { + // given + List list = Arrays.asList("test", "1", "2", "3", "4"); + + // when + List result = CollectionUtils.getRange(list, 2); + + // then + assertThat(result, contains("2", "3", "4")); + } + + @Test + public void shouldHandleNegativeStart() { + // given + List list = Arrays.asList("test", "1", "2", "3", "4"); + + // when + List result = CollectionUtils.getRange(list, -4); + + // then + assertThat(result, equalTo(list)); + } +} From 8eb1b38d217085a805d7fce94306d53062193497 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Fri, 4 Dec 2015 23:47:48 +0100 Subject: [PATCH 09/10] Slim down CommandParts API - Remove all methods that aren't constructors / fancy get() on list; almost possible to remove class entirely --- .../java/fr/xephi/authme/ConsoleLogger.java | 13 ++--- .../authme/command/CommandDescription.java | 3 +- .../xephi/authme/command/CommandHandler.java | 17 ++++--- .../fr/xephi/authme/command/CommandParts.java | 49 +------------------ .../authme/SwitchAntiBotCommand.java | 17 ++++--- .../authme/command/help/HelpProvider.java | 21 +++++--- .../authme/command/help/HelpSyntaxHelper.java | 6 ++- .../fr/xephi/authme/util/StringUtils.java | 2 - 8 files changed, 47 insertions(+), 81 deletions(-) diff --git a/src/main/java/fr/xephi/authme/ConsoleLogger.java b/src/main/java/fr/xephi/authme/ConsoleLogger.java index 890318046..fee14296d 100644 --- a/src/main/java/fr/xephi/authme/ConsoleLogger.java +++ b/src/main/java/fr/xephi/authme/ConsoleLogger.java @@ -2,7 +2,6 @@ package fr.xephi.authme; import com.google.common.base.Throwables; import fr.xephi.authme.settings.Settings; -import fr.xephi.authme.util.StringUtils; import fr.xephi.authme.util.Wrapper; import java.io.IOException; @@ -17,8 +16,10 @@ import java.util.Date; */ public final class ConsoleLogger { + private static final String NEW_LINE = System.getProperty("line.separator"); + private static final DateFormat DATE_FORMAT = new SimpleDateFormat("[MM-dd HH:mm:ss]"); + private static Wrapper wrapper = Wrapper.getInstance(); - private static final DateFormat df = new SimpleDateFormat("[MM-dd HH:mm:ss]"); private ConsoleLogger() { // Service class @@ -57,11 +58,11 @@ public final class ConsoleLogger { */ private static void writeLog(String message) { String dateTime; - synchronized (df) { - dateTime = df.format(new Date()); + synchronized (DATE_FORMAT) { + dateTime = DATE_FORMAT.format(new Date()); } try { - Files.write(Settings.LOG_FILE.toPath(), (dateTime + ": " + message + StringUtils.newline).getBytes(), + Files.write(Settings.LOG_FILE.toPath(), (dateTime + ": " + message + NEW_LINE).getBytes(), StandardOpenOption.APPEND, StandardOpenOption.CREATE); } catch (IOException ignored) { @@ -77,6 +78,6 @@ public final class ConsoleLogger { if (!Settings.useLogging) { return; } - writeLog("" + Throwables.getStackTraceAsString(ex)); + writeLog(Throwables.getStackTraceAsString(ex)); } } diff --git a/src/main/java/fr/xephi/authme/command/CommandDescription.java b/src/main/java/fr/xephi/authme/command/CommandDescription.java index 2911055de..4623ddd6c 100644 --- a/src/main/java/fr/xephi/authme/command/CommandDescription.java +++ b/src/main/java/fr/xephi/authme/command/CommandDescription.java @@ -211,8 +211,9 @@ public class CommandDescription { List referenceList = new ArrayList<>(); // Check whether this command has a parent, if so, add the absolute parent command - if (getParent() != null) + if (getParent() != null) { referenceList.addAll(getParent().getCommandReference(reference).getList()); + } // Get the current label referenceList.add(getLabel(reference)); diff --git a/src/main/java/fr/xephi/authme/command/CommandHandler.java b/src/main/java/fr/xephi/authme/command/CommandHandler.java index bb0b1298c..67a4fbe64 100644 --- a/src/main/java/fr/xephi/authme/command/CommandHandler.java +++ b/src/main/java/fr/xephi/authme/command/CommandHandler.java @@ -2,6 +2,7 @@ package fr.xephi.authme.command; import fr.xephi.authme.AuthMe; import fr.xephi.authme.command.help.HelpProvider; +import fr.xephi.authme.util.CollectionUtils; import fr.xephi.authme.util.StringUtils; import org.bukkit.ChatColor; import org.bukkit.command.CommandSender; @@ -209,28 +210,28 @@ public class CommandHandler { private static void sendImproperArgumentsMessage(CommandSender sender, FoundCommandResult result, CommandParts commandReference, String baseCommand) { // Get the command and the suggested command reference - CommandParts suggestedCommandReference = - new CommandParts(result.getCommandDescription().getCommandReference(commandReference)); - CommandParts helpCommandReference = new CommandParts(suggestedCommandReference.getRange(1)); + List suggestedCommandReference = + result.getCommandDescription().getCommandReference(commandReference).getList(); + List helpCommandReference = CollectionUtils.getRange(suggestedCommandReference, 1); // Show the invalid arguments warning sender.sendMessage(ChatColor.DARK_RED + "Incorrect command arguments!"); // Show the command argument help - HelpProvider.showHelp(sender, commandReference, suggestedCommandReference, + HelpProvider.showHelp(sender, commandReference, new CommandParts(suggestedCommandReference), true, false, true, false, false, false); // Show the command to use for detailed help sender.sendMessage(ChatColor.GOLD + "Detailed help: " + ChatColor.WHITE + "/" + baseCommand - + " help " + helpCommandReference); + + " help " + CommandUtils.labelsToString(helpCommandReference)); } private static void sendCommandAssumptionMessage(CommandSender sender, FoundCommandResult result, CommandParts commandReference) { - CommandParts assumedCommandParts = - new CommandParts(result.getCommandDescription().getCommandReference(commandReference)); + List assumedCommandParts = + result.getCommandDescription().getCommandReference(commandReference).getList(); sender.sendMessage(ChatColor.DARK_RED + "Unknown command, assuming " + ChatColor.GOLD + "/" - + assumedCommandParts + ChatColor.DARK_RED + "!"); + + CommandUtils.labelsToString(assumedCommandParts) + ChatColor.DARK_RED + "!"); } } diff --git a/src/main/java/fr/xephi/authme/command/CommandParts.java b/src/main/java/fr/xephi/authme/command/CommandParts.java index 58df26011..eb6299db5 100644 --- a/src/main/java/fr/xephi/authme/command/CommandParts.java +++ b/src/main/java/fr/xephi/authme/command/CommandParts.java @@ -23,15 +23,6 @@ public class CommandParts { this.parts.add(part); } - /** - * Constructor. - * - * @param commandParts The command parts instance. - */ - public CommandParts(CommandParts commandParts) { - this.parts.addAll(commandParts.getList()); - } - /** * Constructor. * @@ -60,7 +51,7 @@ public class CommandParts { } /** - * Get a part by it's index. + * Get a part by its index. * * @param i Part index. * @@ -75,44 +66,6 @@ public class CommandParts { return this.parts.get(i); } - /** - * Get a range of the parts starting at the specified index up to the end of the range. - * - * @param start The starting index. - * - * @return The parts range. Arguments that were out of bound are not included. - */ - public List getRange(int start) { - return getRange(start, getCount() - start); - } - - /** - * Get a range of the parts. - * - * @param start The starting index. - * @param count The number of parts to get. - * - * @return The parts range. Parts that were out of bound are not included. - */ - @Deprecated - public List getRange(int start, int count) { - // Create a new list to put the range into - List elements = new ArrayList<>(); - - // Get the range - for (int i = start; i < start + count; i++) { - // Get the part and add it if it's valid - String element = get(i); - if (element != null) - elements.add(element); - } - - // Return the list of parts - return elements; - } - - - /** * Convert the parts to a string. * diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/SwitchAntiBotCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/SwitchAntiBotCommand.java index fb844c607..3c0478676 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/SwitchAntiBotCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/SwitchAntiBotCommand.java @@ -2,11 +2,15 @@ package fr.xephi.authme.command.executable.authme; import fr.xephi.authme.AntiBot; import fr.xephi.authme.command.CommandParts; +import fr.xephi.authme.command.CommandUtils; import fr.xephi.authme.command.ExecutableCommand; import fr.xephi.authme.command.help.HelpProvider; +import fr.xephi.authme.util.CollectionUtils; import org.bukkit.ChatColor; import org.bukkit.command.CommandSender; +import java.util.List; + /** */ public class SwitchAntiBotCommand extends ExecutableCommand { @@ -32,16 +36,16 @@ public class SwitchAntiBotCommand extends ExecutableCommand { } // Enable the mod - if (newState.equalsIgnoreCase("ON")) { + if ("ON".equalsIgnoreCase(newState)) { AntiBot.overrideAntiBotStatus(true); - sender.sendMessage("[AuthMe] AntiBot Manual Ovverride: enabled!"); + sender.sendMessage("[AuthMe] AntiBot Manual Override: enabled!"); return true; } // Disable the mod - if (newState.equalsIgnoreCase("OFF")) { + if ("OFF".equalsIgnoreCase(newState)) { AntiBot.overrideAntiBotStatus(false); - sender.sendMessage("[AuthMe] AntiBotMod Manual Ovverride: disabled!"); + sender.sendMessage("[AuthMe] AntiBotMod Manual Override: disabled!"); return true; } @@ -52,8 +56,9 @@ public class SwitchAntiBotCommand extends ExecutableCommand { HelpProvider.showHelp(sender, commandReference, commandReference, true, false, true, false, false, false); // Show the command to use for detailed help - CommandParts helpCommandReference = new CommandParts(commandReference.getRange(1)); - sender.sendMessage(ChatColor.GOLD + "Detailed help: " + ChatColor.WHITE + "/" + commandReference.get(0) + " help " + helpCommandReference); + List helpCommandReference = CollectionUtils.getRange(commandReference.getList(), 1); + sender.sendMessage(ChatColor.GOLD + "Detailed help: " + ChatColor.WHITE + "/" + + commandReference.get(0) + " help " + CommandUtils.labelsToString(helpCommandReference)); return true; } } diff --git a/src/main/java/fr/xephi/authme/command/help/HelpProvider.java b/src/main/java/fr/xephi/authme/command/help/HelpProvider.java index a2f3d1f2c..7f088acf3 100644 --- a/src/main/java/fr/xephi/authme/command/help/HelpProvider.java +++ b/src/main/java/fr/xephi/authme/command/help/HelpProvider.java @@ -3,9 +3,11 @@ package fr.xephi.authme.command.help; import fr.xephi.authme.AuthMe; import fr.xephi.authme.command.CommandDescription; import fr.xephi.authme.command.CommandParts; +import fr.xephi.authme.command.CommandUtils; import fr.xephi.authme.command.FoundCommandResult; import fr.xephi.authme.settings.Settings; +import fr.xephi.authme.util.CollectionUtils; import org.bukkit.ChatColor; import org.bukkit.command.CommandSender; @@ -86,13 +88,14 @@ public class HelpProvider { // Show the unknown command warning sender.sendMessage(ChatColor.DARK_RED + "No help found for '" + helpQuery + "'!"); - // Get the suggested command - CommandParts suggestedCommandParts = new CommandParts(result.getCommandDescription().getCommandReference(commandReference).getRange(1)); - // Show a command suggestion if available and the difference isn't too big - if (commandDifference < 0.75) - if (result.getCommandDescription() != null) - sender.sendMessage(ChatColor.YELLOW + "Did you mean " + ChatColor.GOLD + "/" + baseCommand + " help " + suggestedCommandParts + ChatColor.YELLOW + "?"); + if (commandDifference < 0.75 && result.getCommandDescription() != null) { + // Get the suggested command + List suggestedCommandParts = CollectionUtils.getRange( + result.getCommandDescription().getCommandReference(commandReference).getList(), 1); + sender.sendMessage(ChatColor.YELLOW + "Did you mean " + ChatColor.GOLD + "/" + baseCommand + + " help " + CommandUtils.labelsToString(suggestedCommandParts) + ChatColor.YELLOW + "?"); + } // Show the help command sender.sendMessage(ChatColor.YELLOW + "Use the command " + ChatColor.GOLD + "/" + baseCommand + " help" + ChatColor.YELLOW + " to view help."); @@ -102,10 +105,12 @@ public class HelpProvider { // Show a message when the command handler is assuming a command if (commandDifference > 0) { // Get the suggested command - CommandParts suggestedCommandParts = new CommandParts(result.getCommandDescription().getCommandReference(commandReference).getRange(1)); + List suggestedCommandParts = CollectionUtils.getRange( + result.getCommandDescription().getCommandReference(commandReference).getList(), 1); // Show the suggested command - sender.sendMessage(ChatColor.DARK_RED + "No help found, assuming '" + ChatColor.GOLD + suggestedCommandParts + ChatColor.DARK_RED + "'!"); + sender.sendMessage(ChatColor.DARK_RED + "No help found, assuming '" + ChatColor.GOLD + + CommandUtils.labelsToString(suggestedCommandParts) + ChatColor.DARK_RED + "'!"); } // Print the help header 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 023e4085b..5fa3f5e06 100644 --- a/src/main/java/fr/xephi/authme/command/help/HelpSyntaxHelper.java +++ b/src/main/java/fr/xephi/authme/command/help/HelpSyntaxHelper.java @@ -3,6 +3,8 @@ package fr.xephi.authme.command.help; import fr.xephi.authme.command.CommandArgumentDescription; import fr.xephi.authme.command.CommandDescription; import fr.xephi.authme.command.CommandParts; +import fr.xephi.authme.command.CommandUtils; +import fr.xephi.authme.util.CollectionUtils; import fr.xephi.authme.util.StringUtils; import org.bukkit.ChatColor; @@ -35,8 +37,8 @@ public final class HelpSyntaxHelper { // Get the help command reference, and the command label CommandParts helpCommandReference = commandDescription.getCommandReference(commandReference); - final String parentCommand = new CommandParts( - helpCommandReference.getRange(0, helpCommandReference.getCount() - 1)).toString(); + final String parentCommand = CommandUtils.labelsToString( + CollectionUtils.getRange(helpCommandReference.getList(), 0, helpCommandReference.getCount() - 1)); // Check whether the alternative label should be used String commandLabel; diff --git a/src/main/java/fr/xephi/authme/util/StringUtils.java b/src/main/java/fr/xephi/authme/util/StringUtils.java index 2e37bfce0..1751461d8 100644 --- a/src/main/java/fr/xephi/authme/util/StringUtils.java +++ b/src/main/java/fr/xephi/authme/util/StringUtils.java @@ -13,8 +13,6 @@ import java.util.Arrays; */ public final class StringUtils { - public static final String newline = System.getProperty("line.separator"); - private StringUtils() { // Utility class } From 44aa91dfffb2d674c85d096df71092be3283afe8 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 5 Dec 2015 00:10:45 +0100 Subject: [PATCH 10/10] Remove unnecessary constructor in CommandDescription - Remove one of two unnecessary constructors in CommandDescription (in favor of the builder) - Remove TODO about unlimited arguments (not currently a priority, will create a story instead of leaving a dangling TODO for ages in the code) --- .../command/CommandArgumentDescription.java | 2 - .../authme/command/CommandDescription.java | 65 +++++++++---------- 2 files changed, 32 insertions(+), 35 deletions(-) diff --git a/src/main/java/fr/xephi/authme/command/CommandArgumentDescription.java b/src/main/java/fr/xephi/authme/command/CommandArgumentDescription.java index 5a9cd9d7c..f846b7893 100644 --- a/src/main/java/fr/xephi/authme/command/CommandArgumentDescription.java +++ b/src/main/java/fr/xephi/authme/command/CommandArgumentDescription.java @@ -5,8 +5,6 @@ package fr.xephi.authme.command; */ public class CommandArgumentDescription { - // TODO: Allow argument to consist of infinite parts.