Cleanup (3/?)

src/main/java/fr/xephi/authme/ConsoleLogger.java

@@ -5,6 +5,9 @@ import fr.xephi.authme.settings.Settings;
 import fr.xephi.authme.settings.properties.PluginSettings;
 import fr.xephi.authme.settings.properties.SecuritySettings;
 import fr.xephi.authme.util.ExceptionUtils;
+import org.bukkit.ChatColor;
+import org.bukkit.command.CommandSender;
+import org.bukkit.command.ConsoleCommandSender;
 import org.jetbrains.annotations.NotNull;
 
 import java.io.Closeable;
@@ -109,11 +112,7 @@ public final class ConsoleLogger {
      * @param message The message to log
      */
     public void warning(@NotNull String message) {
-        if (logger != null) {
         logger.warning(message);
-        } else {
-            System.err.println("[WARN] " + message);
-        }
         writeLog("[WARN] " + message);
     }
 
@@ -135,11 +134,7 @@ public final class ConsoleLogger {
      * @param message The message to log
      */
     public void info(@NotNull String message) {
-        if (logger != null) {
         logger.info(message);
-        } else {
-            System.out.println("[INFO] " + message);
-        }
         writeLog("[INFO] " + message);
     }
 
@@ -153,15 +148,43 @@ public final class ConsoleLogger {
      */
     public void fine(@NotNull String message) {
         if (logLevel.includes(LogLevel.FINE)) {
-            if (logger != null) {
             logger.info(message);
-            } else {
-                System.err.println("[FINE] " + message);
-            }
             writeLog("[FINE] " + message);
         }
     }
 
+    /**
+     * Sends a message to the given sender (null safe), and logs the message to the console.
+     * This method is aware that the command sender might be the console sender and avoids
+     * displaying the message twice in this case.
+     *
+     * @param sender the sender to inform
+     * @param message the message to log and send
+     */
+    public void logAndSendMessage(CommandSender sender, @NotNull String message) {
+        info(message);
+        // Make sure sender is not console user, which will see the message from ConsoleLogger already
+        if (sender != null && !(sender instanceof ConsoleCommandSender)) {
+            sender.sendMessage(message);
+        }
+    }
+
+    /**
+     * Sends a warning to the given sender (null safe), and logs the warning to the console.
+     * This method is aware that the command sender might be the console sender and avoids
+     * displaying the message twice in this case.
+     *
+     * @param sender the sender to inform
+     * @param message the warning to log and send
+     */
+    public void logAndSendWarning(CommandSender sender, @NotNull String message) {
+        warning(message);
+        // Make sure sender is not console user, which will see the message from ConsoleLogger already
+        if (sender != null && !(sender instanceof ConsoleCommandSender)) {
+            sender.sendMessage(ChatColor.RED + message);
+        }
+    }
+
     // --------
     // Debug log methods
     // --------
@@ -232,11 +255,7 @@ public final class ConsoleLogger {
 
     private void logAndWriteWithDebugPrefix(@NotNull String message) {
         String debugMessage = "[DEBUG] " + message;
-        if (logger != null) {
         logger.info(debugMessage);
-        } else {
-            System.err.println(debugMessage);
-        }
         writeLog(debugMessage);
     }
 

src/main/java/fr/xephi/authme/command/executable/authme/PurgeCommand.java

@@ -2,6 +2,7 @@ package fr.xephi.authme.command.executable.authme;
 
 import fr.xephi.authme.command.ExecutableCommand;
 import fr.xephi.authme.task.purge.PurgeService;
+import fr.xephi.authme.util.Utils;
 import org.bukkit.ChatColor;
 import org.bukkit.command.CommandSender;
 
@@ -26,10 +27,8 @@ public class PurgeCommand implements ExecutableCommand {
         String daysStr = arguments.get(0);
 
         // Convert the days string to an integer value, and make sure it's valid
-        int days;
+        Integer days = Utils.tryInteger(daysStr);
-        try {
+        if (days == null) {
-            days = Integer.parseInt(daysStr);
-        } catch (NumberFormatException e) {
             sender.sendMessage(ChatColor.RED + "The value you've entered is invalid!");
             return;
         }

src/main/java/fr/xephi/authme/command/executable/authme/ReloadCommand.java

@@ -56,7 +56,7 @@ public class ReloadCommand implements ExecutableCommand {
 
             // We do not change database type for consistency issues, but we'll output a note in the logs
             if (!settings.getProperty(DatabaseSettings.BACKEND).equals(dataSource.getType())) {
-                Utils.logAndSendMessage(sender, "Note: cannot change database type during /authme reload");
+                logger.logAndSendMessage(sender, "Note: cannot change database type during /authme reload");
             }
             performReloadOnServices();
             commonService.send(sender, MessageKey.CONFIG_RELOAD_SUCCESS);
@@ -69,9 +69,10 @@ public class ReloadCommand implements ExecutableCommand {
 
     private void performReloadOnServices() {
         reloadableStore.retrieveAllOfType()
-            .forEach(r -> r.reload());
+            .forEach(Reloadable::reload);
 
         settingsDependentStore.retrieveAllOfType()
             .forEach(s -> s.reload(settings));
     }
+
 }

src/main/java/fr/xephi/authme/datasource/converter/AbstractDataSourceConverter.java

@@ -10,8 +10,6 @@ import org.bukkit.command.CommandSender;
 import java.util.ArrayList;
 import java.util.List;
 
-import static fr.xephi.authme.util.Utils.logAndSendMessage;
-
 /**
  * Converts from one AuthMe data source type to another.
  *
@@ -53,7 +51,7 @@ public abstract class AbstractDataSourceConverter<S extends DataSource> implemen
         try {
             source = getSource();
         } catch (Exception e) {
-            logAndSendMessage(sender, "The data source to convert from could not be initialized");
+            logger.logAndSendMessage(sender, "The data source to convert from could not be initialized");
             logger.logException("Could not initialize source:", e);
             return;
         }
@@ -70,10 +68,10 @@ public abstract class AbstractDataSourceConverter<S extends DataSource> implemen
         }
 
         if (!skippedPlayers.isEmpty()) {
-            logAndSendMessage(sender, "Skipped conversion for players which were already in "
+            logger.logAndSendMessage(sender, "Skipped conversion for players which were already in "
                 + destinationType + ": " + String.join(", ", skippedPlayers));
         }
-        logAndSendMessage(sender, "Database successfully converted from " + source.getType()
+        logger.logAndSendMessage(sender, "Database successfully converted from " + source.getType()
             + " to " + destinationType);
     }
 
@@ -82,4 +80,5 @@ public abstract class AbstractDataSourceConverter<S extends DataSource> implemen
      * @throws Exception during initialization of source
      */
     protected abstract S getSource() throws Exception;
+
 }

src/main/java/fr/xephi/authme/datasource/converter/LoginSecurityConverter.java

@@ -25,8 +25,6 @@ import java.util.List;
 import java.util.Optional;
 import java.util.UUID;
 
-import static fr.xephi.authme.util.Utils.logAndSendMessage;
-
 /**
  * Converts data from LoginSecurity to AuthMe.
  */
@@ -104,9 +102,9 @@ public class LoginSecurityConverter implements Converter {
             }
         }
 
-        logAndSendMessage(sender, "Migrated " + successfulSaves + " accounts successfully from LoginSecurity");
+        logger.logAndSendMessage(sender, "Migrated " + successfulSaves + " accounts successfully from LoginSecurity");
         if (!skippedPlayers.isEmpty()) {
-            logAndSendMessage(sender, "Skipped conversion for players which were already in AuthMe: "
+            logger.logAndSendMessage(sender, "Skipped conversion for players which were already in AuthMe: "
                 + String.join(", ", skippedPlayers));
         }
     }
@@ -205,4 +203,5 @@ public class LoginSecurityConverter implements Converter {
             return null;
         }
     }
+
 }

src/main/java/fr/xephi/authme/datasource/converter/RakamakConverter.java

@@ -88,7 +88,7 @@ public class RakamakConverter implements Converter {
                 database.saveAuth(auth);
                 database.updateSession(auth);
             }
-            Utils.logAndSendMessage(sender, "Rakamak database has been imported successfully");
+            logger.logAndSendMessage(sender, "Rakamak database has been imported successfully");
         } catch (IOException ex) {
             logger.logException("Can't open the rakamak database file! Does it exist?", ex);
         }

src/main/java/fr/xephi/authme/listener/OnJoinVerifier.java

@@ -61,7 +61,10 @@ public class OnJoinVerifier implements Reloadable {
     @Override
     public void reload() {
         String nickRegEx = settings.getProperty(RestrictionSettings.ALLOWED_NICKNAME_CHARACTERS);
-        nicknamePattern = Utils.safePatternCompile(nickRegEx);
+        nicknamePattern = Utils.safePatternCompile(nickRegEx, patternString -> {
+            logger.warning("Failed to compile pattern '" + patternString + "' - defaulting to allowing everything");
+            return Utils.MATCH_ANYTHING_PATTERN;
+        });
     }
 
     /**

src/main/java/fr/xephi/authme/security/crypts/Pbkdf2.java

@@ -9,6 +9,7 @@ import fr.xephi.authme.security.crypts.description.Recommendation;
 import fr.xephi.authme.security.crypts.description.Usage;
 import fr.xephi.authme.settings.Settings;
 import fr.xephi.authme.settings.properties.SecuritySettings;
+import fr.xephi.authme.util.Utils;
 
 import javax.inject.Inject;
 
@@ -41,10 +42,8 @@ public class Pbkdf2 extends HexSaltedMethod {
             return false;
         }
 
-        int iterations;
+        Integer iterations = Utils.tryInteger(line[1]);
-        try {
+        if (iterations == null) {
-            iterations = Integer.parseInt(line[1]);
-        } catch (NumberFormatException e) {
             logger.warning("Cannot read number of rounds for Pbkdf2: '" + line[1] + "'");
             return false;
         }

src/main/java/fr/xephi/authme/security/crypts/Pbkdf2Django.java

@@ -5,6 +5,7 @@ import de.rtner.security.auth.spi.PBKDF2Parameters;
 import fr.xephi.authme.ConsoleLogger;
 import fr.xephi.authme.output.ConsoleLoggerFactory;
 import fr.xephi.authme.security.crypts.description.AsciiRestricted;
+import fr.xephi.authme.util.Utils;
 
 import java.util.Base64;
 
@@ -30,10 +31,8 @@ public class Pbkdf2Django extends HexSaltedMethod {
             return false;
         }
 
-        int iterations;
+        Integer iterations = Utils.tryInteger(line[1]);
-        try {
+        if (iterations == null) {
-            iterations = Integer.parseInt(line[1]);
-        } catch (NumberFormatException e) {
             logger.warning("Cannot read number of rounds for Pbkdf2Django: '" + line[1] + "'");
             return false;
         }

src/main/java/fr/xephi/authme/security/crypts/TwoFactor.java

@@ -7,6 +7,7 @@ import fr.xephi.authme.security.crypts.description.HasSalt;
 import fr.xephi.authme.security.crypts.description.Recommendation;
 import fr.xephi.authme.security.crypts.description.SaltType;
 import fr.xephi.authme.security.crypts.description.Usage;
+import fr.xephi.authme.util.Utils;
 
 import javax.crypto.Mac;
 import javax.crypto.spec.SecretKeySpec;
@@ -88,10 +89,8 @@ public class TwoFactor extends UnsaltedMethod {
 
     private boolean checkPassword(String secretKey, String userInput)
             throws NoSuchAlgorithmException, InvalidKeyException {
-        int code;
+        Integer code = Utils.tryInteger(userInput);
-        try {
+        if (code == null) {
-            code = Integer.parseInt(userInput);
-        } catch (NumberFormatException e) {
             //code is not an integer
             return false;
         }

src/main/java/fr/xephi/authme/security/totp/TotpAuthenticator.java

@@ -10,6 +10,7 @@ import fr.xephi.authme.data.auth.PlayerAuth;
 import fr.xephi.authme.initialization.HasCleanup;
 import fr.xephi.authme.settings.Settings;
 import fr.xephi.authme.settings.properties.PluginSettings;
+import fr.xephi.authme.util.Utils;
 import org.bukkit.entity.Player;
 
 import javax.inject.Inject;
@@ -54,12 +55,7 @@ public class TotpAuthenticator implements HasCleanup {
      */
     public boolean checkCode(String playerName, String totpKey, String inputCode) {
         String nameLower = playerName.toLowerCase();
-        Integer totpCode;
+        Integer totpCode = Utils.tryInteger(inputCode);
-        try {
-            totpCode = Integer.parseInt(inputCode);
-        } catch (NumberFormatException e) {
-            totpCode = null;
-        }
         if (totpCode != null && !usedCodes.contains(nameLower, totpCode)
             && authenticator.authorize(totpKey, totpCode)) {
             usedCodes.put(nameLower, totpCode, System.currentTimeMillis());

src/main/java/fr/xephi/authme/service/BackupService.java

@@ -19,9 +19,6 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 
-import static fr.xephi.authme.util.Utils.logAndSendMessage;
-import static fr.xephi.authme.util.Utils.logAndSendWarning;
-
 /**
  * Performs a backup of the data source.
  */
@@ -66,7 +63,7 @@ public class BackupService {
         if (!settings.getProperty(BackupSettings.ENABLED)) {
             // Print a warning if the backup was requested via command or by another plugin
             if (cause == BackupCause.COMMAND || cause == BackupCause.OTHER) {
-                logAndSendWarning(sender,
+                logger.logAndSendWarning(sender,
                     "Can't perform a backup: disabled in configuration. Cause of the backup: " + cause.name());
             }
             return;
@@ -78,10 +75,10 @@ public class BackupService {
 
         // Do backup and check return value!
         if (doBackup()) {
-            logAndSendMessage(sender,
+            logger.logAndSendMessage(sender,
                 "A backup has been performed successfully. Cause of the backup: " + cause.name());
         } else {
-            logAndSendWarning(sender, "Error while performing a backup! Cause of the backup: " + cause.name());
+            logger.logAndSendWarning(sender, "Error while performing a backup! Cause of the backup: " + cause.name());
         }
     }
 

src/main/java/fr/xephi/authme/service/BukkitService.java

@@ -328,17 +328,16 @@ public class BukkitService implements SettingsDependent {
     }
 
     /**
-     * Returns an optional with a boolean indicating whether bungeecord is enabled or not if the
+     * Returns a boolean indicating whether bungeecord support is enabled or not.
-     * server implementation is Spigot. Otherwise returns an empty optional.
      *
-     * @return Optional with configuration value for Spigot, empty optional otherwise
+     * @return boolean with configuration value for Spigot
      */
-    public Optional<Boolean> isBungeeCordConfiguredForSpigot() {
+    public boolean isBungeeCordConfiguredForSpigot() {
         try {
             YamlConfiguration spigotConfig = Bukkit.spigot().getConfig();
-            return Optional.of(spigotConfig.getBoolean("settings.bungeecord"));
+            return spigotConfig.getBoolean("settings.bungeecord");
         } catch (NoSuchMethodError e) {
-            return Optional.empty();
+            return false;
         }
     }
 

src/main/java/fr/xephi/authme/service/ValidationService.java

@@ -56,7 +56,11 @@ public class ValidationService implements Reloadable {
     @PostConstruct
     @Override
     public void reload() {
-        passwordRegex = Utils.safePatternCompile(settings.getProperty(RestrictionSettings.ALLOWED_PASSWORD_REGEX));
+        passwordRegex = Utils.safePatternCompile(settings.getProperty(RestrictionSettings.ALLOWED_PASSWORD_REGEX),
+            patternString -> {
+                logger.warning("Failed to compile pattern '" + patternString + "' - defaulting to allowing everything");
+                return Utils.MATCH_ANYTHING_PATTERN;
+        });
         restrictedNames = settings.getProperty(RestrictionSettings.ENABLE_RESTRICTED_USERS)
             ? loadNameRestrictions(settings.getProperty(RestrictionSettings.RESTRICTED_USERS))
             : HashMultimap.create();

src/main/java/fr/xephi/authme/settings/SettingsWarner.java

@@ -13,7 +13,6 @@ import fr.xephi.authme.settings.properties.RestrictionSettings;
 import fr.xephi.authme.settings.properties.SecuritySettings;
 
 import javax.inject.Inject;
-import java.util.Optional;
 
 /**
  * Logs warning messages in cases where the configured values suggest a misconfiguration.
@@ -60,7 +59,7 @@ public class SettingsWarner {
         }
 
         // Warn if spigot.yml has settings.bungeecord set to true but config.yml has Hooks.bungeecord set to false
-        if (isTrue(bukkitService.isBungeeCordConfiguredForSpigot())
+        if (bukkitService.isBungeeCordConfiguredForSpigot()
             && !settings.getProperty(HooksSettings.BUNGEECORD)) {
             logger.warning("Note: Hooks.bungeecord is set to false but your server appears to be running in"
                 + " bungeecord mode (see your spigot.yml). In order to allow the datasource caching and the"
@@ -76,7 +75,4 @@ public class SettingsWarner {
         }
     }
 
-    private static boolean isTrue(Optional<Boolean> value) {
-        return value.isPresent() && value.get();
-    }
 }

src/main/java/fr/xephi/authme/settings/SpawnLoader.java

@@ -15,6 +15,7 @@ import org.bukkit.World;
 import org.bukkit.configuration.file.FileConfiguration;
 import org.bukkit.configuration.file.YamlConfiguration;
 import org.bukkit.entity.Player;
+import org.jetbrains.annotations.NotNull;
 
 import javax.inject.Inject;
 import java.io.File;
@@ -229,7 +230,7 @@ public class SpawnLoader implements Reloadable {
      *
      * @return True upon success, false otherwise
      */
-    private boolean isValidSpawnPoint(Location location) {
+    private boolean isValidSpawnPoint(@NotNull Location location) {
         return location.getX() != 0 || location.getY() != 0 || location.getZ() != 0;
     }
 
@@ -241,7 +242,7 @@ public class SpawnLoader implements Reloadable {
      *
      * @return True upon success, false otherwise
      */
-    private boolean setLocation(String prefix, Location location) {
+    private boolean setLocation(@NotNull String prefix, Location location) {
         if (location != null && location.getWorld() != null) {
             authMeConfiguration.set(prefix + ".world", location.getWorld().getName());
             authMeConfiguration.set(prefix + ".x", location.getX());
@@ -271,7 +272,7 @@ public class SpawnLoader implements Reloadable {
      *
      * @return location of the given player if alive, spawn location if dead.
      */
-    public Location getPlayerLocationOrSpawn(Player player) {
+    public Location getPlayerLocationOrSpawn(@NotNull Player player) {
         if (player.isOnline() && player.isDead()) {
             return getSpawnLocation(player);
         }
@@ -286,7 +287,8 @@ public class SpawnLoader implements Reloadable {
      *
      * @return Location corresponding to the values in the path
      */
-    private static Location getLocationFromConfiguration(FileConfiguration configuration, String pathPrefix) {
+    private static Location getLocationFromConfiguration(@NotNull FileConfiguration configuration,
+                                                         @NotNull String pathPrefix) {
         if (containsAllSpawnFields(configuration, pathPrefix)) {
             String prefix = pathPrefix + ".";
             String worldName = Objects.requireNonNull(configuration.getString(prefix + "world"));
@@ -307,7 +309,7 @@ public class SpawnLoader implements Reloadable {
      *
      * @return Location corresponding to the values in the path
      */
-    private static Location getLocationFromCmiConfiguration(FileConfiguration configuration) {
+    private static Location getLocationFromCmiConfiguration(@NotNull FileConfiguration configuration) {
         final String pathPrefix = "Spawn.Main";
         if (isLocationCompleteInCmiConfig(configuration, pathPrefix)) {
             String prefix = pathPrefix + ".";
@@ -331,7 +333,8 @@ public class SpawnLoader implements Reloadable {
      *
      * @return True if all spawn fields are present, false otherwise
      */
-    private static boolean containsAllSpawnFields(FileConfiguration configuration, String pathPrefix) {
+    private static boolean containsAllSpawnFields(@NotNull FileConfiguration configuration,
+                                                  @NotNull String pathPrefix) {
         String[] fields = {"world", "x", "y", "z", "yaw", "pitch"};
         for (String field : fields) {
             if (!configuration.contains(pathPrefix + "." + field)) {
@@ -349,7 +352,8 @@ public class SpawnLoader implements Reloadable {
      *
      * @return True if all spawn fields are present, false otherwise
      */
-    private static boolean isLocationCompleteInCmiConfig(FileConfiguration cmiConfiguration, String pathPrefix) {
+    private static boolean isLocationCompleteInCmiConfig(@NotNull FileConfiguration cmiConfiguration,
+                                                         @NotNull String pathPrefix) {
         String[] fields = {"World", "X", "Y", "Z", "Yaw", "Pitch"};
         for (String field : fields) {
             if (!cmiConfiguration.contains(pathPrefix + "." + field)) {
@@ -367,7 +371,7 @@ public class SpawnLoader implements Reloadable {
      *
      * @return The float
      */
-    private static float getFloat(FileConfiguration configuration, String path) {
+    private static float getFloat(@NotNull FileConfiguration configuration, @NotNull String path) {
         Object value = configuration.get(path);
         // This behavior is consistent with FileConfiguration#getDouble
         return (value instanceof Number) ? ((Number) value).floatValue() : 0;

src/main/java/fr/xephi/authme/settings/WelcomeMessageConfiguration.java

@@ -17,6 +17,7 @@ import org.bukkit.ChatColor;
 import org.bukkit.Server;
 import org.bukkit.entity.HumanEntity;
 import org.bukkit.entity.Player;
+import org.jetbrains.annotations.NotNull;
 
 import javax.annotation.PostConstruct;
 import javax.inject.Inject;
@@ -95,6 +96,7 @@ public class WelcomeMessageConfiguration implements Reloadable {
      * @param player the player for whom the welcome message should be prepared
      * @return the welcome message
      */
+    @NotNull
     public List<String> getWelcomeMessage(Player player) {
         return messageSupplier.getAdaptedMessages(player);
     }
@@ -104,7 +106,7 @@ public class WelcomeMessageConfiguration implements Reloadable {
      *
      * @param player the player for whom the welcome message should be prepared
      */
-    public void sendWelcomeMessage(Player player) {
+    public void sendWelcomeMessage(@NotNull Player player) {
         if (service.getProperty(RegistrationSettings.USE_WELCOME_MESSAGE)) {
             List<String> welcomeMessage = getWelcomeMessage(player);
             if (service.getProperty(RegistrationSettings.BROADCAST_WELCOME_MESSAGE)) {
@@ -118,6 +120,7 @@ public class WelcomeMessageConfiguration implements Reloadable {
     /**
      * @return the lines of the welcome message file
      */
+    @NotNull
     private List<String> readWelcomeFile() {
         if (!(service.getProperty(RegistrationSettings.USE_WELCOME_MESSAGE))) {
             return Collections.emptyList();

src/main/java/fr/xephi/authme/task/MessageTask.java

@@ -2,20 +2,23 @@ package fr.xephi.authme.task;
 
 import org.bukkit.entity.Player;
 import org.bukkit.scheduler.BukkitRunnable;
+import org.jetbrains.annotations.NotNull;
 
 /**
  * Message shown to a player in a regular interval as long as he is not logged in.
  */
 public class MessageTask extends BukkitRunnable {
 
+    @NotNull
     private final Player player;
+    @NotNull
     private final String[] message;
     private boolean isMuted;
 
     /*
      * Constructor.
      */
-    public MessageTask(Player player, String[] lines) {
+    public MessageTask(@NotNull Player player, @NotNull String[] lines) {
         this.player = player;
         this.message = lines;
         isMuted = false;

src/main/java/fr/xephi/authme/task/TimeoutTask.java

@@ -2,14 +2,18 @@ package fr.xephi.authme.task;
 
 import fr.xephi.authme.data.auth.PlayerCache;
 import org.bukkit.entity.Player;
+import org.jetbrains.annotations.NotNull;
 
 /**
  * Kicks a player if he hasn't logged in (scheduled to run after a configured delay).
  */
 public class TimeoutTask implements Runnable {
 
+    @NotNull
     private final Player player;
+    @NotNull
     private final String message;
+    @NotNull
     private final PlayerCache playerCache;
 
     /**
@@ -19,7 +23,7 @@ public class TimeoutTask implements Runnable {
      * @param message the kick message
      * @param playerCache player cache instance
      */
-    public TimeoutTask(Player player, String message, PlayerCache playerCache) {
+    public TimeoutTask(@NotNull Player player, @NotNull String message, @NotNull PlayerCache playerCache) {
         this.message = message;
         this.player = player;
         this.playerCache = playerCache;
@@ -31,4 +35,5 @@ public class TimeoutTask implements Runnable {
             player.kickPlayer(message);
         }
     }
+
 }

src/main/java/fr/xephi/authme/task/purge/PurgeExecutor.java

@@ -11,6 +11,7 @@ import fr.xephi.authme.settings.properties.PurgeSettings;
 import org.bukkit.ChatColor;
 import org.bukkit.OfflinePlayer;
 import org.bukkit.Server;
+import org.jetbrains.annotations.NotNull;
 
 import javax.inject.Inject;
 import java.io.File;
@@ -53,7 +54,7 @@ public class PurgeExecutor {
      * @param players the players to purge
      * @param names   names to purge
      */
-    public void executePurge(Collection<OfflinePlayer> players, Collection<String> names) {
+    public void executePurge(@NotNull Collection<OfflinePlayer> players, @NotNull Collection<String> names) {
         // Purge other data
         purgeFromAuthMe(names);
         purgeEssentials(players);
@@ -68,7 +69,7 @@ public class PurgeExecutor {
      *
      * @param cleared the players whose data should be cleared
      */
-    synchronized void purgeAntiXray(Collection<String> cleared) {
+    synchronized void purgeAntiXray(@NotNull Collection<String> cleared) {
         if (!settings.getProperty(PurgeSettings.REMOVE_ANTI_XRAY_FILE)) {
             return;
         }
@@ -96,7 +97,7 @@ public class PurgeExecutor {
      *
      * @param names the name of the accounts to delete
      */
-    synchronized void purgeFromAuthMe(Collection<String> names) {
+    synchronized void purgeFromAuthMe(@NotNull Collection<String> names) {
         dataSource.purgeRecords(names);
         //TODO ljacqu 20160717: We shouldn't output namedBanned.size() but the actual total that was deleted
         logger.info(ChatColor.GOLD + "Deleted " + names.size() + " user accounts");
@@ -107,7 +108,7 @@ public class PurgeExecutor {
      *
      * @param cleared the players whose data should be cleared
      */
-    synchronized void purgeLimitedCreative(Collection<String> cleared) {
+    synchronized void purgeLimitedCreative(@NotNull Collection<String> cleared) {
         if (!settings.getProperty(PurgeSettings.REMOVE_LIMITED_CREATIVE_INVENTORIES)) {
             return;
         }
@@ -152,7 +153,7 @@ public class PurgeExecutor {
      *
      * @param cleared list of players to clear
      */
-    synchronized void purgeDat(Collection<OfflinePlayer> cleared) {
+    synchronized void purgeDat(@NotNull Collection<OfflinePlayer> cleared) {
         if (!settings.getProperty(PurgeSettings.REMOVE_PLAYER_DAT)) {
             return;
         }
@@ -176,7 +177,7 @@ public class PurgeExecutor {
      *
      * @param cleared list of players to clear
      */
-    synchronized void purgeEssentials(Collection<OfflinePlayer> cleared) {
+    synchronized void purgeEssentials(@NotNull Collection<OfflinePlayer> cleared) {
         if (!settings.getProperty(PurgeSettings.REMOVE_ESSENTIALS_FILES)) {
             return;
         }
@@ -208,7 +209,7 @@ public class PurgeExecutor {
      *
      * @param cleared the players to remove data for
      */
-    synchronized void purgePermissions(Collection<OfflinePlayer> cleared) {
+    synchronized void purgePermissions(@NotNull Collection<OfflinePlayer> cleared) {
         if (!settings.getProperty(PurgeSettings.REMOVE_PERMISSIONS)) {
             return;
         }
@@ -223,4 +224,5 @@ public class PurgeExecutor {
 
         logger.info("AutoPurge: Removed permissions from " + cleared.size() + " player(s).");
     }
+
 }

src/main/java/fr/xephi/authme/task/purge/PurgeService.java

@@ -10,14 +10,13 @@ import fr.xephi.authme.settings.properties.PurgeSettings;
 import fr.xephi.authme.util.Utils;
 import org.bukkit.OfflinePlayer;
 import org.bukkit.command.CommandSender;
+import org.jetbrains.annotations.NotNull;
 
 import javax.inject.Inject;
 import java.util.Calendar;
 import java.util.Collection;
 import java.util.Set;
 
-import static fr.xephi.authme.util.Utils.logAndSendMessage;
-
 /**
  * Initiates purge tasks.
  */
@@ -77,7 +76,7 @@ public class PurgeService {
         //todo: note this should may run async because it may executes a SQL-Query
         Set<String> toPurge = dataSource.getRecordsToPurge(until);
         if (Utils.isCollectionEmpty(toPurge)) {
-            logAndSendMessage(sender, "No players to purge");
+            logger.logAndSendMessage(sender, "No players to purge");
             return;
         }
 
@@ -91,9 +90,9 @@ public class PurgeService {
      * @param names The names to remove
      * @param players Collection of OfflinePlayers (including those with the given names)
      */
-    public void purgePlayers(CommandSender sender, Set<String> names, OfflinePlayer[] players) {
+    public void purgePlayers(CommandSender sender, @NotNull Set<String> names, @NotNull OfflinePlayer[] players) {
         if (isPurging) {
-            logAndSendMessage(sender, "Purge is already in progress! Aborting purge request");
+            logger.logAndSendMessage(sender, "Purge is already in progress! Aborting purge request");
             return;
         }
 
@@ -117,7 +116,8 @@ public class PurgeService {
      * @param players the players (associated with the names)
      * @param names the lowercase names
      */
-    void executePurge(Collection<OfflinePlayer> players, Collection<String> names) {
+    void executePurge(@NotNull Collection<OfflinePlayer> players, @NotNull Collection<String> names) {
         purgeExecutor.executePurge(players, names);
     }
+
 }

src/main/java/fr/xephi/authme/task/purge/PurgeTask.java

@@ -10,6 +10,7 @@ import org.bukkit.OfflinePlayer;
 import org.bukkit.command.CommandSender;
 import org.bukkit.entity.Player;
 import org.bukkit.scheduler.BukkitRunnable;
+import org.jetbrains.annotations.NotNull;
 
 import java.util.HashSet;
 import java.util.Set;
@@ -21,11 +22,16 @@ class PurgeTask extends BukkitRunnable {
     private static final int INTERVAL_CHECK = 5;
 
     private final ConsoleLogger logger = ConsoleLoggerFactory.get(PurgeTask.class);
+
+    @NotNull
     private final PurgeService purgeService;
+    @NotNull
     private final PermissionsManager permissionsManager;
     private final UUID sender;
+    @NotNull
     private final Set<String> toPurge;
 
+    @NotNull
     private final OfflinePlayer[] offlinePlayers;
     private final int totalPurgeCount;
 
@@ -40,8 +46,8 @@ class PurgeTask extends BukkitRunnable {
      * @param toPurge lowercase names to purge
      * @param offlinePlayers offline players to map to the names
      */
-    PurgeTask(PurgeService service, PermissionsManager permissionsManager, CommandSender sender,
+    PurgeTask(@NotNull PurgeService service, @NotNull PermissionsManager permissionsManager, CommandSender sender,
-              Set<String> toPurge, OfflinePlayer[] offlinePlayers) {
+              @NotNull Set<String> toPurge, OfflinePlayer[] offlinePlayers) {
         this.purgeService = service;
         this.permissionsManager = permissionsManager;
 
@@ -116,7 +122,7 @@ class PurgeTask extends BukkitRunnable {
         purgeService.setPurging(false);
     }
 
-    private void sendMessage(String message) {
+    private void sendMessage(@NotNull String message) {
         if (sender == null) {
             Bukkit.getConsoleSender().sendMessage(message);
         } else {
@@ -126,4 +132,5 @@ class PurgeTask extends BukkitRunnable {
             }
         }
     }
+
 }

src/main/java/fr/xephi/authme/util/Utils.java

@@ -8,6 +8,8 @@ import org.bukkit.command.ConsoleCommandSender;
 import org.jetbrains.annotations.NotNull;
 
 import java.util.Collection;
+import java.util.function.Function;
+import java.util.function.Supplier;
 import java.util.regex.Pattern;
 
 /**
@@ -18,7 +20,8 @@ public final class Utils {
     /** Number of milliseconds in a minute. */
     public static final long MILLIS_PER_MINUTE = 60_000L;
 
-    private static final ConsoleLogger logger = ConsoleLoggerFactory.get(Utils.class);
+    /** A pattern that matches anything. */
+    public static final Pattern MATCH_ANYTHING_PATTERN = Pattern.compile(".*?");
 
     // Utility class
     private Utils() {
@@ -28,16 +31,15 @@ public final class Utils {
      * Compile Pattern sneaky without throwing Exception.
      *
      * @param pattern  pattern string to compile
+     * @param fallback the fallback pattern supplier
      *
      * @return the given regex compiled into Pattern object.
      */
-    @NotNull
+    public static Pattern safePatternCompile(@NotNull String pattern, @NotNull Function<String, Pattern> fallback) {
-    public static Pattern safePatternCompile(@NotNull String pattern) {
         try {
             return Pattern.compile(pattern);
         } catch (Exception e) {
-            logger.warning("Failed to compile pattern '" + pattern + "' - defaulting to allowing everything");
+            return fallback.apply(pattern);
-            return Pattern.compile(".*?");
         }
     }
 
@@ -57,38 +59,6 @@ public final class Utils {
         }
     }
 
-    /**
-     * Sends a message to the given sender (null safe), and logs the message to the console.
-     * This method is aware that the command sender might be the console sender and avoids
-     * displaying the message twice in this case.
-     *
-     * @param sender the sender to inform
-     * @param message the message to log and send
-     */
-    public static void logAndSendMessage(CommandSender sender, @NotNull String message) {
-        logger.info(message);
-        // Make sure sender is not console user, which will see the message from ConsoleLogger already
-        if (sender != null && !(sender instanceof ConsoleCommandSender)) {
-            sender.sendMessage(message);
-        }
-    }
-
-    /**
-     * Sends a warning to the given sender (null safe), and logs the warning to the console.
-     * This method is aware that the command sender might be the console sender and avoids
-     * displaying the message twice in this case.
-     *
-     * @param sender the sender to inform
-     * @param message the warning to log and send
-     */
-    public static void logAndSendWarning(CommandSender sender, @NotNull String message) {
-        logger.warning(message);
-        // Make sure sender is not console user, which will see the message from ConsoleLogger already
-        if (sender != null && !(sender instanceof ConsoleCommandSender)) {
-            sender.sendMessage(ChatColor.RED + message);
-        }
-    }
-
     /**
      * Null-safe way to check whether a collection is empty or not.
      *
@@ -110,4 +80,17 @@ public final class Utils {
         return StringUtils.isEmpty(email) || "your@email.com".equalsIgnoreCase(email);
     }
 
+    /**
+     * Tries to parse a String as an Integer, returns null on fail.
+     *
+     * @return the parsed Integer value
+     */
+    public static Integer tryInteger(@NotNull String string) {
+        try {
+            return Integer.parseInt(string);
+        } catch (NumberFormatException e) {
+            return null;
+        }
+    }
+
 }

src/test/java/fr/xephi/authme/ClassesConsistencyTest.java

@@ -29,6 +29,7 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
 import static org.hamcrest.Matchers.equalTo;
@@ -52,7 +53,7 @@ public class ClassesConsistencyTest {
     private static final Set<Class<?>> IMMUTABLE_TYPES = ImmutableSet.of(
         /* JDK */
         int.class, long.class, float.class, String.class, File.class, Enum.class, collectionsUnmodifiableList(),
-        Charset.class,
+        Charset.class, Pattern.class,
         /* AuthMe */
         Property.class, RegistrationMethod.class, DataSourceColumn.class, PlayerAuthColumn.class,
         /* Guava */

src/test/java/fr/xephi/authme/ConsoleLoggerTest.java

@@ -4,6 +4,10 @@ import fr.xephi.authme.output.LogLevel;
 import fr.xephi.authme.settings.Settings;
 import fr.xephi.authme.settings.properties.PluginSettings;
 import fr.xephi.authme.settings.properties.SecuritySettings;
+import org.bukkit.ChatColor;
+import org.bukkit.command.CommandSender;
+import org.bukkit.command.ConsoleCommandSender;
+import org.bukkit.entity.Player;
 import org.junit.After;
 import org.junit.AfterClass;
 import org.junit.Before;
@@ -35,6 +39,7 @@ import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 
 /**
@@ -145,6 +150,7 @@ public class ConsoleLoggerTest {
         assertThat(String.join("", loggedLines), containsString(getClass().getCanonicalName()));
     }
 
+    @SuppressWarnings("unchecked")
     @Test
     public void shouldSupportVariousDebugMethods() throws IOException {
         // given
@@ -205,6 +211,106 @@ public class ConsoleLoggerTest {
         assertThat(ReflectionTestUtils.getFieldValue(ConsoleLogger.class, null, "fileWriter"), nullValue());
     }
 
+    @Test
+    public void shouldLogAndSendMessage() {
+        // given
+        Settings settings = newSettings(false, LogLevel.INFO);
+        ConsoleLogger.initializeSharedSettings(settings);
+        consoleLogger.initializeSettings(settings);
+        Player player = mock(Player.class);
+        String message = "Finished adding foo to the bar";
+
+        // when
+        consoleLogger.logAndSendMessage(player, message);
+
+        // then
+        verify(logger).info(message);
+        verify(player).sendMessage(message);
+    }
+
+    @Test
+    public void shouldHandleNullAsCommandSender() {
+        // given
+        Settings settings = newSettings(false, LogLevel.INFO);
+        ConsoleLogger.initializeSharedSettings(settings);
+        consoleLogger.initializeSettings(settings);
+        String message = "Test test, test.";
+
+        // when
+        consoleLogger.logAndSendMessage(null, message);
+
+        // then
+        verify(logger).info(message);
+    }
+
+    @Test
+    public void shouldNotSendToCommandSenderTwice() {
+        // given
+        Settings settings = newSettings(false, LogLevel.INFO);
+        ConsoleLogger.initializeSharedSettings(settings);
+        consoleLogger.initializeSettings(settings);
+        CommandSender sender = mock(ConsoleCommandSender.class);
+        String message = "Test test, test.";
+
+        // when
+        consoleLogger.logAndSendMessage(sender, message);
+
+        // then
+        verify(logger).info(message);
+        verifyNoInteractions(sender);
+    }
+
+    @Test
+    public void shouldLogAndSendWarning() {
+        // given
+        Settings settings = newSettings(false, LogLevel.INFO);
+        ConsoleLogger.initializeSharedSettings(settings);
+        consoleLogger.initializeSettings(settings);
+        String message = "Error while performing action";
+        CommandSender sender = mock(CommandSender.class);
+
+        // when
+        consoleLogger.logAndSendWarning(sender, message);
+
+        // then
+        verify(logger).warning(message);
+        verify(sender).sendMessage(ChatColor.RED + message);
+    }
+
+    @Test
+    public void shouldLogWarningAndNotSendToConsoleSender() {
+        // given
+        Settings settings = newSettings(false, LogLevel.INFO);
+        ConsoleLogger.initializeSharedSettings(settings);
+        consoleLogger.initializeSettings(settings);
+        String message = "Error while performing action";
+        CommandSender sender = mock(ConsoleCommandSender.class);
+
+        // when
+        consoleLogger.logAndSendWarning(sender, message);
+
+        // then
+        verify(logger).warning(message);
+        verifyNoInteractions(sender);
+    }
+
+    @SuppressWarnings("ConstantConditions")
+    @Test
+    public void shouldLogWarningAndHandleNullCommandSender() {
+        // given
+        Settings settings = newSettings(false, LogLevel.INFO);
+        ConsoleLogger.initializeSharedSettings(settings);
+        consoleLogger.initializeSettings(settings);
+        String message = "Error while performing action";
+        CommandSender sender = null;
+
+        // when
+        consoleLogger.logAndSendWarning(sender, message);
+
+        // then
+        verify(logger).warning(message);
+    }
+
     private static Settings newSettings(boolean logToFile, LogLevel logLevel) {
         Settings settings = mock(Settings.class);
         given(settings.getProperty(SecuritySettings.USE_LOGGING)).willReturn(logToFile);

src/test/java/fr/xephi/authme/settings/SettingsWarnerTest.java

@@ -53,7 +53,7 @@ public class SettingsWarnerTest {
         given(settings.getProperty(PluginSettings.SESSIONS_TIMEOUT)).willReturn(-5);
         given(settings.getProperty(SecuritySettings.PASSWORD_HASH)).willReturn(HashAlgorithm.BCRYPT);
         given(settings.getProperty(HooksSettings.BUNGEECORD)).willReturn(false);
-        given(bukkitService.isBungeeCordConfiguredForSpigot()).willReturn(Optional.of(true));
+        given(bukkitService.isBungeeCordConfiguredForSpigot()).willReturn(true);
 
         // when
         settingsWarner.logWarningsForMisconfigurations();
@@ -70,7 +70,7 @@ public class SettingsWarnerTest {
         given(settings.getProperty(EmailSettings.PORT25_USE_TLS)).willReturn(false);
         given(settings.getProperty(PluginSettings.SESSIONS_ENABLED)).willReturn(false);
         given(settings.getProperty(SecuritySettings.PASSWORD_HASH)).willReturn(HashAlgorithm.MD5);
-        given(bukkitService.isBungeeCordConfiguredForSpigot()).willReturn(Optional.empty());
+        given(bukkitService.isBungeeCordConfiguredForSpigot()).willReturn(false);
 
         // when
         settingsWarner.logWarningsForMisconfigurations();

src/test/java/fr/xephi/authme/util/UtilsTest.java

@@ -37,7 +37,7 @@ public class UtilsTest {
         String pattern = "gr(a|e)ys?";
 
         // when
-        Pattern result = Utils.safePatternCompile(pattern);
+        Pattern result = Utils.safePatternCompile(pattern, patternString -> null);
 
         // then
         assertThat(result.toString(), equalTo(pattern));
@@ -49,55 +49,12 @@ public class UtilsTest {
         String invalidPattern = "gr(a|eys?"; // missing closing ')'
 
         // when
-        Pattern result = Utils.safePatternCompile(invalidPattern);
+        Pattern result = Utils.safePatternCompile(invalidPattern, patternString -> Utils.MATCH_ANYTHING_PATTERN);
 
         // then
         assertThat(result.toString(), equalTo(".*?"));
     }
 
-    @Test
-    public void shouldLogAndSendMessage() {
-        // given
-        Logger logger = TestHelper.setupLogger();
-        Player player = mock(Player.class);
-        String message = "Finished adding foo to the bar";
-
-        // when
-        Utils.logAndSendMessage(player, message);
-
-        // then
-        verify(logger).info(message);
-        verify(player).sendMessage(message);
-    }
-
-    @Test
-    public void shouldHandleNullAsCommandSender() {
-        // given
-        Logger logger = TestHelper.setupLogger();
-        String message = "Test test, test.";
-
-        // when
-        Utils.logAndSendMessage(null, message);
-
-        // then
-        verify(logger).info(message);
-    }
-
-    @Test
-    public void shouldNotSendToCommandSenderTwice() {
-        // given
-        Logger logger = TestHelper.setupLogger();
-        CommandSender sender = mock(ConsoleCommandSender.class);
-        String message = "Test test, test.";
-
-        // when
-        Utils.logAndSendMessage(sender, message);
-
-        // then
-        verify(logger).info(message);
-        verifyNoInteractions(sender);
-    }
-
     @SuppressWarnings("ConstantConditions")
     @Test
     public void shouldCheckIfCollectionIsEmpty() {
@@ -111,51 +68,6 @@ public class UtilsTest {
         assertThat(Utils.isCollectionEmpty(null), equalTo(true));
     }
 
-    @Test
-    public void shouldLogAndSendWarning() {
-        // given
-        Logger logger = TestHelper.setupLogger();
-        String message = "Error while performing action";
-        CommandSender sender = mock(CommandSender.class);
-
-        // when
-        Utils.logAndSendWarning(sender, message);
-
-        // then
-        verify(logger).warning(message);
-        verify(sender).sendMessage(ChatColor.RED + message);
-    }
-
-    @Test
-    public void shouldLogWarningAndNotSendToConsoleSender() {
-        // given
-        Logger logger = TestHelper.setupLogger();
-        String message = "Error while performing action";
-        CommandSender sender = mock(ConsoleCommandSender.class);
-
-        // when
-        Utils.logAndSendWarning(sender, message);
-
-        // then
-        verify(logger).warning(message);
-        verifyNoInteractions(sender);
-    }
-
-    @SuppressWarnings("ConstantConditions")
-    @Test
-    public void shouldLogWarningAndHandleNullCommandSender() {
-        // given
-        Logger logger = TestHelper.setupLogger();
-        String message = "Error while performing action";
-        CommandSender sender = null;
-
-        // when
-        Utils.logAndSendWarning(sender, message);
-
-        // then
-        verify(logger).warning(message);
-    }
-
     @Test
     public void shouldCheckIfClassIsLoaded() {
         // given / when / then
@@ -174,4 +86,5 @@ public class UtilsTest {
         assertThat(Utils.isEmailEmpty("my@example.org"), equalTo(false));
         assertThat(Utils.isEmailEmpty("hey"), equalTo(false));
     }
+
 }

src/test/java/tools/dependencygraph/DrawDependency.java

@@ -52,7 +52,7 @@ public class DrawDependency implements ToolTask {
 
     private boolean mapToSupertype;
     // Map with the graph's nodes: value is one of the key's dependencies
-    private Multimap<Class<?>, String> foundDependencies = HashMultimap.create();
+    private final Multimap<Class<?>, String> foundDependencies = HashMultimap.create();
 
     @Override
     public String getTaskName() {
@@ -75,7 +75,7 @@ public class DrawDependency implements ToolTask {
         System.out.println("Specify the number of times to do this: [0=keep all]");
         int stripVerticesCount;
         try {
-            stripVerticesCount = Integer.valueOf(scanner.nextLine());
+            stripVerticesCount = Integer.parseInt(scanner.nextLine());
         } catch (NumberFormatException e) {
             stripVerticesCount = 0;
         }
@@ -87,9 +87,9 @@ public class DrawDependency implements ToolTask {
 
         // Create dot file content
         final String pattern = "\t\"%s\" -> \"%s\";";
-        String dotFile = "";
+        StringBuilder dotFile = new StringBuilder();
         for (Map.Entry<Class<?>, String> entry : foundDependencies.entries()) {
-            dotFile += "\n" + String.format(pattern, entry.getValue(), entry.getKey().getSimpleName());
+            dotFile.append("\n").append(String.format(pattern, entry.getValue(), entry.getKey().getSimpleName()));
         }
 
         // Write dot file