Reduce duplication in Log filter implementations

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

@@ -4,7 +4,7 @@ import java.util.logging.Filter;
 import java.util.logging.LogRecord;
 
 /**
- * Console filter Class
+ * Console filter to replace sensitive AuthMe commands with a generic message.
  *
  * @author Xephi59
  */
@@ -12,20 +12,15 @@ public class ConsoleFilter implements Filter {
 
     @Override
     public boolean isLoggable(LogRecord record) {
-        try {
+        if (record == null || record.getMessage() == null) {
-            if (record == null || record.getMessage() == null)
-                return true;
-            String logM = record.getMessage().toLowerCase();
-            if (!logM.contains("issued server command:"))
-                return true;
-            if (!logM.contains("/login ") && !logM.contains("/l ") && !logM.contains("/reg ") && !logM.contains("/changepassword ") && !logM.contains("/unregister ") && !logM.contains("/authme register ") && !logM.contains("/authme changepassword ") && !logM.contains("/authme reg ") && !logM.contains("/authme cp ") && !logM.contains("/register "))
-                return true;
-            String playerName = record.getMessage().split(" ")[0];
-            record.setMessage(playerName + " issued an AuthMe command!");
-            return true;
-        } catch (NullPointerException npe) {
             return true;
         }
+
+        if (LogFilterHelper.isSensitiveAuthMeCommand(record.getMessage())) {
+            String playerName = record.getMessage().split(" ")[0];
+            record.setMessage(playerName + " issued an AuthMe command");
+        }
+        return true;
     }
 
 }

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

@@ -1,8 +1,8 @@
 package fr.xephi.authme.output;
 
-import fr.xephi.authme.util.StringUtils;
 import org.apache.logging.log4j.Level;
 import org.apache.logging.log4j.Marker;
+import org.apache.logging.log4j.core.Filter;
 import org.apache.logging.log4j.core.LogEvent;
 import org.apache.logging.log4j.core.Logger;
 import org.apache.logging.log4j.message.Message;
@@ -12,14 +12,7 @@ import org.apache.logging.log4j.message.Message;
  *
  * @author Xephi59
  */
-public class Log4JFilter implements org.apache.logging.log4j.core.Filter {
+public class Log4JFilter implements Filter {
-
-    /**
-     * List of commands (lower-case) to skip.
-     */
-    private static final String[] COMMANDS_TO_SKIP = {"/login ", "/l ", "/reg ", "/changepassword ",
-        "/unregister ", "/authme register ", "/authme changepassword ", "/authme reg ", "/authme cp ",
-        "/register "};
 
     /**
      * Constructor.
@@ -28,13 +21,12 @@ public class Log4JFilter implements org.apache.logging.log4j.core.Filter {
     }
 
     /**
-     * Validates a Message instance and returns the {@link Result} value
+     * Validate a Message instance and return the {@link Result} value
-     * depending depending on whether the message contains sensitive AuthMe
+     * depending on whether the message contains sensitive AuthMe data.
-     * data.
      *
-     * @param message the Message object to verify
+     * @param message The Message object to verify
      *
-     * @return the Result value
+     * @return The Result value
      */
     private static Result validateMessage(Message message) {
         if (message == null) {
@@ -44,24 +36,17 @@ public class Log4JFilter implements org.apache.logging.log4j.core.Filter {
     }
 
     /**
-     * Validates a message and returns the {@link Result} value depending
+     * Validate a message and return the {@link Result} value depending
-     * depending on whether the message contains sensitive AuthMe data.
+     * on whether the message contains sensitive AuthMe data.
      *
-     * @param message the message to verify
+     * @param message The message to verify
      *
-     * @return the Result value
+     * @return The Result value
      */
     private static Result validateMessage(String message) {
-        if (message == null) {
+        return LogFilterHelper.isSensitiveAuthMeCommand(message)
-            return Result.NEUTRAL;
+            ? Result.DENY
-        }
+            : Result.NEUTRAL;
-
-        String lowerMessage = message.toLowerCase();
-        if (lowerMessage.contains("issued server command:")
-            && StringUtils.containsAny(lowerMessage, COMMANDS_TO_SKIP)) {
-            return Result.DENY;
-        }
-        return Result.NEUTRAL;
     }
 
     @Override
@@ -73,14 +58,12 @@ public class Log4JFilter implements org.apache.logging.log4j.core.Filter {
     }
 
     @Override
-    public Result filter(Logger arg0, Level arg1, Marker arg2, String message,
+    public Result filter(Logger arg0, Level arg1, Marker arg2, String message, Object... arg4) {
-                         Object... arg4) {
         return validateMessage(message);
     }
 
     @Override
-    public Result filter(Logger arg0, Level arg1, Marker arg2, Object message,
+    public Result filter(Logger arg0, Level arg1, Marker arg2, Object message, Throwable arg4) {
-                         Throwable arg4) {
         if (message == null) {
             return Result.NEUTRAL;
         }
@@ -88,8 +71,7 @@ public class Log4JFilter implements org.apache.logging.log4j.core.Filter {
     }
 
     @Override
-    public Result filter(Logger arg0, Level arg1, Marker arg2, Message message,
+    public Result filter(Logger arg0, Level arg1, Marker arg2, Message message, Throwable arg4) {
-                         Throwable arg4) {
         return validateMessage(message);
     }
 

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

@@ -0,0 +1,34 @@
+package fr.xephi.authme.output;
+
+import fr.xephi.authme.util.StringUtils;
+
+/**
+ * Service class for the log filters.
+ */
+public final class LogFilterHelper {
+
+    private static final String ISSUED_COMMAND_TEXT = "issued server command:";
+
+    private static final String[] COMMANDS_TO_SKIP = {"/login ", "/l ", "/reg ", "/changepassword ",
+        "/unregister ", "/authme register ", "/authme changepassword ", "/authme reg ", "/authme cp ",
+        "/register "};
+
+    private LogFilterHelper() {
+        // Util class
+    }
+
+    /**
+     * Validate a message and return whether the message contains a sensitive AuthMe command.
+     *
+     * @param message The message to verify
+     *
+     * @return True if it is a sensitive AuthMe command, false otherwise
+     */
+    public static boolean isSensitiveAuthMeCommand(String message) {
+        if (message == null) {
+            return false;
+        }
+        String lowerMessage = message.toLowerCase();
+        return lowerMessage.contains(ISSUED_COMMAND_TEXT) && StringUtils.containsAny(lowerMessage, COMMANDS_TO_SKIP);
+    }
+}

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

@@ -0,0 +1,77 @@
+package fr.xephi.authme.output;
+
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import java.util.logging.LogRecord;
+
+import static org.hamcrest.Matchers.equalTo;
+import static org.junit.Assert.assertThat;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+/**
+ * Test for {@link ConsoleFilter}.
+ */
+public class ConsoleFilterTest {
+
+    private final ConsoleFilter filter = new ConsoleFilter();
+
+    private static final String SENSITIVE_COMMAND = "User issued server command: /login test test";
+    private static final String NORMAL_COMMAND = "User issued server command: /motd 2";
+
+
+    @Test
+    public void shouldReplaceSensitiveRecord() {
+        // given
+        LogRecord record = createRecord(SENSITIVE_COMMAND);
+
+        // when
+        boolean result = filter.isLoggable(record);
+
+        // then
+        assertThat(result, equalTo(true));
+        verify(record).setMessage("User issued an AuthMe command");
+    }
+
+    @Test
+    public void shouldNotFilterRegularCommand() {
+        // given
+        LogRecord record = createRecord(NORMAL_COMMAND);
+
+        // when
+        boolean result = filter.isLoggable(record);
+
+        // then
+        assertThat(result, equalTo(true));
+        verify(record, never()).setMessage("User issued an AuthMe command");
+    }
+
+    @Test
+    public void shouldManageRecordWithNullMessage() {
+        // given
+        LogRecord record = createRecord(null);
+
+        // when
+        boolean result = filter.isLoggable(record);
+
+        // then
+        assertThat(result, equalTo(true));
+        verify(record, never()).setMessage("User issued an AuthMe command");
+    }
+
+
+    /**
+     * Creates a mock of {@link LogRecord} and sets it to return the given message.
+     *
+     * @param message The message to set.
+     *
+     * @return Mock of LogRecord
+     */
+    private static LogRecord createRecord(String message) {
+        LogRecord record = Mockito.mock(LogRecord.class);
+        when(record.getMessage()).thenReturn(message);
+        return record;
+    }
+}

src/test/java/fr/xephi/authme/permission/AdminPermissionTest.java

@@ -20,7 +20,8 @@ public class AdminPermissionTest {
         // when/then
         for (AdminPermission permission : AdminPermission.values()) {
             if (!permission.getNode().startsWith(requiredPrefix)) {
-                fail("The permission '" + permission + "' does not start with the required prefix '" + requiredPrefix + "'");
+                fail("The permission '" + permission + "' does not start with the required prefix '"
+                    + requiredPrefix + "'");
             }
         }
     }
@@ -33,7 +34,8 @@ public class AdminPermissionTest {
         // when/then
         for (AdminPermission permission : AdminPermission.values()) {
             if (!permission.getNode().contains(requiredBranch)) {
-                fail("The permission '" + permission + "' does not contain with the required branch '" + requiredBranch + "'");
+                fail("The permission '" + permission + "' does not contain with the required branch '"
+                    + requiredBranch + "'");
             }
         }
     }