From 1eaf321575f35cc7a42afdb7dcd922519e99dd4f Mon Sep 17 00:00:00 2001 From: ljacqu Date: Thu, 25 Jan 2018 21:48:48 +0100 Subject: [PATCH] #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 --- .../message/AbstractMessageFileHandler.java | 46 +++-------------- .../message/HelpMessagesFileHandler.java | 49 +++++++++++++++++-- .../fr/xephi/authme/message/Messages.java | 8 --- .../authme/message/MessagesFileHandler.java | 8 --- .../message/MessagesIntegrationTest.java | 26 ++++------ 5 files changed, 61 insertions(+), 76 deletions(-) diff --git a/src/main/java/fr/xephi/authme/message/AbstractMessageFileHandler.java b/src/main/java/fr/xephi/authme/message/AbstractMessageFileHandler.java index f88c6f3f5..4dbbe33bf 100644 --- a/src/main/java/fr/xephi/authme/message/AbstractMessageFileHandler.java +++ b/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. * diff --git a/src/main/java/fr/xephi/authme/message/HelpMessagesFileHandler.java b/src/main/java/fr/xephi/authme/message/HelpMessagesFileHandler.java index 18eed4ae8..378887537 100644 --- a/src/main/java/fr/xephi/authme/message/HelpMessagesFileHandler.java +++ b/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"; - } } diff --git a/src/main/java/fr/xephi/authme/message/Messages.java b/src/main/java/fr/xephi/authme/message/Messages.java index 8c9f2f42e..11227affc 100644 --- a/src/main/java/fr/xephi/authme/message/Messages.java +++ b/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"); diff --git a/src/main/java/fr/xephi/authme/message/MessagesFileHandler.java b/src/main/java/fr/xephi/authme/message/MessagesFileHandler.java index 716bf33ed..7ca19249e 100644 --- a/src/main/java/fr/xephi/authme/message/MessagesFileHandler.java +++ b/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"; - } } diff --git a/src/test/java/fr/xephi/authme/message/MessagesIntegrationTest.java b/src/test/java/fr/xephi/authme/message/MessagesIntegrationTest.java index 1c67f4d0d..7e854334d 100644 --- a/src/test/java/fr/xephi/authme/message/MessagesIntegrationTest.java +++ b/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 expectedTexts = ImmutableMap.builder() .put(new Duration(1, TimeUnit.SECONDS), "1 second") .put(new Duration(12, TimeUnit.SECONDS), "12 seconds")