#450 Make reload & messages functionality use NewSetting

- Change ReloadCommand to use the new setting functionality
- Check and construct the messages file in NewSetting
- Unrelated: change MessagesManager not to extend CustomConfiguration anymore

src/main/java/fr/xephi/authme/AuthMe.java

@@ -34,7 +34,6 @@ import fr.xephi.authme.listener.AuthMePlayerListener18;
 import fr.xephi.authme.listener.AuthMeServerListener;
 import fr.xephi.authme.listener.AuthMeTabCompletePacketAdapter;
 import fr.xephi.authme.mail.SendMailSSL;
-import fr.xephi.authme.modules.ModuleManager;
 import fr.xephi.authme.output.ConsoleFilter;
 import fr.xephi.authme.output.Log4JFilter;
 import fr.xephi.authme.output.MessageKey;
@@ -110,7 +109,6 @@ public class AuthMe extends JavaPlugin {
     private NewSetting newSettings;
     private Messages messages;
     private JsonCache playerBackup;
-    private ModuleManager moduleManager;
     private PasswordSecurity passwordSecurity;
     private DataSource database;
 
@@ -219,15 +217,14 @@ public class AuthMe extends JavaPlugin {
         setPluginInfos();
 
         // Load settings and custom configurations, if it fails, stop the server due to security reasons.
+        newSettings = createNewSetting();
         if (loadSettings()) {
             server.shutdown();
             setEnabled(false);
             return;
         }
-        newSettings = createNewSetting();
 
-        // Set up messages & password security
+        messages = new Messages(Settings.messageFile);
-        messages = Messages.getInstance();
 
         // Connect to the database and setup tables
         try {
@@ -247,9 +244,6 @@ public class AuthMe extends JavaPlugin {
         permsMan = initializePermissionsManager();
         commandHandler = initializeCommandHandler(permsMan, messages, passwordSecurity, newSettings);
 
-        // Set up the module manager
-        setupModuleManager();
-
         // Setup otherAccounts file
         this.otherAccounts = OtherAccounts.getInstance();
 
@@ -327,21 +321,6 @@ public class AuthMe extends JavaPlugin {
         ConsoleLogger.info("AuthMe " + this.getDescription().getVersion() + " correctly enabled!");
     }
 
-    /**
-     * Set up the module manager.
-     */
-    private void setupModuleManager() {
-        // TODO: Clean this up!
-        // TODO: split the plugin in more modules
-        // TODO: log number of loaded modules
-
-        // Define the module manager instance
-        moduleManager = new ModuleManager(this);
-
-        // Load the modules
-        // int loaded = moduleManager.loadModules();
-    }
-
     /**
      * Set up the mail API, if enabled.
      */
@@ -469,7 +448,7 @@ public class AuthMe extends JavaPlugin {
 
     private NewSetting createNewSetting() {
         File configFile = new File(getDataFolder() + "config.yml");
-        return new NewSetting(getConfig(), configFile);
+        return new NewSetting(getConfig(), configFile, getDataFolder());
     }
 
     /**
@@ -536,18 +515,15 @@ public class AuthMe extends JavaPlugin {
         }
 
         // Do backup on stop if enabled
+        if (newSettings != null) {
             new PerformBackup(plugin, newSettings).doBackup(PerformBackup.BackupCause.STOP);
-
-        // Unload modules
-        if (moduleManager != null) {
-            moduleManager.unloadModules();
         }
 
         List<BukkitTask> pendingTasks = getServer().getScheduler().getPendingTasks();
         for (Iterator<BukkitTask> iterator = pendingTasks.iterator(); iterator.hasNext();) {
             BukkitTask pendingTask = iterator.next();
             if (!pendingTask.getOwner().equals(this) || pendingTask.isSync()) {
-                //remove all unrelevant tasks
+                //remove all irrelevant tasks
                 iterator.remove();
             }
         }
@@ -973,10 +949,6 @@ public class AuthMe extends JavaPlugin {
         return count >= Settings.getMaxJoinPerIp;
     }
 
-    public ModuleManager getModuleManager() {
-        return moduleManager;
-    }
-
     /**
      * Handle Bukkit commands.
      *

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

@@ -12,6 +12,7 @@ import fr.xephi.authme.settings.NewSetting;
 import fr.xephi.authme.settings.domain.Property;
 import org.bukkit.command.CommandSender;
 
+import java.io.File;
 import java.util.List;
 
 /**
@@ -101,6 +102,16 @@ public class CommandService {
         return authMe.getDataSource();
     }
 
+    /**
+     * Return the AuthMe instance for further manipulation. Use only if other methods from
+     * the command service cannot be used.
+     *
+     * @return The AuthMe instance
+     */
+    public AuthMe getAuthMe() {
+        return authMe;
+    }
+
     /**
      * Return the PasswordSecurity instance.
      *
@@ -152,6 +163,15 @@ public class CommandService {
         return messages.retrieve(key);
     }
 
+    /**
+     * Change the messages instance to retrieve messages from the given file.
+     *
+     * @param file The new file to read messages from
+     */
+    public void reloadMessages(File file) {
+        messages.reload(file);
+    }
+
     /**
      * Retrieve the given property's value.
      *
@@ -163,4 +183,13 @@ public class CommandService {
         return settings.getProperty(property);
     }
 
+    /**
+     * Return the settings manager.
+     *
+     * @return The settings manager
+     */
+    public NewSetting getSettings() {
+        return settings;
+    }
+
 }

src/main/java/fr/xephi/authme/command/executable/authme/ReloadCommand.java

@@ -1,36 +1,32 @@
 package fr.xephi.authme.command.executable.authme;
 
-import java.util.List;
-
-import org.bukkit.command.CommandSender;
-
 import fr.xephi.authme.AuthMe;
 import fr.xephi.authme.ConsoleLogger;
 import fr.xephi.authme.command.CommandService;
 import fr.xephi.authme.command.ExecutableCommand;
 import fr.xephi.authme.output.MessageKey;
-import fr.xephi.authme.output.Messages;
+import org.bukkit.command.CommandSender;
-import fr.xephi.authme.settings.Settings;
 
+import java.util.List;
+
+/**
+ * The reload command.
+ */
 public class ReloadCommand implements ExecutableCommand {
 
     @Override
     public void executeCommand(CommandSender sender, List<String> arguments, CommandService commandService) {
-        // AuthMe plugin instance
+        AuthMe plugin = commandService.getAuthMe();
-        AuthMe plugin = AuthMe.getInstance();
-
         try {
-            Settings.reload();
+            commandService.getSettings().reload();
-            Messages.getInstance().reloadManager();
+            commandService.reloadMessages(commandService.getSettings().getMessagesFile());
-            plugin.getModuleManager().reloadModules();
             plugin.setupDatabase();
+            commandService.send(sender, MessageKey.CONFIG_RELOAD_SUCCESS);
         } catch (Exception e) {
             sender.sendMessage("Error occurred during reload of AuthMe: aborting");
             ConsoleLogger.showError("Fatal error occurred! AuthMe instance ABORTED!");
             ConsoleLogger.writeStackTrace(e);
             plugin.stopOrUnload();
         }
-
-        commandService.send(sender, MessageKey.CONFIG_RELOAD_SUCCESS);
     }
 }

src/main/java/fr/xephi/authme/output/Messages.java

@@ -1,9 +1,10 @@
 package fr.xephi.authme.output;
 
-import fr.xephi.authme.settings.Settings;
 import fr.xephi.authme.util.StringUtils;
 import org.bukkit.command.CommandSender;
 
+import java.io.File;
+
 /**
  * Class for retrieving and sending translatable messages to players.
  * This class detects when the language settings have changed and will
@@ -11,27 +12,15 @@ import org.bukkit.command.CommandSender;
  */
 public class Messages {
 
-    private static Messages singleton;
-    private final String language;
     private MessagesManager manager;
 
-
-    private Messages(String language, MessagesManager manager) {
-        this.language = language;
-        this.manager = manager;
-    }
-
     /**
-     * Get the instance of Messages.
+     * Constructor.
      *
-     * @return The Messages instance
+     * @param messageFile The messages file to use
      */
-    public static Messages getInstance() {
+    public Messages(File messageFile) {
-        if (singleton == null) {
+        manager = new MessagesManager(messageFile);
-            MessagesManager manager = new MessagesManager(Settings.messageFile);
-            singleton = new Messages(Settings.messagesLanguage, manager);
-        }
-        return singleton;
     }
 
     /**
@@ -60,7 +49,8 @@ public class Messages {
         String message = retrieveSingle(key);
         String[] tags = key.getTags();
         if (replacements.length != tags.length) {
-            throw new RuntimeException("Given replacement size does not match the tags in message key '" + key + "'");
+            throw new IllegalStateException(
+                "Given replacement size does not match the tags in message key '" + key + "'");
         }
 
         for (int i = 0; i < tags.length; ++i) {
@@ -80,9 +70,6 @@ public class Messages {
      * @return The message split by new lines
      */
     public String[] retrieve(MessageKey key) {
-        if (!Settings.messagesLanguage.equalsIgnoreCase(language)) {
-            reloadManager();
-        }
         return manager.retrieve(key.getKey());
     }
 
@@ -100,8 +87,8 @@ public class Messages {
     /**
      * Reload the messages manager.
      */
-    public void reloadManager() {
+    public void reload(File messagesFile) {
-        manager = new MessagesManager(Settings.messageFile);
+        manager = new MessagesManager(messagesFile);
     }
 
 }

src/main/java/fr/xephi/authme/output/MessagesManager.java

@@ -1,8 +1,8 @@
 package fr.xephi.authme.output;
 
 import fr.xephi.authme.ConsoleLogger;
-import fr.xephi.authme.settings.CustomConfiguration;
 import org.bukkit.ChatColor;
+import org.bukkit.configuration.file.YamlConfiguration;
 
 import java.io.File;
 
@@ -12,7 +12,10 @@ import java.io.File;
  * This class is used within {@link Messages}, which offers a high-level interface for accessing
  * or sending messages from a properties file.
  */
-class MessagesManager extends CustomConfiguration {
+class MessagesManager {
+
+    private final YamlConfiguration configuration;
+    private final String fileName;
 
     /**
      * Constructor for Messages.
@@ -20,8 +23,8 @@ class MessagesManager extends CustomConfiguration {
      * @param file the configuration file
      */
     MessagesManager(File file) {
-        super(file);
+        this.fileName = file.getName();
-        load();
+        this.configuration = YamlConfiguration.loadConfiguration(file);
     }
 
     /**
@@ -31,24 +34,22 @@ class MessagesManager extends CustomConfiguration {
      *
      * @return The message
      */
-    String[] retrieve(String key) {
+    public String[] retrieve(String key) {
-        String message = (String) get(key);
+        String message = configuration.getString(key);
         if (message != null) {
             return formatMessage(message);
         }
 
         // Message is null: log key not being found and send error back as message
         String retrievalError = "Error getting message with key '" + key + "'. ";
-        ConsoleLogger.showError(retrievalError + "Please verify your config file at '"
+        ConsoleLogger.showError(retrievalError + "Please verify your config file at '" + fileName + "'");
-            + getConfigFile().getName() + "'");
         return new String[]{
             retrievalError + "Please contact the admin to verify or update the AuthMe messages file."};
     }
 
-    static String[] formatMessage(String message) {
+    private static String[] formatMessage(String message) {
         String[] lines = message.split("&n");
         for (int i = 0; i < lines.length; ++i) {
-            // We don't initialize a StringBuilder here because mostly we will only have one entry
             lines[i] = ChatColor.translateAlternateColorCodes('&', lines[i]);
         }
         return lines;

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

@@ -3,11 +3,13 @@ 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.properties.PluginSettings;
 import fr.xephi.authme.settings.properties.SettingsFieldRetriever;
 import fr.xephi.authme.settings.propertymap.PropertyMap;
 import fr.xephi.authme.util.CollectionUtils;
 import fr.xephi.authme.util.StringUtils;
 import org.bukkit.configuration.file.FileConfiguration;
+import org.bukkit.configuration.file.YamlConfiguration;
 import org.yaml.snakeyaml.DumperOptions;
 import org.yaml.snakeyaml.Yaml;
 
@@ -19,23 +21,30 @@ import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 
+import static fr.xephi.authme.util.StringUtils.makePath;
+
 /**
  * The new settings manager.
  */
 public class NewSetting {
 
-    private File file;
+    private final File pluginFolder;
+    private final File configFile;
     private FileConfiguration configuration;
+    private File messagesFile;
 
     /**
      * Constructor. Checks the given {@link FileConfiguration} object for completeness.
      *
      * @param configuration The configuration to interact with
-     * @param file The configuration file
+     * @param configFile The configuration file
+     * @param pluginFolder The AuthMe plugin folder
      */
-    public NewSetting(FileConfiguration configuration, File file) {
+    public NewSetting(FileConfiguration configuration, File configFile, File pluginFolder) {
         this.configuration = configuration;
-        this.file = file;
+        this.configFile = configFile;
+        this.pluginFolder = pluginFolder;
+        messagesFile = buildMessagesFile();
 
         PropertyMap propertyMap = SettingsFieldRetriever.getAllPropertyFields();
         if (SettingsMigrationService.checkAndMigrate(configuration, propertyMap)) {
@@ -49,13 +58,14 @@ public class NewSetting {
      * Constructor for testing purposes, allowing more options.
      *
      * @param configuration The FileConfiguration object to use
-     * @param file The file to write to
+     * @param configFile The file to write to
      * @param propertyMap The property map whose properties should be verified for presence, or null to skip this
      */
     @VisibleForTesting
-    NewSetting(FileConfiguration configuration, File file, PropertyMap propertyMap) {
+    NewSetting(FileConfiguration configuration, File configFile, PropertyMap propertyMap) {
         this.configuration = configuration;
-        this.file = file;
+        this.configFile = configFile;
+        this.pluginFolder = new File("");
 
         if (propertyMap != null && SettingsMigrationService.checkAndMigrate(configuration, propertyMap)) {
             save(propertyMap);
@@ -91,8 +101,24 @@ public class NewSetting {
         save(SettingsFieldRetriever.getAllPropertyFields());
     }
 
+    /**
+     * Return the messages file based on the messages language config.
+     *
+     * @return The messages file to read messages from
+     */
+    public File getMessagesFile() {
+        return messagesFile;
+    }
+
+    /**
+     * Reload the configuration.
+     */
+    public void reload() {
+        configuration = YamlConfiguration.loadConfiguration(configFile);
+    }
+
     private void save(PropertyMap propertyMap) {
-        try (FileWriter writer = new FileWriter(file)) {
+        try (FileWriter writer = new FileWriter(configFile)) {
             Yaml simpleYaml = newYaml(false);
             Yaml singleQuoteYaml = newYaml(true);
 
@@ -161,6 +187,20 @@ public class NewSetting {
         return join("\n" + indent(indent), representation.split("\\n"));
     }
 
+    private File buildMessagesFile() {
+        String languageCode = getProperty(PluginSettings.MESSAGES_LANGUAGE);
+        File messagesFile = buildMessagesFileFromCode(languageCode);
+        if (messagesFile.exists()) {
+            return messagesFile;
+        }
+        return buildMessagesFileFromCode("en");
+    }
+
+    private File buildMessagesFileFromCode(String language) {
+        return new File(pluginFolder.getName(),
+            makePath("messages", "messages_" + language + ".yml"));
+    }
+
     private static Yaml newYaml(boolean useSingleQuotes) {
         DumperOptions options = new DumperOptions();
         options.setDefaultFlowStyle(DumperOptions.FlowStyle.BLOCK);

src/main/java/fr/xephi/authme/util/StringUtils.java

@@ -4,6 +4,7 @@ import net.ricecode.similarity.LevenshteinDistanceStrategy;
 import net.ricecode.similarity.StringSimilarityService;
 import net.ricecode.similarity.StringSimilarityServiceImpl;
 
+import java.io.File;
 import java.util.Arrays;
 
 /**
@@ -37,12 +38,12 @@ public final class StringUtils {
     }
 
     /**
-     * Returns whether the given string contains any of the provided elements.
+     * Return whether the given string contains any of the provided elements.
      *
-     * @param str    the string to analyze
+     * @param str    The string to analyze
-     * @param pieces the items to check the string for
+     * @param pieces The items to check the string for
      *
-     * @return true if the string contains at least one of the items
+     * @return True if the string contains at least one of the items
      */
     public static boolean containsAny(String str, String... pieces) {
         if (str == null) {
@@ -60,21 +61,21 @@ public final class StringUtils {
      * Null-safe method for checking whether a string is empty. Note that the string
      * is trimmed, so this method also considers a string with whitespace as empty.
      *
-     * @param str the string to verify
+     * @param str The string to verify
      *
-     * @return true if the string is empty, false otherwise
+     * @return True if the string is empty, false otherwise
      */
     public static boolean isEmpty(String str) {
         return str == null || str.trim().isEmpty();
     }
 
     /**
-     * Joins a list of elements into a single string with the specified delimiter.
+     * Join a list of elements into a single string with the specified delimiter.
      *
-     * @param delimiter the delimiter to use
+     * @param delimiter The delimiter to use
-     * @param elements  the elements to join
+     * @param elements  The elements to join
      *
-     * @return a new String that is composed of the elements separated by the delimiter
+     * @return A new String that is composed of the elements separated by the delimiter
      */
     public static String join(String delimiter, Iterable<String> elements) {
         if (delimiter == null) {
@@ -95,12 +96,12 @@ public final class StringUtils {
     }
 
     /**
-     * Joins a list of elements into a single string with the specified delimiter.
+     * Join a list of elements into a single string with the specified delimiter.
      *
-     * @param delimiter the delimiter to use
+     * @param delimiter The delimiter to use
-     * @param elements  the elements to join
+     * @param elements  The elements to join
      *
-     * @return a new String that is composed of the elements separated by the delimiter
+     * @return A new String that is composed of the elements separated by the delimiter
      */
     public static String join(String delimiter, String... elements) {
         return join(delimiter, Arrays.asList(elements));
@@ -117,4 +118,15 @@ public final class StringUtils {
         return "[" + th.getClass().getSimpleName() + "]: " + th.getMessage();
     }
 
+    /**
+     * Construct a file path from the given elements, i.e. separate the given elements by the file separator.
+     *
+     * @param elements The elements to create a path with
+     *
+     * @return The created path
+     */
+    public static String makePath(String... elements) {
+        return join(File.separator, elements);
+    }
+
 }

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

@@ -18,6 +18,7 @@ import org.junit.Ignore;
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
 
+import java.io.File;
 import java.util.Arrays;
 import java.util.List;
 
@@ -95,19 +96,6 @@ public class CommandServiceTest {
         verify(commandMapper).mapPartsToCommand(sender, commandParts);
     }
 
-    @Test
-    @Ignore
-    public void shouldRunTaskInAsync() {
-        // given
-        Runnable runnable = mock(Runnable.class);
-
-        // when
-        commandService.runTaskAsynchronously(runnable);
-
-        // then
-        // TODO ljacqu 20151226: AuthMe#getServer() is final, i.e. not mockable
-    }
-
     @Test
     public void shouldGetDataSource() {
         // given
@@ -193,10 +181,40 @@ public class CommandServiceTest {
         given(settings.getProperty(property)).willReturn(7);
 
         // when
-        int result = settings.getProperty(property);
+        int result = commandService.getProperty(property);
 
         // then
         assertThat(result, equalTo(7));
         verify(settings).getProperty(property);
     }
+
+    @Test
+    public void shouldReloadMessages() {
+        // given
+        File file = new File("some/bogus-file.test");
+
+        // when
+        commandService.reloadMessages(file);
+
+        // then
+        verify(messages).reload(file);
+    }
+
+    @Test
+    public void shouldReturnSettings() {
+        // given/when
+        NewSetting result = commandService.getSettings();
+
+        // then
+        assertThat(result, equalTo(settings));
+    }
+
+    @Test
+    public void shouldReturnAuthMe() {
+        // given/when
+        AuthMe result = commandService.getAuthMe();
+
+        // then
+        assertThat(result, equalTo(authMe));
+    }
 }

src/test/java/fr/xephi/authme/output/MessagesIntegrationTest.java

@@ -1,6 +1,5 @@
 package fr.xephi.authme.output;
 
-import fr.xephi.authme.settings.Settings;
 import fr.xephi.authme.util.WrapperMock;
 import org.bukkit.command.CommandSender;
 import org.bukkit.entity.Player;
@@ -37,15 +36,12 @@ public class MessagesIntegrationTest {
     public void setUpMessages() {
         WrapperMock.createInstance();
 
-        Settings.messagesLanguage = "en";
         URL url = getClass().getClassLoader().getResource(YML_TEST_FILE);
         if (url == null) {
             throw new RuntimeException("File '" + YML_TEST_FILE + "' could not be loaded");
         }
 
-        Settings.messageFile = new File(url.getFile());
+        messages = new Messages(new File(url.getFile()));
-        Settings.messagesLanguage = "en";
-        messages = Messages.getInstance();
     }
 
     @Test

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

@@ -2,6 +2,7 @@ package fr.xephi.authme.settings;
 
 import fr.xephi.authme.settings.domain.Property;
 import fr.xephi.authme.settings.properties.TestConfiguration;
+import fr.xephi.authme.settings.propertymap.PropertyMap;
 import org.bukkit.configuration.file.YamlConfiguration;
 import org.junit.Test;
 import org.mockito.invocation.InvocationOnMock;
@@ -40,7 +41,7 @@ public class NewSettingTest {
         setReturnValue(file, TestConfiguration.SYSTEM_NAME, "myTestSys");
 
         // when / then
-        NewSetting settings = new NewSetting(file, new File("conf.txt"), null);
+        NewSetting settings = new NewSetting(file, new File("conf.txt"), (PropertyMap) null);
 
         assertThat(settings.getProperty(TestConfiguration.VERSION_NUMBER), equalTo(20));
         assertThat(settings.getProperty(TestConfiguration.SKIP_BORING_FEATURES), equalTo(true));

src/test/java/fr/xephi/authme/util/StringUtilsTest.java

@@ -2,6 +2,7 @@ package fr.xephi.authme.util;
 
 import org.junit.Test;
 
+import java.io.File;
 import java.net.MalformedURLException;
 import java.util.Arrays;
 import java.util.List;
@@ -135,4 +136,13 @@ public class StringUtilsTest {
         assertThat(StringUtils.getDifference("test", "bear"), equalTo(0.75));
         assertThat(StringUtils.getDifference("test", "something"), greaterThan(0.88));
     }
+
+    @Test
+    public void shouldConstructPath() {
+        // given/when
+        String result = StringUtils.makePath("path", "to", "test-file.txt");
+
+        // then
+        assertThat(result, equalTo("path" + File.separator + "to" + File.separator + "test-file.txt"));
+    }
 }