#758 Show login usage message on invalid arguments, #1053 send translatable "no permission" message

- Add default method to ExecutableCommand interface that allows to define the message key to show if a command's arguments are invalid. If not defined the behavior is as before: show the output of /<command> help
- Use translatable "no permission" message instead of hardcoded one

src/main/java/fr/xephi/authme/command/CommandHandler.java

@@ -3,6 +3,8 @@ package fr.xephi.authme.command;
 import ch.jalu.injector.Injector;
 import fr.xephi.authme.AuthMe;
 import fr.xephi.authme.command.help.HelpProvider;
+import fr.xephi.authme.message.MessageKey;
+import fr.xephi.authme.message.Messages;
 import fr.xephi.authme.permission.PermissionsManager;
 import fr.xephi.authme.util.StringUtils;
 import org.bukkit.ChatColor;
@@ -29,6 +31,7 @@ public class CommandHandler {
 
     private final CommandMapper commandMapper;
     private final PermissionsManager permissionsManager;
+    private final Messages messages;
     private final HelpProvider helpProvider;
 
     /**
@@ -37,10 +40,11 @@ public class CommandHandler {
     private Map<Class<? extends ExecutableCommand>, ExecutableCommand> commands = new HashMap<>();
 
     @Inject
-    public CommandHandler(Injector injector, CommandMapper commandMapper,
+    CommandHandler(Injector injector, CommandMapper commandMapper, PermissionsManager permissionsManager,
-                          PermissionsManager permissionsManager, HelpProvider helpProvider) {
+                          Messages messages, HelpProvider helpProvider) {
         this.commandMapper = commandMapper;
         this.permissionsManager = permissionsManager;
+        this.messages = messages;
         this.helpProvider = helpProvider;
         initializeCommands(injector, commandMapper.getCommandClasses());
     }
@@ -80,7 +84,7 @@ public class CommandHandler {
                 sendUnknownCommandMessage(sender, result);
                 break;
             case NO_PERMISSION:
-                sendPermissionDeniedError(sender);
+                messages.send(sender, MessageKey.NO_PERMISSION);
                 break;
             default:
                 throw new IllegalStateException("Unknown result status '" + result.getResultStatus() + "'");
@@ -151,11 +155,20 @@ public class CommandHandler {
     private void sendImproperArgumentsMessage(CommandSender sender, FoundCommandResult result) {
         CommandDescription command = result.getCommandDescription();
         if (!permissionsManager.hasPermission(sender, command.getPermission())) {
-            sendPermissionDeniedError(sender);
+            messages.send(sender, MessageKey.NO_PERMISSION);
             return;
         }
 
-        // Show the command argument help
+        ExecutableCommand executableCommand = commands.get(command.getExecutableCommand());
+        MessageKey usageMessage = executableCommand.getArgumentsMismatchMessage();
+        if (usageMessage == null) {
+            showHelpForCommand(sender, result);
+        } else {
+            messages.send(sender, usageMessage);
+        }
+    }
+
+    private void showHelpForCommand(CommandSender sender, FoundCommandResult result) {
         sender.sendMessage(ChatColor.DARK_RED + "Incorrect command arguments!");
         helpProvider.outputHelp(sender, result, HelpProvider.SHOW_ARGUMENTS);
 
@@ -164,10 +177,4 @@ public class CommandHandler {
         sender.sendMessage(ChatColor.GOLD + "Detailed help: " + ChatColor.WHITE
             + "/" + labels.get(0) + " help " + childLabel);
     }
-
-    // TODO ljacqu 20151212: Remove me once I am a MessageKey
-    private static void sendPermissionDeniedError(CommandSender sender) {
-        sender.sendMessage(ChatColor.DARK_RED + "You don't have permission to use this command!");
-    }
-
 }

src/main/java/fr/xephi/authme/command/ExecutableCommand.java

@@ -1,5 +1,6 @@
 package fr.xephi.authme.command;
 
+import fr.xephi.authme.message.MessageKey;
 import org.bukkit.command.CommandSender;
 
 import java.util.List;
@@ -17,4 +18,14 @@ public interface ExecutableCommand {
      */
     void executeCommand(CommandSender sender, List<String> arguments);
 
+    /**
+     * Returns the message to show to the user if the command is used with the wrong commands.
+     * If null is returned, the standard help (/<i>command</i> help) output is shown.
+     *
+     * @return the message explaining the command's usage, or {@code null} for default behavior
+     */
+    default MessageKey getArgumentsMismatchMessage() {
+        return null;
+    }
+
 }

src/main/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommand.java

@@ -49,4 +49,9 @@ public class ChangePasswordCommand extends PlayerCommand {
 
         management.performPasswordChange(player, oldPassword, newPassword);
     }
+
+    @Override
+    protected String getAlternativeCommand() {
+        return "/authme password <playername> <password>";
+    }
 }

src/main/java/fr/xephi/authme/command/executable/login/LoginCommand.java

@@ -1,6 +1,7 @@
 package fr.xephi.authme.command.executable.login;
 
 import fr.xephi.authme.command.PlayerCommand;
+import fr.xephi.authme.message.MessageKey;
 import fr.xephi.authme.process.Management;
 import org.bukkit.entity.Player;
 
@@ -20,4 +21,14 @@ public class LoginCommand extends PlayerCommand {
         final String password = arguments.get(0);
         management.performLogin(player, password);
     }
+
+    @Override
+    public MessageKey getArgumentsMismatchMessage() {
+        return MessageKey.USAGE_LOGIN;
+    }
+
+    @Override
+    protected String getAlternativeCommand() {
+        return "/authme forcelogin <player>";
+    }
 }

src/main/java/fr/xephi/authme/command/executable/register/RegisterCommand.java

@@ -72,6 +72,11 @@ public class RegisterCommand extends PlayerCommand {
         return "/authme register <playername> <password>";
     }
 
+    @Override
+    public MessageKey getArgumentsMismatchMessage() {
+        return MessageKey.USAGE_LOGIN;
+    }
+
     private void handlePasswordRegistration(Player player, List<String> arguments) {
         if (isSecondArgValidForPasswordRegistration(player, arguments)) {
             final String password = arguments.get(0);

src/test/java/fr/xephi/authme/command/CommandHandlerTest.java

@@ -6,6 +6,8 @@ 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.message.MessageKey;
+import fr.xephi.authme.message.Messages;
 import fr.xephi.authme.permission.PermissionsManager;
 import org.bukkit.command.CommandSender;
 import org.junit.Before;
@@ -60,6 +62,8 @@ public class CommandHandlerTest {
     @Mock
     private PermissionsManager permissionsManager;
     @Mock
+    private Messages messages;
+    @Mock
     private HelpProvider helpProvider;
 
     private Map<Class<? extends ExecutableCommand>, ExecutableCommand> mockedCommands = new HashMap<>();
@@ -71,7 +75,7 @@ public class CommandHandlerTest {
             ExecutableCommand.class, TestLoginCommand.class, TestRegisterCommand.class, TestUnregisterCommand.class));
         setInjectorToMockExecutableCommandClasses();
 
-        handler = new CommandHandler(injector, commandMapper, permissionsManager, helpProvider);
+        handler = new CommandHandler(injector, commandMapper, permissionsManager, messages, helpProvider);
     }
 
     /**
@@ -138,7 +142,7 @@ public class CommandHandlerTest {
         // then
         verify(commandMapper).mapPartsToCommand(sender, asList("unreg", "testPlayer"));
         verify(command, never()).getExecutableCommand();
-        verify(sender).sendMessage(argThat(containsString("don't have permission")));
+        verify(messages).send(sender, MessageKey.NO_PERMISSION);
     }
 
     @Test
@@ -148,6 +152,7 @@ public class CommandHandlerTest {
         String[] bukkitArgs = {"testPlayer"};
         CommandSender sender = mock(CommandSender.class);
         CommandDescription command = mock(CommandDescription.class);
+        given(command.getExecutableCommand()).willReturn((Class) TestUnregisterCommand.class);
         given(commandMapper.mapPartsToCommand(any(CommandSender.class), anyList())).willReturn(
             new FoundCommandResult(command, asList("unreg"), asList("testPlayer"), 0.0, INCORRECT_ARGUMENTS));
         given(permissionsManager.hasPermission(sender, command.getPermission())).willReturn(true);
@@ -157,10 +162,30 @@ public class CommandHandlerTest {
 
         // then
         verify(commandMapper).mapPartsToCommand(sender, asList("unreg", "testPlayer"));
-        verify(command, never()).getExecutableCommand();
+        verify(sender, atLeastOnce()).sendMessage(argThat(containsString("Incorrect command arguments")));
-        ArgumentCaptor<String> captor = ArgumentCaptor.forClass(String.class);
+    }
-        verify(sender, atLeastOnce()).sendMessage(captor.capture());
+
-        assertThat(captor.getAllValues().get(0), containsString("Incorrect command arguments"));
+    @Test
+    public void shouldUseCustomMessageUponArgumentMismatch() {
+        // given
+        String bukkitLabel = "unreg";
+        String[] bukkitArgs = {"testPlayer"};
+        CommandSender sender = mock(CommandSender.class);
+        CommandDescription command = mock(CommandDescription.class);
+        given(command.getExecutableCommand()).willReturn((Class) TestUnregisterCommand.class);
+        given(mockedCommands.get(TestUnregisterCommand.class).getArgumentsMismatchMessage())
+            .willReturn(MessageKey.USAGE_RECOVER_EMAIL);
+        given(commandMapper.mapPartsToCommand(any(CommandSender.class), anyList())).willReturn(
+            new FoundCommandResult(command, asList("unreg"), asList("testPlayer"), 0.0, INCORRECT_ARGUMENTS));
+        given(permissionsManager.hasPermission(sender, command.getPermission())).willReturn(true);
+
+        // when
+        handler.processCommand(sender, bukkitLabel, bukkitArgs);
+
+        // then
+        verify(commandMapper).mapPartsToCommand(sender, asList("unreg", "testPlayer"));
+        verify(messages).send(sender, MessageKey.USAGE_RECOVER_EMAIL);
+        verify(sender, never()).sendMessage(anyString());
     }
 
     @Test
@@ -180,9 +205,7 @@ public class CommandHandlerTest {
         // then
         verify(commandMapper).mapPartsToCommand(sender, asList("unreg", "testPlayer"));
         verify(command, never()).getExecutableCommand();
-        ArgumentCaptor<String> captor = ArgumentCaptor.forClass(String.class);
+        verify(messages).send(sender, MessageKey.NO_PERMISSION);
-        verify(sender).sendMessage(captor.capture());
-        assertThat(captor.getValue(), containsString("You don't have permission"));
     }
 
     @Test

src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java

@@ -58,7 +58,7 @@ public class ChangePasswordCommandTest {
         command.executeCommand(sender, Collections.emptyList());
 
         // then
-        verify(sender).sendMessage(argThat(containsString("only for players")));
+        verify(sender).sendMessage(argThat(containsString("use /authme password <playername> <password> instead")));
     }
 
     @Test

src/test/java/fr/xephi/authme/command/executable/login/LoginCommandTest.java

@@ -10,7 +10,6 @@ import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
 
-import java.util.ArrayList;
 import java.util.Collections;
 
 import static org.hamcrest.Matchers.containsString;
@@ -39,11 +38,11 @@ public class LoginCommandTest {
         CommandSender sender = mock(BlockCommandSender.class);
 
         // when
-        command.executeCommand(sender, new ArrayList<String>());
+        command.executeCommand(sender, Collections.emptyList());
 
         // then
         verifyZeroInteractions(management);
-        verify(sender).sendMessage(argThat(containsString("only for players")));
+        verify(sender).sendMessage(argThat(containsString("/authme forcelogin <player>")));
     }
 
     @Test