Update injector version and move shutdown logic to separate classes

pom.xml

@@ -840,7 +840,7 @@
         <dependency>
             <groupId>ch.jalu</groupId>
             <artifactId>injector</artifactId>
-            <version>0.2</version>
+            <version>0.3</version>
         </dependency>
 
         <!-- String comparison library. Used for dynamic help system. -->

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

@@ -7,14 +7,13 @@ import fr.xephi.authme.api.API;
 import fr.xephi.authme.api.NewAPI;
 import fr.xephi.authme.cache.auth.PlayerAuth;
 import fr.xephi.authme.cache.auth.PlayerCache;
-import fr.xephi.authme.cache.backup.PlayerDataStorage;
-import fr.xephi.authme.cache.limbo.LimboCache;
 import fr.xephi.authme.command.CommandHandler;
 import fr.xephi.authme.datasource.DataSource;
-import fr.xephi.authme.hooks.PluginHooks;
 import fr.xephi.authme.initialization.DataFolder;
 import fr.xephi.authme.initialization.Initializer;
 import fr.xephi.authme.initialization.MetricsManager;
+import fr.xephi.authme.initialization.OnShutdownPlayerSaver;
+import fr.xephi.authme.initialization.TaskCloser;
 import fr.xephi.authme.listener.BlockListener;
 import fr.xephi.authme.listener.EntityListener;
 import fr.xephi.authme.listener.PlayerListener;
@@ -28,7 +27,6 @@ import fr.xephi.authme.permission.PermissionsSystemType;
 import fr.xephi.authme.process.Management;
 import fr.xephi.authme.security.crypts.SHA256;
 import fr.xephi.authme.settings.Settings;
-import fr.xephi.authme.settings.SpawnLoader;
 import fr.xephi.authme.settings.properties.PluginSettings;
 import fr.xephi.authme.settings.properties.RestrictionSettings;
 import fr.xephi.authme.settings.properties.SecuritySettings;
@@ -38,8 +36,6 @@ import fr.xephi.authme.util.BukkitService;
 import fr.xephi.authme.util.GeoLiteAPI;
 import fr.xephi.authme.util.MigrationService;
 import fr.xephi.authme.util.Utils;
-import fr.xephi.authme.util.ValidationService;
-import org.bukkit.Location;
 import org.bukkit.Server;
 import org.bukkit.command.Command;
 import org.bukkit.command.CommandSender;
@@ -49,15 +45,9 @@ import org.bukkit.plugin.PluginLoader;
 import org.bukkit.plugin.PluginManager;
 import org.bukkit.plugin.java.JavaPlugin;
 import org.bukkit.scheduler.BukkitScheduler;
-import org.bukkit.scheduler.BukkitWorker;
 
 import java.io.File;
-import java.util.ArrayList;
-import java.util.Collection;
 import java.util.Date;
-import java.util.Iterator;
-import java.util.List;
-import java.util.logging.Level;
 
 import static fr.xephi.authme.util.BukkitService.TICKS_PER_MINUTE;
 import static fr.xephi.authme.util.Utils.isClassLoaded;
@@ -76,17 +66,13 @@ public class AuthMe extends JavaPlugin {
     private static String pluginVersion = "N/D";
     private static String pluginBuildNumber = "Unknown";
 
-    /*
-     * Private instances
-     */
+    // Private instances
     private Management management;
     private CommandHandler commandHandler;
     private PermissionsManager permsMan;
     private Settings settings;
     private Messages messages;
     private DataSource database;
-    private PluginHooks pluginHooks;
-    private SpawnLoader spawnLoader;
     private BukkitService bukkitService;
     private Injector injector;
     private GeoLiteAPI geoLiteApi;
@@ -145,6 +131,7 @@ public class AuthMe extends JavaPlugin {
         } catch (Exception e) {
             ConsoleLogger.logException("Aborting initialization of AuthMe:", e);
             stopOrUnload();
+            return;
         }
 
         // Show settings warnings
@@ -247,8 +234,6 @@ public class AuthMe extends JavaPlugin {
         messages = injector.getSingleton(Messages.class);
         permsMan = injector.getSingleton(PermissionsManager.class);
         bukkitService = injector.getSingleton(BukkitService.class);
-        pluginHooks = injector.getSingleton(PluginHooks.class);
-        spawnLoader = injector.getSingleton(SpawnLoader.class);
         commandHandler = injector.getSingleton(CommandHandler.class);
         management = injector.getSingleton(Management.class);
         geoLiteApi = injector.getSingleton(GeoLiteAPI.class);
@@ -275,7 +260,9 @@ public class AuthMe extends JavaPlugin {
     }
 
     /**
-     * Register all event listeners.
+     * Registers all event listeners.
+     *
+     * @param injector the injector
      */
     protected void registerEventListeners(Injector injector) {
         // Get the plugin manager instance
@@ -334,16 +321,12 @@ public class AuthMe extends JavaPlugin {
 
     @Override
     public void onDisable() {
-        // Save player data
-        BukkitService bukkitService = injector.getIfAvailable(BukkitService.class);
-        LimboCache limboCache = injector.getIfAvailable(LimboCache.class);
-        ValidationService validationService = injector.getIfAvailable(ValidationService.class);
-
-        if (bukkitService != null && limboCache != null && validationService != null) {
-            Collection<? extends Player> players = bukkitService.getOnlinePlayers();
-            for (Player player : players) {
-                savePlayer(player, limboCache, validationService);
-            }
+        // onDisable is also called when we prematurely abort, so any field may be null
+        OnShutdownPlayerSaver onShutdownPlayerSaver = injector == null
+            ? null
+            : injector.createIfHasDependencies(OnShutdownPlayerSaver.class);
+        if (onShutdownPlayerSaver != null) {
+            onShutdownPlayerSaver.saveAllPlayers();
         }
 
         // Do backup on stop if enabled
@@ -351,93 +334,17 @@ public class AuthMe extends JavaPlugin {
             new PerformBackup(this, settings).doBackup(PerformBackup.BackupCause.STOP);
         }
 
-        new Thread(new Runnable() {
-            @Override
-            public void run() {
-                List<Integer> pendingTasks = new ArrayList<>();
-                //returns only the async takss
-                for (BukkitWorker pendingTask : getServer().getScheduler().getActiveWorkers()) {
-                    if (pendingTask.getOwner().equals(AuthMe.this)
-                        //it's not a peridic task
-                        && !getServer().getScheduler().isQueued(pendingTask.getTaskId())) {
-                        pendingTasks.add(pendingTask.getTaskId());
-                    }
-                }
-
-                getLogger().log(Level.INFO, "Waiting for {0} tasks to finish", pendingTasks.size());
-                int progress = 0;
-
-                //one minute + some time checking the running state
-                int tries = 60;
-                while (!pendingTasks.isEmpty()) {
-                    if (tries <= 0) {
-                        getLogger().log(Level.INFO, "Async tasks times out after to many tries {0}", pendingTasks);
-                        break;
-                    }
-
-                    try {
-                        Thread.sleep(1000);
-                    } catch (InterruptedException ignored) {
-                        Thread.currentThread().interrupt();
-                        break;
-                    }
-
-                    for (Iterator<Integer> iterator = pendingTasks.iterator(); iterator.hasNext(); ) {
-                        int taskId = iterator.next();
-                        if (!getServer().getScheduler().isCurrentlyRunning(taskId)) {
-                            iterator.remove();
-                            progress++;
-                            getLogger().log(Level.INFO, "Progress: {0} / {1}", new Object[]{progress, pendingTasks.size()});
-                        }
-                    }
-
-                    tries--;
-                }
-
-                if (database != null) {
-                    database.close();
-                }
-            }
-        }, "AuthMe-DataSource#close").start();
+        // Wait for tasks and close data source
+        new Thread(
+            new TaskCloser(this, database),
+            "AuthMe-DataSource#close"
+        ).start();
 
         // Disabled correctly
         ConsoleLogger.info("AuthMe " + this.getDescription().getVersion() + " disabled!");
         ConsoleLogger.close();
     }
 
-    // Save Player Data
-    private void savePlayer(Player player, LimboCache limboCache, ValidationService validationService) {
-        final String name = player.getName().toLowerCase();
-        if (safeIsNpc(player) || validationService.isUnrestricted(name)) {
-            return;
-        }
-        if (limboCache.hasPlayerData(name)) {
-            limboCache.restoreData(player);
-            limboCache.removeFromCache(player);
-        } else {
-            if (settings.getProperty(RestrictionSettings.SAVE_QUIT_LOCATION)) {
-                Location loc = spawnLoader.getPlayerLocationOrSpawn(player);
-                final PlayerAuth auth = PlayerAuth.builder()
-                    .name(player.getName().toLowerCase())
-                    .realName(player.getName())
-                    .location(loc).build();
-                database.updateQuitLoc(auth);
-            }
-            if (settings.getProperty(RestrictionSettings.TELEPORT_UNAUTHED_TO_SPAWN)
-                && !settings.getProperty(RestrictionSettings.NO_TELEPORT)) {
-                PlayerDataStorage playerDataStorage = injector.getIfAvailable(PlayerDataStorage.class);
-                if (playerDataStorage != null && !playerDataStorage.hasData(player)) {
-                    playerDataStorage.saveData(player);
-                }
-            }
-        }
-        playerCache.removePlayer(name);
-    }
-
-    private boolean safeIsNpc(Player player) {
-        return pluginHooks != null && pluginHooks.isNpc(player) || player.hasMetadata("NPC");
-    }
-
     public String replaceAllInfo(String message, Player player) {
         String playersOnline = Integer.toString(bukkitService.getOnlinePlayers().size());
         String ipAddress = Utils.getPlayerIp(player);

src/main/java/fr/xephi/authme/initialization/Initializer.java

@@ -37,7 +37,7 @@ import static fr.xephi.authme.settings.properties.EmailSettings.RECALL_PLAYERS;
 import static fr.xephi.authme.util.BukkitService.TICKS_PER_MINUTE;
 
 /**
- * Initializes the plugin's data source.
+ * Initializes various services.
  */
 public class Initializer {
 
@@ -70,6 +70,7 @@ public class Initializer {
     /**
      * Sets up the data source.
      *
+     * @param settings the settings
      * @throws ClassNotFoundException if no driver could be found for the datasource
      * @throws SQLException           when initialization of a SQL datasource failed
      * @throws IOException            if flat file cannot be read
@@ -119,6 +120,9 @@ public class Initializer {
 
     /**
      * Sets up the console filter if enabled.
+     *
+     * @param settings the settings
+     * @param logger the plugin logger
      */
     public void setupConsoleFilter(Settings settings, Logger logger) {
         if (!settings.getProperty(SecuritySettings.REMOVE_PASSWORD_FROM_CONSOLE)) {

src/main/java/fr/xephi/authme/initialization/OnShutdownPlayerSaver.java

@@ -0,0 +1,85 @@
+package fr.xephi.authme.initialization;
+
+import fr.xephi.authme.cache.auth.PlayerAuth;
+import fr.xephi.authme.cache.auth.PlayerCache;
+import fr.xephi.authme.cache.backup.PlayerDataStorage;
+import fr.xephi.authme.cache.limbo.LimboCache;
+import fr.xephi.authme.datasource.DataSource;
+import fr.xephi.authme.hooks.PluginHooks;
+import fr.xephi.authme.settings.Settings;
+import fr.xephi.authme.settings.SpawnLoader;
+import fr.xephi.authme.settings.properties.RestrictionSettings;
+import fr.xephi.authme.util.BukkitService;
+import fr.xephi.authme.util.ValidationService;
+import org.bukkit.Location;
+import org.bukkit.entity.Player;
+
+import javax.inject.Inject;
+
+/**
+ * Saves all players' data when the plugin shuts down.
+ */
+public class OnShutdownPlayerSaver {
+
+    @Inject
+    private BukkitService bukkitService;
+    @Inject
+    private Settings settings;
+    @Inject
+    private ValidationService validationService;
+    @Inject
+    private LimboCache limboCache;
+    @Inject
+    private DataSource dataSource;
+    @Inject
+    private PlayerDataStorage playerDataStorage;
+    @Inject
+    private SpawnLoader spawnLoader;
+    @Inject
+    private PluginHooks pluginHooks;
+    @Inject
+    private PlayerCache playerCache;
+
+    OnShutdownPlayerSaver() {
+    }
+
+    /**
+     * Saves the data of all online players.
+     */
+    public void saveAllPlayers() {
+        for (Player player : bukkitService.getOnlinePlayers()) {
+            savePlayer(player);
+        }
+    }
+
+    private void savePlayer(Player player) {
+        final String name = player.getName().toLowerCase();
+        if (pluginHooks.isNpc(player) || validationService.isUnrestricted(name)) {
+            return;
+        }
+        if (limboCache.hasPlayerData(name)) {
+            limboCache.restoreData(player);
+            limboCache.removeFromCache(player);
+        } else {
+            saveLoggedinPlayer(player);
+        }
+        playerCache.removePlayer(name);
+    }
+
+    private void saveLoggedinPlayer(Player player) {
+        if (settings.getProperty(RestrictionSettings.SAVE_QUIT_LOCATION)) {
+            Location loc = spawnLoader.getPlayerLocationOrSpawn(player);
+            final PlayerAuth auth = PlayerAuth.builder()
+                .name(player.getName().toLowerCase())
+                .realName(player.getName())
+                .location(loc).build();
+            dataSource.updateQuitLoc(auth);
+        }
+        if (settings.getProperty(RestrictionSettings.TELEPORT_UNAUTHED_TO_SPAWN)
+            && !settings.getProperty(RestrictionSettings.NO_TELEPORT)) {
+            if (!playerDataStorage.hasData(player)) {
+                playerDataStorage.saveData(player);
+            }
+        }
+    }
+}

src/main/java/fr/xephi/authme/initialization/TaskCloser.java

@@ -0,0 +1,88 @@
+package fr.xephi.authme.initialization;
+
+import fr.xephi.authme.AuthMe;
+import fr.xephi.authme.datasource.DataSource;
+import org.bukkit.scheduler.BukkitScheduler;
+import org.bukkit.scheduler.BukkitWorker;
+
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+/**
+ * Waits for asynchronous tasks to complete before closing the data source
+ * so the plugin can shut down properly.
+ */
+public class TaskCloser implements Runnable {
+
+    private final BukkitScheduler scheduler;
+    private final Logger logger;
+    private final AuthMe plugin;
+    private final DataSource dataSource;
+
+    /**
+     * Constructor.
+     *
+     * @param plugin the plugin instance
+     * @param dataSource the data source (nullable)
+     */
+    public TaskCloser(AuthMe plugin, DataSource dataSource) {
+        this.scheduler = plugin.getServer().getScheduler();
+        this.logger = plugin.getLogger();
+        this.plugin = plugin;
+        this.dataSource = dataSource;
+    }
+
+    @Override
+    public void run() {
+        List<Integer> pendingTasks = getPendingTasks();
+        logger.log(Level.INFO, "Waiting for {0} tasks to finish", pendingTasks.size());
+        int progress = 0;
+
+        //one minute + some time checking the running state
+        int tries = 60;
+        while (!pendingTasks.isEmpty()) {
+            if (tries <= 0) {
+                logger.log(Level.INFO, "Async tasks times out after to many tries {0}", pendingTasks);
+                break;
+            }
+
+            try {
+                Thread.sleep(1000);
+            } catch (InterruptedException ignored) {
+                Thread.currentThread().interrupt();
+                break;
+            }
+
+            for (Iterator<Integer> iterator = pendingTasks.iterator(); iterator.hasNext(); ) {
+                int taskId = iterator.next();
+                if (!scheduler.isCurrentlyRunning(taskId)) {
+                    iterator.remove();
+                    progress++;
+                    logger.log(Level.INFO, "Progress: {0} / {1}", new Object[]{progress, pendingTasks.size()});
+                }
+            }
+
+            tries--;
+        }
+
+        if (dataSource != null) {
+            dataSource.close();
+        }
+    }
+
+    private List<Integer> getPendingTasks() {
+        List<Integer> pendingTasks = new ArrayList<>();
+        //returns only the async tasks
+        for (BukkitWorker pendingTask : scheduler.getActiveWorkers()) {
+            if (pendingTask.getOwner().equals(plugin)
+                //it's not a periodic task
+                && !scheduler.isQueued(pendingTask.getTaskId())) {
+                pendingTasks.add(pendingTask.getTaskId());
+            }
+        }
+        return pendingTasks;
+    }
+}

src/test/java/fr/xephi/authme/security/PasswordSecurityTest.java

@@ -80,7 +80,7 @@ public class PasswordSecurityTest {
                 return null;
             }
         }).when(pluginManager).callEvent(any(Event.class));
-        injector = new InjectorBuilder().addDefaultHandlers("!! impossible package !!").create();
+        injector = new InjectorBuilder().addDefaultHandlers("fr.xephi.authme").create();
         injector.register(Settings.class, settings);
         injector.register(DataSource.class, dataSource);
         injector.register(PluginManager.class, pluginManager);
@@ -98,7 +98,7 @@ public class PasswordSecurityTest {
         given(dataSource.getPassword(playerName)).willReturn(password);
         given(method.comparePassword(clearTextPass, password, playerLowerCase)).willReturn(true);
         initSettings(HashAlgorithm.BCRYPT, false);
-        PasswordSecurity security = injector.newInstance(PasswordSecurity.class);
+        PasswordSecurity security = newPasswordSecurity();
 
         // when
         boolean result = security.comparePassword(clearTextPass, playerName);
@@ -121,7 +121,7 @@ public class PasswordSecurityTest {
         given(dataSource.getPassword(playerName)).willReturn(password);
         given(method.comparePassword(clearTextPass, password, playerLowerCase)).willReturn(false);
         initSettings(HashAlgorithm.CUSTOM, false);
-        PasswordSecurity security = injector.newInstance(PasswordSecurity.class);
+        PasswordSecurity security = newPasswordSecurity();
 
         // when
         boolean result = security.comparePassword(clearTextPass, playerName);
@@ -141,7 +141,7 @@ public class PasswordSecurityTest {
 
         given(dataSource.getPassword(playerName)).willReturn(null);
         initSettings(HashAlgorithm.MD5, false);
-        PasswordSecurity security = injector.newInstance(PasswordSecurity.class);
+        PasswordSecurity security = newPasswordSecurity();
 
         // when
         boolean result = security.comparePassword(clearTextPass, playerName);
@@ -169,7 +169,7 @@ public class PasswordSecurityTest {
         given(method.comparePassword(clearTextPass, password, playerLowerCase)).willReturn(false);
         given(method.computeHash(clearTextPass, playerLowerCase)).willReturn(newPassword);
         initSettings(HashAlgorithm.MD5, true);
-        PasswordSecurity security = injector.newInstance(PasswordSecurity.class);
+        PasswordSecurity security = newPasswordSecurity();
 
         // when
         boolean result = security.comparePassword(clearTextPass, playerName);
@@ -194,7 +194,7 @@ public class PasswordSecurityTest {
         given(dataSource.getPassword(playerName)).willReturn(password);
         given(method.comparePassword(clearTextPass, password, playerName)).willReturn(false);
         initSettings(HashAlgorithm.MD5, true);
-        PasswordSecurity security = injector.newInstance(PasswordSecurity.class);
+        PasswordSecurity security = newPasswordSecurity();
 
         // when
         boolean result = security.comparePassword(clearTextPass, playerName);
@@ -213,7 +213,7 @@ public class PasswordSecurityTest {
         HashedPassword hashedPassword = new HashedPassword("$T$est#Hash", "__someSalt__");
         given(method.computeHash(password, usernameLowerCase)).willReturn(hashedPassword);
         initSettings(HashAlgorithm.JOOMLA, true);
-        PasswordSecurity security = injector.newInstance(PasswordSecurity.class);
+        PasswordSecurity security = newPasswordSecurity();
 
         // when
         HashedPassword result = security.computeHash(password, username);
@@ -236,7 +236,7 @@ public class PasswordSecurityTest {
         given(method.computeHash(password, username)).willReturn(hashedPassword);
         given(method.hasSeparateSalt()).willReturn(true);
         initSettings(HashAlgorithm.XAUTH, false);
-        PasswordSecurity security = injector.newInstance(PasswordSecurity.class);
+        PasswordSecurity security = newPasswordSecurity();
 
         // when
         boolean result = security.comparePassword(password, hashedPassword, username);
@@ -252,7 +252,7 @@ public class PasswordSecurityTest {
     public void shouldReloadSettings() {
         // given
         initSettings(HashAlgorithm.BCRYPT, false);
-        PasswordSecurity passwordSecurity = injector.newInstance(PasswordSecurity.class);
+        PasswordSecurity passwordSecurity = newPasswordSecurity();
         given(settings.getProperty(SecuritySettings.PASSWORD_HASH)).willReturn(HashAlgorithm.MD5);
         given(settings.getProperty(SecuritySettings.SUPPORT_OLD_PASSWORD_HASH)).willReturn(true);
 
@@ -266,6 +266,16 @@ public class PasswordSecurityTest {
             equalTo((Object) Boolean.TRUE));
     }
 
+    private PasswordSecurity newPasswordSecurity() {
+        // Use this method to make sure we have all dependents of PasswordSecurity already registered as mocks
+        PasswordSecurity passwordSecurity = injector.createIfHasDependencies(PasswordSecurity.class);
+        if (passwordSecurity == null) {
+            throw new IllegalStateException("Cannot create PasswordSecurity directly! "
+                + "Did you forget to provide a dependency as mock?");
+        }
+        return passwordSecurity;
+    }
+
     private void initSettings(HashAlgorithm algorithm, boolean supportOldPassword) {
         given(settings.getProperty(SecuritySettings.PASSWORD_HASH)).willReturn(algorithm);
         given(settings.getProperty(SecuritySettings.SUPPORT_OLD_PASSWORD_HASH)).willReturn(supportOldPassword);