#411 Create tests for command migration and manager, minor tweaks

src/main/java/fr/xephi/authme/settings/commandconfig/Command.java

@@ -42,4 +42,9 @@ public class Command {
     public void setExecutor(Executor executor) {
         this.executor = executor;
     }
+
+    @Override
+    public String toString() {
+        return command + " (" + executor + ")";
+    }
 }

src/main/java/fr/xephi/authme/settings/commandconfig/CommandConfig.java

@@ -1,6 +1,6 @@
 package fr.xephi.authme.settings.commandconfig;
 
-import java.util.HashMap;
+import java.util.LinkedHashMap;
 import java.util.Map;
 
 /**
@@ -10,9 +10,9 @@ import java.util.Map;
  */
 public class CommandConfig {
 
-    private Map<String, Command> onJoin = new HashMap<>();
+    private Map<String, Command> onJoin = new LinkedHashMap<>();
-    private Map<String, Command> onLogin = new HashMap<>();
+    private Map<String, Command> onLogin = new LinkedHashMap<>();
-    private Map<String, Command> onRegister = new HashMap<>();
+    private Map<String, Command> onRegister = new LinkedHashMap<>();
 
     public Map<String, Command> getOnJoin() {
         return onJoin;

src/main/java/fr/xephi/authme/settings/commandconfig/CommandManager.java

@@ -10,7 +10,6 @@ import fr.xephi.authme.settings.Settings;
 import fr.xephi.authme.util.FileUtils;
 import org.bukkit.entity.Player;
 
-import javax.annotation.PostConstruct;
 import javax.inject.Inject;
 import java.io.File;
 import java.util.Map;
@@ -20,23 +19,21 @@ import java.util.Map;
  */
 public class CommandManager implements Reloadable {
 
+    private final File dataFolder;
+    private final BukkitService bukkitService;
+    private final CommandsMigrater commandsMigrater;
+    private final Settings settings;
+
     private CommandConfig commandConfig;
 
     @Inject
-    @DataFolder
+    CommandManager(@DataFolder File dataFolder, BukkitService bukkitService,
-    private File dataFolder;
+                   CommandsMigrater commandsMigrater, Settings settings) {
-
+        this.dataFolder = dataFolder;
-    @Inject
+        this.bukkitService = bukkitService;
-    private BukkitService bukkitService;
+        this.commandsMigrater = commandsMigrater;
-
+        this.settings = settings;
-    @Inject
+        reload();
-    private CommandsMigrater commandsMigrater;
-
-    @Inject
-    private Settings settings;
-
-
-    CommandManager() {
     }
 
     public void runCommandsOnJoin(Player player) {
@@ -62,7 +59,6 @@ public class CommandManager implements Reloadable {
         }
     }
 
-    @PostConstruct
     @Override
     public void reload() {
         File file = new File(dataFolder, "commands.yml");

src/main/java/fr/xephi/authme/settings/commandconfig/CommandsMigrater.java

@@ -21,6 +21,12 @@ class CommandsMigrater {
     CommandsMigrater() {
     }
 
+    /**
+     * Adds command settings from their old location (in config.yml) to the given command configuration object.
+     *
+     * @param commandConfig the command config object to move old commands to
+     * @return true if commands have been moved, false if no migration was necessary
+     */
     boolean transformOldCommands(CommandConfig commandConfig) {
         boolean didMoveCommands = false;
         for (MigratableCommandSection section : MigratableCommandSection.values()) {
@@ -37,12 +43,12 @@ class CommandsMigrater {
         ON_JOIN(
             SettingsMigrationService::getOnLoginCommands,
             Executor.PLAYER,
-            CommandConfig::getOnJoin),
+            CommandConfig::getOnLogin),
 
         ON_JOIN_CONSOLE(
             SettingsMigrationService::getOnLoginConsoleCommands,
             Executor.CONSOLE,
-            CommandConfig::getOnJoin),
+            CommandConfig::getOnLogin),
 
         ON_REGISTER(
             SettingsMigrationService::getOnRegisterCommands,
@@ -91,7 +97,7 @@ class CommandsMigrater {
             }
             Map<String, Command> commandMap = commandMapGetter.apply(commandConfig);
             commands.forEach(cmd -> commandMap.put(RandomStringUtils.generate(10), cmd));
-            ConsoleLogger.info("Migrated " + commands.size() + " of type " + this);
+            ConsoleLogger.info("Migrated " + commands.size() + " commands of type " + this);
             return true;
         }
     }

src/test/java/fr/xephi/authme/settings/commandconfig/CommandConfigTestHelper.java

@@ -0,0 +1,35 @@
+package fr.xephi.authme.settings.commandconfig;
+
+import org.hamcrest.Description;
+import org.hamcrest.Matcher;
+import org.hamcrest.TypeSafeMatcher;
+
+/**
+ * Helper class for tests around the command configuration.
+ */
+final class CommandConfigTestHelper {
+
+    private CommandConfigTestHelper() {
+    }
+
+    /**
+     * Returns a matcher for verifying a {@link Command} object.
+     *
+     * @param cmd the expected command line
+     * @param executor the expected executor
+     * @return the matcher
+     */
+    public static Matcher<Command> isCommand(String cmd, Executor executor) {
+        return new TypeSafeMatcher<Command>() {
+            @Override
+            protected boolean matchesSafely(Command item) {
+                return executor.equals(item.getExecutor()) && cmd.equals(item.getCommand());
+            }
+
+            @Override
+            public void describeTo(Description description) {
+                description.appendText("Command '" + cmd + "' run by '" + executor + "'");
+            }
+        };
+    }
+}

src/test/java/fr/xephi/authme/settings/commandconfig/CommandManagerTest.java

@@ -0,0 +1,115 @@
+package fr.xephi.authme.settings.commandconfig;
+
+import com.google.common.io.Files;
+import fr.xephi.authme.ReflectionTestUtils;
+import fr.xephi.authme.TestHelper;
+import fr.xephi.authme.service.BukkitService;
+import fr.xephi.authme.settings.Settings;
+import org.bukkit.configuration.file.FileConfiguration;
+import org.bukkit.configuration.file.YamlConfiguration;
+import org.junit.Before;
+import org.junit.Ignore;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import java.io.File;
+import java.io.IOException;
+
+import static fr.xephi.authme.settings.commandconfig.CommandConfigTestHelper.isCommand;
+import static org.hamcrest.Matchers.anEmptyMap;
+import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.equalTo;
+import static org.junit.Assert.assertThat;
+
+/**
+ * Test for {@link CommandManager}.
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class CommandManagerTest {
+
+    private static final String TEST_FILES_FOLDER = "/fr/xephi/authme/settings/commandconfig/";
+
+    @Rule
+    public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+    private CommandManager manager;
+    @Mock
+    private BukkitService bukkitService;
+    @Mock
+    private Settings settings;
+    @Mock
+    private CommandsMigrater commandsMigrater;
+    private File testFolder;
+
+    @Before
+    public void setup() throws IOException {
+        testFolder = temporaryFolder.newFolder();
+    }
+
+    @Test
+    @SuppressWarnings("unchecked")
+    public void shouldLoadCompleteFile() {
+        // given
+        copyJarFileAsCommandsYml(TEST_FILES_FOLDER + "commands.complete.yml");
+
+        // when
+        initManager();
+
+        // then
+        CommandConfig commandConfig = ReflectionTestUtils.getFieldValue(CommandManager.class, manager, "commandConfig");
+        assertThat(commandConfig.getOnJoin().keySet(), contains("broadcast"));
+        assertThat(commandConfig.getOnJoin().values(), contains(isCommand("broadcast %p has joined", Executor.CONSOLE)));
+        assertThat(commandConfig.getOnRegister().keySet(), contains("announce", "notify"));
+        assertThat(commandConfig.getOnRegister().values(), contains(
+            isCommand("me I just registered", Executor.PLAYER),
+            isCommand("log %p registered", Executor.CONSOLE)));
+        assertThat(commandConfig.getOnLogin().keySet(), contains("welcome", "show_motd", "display_list"));
+        assertThat(commandConfig.getOnLogin().values(), contains(
+            isCommand("msg %p Welcome back", Executor.CONSOLE),
+            isCommand("motd", Executor.PLAYER),
+            isCommand("list", Executor.PLAYER)));
+    }
+
+    @Test
+    @Ignore // TODO #411: Implement tested behavior
+    public void shouldLoadIncompleteFile() {
+        // given
+        File configFile = copyJarFileAsCommandsYml(TEST_FILES_FOLDER + "commands.incomplete.yml");
+
+        // when
+        initManager();
+
+        // then
+        CommandConfig commandConfig = ReflectionTestUtils.getFieldValue(CommandManager.class, manager, "commandConfig");
+        assertThat(commandConfig.getOnJoin().values(), contains(isCommand("broadcast %p has joined", Executor.CONSOLE)));
+        assertThat(commandConfig.getOnLogin().values(), contains(
+            isCommand("msg %p Welcome back", Executor.CONSOLE),
+            isCommand("list", Executor.PLAYER)));
+        assertThat(commandConfig.getOnRegister(), anEmptyMap());
+
+        // verify that we have rewritten the file with the proper sections
+        FileConfiguration configuration = YamlConfiguration.loadConfiguration(configFile);
+        assertThat(configuration.contains("onRegister"), equalTo(true));
+        assertThat(configuration.contains("doesNotExist"), equalTo(false));
+    }
+
+    private void initManager() {
+        manager = new CommandManager(testFolder, bukkitService, commandsMigrater, settings);
+    }
+
+    private File copyJarFileAsCommandsYml(String path) {
+        File source = TestHelper.getJarFile(path);
+        File destination = new File(testFolder, "commands.yml");
+        try {
+            Files.copy(source, destination);
+            return destination;
+        } catch (IOException e) {
+            throw new IllegalStateException(e);
+        }
+    }
+
+}

src/test/java/fr/xephi/authme/settings/commandconfig/CommandsMigraterTest.java

@@ -0,0 +1,111 @@
+package fr.xephi.authme.settings.commandconfig;
+
+import fr.xephi.authme.TestHelper;
+import fr.xephi.authme.settings.SettingsMigrationService;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+import static fr.xephi.authme.settings.commandconfig.CommandConfigTestHelper.isCommand;
+import static java.util.Collections.emptyList;
+import static org.hamcrest.Matchers.anEmptyMap;
+import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.sameInstance;
+import static org.junit.Assert.assertThat;
+import static org.mockito.BDDMockito.given;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verifyZeroInteractions;
+
+/**
+ * Test for {@link CommandsMigrater}.
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class CommandsMigraterTest {
+
+    @InjectMocks
+    private CommandsMigrater commandsMigrater;
+
+    @Mock
+    private SettingsMigrationService settingsMigrationService;
+
+    private CommandConfig commandConfig = new CommandConfig();
+
+    @BeforeClass
+    public static void setUpLogger() {
+        TestHelper.setupLogger();
+    }
+
+    @Test
+    public void shouldNotPerformAnyMigration() {
+        // given
+        given(settingsMigrationService.getOnLoginCommands()).willReturn(emptyList());
+        given(settingsMigrationService.getOnLoginConsoleCommands()).willReturn(emptyList());
+        given(settingsMigrationService.getOnRegisterCommands()).willReturn(emptyList());
+        given(settingsMigrationService.getOnRegisterConsoleCommands()).willReturn(emptyList());
+        commandConfig.getOnRegister().put("existing", new Command("existing cmd", Executor.PLAYER));
+        CommandConfig configSpy = spy(commandConfig);
+
+        // when
+        boolean result = commandsMigrater.transformOldCommands(configSpy);
+
+        // then
+        assertThat(result, equalTo(false));
+        verifyZeroInteractions(configSpy);
+        assertThat(configSpy.getOnRegister().keySet(), contains("existing"));
+        assertThat(configSpy.getOnLogin(), anEmptyMap());
+    }
+
+    @Test
+    @SuppressWarnings("unchecked")
+    public void shouldPerformMigration() {
+        // given
+        List<String> onLogin = Collections.singletonList("on login command");
+        given(settingsMigrationService.getOnLoginCommands()).willReturn(onLogin);
+        List<String> onLoginConsole = Arrays.asList("cmd1", "cmd2 %p", "cmd3");
+        given(settingsMigrationService.getOnLoginConsoleCommands()).willReturn(onLoginConsole);
+        given(settingsMigrationService.getOnRegisterCommands()).willReturn(emptyList());
+        List<String> onRegisterConsole = Arrays.asList("log %p registered", "whois %p");
+        given(settingsMigrationService.getOnRegisterConsoleCommands()).willReturn(onRegisterConsole);
+
+        Map<String, Command> onLoginCommands = new LinkedHashMap<>();
+        onLoginCommands.put("bcast", new Command("bcast %p returned", Executor.CONSOLE));
+        commandConfig.setOnLogin(onLoginCommands);
+        Map<String, Command> onRegisterCommands = new LinkedHashMap<>();
+        onRegisterCommands.put("ex_cmd", new Command("existing", Executor.CONSOLE));
+        onRegisterCommands.put("ex_cmd2", new Command("existing2", Executor.PLAYER));
+        commandConfig.setOnRegister(onRegisterCommands);
+
+        // when
+        boolean result = commandsMigrater.transformOldCommands(commandConfig);
+
+        // then
+        assertThat(result, equalTo(true));
+        assertThat(commandConfig.getOnLogin(), sameInstance(onLoginCommands));
+        Collection<Command> loginCmdList = onLoginCommands.values();
+        assertThat(loginCmdList, contains(
+            equalTo(onLoginCommands.get("bcast")),
+            isCommand("on login command", Executor.PLAYER),
+            isCommand("cmd1", Executor.CONSOLE),
+            isCommand("cmd2 %p", Executor.CONSOLE),
+            isCommand("cmd3", Executor.CONSOLE)));
+
+        assertThat(commandConfig.getOnRegister(), sameInstance(onRegisterCommands));
+        Collection<Command> registerCmdList = onRegisterCommands.values();
+        assertThat(registerCmdList, contains(
+            isCommand("existing", Executor.CONSOLE),
+            isCommand("existing2", Executor.PLAYER),
+            isCommand("log %p registered", Executor.CONSOLE),
+            isCommand("whois %p", Executor.CONSOLE)));
+    }
+}

src/test/resources/fr/xephi/authme/settings/commandconfig/commands.complete.yml

@@ -0,0 +1,23 @@
+# Sample commands.yml file
+
+onJoin:
+    broadcast:
+        command: 'broadcast %p has joined'
+        executor: CONSOLE
+onRegister:
+    announce:
+        command: 'me I just registered'
+        executor: PLAYER
+    notify:
+        command: 'log %p registered'
+        executor: CONSOLE
+onLogin:
+    welcome:
+        command: 'msg %p Welcome back'
+        executor: CONSOLE
+    show_motd:
+        command: 'motd'
+        executor: PLAYER
+    display_list:
+        command: 'list'
+        executor: PLAYER

src/test/resources/fr/xephi/authme/settings/commandconfig/commands.incomplete.yml

@@ -0,0 +1,20 @@
+# Sample commands.yml file with wrong and missing sections
+
+onJoin:
+    broadcast:
+        command: 'broadcast %p has joined'
+        executor: CONSOLE
+onLogin:
+    welcome:
+        command: 'msg %p Welcome back'
+        executor: CONSOLE
+    show_motd:
+        # command: 'motd' <-- mandatory property, so entry should be ignored
+        executor: PLAYER
+    display_list:
+        command: 'list'
+        executor: WRONG_EXECUTOR
+doesNotExist:
+    wrongEntry:
+        command: 'should be ignored'
+        executor: PLAYER