#450 Create migration service for new settings

- Create service to check if new settings is up-to-date
- Remove up-to-date checks from old Settings
- Update tests

src/main/java/fr/xephi/authme/settings/NewSetting.java

@@ -41,7 +41,9 @@ public class NewSetting {
         // be removed as we should check to rewrite the config.yml file only at one place
         // --------
         // PropertyMap propertyMap = SettingsFieldRetriever.getAllPropertyFields();
-        // if (!containsAllSettings(propertyMap)) {
+        // if (SettingsMigrationService.checkAndMigrate(configuration, propertyMap)) {
+        //     ConsoleLogger.info("Merged new config options");
+        //     ConsoleLogger.info("Please check your config.yml file for new settings!");
         //     save(propertyMap);
         // }
     }
@@ -58,7 +60,7 @@ public class NewSetting {
         this.configuration = configuration;
         this.file = file;
 
-        if (propertyMap != null && !containsAllSettings(propertyMap)) {
+        if (propertyMap != null && SettingsMigrationService.checkAndMigrate(configuration, propertyMap)) {
             save(propertyMap);
         }
     }
@@ -135,16 +137,6 @@ public class NewSetting {
         }
     }
 
-    @VisibleForTesting
-    boolean containsAllSettings(PropertyMap propertyMap) {
-        for (Property<?> property : propertyMap.keySet()) {
-            if (!property.isPresent(configuration)) {
-                return false;
-            }
-        }
-        return true;
-    }
-
     private static String indent(int level) {
         // YAML uses indentation of 4 spaces
         StringBuilder sb = new StringBuilder(level * 4);

src/main/java/fr/xephi/authme/settings/Settings.java

@@ -127,9 +127,6 @@ public final class Settings {
             plugin.saveDefaultConfig();
         }
         configFile.load(SETTINGS_FILE);
-        if (exist) {
-            instance.mergeConfig();
-        }
         loadVariables();
         if (exist) {
             instance.saveDefaults();
@@ -510,235 +507,6 @@ public final class Settings {
         return correct;
     }
 
-    public void mergeConfig() {
-        boolean changes = false;
-        if (contains("Xenoforo.predefinedSalt")) {
-            set("Xenoforo.predefinedSalt", null);
-            changes = true;
-        }
-        if (!contains("Protection.enableProtection")) {
-            set("Protection.enableProtection", false);
-            changes = true;
-        }
-        if (!contains("settings.restrictions.removeSpeed")) {
-            set("settings.restrictions.removeSpeed", true);
-            changes = true;
-        }
-        if (!contains("Protection.countries")) {
-            countries = new ArrayList<>();
-            countries.add("US");
-            countries.add("GB");
-            set("Protection.countries", countries);
-            changes = true;
-        }
-        if (!contains("Protection.enableAntiBot")) {
-            set("Protection.enableAntiBot", false);
-            changes = true;
-        }
-        if (!contains("Protection.antiBotSensibility")) {
-            set("Protection.antiBotSensibility", 5);
-            changes = true;
-        }
-        if (!contains("Protection.antiBotDuration")) {
-            set("Protection.antiBotDuration", 10);
-            changes = true;
-        }
-        if (!contains("settings.forceCommands")) {
-            set("settings.forceCommands", new ArrayList<String>());
-            changes = true;
-        }
-        if (!contains("settings.forceCommandsAsConsole")) {
-            set("settings.forceCommandsAsConsole", new ArrayList<String>());
-            changes = true;
-        }
-        if (!contains("Email.recallPlayers")) {
-            set("Email.recallPlayers", false);
-            changes = true;
-        }
-        if (!contains("Email.delayRecall")) {
-            set("Email.delayRecall", 5);
-            changes = true;
-        }
-        if (!contains("settings.useWelcomeMessage")) {
-            set("settings.useWelcomeMessage", true);
-            changes = true;
-        }
-        if (!contains("settings.restrictions.enablePasswordConfirmation")) {
-            set("settings.restrictions.enablePasswordConfirmation", true);
-            changes = true;
-        }
-        if (contains("settings.restrictions.enablePasswordVerifier")) {
-            set("settings.restrictions.enablePasswordVerifier", null);
-            changes = true;
-        }
-        if (!contains("settings.security.unsafePasswords")) {
-            List<String> str = new ArrayList<>();
-            str.add("123456");
-            str.add("password");
-            set("settings.security.unsafePasswords", str);
-            changes = true;
-        }
-        if (!contains("Protection.countriesBlacklist")) {
-            countriesBlacklist = new ArrayList<>();
-            countriesBlacklist.add("A1");
-            set("Protection.countriesBlacklist", countriesBlacklist);
-            changes = true;
-        }
-        if (!contains("settings.helpHeader")) {
-            set("settings.helpHeader", "AuthMeReloaded");
-            changes = true;
-        }
-        if (!contains("settings.broadcastWelcomeMessage")) {
-            set("settings.broadcastWelcomeMessage", false);
-            changes = true;
-        }
-        if (!contains("settings.registration.forceKickAfterRegister")) {
-            set("settings.registration.forceKickAfterRegister", false);
-            changes = true;
-        }
-        if (!contains("settings.registration.forceLoginAfterRegister")) {
-            set("settings.registration.forceLoginAfterRegister", false);
-            changes = true;
-        }
-        if (!contains("DataSource.mySQLColumnLogged")) {
-            set("DataSource.mySQLColumnLogged", "isLogged");
-            changes = true;
-        }
-        if (!contains("settings.restrictions.spawnPriority")) {
-            set("settings.restrictions.spawnPriority", "authme,essentials,multiverse,default");
-            changes = true;
-        }
-        if (!contains("settings.restrictions.maxLoginPerIp")) {
-            set("settings.restrictions.maxLoginPerIp", 0);
-            changes = true;
-        }
-        if (!contains("settings.restrictions.maxJoinPerIp")) {
-            set("settings.restrictions.maxJoinPerIp", 0);
-            changes = true;
-        }
-        if (!contains("VeryGames.enableIpCheck")) {
-            set("VeryGames.enableIpCheck", false);
-            changes = true;
-        }
-        if (configFile.getString("settings.restrictions.allowedNicknameCharacters").equals("[a-zA-Z0-9_?]*")) {
-            set("settings.restrictions.allowedNicknameCharacters", "[a-zA-Z0-9_]*");
-            changes = true;
-        }
-        if (contains("settings.delayJoinMessage")) {
-            set("settings.delayJoinMessage", null);
-            changes = true;
-        }
-        if (!contains("settings.delayJoinLeaveMessages")) {
-            set("settings.delayJoinLeaveMessages", true);
-            changes = true;
-        }
-        if (!contains("settings.restrictions.noTeleport")) {
-            set("settings.restrictions.noTeleport", false);
-            changes = true;
-        }
-        if (contains("Converter.Rakamak.newPasswordHash")) {
-            set("Converter.Rakamak.newPasswordHash", null);
-            changes = true;
-        }
-        if (!contains("Converter.CrazyLogin.fileName")) {
-            set("Converter.CrazyLogin.fileName", "accounts.db");
-            changes = true;
-        }
-        if (!contains("settings.restrictions.allowedPasswordCharacters")) {
-            set("settings.restrictions.allowedPasswordCharacters", "[\\x21-\\x7E]*");
-            changes = true;
-        }
-        if (!contains("settings.applyBlindEffect")) {
-            set("settings.applyBlindEffect", false);
-            changes = true;
-        }
-        if (!contains("Email.emailBlacklisted")) {
-            set("Email.emailBlacklisted", new ArrayList<String>());
-            changes = true;
-        }
-        if (contains("Performances")) {
-            set("Performances", null);
-            changes = true;
-        }
-        if (contains("Passpartu.enablePasspartu")) {
-            set("Passpartu.enablePasspartu", null);
-            changes = true;
-        }
-        if (contains("Passpartu")) {
-            set("Passpartu", null);
-            changes = true;
-        }
-        if (!contains("Email.emailWhitelisted")) {
-            set("Email.emailWhitelisted", new ArrayList<String>());
-            changes = true;
-        }
-        if (!contains("settings.forceRegisterCommands")) {
-            set("settings.forceRegisterCommands", new ArrayList<String>());
-            changes = true;
-        }
-        if (!contains("settings.forceRegisterCommandsAsConsole")) {
-            set("settings.forceRegisterCommandsAsConsole", new ArrayList<String>());
-            changes = true;
-        }
-        if (!contains("Hooks.customAttributes")) {
-            set("Hooks.customAttributes", false);
-            changes = true;
-        }
-        if (!contains("Purge.removePermissions")) {
-            set("Purge.removePermissions", false);
-            changes = true;
-        }
-        if (contains("Hooks.notifications")) {
-            set("Hooks.notifications", null);
-            changes = true;
-        }
-        if (contains("Hooks.chestshop")) {
-            set("Hooks.chestshop", null);
-            changes = true;
-        }
-        if (contains("Hooks.legacyChestshop")) {
-            set("Hooks.legacyChestshop", null);
-            changes = true;
-        }
-        if (!contains("Email.generateImage")) {
-            set("Email.generateImage", false);
-            changes = true;
-        }
-        if (!contains("DataSource.mySQLRealName")) {
-            set("DataSource.mySQLRealName", "realname");
-            changes = true;
-        }
-
-        if (!contains("settings.preventOtherCase")) {
-            set("settings.preventOtherCase", false);
-            changes = true;
-        }
-
-        if (contains("Email.mailText")) {
-            set("Email.mailText", null);
-            ConsoleLogger.showError("Remove Email.mailText from config, we now use the email.html file");
-        }
-
-        if (!contains("Security.stop.kickPlayersBeforeStopping")) {
-            set("Security.stop.kickPlayersBeforeStopping", true);
-            changes = true;
-        }
-
-        if (!contains("Email.emailOauth2Token"))
-            set("Email.emailOauth2Token", "");
-
-        if (!contains("Hooks.sendPlayerTo")) {
-            set("Hooks.sendPlayerTo", "");
-            changes = true;
-        }
-
-        if (changes) {
-            save();
-            plugin.getLogger().warning("Merged new Config Options - I'm not an error, please don't report me");
-            plugin.getLogger().warning("Please check your config.yml file for new configs!");
-        }
-    }
-
     /**
      * @param path
      *

src/main/java/fr/xephi/authme/settings/SettingsMigrationService.java

@@ -0,0 +1,102 @@
+package fr.xephi.authme.settings;
+
+import com.google.common.annotations.VisibleForTesting;
+import fr.xephi.authme.ConsoleLogger;
+import fr.xephi.authme.settings.domain.Property;
+import fr.xephi.authme.settings.propertymap.PropertyMap;
+import fr.xephi.authme.util.StringUtils;
+import fr.xephi.authme.util.Wrapper;
+import org.bukkit.configuration.file.FileConfiguration;
+
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+
+import static fr.xephi.authme.settings.properties.RestrictionSettings.ALLOWED_NICKNAME_CHARACTERS;
+
+/**
+ * Service for verifying that the configuration is up-to-date.
+ */
+public final class SettingsMigrationService {
+
+    private SettingsMigrationService() {
+    }
+
+    /**
+     * Checks the config file and does any necessary migrations.
+     *
+     * @param configuration The file configuration to check and migrate
+     * @param propertyMap The property map of all existing properties
+     * @return True if there is a change and the config must be saved, false if the config is up-to-date
+     */
+    public static boolean checkAndMigrate(FileConfiguration configuration, PropertyMap propertyMap) {
+        return performMigrations(configuration) || hasDeprecatedProperties(configuration)
+            || !containsAllSettings(configuration, propertyMap);
+    }
+
+    private static boolean performMigrations(FileConfiguration configuration) {
+        boolean changes = false;
+        if ("[a-zA-Z0-9_?]*".equals(configuration.getString(ALLOWED_NICKNAME_CHARACTERS.getPath()))) {
+            configuration.set(ALLOWED_NICKNAME_CHARACTERS.getPath(), "[a-zA-Z0-9_]*");
+            changes = true;
+        }
+        // TODO #450: Don't get the data folder statically
+        Wrapper w = Wrapper.getInstance();
+        changes = changes || performMailTextToFileMigration(configuration, w.getDataFolder());
+
+        return changes;
+    }
+
+    @VisibleForTesting
+    static boolean containsAllSettings(FileConfiguration configuration, PropertyMap propertyMap) {
+        for (Property<?> property : propertyMap.keySet()) {
+            if (!property.isPresent(configuration)) {
+                return false;
+            }
+        }
+        return true;
+    }
+
+    private static boolean hasDeprecatedProperties(FileConfiguration configuration) {
+        String[] deprecatedProperties = {
+            "Converter.Rakamak.newPasswordHash", "Hooks.chestshop", "Hooks.legacyChestshop", "Hooks.notifications",
+            "Passpartu", "Performances", "settings.delayJoinMessage", "settings.restrictions.enablePasswordVerifier",
+            "Xenoforo.predefinedSalt"};
+        for (String deprecatedPath : deprecatedProperties) {
+            if (configuration.contains(deprecatedPath)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    // --------
+    // Specific migrations
+    // --------
+
+    /**
+     * Check if {@code Email.mailText} is present and move it to the Email.html file if it doesn't exist yet.
+     *
+     * @param configuration The file configuration to verify
+     * @param dataFolder The plugin data folder
+     * @return True if a migration has been completed, false otherwise
+     */
+    private static boolean performMailTextToFileMigration(FileConfiguration configuration, File dataFolder) {
+        final String oldSettingPath = "Email.mailText";
+        if (!configuration.contains(oldSettingPath)) {
+            return false;
+        }
+
+        final File emailFile = new File(dataFolder, "email.html");
+        if (!emailFile.exists()) {
+            try (FileWriter fw = new FileWriter(emailFile)) {
+                fw.write(configuration.getString("Email.mailText"));
+            } catch (IOException e) {
+                ConsoleLogger.showError("Could not create email.html configuration file: "
+                    + StringUtils.formatException(e));
+            }
+        }
+        return true;
+    }
+
+}

src/test/java/fr/xephi/authme/settings/ConfigFileConsistencyTest.java

@@ -1,7 +1,5 @@
 package fr.xephi.authme.settings;
 
-import fr.xephi.authme.ReflectionTestUtils;
-import fr.xephi.authme.settings.NewSetting;
 import fr.xephi.authme.settings.domain.Property;
 import fr.xephi.authme.settings.properties.SettingsFieldRetriever;
 import fr.xephi.authme.settings.propertymap.PropertyMap;
@@ -36,16 +34,14 @@ public class ConfigFileConsistencyTest {
         // given
         URL url = this.getClass().getResource(CONFIG_FILE);
         File configFile = new File(url.getFile());
-        NewSetting settings = new NewSetting(YamlConfiguration.loadConfiguration(configFile), new File("bogus"), null);
+        FileConfiguration configuration = YamlConfiguration.loadConfiguration(configFile);
 
         // when
-        boolean result = settings.containsAllSettings(SettingsFieldRetriever.getAllPropertyFields());
+        boolean result = SettingsMigrationService.containsAllSettings(
+            configuration, SettingsFieldRetriever.getAllPropertyFields());
 
         // then
         if (!result) {
-            FileConfiguration configuration =
-                (FileConfiguration) ReflectionTestUtils.getFieldValue(NewSetting.class, settings, "configuration");
-
             Set<String> knownProperties = getAllKnownPropertyPaths();
             List<String> missingProperties = new ArrayList<>();
             for (String path : knownProperties) {

src/test/java/fr/xephi/authme/settings/NewSettingIntegrationTest.java

@@ -2,10 +2,10 @@ package fr.xephi.authme.settings;
 
 import com.google.common.collect.ImmutableMap;
 import fr.xephi.authme.ReflectionTestUtils;
-import fr.xephi.authme.settings.NewSetting;
 import fr.xephi.authme.settings.domain.Property;
 import fr.xephi.authme.settings.properties.TestConfiguration;
 import fr.xephi.authme.settings.propertymap.PropertyMap;
+import fr.xephi.authme.util.WrapperMock;
 import org.bukkit.configuration.file.YamlConfiguration;
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -35,6 +35,7 @@ public class NewSettingIntegrationTest {
 
     @BeforeClass
     public static void generatePropertyMap() {
+        WrapperMock.createInstance();
         propertyMap = new PropertyMap();
         for (Field field : TestConfiguration.class.getDeclaredFields()) {
             Object fieldValue = ReflectionTestUtils.getFieldValue(TestConfiguration.class, null, field.getName());

src/test/java/fr/xephi/authme/settings/NewSettingTest.java

@@ -1,6 +1,5 @@
 package fr.xephi.authme.settings;
 
-import fr.xephi.authme.settings.NewSetting;
 import fr.xephi.authme.settings.domain.Property;
 import fr.xephi.authme.settings.properties.TestConfiguration;
 import org.bukkit.configuration.file.YamlConfiguration;