diff --git a/src/main/java/fr/xephi/authme/settings/SettingsMigrationService.java b/src/main/java/fr/xephi/authme/settings/SettingsMigrationService.java index 641492be..c394fa36 100644 --- a/src/main/java/fr/xephi/authme/settings/SettingsMigrationService.java +++ b/src/main/java/fr/xephi/authme/settings/SettingsMigrationService.java @@ -2,7 +2,6 @@ package fr.xephi.authme.settings; import ch.jalu.configme.migration.PlainMigrationService; import ch.jalu.configme.properties.Property; -import ch.jalu.configme.properties.StringListProperty; import ch.jalu.configme.resource.PropertyResource; import com.google.common.base.MoreObjects; import fr.xephi.authme.ConsoleLogger; @@ -19,7 +18,6 @@ import javax.inject.Inject; import java.io.File; import java.io.FileWriter; import java.io.IOException; -import java.util.Collections; import java.util.List; import java.util.Set; @@ -39,15 +37,6 @@ public class SettingsMigrationService extends PlainMigrationService { private final File pluginFolder; - // Stores old commands that need to be migrated to the new commands configuration - // We need to store it in here for retrieval when we build the CommandConfig. Retrieving it from the config.yml is - // not possible since this migration service may trigger config.yml to be resaved. As the old command settings - // don't exist in the code anymore, as soon as config.yml is resaved we lose this information. - private List onLoginCommands = Collections.emptyList(); - private List onLoginConsoleCommands = Collections.emptyList(); - private List onRegisterCommands = Collections.emptyList(); - private List onRegisterConsoleCommands = Collections.emptyList(); - @Inject SettingsMigrationService(@DataFolder File pluginFolder) { this.pluginFolder = pluginFolder; @@ -62,8 +51,6 @@ public class SettingsMigrationService extends PlainMigrationService { changes = true; } - gatherOldCommandSettings(resource); - // Note ljacqu 20160211: Concatenating migration methods with | instead of the usual || // ensures that all migrations will be performed return changes @@ -97,35 +84,6 @@ public class SettingsMigrationService extends PlainMigrationService { return false; } - // ---------------- - // Forced commands relocation (from config.yml to commands.yml) - // ---------------- - private void gatherOldCommandSettings(PropertyResource resource) { - onLoginCommands = getStringList(resource, "settings.forceCommands"); - onLoginConsoleCommands = getStringList(resource, "settings.forceCommandsAsConsole"); - onRegisterCommands = getStringList(resource, "settings.forceRegisterCommands"); - onRegisterConsoleCommands = getStringList(resource, "settings.forceRegisterCommandsAsConsole"); - } - - private List getStringList(PropertyResource resource, String path) { - return new StringListProperty(path).getValue(resource); - } - - public List getOnLoginCommands() { - return onLoginCommands; - } - - public List getOnLoginConsoleCommands() { - return onLoginConsoleCommands; - } - - public List getOnRegisterCommands() { - return onRegisterCommands; - } - - public List getOnRegisterConsoleCommands() { - return onRegisterConsoleCommands; - } // -------- // Specific migrations diff --git a/src/main/java/fr/xephi/authme/settings/commandconfig/CommandMigrationService.java b/src/main/java/fr/xephi/authme/settings/commandconfig/CommandMigrationService.java index 0b631fca..9775f8e0 100644 --- a/src/main/java/fr/xephi/authme/settings/commandconfig/CommandMigrationService.java +++ b/src/main/java/fr/xephi/authme/settings/commandconfig/CommandMigrationService.java @@ -5,16 +5,8 @@ import ch.jalu.configme.properties.Property; import ch.jalu.configme.resource.PropertyResource; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; -import fr.xephi.authme.ConsoleLogger; -import fr.xephi.authme.settings.SettingsMigrationService; -import fr.xephi.authme.util.RandomStringUtils; -import javax.inject.Inject; import java.util.List; -import java.util.Map; -import java.util.function.BiFunction; -import java.util.function.Function; -import java.util.stream.Collectors; /** * Migrates the commands from their old location, in config.yml, to the dedicated commands configuration file. @@ -26,131 +18,20 @@ class CommandMigrationService implements MigrationService { static final List COMMAND_CONFIG_PROPERTIES = ImmutableList.of( "onJoin", "onLogin", "onSessionLogin", "onFirstLogin", "onRegister", "onUnregister", "onLogout"); - @Inject - private SettingsMigrationService settingsMigrationService; - CommandMigrationService() { } @Override public boolean checkAndMigrate(PropertyResource resource, List> properties) { final CommandConfig commandConfig = CommandSettingsHolder.COMMANDS.getValue(resource); - final boolean didMoveCommands = transformOldCommands(commandConfig); - - if (didMoveCommands || isFileEmpty(resource)) { + if (isFileEmpty(resource)) { resource.setValue("", commandConfig); return true; } return false; } - private boolean isFileEmpty(PropertyResource resource) { + private static boolean isFileEmpty(PropertyResource resource) { return COMMAND_CONFIG_PROPERTIES.stream().anyMatch(property -> resource.getObject(property) == null); } - - /** - * 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 - */ - @VisibleForTesting - boolean transformOldCommands(CommandConfig commandConfig) { - boolean didMoveCommands = false; - for (MigratableCommandSection section : MigratableCommandSection.values()) { - didMoveCommands |= section.convertCommands(settingsMigrationService, commandConfig); - } - return didMoveCommands; - } - - /** - * Enum defining the forced command settings that should be moved from config.yml to the new commands.yml file. - */ - private enum MigratableCommandSection { - - ON_JOIN( - SettingsMigrationService::getOnLoginCommands, - Executor.PLAYER, - CommandConfig::getOnLogin, - OnLoginCommand::new), - - ON_JOIN_CONSOLE( - SettingsMigrationService::getOnLoginConsoleCommands, - Executor.CONSOLE, - CommandConfig::getOnLogin, - OnLoginCommand::new), - - ON_REGISTER( - SettingsMigrationService::getOnRegisterCommands, - Executor.PLAYER, - CommandConfig::getOnRegister), - - ON_REGISTER_CONSOLE( - SettingsMigrationService::getOnRegisterConsoleCommands, - Executor.CONSOLE, - CommandConfig::getOnRegister); - - private final Function> legacyCommandsGetter; - private final Executor executor; - private final Function> commandMapGetter; - private final BiFunction commandConstructor; - - /** - * Constructor. - * - * @param legacyCommandsGetter getter on MigrationService to get the deprecated command entries - * @param executor the executor of the commands - * @param commandMapGetter the getter for the commands map in the new settings structure to add the old - * settings to after conversion - */ - MigratableCommandSection(Function> legacyCommandsGetter, - Executor executor, - Function> commandMapGetter) { - this(legacyCommandsGetter, executor, commandMapGetter, Command::new); - } - - /** - * Constructor. - * - * @param legacyCommandsGetter getter on MigrationService to get the deprecated command entries - * @param executor the executor of the commands - * @param commandMapGetter the getter for the commands map in the new settings structure to add the old - * settings to after conversion - * @param commandConstructor constructor for creating a command object - */ - MigratableCommandSection( - Function> legacyCommandsGetter, - Executor executor, - Function> commandMapGetter, - BiFunction commandConstructor) { - - this.legacyCommandsGetter = legacyCommandsGetter; - this.executor = executor; - // This is horrible to be doing but this way we don't need to cast in convertCommands() - this.commandMapGetter = (Function) commandMapGetter; - this.commandConstructor = (BiFunction) commandConstructor; - } - - /** - * Adds the commands from the sections' settings migration service to the appropriate place in the new - * command config object. - * - * @param settingsMigrationService settings migration service to read old commands from - * @param commandConfig command config object to add converted commands to - * @return true if there were commands to migrate, false otherwise - */ - boolean convertCommands(SettingsMigrationService settingsMigrationService, CommandConfig commandConfig) { - List commands = legacyCommandsGetter.apply(settingsMigrationService).stream() - .map(cmd -> commandConstructor.apply(cmd, executor)).collect(Collectors.toList()); - - if (commands.isEmpty()) { - return false; - } - Map commandMap = commandMapGetter.apply(commandConfig); - commands.forEach(cmd -> commandMap.put(RandomStringUtils.generate(10), cmd)); - ConsoleLogger.info("Moving " + commands.size() + " commands of type " + this - + " from config.yml to commands.yml"); - return true; - } - } } diff --git a/src/test/java/fr/xephi/authme/settings/SettingsMigrationServiceTest.java b/src/test/java/fr/xephi/authme/settings/SettingsMigrationServiceTest.java index 0a952bb0..64e9c139 100644 --- a/src/test/java/fr/xephi/authme/settings/SettingsMigrationServiceTest.java +++ b/src/test/java/fr/xephi/authme/settings/SettingsMigrationServiceTest.java @@ -97,25 +97,6 @@ public class SettingsMigrationServiceTest { assertThat(migrationService.returnedValues, contains(true, false)); } - @Test - public void shouldKeepOldForceCommandSettings() throws IOException { - // given - File dataFolder = temporaryFolder.newFolder(); - File configFile = new File(dataFolder, "config.yml"); - Files.copy(getJarFile(OLD_CONFIG_FILE), configFile); - PropertyResource resource = new YamlFileResource(configFile); - SettingsMigrationService migrationService = new SettingsMigrationService(dataFolder); - - // when - migrationService.performMigrations(resource, AuthMeSettingsRetriever.buildConfigurationData().getProperties()); - - // then - assertThat(migrationService.getOnLoginCommands(), contains("spawn")); - assertThat(migrationService.getOnLoginConsoleCommands(), contains("sethome %p:lastloc", "msg %p Welcome back")); - assertThat(migrationService.getOnRegisterCommands(), contains("me registers", "msg CONSOLE hi")); - assertThat(migrationService.getOnRegisterConsoleCommands(), contains("sethome %p:regloc")); - } - private void verifyHasUpToDateSettings(Settings settings, File dataFolder) throws IOException { assertThat(settings.getProperty(ALLOWED_NICKNAME_CHARACTERS), equalTo(ALLOWED_NICKNAME_CHARACTERS.getDefaultValue())); assertThat(settings.getProperty(DELAY_JOIN_MESSAGE), equalTo(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 deleted file mode 100644 index 88ee90f3..00000000 --- a/src/test/java/fr/xephi/authme/settings/commandconfig/CommandConfigTestHelper.java +++ /dev/null @@ -1,35 +0,0 @@ -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 - */ - 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/CommandMigrationServiceTest.java b/src/test/java/fr/xephi/authme/settings/commandconfig/CommandMigrationServiceTest.java index 6bdc3207..1c68a457 100644 --- a/src/test/java/fr/xephi/authme/settings/commandconfig/CommandMigrationServiceTest.java +++ b/src/test/java/fr/xephi/authme/settings/commandconfig/CommandMigrationServiceTest.java @@ -6,34 +6,17 @@ import ch.jalu.configme.configurationdata.ConfigurationDataBuilder; import ch.jalu.configme.resource.PropertyResource; import ch.jalu.configme.resource.YamlFileResource; 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.junit.MockitoJUnitRunner; import java.io.File; -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 java.util.Optional; -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.containsInAnyOrder; 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 CommandMigrationService}. @@ -44,81 +27,11 @@ public class CommandMigrationServiceTest { @InjectMocks private CommandMigrationService commandMigrationService; - @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 = commandMigrationService.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<>(); - OnLoginCommand existingCommand = new OnLoginCommand("helpop %p has many alts", Executor.CONSOLE); - existingCommand.setIfNumberOfAccountsAtLeast(Optional.of(2)); - onLoginCommands.put("alert_on_alts", existingCommand); - 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 = commandMigrationService.transformOldCommands(commandConfig); - - // then - assertThat(result, equalTo(true)); - assertThat(commandConfig.getOnLogin(), sameInstance(onLoginCommands)); - Collection loginCmdList = onLoginCommands.values(); - assertThat(loginCmdList, contains( - equalTo(existingCommand), - 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))); - } - @Test public void shouldRewriteForEmptyFile() { // given diff --git a/src/test/java/fr/xephi/authme/settings/commandconfig/CommandYmlConsistencyTest.java b/src/test/java/fr/xephi/authme/settings/commandconfig/CommandYmlConsistencyTest.java index 8c599e92..34299481 100644 --- a/src/test/java/fr/xephi/authme/settings/commandconfig/CommandYmlConsistencyTest.java +++ b/src/test/java/fr/xephi/authme/settings/commandconfig/CommandYmlConsistencyTest.java @@ -4,22 +4,17 @@ import ch.jalu.configme.configurationdata.ConfigurationDataBuilder; import ch.jalu.configme.resource.PropertyResource; import ch.jalu.configme.resource.YamlFileResource; import fr.xephi.authme.TestHelper; -import fr.xephi.authme.settings.SettingsMigrationService; -import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; import org.junit.runner.RunWith; import org.mockito.InjectMocks; -import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; import java.io.File; -import java.util.Collections; import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertThat; -import static org.mockito.BDDMockito.given; /** * Tests that commands.yml is well-formed. @@ -30,20 +25,9 @@ public class CommandYmlConsistencyTest { @InjectMocks private CommandMigrationService commandMigrationService; - @Mock - private SettingsMigrationService settingsMigrationService; - @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); - @Before - public void setUpSettingsMigrationService() { - given(settingsMigrationService.getOnLoginCommands()).willReturn(Collections.emptyList()); - given(settingsMigrationService.getOnLoginConsoleCommands()).willReturn(Collections.emptyList()); - given(settingsMigrationService.getOnRegisterCommands()).willReturn(Collections.emptyList()); - given(settingsMigrationService.getOnRegisterConsoleCommands()).willReturn(Collections.emptyList()); - } - @Test public void shouldLoadWithNoMigrations() { // given