From 7788ad62302fc9f7d62da7af1fa6be627aacb0e1 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sun, 3 Jul 2016 19:53:44 +0200 Subject: [PATCH] #765 Replace Utils usages with TeleportService + misc householding - Remove some legacy settings - Move Utils "addNormal" method to AuthGroupHandler (Reapplied changes from orphaned fe29089) --- src/main/java/fr/xephi/authme/AuthMe.java | 8 +-- .../authme/UnregisterAdminCommand.java | 7 ++- .../authme/permission/AuthGroupHandler.java | 24 +++++++++ .../process/logout/AsynchronousLogout.java | 1 + .../authme/process/quit/AsynchronousQuit.java | 6 ++- .../quit/ProcessSyncronousPlayerQuit.java | 6 ++- .../register/ProcessSyncPasswordRegister.java | 6 ++- .../unregister/AsynchronousUnregister.java | 7 ++- .../security/crypts/EncryptionMethod.java | 4 +- .../fr/xephi/authme/settings/Settings.java | 1 - .../fr/xephi/authme/util/BukkitService.java | 17 +++++++ src/main/java/fr/xephi/authme/util/Utils.java | 49 ------------------- .../java/fr/xephi/authme/util/UtilsTest.java | 6 +++ 13 files changed, 78 insertions(+), 64 deletions(-) diff --git a/src/main/java/fr/xephi/authme/AuthMe.java b/src/main/java/fr/xephi/authme/AuthMe.java index 744f2211..bb1a42e1 100644 --- a/src/main/java/fr/xephi/authme/AuthMe.java +++ b/src/main/java/fr/xephi/authme/AuthMe.java @@ -30,6 +30,7 @@ import fr.xephi.authme.output.ConsoleFilter; import fr.xephi.authme.output.Log4JFilter; import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.output.Messages; +import fr.xephi.authme.permission.AuthGroupHandler; import fr.xephi.authme.permission.PermissionsManager; import fr.xephi.authme.permission.PermissionsSystemType; import fr.xephi.authme.process.Management; @@ -422,11 +423,12 @@ public class AuthMe extends JavaPlugin { // Save player data BukkitService bukkitService = initializer.getIfAvailable(BukkitService.class); LimboCache limboCache = initializer.getIfAvailable(LimboCache.class); + AuthGroupHandler authGroupHandler = initializer.getIfAvailable(AuthGroupHandler.class); if (bukkitService != null && limboCache != null) { Collection players = bukkitService.getOnlinePlayers(); for (Player player : players) { - savePlayer(player, limboCache); + savePlayer(player, limboCache, authGroupHandler); } } @@ -559,7 +561,7 @@ public class AuthMe extends JavaPlugin { } // Save Player Data - private void savePlayer(Player player, LimboCache limboCache) { + private void savePlayer(Player player, LimboCache limboCache, AuthGroupHandler authGroupHandler) { if (safeIsNpc(player) || Utils.isUnrestricted(player)) { return; } @@ -569,7 +571,7 @@ public class AuthMe extends JavaPlugin { if (!newSettings.getProperty(RestrictionSettings.NO_TELEPORT)) { player.teleport(limbo.getLoc()); } - Utils.addNormal(player, limbo.getGroup()); + authGroupHandler.addNormal(player, limbo.getGroup()); player.setOp(limbo.isOperator()); player.setAllowFlight(limbo.isCanFly()); player.setWalkSpeed(limbo.getWalkSpeed()); diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/UnregisterAdminCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/UnregisterAdminCommand.java index 2e746f0d..89f714ef 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/UnregisterAdminCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/UnregisterAdminCommand.java @@ -13,6 +13,7 @@ import fr.xephi.authme.settings.properties.RegistrationSettings; import fr.xephi.authme.settings.properties.RestrictionSettings; import fr.xephi.authme.task.LimboPlayerTaskManager; import fr.xephi.authme.util.BukkitService; +import fr.xephi.authme.util.TeleportationService; import fr.xephi.authme.util.Utils; import org.bukkit.command.CommandSender; import org.bukkit.entity.Player; @@ -50,6 +51,9 @@ public class UnregisterAdminCommand implements ExecutableCommand { @Inject private AuthGroupHandler authGroupHandler; + @Inject + private TeleportationService teleportationService; + @Override public void executeCommand(final CommandSender sender, List arguments) { @@ -93,8 +97,7 @@ public class UnregisterAdminCommand implements ExecutableCommand { * @param target the player that was unregistered */ private void applyUnregisteredEffectsAndTasks(Player target) { - // TODO #765: Remove use of Utils method and behave according to settings - Utils.teleportToSpawn(target); + teleportationService.teleportOnJoin(target); limboCache.addLimboPlayer(target); limboPlayerTaskManager.registerTimeoutTask(target); diff --git a/src/main/java/fr/xephi/authme/permission/AuthGroupHandler.java b/src/main/java/fr/xephi/authme/permission/AuthGroupHandler.java index bbad6f0f..5631b58f 100644 --- a/src/main/java/fr/xephi/authme/permission/AuthGroupHandler.java +++ b/src/main/java/fr/xephi/authme/permission/AuthGroupHandler.java @@ -86,4 +86,28 @@ public class AuthGroupHandler { } } + /** + * TODO: This method requires better explanation. + *

+ * Set the normal group of a player. + * + * @param player The player. + * @param group The normal group. + * + * @return True on success, false on failure. + */ + public boolean addNormal(Player player, String group) { + // Check whether the permissions check is enabled + if (!settings.getProperty(PluginSettings.ENABLE_PERMISSION_CHECK)) { + return false; + } + + // Remove old groups + permissionsManager.removeGroups(player, Arrays.asList(Settings.unRegisteredGroup, + Settings.getRegisteredGroup, Settings.getUnloggedinGroup)); + + // Add the normal group, return the result + return permissionsManager.addGroup(player, group); + } + } diff --git a/src/main/java/fr/xephi/authme/process/logout/AsynchronousLogout.java b/src/main/java/fr/xephi/authme/process/logout/AsynchronousLogout.java index f58cd359..7aaa20cb 100644 --- a/src/main/java/fr/xephi/authme/process/logout/AsynchronousLogout.java +++ b/src/main/java/fr/xephi/authme/process/logout/AsynchronousLogout.java @@ -52,6 +52,7 @@ public class AsynchronousLogout implements AsynchronousProcess { limboCache.updateLimboPlayer(player); playerCache.removePlayer(name); + // TODO LJ: No more teleport here? database.setUnlogged(name); syncProcessManager.processSyncPlayerLogout(player); } diff --git a/src/main/java/fr/xephi/authme/process/quit/AsynchronousQuit.java b/src/main/java/fr/xephi/authme/process/quit/AsynchronousQuit.java index 726fc1a2..ff24bbe6 100644 --- a/src/main/java/fr/xephi/authme/process/quit/AsynchronousQuit.java +++ b/src/main/java/fr/xephi/authme/process/quit/AsynchronousQuit.java @@ -12,6 +12,7 @@ import fr.xephi.authme.process.SyncProcessManager; import fr.xephi.authme.settings.SpawnLoader; import fr.xephi.authme.settings.properties.PluginSettings; import fr.xephi.authme.settings.properties.RestrictionSettings; +import fr.xephi.authme.util.BukkitService; import fr.xephi.authme.util.Utils; import org.bukkit.Location; import org.bukkit.entity.Player; @@ -44,6 +45,9 @@ public class AsynchronousQuit implements AsynchronousProcess { @Inject private SpawnLoader spawnLoader; + @Inject + private BukkitService bukkitService; + AsynchronousQuit() { } @@ -76,7 +80,7 @@ public class AsynchronousQuit implements AsynchronousProcess { playerCache.removePlayer(name); if (plugin.isEnabled() && service.getProperty(PluginSettings.SESSIONS_ENABLED)) { - BukkitTask task = plugin.getServer().getScheduler().runTaskLaterAsynchronously(plugin, new Runnable() { + BukkitTask task = bukkitService.runTaskLaterAsynchronously(new Runnable() { @Override public void run() { diff --git a/src/main/java/fr/xephi/authme/process/quit/ProcessSyncronousPlayerQuit.java b/src/main/java/fr/xephi/authme/process/quit/ProcessSyncronousPlayerQuit.java index 88153345..073f2355 100644 --- a/src/main/java/fr/xephi/authme/process/quit/ProcessSyncronousPlayerQuit.java +++ b/src/main/java/fr/xephi/authme/process/quit/ProcessSyncronousPlayerQuit.java @@ -3,6 +3,7 @@ package fr.xephi.authme.process.quit; import fr.xephi.authme.cache.backup.JsonCache; import fr.xephi.authme.cache.limbo.LimboCache; import fr.xephi.authme.cache.limbo.LimboPlayer; +import fr.xephi.authme.permission.AuthGroupHandler; import fr.xephi.authme.process.ProcessService; import fr.xephi.authme.process.SynchronousProcess; import fr.xephi.authme.settings.properties.RestrictionSettings; @@ -24,6 +25,9 @@ public class ProcessSyncronousPlayerQuit implements SynchronousProcess { @Inject private LimboCache limboCache; + @Inject + private AuthGroupHandler authGroupHandler; + public void processSyncQuit(Player player) { LimboPlayer limbo = limboCache.getLimboPlayer(player.getName().toLowerCase()); if (limbo != null) { // it mean player is not authenticated @@ -33,7 +37,7 @@ public class ProcessSyncronousPlayerQuit implements SynchronousProcess { } else { // Restore data if its about to delete LimboPlayer if (!StringUtils.isEmpty(limbo.getGroup())) { - Utils.addNormal(player, limbo.getGroup()); + authGroupHandler.addNormal(player, limbo.getGroup()); } player.setOp(limbo.isOperator()); player.setAllowFlight(limbo.isCanFly()); diff --git a/src/main/java/fr/xephi/authme/process/register/ProcessSyncPasswordRegister.java b/src/main/java/fr/xephi/authme/process/register/ProcessSyncPasswordRegister.java index 208752ec..8d5653b9 100644 --- a/src/main/java/fr/xephi/authme/process/register/ProcessSyncPasswordRegister.java +++ b/src/main/java/fr/xephi/authme/process/register/ProcessSyncPasswordRegister.java @@ -80,7 +80,8 @@ public class ProcessSyncPasswordRegister implements SynchronousProcess { */ private void requestLogin(Player player) { final String name = player.getName().toLowerCase(); - Utils.teleportToSpawn(player); + // TODO #765: Check if teleportation is needed here, and how + // Utils.teleportToSpawn(player); limboCache.updateLimboPlayer(player); limboPlayerTaskManager.registerTimeoutTask(player); @@ -95,7 +96,8 @@ public class ProcessSyncPasswordRegister implements SynchronousProcess { final String name = player.getName().toLowerCase(); LimboPlayer limbo = limboCache.getLimboPlayer(name); if (limbo != null) { - Utils.teleportToSpawn(player); + // TODO #765: Check if teleportation is needed here, and how + // Utils.teleportToSpawn(player); if (service.getProperty(PROTECT_INVENTORY_BEFORE_LOGIN)) { RestoreInventoryEvent event = new RestoreInventoryEvent(player); diff --git a/src/main/java/fr/xephi/authme/process/unregister/AsynchronousUnregister.java b/src/main/java/fr/xephi/authme/process/unregister/AsynchronousUnregister.java index b9b87252..f83e95be 100644 --- a/src/main/java/fr/xephi/authme/process/unregister/AsynchronousUnregister.java +++ b/src/main/java/fr/xephi/authme/process/unregister/AsynchronousUnregister.java @@ -14,7 +14,7 @@ import fr.xephi.authme.settings.Settings; import fr.xephi.authme.settings.properties.RegistrationSettings; import fr.xephi.authme.settings.properties.RestrictionSettings; import fr.xephi.authme.task.LimboPlayerTaskManager; -import fr.xephi.authme.util.Utils; +import fr.xephi.authme.util.TeleportationService; import org.bukkit.entity.Player; import org.bukkit.potion.PotionEffect; import org.bukkit.potion.PotionEffectType; @@ -43,6 +43,9 @@ public class AsynchronousUnregister implements AsynchronousProcess { @Inject private LimboPlayerTaskManager limboPlayerTaskManager; + @Inject + private TeleportationService teleportationService; + AsynchronousUnregister() { } @@ -56,7 +59,7 @@ public class AsynchronousUnregister implements AsynchronousProcess { } if (service.getProperty(RegistrationSettings.FORCE)) { - Utils.teleportToSpawn(player); + teleportationService.teleportOnJoin(player); player.saveData(); playerCache.removePlayer(player.getName().toLowerCase()); if (!Settings.getRegisteredGroup.isEmpty()) { diff --git a/src/main/java/fr/xephi/authme/security/crypts/EncryptionMethod.java b/src/main/java/fr/xephi/authme/security/crypts/EncryptionMethod.java index b1c0b003..83cc6c8c 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/EncryptionMethod.java +++ b/src/main/java/fr/xephi/authme/security/crypts/EncryptionMethod.java @@ -3,9 +3,7 @@ package fr.xephi.authme.security.crypts; /** * Public interface for custom password encryption methods. *

- * Note that {@link fr.xephi.authme.security.PasswordSecurity} requires classes implementing this interface - * to either have the default constructor or an accessible constructor with one parameter of type - * {@link fr.xephi.authme.settings.NewSetting}. + * Instantiation of these methods is done via automatic dependency injection. */ public interface EncryptionMethod { diff --git a/src/main/java/fr/xephi/authme/settings/Settings.java b/src/main/java/fr/xephi/authme/settings/Settings.java index 51bbb11d..bd44a382 100644 --- a/src/main/java/fr/xephi/authme/settings/Settings.java +++ b/src/main/java/fr/xephi/authme/settings/Settings.java @@ -46,7 +46,6 @@ public final class Settings { isTeleportToSpawnEnabled = load(RestrictionSettings.TELEPORT_UNAUTHED_TO_SPAWN); isAllowRestrictedIp = load(RestrictionSettings.ENABLE_RESTRICTED_USERS); isSaveQuitLocationEnabled = load(RestrictionSettings.SAVE_QUIT_LOCATION); - isRemoveSpeedEnabled = load(RestrictionSettings.REMOVE_SPEED); getUnloggedinGroup = load(SecuritySettings.UNLOGGEDIN_GROUP); getNonActivatedGroup = configFile.getInt("ExternalBoardOptions.nonActivedUserGroup", -1); unRegisteredGroup = configFile.getString("GroupOptions.UnregisteredPlayerGroup", ""); diff --git a/src/main/java/fr/xephi/authme/util/BukkitService.java b/src/main/java/fr/xephi/authme/util/BukkitService.java index a88ebafc..652105e7 100644 --- a/src/main/java/fr/xephi/authme/util/BukkitService.java +++ b/src/main/java/fr/xephi/authme/util/BukkitService.java @@ -106,6 +106,23 @@ public class BukkitService { return Bukkit.getScheduler().runTaskAsynchronously(authMe, task); } + /** + * Asynchronous tasks should never access any API in Bukkit. Great care + * should be taken to assure the thread-safety of asynchronous tasks. + *

+ * Returns a task that will run asynchronously after the specified number + * of server ticks. + * + * @param task the task to be run + * @param delay the ticks to wait before running the task + * @return a BukkitTask that contains the id number + * @throws IllegalArgumentException if plugin is null + * @throws IllegalArgumentException if task is null + */ + public BukkitTask runTaskLaterAsynchronously(Runnable task, long delay) { + return Bukkit.getScheduler().runTaskLaterAsynchronously(authMe, task, delay); + } + /** * Broadcast a message to all players. * diff --git a/src/main/java/fr/xephi/authme/util/Utils.java b/src/main/java/fr/xephi/authme/util/Utils.java index d26ae366..0c3d56a5 100644 --- a/src/main/java/fr/xephi/authme/util/Utils.java +++ b/src/main/java/fr/xephi/authme/util/Utils.java @@ -1,15 +1,10 @@ package fr.xephi.authme.util; -import fr.xephi.authme.AuthMe; import fr.xephi.authme.ConsoleLogger; -import fr.xephi.authme.events.AuthMeTeleportEvent; -import fr.xephi.authme.permission.PermissionsManager; import fr.xephi.authme.settings.Settings; -import org.bukkit.Location; import org.bukkit.OfflinePlayer; import org.bukkit.entity.Player; -import java.util.Arrays; import java.util.regex.Pattern; /** @@ -17,41 +12,9 @@ import java.util.regex.Pattern; */ public final class Utils { - private static AuthMe plugin = AuthMe.getInstance(); - private Utils() { } - /** - * TODO: This method requires better explanation. - *

- * Set the normal group of a player. - * - * @param player The player. - * @param group The normal group. - * - * @return True on success, false on failure. - */ - public static boolean addNormal(Player player, String group) { - if (!Settings.isPermissionCheckEnabled) { - return false; - } - - // Get the permissions manager, and make sure it's valid - PermissionsManager permsMan = plugin.getPermissionsManager(); - if (permsMan == null) { - ConsoleLogger.showError("Failed to access permissions manager instance, aborting."); - return false; - } - - // Remove old groups - permsMan.removeGroups(player, Arrays.asList(Settings.unRegisteredGroup, - Settings.getRegisteredGroup, Settings.getUnloggedinGroup)); - - // Add the normal group, return the result - return permsMan.addGroup(player, group); - } - @Deprecated public static boolean isUnrestricted(Player player) { // TODO ljacqu 20160602: Checking for Settings.isAllowRestrictedIp is wrong! Nothing in the config suggests @@ -60,18 +23,6 @@ public final class Utils { && Settings.getUnrestrictedName.contains(player.getName().toLowerCase()); } - @Deprecated - public static void teleportToSpawn(Player player) { - if (Settings.isTeleportToSpawnEnabled && !Settings.noTeleport) { - Location spawn = plugin.getSpawnLocation(player); - AuthMeTeleportEvent tpEvent = new AuthMeTeleportEvent(player, spawn); - plugin.getServer().getPluginManager().callEvent(tpEvent); - if (!tpEvent.isCancelled()) { - player.teleport(tpEvent.getTo()); - } - } - } - public static String getUUIDorName(OfflinePlayer player) { try { return player.getUniqueId().toString(); diff --git a/src/test/java/fr/xephi/authme/util/UtilsTest.java b/src/test/java/fr/xephi/authme/util/UtilsTest.java index 58af9b7c..b57300fd 100644 --- a/src/test/java/fr/xephi/authme/util/UtilsTest.java +++ b/src/test/java/fr/xephi/authme/util/UtilsTest.java @@ -90,4 +90,10 @@ public class UtilsTest { // then assertThat(result, equalTo(name)); } + + @Test + public void shouldHavePrivateConstructorOnly() { + // given / when / then + TestHelper.validateHasOnlyPrivateEmptyConstructor(Utils.class); + } }