#1467 Try to clean up abstract message file handler hierarchy

- Move some handling with the default file configuration down to the help message file handler since it is the only one with such a behavior now

src/main/java/fr/xephi/authme/message/AbstractMessageFileHandler.java

@@ -13,8 +13,6 @@ import org.bukkit.configuration.file.YamlConfiguration;
 import javax.annotation.PostConstruct;
 import javax.inject.Inject;
 import java.io.File;
-import java.io.InputStream;
-import java.io.InputStreamReader;
 
 /**
  * Handles a YAML message file with a default file fallback.
@@ -33,21 +31,15 @@ public abstract class AbstractMessageFileHandler implements Reloadable {
     private String filename;
     private FileConfiguration configuration;
     private final String defaultFile;
-    private FileConfiguration defaultConfiguration;
-    private final String updateCommandAddition;
 
     protected AbstractMessageFileHandler() {
         this.defaultFile = createFilePath(DEFAULT_LANGUAGE);
-        String updateCommand = getUpdateCommand();
-        this.updateCommandAddition = updateCommand == null
-            ? ""
-            : " or run " + updateCommand;
     }
 
     @Override
     @PostConstruct
     public void reload() {
-        String language = getLanguage();
+        String language = settings.getProperty(PluginSettings.MESSAGES_LANGUAGE);
         filename = createFilePath(language);
         File messagesFile = initializeFile(filename);
         configuration = YamlConfiguration.loadConfiguration(messagesFile);
@@ -61,6 +53,10 @@ public abstract class AbstractMessageFileHandler implements Reloadable {
         return new File(dataFolder, filename);
     }
 
+    protected String getFilename() {
+        return filename;
+    }
+
     /**
      * Returns whether the message file configuration has an entry at the given path.
      *
@@ -79,13 +75,9 @@ public abstract class AbstractMessageFileHandler implements Reloadable {
      */
     public String getMessage(String key) {
         String message = configuration.getString(key);
-
-        if (message == null) {
-            ConsoleLogger.warning("Error getting message with key '" + key + "'. "
-                + "Please update your config file '" + filename + "'" + updateCommandAddition);
-            return getDefault(key);
-        }
-        return message;
+        return message == null
+            ? "Error retrieving message '" + key + "'"
+            : message;
     }
 
     /**
@@ -99,23 +91,6 @@ public abstract class AbstractMessageFileHandler implements Reloadable {
         return configuration.getString(key);
     }
 
-    /**
-     * Gets the message from the default file.
-     *
-     * @param key the key to retrieve the message for
-     * @return the message from the default file
-     */
-    private String getDefault(String key) {
-        if (defaultConfiguration == null) {
-            InputStream stream = FileUtils.getResourceFromJar(defaultFile);
-            defaultConfiguration = YamlConfiguration.loadConfiguration(new InputStreamReader(stream));
-        }
-        String message = defaultConfiguration.getString(key);
-        return message == null
-            ? "Error retrieving message '" + key + "'"
-            : message;
-    }
-
     /**
      * Creates the path to the messages file for the given language code.
      *
@@ -124,11 +99,6 @@ public abstract class AbstractMessageFileHandler implements Reloadable {
      */
     protected abstract String createFilePath(String language);
 
-    /**
-     * @return command with which the messages file can be updated; output when a message is missing from the file
-     */
-    protected abstract String getUpdateCommand();
-
     /**
      * Copies the messages file from the JAR to the local messages/ folder if it doesn't exist.
      *

src/main/java/fr/xephi/authme/message/HelpMessagesFileHandler.java

@@ -1,23 +1,62 @@
 package fr.xephi.authme.message;
 
+import fr.xephi.authme.ConsoleLogger;
+import fr.xephi.authme.util.FileUtils;
+import org.bukkit.configuration.file.FileConfiguration;
+import org.bukkit.configuration.file.YamlConfiguration;
+
 import javax.inject.Inject;
+import java.io.InputStream;
+import java.io.InputStreamReader;
 
 /**
  * File handler for the help_xx.yml resource.
  */
 public class HelpMessagesFileHandler extends AbstractMessageFileHandler {
 
+    private FileConfiguration defaultConfiguration;
+
     @Inject // Trigger injection in the superclass
     HelpMessagesFileHandler() {
     }
 
+    /**
+     * Returns the message for the given key.
+     *
+     * @param key the key to retrieve the message for
+     * @return the message
+     */
+    @Override
+    public String getMessage(String key) {
+        String message = getMessageIfExists(key);
+
+        if (message == null) {
+            ConsoleLogger.warning("Error getting message with key '" + key + "'. "
+                + "Please update your config file '" + getFilename() + "' or run /authme messages help");
+            return getDefault(key);
+        }
+        return message;
+    }
+
+    /**
+     * Gets the message from the default file.
+     *
+     * @param key the key to retrieve the message for
+     * @return the message from the default file
+     */
+    private String getDefault(String key) {
+        if (defaultConfiguration == null) {
+            InputStream stream = FileUtils.getResourceFromJar(createFilePath(DEFAULT_LANGUAGE));
+            defaultConfiguration = YamlConfiguration.loadConfiguration(new InputStreamReader(stream));
+        }
+        String message = defaultConfiguration.getString(key);
+        return message == null
+            ? "Error retrieving message '" + key + "'"
+            : message;
+    }
+
     @Override
     protected String createFilePath(String language) {
         return "messages/help_" + language + ".yml";
     }
-
-    @Override
-    protected String getUpdateCommand() {
-        return "/authme messages help";
-    }
 }

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

@@ -135,14 +135,6 @@ public class Messages {
         return message;
     }
 
-    /**
-     * Triggers a reload of the messages file. Note that this method is not necessary
-     * to be called for /authme reload.
-     */
-    public void reloadMessagesFile() {
-        messagesFileHandler.reload();
-    }
-
     private static String formatMessage(String message) {
         return ChatColor.translateAlternateColorCodes('&', message)
             .replace(NEWLINE_TAG, "\n");

src/main/java/fr/xephi/authme/message/MessagesFileHandler.java

@@ -10,9 +10,6 @@ import javax.inject.Inject;
  */
 public class MessagesFileHandler extends AbstractMessageFileHandler {
 
-    // TODO #1467: With the migration the messages handler has become so different that it would be worth
-    // to remove the extension and extract common features into a helper class instead
-
     @Inject
     private MessageUpdater messageUpdater;
 
@@ -43,9 +40,4 @@ public class MessagesFileHandler extends AbstractMessageFileHandler {
     protected String createFilePath(String language) {
         return "messages/messages_" + language + ".yml";
     }
-
-    @Override
-    protected String getUpdateCommand() {
-        return "/authme messages";
-    }
 }

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

@@ -46,6 +46,7 @@ public class MessagesIntegrationTest {
 
     private static final String YML_TEST_FILE = TestHelper.PROJECT_ROOT + "message/messages_test.yml";
     private Messages messages;
+    private MessagesFileHandler messagesFileHandler;
 
     @Rule
     public TemporaryFolder temporaryFolder = new TemporaryFolder();
@@ -72,8 +73,8 @@ public class MessagesIntegrationTest {
         FileUtils.create(testFile);
         Files.copy(TestHelper.getJarFile(YML_TEST_FILE), testFile);
 
-        MessagesFileHandler fileHandler = createMessagesFileHandler();
-        messages = new Messages(fileHandler);
+        messagesFileHandler = createMessagesFileHandler();
+        messages = new Messages(messagesFileHandler);
     }
 
     @Test
@@ -210,20 +211,6 @@ public class MessagesIntegrationTest {
         verify(logger).warning(argThat(containsString("Invalid number of replacements")));
     }
 
-    @Test
-    public void shouldGetMessageFromDefaultFile() {
-        // given
-        // Key is only present in default file
-        MessageKey key = MessageKey.MUST_REGISTER_MESSAGE;
-
-        // when
-        String message = messages.retrieveSingle(key);
-
-        // then
-        assertThat(message,
-            equalTo("§4Only registered users can join the server! Please visit http://example.com to register yourself!"));
-    }
-
     @Test
     public void shouldNotUseMessageFromDefaultFile() {
         // given
@@ -250,8 +237,13 @@ public class MessagesIntegrationTest {
     }
 
     @Test
-    public void shouldFormatDurationObjects() {
+    public void shouldFormatDurationObjects() throws IOException {
         // given
+        // Use the JAR's messages_en.yml file for this, so copy to the file we're using and reload the file handler
+        File testFile = new File(dataFolder, "messages/messages_test.yml");
+        Files.copy(TestHelper.getJarFile("/messages/messages_en.yml"), testFile);
+        messagesFileHandler.reload();
+
         Map<Duration, String> expectedTexts = ImmutableMap.<Duration, String>builder()
             .put(new Duration(1, TimeUnit.SECONDS), "1 second")
             .put(new Duration(12, TimeUnit.SECONDS), "12 seconds")