diff --git a/pom.xml b/pom.xml index 255e5521..f67d6a1c 100644 --- a/pom.xml +++ b/pom.xml @@ -840,7 +840,7 @@ ch.jalu injector - 0.2 + 0.3 diff --git a/src/main/java/fr/xephi/authme/AuthMe.java b/src/main/java/fr/xephi/authme/AuthMe.java index 9d8af144..2412ead6 100644 --- a/src/main/java/fr/xephi/authme/AuthMe.java +++ b/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 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 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 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); diff --git a/src/main/java/fr/xephi/authme/initialization/Initializer.java b/src/main/java/fr/xephi/authme/initialization/Initializer.java index 5c5a8a1d..a6d5a58e 100644 --- a/src/main/java/fr/xephi/authme/initialization/Initializer.java +++ b/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)) { diff --git a/src/main/java/fr/xephi/authme/initialization/OnShutdownPlayerSaver.java b/src/main/java/fr/xephi/authme/initialization/OnShutdownPlayerSaver.java new file mode 100644 index 00000000..852f5380 --- /dev/null +++ b/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); + } + } + } +} diff --git a/src/main/java/fr/xephi/authme/initialization/TaskCloser.java b/src/main/java/fr/xephi/authme/initialization/TaskCloser.java new file mode 100644 index 00000000..d4119f92 --- /dev/null +++ b/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 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 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 getPendingTasks() { + List 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; + } +} diff --git a/src/test/java/fr/xephi/authme/security/PasswordSecurityTest.java b/src/test/java/fr/xephi/authme/security/PasswordSecurityTest.java index 064b27fa..2e6adff4 100644 --- a/src/test/java/fr/xephi/authme/security/PasswordSecurityTest.java +++ b/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);