diff --git a/src/main/java/fr/xephi/authme/AuthMe.java b/src/main/java/fr/xephi/authme/AuthMe.java index 1c3fb05d1..630ebf0d1 100644 --- a/src/main/java/fr/xephi/authme/AuthMe.java +++ b/src/main/java/fr/xephi/authme/AuthMe.java @@ -231,7 +231,7 @@ public class AuthMe extends JavaPlugin { return; } - messages = new Messages(newSettings.getMessagesFile()); + messages = new Messages(newSettings.getMessagesFile(), newSettings.getDefaultMessagesFile()); // Connect to the database and setup tables try { diff --git a/src/main/java/fr/xephi/authme/output/Messages.java b/src/main/java/fr/xephi/authme/output/Messages.java index 731844d07..c0964ded2 100644 --- a/src/main/java/fr/xephi/authme/output/Messages.java +++ b/src/main/java/fr/xephi/authme/output/Messages.java @@ -1,7 +1,11 @@ package fr.xephi.authme.output; +import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.util.StringUtils; +import org.bukkit.ChatColor; import org.bukkit.command.CommandSender; +import org.bukkit.configuration.file.FileConfiguration; +import org.bukkit.configuration.file.YamlConfiguration; import java.io.File; @@ -12,15 +16,19 @@ import java.io.File; */ public class Messages { - private MessagesManager manager; + private FileConfiguration configuration; + private String fileName; + private File defaultFile; + private FileConfiguration defaultConfiguration; /** * Constructor. * * @param messageFile The messages file to use */ - public Messages(File messageFile) { - manager = new MessagesManager(messageFile); + public Messages(File messageFile, File defaultFile) { + initializeFile(messageFile); + this.defaultFile = defaultFile; } /** @@ -30,7 +38,7 @@ public class Messages { * @param key The key of the message to send */ public void send(CommandSender sender, MessageKey key) { - String[] lines = manager.retrieve(key.getKey()); + String[] lines = retrieve(key); for (String line : lines) { sender.sendMessage(line); } @@ -38,7 +46,7 @@ public class Messages { /** * Send the given message code to the player with the given tag replacements. Note that this method - * issues an exception if the number of supplied replacements doesn't correspond to the number of tags + * logs an error if the number of supplied replacements doesn't correspond to the number of tags * the message key contains. * * @param sender The entity to send the message to @@ -48,13 +56,13 @@ public class Messages { public void send(CommandSender sender, MessageKey key, String... replacements) { String message = retrieveSingle(key); String[] tags = key.getTags(); - if (replacements.length != tags.length) { - throw new IllegalStateException( - "Given replacement size does not match the tags in message key '" + key + "'"); - } - - for (int i = 0; i < tags.length; ++i) { - message = message.replace(tags[i], replacements[i]); + if (replacements.length == tags.length) { + for (int i = 0; i < tags.length; ++i) { + message = message.replace(tags[i], replacements[i]); + } + } else { + ConsoleLogger.showError("Invalid number of replacements for message key '" + key + "'"); + send(sender, key); } for (String line : message.split("\n")) { @@ -66,11 +74,18 @@ public class Messages { * Retrieve the message from the text file and return it split by new line as an array. * * @param key The message key to retrieve - * * @return The message split by new lines */ public String[] retrieve(MessageKey key) { - return manager.retrieve(key.getKey()); + final String code = key.getKey(); + String message = configuration.getString(code); + + if (message == null) { + ConsoleLogger.showError("Error getting message with key '" + code + "'. " + + "Please verify your config file at '" + fileName + "'"); + return formatMessage(getDefault(code)); + } + return formatMessage(message); } /** @@ -88,7 +103,38 @@ public class Messages { * Reload the messages manager. */ public void reload(File messagesFile) { - manager = new MessagesManager(messagesFile); + initializeFile(messagesFile); + } + + private void initializeFile(File messageFile) { + this.configuration = YamlConfiguration.loadConfiguration(messageFile); + this.fileName = messageFile.getName(); + } + + private String getDefault(String code) { + if (defaultFile == null) { + return getDefaultErrorMessage(code); + } + + if (defaultConfiguration == null) { + defaultConfiguration = YamlConfiguration.loadConfiguration(defaultFile); + } + String message = defaultConfiguration.getString(code); + return (message == null) + ? "Error retrieving message '" + code + "'" + : message; + } + + private static String getDefaultErrorMessage(String code) { + return "Error retrieving message '" + code + "'"; + } + + private static String[] formatMessage(String message) { + String[] lines = message.split("&n"); + for (int i = 0; i < lines.length; ++i) { + lines[i] = ChatColor.translateAlternateColorCodes('&', lines[i]); + } + return lines; } } diff --git a/src/main/java/fr/xephi/authme/output/MessagesManager.java b/src/main/java/fr/xephi/authme/output/MessagesManager.java deleted file mode 100644 index a280ea3ae..000000000 --- a/src/main/java/fr/xephi/authme/output/MessagesManager.java +++ /dev/null @@ -1,58 +0,0 @@ -package fr.xephi.authme.output; - -import fr.xephi.authme.ConsoleLogger; -import org.bukkit.ChatColor; -import org.bukkit.configuration.file.YamlConfiguration; - -import java.io.File; - -/** - * Class responsible for reading messages from a file and formatting them for Minecraft. - *
- * This class is used within {@link Messages}, which offers a high-level interface for accessing - * or sending messages from a properties file. - */ -class MessagesManager { - - private final YamlConfiguration configuration; - private final String fileName; - - /** - * Constructor for Messages. - * - * @param file the configuration file - */ - MessagesManager(File file) { - this.fileName = file.getName(); - this.configuration = YamlConfiguration.loadConfiguration(file); - } - - /** - * Retrieve the message from the configuration file. - * - * @param key The key to retrieve - * - * @return The message - */ - public String[] retrieve(String 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 '" + fileName + "'"); - return new String[]{ - retrievalError + "Please contact the admin to verify or update the AuthMe messages file."}; - } - - private static String[] formatMessage(String message) { - String[] lines = message.split("&n"); - for (int i = 0; i < lines.length; ++i) { - lines[i] = ChatColor.translateAlternateColorCodes('&', lines[i]); - } - return lines; - } - -} diff --git a/src/main/java/fr/xephi/authme/settings/NewSetting.java b/src/main/java/fr/xephi/authme/settings/NewSetting.java index 29e833d0a..7c509c95d 100644 --- a/src/main/java/fr/xephi/authme/settings/NewSetting.java +++ b/src/main/java/fr/xephi/authme/settings/NewSetting.java @@ -18,6 +18,7 @@ import org.yaml.snakeyaml.Yaml; import java.io.File; import java.io.FileWriter; import java.io.IOException; +import java.net.URL; import java.nio.charset.Charset; import java.util.ArrayList; import java.util.Arrays; @@ -108,6 +109,21 @@ public class NewSetting { return messagesFile; } + /** + * Return the default messages file within the JAR that should contain all messages. + * + * @return The default messages file, or {@code null} if it could not be retrieved + */ + public File getDefaultMessagesFile() { + String defaultFilePath = "/messages/messages_en.yml"; + URL url = NewSetting.class.getResource(defaultFilePath); + if (url == null) { + return null; + } + File file = new File(url.getFile()); + return file.exists() ? file : null; + } + public String getEmailMessage() { return emailMessage; } diff --git a/src/test/java/fr/xephi/authme/TestHelper.java b/src/test/java/fr/xephi/authme/TestHelper.java new file mode 100644 index 000000000..9f2ec360a --- /dev/null +++ b/src/test/java/fr/xephi/authme/TestHelper.java @@ -0,0 +1,28 @@ +package fr.xephi.authme; + +import java.io.File; +import java.net.URL; + +/** + * AuthMe test utilities. + */ +public final class TestHelper { + + private TestHelper() { + } + + /** + * Return a {@link File} to an existing file from the main (non-test) resources folder. + * + * @param path The absolute path to the file + * @return The project file + */ + public static File getJarFile(String path) { + URL url = TestHelper.class.getResource(path); + if (url == null) { + throw new IllegalStateException("File '" + path + "' could not be loaded"); + } + return new File(url.getFile()); + } + +} diff --git a/src/test/java/fr/xephi/authme/output/MessagesFileConsistencyTest.java b/src/test/java/fr/xephi/authme/output/MessagesFileConsistencyTest.java new file mode 100644 index 000000000..807d54bf5 --- /dev/null +++ b/src/test/java/fr/xephi/authme/output/MessagesFileConsistencyTest.java @@ -0,0 +1,43 @@ +package fr.xephi.authme.output; + +import fr.xephi.authme.TestHelper; +import fr.xephi.authme.util.StringUtils; +import org.bukkit.configuration.file.FileConfiguration; +import org.bukkit.configuration.file.YamlConfiguration; +import org.junit.Test; + +import java.io.File; + +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThat; + +/** + * Tests that the project's default language file contains a text for all message keys. + */ +public class MessagesFileConsistencyTest { + + private static final String DEFAULT_MESSAGES_FILE = "/messages/messages_en.yml"; + + @Test + public void shouldHaveAllMessages() { + File file = TestHelper.getJarFile(DEFAULT_MESSAGES_FILE); + FileConfiguration configuration = YamlConfiguration.loadConfiguration(file); + for (MessageKey messageKey : MessageKey.values()) { + verifyHasMessage(messageKey, configuration); + } + } + + private static void verifyHasMessage(MessageKey messageKey, FileConfiguration configuration) { + final String key = messageKey.getKey(); + final String message = configuration.getString(key); + + assertThat("Default messages file should have message for key '" + key + "'", + StringUtils.isEmpty(message), equalTo(false)); + + + for (String tag : messageKey.getTags()) { + assertThat("The message for key '" + key + "' contains the tag '" + tag + "' in the default messages file", + message.contains(tag), equalTo(true)); + } + } +} diff --git a/src/test/java/fr/xephi/authme/output/MessagesIntegrationTest.java b/src/test/java/fr/xephi/authme/output/MessagesIntegrationTest.java index 2d3e7b08f..e85790778 100644 --- a/src/test/java/fr/xephi/authme/output/MessagesIntegrationTest.java +++ b/src/test/java/fr/xephi/authme/output/MessagesIntegrationTest.java @@ -1,6 +1,8 @@ package fr.xephi.authme.output; +import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.ConsoleLoggerTestInitializer; +import fr.xephi.authme.TestHelper; import fr.xephi.authme.util.WrapperMock; import org.bukkit.command.CommandSender; import org.bukkit.entity.Player; @@ -11,11 +13,11 @@ import org.mockito.ArgumentCaptor; import org.mockito.Mockito; import java.io.File; -import java.net.URL; +import java.util.logging.Logger; import static org.hamcrest.Matchers.arrayWithSize; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.startsWith; import static org.junit.Assert.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -26,7 +28,8 @@ import static org.mockito.Mockito.verify; */ public class MessagesIntegrationTest { - private static final String YML_TEST_FILE = "messages_test.yml"; + private static final String YML_TEST_FILE = "/messages_test.yml"; + private static final String YML_DEFAULT_TEST_FILE = "/messages_default.yml"; private Messages messages; @BeforeClass @@ -39,15 +42,15 @@ public class MessagesIntegrationTest { * Loads the messages in the file {@code messages_test.yml} in the test resources folder. * The file does not contain all messages defined in {@link MessageKey} and its contents * reflect various test cases -- not what the keys stand for. + *
+ * Similarly, the {@code messages_default.yml} from the test resources represents a default
+ * file that should contain all messages, but again, for testing, it just contains a few.
*/
@Before
public void setUpMessages() {
- URL url = getClass().getClassLoader().getResource(YML_TEST_FILE);
- if (url == null) {
- throw new RuntimeException("File '" + YML_TEST_FILE + "' could not be loaded");
- }
-
- messages = new Messages(new File(url.getFile()));
+ File testFile = TestHelper.getJarFile(YML_TEST_FILE);
+ File defaultFile = TestHelper.getJarFile(YML_DEFAULT_TEST_FILE);
+ messages = new Messages(testFile, defaultFile);
}
@Test
@@ -101,20 +104,6 @@ public class MessagesIntegrationTest {
assertThat(message[0], equalTo("Apostrophes ' should be loaded correctly, don't you think?"));
}
- @Test
- public void shouldReturnErrorForUnknownCode() {
- // given
- // The following is a key that is not defined in the test file
- MessageKey key = MessageKey.UNREGISTERED_SUCCESS;
-
- // when
- String[] message = messages.retrieve(key);
-
- // then
- assertThat(message, arrayWithSize(1));
- assertThat(message[0], startsWith("Error getting message with key '"));
- }
-
@Test
public void shouldSendMessageToPlayer() {
// given
@@ -176,25 +165,88 @@ public class MessagesIntegrationTest {
assertThat(message, equalTo("Use /captcha THE_CAPTCHA to solve the captcha"));
}
- @Test(expected = RuntimeException.class)
- public void shouldThrowForInvalidReplacementCount() {
+ @Test
+ public void shouldLogErrorForInvalidReplacementCount() {
// given
+ Logger logger = mock(Logger.class);
+ ConsoleLogger.setLogger(logger);
MessageKey key = MessageKey.CAPTCHA_WRONG_ERROR;
// when
messages.send(mock(CommandSender.class), key, "rep", "rep2");
- // then - expect exception
+ // then
+ ArgumentCaptor