diff --git a/src/main/java/fr/xephi/authme/settings/commandconfig/Command.java b/src/main/java/fr/xephi/authme/settings/commandconfig/Command.java index 5cb50998a..3828dc0ee 100644 --- a/src/main/java/fr/xephi/authme/settings/commandconfig/Command.java +++ b/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 + ")"; + } } diff --git a/src/main/java/fr/xephi/authme/settings/commandconfig/CommandConfig.java b/src/main/java/fr/xephi/authme/settings/commandconfig/CommandConfig.java index 53246cdb3..19c05505f 100644 --- a/src/main/java/fr/xephi/authme/settings/commandconfig/CommandConfig.java +++ b/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 onJoin = new HashMap<>(); - private Map onLogin = new HashMap<>(); - private Map onRegister = new HashMap<>(); + private Map onJoin = new LinkedHashMap<>(); + private Map onLogin = new LinkedHashMap<>(); + private Map onRegister = new LinkedHashMap<>(); public Map getOnJoin() { return onJoin; diff --git a/src/main/java/fr/xephi/authme/settings/commandconfig/CommandManager.java b/src/main/java/fr/xephi/authme/settings/commandconfig/CommandManager.java index b37e01eb7..015df7f68 100644 --- a/src/main/java/fr/xephi/authme/settings/commandconfig/CommandManager.java +++ b/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 - private File dataFolder; - - @Inject - private BukkitService bukkitService; - - @Inject - private CommandsMigrater commandsMigrater; - - @Inject - private Settings settings; - - - CommandManager() { + CommandManager(@DataFolder File dataFolder, BukkitService bukkitService, + CommandsMigrater commandsMigrater, Settings settings) { + this.dataFolder = dataFolder; + this.bukkitService = bukkitService; + this.commandsMigrater = commandsMigrater; + this.settings = settings; + reload(); } 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"); diff --git a/src/main/java/fr/xephi/authme/settings/commandconfig/CommandsMigrater.java b/src/main/java/fr/xephi/authme/settings/commandconfig/CommandsMigrater.java index 5defb4c00..8a2df1671 100644 --- a/src/main/java/fr/xephi/authme/settings/commandconfig/CommandsMigrater.java +++ b/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 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; } } diff --git a/src/test/java/fr/xephi/authme/settings/commandconfig/CommandConfigTestHelper.java b/src/test/java/fr/xephi/authme/settings/commandconfig/CommandConfigTestHelper.java new file mode 100644 index 000000000..f9fb4e5f4 --- /dev/null +++ b/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 isCommand(String cmd, Executor executor) { + return new TypeSafeMatcher() { + @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 + "'"); + } + }; + } +} diff --git a/src/test/java/fr/xephi/authme/settings/commandconfig/CommandManagerTest.java b/src/test/java/fr/xephi/authme/settings/commandconfig/CommandManagerTest.java new file mode 100644 index 000000000..eea0f569b --- /dev/null +++ b/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); + } + } + +} diff --git a/src/test/java/fr/xephi/authme/settings/commandconfig/CommandsMigraterTest.java b/src/test/java/fr/xephi/authme/settings/commandconfig/CommandsMigraterTest.java new file mode 100644 index 000000000..ee1468844 --- /dev/null +++ b/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 onLogin = Collections.singletonList("on login command"); + given(settingsMigrationService.getOnLoginCommands()).willReturn(onLogin); + List onLoginConsole = Arrays.asList("cmd1", "cmd2 %p", "cmd3"); + given(settingsMigrationService.getOnLoginConsoleCommands()).willReturn(onLoginConsole); + given(settingsMigrationService.getOnRegisterCommands()).willReturn(emptyList()); + List onRegisterConsole = Arrays.asList("log %p registered", "whois %p"); + given(settingsMigrationService.getOnRegisterConsoleCommands()).willReturn(onRegisterConsole); + + Map onLoginCommands = new LinkedHashMap<>(); + onLoginCommands.put("bcast", new Command("bcast %p returned", Executor.CONSOLE)); + commandConfig.setOnLogin(onLoginCommands); + Map 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 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 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))); + } +} diff --git a/src/test/resources/fr/xephi/authme/settings/commandconfig/commands.complete.yml b/src/test/resources/fr/xephi/authme/settings/commandconfig/commands.complete.yml new file mode 100644 index 000000000..8c6bf79de --- /dev/null +++ b/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 diff --git a/src/test/resources/fr/xephi/authme/settings/commandconfig/commands.incomplete.yml b/src/test/resources/fr/xephi/authme/settings/commandconfig/commands.incomplete.yml new file mode 100644 index 000000000..2eef86b03 --- /dev/null +++ b/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