From c78e12de046c331bc3947ed335a195d10d505183 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Fri, 4 Dec 2015 21:33:50 +0100 Subject: [PATCH 01/28] 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 02/28] 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 03/28] 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 04/28] 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 05/28] 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.