#1055 Grouping and sorting of text messages (PR #198)

* Initial grouping of messages_en.yml (thanks to input by @Twonox)
* Change MessageFileVerifier to only do verification - no writing
* Create classes for sorting and grouping messages as per the messages_en file

src/test/java/tools/messages/MessageFileComments.java

@@ -0,0 +1,13 @@
+package tools.messages;
+
+import java.util.List;
+
+/**
+ * Represents a section of one or more consecutive comment lines in a file.
+ */
+public class MessageFileComments extends MessageFileElement {
+
+    public MessageFileComments(List<String> lines) {
+        super(lines);
+    }
+}

src/test/java/tools/messages/MessageFileElement.java

@@ -0,0 +1,20 @@
+package tools.messages;
+
+import java.util.Collections;
+import java.util.List;
+
+/**
+ * An element (a logical unit) in a messages file.
+ */
+public abstract class MessageFileElement {
+
+    private final List<String> lines;
+
+    protected MessageFileElement(List<String> lines) {
+        this.lines = Collections.unmodifiableList(lines);
+    }
+
+    public List<String> getLines() {
+        return lines;
+    }
+}

src/test/java/tools/messages/MessageFileElementMerger.java

@@ -0,0 +1,132 @@
+package tools.messages;
+
+import fr.xephi.authme.message.MessageKey;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+/**
+ * Creates the same order of message file elements as the given default message elements,
+ * using the local file's own elements as much as possible and filling in elements from the
+ * default messages file where necessary.
+ * <p>
+ * The current implementation (of this merger and of the {@link MessageFileElementReader reader})
+ * has the following limitations:
+ * <ul>
+ *   <li>It assumes that new comments are only ever added to the bottom of the default file.</li>
+ *   <li>If a file only has a partial number of the comments present in the default messages file,
+ *       the file's comments will be moved to the top. This most likely adds the comment above the
+ *       wrong group of messages.</li>
+ *   <li>Assumes that the text for a message only takes one line.</li>
+ *   <li>Ignores the last comment section of a file if it is not followed by any message entry.</li>
+ * </ul>
+ */
+public class MessageFileElementMerger {
+
+    /** Ordered list of comments in the messages file. */
+    private final List<MessageFileComments> comments;
+    /** List of message entries by corresponding MessageKey. */
+    private final Map<MessageKey, MessageFileEntry> entries;
+    /**
+     * Ordered list of file elements of the default file. The entries of the (non-default) messages
+     * file are based on this.
+     */
+    private final List<MessageFileElement> defaultFileElements;
+    /** Missing tags in message entries. */
+    private final Map<MessageKey, Collection<String>> missingTags;
+    /** Counter for encountered comment elements. */
+    private int commentsCounter = 0;
+
+    private MessageFileElementMerger(List<MessageFileElement> defaultFileElements,
+                                     List<MessageFileComments> comments,
+                                     Map<MessageKey, MessageFileEntry> entries,
+                                     Map<MessageKey, Collection<String>> missingTags) {
+        this.defaultFileElements = defaultFileElements;
+        this.comments = comments;
+        this.entries = entries;
+        this.missingTags = missingTags;
+    }
+
+    /**
+     * Returns a list of file elements that follow the order and type of the provided default file elements.
+     * In other words, using the list of default file elements as template and fallback, it returns the provided
+     * file elements in the same order and fills in default file elements if an equivalent in {@code fileElements}
+     * is not present.
+     *
+     * @param fileElements file elements to sort and merge
+     * @param defaultFileElements file elements of the default file to base the operation on
+     * @param missingTags list of missing tags per message key
+     * @return ordered and complete list of file elements
+     */
+    public static List<MessageFileElement> mergeElements(List<MessageFileElement> fileElements,
+                                                         List<MessageFileElement> defaultFileElements,
+                                                         Map<MessageKey, Collection<String>> missingTags) {
+        List<MessageFileComments> comments = filteredStream(fileElements, MessageFileComments.class)
+            .collect(Collectors.toList());
+        Map<MessageKey, MessageFileEntry> entries = filteredStream(fileElements, MessageFileEntry.class)
+            .collect(Collectors.toMap(MessageFileEntry::getMessageKey, Function.identity(), (e1, e2) -> e1));
+
+        MessageFileElementMerger merger = new MessageFileElementMerger(
+            defaultFileElements, comments, entries, missingTags);
+        return merger.mergeElements();
+    }
+
+    private List<MessageFileElement> mergeElements() {
+        List<MessageFileElement> mergedElements = new ArrayList<>(defaultFileElements.size());
+        for (MessageFileElement element : defaultFileElements) {
+            if (element instanceof MessageFileComments) {
+                mergedElements.add(getCommentsEntry((MessageFileComments) element));
+            } else if (element instanceof MessageFileEntry) {
+                mergedElements.add(getEntryForDefaultMessageEntry((MessageFileEntry) element));
+            } else {
+                throw new IllegalStateException("Found element of unknown subtype '" + element.getClass() + "'");
+            }
+        }
+        return mergedElements;
+    }
+
+    private MessageFileComments getCommentsEntry(MessageFileComments defaultComments) {
+        if (comments.size() > commentsCounter) {
+            MessageFileComments localComments = comments.get(commentsCounter);
+            ++commentsCounter;
+            return localComments;
+        }
+        return defaultComments;
+    }
+
+    private MessageFileElement getEntryForDefaultMessageEntry(MessageFileEntry entry) {
+        MessageKey messageKey = entry.getMessageKey();
+        if (messageKey == null) {
+            throw new IllegalStateException("Default message file should not have unknown entries, but "
+                + " entry with lines '" + entry.getLines() + "' has message key = null");
+        }
+
+        MessageFileEntry localEntry = entries.get(messageKey);
+        if (localEntry == null) {
+            return entry.convertToMissingEntryComment();
+        }
+
+        Collection<String> absentTags = missingTags.get(messageKey);
+        return absentTags == null
+            ? localEntry
+            : localEntry.convertToEntryWithMissingTagsComment(absentTags);
+    }
+
+    /**
+     * Creates a stream of the entries in {@code collection} with only the elements which are of type {@code clazz}.
+     *
+     * @param collection the collection to stream over
+     * @param clazz the class to restrict the elements to
+     * @param <P> the collection type (parent)
+     * @param <C> the type to restrict to (child)
+     * @return stream over all elements of the given type
+     */
+    private static <P, C extends P> Stream<C> filteredStream(Collection<P> collection, Class<C> clazz) {
+        return collection.stream().filter(clazz::isInstance).map(clazz::cast);
+    }
+}

src/test/java/tools/messages/MessageFileElementReader.java

@@ -0,0 +1,78 @@
+package tools.messages;
+
+import tools.utils.FileIoUtils;
+
+import java.io.File;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+/**
+ * Reads a messages file and returns the lines as corresponding {@link MessageFileElement} objects.
+ *
+ * @see MessageFileElementMerger
+ */
+public class MessageFileElementReader {
+
+    private final List<MessageFileElement> elements = new ArrayList<>();
+
+    private MessageFileElementReader() {
+    }
+
+    /**
+     * Returns the message files as separate {@link MessageFileElement elements}.
+     *
+     * @param file the file to read
+     * @return the file's elements
+     */
+    public static List<MessageFileElement> readFileIntoElements(File file) {
+        checkArgument(file.exists(), "Template file '" + file + "' must exist");
+
+        MessageFileElementReader reader = new MessageFileElementReader();
+        reader.loadElements(file.toPath());
+        return reader.elements;
+    }
+
+    private void loadElements(Path path) {
+        List<String> currentCommentSection = new ArrayList<>(10);
+        for (String line : FileIoUtils.readLinesFromFile(path)) {
+            if (isTodoComment(line)) {
+                continue;
+            }
+
+            if (isCommentLine(line)) {
+                currentCommentSection.add(line);
+            } else if (MessageFileEntry.isMessageEntry(line)) {
+                if (!currentCommentSection.isEmpty()) {
+                    processTempCommentsList(currentCommentSection);
+                }
+                elements.add(new MessageFileEntry(line));
+            } else {
+                throw new IllegalStateException("Could not match line '" + line + "' to any type");
+            }
+        }
+    }
+
+    /**
+     * Creates a message file comments element for one or more read comment lines. Does not add
+     * a comments element if the read lines are only empty lines.
+     *
+     * @param comments the read comment lines
+     */
+    private void processTempCommentsList(List<String> comments) {
+        if (comments.stream().anyMatch(c -> !c.trim().isEmpty())) {
+            elements.add(new MessageFileComments(new ArrayList<>(comments)));
+        }
+        comments.clear();
+    }
+
+    private static boolean isCommentLine(String line) {
+        return line.trim().isEmpty() || line.trim().startsWith("#");
+    }
+
+    private static boolean isTodoComment(String line) {
+        return line.startsWith("# TODO ");
+    }
+}

src/test/java/tools/messages/MessageFileEntry.java

@@ -0,0 +1,85 @@
+package tools.messages;
+
+import fr.xephi.authme.message.MessageKey;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+
+import static java.util.Collections.singletonList;
+
+/**
+ * Entry in a message file for a message key.
+ */
+public class MessageFileEntry extends MessageFileElement {
+
+    private static final Pattern MESSAGE_ENTRY_REGEX = Pattern.compile("([a-zA-Z_-]+): .*");
+    private final MessageKey messageKey;
+
+    public MessageFileEntry(String line) {
+        this(singletonList(line), extractMessageKey(line));
+    }
+
+    private MessageFileEntry(List<String> lines, MessageKey messageKey) {
+        super(lines);
+        this.messageKey = messageKey;
+    }
+
+    public static boolean isMessageEntry(String line) {
+        return MESSAGE_ENTRY_REGEX.matcher(line).matches();
+    }
+
+    public MessageKey getMessageKey() {
+        return messageKey;
+    }
+
+    /**
+     * Based on this entry, creates a comments element indicating that this message is missing.
+     *
+     * @return comments element based on this message element
+     */
+    public MessageFileComments convertToMissingEntryComment() {
+        List<String> comments = getLines().stream().map(l -> "# TODO " + l).collect(Collectors.toList());
+        return new MessageFileComments(comments);
+    }
+
+    /**
+     * Creates an adapted message file entry object with a comment for missing tags.
+     *
+     * @param missingTags the tags missing in the message
+     * @return message file entry with verification comment
+     */
+    public MessageFileEntry convertToEntryWithMissingTagsComment(Collection<String> missingTags) {
+        List<String> lines = new ArrayList<>(getLines().size() + 1);
+        lines.add("# TODO: Missing tags " + String.join(", ", missingTags));
+        lines.addAll(getLines());
+        return new MessageFileEntry(lines, messageKey);
+    }
+
+    /**
+     * Returns the {@link MessageKey} this entry is for. Returns {@code null} if the message key could not be matched.
+     *
+     * @param line the line to process
+     * @return the associated message key, or {@code null} if no match was found
+     */
+    private static MessageKey extractMessageKey(String line) {
+        Matcher matcher = MESSAGE_ENTRY_REGEX.matcher(line);
+        if (matcher.find()) {
+            String key = matcher.group(1);
+            return fromKey(key);
+        }
+        throw new IllegalStateException("Could not extract message key from line '" + line + "'");
+    }
+
+    private static MessageKey fromKey(String key) {
+        for (MessageKey messageKey : MessageKey.values()) {
+            if (messageKey.getKey().equals(key)) {
+                return messageKey;
+            }
+        }
+        return null;
+    }
+}

src/test/java/tools/messages/MessageFileVerifier.java

@@ -1,21 +1,14 @@
 package tools.messages;
 
 import com.google.common.base.Preconditions;
-import com.google.common.base.Predicate;
 import com.google.common.collect.HashMultimap;
-import com.google.common.collect.Iterables;
 import com.google.common.collect.Multimap;
 import fr.xephi.authme.message.MessageKey;
 import org.bukkit.configuration.file.FileConfiguration;
 import org.bukkit.configuration.file.YamlConfiguration;
-import tools.utils.FileIoUtils;
 
 import java.io.File;
-import java.util.ArrayList;
-import java.util.Collection;
 import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
 import java.util.Set;
 
 /**
@@ -26,8 +19,8 @@ public class MessageFileVerifier {
 
     private final File messagesFile;
     private final Set<String> unknownKeys = new HashSet<>();
-    private final List<MissingKey> missingKeys = new ArrayList<>();
-    private final Multimap<String, String> missingTags = HashMultimap.create();
+    private final Set<MessageKey> missingKeys = new HashSet<>();
+    private final Multimap<MessageKey, String> missingTags = HashMultimap.create();
 
     /**
      * Create a verifier that verifies the given messages file.
@@ -56,16 +49,16 @@ public class MessageFileVerifier {
      *
      * @return The list of missing keys in the file
      */
-    public List<MissingKey> getMissingKeys() {
+    public Set<MessageKey> getMissingKeys() {
         return missingKeys;
     }
 
     /**
      * Return the collection of tags the message key defines that aren't present in the read line.
      *
-     * @return Collection of missing tags per message key. Key = message key, value = missing tag.
+     * @return Collection of missing tags per message key
      */
-    public Multimap<String, String> getMissingTags() {
+    public Multimap<MessageKey, String> getMissingTags() {
         return missingTags;
     }
 
@@ -78,7 +71,7 @@ public class MessageFileVerifier {
             if (configuration.isString(key)) {
                 checkTagsInMessage(messageKey, configuration.getString(key));
             } else {
-                missingKeys.add(new MissingKey(key));
+                missingKeys.add(messageKey);
             }
         }
 
@@ -93,84 +86,11 @@ public class MessageFileVerifier {
     private void checkTagsInMessage(MessageKey messageKey, String message) {
         for (String tag : messageKey.getTags()) {
             if (!message.contains(tag)) {
-                missingTags.put(messageKey.getKey(), tag);
+                missingTags.put(messageKey, tag);
             }
         }
     }
 
-    /**
-     * Add missing keys to the file with the provided default (English) message.
-     *
-     * @param defaultMessages The collection of default messages
-     */
-    public void addMissingKeys(FileConfiguration defaultMessages) {
-        final List<String> fileLines = FileIoUtils.readLinesFromFile(messagesFile.toPath());
-
-        List<MissingKey> keysToAdd = new ArrayList<>();
-        for (MissingKey entry : missingKeys) {
-            final String key = entry.getKey();
-
-            if (!entry.getWasAdded() && defaultMessages.get(key) != null) {
-                keysToAdd.add(entry);
-            }
-        }
-
-        // Add missing keys as comments to the bottom of the file
-        for (MissingKey keyToAdd : keysToAdd) {
-            final String key = keyToAdd.getKey();
-            int indexOfComment = Iterables.indexOf(fileLines, isCommentFor(key));
-            if (indexOfComment != -1) {
-                // Comment for keyToAdd already exists, so remove it since we're going to add it
-                fileLines.remove(indexOfComment);
-            }
-            String comment = commentForKey(key) + "'" +
-                defaultMessages.getString(key).replace("'", "''") + "'";
-            fileLines.add(comment);
-            keyToAdd.setWasAdded(true);
-        }
-
-        // Add a comment above messages missing a tag
-        for (Map.Entry<String, Collection<String>> entry : missingTags.asMap().entrySet()) {
-            final String key = entry.getKey();
-            addCommentForMissingTags(fileLines, key, entry.getValue());
-        }
-
-        FileIoUtils.writeToFile(messagesFile.toPath(), String.join("\n", fileLines));
-    }
-
-    /**
-     * Add a comment above a message to note the tags the message is missing. Removes
-     * any similar comment that may already be above the message.
-     *
-     * @param fileLines The lines of the file (to modify)
-     * @param key The key of the message
-     * @param tags The missing tags
-     */
-    private void addCommentForMissingTags(List<String> fileLines, final String key, Collection<String> tags) {
-        int indexForComment = Iterables.indexOf(fileLines, isCommentFor(key));
-        if (indexForComment == -1) {
-            indexForComment = Iterables.indexOf(fileLines, input -> input.startsWith(key + ": "));
-            if (indexForComment == -1) {
-                System.err.println("Error adding comment for key '" + key + "': couldn't find entry in file lines");
-                return;
-            }
-        } else {
-            fileLines.remove(indexForComment);
-        }
-
-        String tagWord = tags.size() > 1 ? "tags" : "tag";
-        fileLines.add(indexForComment, commentForKey(key)
-            + String.format("Missing %s %s", tagWord, String.join(", ", tags)));
-    }
-
-    private static String commentForKey(String key) {
-        return String.format("# TODO %s: ", key);
-    }
-
-    private static Predicate<String> isCommentFor(final String key) {
-        return input -> input.startsWith(commentForKey(key));
-    }
-
     private static boolean messageKeyExists(String key) {
         for (MessageKey messageKey : MessageKey.values()) {
             if (messageKey.getKey().equals(key)) {

src/test/java/tools/messages/MissingKey.java

@@ -1,37 +0,0 @@
-package tools.messages;
-
-/**
- * Missing message key in a file.
- */
-public class MissingKey {
-
-    private final String key;
-    private boolean wasAdded;
-
-    public MissingKey(String key) {
-        this.key = key;
-    }
-
-    /**
-     * @return the message key that is missing
-     */
-    public String getKey() {
-        return key;
-    }
-
-    public void setWasAdded(boolean wasAdded) {
-        this.wasAdded = wasAdded;
-    }
-
-    /**
-     * @return true if a comment was added to the file, false otherwise
-     */
-    public boolean getWasAdded() {
-        return wasAdded;
-    }
-
-    @Override
-    public String toString() {
-        return key;
-    }
-}

src/test/java/tools/messages/VerifyMessagesTask.java

@@ -1,9 +1,9 @@
 package tools.messages;
 
 import com.google.common.collect.Multimap;
+import fr.xephi.authme.message.MessageKey;
 import fr.xephi.authme.util.StringUtils;
-import org.bukkit.configuration.file.FileConfiguration;
-import org.bukkit.configuration.file.YamlConfiguration;
+import tools.utils.FileIoUtils;
 import tools.utils.ToolTask;
 import tools.utils.ToolsConstants;
 
@@ -54,9 +54,9 @@ public final class VerifyMessagesTask implements ToolTask {
             messageFiles = Collections.singletonList(customFile);
         }
 
-        FileConfiguration defaultMessages = null;
+        List<MessageFileElement> defaultFileElements = null;
         if (addMissingKeys) {
-            defaultMessages = YamlConfiguration.loadConfiguration(new File(DEFAULT_MESSAGES_FILE));
+            defaultFileElements = MessageFileElementReader.readFileIntoElements(new File(DEFAULT_MESSAGES_FILE));
         }
 
         // Verify the given files
@@ -64,9 +64,10 @@ public final class VerifyMessagesTask implements ToolTask {
             System.out.println("Verifying '" + file.getName() + "'");
             MessageFileVerifier verifier = new MessageFileVerifier(file);
             if (addMissingKeys) {
-                verifyFileAndAddKeys(verifier, defaultMessages);
+                outputVerificationResults(verifier);
+                updateMessagesFile(file, verifier, defaultFileElements);
             } else {
-                verifyFile(verifier);
+                outputVerificationResults(verifier);
             }
         }
 
@@ -75,8 +76,13 @@ public final class VerifyMessagesTask implements ToolTask {
         }
     }
 
-    private static void verifyFile(MessageFileVerifier verifier) {
-        List<MissingKey> missingKeys = verifier.getMissingKeys();
+    /**
+     * Outputs the verification results to the console.
+     *
+     * @param verifier the verifier whose results should be output
+     */
+    private static void outputVerificationResults(MessageFileVerifier verifier) {
+        Set<MessageKey> missingKeys = verifier.getMissingKeys();
         if (!missingKeys.isEmpty()) {
             System.out.println("  Missing keys: " + missingKeys);
         }
@@ -86,44 +92,32 @@ public final class VerifyMessagesTask implements ToolTask {
             System.out.println("  Unknown keys: " + unknownKeys);
         }
 
-        Multimap<String, String> missingTags = verifier.getMissingTags();
-        for (Map.Entry<String, String> entry : missingTags.entries()) {
-            System.out.println("  Missing tag '" + entry.getValue() + "' in entry with key '" + entry.getKey() + "'");
+        Multimap<MessageKey, String> missingTags = verifier.getMissingTags();
+        for (Map.Entry<MessageKey, String> entry : missingTags.entries()) {
+            System.out.println("  Missing tag '" + entry.getValue() + "' in entry '" + entry.getKey() + "'");
         }
     }
 
-    public static void verifyFileAndAddKeys(MessageFileVerifier verifier, FileConfiguration defaultMessages) {
-        List<MissingKey> missingKeys = verifier.getMissingKeys();
-        if (!missingKeys.isEmpty() || !verifier.getMissingTags().isEmpty()) {
-            verifier.addMissingKeys(defaultMessages);
-            List<String> addedKeys = getMissingKeysWithAdded(missingKeys, true);
-            System.out.println("  Added missing keys " + addedKeys);
-
-            List<String> unsuccessfulKeys = getMissingKeysWithAdded(missingKeys, false);
-            if (!unsuccessfulKeys.isEmpty()) {
-                System.err.println("  Warning! Could not add all missing keys (problem with loading " +
-                    "default messages?)");
-                System.err.println("  Could not add keys " + unsuccessfulKeys);
-            }
-        }
-
-        Set<String> unknownKeys = verifier.getUnknownKeys();
-        if (!unknownKeys.isEmpty()) {
-            System.out.println("  Unknown keys: " + unknownKeys);
-        }
-
-        Multimap<String, String> missingTags = verifier.getMissingTags();
-        for (Map.Entry<String, String> entry : missingTags.entries()) {
-            System.out.println("  Missing tag '" + entry.getValue() + "' in entry with key '" + entry.getKey() + "'");
-        }
+    /**
+     * Updates a messages file to have the same order as the default file and to contain all
+     * failed verifications as comments.
+     *
+     * @param file the file to update
+     * @param verifier the verifier whose results should be used
+     * @param defaultFileElements default file elements to base the new file structure on
+     */
+    private static void updateMessagesFile(File file, MessageFileVerifier verifier,
+                                           List<MessageFileElement> defaultFileElements) {
+        List<MessageFileElement> messageFileElements = MessageFileElementReader.readFileIntoElements(file);
+        String newMessageFileContents = MessageFileElementMerger
+            .mergeElements(messageFileElements, defaultFileElements, verifier.getMissingTags().asMap())
+            .stream()
+            .map(MessageFileElement::getLines)
+            .flatMap(List::stream)
+            .collect(Collectors.joining("\n"));
+        FileIoUtils.writeToFile(file.toPath(), newMessageFileContents + "\n");
     }
 
-    private static List<String> getMissingKeysWithAdded(List<MissingKey> missingKeys, boolean wasAdded) {
-        return missingKeys.stream()
-            .filter(e -> e.getWasAdded() == wasAdded)
-            .map(MissingKey::getKey)
-            .collect(Collectors.toList());
-    }
 
     private static List<File> getMessagesFiles() {
         File[] files = listFilesOrThrow(new File(MESSAGES_FOLDER));