diff --git a/src/main/java/fr/xephi/authme/command/CommandDescription.java b/src/main/java/fr/xephi/authme/command/CommandDescription.java index 7ff0e7394..115d0f23d 100644 --- a/src/main/java/fr/xephi/authme/command/CommandDescription.java +++ b/src/main/java/fr/xephi/authme/command/CommandDescription.java @@ -13,7 +13,7 @@ import static java.util.Arrays.asList; /** * Command description – defines which labels ("names") will lead to a command and points to the * {@link ExecutableCommand} implementation that executes the logic of the command. - * + *

* CommandDescription instances are built hierarchically: they have one parent, or {@code null} for base commands * (main commands such as {@code /authme}), and may have multiple children extending the mapping of the parent: e.g. if * {@code /authme} has a child whose label is {@code "register"}, then {@code /authme register} is the command that @@ -37,7 +37,7 @@ public class CommandDescription { /** * The executable command instance described by this object. */ - private ExecutableCommand executableCommand; + private Class executableCommand; /** * The parent command. */ @@ -57,7 +57,7 @@ public class CommandDescription { /** * Private constructor. Use {@link CommandDescription#builder()} to create instances of this class. - *

+ *

* Note for developers: Instances should be created with {@link CommandDescription#createInstance} to be properly * registered in the command tree. */ @@ -67,21 +67,20 @@ public class CommandDescription { /** * Create an instance. * - * @param labels List of command labels. - * @param description Command description. - * @param detailedDescription Detailed comment description. - * @param executableCommand The executable command, or null. - * @param parent Parent command. - * @param arguments Command arguments. - * @param permission The permission node required to execute this command. + * @param labels command labels + * @param description description of the command + * @param detailedDescription detailed command description + * @param executableCommand class of the command implementation + * @param parent parent command + * @param arguments command arguments + * @param permission permission node required to execute this command * - * @return The created instance + * @return the created instance * @see CommandDescription#builder() */ private static CommandDescription createInstance(List labels, String description, - String detailedDescription, ExecutableCommand executableCommand, - CommandDescription parent, List arguments, - PermissionNode permission) { + String detailedDescription, Class executableCommand, CommandDescription parent, + List arguments, PermissionNode permission) { CommandDescription instance = new CommandDescription(); instance.labels = labels; instance.description = description; @@ -133,7 +132,7 @@ public class CommandDescription { * * @return The executable command object. */ - public ExecutableCommand getExecutableCommand() { + public Class getExecutableCommand() { return executableCommand; } @@ -219,7 +218,7 @@ public class CommandDescription { private List labels; private String description; private String detailedDescription; - private ExecutableCommand executableCommand; + private Class executableCommand; private CommandDescription parent; private List arguments = new ArrayList<>(); private PermissionNode permission; @@ -260,7 +259,7 @@ public class CommandDescription { return this; } - public CommandBuilder executableCommand(ExecutableCommand executableCommand) { + public CommandBuilder executableCommand(Class executableCommand) { this.executableCommand = executableCommand; return this; } diff --git a/src/main/java/fr/xephi/authme/command/CommandHandler.java b/src/main/java/fr/xephi/authme/command/CommandHandler.java index 8f493acbd..0b76a8222 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.initialization.AuthMeServiceInitializer; import fr.xephi.authme.permission.PermissionsManager; import fr.xephi.authme.util.StringUtils; import org.bukkit.ChatColor; @@ -9,7 +10,10 @@ import org.bukkit.command.CommandSender; import javax.inject.Inject; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; +import java.util.Set; /** * The AuthMe command handler, responsible for mapping incoming commands to the correct {@link CommandDescription} @@ -23,12 +27,23 @@ public class CommandHandler { */ private static final double SUGGEST_COMMAND_THRESHOLD = 0.75; - @Inject - private CommandService commandService; + private final CommandMapper commandMapper; + private final PermissionsManager permissionsManager; + private final HelpProvider helpProvider; + + /** + * Map with ExecutableCommand children. The key is the type of the value. + */ + private Map, ExecutableCommand> commands = new HashMap<>(); @Inject - private PermissionsManager permissionsManager; - + public CommandHandler(AuthMeServiceInitializer initializer, CommandMapper commandMapper, + PermissionsManager permissionsManager, HelpProvider helpProvider) { + this.commandMapper = commandMapper; + this.permissionsManager = permissionsManager; + this.helpProvider = helpProvider; + initializeCommands(initializer, commandMapper.getCommandClasses()); + } /** * Map a command that was invoked to the proper {@link CommandDescription} or return a useful error @@ -45,7 +60,7 @@ public class CommandHandler { List parts = skipEmptyArguments(bukkitArgs); parts.add(0, bukkitCommandLabel); - FoundCommandResult result = commandService.mapPartsToCommand(sender, parts); + FoundCommandResult result = commandMapper.mapPartsToCommand(sender, parts); handleCommandResult(sender, result); return !FoundResultStatus.MISSING_BASE_COMMAND.equals(result.getResultStatus()); } @@ -72,6 +87,18 @@ public class CommandHandler { } } + /** + * Initializes all required ExecutableCommand objects. + * + * @param commandClasses the classes to instantiate + */ + private void initializeCommands(AuthMeServiceInitializer initializer, + Set> commandClasses) { + for (Class clazz : commandClasses) { + commands.put(clazz, initializer.newInstance(clazz)); + } + } + /** * Execute the command for the given command sender. * @@ -79,7 +106,7 @@ public class CommandHandler { * @param result The mapped result */ private void executeCommand(CommandSender sender, FoundCommandResult result) { - ExecutableCommand executableCommand = result.getCommandDescription().getExecutableCommand(); + ExecutableCommand executableCommand = commands.get(result.getCommandDescription().getExecutableCommand()); List arguments = result.getArguments(); executableCommand.executeCommand(sender, arguments); } @@ -129,7 +156,7 @@ public class CommandHandler { // Show the command argument help sender.sendMessage(ChatColor.DARK_RED + "Incorrect command arguments!"); - commandService.outputHelp(sender, result, HelpProvider.SHOW_ARGUMENTS); + helpProvider.outputHelp(sender, result, HelpProvider.SHOW_ARGUMENTS); List labels = result.getLabels(); String childLabel = labels.size() >= 2 ? labels.get(1) : ""; diff --git a/src/main/java/fr/xephi/authme/command/CommandInitializer.java b/src/main/java/fr/xephi/authme/command/CommandInitializer.java index 7f6b2e26d..ce02b9a8d 100644 --- a/src/main/java/fr/xephi/authme/command/CommandInitializer.java +++ b/src/main/java/fr/xephi/authme/command/CommandInitializer.java @@ -33,11 +33,9 @@ import fr.xephi.authme.command.executable.login.LoginCommand; import fr.xephi.authme.command.executable.logout.LogoutCommand; import fr.xephi.authme.command.executable.register.RegisterCommand; import fr.xephi.authme.command.executable.unregister.UnregisterCommand; -import fr.xephi.authme.initialization.AuthMeServiceInitializer; import fr.xephi.authme.permission.AdminPermission; import fr.xephi.authme.permission.PlayerPermission; -import javax.inject.Inject; import java.util.Arrays; import java.util.Collection; import java.util.List; @@ -48,16 +46,17 @@ import java.util.Set; */ public class CommandInitializer { - private AuthMeServiceInitializer initializer; - private Set commands; - @Inject - public CommandInitializer(AuthMeServiceInitializer initializer) { - this.initializer = initializer; + public CommandInitializer() { buildCommands(); } + /** + * Returns the description of all AuthMe commands. + * + * @return the command descriptions + */ public Set getCommands() { return commands; } @@ -68,7 +67,7 @@ public class CommandInitializer { .labels("authme") .description("Main command") .detailedDescription("The main AuthMeReloaded command. The root for all admin commands.") - .executableCommand(initializer.newInstance(AuthMeCommand.class)) + .executableCommand(AuthMeCommand.class) .build(); // Register the register command @@ -80,7 +79,7 @@ public class CommandInitializer { .withArgument("player", "Player name", false) .withArgument("password", "Password", false) .permission(AdminPermission.REGISTER) - .executableCommand(initializer.newInstance(RegisterAdminCommand.class)) + .executableCommand(RegisterAdminCommand.class) .build(); // Register the unregister command @@ -91,7 +90,7 @@ public class CommandInitializer { .detailedDescription("Unregister the specified player.") .withArgument("player", "Player name", false) .permission(AdminPermission.UNREGISTER) - .executableCommand(initializer.newInstance(UnregisterAdminCommand.class)) + .executableCommand(UnregisterAdminCommand.class) .build(); // Register the forcelogin command @@ -102,7 +101,7 @@ public class CommandInitializer { .detailedDescription("Enforce the specified player to login.") .withArgument("player", "Online player name", true) .permission(AdminPermission.FORCE_LOGIN) - .executableCommand(initializer.newInstance(ForceLoginCommand.class)) + .executableCommand(ForceLoginCommand.class) .build(); // Register the changepassword command @@ -114,7 +113,7 @@ public class CommandInitializer { .withArgument("player", "Player name", false) .withArgument("pwd", "New password", false) .permission(AdminPermission.CHANGE_PASSWORD) - .executableCommand(initializer.newInstance(ChangePasswordAdminCommand.class)) + .executableCommand(ChangePasswordAdminCommand.class) .build(); // Register the last login command @@ -125,7 +124,7 @@ public class CommandInitializer { .detailedDescription("View the date of the specified players last login.") .withArgument("player", "Player name", true) .permission(AdminPermission.LAST_LOGIN) - .executableCommand(initializer.newInstance(LastLoginCommand.class)) + .executableCommand(LastLoginCommand.class) .build(); // Register the accounts command @@ -136,7 +135,7 @@ public class CommandInitializer { .detailedDescription("Display all accounts of a player by his player name or IP.") .withArgument("player", "Player name or IP", true) .permission(AdminPermission.ACCOUNTS) - .executableCommand(initializer.newInstance(AccountsCommand.class)) + .executableCommand(AccountsCommand.class) .build(); // Register the getemail command @@ -147,7 +146,7 @@ public class CommandInitializer { .detailedDescription("Display the email address of the specified player if set.") .withArgument("player", "Player name", true) .permission(AdminPermission.GET_EMAIL) - .executableCommand(initializer.newInstance(GetEmailCommand.class)) + .executableCommand(GetEmailCommand.class) .build(); // Register the setemail command @@ -159,7 +158,7 @@ public class CommandInitializer { .withArgument("player", "Player name", false) .withArgument("email", "Player email", false) .permission(AdminPermission.CHANGE_EMAIL) - .executableCommand(initializer.newInstance(SetEmailCommand.class)) + .executableCommand(SetEmailCommand.class) .build(); // Register the getip command @@ -170,7 +169,7 @@ public class CommandInitializer { .detailedDescription("Get the IP address of the specified online player.") .withArgument("player", "Player name", false) .permission(AdminPermission.GET_IP) - .executableCommand(initializer.newInstance(GetIpCommand.class)) + .executableCommand(GetIpCommand.class) .build(); // Register the spawn command @@ -180,7 +179,7 @@ public class CommandInitializer { .description("Teleport to spawn") .detailedDescription("Teleport to the spawn.") .permission(AdminPermission.SPAWN) - .executableCommand(initializer.newInstance(SpawnCommand.class)) + .executableCommand(SpawnCommand.class) .build(); // Register the setspawn command @@ -190,7 +189,7 @@ public class CommandInitializer { .description("Change the spawn") .detailedDescription("Change the player's spawn to your current position.") .permission(AdminPermission.SET_SPAWN) - .executableCommand(initializer.newInstance(SetSpawnCommand.class)) + .executableCommand(SetSpawnCommand.class) .build(); // Register the firstspawn command @@ -200,7 +199,7 @@ public class CommandInitializer { .description("Teleport to first spawn") .detailedDescription("Teleport to the first spawn.") .permission(AdminPermission.FIRST_SPAWN) - .executableCommand(initializer.newInstance(FirstSpawnCommand.class)) + .executableCommand(FirstSpawnCommand.class) .build(); // Register the setfirstspawn command @@ -210,7 +209,7 @@ public class CommandInitializer { .description("Change the first spawn") .detailedDescription("Change the first player's spawn to your current position.") .permission(AdminPermission.SET_FIRST_SPAWN) - .executableCommand(initializer.newInstance(SetFirstSpawnCommand.class)) + .executableCommand(SetFirstSpawnCommand.class) .build(); // Register the purge command @@ -221,7 +220,7 @@ public class CommandInitializer { .detailedDescription("Purge old AuthMeReloaded data longer than the specified amount of days ago.") .withArgument("days", "Number of days", false) .permission(AdminPermission.PURGE) - .executableCommand(initializer.newInstance(PurgeCommand.class)) + .executableCommand(PurgeCommand.class) .build(); // Register the purgelastposition command @@ -233,7 +232,7 @@ public class CommandInitializer { .detailedDescription("Purge the last know position of the specified player or all of them.") .withArgument("player/*", "Player name or * for all players", false) .permission(AdminPermission.PURGE_LAST_POSITION) - .executableCommand(initializer.newInstance(PurgeLastPositionCommand.class)) + .executableCommand(PurgeLastPositionCommand.class) .build(); // Register the purgebannedplayers command @@ -243,7 +242,7 @@ public class CommandInitializer { .description("Purge banned players data") .detailedDescription("Purge all AuthMeReloaded data for banned players.") .permission(AdminPermission.PURGE_BANNED_PLAYERS) - .executableCommand(initializer.newInstance(PurgeBannedPlayersCommand.class)) + .executableCommand(PurgeBannedPlayersCommand.class) .build(); // Register the switchantibot command @@ -254,7 +253,7 @@ public class CommandInitializer { .detailedDescription("Switch or toggle the AntiBot mode to the specified state.") .withArgument("mode", "ON / OFF", true) .permission(AdminPermission.SWITCH_ANTIBOT) - .executableCommand(initializer.newInstance(SwitchAntiBotCommand.class)) + .executableCommand(SwitchAntiBotCommand.class) .build(); // Register the reload command @@ -264,7 +263,7 @@ public class CommandInitializer { .description("Reload plugin") .detailedDescription("Reload the AuthMeReloaded plugin.") .permission(AdminPermission.RELOAD) - .executableCommand(initializer.newInstance(ReloadCommand.class)) + .executableCommand(ReloadCommand.class) .build(); // Register the version command @@ -274,7 +273,7 @@ public class CommandInitializer { .description("Version info") .detailedDescription("Show detailed information about the installed AuthMeReloaded version, the " + "developers, contributors, and license.") - .executableCommand(initializer.newInstance(VersionCommand.class)) + .executableCommand(VersionCommand.class) .build(); CommandDescription.builder() @@ -285,7 +284,7 @@ public class CommandInitializer { .withArgument("job", "Conversion job: xauth / crazylogin / rakamak / " + "royalauth / vauth / sqlitetosql", false) .permission(AdminPermission.CONVERTER) - .executableCommand(initializer.newInstance(ConverterCommand.class)) + .executableCommand(ConverterCommand.class) .build(); // Register the base login command @@ -296,7 +295,7 @@ public class CommandInitializer { .detailedDescription("Command to log in using AuthMeReloaded.") .withArgument("password", "Login password", false) .permission(PlayerPermission.LOGIN) - .executableCommand(initializer.newInstance(LoginCommand.class)) + .executableCommand(LoginCommand.class) .build(); // Register the base logout command @@ -306,7 +305,7 @@ public class CommandInitializer { .description("Logout command") .detailedDescription("Command to logout using AuthMeReloaded.") .permission(PlayerPermission.LOGOUT) - .executableCommand(initializer.newInstance(LogoutCommand.class)) + .executableCommand(LogoutCommand.class) .build(); // Register the base register command @@ -318,7 +317,7 @@ public class CommandInitializer { .withArgument("password", "Password", true) .withArgument("verifyPassword", "Verify password", true) .permission(PlayerPermission.REGISTER) - .executableCommand(initializer.newInstance(RegisterCommand.class)) + .executableCommand(RegisterCommand.class) .build(); // Register the base unregister command @@ -329,7 +328,7 @@ public class CommandInitializer { .detailedDescription("Command to unregister using AuthMeReloaded.") .withArgument("password", "Password", false) .permission(PlayerPermission.UNREGISTER) - .executableCommand(initializer.newInstance(UnregisterCommand.class)) + .executableCommand(UnregisterCommand.class) .build(); // Register the base changepassword command @@ -341,7 +340,7 @@ public class CommandInitializer { .withArgument("oldPassword", "Old Password", false) .withArgument("newPassword", "New Password.", false) .permission(PlayerPermission.CHANGE_PASSWORD) - .executableCommand(initializer.newInstance(ChangePasswordCommand.class)) + .executableCommand(ChangePasswordCommand.class) .build(); // Register the base Email command @@ -350,7 +349,7 @@ public class CommandInitializer { .labels("email") .description("Email command") .detailedDescription("The AuthMeReloaded Email command base.") - .executableCommand(initializer.newInstance(EmailBaseCommand.class)) + .executableCommand(EmailBaseCommand.class) .build(); // Register the add command @@ -362,7 +361,7 @@ public class CommandInitializer { .withArgument("email", "Email address", false) .withArgument("verifyEmail", "Email address verification", false) .permission(PlayerPermission.ADD_EMAIL) - .executableCommand(initializer.newInstance(AddEmailCommand.class)) + .executableCommand(AddEmailCommand.class) .build(); // Register the change command @@ -374,7 +373,7 @@ public class CommandInitializer { .withArgument("oldEmail", "Old email address", false) .withArgument("newEmail", "New email address", false) .permission(PlayerPermission.CHANGE_EMAIL) - .executableCommand(initializer.newInstance(ChangeEmailCommand.class)) + .executableCommand(ChangeEmailCommand.class) .build(); // Register the recover command @@ -386,7 +385,7 @@ public class CommandInitializer { "a new password.") .withArgument("email", "Email address", false) .permission(PlayerPermission.RECOVER_EMAIL) - .executableCommand(initializer.newInstance(RecoverEmailCommand.class)) + .executableCommand(RecoverEmailCommand.class) .build(); // Register the base captcha command @@ -397,7 +396,7 @@ public class CommandInitializer { .detailedDescription("Captcha command for AuthMeReloaded.") .withArgument("captcha", "The Captcha", false) .permission(PlayerPermission.CAPTCHA) - .executableCommand(initializer.newInstance(CaptchaCommand.class)) + .executableCommand(CaptchaCommand.class) .build(); Set baseCommands = ImmutableSet.of( @@ -415,12 +414,11 @@ public class CommandInitializer { } /** - * Set the help command on all base commands, e.g. to register /authme help or /register help. + * Sets the help command on all base commands, e.g. to register /authme help or /register help. * - * @param commands The list of base commands to register a help child command on + * @param commands the list of base commands to register a help child command on */ private void setHelpOnAllBases(Collection commands) { - final HelpCommand helpCommandExecutable = initializer.newInstance(HelpCommand.class); final List helpCommandLabels = Arrays.asList("help", "hlp", "h", "sos", "?"); for (CommandDescription base : commands) { @@ -430,7 +428,7 @@ public class CommandInitializer { .description("View help") .detailedDescription("View detailed help for /" + base.getLabels().get(0) + " commands.") .withArgument("query", "The command or query to view help for.", true) - .executableCommand(helpCommandExecutable) + .executableCommand(HelpCommand.class) .build(); } } diff --git a/src/main/java/fr/xephi/authme/command/CommandMapper.java b/src/main/java/fr/xephi/authme/command/CommandMapper.java index 755cea0df..d0706c80b 100644 --- a/src/main/java/fr/xephi/authme/command/CommandMapper.java +++ b/src/main/java/fr/xephi/authme/command/CommandMapper.java @@ -8,6 +8,7 @@ import org.bukkit.command.CommandSender; import javax.inject.Inject; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; import java.util.Set; @@ -69,6 +70,23 @@ public class CommandMapper { return getCommandWithSmallestDifference(base, parts); } + /** + * Return all {@link ExecutableCommand} classes referenced in {@link CommandDescription} objects. + * + * @return all classes + * @see CommandInitializer#getCommands + */ + public Set> getCommandClasses() { + Set> classes = new HashSet<>(50); + for (CommandDescription command : baseCommands) { + classes.add(command.getExecutableCommand()); + for (CommandDescription child : command.getChildren()) { + classes.add(child.getExecutableCommand()); + } + } + return classes; + } + private FoundCommandResult getCommandWithSmallestDifference(CommandDescription base, List parts) { // Return the base command with incorrect arg count error if we only have one part if (parts.size() <= 1) { @@ -141,7 +159,7 @@ public class CommandMapper { private static FoundCommandResult transformResultForHelp(FoundCommandResult result) { if (result.getCommandDescription() != null - && HELP_COMMAND_CLASS.isAssignableFrom(result.getCommandDescription().getExecutableCommand().getClass())) { + && HELP_COMMAND_CLASS.isAssignableFrom(result.getCommandDescription().getExecutableCommand())) { // For "/authme help register" we have labels = [authme, help] and arguments = [register] // But for the help command we want labels = [authme, help] and arguments = [authme, register], // so we can use the arguments as the labels to the command to show help for diff --git a/src/main/java/fr/xephi/authme/command/CommandService.java b/src/main/java/fr/xephi/authme/command/CommandService.java index acf150566..5efe79e80 100644 --- a/src/main/java/fr/xephi/authme/command/CommandService.java +++ b/src/main/java/fr/xephi/authme/command/CommandService.java @@ -1,6 +1,5 @@ package fr.xephi.authme.command; -import fr.xephi.authme.command.help.HelpProvider; import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.output.Messages; import fr.xephi.authme.settings.NewSetting; @@ -9,7 +8,6 @@ import fr.xephi.authme.util.ValidationService; import org.bukkit.command.CommandSender; import javax.inject.Inject; -import java.util.List; /** * Service for implementations of {@link ExecutableCommand} to execute some common tasks. @@ -20,10 +18,6 @@ public class CommandService { @Inject private Messages messages; @Inject - private HelpProvider helpProvider; - @Inject - private CommandMapper commandMapper; - @Inject private NewSetting settings; @Inject private ValidationService validationService; @@ -49,31 +43,6 @@ public class CommandService { messages.send(sender, messageKey, replacements); } - /** - * Map command parts to a command description. - * - * @param sender The command sender issuing the request (for permission check), or null to skip permissions - * @param commandParts The received command parts to map to a command - * @return The computed mapping result - */ - public FoundCommandResult mapPartsToCommand(CommandSender sender, List commandParts) { - return commandMapper.mapPartsToCommand(sender, commandParts); - } - - /** - * Output the help for a given command. - * - * @param sender The sender to output the help to - * @param result The result to output information about - * @param options Output options, see {@link HelpProvider} - */ - public void outputHelp(CommandSender sender, FoundCommandResult result, int options) { - List lines = helpProvider.printHelp(sender, result, options); - for (String line : lines) { - sender.sendMessage(line); - } - } - /** * Retrieve a message by its message key. * diff --git a/src/main/java/fr/xephi/authme/command/executable/HelpCommand.java b/src/main/java/fr/xephi/authme/command/executable/HelpCommand.java index e912fb12d..2076bcbce 100644 --- a/src/main/java/fr/xephi/authme/command/executable/HelpCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/HelpCommand.java @@ -1,6 +1,6 @@ package fr.xephi.authme.command.executable; -import fr.xephi.authme.command.CommandService; +import fr.xephi.authme.command.CommandMapper; import fr.xephi.authme.command.CommandUtils; import fr.xephi.authme.command.ExecutableCommand; import fr.xephi.authme.command.FoundCommandResult; @@ -18,14 +18,17 @@ import static fr.xephi.authme.command.FoundResultStatus.UNKNOWN_LABEL; public class HelpCommand implements ExecutableCommand { @Inject - private CommandService commandService; + private CommandMapper commandMapper; + + @Inject + private HelpProvider helpProvider; // Convention: arguments is not the actual invoked arguments but the command that was invoked, // e.g. "/authme help register" would typically be arguments = [register], but here we pass [authme, register] @Override public void executeCommand(CommandSender sender, List arguments) { - FoundCommandResult result = commandService.mapPartsToCommand(sender, arguments); + FoundCommandResult result = commandMapper.mapPartsToCommand(sender, arguments); FoundResultStatus resultStatus = result.getResultStatus(); if (MISSING_BASE_COMMAND.equals(resultStatus)) { @@ -43,9 +46,9 @@ public class HelpCommand implements ExecutableCommand { int mappedCommandLevel = result.getCommandDescription().getLabelCount(); if (mappedCommandLevel == 1) { - commandService.outputHelp(sender, result, HelpProvider.SHOW_CHILDREN); + helpProvider.outputHelp(sender, result, HelpProvider.SHOW_CHILDREN); } else { - commandService.outputHelp(sender, result, HelpProvider.ALL_OPTIONS); + helpProvider.outputHelp(sender, result, HelpProvider.ALL_OPTIONS); } } 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 37a709cf9..64a7241f2 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 @@ -1,7 +1,7 @@ package fr.xephi.authme.command.executable.authme; import fr.xephi.authme.AntiBot; -import fr.xephi.authme.command.CommandService; +import fr.xephi.authme.command.CommandMapper; import fr.xephi.authme.command.ExecutableCommand; import fr.xephi.authme.command.FoundCommandResult; import fr.xephi.authme.command.help.HelpProvider; @@ -21,7 +21,10 @@ public class SwitchAntiBotCommand implements ExecutableCommand { private AntiBot antiBot; @Inject - private CommandService commandService; + private CommandMapper commandMapper; + + @Inject + private HelpProvider helpProvider; @Override public void executeCommand(final CommandSender sender, List arguments) { @@ -41,8 +44,8 @@ public class SwitchAntiBotCommand implements ExecutableCommand { sender.sendMessage("[AuthMe] AntiBot Manual Override: disabled!"); } else { sender.sendMessage(ChatColor.DARK_RED + "Invalid AntiBot mode!"); - FoundCommandResult result = commandService.mapPartsToCommand(sender, Arrays.asList("authme", "antibot")); - commandService.outputHelp(sender, result, HelpProvider.SHOW_ARGUMENTS); + FoundCommandResult result = commandMapper.mapPartsToCommand(sender, Arrays.asList("authme", "antibot")); + helpProvider.outputHelp(sender, result, HelpProvider.SHOW_ARGUMENTS); sender.sendMessage(ChatColor.GOLD + "Detailed help: " + ChatColor.WHITE + "/authme help antibot"); } } diff --git a/src/main/java/fr/xephi/authme/command/executable/email/EmailBaseCommand.java b/src/main/java/fr/xephi/authme/command/executable/email/EmailBaseCommand.java index d75973f0d..0484e2189 100644 --- a/src/main/java/fr/xephi/authme/command/executable/email/EmailBaseCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/email/EmailBaseCommand.java @@ -1,6 +1,6 @@ package fr.xephi.authme.command.executable.email; -import fr.xephi.authme.command.CommandService; +import fr.xephi.authme.command.CommandMapper; import fr.xephi.authme.command.ExecutableCommand; import fr.xephi.authme.command.FoundCommandResult; import fr.xephi.authme.command.help.HelpProvider; @@ -16,11 +16,14 @@ import java.util.List; public class EmailBaseCommand implements ExecutableCommand { @Inject - private CommandService commandService; + private CommandMapper commandMapper; + + @Inject + private HelpProvider helpProvider; @Override public void executeCommand(CommandSender sender, List arguments) { - FoundCommandResult result = commandService.mapPartsToCommand(sender, Collections.singletonList("email")); - commandService.outputHelp(sender, result, HelpProvider.SHOW_CHILDREN); + FoundCommandResult result = commandMapper.mapPartsToCommand(sender, Collections.singletonList("email")); + helpProvider.outputHelp(sender, result, HelpProvider.SHOW_CHILDREN); } } 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 7257b4d8b..4e07639df 100644 --- a/src/main/java/fr/xephi/authme/command/help/HelpProvider.java +++ b/src/main/java/fr/xephi/authme/command/help/HelpProvider.java @@ -53,7 +53,7 @@ public class HelpProvider implements SettingsDependent { loadSettings(settings); } - public List printHelp(CommandSender sender, FoundCommandResult result, int options) { + private List printHelp(CommandSender sender, FoundCommandResult result, int options) { if (result.getCommandDescription() == null) { return singletonList(ChatColor.DARK_RED + "Failed to retrieve any help information!"); } @@ -87,6 +87,20 @@ public class HelpProvider implements SettingsDependent { return lines; } + /** + * Output the help for a given command. + * + * @param sender The sender to output the help to + * @param result The result to output information about + * @param options Output options, see {@link HelpProvider} + */ + public void outputHelp(CommandSender sender, FoundCommandResult result, int options) { + List lines = printHelp(sender, result, options); + for (String line : lines) { + sender.sendMessage(line); + } + } + @Override public void loadSettings(NewSetting settings) { helpHeader = settings.getProperty(PluginSettings.HELP_HEADER); diff --git a/src/test/java/fr/xephi/authme/command/CommandConsistencyTest.java b/src/test/java/fr/xephi/authme/command/CommandConsistencyTest.java index bb297268c..287b398a2 100644 --- a/src/test/java/fr/xephi/authme/command/CommandConsistencyTest.java +++ b/src/test/java/fr/xephi/authme/command/CommandConsistencyTest.java @@ -1,13 +1,9 @@ package fr.xephi.authme.command; - -import fr.xephi.authme.initialization.AuthMeServiceInitializer; import org.bukkit.configuration.MemorySection; import org.bukkit.configuration.file.FileConfiguration; import org.bukkit.configuration.file.YamlConfiguration; import org.junit.Test; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.stubbing.Answer; import java.util.ArrayList; import java.util.Collection; @@ -22,9 +18,6 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.assertThat; -import static org.mockito.BDDMockito.given; -import static org.mockito.Matchers.any; -import static org.mockito.Mockito.mock; /** * Checks that the commands declared in plugin.yml correspond @@ -59,14 +52,7 @@ public class CommandConsistencyTest { */ @SuppressWarnings("unchecked") private static Collection> initializeCommands() { - AuthMeServiceInitializer injector = mock(AuthMeServiceInitializer.class); - given(injector.newInstance(any(Class.class))).willAnswer(new Answer() { - @Override - public Object answer(InvocationOnMock invocation) throws Throwable { - return mock((Class) invocation.getArguments()[0]); - } - }); - CommandInitializer initializer = new CommandInitializer(injector); + CommandInitializer initializer = new CommandInitializer(); Collection> commandLabels = new ArrayList<>(); for (CommandDescription baseCommand : initializer.getCommands()) { commandLabels.add(baseCommand.getLabels()); diff --git a/src/test/java/fr/xephi/authme/command/CommandHandlerTest.java b/src/test/java/fr/xephi/authme/command/CommandHandlerTest.java index d604a5b6d..ff3fd5a78 100644 --- a/src/test/java/fr/xephi/authme/command/CommandHandlerTest.java +++ b/src/test/java/fr/xephi/authme/command/CommandHandlerTest.java @@ -1,17 +1,25 @@ package fr.xephi.authme.command; +import com.google.common.collect.Sets; +import fr.xephi.authme.command.TestCommandsUtil.TestLoginCommand; +import fr.xephi.authme.command.TestCommandsUtil.TestRegisterCommand; +import fr.xephi.authme.command.TestCommandsUtil.TestUnregisterCommand; +import fr.xephi.authme.command.help.HelpProvider; +import fr.xephi.authme.initialization.AuthMeServiceInitializer; import fr.xephi.authme.permission.PermissionsManager; import org.bukkit.command.CommandSender; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; -import org.mockito.Captor; -import org.mockito.InjectMocks; import org.mockito.Mock; +import org.mockito.invocation.InvocationOnMock; import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.stubbing.Answer; import java.util.Collections; -import java.util.List; +import java.util.HashMap; +import java.util.Map; import static fr.xephi.authme.command.FoundResultStatus.INCORRECT_ARGUMENTS; import static fr.xephi.authme.command.FoundResultStatus.MISSING_BASE_COMMAND; @@ -19,7 +27,6 @@ import static fr.xephi.authme.command.FoundResultStatus.NO_PERMISSION; import static fr.xephi.authme.command.FoundResultStatus.SUCCESS; import static fr.xephi.authme.command.FoundResultStatus.UNKNOWN_LABEL; import static java.util.Arrays.asList; -import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsString; import static org.junit.Assert.assertThat; import static org.mockito.BDDMockito.given; @@ -29,6 +36,7 @@ import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.argThat; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -43,20 +51,53 @@ import static org.mockito.Mockito.verify; @RunWith(MockitoJUnitRunner.class) public class CommandHandlerTest { - @InjectMocks private CommandHandler handler; @Mock - private CommandService commandService; - + private AuthMeServiceInitializer initializer; + @Mock + private CommandInitializer commandInitializer; @Mock private CommandMapper commandMapper; - @Mock private PermissionsManager permissionsManager; + @Mock + private HelpProvider helpProvider; - @Captor - private ArgumentCaptor> captor; + private Map, ExecutableCommand> mockedCommands = new HashMap<>(); + + @Before + @SuppressWarnings("unchecked") + public void initializeCommandMapper() { + given(commandMapper.getCommandClasses()).willReturn(Sets.newHashSet(ExecutableCommand.class, + TestLoginCommand.class, TestRegisterCommand.class, TestUnregisterCommand.class)); + setInjectorToMockExecutableCommandClasses(); + handler = new CommandHandler(initializer, commandMapper, permissionsManager, helpProvider); + } + + /** + * Makes the initializer return a mock when {@link AuthMeServiceInitializer#newInstance(Class)} is invoked + * with (a child of) ExecutableCommand.class. The mocks the initializer creates are stored in {@link #mockedCommands}. + *

+ * The {@link CommandMapper} is mocked in {@link #initializeCommandMapper()} to return certain test classes. + */ + private void setInjectorToMockExecutableCommandClasses() { + given(initializer.newInstance(any(Class.class))).willAnswer(new Answer() { + @Override + public Object answer(InvocationOnMock invocation) throws Throwable { + Class clazz = (Class) invocation.getArguments()[0]; + if (ExecutableCommand.class.isAssignableFrom(clazz)) { + @SuppressWarnings("unchecked") + Class commandClass = (Class) clazz; + ExecutableCommand mock = mock(commandClass); + mockedCommands.put(commandClass, mock); + return mock; + } + throw new IllegalStateException("Unexpected class '" + clazz.getName() + + "': Not a child of ExecutableCommand"); + } + }); + } @Test @@ -66,22 +107,18 @@ public class CommandHandlerTest { String[] bukkitArgs = {"Login", "myPass"}; CommandSender sender = mock(CommandSender.class); - ExecutableCommand executableCommand = mock(ExecutableCommand.class); CommandDescription command = mock(CommandDescription.class); - given(command.getExecutableCommand()).willReturn(executableCommand); - given(commandService.mapPartsToCommand(any(CommandSender.class), anyListOf(String.class))) + doReturn(TestLoginCommand.class).when(command).getExecutableCommand(); + given(commandMapper.mapPartsToCommand(any(CommandSender.class), anyListOf(String.class))) .willReturn(new FoundCommandResult(command, asList("Authme", "Login"), asList("myPass"), 0.0, SUCCESS)); // when handler.processCommand(sender, bukkitLabel, bukkitArgs); // then - verify(commandService).mapPartsToCommand(eq(sender), captor.capture()); - assertThat(captor.getValue(), contains("Authme", "Login", "myPass")); - - verify(executableCommand).executeCommand(eq(sender), captor.capture()); - assertThat(captor.getValue(), contains("myPass")); - + ExecutableCommand executableCommand = mockedCommands.get(TestLoginCommand.class); + verify(commandMapper).mapPartsToCommand(sender, asList("Authme", "Login", "myPass")); + verify(executableCommand).executeCommand(sender, asList("myPass")); // Ensure that no error message was issued to the command sender verify(sender, never()).sendMessage(anyString()); } @@ -93,15 +130,14 @@ public class CommandHandlerTest { String[] bukkitArgs = {"testPlayer"}; CommandSender sender = mock(CommandSender.class); CommandDescription command = mock(CommandDescription.class); - given(commandService.mapPartsToCommand(any(CommandSender.class), anyListOf(String.class))) + given(commandMapper.mapPartsToCommand(any(CommandSender.class), anyListOf(String.class))) .willReturn(new FoundCommandResult(command, asList("unreg"), asList("testPlayer"), 0.0, NO_PERMISSION)); // when handler.processCommand(sender, bukkitLabel, bukkitArgs); // then - verify(commandService).mapPartsToCommand(eq(sender), captor.capture()); - assertThat(captor.getValue(), contains("unreg", "testPlayer")); + verify(commandMapper).mapPartsToCommand(sender, asList("unreg", "testPlayer")); verify(command, never()).getExecutableCommand(); verify(sender).sendMessage(argThat(containsString("don't have permission"))); } @@ -113,7 +149,7 @@ public class CommandHandlerTest { String[] bukkitArgs = {"testPlayer"}; CommandSender sender = mock(CommandSender.class); CommandDescription command = mock(CommandDescription.class); - given(commandService.mapPartsToCommand(any(CommandSender.class), anyListOf(String.class))).willReturn( + given(commandMapper.mapPartsToCommand(any(CommandSender.class), anyListOf(String.class))).willReturn( new FoundCommandResult(command, asList("unreg"), asList("testPlayer"), 0.0, INCORRECT_ARGUMENTS)); given(permissionsManager.hasPermission(sender, command.getPermission())).willReturn(true); @@ -121,9 +157,7 @@ public class CommandHandlerTest { handler.processCommand(sender, bukkitLabel, bukkitArgs); // then - verify(commandService).mapPartsToCommand(eq(sender), captor.capture()); - assertThat(captor.getValue(), contains("unreg", "testPlayer")); - + verify(commandMapper).mapPartsToCommand(sender, asList("unreg", "testPlayer")); verify(command, never()).getExecutableCommand(); ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); verify(sender, atLeastOnce()).sendMessage(captor.capture()); @@ -137,7 +171,7 @@ public class CommandHandlerTest { String[] bukkitArgs = {"testPlayer"}; CommandSender sender = mock(CommandSender.class); CommandDescription command = mock(CommandDescription.class); - given(commandService.mapPartsToCommand(any(CommandSender.class), anyListOf(String.class))).willReturn( + given(commandMapper.mapPartsToCommand(any(CommandSender.class), anyListOf(String.class))).willReturn( new FoundCommandResult(command, asList("unreg"), asList("testPlayer"), 0.0, INCORRECT_ARGUMENTS)); given(permissionsManager.hasPermission(sender, command.getPermission())).willReturn(false); @@ -145,9 +179,7 @@ public class CommandHandlerTest { handler.processCommand(sender, bukkitLabel, bukkitArgs); // then - verify(commandService).mapPartsToCommand(eq(sender), captor.capture()); - assertThat(captor.getValue(), contains("unreg", "testPlayer")); - + verify(commandMapper).mapPartsToCommand(sender, asList("unreg", "testPlayer")); verify(command, never()).getExecutableCommand(); ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); verify(sender).sendMessage(captor.capture()); @@ -161,15 +193,14 @@ public class CommandHandlerTest { String[] bukkitArgs = {"testPlayer"}; CommandSender sender = mock(CommandSender.class); CommandDescription command = mock(CommandDescription.class); - given(commandService.mapPartsToCommand(any(CommandSender.class), anyListOf(String.class))).willReturn( + given(commandMapper.mapPartsToCommand(any(CommandSender.class), anyListOf(String.class))).willReturn( new FoundCommandResult(command, asList("unreg"), asList("testPlayer"), 0.0, MISSING_BASE_COMMAND)); // when handler.processCommand(sender, bukkitLabel, bukkitArgs); // then - verify(commandService).mapPartsToCommand(eq(sender), captor.capture()); - assertThat(captor.getValue(), contains("unreg", "testPlayer")); + verify(commandMapper).mapPartsToCommand(sender, asList("unreg", "testPlayer")); verify(command, never()).getExecutableCommand(); verify(sender).sendMessage(argThat(containsString("Failed to parse"))); } @@ -182,16 +213,14 @@ public class CommandHandlerTest { CommandSender sender = mock(CommandSender.class); CommandDescription command = mock(CommandDescription.class); given(command.getLabels()).willReturn(Collections.singletonList("test_cmd")); - given(commandService.mapPartsToCommand(any(CommandSender.class), anyListOf(String.class))).willReturn( + given(commandMapper.mapPartsToCommand(any(CommandSender.class), anyListOf(String.class))).willReturn( new FoundCommandResult(command, asList("unreg"), asList("testPlayer"), 0.01, UNKNOWN_LABEL)); // when handler.processCommand(sender, bukkitLabel, bukkitArgs); // then - verify(commandService).mapPartsToCommand(eq(sender), captor.capture()); - assertThat(captor.getValue(), contains("unreg", "testPlayer")); - + verify(commandMapper).mapPartsToCommand(sender, asList("unreg", "testPlayer")); verify(command, never()).getExecutableCommand(); ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); verify(sender, times(3)).sendMessage(captor.capture()); @@ -210,16 +239,14 @@ public class CommandHandlerTest { CommandSender sender = mock(CommandSender.class); CommandDescription command = mock(CommandDescription.class); given(command.getLabels()).willReturn(Collections.singletonList("test_cmd")); - given(commandService.mapPartsToCommand(any(CommandSender.class), anyListOf(String.class))).willReturn( + given(commandMapper.mapPartsToCommand(any(CommandSender.class), anyListOf(String.class))).willReturn( new FoundCommandResult(command, asList("unreg"), asList("testPlayer"), 1.0, UNKNOWN_LABEL)); // when handler.processCommand(sender, bukkitLabel, bukkitArgs); // then - verify(commandService).mapPartsToCommand(eq(sender), captor.capture()); - assertThat(captor.getValue(), contains("unreg", "testPlayer")); - + verify(commandMapper).mapPartsToCommand(sender, asList("unreg", "testPlayer")); verify(command, never()).getExecutableCommand(); ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); verify(sender, times(2)).sendMessage(captor.capture()); @@ -232,25 +259,21 @@ public class CommandHandlerTest { public void shouldStripWhitespace() { // given String bukkitLabel = "AuthMe"; - String[] bukkitArgs = {" ", "", "LOGIN", " ", "testArg", " "}; + String[] bukkitArgs = {" ", "", "REGISTER", " ", "testArg", " "}; CommandSender sender = mock(CommandSender.class); - ExecutableCommand executableCommand = mock(ExecutableCommand.class); CommandDescription command = mock(CommandDescription.class); - given(command.getExecutableCommand()).willReturn(executableCommand); - given(commandService.mapPartsToCommand(eq(sender), anyListOf(String.class))) - .willReturn(new FoundCommandResult(command, asList("AuthMe", "LOGIN"), asList("testArg"), 0.0, SUCCESS)); + doReturn(TestRegisterCommand.class).when(command).getExecutableCommand(); + given(commandMapper.mapPartsToCommand(eq(sender), anyListOf(String.class))) + .willReturn(new FoundCommandResult(command, asList("AuthMe", "REGISTER"), asList("testArg"), 0.0, SUCCESS)); // when handler.processCommand(sender, bukkitLabel, bukkitArgs); // then - verify(commandService).mapPartsToCommand(eq(sender), captor.capture()); - assertThat(captor.getValue(), contains("AuthMe", "LOGIN", "testArg")); - - verify(command.getExecutableCommand()).executeCommand(eq(sender), captor.capture()); - assertThat(captor.getValue(), contains("testArg")); - + ExecutableCommand executableCommand = mockedCommands.get(TestRegisterCommand.class); + verify(commandMapper).mapPartsToCommand(sender, asList("AuthMe", "REGISTER", "testArg")); + verify(executableCommand).executeCommand(sender, asList("testArg")); verify(sender, never()).sendMessage(anyString()); } diff --git a/src/test/java/fr/xephi/authme/command/CommandInitializerTest.java b/src/test/java/fr/xephi/authme/command/CommandInitializerTest.java index 81f4741a1..97268e491 100644 --- a/src/test/java/fr/xephi/authme/command/CommandInitializerTest.java +++ b/src/test/java/fr/xephi/authme/command/CommandInitializerTest.java @@ -1,13 +1,10 @@ package fr.xephi.authme.command; -import fr.xephi.authme.initialization.AuthMeServiceInitializer; import fr.xephi.authme.permission.AdminPermission; import fr.xephi.authme.permission.PermissionNode; import fr.xephi.authme.util.StringUtils; import org.junit.BeforeClass; import org.junit.Test; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.stubbing.Answer; import java.util.ArrayList; import java.util.Collection; @@ -20,14 +17,8 @@ import java.util.regex.Pattern; import static fr.xephi.authme.permission.DefaultPermission.OP_ONLY; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.not; -import static org.hamcrest.Matchers.nullValue; -import static org.junit.Assert.assertSame; import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; -import static org.mockito.Matchers.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; /** * Test for {@link CommandInitializer} to guarantee the integrity of the defined commands. @@ -45,15 +36,7 @@ public class CommandInitializerTest { @SuppressWarnings("unchecked") @BeforeClass public static void initializeCommandCollection() { - AuthMeServiceInitializer initializer = mock(AuthMeServiceInitializer.class); - when(initializer.newInstance(any(Class.class))).thenAnswer(new Answer() { - @Override - public Object answer(InvocationOnMock invocation) { - Class clazz = (Class) invocation.getArguments()[0]; - return mock(clazz); - } - }); - CommandInitializer commandInitializer = new CommandInitializer(initializer); + CommandInitializer commandInitializer = new CommandInitializer(); commands = commandInitializer.getCommands(); } @@ -174,34 +157,6 @@ public class CommandInitializerTest { walkThroughCommands(commands, descriptionTester); } - /** - * Check that the implementation of {@link ExecutableCommand} a command points to is the same for each type: - * it is inefficient to instantiate the same type multiple times. - */ - @Test - public void shouldNotHaveMultipleInstancesOfSameExecutableCommandSubType() { - // given - final Map, ExecutableCommand> implementations = new HashMap<>(); - BiConsumer descriptionTester = new BiConsumer() { - @Override - public void accept(CommandDescription command, int depth) { - assertThat(command.getExecutableCommand(), not(nullValue())); - ExecutableCommand commandExec = command.getExecutableCommand(); - ExecutableCommand storedExec = implementations.get(command.getExecutableCommand().getClass()); - if (storedExec == null) { - implementations.put(commandExec.getClass(), commandExec); - } else { - assertSame("has same implementation of '" + storedExec.getClass().getName() + "' for command with " - + "parent " + (command.getParent() == null ? "null" : command.getParent().getLabels()), - storedExec, commandExec); - } - } - }; - - // when/then - walkThroughCommands(commands, descriptionTester); - } - @Test public void shouldHaveOptionalArgumentsAfterMandatoryOnes() { // given @@ -291,7 +246,7 @@ public class CommandInitializerTest { } private void testCollectionForCommand(CommandDescription command, int argCount, Map, Integer> collection) { - final Class clazz = command.getExecutableCommand().getClass(); + final Class clazz = command.getExecutableCommand(); Integer existingCount = collection.get(clazz); if (existingCount == null) { collection.put(clazz, argCount); diff --git a/src/test/java/fr/xephi/authme/command/CommandMapperTest.java b/src/test/java/fr/xephi/authme/command/CommandMapperTest.java index 96eab841f..26e43ab0b 100644 --- a/src/test/java/fr/xephi/authme/command/CommandMapperTest.java +++ b/src/test/java/fr/xephi/authme/command/CommandMapperTest.java @@ -1,5 +1,9 @@ package fr.xephi.authme.command; +import fr.xephi.authme.command.TestCommandsUtil.TestLoginCommand; +import fr.xephi.authme.command.TestCommandsUtil.TestRegisterCommand; +import fr.xephi.authme.command.TestCommandsUtil.TestUnregisterCommand; +import fr.xephi.authme.command.executable.HelpCommand; import fr.xephi.authme.permission.PermissionNode; import fr.xephi.authme.permission.PermissionsManager; import org.bukkit.command.CommandSender; @@ -14,6 +18,7 @@ import static fr.xephi.authme.command.TestCommandsUtil.getCommandWithLabel; import static java.util.Arrays.asList; import static java.util.Collections.singletonList; import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.not; @@ -275,4 +280,14 @@ public class CommandMapperTest { assertThat(result.getArguments(), contains(parts.get(2))); } + @Test + public void shouldReturnExecutableCommandClasses() { + // given / when + Set> commandClasses = mapper.getCommandClasses(); + + // then + assertThat(commandClasses, containsInAnyOrder(ExecutableCommand.class, HelpCommand.class, + TestLoginCommand.class, TestRegisterCommand.class, TestUnregisterCommand.class)); + } + } diff --git a/src/test/java/fr/xephi/authme/command/CommandServiceTest.java b/src/test/java/fr/xephi/authme/command/CommandServiceTest.java index 334fc416f..3a3b5312d 100644 --- a/src/test/java/fr/xephi/authme/command/CommandServiceTest.java +++ b/src/test/java/fr/xephi/authme/command/CommandServiceTest.java @@ -1,6 +1,5 @@ package fr.xephi.authme.command; -import fr.xephi.authme.command.help.HelpProvider; import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.output.Messages; import fr.xephi.authme.settings.NewSetting; @@ -11,19 +10,14 @@ import org.bukkit.command.CommandSender; import org.bukkit.entity.Player; import org.junit.Test; import org.junit.runner.RunWith; -import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; -import java.util.Arrays; -import java.util.List; - import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertThat; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; /** @@ -35,10 +29,6 @@ public class CommandServiceTest { @InjectMocks private CommandService commandService; @Mock - private CommandMapper commandMapper; - @Mock - private HelpProvider helpProvider; - @Mock private Messages messages; @Mock private NewSetting settings; @@ -69,40 +59,6 @@ public class CommandServiceTest { verify(messages).send(sender, MessageKey.ANTIBOT_AUTO_ENABLED_MESSAGE, "10"); } - @Test - public void shouldMapPartsToCommand() { - // given - CommandSender sender = mock(Player.class); - List commandParts = Arrays.asList("authme", "test", "test2"); - FoundCommandResult givenResult = mock(FoundCommandResult.class); - given(commandMapper.mapPartsToCommand(sender, commandParts)).willReturn(givenResult); - - // when - FoundCommandResult result = commandService.mapPartsToCommand(sender, commandParts); - - // then - assertThat(result, equalTo(givenResult)); - verify(commandMapper).mapPartsToCommand(sender, commandParts); - } - - @Test - public void shouldOutputHelp() { - // given - CommandSender sender = mock(CommandSender.class); - FoundCommandResult result = mock(FoundCommandResult.class); - int options = HelpProvider.SHOW_LONG_DESCRIPTION; - List messages = Arrays.asList("Test message 1", "Other test message", "Third message for test"); - given(helpProvider.printHelp(sender, result, options)).willReturn(messages); - - // when - commandService.outputHelp(sender, result, options); - - // then - ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); - verify(sender, times(3)).sendMessage(captor.capture()); - assertThat(captor.getAllValues(), equalTo(messages)); - } - @Test public void shouldRetrieveMessage() { // given diff --git a/src/test/java/fr/xephi/authme/command/CommandUtilsTest.java b/src/test/java/fr/xephi/authme/command/CommandUtilsTest.java index a290967c0..bdff3c251 100644 --- a/src/test/java/fr/xephi/authme/command/CommandUtilsTest.java +++ b/src/test/java/fr/xephi/authme/command/CommandUtilsTest.java @@ -9,7 +9,6 @@ import java.util.List; import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertThat; -import static org.mockito.Mockito.mock; /** * Test for {@link CommandUtils}. @@ -59,14 +58,14 @@ public class CommandUtilsTest { .labels("authme", "auth") .description("Base") .detailedDescription("Test base command.") - .executableCommand(mock(ExecutableCommand.class)) + .executableCommand(ExecutableCommand.class) .build(); CommandDescription command = CommandDescription.builder() .parent(base) .labels("help", "h", "?") .description("Child") .detailedDescription("Test child command.") - .executableCommand(mock(ExecutableCommand.class)) + .executableCommand(ExecutableCommand.class) .build(); // when @@ -131,6 +130,6 @@ public class CommandUtilsTest { .labels("authme", "auth") .description("Base") .detailedDescription("Test base command.") - .executableCommand(mock(ExecutableCommand.class)); + .executableCommand(ExecutableCommand.class); } } diff --git a/src/test/java/fr/xephi/authme/command/TestCommandsUtil.java b/src/test/java/fr/xephi/authme/command/TestCommandsUtil.java index 65fb74e25..285369973 100644 --- a/src/test/java/fr/xephi/authme/command/TestCommandsUtil.java +++ b/src/test/java/fr/xephi/authme/command/TestCommandsUtil.java @@ -4,6 +4,7 @@ import fr.xephi.authme.command.executable.HelpCommand; import fr.xephi.authme.permission.AdminPermission; import fr.xephi.authme.permission.PermissionNode; import fr.xephi.authme.permission.PlayerPermission; +import org.bukkit.command.CommandSender; import java.util.Collection; import java.util.List; @@ -12,7 +13,6 @@ import java.util.Set; import static com.google.common.collect.Sets.newHashSet; import static java.util.Arrays.asList; import static java.util.Collections.singletonList; -import static org.mockito.Mockito.mock; /** * Util class for generating and retrieving test commands. @@ -29,22 +29,24 @@ public final class TestCommandsUtil { */ public static Set generateCommands() { // Register /authme - CommandDescription authMeBase = createCommand(null, null, singletonList("authme")); + CommandDescription authMeBase = createCommand(null, null, singletonList("authme"), ExecutableCommand.class); // Register /authme login - createCommand(PlayerPermission.LOGIN, authMeBase, singletonList("login"), newArgument("password", false)); + createCommand(PlayerPermission.LOGIN, authMeBase, singletonList("login"), + TestLoginCommand.class, newArgument("password", false)); // Register /authme register , aliases: /authme reg, /authme r - createCommand(PlayerPermission.LOGIN, authMeBase, asList("register", "reg", "r"), + createCommand(PlayerPermission.LOGIN, authMeBase, asList("register", "reg", "r"), TestRegisterCommand.class, newArgument("password", false), newArgument("confirmation", false)); // Register /email [player] - CommandDescription emailBase = createCommand(null, null, singletonList("email"), newArgument("player", true)); + CommandDescription emailBase = createCommand(null, null, singletonList("email"), ExecutableCommand.class, + newArgument("player", true)); // Register /email helptest -- use only to test for help command arguments special case - CommandDescription.builder().parent(emailBase).labels("helptest").executableCommand(mock(HelpCommand.class)) + CommandDescription.builder().parent(emailBase).labels("helptest").executableCommand(HelpCommand.class) .description("test").detailedDescription("Test.").withArgument("Query", "", false).build(); // Register /unregister , alias: /unreg CommandDescription unregisterBase = createCommand(AdminPermission.UNREGISTER, null, - asList("unregister", "unreg"), newArgument("player", false)); + asList("unregister", "unreg"), TestUnregisterCommand.class, newArgument("player", false)); return newHashSet(authMeBase, emailBase, unregisterBase); } @@ -83,14 +85,15 @@ public final class TestCommandsUtil { /** Shortcut command to initialize a new test command. */ private static CommandDescription createCommand(PermissionNode permission, CommandDescription parent, - List labels, CommandArgumentDescription... arguments) { + List labels, Class commandClass, + CommandArgumentDescription... arguments) { CommandDescription.CommandBuilder command = CommandDescription.builder() .labels(labels) .parent(parent) .permission(permission) .description(labels.get(0) + " cmd") .detailedDescription("'" + labels.get(0) + "' test command") - .executableCommand(mock(ExecutableCommand.class)); + .executableCommand(commandClass); if (arguments != null && arguments.length > 0) { for (CommandArgumentDescription argument : arguments) { @@ -106,4 +109,25 @@ public final class TestCommandsUtil { return new CommandArgumentDescription(label, "'" + label + "' argument description", isOptional); } + public static class TestLoginCommand implements ExecutableCommand { + @Override + public void executeCommand(CommandSender sender, List arguments) { + // noop + } + } + + public static class TestRegisterCommand implements ExecutableCommand { + @Override + public void executeCommand(CommandSender sender, List arguments) { + // noop + } + } + + public static class TestUnregisterCommand implements ExecutableCommand { + @Override + public void executeCommand(CommandSender sender, List arguments) { + // noop + } + } + } diff --git a/src/test/java/fr/xephi/authme/command/executable/authme/SwitchAntiBotCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/authme/SwitchAntiBotCommandTest.java index cd7045579..9b380411c 100644 --- a/src/test/java/fr/xephi/authme/command/executable/authme/SwitchAntiBotCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/authme/SwitchAntiBotCommandTest.java @@ -1,7 +1,7 @@ package fr.xephi.authme.command.executable.authme; import fr.xephi.authme.AntiBot; -import fr.xephi.authme.command.CommandService; +import fr.xephi.authme.command.CommandMapper; import fr.xephi.authme.command.FoundCommandResult; import fr.xephi.authme.command.help.HelpProvider; import org.bukkit.command.CommandSender; @@ -35,7 +35,10 @@ public class SwitchAntiBotCommandTest { private AntiBot antiBot; @Mock - private CommandService service; + private CommandMapper commandMapper; + + @Mock + private HelpProvider helpProvider; @Test public void shouldReturnAntiBotState() { @@ -81,7 +84,7 @@ public class SwitchAntiBotCommandTest { // given CommandSender sender = mock(CommandSender.class); FoundCommandResult foundCommandResult = mock(FoundCommandResult.class); - given(service.mapPartsToCommand(sender, asList("authme", "antibot"))).willReturn(foundCommandResult); + given(commandMapper.mapPartsToCommand(sender, asList("authme", "antibot"))).willReturn(foundCommandResult); // when command.executeCommand(sender, Collections.singletonList("wrong")); @@ -89,6 +92,6 @@ public class SwitchAntiBotCommandTest { // then verify(antiBot, never()).overrideAntiBotStatus(anyBoolean()); verify(sender).sendMessage(argThat(containsString("Invalid"))); - verify(service).outputHelp(sender, foundCommandResult, HelpProvider.SHOW_ARGUMENTS); + verify(helpProvider).outputHelp(sender, foundCommandResult, HelpProvider.SHOW_ARGUMENTS); } } diff --git a/src/test/java/fr/xephi/authme/command/help/HelpProviderTest.java b/src/test/java/fr/xephi/authme/command/help/HelpProviderTest.java index f40350e7a..b5450fb0c 100644 --- a/src/test/java/fr/xephi/authme/command/help/HelpProviderTest.java +++ b/src/test/java/fr/xephi/authme/command/help/HelpProviderTest.java @@ -13,6 +13,7 @@ import org.bukkit.command.CommandSender; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; +import org.mockito.ArgumentCaptor; import java.util.Arrays; import java.util.Collections; @@ -33,7 +34,9 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; import static org.junit.Assert.assertThat; import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; /** * Test for {@link HelpProvider}. @@ -67,9 +70,10 @@ public class HelpProviderTest { FoundCommandResult result = newFoundResult(command, Arrays.asList("authme", "login")); // when - List lines = helpProvider.printHelp(sender, result, SHOW_LONG_DESCRIPTION); + helpProvider.outputHelp(sender, result, SHOW_LONG_DESCRIPTION); // then + List lines = getLines(sender); assertThat(lines, hasSize(5)); assertThat(lines.get(0), containsString(HELP_HEADER + " HELP")); assertThat(removeColors(lines.get(1)), containsString("Command: /authme login ")); @@ -85,9 +89,10 @@ public class HelpProviderTest { FoundCommandResult result = newFoundResult(command, Arrays.asList("authme", "reg")); // when - List lines = helpProvider.printHelp(sender, result, HIDE_COMMAND | SHOW_ARGUMENTS); + helpProvider.outputHelp(sender, result, HIDE_COMMAND | SHOW_ARGUMENTS); // then + List lines = getLines(sender); assertThat(lines, hasSize(4)); assertThat(lines.get(0), containsString(HELP_HEADER + " HELP")); assertThat(removeColors(lines.get(1)), equalTo("Arguments:")); @@ -102,9 +107,10 @@ public class HelpProviderTest { FoundCommandResult result = newFoundResult(command, Collections.singletonList("email")); // when - List lines = helpProvider.printHelp(sender, result, HIDE_COMMAND | SHOW_ARGUMENTS); + helpProvider.outputHelp(sender, result, HIDE_COMMAND | SHOW_ARGUMENTS); // then + List lines = getLines(sender); assertThat(lines, hasSize(3)); assertThat(removeColors(lines.get(2)), containsString("player: 'player' argument description (Optional)")); } @@ -117,9 +123,10 @@ public class HelpProviderTest { FoundCommandResult result = newFoundResult(command, Collections.singletonList("authme")); // when - List lines = helpProvider.printHelp(sender, result, HIDE_COMMAND | SHOW_ARGUMENTS); + helpProvider.outputHelp(sender, result, HIDE_COMMAND | SHOW_ARGUMENTS); // then + List lines = getLines(sender); assertThat(lines, hasSize(1)); // only has the help banner } @@ -133,9 +140,10 @@ public class HelpProviderTest { given(permissionsManager.hasPermission(sender, command.getPermission())).willReturn(true); // when - List lines = helpProvider.printHelp(sender, result, HIDE_COMMAND | SHOW_PERMISSIONS); + helpProvider.outputHelp(sender, result, HIDE_COMMAND | SHOW_PERMISSIONS); // then + List lines = getLines(sender); assertThat(lines, hasSize(5)); assertThat(removeColors(lines.get(1)), containsString("Permissions:")); assertThat(removeColors(lines.get(2)), @@ -154,9 +162,10 @@ public class HelpProviderTest { given(permissionsManager.hasPermission(sender, command.getPermission())).willReturn(false); // when - List lines = helpProvider.printHelp(sender, result, HIDE_COMMAND | SHOW_PERMISSIONS); + helpProvider.outputHelp(sender, result, HIDE_COMMAND | SHOW_PERMISSIONS); // then + List lines = getLines(sender); assertThat(lines, hasSize(5)); assertThat(removeColors(lines.get(1)), containsString("Permissions:")); assertThat(removeColors(lines.get(2)), @@ -172,9 +181,10 @@ public class HelpProviderTest { FoundCommandResult result = newFoundResult(command, Collections.singletonList("authme")); // when - List lines = helpProvider.printHelp(sender, result, HIDE_COMMAND | SHOW_PERMISSIONS); + helpProvider.outputHelp(sender, result, HIDE_COMMAND | SHOW_PERMISSIONS); // then + List lines = getLines(sender); assertThat(lines, hasSize(1)); } @@ -187,9 +197,10 @@ public class HelpProviderTest { FoundCommandResult result = newFoundResult(command, Collections.singletonList("test")); // when - List lines = helpProvider.printHelp(sender, result, HIDE_COMMAND | SHOW_PERMISSIONS); + helpProvider.outputHelp(sender, result, HIDE_COMMAND | SHOW_PERMISSIONS); // then + List lines = getLines(sender); assertThat(lines, hasSize(1)); } @@ -200,9 +211,10 @@ public class HelpProviderTest { FoundCommandResult result = newFoundResult(command, Arrays.asList("authme", "reg")); // when - List lines = helpProvider.printHelp(sender, result, HIDE_COMMAND | SHOW_ALTERNATIVES); + helpProvider.outputHelp(sender, result, HIDE_COMMAND | SHOW_ALTERNATIVES); // then + List lines = getLines(sender); assertThat(lines, hasSize(4)); assertThat(removeColors(lines.get(1)), containsString("Alternatives:")); assertThat(removeColors(lines.get(2)), containsString("/authme register ")); @@ -216,9 +228,10 @@ public class HelpProviderTest { FoundCommandResult result = newFoundResult(command, Arrays.asList("authme", "login")); // when - List lines = helpProvider.printHelp(sender, result, HIDE_COMMAND | SHOW_ALTERNATIVES); + helpProvider.outputHelp(sender, result, HIDE_COMMAND | SHOW_ALTERNATIVES); // then + List lines = getLines(sender); assertThat(lines, hasSize(1)); } @@ -229,9 +242,10 @@ public class HelpProviderTest { FoundCommandResult result = newFoundResult(command, Collections.singletonList("authme")); // when - List lines = helpProvider.printHelp(sender, result, HIDE_COMMAND | SHOW_CHILDREN); + helpProvider.outputHelp(sender, result, HIDE_COMMAND | SHOW_CHILDREN); // then + List lines = getLines(sender); assertThat(lines, hasSize(4)); assertThat(removeColors(lines.get(1)), containsString("Commands:")); assertThat(removeColors(lines.get(2)), containsString("/authme login: login cmd")); @@ -245,9 +259,10 @@ public class HelpProviderTest { FoundCommandResult result = newFoundResult(command, Collections.singletonList("authme")); // when - List lines = helpProvider.printHelp(sender, result, HIDE_COMMAND | SHOW_CHILDREN); + helpProvider.outputHelp(sender, result, HIDE_COMMAND | SHOW_CHILDREN); // then + List lines = getLines(sender); assertThat(lines, hasSize(1)); } @@ -258,9 +273,10 @@ public class HelpProviderTest { Collections.emptyList(), 0.0, FoundResultStatus.UNKNOWN_LABEL); // when - List lines = helpProvider.printHelp(sender, result, ALL_OPTIONS); + helpProvider.outputHelp(sender, result, ALL_OPTIONS); // then + List lines = getLines(sender); assertThat(lines, hasSize(1)); assertThat(lines.get(0), containsString("Failed to retrieve any help information")); } @@ -276,9 +292,10 @@ public class HelpProviderTest { FoundCommandResult result = newFoundResult(command, Arrays.asList("authme", "ragister")); // when - List lines = helpProvider.printHelp(sender, result, 0); + helpProvider.outputHelp(sender, result, 0); // then + List lines = getLines(sender); assertThat(lines, hasSize(2)); assertThat(lines.get(0), containsString(HELP_HEADER + " HELP")); assertThat(removeColors(lines.get(1)), containsString("Command: /authme register ")); @@ -328,5 +345,11 @@ public class HelpProviderTest { } return str; } + + private static List getLines(CommandSender sender) { + ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); + verify(sender, atLeastOnce()).sendMessage(captor.capture()); + return captor.getAllValues(); + } } diff --git a/src/test/java/tools/commands/CommandPageCreater.java b/src/test/java/tools/commands/CommandPageCreater.java index b84782052..d5375a2f8 100644 --- a/src/test/java/tools/commands/CommandPageCreater.java +++ b/src/test/java/tools/commands/CommandPageCreater.java @@ -4,11 +4,7 @@ import fr.xephi.authme.command.CommandArgumentDescription; import fr.xephi.authme.command.CommandDescription; import fr.xephi.authme.command.CommandInitializer; import fr.xephi.authme.command.CommandUtils; -import fr.xephi.authme.command.ExecutableCommand; -import fr.xephi.authme.initialization.AuthMeServiceInitializer; import fr.xephi.authme.permission.PermissionNode; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.stubbing.Answer; import tools.utils.AutoToolTask; import tools.utils.FileUtils; import tools.utils.TagValue.NestedTagValue; @@ -19,10 +15,6 @@ import java.util.Collection; import java.util.Scanner; import java.util.Set; -import static org.mockito.Matchers.isA; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - public class CommandPageCreater implements AutoToolTask { private static final String OUTPUT_FILE = ToolsConstants.DOCS_FOLDER + "commands.md"; @@ -39,7 +31,7 @@ public class CommandPageCreater implements AutoToolTask { @Override public void executeDefault() { - CommandInitializer commandInitializer = new CommandInitializer(getMockInitializer()); + CommandInitializer commandInitializer = new CommandInitializer(); final Set baseCommands = commandInitializer.getCommands(); NestedTagValue commandTags = new NestedTagValue(); addCommandsInfo(commandTags, baseCommands); @@ -84,25 +76,4 @@ public class CommandPageCreater implements AutoToolTask { } return result.toString(); } - - /** - * Creates an initializer mock that returns mocks of any {@link ExecutableCommand} subclasses passed to it. - * - * @return the initializer mock - */ - @SuppressWarnings("unchecked") - private static AuthMeServiceInitializer getMockInitializer() { - AuthMeServiceInitializer initializer = mock(AuthMeServiceInitializer.class); - when(initializer.newInstance(isA(Class.class))).thenAnswer(new Answer() { - @Override - public Object answer(InvocationOnMock invocation) throws Throwable { - Class clazz = (Class) invocation.getArguments()[0]; - if (ExecutableCommand.class.isAssignableFrom(clazz)) { - return mock(clazz); - } - throw new IllegalStateException("Unexpected request to instantiate class of type " + clazz.getName()); - } - }); - return initializer; - } }