From 021497b9e6bb497278f38ba8d49ac81ba1c94e86 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sun, 5 Mar 2017 21:47:46 +0100 Subject: [PATCH] #1113 Handle LimboPlayers within LimboService (remove LimboCache) (work in progress) - Delete LimboCache and LimboPlayerStorage: LimboService now handles all LimboPlayer actions - Revoke player rights when creating a LimboPlayer, within the LimboService - Various fixes and improvements --- .../authme/RegisterAdminCommand.java | 6 - .../xephi/authme/data/limbo/LimboCache.java | 151 ------------ .../xephi/authme/data/limbo/LimboPlayer.java | 10 +- .../authme/data/limbo/LimboPlayerStorage.java | 213 ----------------- .../xephi/authme/data/limbo/LimboService.java | 104 ++++++++- .../authme/process/join/AsynchronousJoin.java | 23 +- .../process/login/ProcessSyncPlayerLogin.java | 5 +- .../process/logout/AsynchronousLogout.java | 8 +- .../ProcessSynchronousPlayerLogout.java | 17 +- .../unregister/AsynchronousUnregister.java | 20 +- .../authme/task/LimboPlayerTaskManager.java | 31 +-- .../authme/RegisterAdminCommandTest.java | 4 - .../authme/data/limbo/LimboCacheTest.java | 215 ------------------ .../data/limbo/LimboPlayerStorageTest.java | 150 ------------ .../AsynchronousUnregisterTest.java | 13 +- .../task/LimboPlayerTaskManagerTest.java | 18 +- 16 files changed, 138 insertions(+), 850 deletions(-) delete mode 100644 src/main/java/fr/xephi/authme/data/limbo/LimboCache.java delete mode 100644 src/main/java/fr/xephi/authme/data/limbo/LimboPlayerStorage.java delete mode 100644 src/test/java/fr/xephi/authme/data/limbo/LimboCacheTest.java delete mode 100644 src/test/java/fr/xephi/authme/data/limbo/LimboPlayerStorageTest.java diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/RegisterAdminCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/RegisterAdminCommand.java index d4ad4946..2f1341b9 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/RegisterAdminCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/RegisterAdminCommand.java @@ -3,7 +3,6 @@ package fr.xephi.authme.command.executable.authme; import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.command.ExecutableCommand; import fr.xephi.authme.data.auth.PlayerAuth; -import fr.xephi.authme.data.limbo.LimboService; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.message.MessageKey; import fr.xephi.authme.security.PasswordSecurity; @@ -38,9 +37,6 @@ public class RegisterAdminCommand implements ExecutableCommand { @Inject private ValidationService validationService; - @Inject - private LimboService limboService; - @Override public void executeCommand(final CommandSender sender, List arguments) { // Get the player name and password @@ -83,8 +79,6 @@ public class RegisterAdminCommand implements ExecutableCommand { bukkitService.scheduleSyncTaskFromOptionallyAsyncTask(new Runnable() { @Override public void run() { - // TODO #1113: Is it necessary to restore here or is this covered by the quit process? - limboService.restoreData(player); player.kickPlayer(commonService.retrieveSingleMessage(MessageKey.KICK_FOR_ADMIN_REGISTER)); } }); diff --git a/src/main/java/fr/xephi/authme/data/limbo/LimboCache.java b/src/main/java/fr/xephi/authme/data/limbo/LimboCache.java deleted file mode 100644 index 99416636..00000000 --- a/src/main/java/fr/xephi/authme/data/limbo/LimboCache.java +++ /dev/null @@ -1,151 +0,0 @@ -package fr.xephi.authme.data.limbo; - -import fr.xephi.authme.ConsoleLogger; -import fr.xephi.authme.permission.PermissionsManager; -import fr.xephi.authme.settings.SpawnLoader; -import org.bukkit.Location; -import org.bukkit.entity.Player; - -import javax.inject.Inject; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; - -import static com.google.common.base.Preconditions.checkNotNull; - -/** - * Manages all {@link LimboPlayer} instances. - */ -class LimboCache { - - private final Map cache = new ConcurrentHashMap<>(); - - private LimboPlayerStorage limboPlayerStorage; - private PermissionsManager permissionsManager; - private SpawnLoader spawnLoader; - - @Inject - LimboCache(PermissionsManager permissionsManager, SpawnLoader spawnLoader, LimboPlayerStorage limboPlayerStorage) { - this.permissionsManager = permissionsManager; - this.spawnLoader = spawnLoader; - this.limboPlayerStorage = limboPlayerStorage; - } - - /** - * Load player data if exist, otherwise current player's data will be stored. - * - * @param player Player instance to add. - */ - public void addPlayerData(Player player) { - String name = player.getName().toLowerCase(); - Location location = spawnLoader.getPlayerLocationOrSpawn(player); - boolean operator = player.isOp(); - boolean flyEnabled = player.getAllowFlight(); - float walkSpeed = player.getWalkSpeed(); - float flySpeed = player.getFlySpeed(); - String playerGroup = ""; - if (permissionsManager.hasGroupSupport()) { - playerGroup = permissionsManager.getPrimaryGroup(player); - } - ConsoleLogger.debug("Player `{0}` has primary group `{1}`", player.getName(), playerGroup); - - if (limboPlayerStorage.hasData(player)) { - LimboPlayer cache = limboPlayerStorage.readData(player); - if (cache != null) { - location = cache.getLocation(); - playerGroup = cache.getGroup(); - operator = cache.isOperator(); - flyEnabled = cache.isCanFly(); - walkSpeed = cache.getWalkSpeed(); - flySpeed = cache.getFlySpeed(); - } - } else { - limboPlayerStorage.saveData(player); - } - - cache.put(name, new LimboPlayer(location, operator, playerGroup, flyEnabled, walkSpeed, flySpeed)); - } - - /** - * Restore player's data to player if exist. - * - * @param player Player instance to restore - */ - public void restoreData(Player player) { - String lowerName = player.getName().toLowerCase(); - if (cache.containsKey(lowerName)) { - LimboPlayer data = cache.get(lowerName); - player.setOp(data.isOperator()); - player.setAllowFlight(data.isCanFly()); - float walkSpeed = data.getWalkSpeed(); - float flySpeed = data.getFlySpeed(); - // Reset the speed value if it was 0 - if (walkSpeed < 0.01f) { - walkSpeed = LimboPlayer.DEFAULT_WALK_SPEED; - } - if (flySpeed < 0.01f) { - flySpeed = LimboPlayer.DEFAULT_FLY_SPEED; - } - player.setWalkSpeed(walkSpeed); - player.setFlySpeed(flySpeed); - data.clearTasks(); - } - } - - /** - * Remove PlayerData from cache and disk. - * - * @param player Player player to remove. - */ - public void deletePlayerData(Player player) { - removeFromCache(player); - limboPlayerStorage.removeData(player); - } - - /** - * Remove PlayerData from cache. - * - * @param player player to remove. - */ - public void removeFromCache(Player player) { - String name = player.getName().toLowerCase(); - LimboPlayer cachedPlayer = cache.remove(name); - if (cachedPlayer != null) { - cachedPlayer.clearTasks(); - } - } - - /** - * Method getPlayerData. - * - * @param name String - * - * @return PlayerData - */ - public LimboPlayer getPlayerData(String name) { - checkNotNull(name); - return cache.get(name.toLowerCase()); - } - - /** - * Method hasPlayerData. - * - * @param name String - * - * @return boolean - */ - public boolean hasPlayerData(String name) { - checkNotNull(name); - return cache.containsKey(name.toLowerCase()); - } - - /** - * Method updatePlayerData. - * - * @param player Player - */ - public void updatePlayerData(Player player) { - checkNotNull(player); - removeFromCache(player); - addPlayerData(player); - } -} diff --git a/src/main/java/fr/xephi/authme/data/limbo/LimboPlayer.java b/src/main/java/fr/xephi/authme/data/limbo/LimboPlayer.java index 45fcd7ad..6ba4ae2c 100644 --- a/src/main/java/fr/xephi/authme/data/limbo/LimboPlayer.java +++ b/src/main/java/fr/xephi/authme/data/limbo/LimboPlayer.java @@ -118,13 +118,7 @@ public class LimboPlayer { * Clears all tasks associated to the player. */ public void clearTasks() { - if (messageTask != null) { - messageTask.cancel(); - } - messageTask = null; - if (timeoutTask != null) { - timeoutTask.cancel(); - } - timeoutTask = null; + setMessageTask(null); + setTimeoutTask(null); } } diff --git a/src/main/java/fr/xephi/authme/data/limbo/LimboPlayerStorage.java b/src/main/java/fr/xephi/authme/data/limbo/LimboPlayerStorage.java deleted file mode 100644 index 4ab45a8d..00000000 --- a/src/main/java/fr/xephi/authme/data/limbo/LimboPlayerStorage.java +++ /dev/null @@ -1,213 +0,0 @@ -package fr.xephi.authme.data.limbo; - -import com.google.common.io.Files; -import com.google.gson.Gson; -import com.google.gson.GsonBuilder; -import com.google.gson.JsonDeserializationContext; -import com.google.gson.JsonDeserializer; -import com.google.gson.JsonElement; -import com.google.gson.JsonObject; -import com.google.gson.JsonSerializationContext; -import com.google.gson.JsonSerializer; -import fr.xephi.authme.ConsoleLogger; -import fr.xephi.authme.initialization.DataFolder; -import fr.xephi.authme.permission.PermissionsManager; -import fr.xephi.authme.service.BukkitService; -import fr.xephi.authme.settings.SpawnLoader; -import fr.xephi.authme.util.FileUtils; -import fr.xephi.authme.util.PlayerUtils; -import org.bukkit.Location; -import org.bukkit.World; -import org.bukkit.entity.Player; - -import javax.inject.Inject; -import java.io.File; -import java.io.IOException; -import java.lang.reflect.Type; -import java.nio.charset.StandardCharsets; - -/** - * Class used to store player's data (OP, flying, speed, position) to disk. - */ -class LimboPlayerStorage { - - private final Gson gson; - private final File cacheDir; - private PermissionsManager permissionsManager; - private SpawnLoader spawnLoader; - private BukkitService bukkitService; - - @Inject - LimboPlayerStorage(@DataFolder File dataFolder, PermissionsManager permsMan, - SpawnLoader spawnLoader, BukkitService bukkitService) { - this.permissionsManager = permsMan; - this.spawnLoader = spawnLoader; - this.bukkitService = bukkitService; - - cacheDir = new File(dataFolder, "playerdata"); - if (!cacheDir.exists() && !cacheDir.isDirectory() && !cacheDir.mkdir()) { - ConsoleLogger.warning("Failed to create userdata directory."); - } - gson = new GsonBuilder() - .registerTypeAdapter(LimboPlayer.class, new LimboPlayerSerializer()) - .registerTypeAdapter(LimboPlayer.class, new LimboPlayerDeserializer()) - .setPrettyPrinting() - .create(); - } - - /** - * Read and construct new PlayerData from existing player data. - * - * @param player player to read - * - * @return PlayerData object if the data is exist, null otherwise. - */ - public LimboPlayer readData(Player player) { - String id = PlayerUtils.getUUIDorName(player); - File file = new File(cacheDir, id + File.separator + "data.json"); - if (!file.exists()) { - return null; - } - - try { - String str = Files.toString(file, StandardCharsets.UTF_8); - return gson.fromJson(str, LimboPlayer.class); - } catch (IOException e) { - ConsoleLogger.logException("Could not read player data on disk for '" + player.getName() + "'", e); - return null; - } - } - - /** - * Save player data (OP, flying, location, etc) to disk. - * - * @param player player to save - */ - public void saveData(Player player) { - String id = PlayerUtils.getUUIDorName(player); - Location location = spawnLoader.getPlayerLocationOrSpawn(player); - String group = ""; - if (permissionsManager.hasGroupSupport()) { - group = permissionsManager.getPrimaryGroup(player); - } - boolean operator = player.isOp(); - boolean canFly = player.getAllowFlight(); - float walkSpeed = player.getWalkSpeed(); - float flySpeed = player.getFlySpeed(); - LimboPlayer limboPlayer = new LimboPlayer(location, operator, group, canFly, walkSpeed, flySpeed); - try { - File file = new File(cacheDir, id + File.separator + "data.json"); - Files.createParentDirs(file); - Files.touch(file); - Files.write(gson.toJson(limboPlayer), file, StandardCharsets.UTF_8); - } catch (IOException e) { - ConsoleLogger.logException("Failed to write " + player.getName() + " data.", e); - } - } - - /** - * Remove player data, this will delete - * "playerdata/<uuid or name>/" folder from disk. - * - * @param player player to remove - */ - public void removeData(Player player) { - String id = PlayerUtils.getUUIDorName(player); - File file = new File(cacheDir, id); - if (file.exists()) { - FileUtils.purgeDirectory(file); - if (!file.delete()) { - ConsoleLogger.warning("Failed to remove " + player.getName() + " cache."); - } - } - } - - /** - * Use to check is player data is exist. - * - * @param player player to check - * - * @return true if data exist, false otherwise. - */ - public boolean hasData(Player player) { - String id = PlayerUtils.getUUIDorName(player); - File file = new File(cacheDir, id + File.separator + "data.json"); - return file.exists(); - } - - private class LimboPlayerDeserializer implements JsonDeserializer { - @Override - public LimboPlayer deserialize(JsonElement jsonElement, Type type, - JsonDeserializationContext context) { - JsonObject jsonObject = jsonElement.getAsJsonObject(); - if (jsonObject == null) { - return null; - } - - Location loc = null; - String group = ""; - boolean operator = false; - boolean canFly = false; - float walkSpeed = LimboPlayer.DEFAULT_WALK_SPEED; - float flySpeed = LimboPlayer.DEFAULT_FLY_SPEED; - - JsonElement e; - if ((e = jsonObject.getAsJsonObject("location")) != null) { - JsonObject obj = e.getAsJsonObject(); - World world = bukkitService.getWorld(obj.get("world").getAsString()); - if (world != null) { - double x = obj.get("x").getAsDouble(); - double y = obj.get("y").getAsDouble(); - double z = obj.get("z").getAsDouble(); - float yaw = obj.get("yaw").getAsFloat(); - float pitch = obj.get("pitch").getAsFloat(); - loc = new Location(world, x, y, z, yaw, pitch); - } - } - if ((e = jsonObject.get("group")) != null) { - group = e.getAsString(); - } - if ((e = jsonObject.get("operator")) != null) { - operator = e.getAsBoolean(); - } - if ((e = jsonObject.get("can-fly")) != null) { - canFly = e.getAsBoolean(); - } - if ((e = jsonObject.get("walk-speed")) != null) { - walkSpeed = e.getAsFloat(); - } - if ((e = jsonObject.get("fly-speed")) != null) { - flySpeed = e.getAsFloat(); - } - - return new LimboPlayer(loc, operator, group, canFly, walkSpeed, flySpeed); - } - } - - private class LimboPlayerSerializer implements JsonSerializer { - @Override - public JsonElement serialize(LimboPlayer limboPlayer, Type type, - JsonSerializationContext context) { - JsonObject obj = new JsonObject(); - obj.addProperty("group", limboPlayer.getGroup()); - - Location loc = limboPlayer.getLocation(); - JsonObject obj2 = new JsonObject(); - obj2.addProperty("world", loc.getWorld().getName()); - obj2.addProperty("x", loc.getX()); - obj2.addProperty("y", loc.getY()); - obj2.addProperty("z", loc.getZ()); - obj2.addProperty("yaw", loc.getYaw()); - obj2.addProperty("pitch", loc.getPitch()); - obj.add("location", obj2); - - obj.addProperty("operator", limboPlayer.isOperator()); - obj.addProperty("can-fly", limboPlayer.isCanFly()); - obj.addProperty("walk-speed", limboPlayer.getWalkSpeed()); - obj.addProperty("fly-speed", limboPlayer.getFlySpeed()); - return obj; - } - } - - -} diff --git a/src/main/java/fr/xephi/authme/data/limbo/LimboService.java b/src/main/java/fr/xephi/authme/data/limbo/LimboService.java index 0c3b2532..0333a165 100644 --- a/src/main/java/fr/xephi/authme/data/limbo/LimboService.java +++ b/src/main/java/fr/xephi/authme/data/limbo/LimboService.java @@ -1,8 +1,16 @@ package fr.xephi.authme.data.limbo; +import fr.xephi.authme.ConsoleLogger; +import fr.xephi.authme.permission.PermissionsManager; +import fr.xephi.authme.settings.Settings; +import fr.xephi.authme.settings.SpawnLoader; +import fr.xephi.authme.settings.properties.RestrictionSettings; +import org.bukkit.Location; import org.bukkit.entity.Player; import javax.inject.Inject; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; /** * Service for managing players that are in "limbo," a temporary state players are @@ -10,8 +18,16 @@ import javax.inject.Inject; */ public class LimboService { + private Map entries = new ConcurrentHashMap<>(); + @Inject - private LimboCache limboCache; + private SpawnLoader spawnLoader; + + @Inject + private PermissionsManager permissionsManager; + + @Inject + private Settings settings; LimboService() { } @@ -19,13 +35,34 @@ public class LimboService { /** * Restores the limbo data and subsequently deletes the entry. + *

+ * Note that teleportation on the player is performed by {@link fr.xephi.authme.service.TeleportationService} and + * changing the permission group is handled by {@link fr.xephi.authme.permission.AuthGroupHandler}. * * @param player the player whose data should be restored */ public void restoreData(Player player) { - // TODO #1113: Think about architecture for various "restore" strategies - limboCache.restoreData(player); - limboCache.deletePlayerData(player); + String lowerName = player.getName().toLowerCase(); + LimboPlayer limbo = entries.remove(lowerName); + + if (limbo == null) { + ConsoleLogger.debug("No LimboPlayer found for `{0}` - cannot restore", lowerName); + } else { + player.setOp(limbo.isOperator()); + player.setAllowFlight(limbo.isCanFly()); + float walkSpeed = limbo.getWalkSpeed(); + float flySpeed = limbo.getFlySpeed(); + // Reset the speed value if it was 0 + if (walkSpeed < 0.01f) { + walkSpeed = LimboPlayer.DEFAULT_WALK_SPEED; + } + if (flySpeed < 0.01f) { + flySpeed = LimboPlayer.DEFAULT_FLY_SPEED; + } + player.setWalkSpeed(walkSpeed); + player.setFlySpeed(flySpeed); + limbo.clearTasks(); + } } /** @@ -35,7 +72,7 @@ public class LimboService { * @return the associated limbo player, or null if none available */ public LimboPlayer getLimboPlayer(String name) { - return limboCache.getPlayerData(name); + return entries.get(name.toLowerCase()); } /** @@ -45,16 +82,65 @@ public class LimboService { * @return true if present, false otherwise */ public boolean hasLimboPlayer(String name) { - return limboCache.hasPlayerData(name); + return entries.containsKey(name.toLowerCase()); } /** - * Creates a LimboPlayer for the given player. + * Creates a LimboPlayer for the given player and revokes all "limbo data" from the player. * * @param player the player to process */ public void createLimboPlayer(Player player) { - // TODO #1113: We should remove the player's data in here as well - limboCache.addPlayerData(player); + final String name = player.getName().toLowerCase(); + + LimboPlayer existingLimbo = entries.remove(name); + if (existingLimbo != null) { + existingLimbo.clearTasks(); + ConsoleLogger.debug("LimboPlayer for `{0}` was already present", name); + } + + LimboPlayer limboPlayer = newLimboPlayer(player); + revokeLimboStates(player); + entries.put(name, limboPlayer); + } + + /** + * Creates a LimboPlayer with the given player's details. + * + * @param player the player to process + * @return limbo player with the player's data + */ + private LimboPlayer newLimboPlayer(Player player) { + Location location = spawnLoader.getPlayerLocationOrSpawn(player); + boolean isOperator = player.isOp(); + boolean flyEnabled = player.getAllowFlight(); + float walkSpeed = player.getWalkSpeed(); + float flySpeed = player.getFlySpeed(); + String playerGroup = ""; + if (permissionsManager.hasGroupSupport()) { + playerGroup = permissionsManager.getPrimaryGroup(player); + } + ConsoleLogger.debug("Player `{0}` has primary group `{1}`", player.getName(), playerGroup); + + return new LimboPlayer(location, isOperator, playerGroup, flyEnabled, walkSpeed, flySpeed); + } + + /** + * Removes the data that is saved in a LimboPlayer from the player. + *

+ * Note that teleportation on the player is performed by {@link fr.xephi.authme.service.TeleportationService} and + * changing the permission group is handled by {@link fr.xephi.authme.permission.AuthGroupHandler}. + * + * @param player the player to set defaults to + */ + private void revokeLimboStates(Player player) { + player.setOp(false); + player.setAllowFlight(false); + + if (!settings.getProperty(RestrictionSettings.ALLOW_UNAUTHED_MOVEMENT) + && settings.getProperty(RestrictionSettings.REMOVE_SPEED)) { + player.setFlySpeed(0.0f); + player.setWalkSpeed(0.0f); + } } } diff --git a/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java b/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java index b03a70f8..af55ec65 100644 --- a/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java +++ b/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java @@ -111,7 +111,6 @@ public class AsynchronousJoin implements AsynchronousProcess { final boolean isAuthAvailable = database.isAuthAvailable(name); if (isAuthAvailable) { - limboService.createLimboPlayer(player); service.setGroup(player, AuthGroupType.REGISTERED_UNAUTHENTICATED); // Protect inventory @@ -141,11 +140,6 @@ public class AsynchronousJoin implements AsynchronousProcess { } } } else { - // TODO #1113: Why delete and add LimboPlayer again? - // Not Registered. Delete old data, load default one. - // limboCache.deletePlayerData(player); - // limboCache.addPlayerData(player); - // Groups logic service.setGroup(player, AuthGroupType.UNREGISTERED); @@ -158,12 +152,11 @@ public class AsynchronousJoin implements AsynchronousProcess { final int registrationTimeout = service.getProperty(RestrictionSettings.TIMEOUT) * TICKS_PER_SECOND; bukkitService.scheduleSyncTaskFromOptionallyAsyncTask(() -> { - player.setOp(false); - if (!service.getProperty(RestrictionSettings.ALLOW_UNAUTHED_MOVEMENT) - && service.getProperty(RestrictionSettings.REMOVE_SPEED)) { - player.setFlySpeed(0.0f); - player.setWalkSpeed(0.0f); - } + // TODO #1113: Find an elegant way to deop unregistered players (and disable fly status etc.?) + limboService.createLimboPlayer(player); + limboPlayerTaskManager.registerTimeoutTask(player); + limboPlayerTaskManager.registerMessageTask(name, isAuthAvailable); + player.setNoDamageTicks(registrationTimeout); if (pluginHookService.isEssentialsAvailable() && service.getProperty(HooksSettings.USE_ESSENTIALS_MOTD)) { player.performCommand("motd"); @@ -175,10 +168,6 @@ public class AsynchronousJoin implements AsynchronousProcess { } commandManager.runCommandsOnJoin(player); }); - - // Timeout and message task - limboPlayerTaskManager.registerTimeoutTask(player); - limboPlayerTaskManager.registerMessageTask(name, isAuthAvailable); } /** @@ -189,7 +178,7 @@ public class AsynchronousJoin implements AsynchronousProcess { * @param domain The hostname of the IP address * * @return True if the name is restricted (IP/domain is not allowed for the given name), - * false if the restrictions are met or if the name has no restrictions to it + * false if the restrictions are met or if the name has no restrictions to it */ private boolean isNameRestricted(String name, String ip, String domain) { if (!service.getProperty(RestrictionSettings.ENABLE_RESTRICTED_USERS)) { diff --git a/src/main/java/fr/xephi/authme/process/login/ProcessSyncPlayerLogin.java b/src/main/java/fr/xephi/authme/process/login/ProcessSyncPlayerLogin.java index 4cdef994..206a55fa 100644 --- a/src/main/java/fr/xephi/authme/process/login/ProcessSyncPlayerLogin.java +++ b/src/main/java/fr/xephi/authme/process/login/ProcessSyncPlayerLogin.java @@ -65,15 +65,12 @@ public class ProcessSyncPlayerLogin implements SynchronousProcess { public void processPlayerLogin(Player player) { final String name = player.getName().toLowerCase(); + commonService.setGroup(player, AuthGroupType.LOGGED_IN); final LimboPlayer limbo = limboService.getLimboPlayer(name); // Limbo contains the State of the Player before /login if (limbo != null) { limboService.restoreData(player); - // do we really need to use location from database for now? - // because LimboCache#restoreData teleport player to last location. } - // TODO #1113: Need to set group before deleting limboPlayer?? - commonService.setGroup(player, AuthGroupType.LOGGED_IN); if (commonService.getProperty(PROTECT_INVENTORY_BEFORE_LOGIN)) { restoreInventory(player); 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 1120fd59..1f59d965 100644 --- a/src/main/java/fr/xephi/authme/process/logout/AsynchronousLogout.java +++ b/src/main/java/fr/xephi/authme/process/logout/AsynchronousLogout.java @@ -2,12 +2,11 @@ package fr.xephi.authme.process.logout; import fr.xephi.authme.data.auth.PlayerAuth; import fr.xephi.authme.data.auth.PlayerCache; -import fr.xephi.authme.data.limbo.LimboService; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.message.MessageKey; import fr.xephi.authme.process.AsynchronousProcess; -import fr.xephi.authme.service.CommonService; import fr.xephi.authme.process.SyncProcessManager; +import fr.xephi.authme.service.CommonService; import fr.xephi.authme.settings.properties.RestrictionSettings; import org.bukkit.entity.Player; @@ -24,9 +23,6 @@ public class AsynchronousLogout implements AsynchronousProcess { @Inject private PlayerCache playerCache; - @Inject - private LimboService limboService; - @Inject private SyncProcessManager syncProcessManager; @@ -47,8 +43,6 @@ public class AsynchronousLogout implements AsynchronousProcess { database.updateQuitLoc(auth); } - // TODO #1113: Would we not have to RESTORE the limbo player here? - limboService.createLimboPlayer(player); playerCache.removePlayer(name); database.setUnlogged(name); syncProcessManager.processSyncPlayerLogout(player); diff --git a/src/main/java/fr/xephi/authme/process/logout/ProcessSynchronousPlayerLogout.java b/src/main/java/fr/xephi/authme/process/logout/ProcessSynchronousPlayerLogout.java index 9f5dc32d..ffafe16b 100644 --- a/src/main/java/fr/xephi/authme/process/logout/ProcessSynchronousPlayerLogout.java +++ b/src/main/java/fr/xephi/authme/process/logout/ProcessSynchronousPlayerLogout.java @@ -2,6 +2,7 @@ package fr.xephi.authme.process.logout; import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.data.SessionManager; +import fr.xephi.authme.data.limbo.LimboService; import fr.xephi.authme.events.LogoutEvent; import fr.xephi.authme.listener.protocollib.ProtocolLibService; import fr.xephi.authme.message.MessageKey; @@ -36,6 +37,9 @@ public class ProcessSynchronousPlayerLogout implements SynchronousProcess { @Inject private LimboPlayerTaskManager limboPlayerTaskManager; + @Inject + private LimboService limboService; + @Inject private SessionManager sessionManager; @@ -53,11 +57,11 @@ public class ProcessSynchronousPlayerLogout implements SynchronousProcess { protocolLibService.sendBlankInventoryPacket(player); } + applyLogoutEffect(player); + limboPlayerTaskManager.registerTimeoutTask(player); limboPlayerTaskManager.registerMessageTask(name, true); - applyLogoutEffect(player); - // Player is now logout... Time to fire event ! bukkitService.callEvent(new LogoutEvent(player)); @@ -77,15 +81,8 @@ public class ProcessSynchronousPlayerLogout implements SynchronousProcess { } // Set player's data to unauthenticated + limboService.createLimboPlayer(player); service.setGroup(player, AuthGroupType.REGISTERED_UNAUTHENTICATED); - player.setOp(false); - player.setAllowFlight(false); - // Remove speed - if (!service.getProperty(RestrictionSettings.ALLOW_UNAUTHED_MOVEMENT) - && service.getProperty(RestrictionSettings.REMOVE_SPEED)) { - player.setFlySpeed(0.0f); - player.setWalkSpeed(0.0f); - } } } 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 879edd18..66790156 100644 --- a/src/main/java/fr/xephi/authme/process/unregister/AsynchronousUnregister.java +++ b/src/main/java/fr/xephi/authme/process/unregister/AsynchronousUnregister.java @@ -111,12 +111,13 @@ public class AsynchronousUnregister implements AsynchronousProcess { teleportationService.teleportOnJoin(player); player.saveData(); - // TODO #1113: Why delete? limboCache.deletePlayerData(player); - limboService.createLimboPlayer(player); + bukkitService.scheduleSyncTaskFromOptionallyAsyncTask(() -> { + limboService.createLimboPlayer(player); + limboPlayerTaskManager.registerTimeoutTask(player); + limboPlayerTaskManager.registerMessageTask(name, false); - limboPlayerTaskManager.registerTimeoutTask(player); - limboPlayerTaskManager.registerMessageTask(name, false); - applyBlindEffect(player); + applyBlindEffect(player); + }); } authGroupHandler.setGroup(player, AuthGroupType.UNREGISTERED); service.send(player, MessageKey.UNREGISTERED_SUCCESS); @@ -124,13 +125,8 @@ public class AsynchronousUnregister implements AsynchronousProcess { private void applyBlindEffect(final Player player) { if (service.getProperty(RegistrationSettings.APPLY_BLIND_EFFECT)) { - final int timeout = service.getProperty(RestrictionSettings.TIMEOUT) * TICKS_PER_SECOND; - bukkitService.runTask(new Runnable() { - @Override - public void run() { - player.addPotionEffect(new PotionEffect(PotionEffectType.BLINDNESS, timeout, 2)); - } - }); + int timeout = service.getProperty(RestrictionSettings.TIMEOUT) * TICKS_PER_SECOND; + player.addPotionEffect(new PotionEffect(PotionEffectType.BLINDNESS, timeout, 2)); } } } diff --git a/src/main/java/fr/xephi/authme/task/LimboPlayerTaskManager.java b/src/main/java/fr/xephi/authme/task/LimboPlayerTaskManager.java index 25e11863..f66f9445 100644 --- a/src/main/java/fr/xephi/authme/task/LimboPlayerTaskManager.java +++ b/src/main/java/fr/xephi/authme/task/LimboPlayerTaskManager.java @@ -11,7 +11,6 @@ import fr.xephi.authme.settings.Settings; import fr.xephi.authme.settings.properties.RegistrationSettings; import fr.xephi.authme.settings.properties.RestrictionSettings; import org.bukkit.entity.Player; -import org.bukkit.scheduler.BukkitRunnable; import org.bukkit.scheduler.BukkitTask; import javax.inject.Inject; @@ -19,7 +18,7 @@ import javax.inject.Inject; import static fr.xephi.authme.service.BukkitService.TICKS_PER_SECOND; /** - * Registers tasks associated with a PlayerData. + * Registers tasks associated with a LimboPlayer. */ public class LimboPlayerTaskManager { @@ -55,9 +54,8 @@ public class LimboPlayerTaskManager { if (interval > 0) { final LimboPlayer limboPlayer = limboService.getLimboPlayer(name); if (limboPlayer == null) { - ConsoleLogger.info("PlayerData for '" + name + "' is not available"); + ConsoleLogger.info("LimboPlayer for '" + name + "' is not available (MessageTask)"); } else { - cancelTask(limboPlayer.getMessageTask()); MessageTask messageTask = new MessageTask(name, messages.retrieve(key), bukkitService, playerCache); bukkitService.runTaskTimer(messageTask, 2 * TICKS_PER_SECOND, interval * TICKS_PER_SECOND); limboPlayer.setMessageTask(messageTask); @@ -75,9 +73,8 @@ public class LimboPlayerTaskManager { if (timeout > 0) { final LimboPlayer limboPlayer = limboService.getLimboPlayer(player.getName()); if (limboPlayer == null) { - ConsoleLogger.info("PlayerData for '" + player.getName() + "' is not available"); + ConsoleLogger.info("LimboPlayer for '" + player.getName() + "' is not available (TimeoutTask)"); } else { - cancelTask(limboPlayer.getTimeoutTask()); String message = messages.retrieveSingle(MessageKey.LOGIN_TIMEOUT_ERROR); BukkitTask task = bukkitService.runTaskLater(new TimeoutTask(player, message, playerCache), timeout); limboPlayer.setTimeoutTask(task); @@ -120,28 +117,6 @@ public class LimboPlayerTaskManager { } } - /** - * Null-safe method to cancel a potentially existing task. - * - * @param task the task to cancel (or null) - */ - private static void cancelTask(BukkitTask task) { - if (task != null) { - task.cancel(); - } - } - - /** - * Null-safe method to cancel a potentially existing task. - * - * @param task the task to cancel (or null) - */ - private static void cancelTask(BukkitRunnable task) { - if (task != null) { - task.cancel(); - } - } - /** * Null-safe method to set the muted flag on a message task. * diff --git a/src/test/java/fr/xephi/authme/command/executable/authme/RegisterAdminCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/authme/RegisterAdminCommandTest.java index ebe50a74..5ee2161e 100644 --- a/src/test/java/fr/xephi/authme/command/executable/authme/RegisterAdminCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/authme/RegisterAdminCommandTest.java @@ -2,7 +2,6 @@ package fr.xephi.authme.command.executable.authme; import fr.xephi.authme.TestHelper; import fr.xephi.authme.data.auth.PlayerAuth; -import fr.xephi.authme.data.limbo.LimboService; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.message.MessageKey; import fr.xephi.authme.security.PasswordSecurity; @@ -56,9 +55,6 @@ public class RegisterAdminCommandTest { @Mock private ValidationService validationService; - @Mock - private LimboService limboService; - @BeforeClass public static void setUpLogger() { TestHelper.setupLogger(); diff --git a/src/test/java/fr/xephi/authme/data/limbo/LimboCacheTest.java b/src/test/java/fr/xephi/authme/data/limbo/LimboCacheTest.java deleted file mode 100644 index 8d307c25..00000000 --- a/src/test/java/fr/xephi/authme/data/limbo/LimboCacheTest.java +++ /dev/null @@ -1,215 +0,0 @@ -package fr.xephi.authme.data.limbo; - -import fr.xephi.authme.ReflectionTestUtils; -import fr.xephi.authme.permission.PermissionsManager; -import fr.xephi.authme.settings.SpawnLoader; -import org.bukkit.Location; -import org.bukkit.entity.Player; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.InjectMocks; -import org.mockito.Mock; -import org.mockito.junit.MockitoJUnitRunner; - -import java.util.Map; - -import static org.hamcrest.Matchers.aMapWithSize; -import static org.hamcrest.Matchers.anEmptyMap; -import static org.hamcrest.Matchers.equalTo; -import static org.junit.Assert.assertThat; -import static org.mockito.BDDMockito.given; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; - -/** - * Test for {@link LimboCache}. - */ -@RunWith(MockitoJUnitRunner.class) -public class LimboCacheTest { - - @InjectMocks - private LimboCache limboCache; - - @Mock - private PermissionsManager permissionsManager; - - @Mock - private SpawnLoader spawnLoader; - - @Mock - private LimboPlayerStorage limboPlayerStorage; - - @Test - public void shouldAddPlayerData() { - // given - Player player = mock(Player.class); - String name = "Bobby"; - given(player.getName()).willReturn(name); - Location location = mock(Location.class); - given(spawnLoader.getPlayerLocationOrSpawn(player)).willReturn(location); - given(player.isOp()).willReturn(true); - float walkSpeed = 2.1f; - given(player.getWalkSpeed()).willReturn(walkSpeed); - given(player.getAllowFlight()).willReturn(true); - float flySpeed = 3.0f; - given(player.getFlySpeed()).willReturn(flySpeed); - given(permissionsManager.hasGroupSupport()).willReturn(true); - String group = "test-group"; - given(permissionsManager.getPrimaryGroup(player)).willReturn(group); - given(limboPlayerStorage.hasData(player)).willReturn(false); - - // when - limboCache.addPlayerData(player); - - // then - LimboPlayer limboPlayer = limboCache.getPlayerData(name); - assertThat(limboPlayer.getLocation(), equalTo(location)); - assertThat(limboPlayer.isOperator(), equalTo(true)); - assertThat(limboPlayer.getWalkSpeed(), equalTo(walkSpeed)); - assertThat(limboPlayer.isCanFly(), equalTo(true)); - assertThat(limboPlayer.getFlySpeed(), equalTo(flySpeed)); - assertThat(limboPlayer.getGroup(), equalTo(group)); - } - - @Test - public void shouldGetPlayerDataFromDisk() { - // given - String name = "player01"; - Player player = mock(Player.class); - given(player.getName()).willReturn(name); - given(limboPlayerStorage.hasData(player)).willReturn(true); - LimboPlayer limboPlayer = mock(LimboPlayer.class); - given(limboPlayerStorage.readData(player)).willReturn(limboPlayer); - float walkSpeed = 2.4f; - given(limboPlayer.getWalkSpeed()).willReturn(walkSpeed); - given(limboPlayer.isCanFly()).willReturn(true); - float flySpeed = 1.0f; - given(limboPlayer.getFlySpeed()).willReturn(flySpeed); - String group = "primary-group"; - given(limboPlayer.getGroup()).willReturn(group); - - // when - limboCache.addPlayerData(player); - - // then - LimboPlayer result = limboCache.getPlayerData(name); - assertThat(result.getWalkSpeed(), equalTo(walkSpeed)); - assertThat(result.isCanFly(), equalTo(true)); - assertThat(result.getFlySpeed(), equalTo(flySpeed)); - assertThat(result.getGroup(), equalTo(group)); - } - - @Test - public void shouldRestorePlayerInfo() { - // given - String name = "Champ"; - Player player = mock(Player.class); - given(player.getName()).willReturn(name); - LimboPlayer limboPlayer = mock(LimboPlayer.class); - given(limboPlayer.isOperator()).willReturn(true); - float walkSpeed = 2.4f; - given(limboPlayer.getWalkSpeed()).willReturn(walkSpeed); - given(limboPlayer.isCanFly()).willReturn(true); - float flySpeed = 1.0f; - given(limboPlayer.getFlySpeed()).willReturn(flySpeed); - getCache().put(name.toLowerCase(), limboPlayer); - - // when - limboCache.restoreData(player); - - // then - verify(player).setOp(true); - verify(player).setWalkSpeed(walkSpeed); - verify(player).setAllowFlight(true); - verify(player).setFlySpeed(flySpeed); - verify(limboPlayer).clearTasks(); - } - - @Test - public void shouldResetPlayerSpeed() { - // given - String name = "Champ"; - Player player = mock(Player.class); - given(player.getName()).willReturn(name); - LimboPlayer limboPlayer = mock(LimboPlayer.class); - given(limboPlayer.isOperator()).willReturn(true); - given(limboPlayer.getWalkSpeed()).willReturn(0f); - given(limboPlayer.isCanFly()).willReturn(true); - given(limboPlayer.getFlySpeed()).willReturn(0f); - getCache().put(name.toLowerCase(), limboPlayer); - - // when - limboCache.restoreData(player); - - // then - verify(player).setWalkSpeed(LimboPlayer.DEFAULT_WALK_SPEED); - verify(player).setFlySpeed(LimboPlayer.DEFAULT_FLY_SPEED); - } - - @Test - public void shouldNotInteractWithPlayerIfNoDataAvailable() { - // given - String name = "player"; - Player player = mock(Player.class); - given(player.getName()).willReturn(name); - - // when - limboCache.restoreData(player); - - // then - verify(player).getName(); - verifyNoMoreInteractions(player); - } - - @Test - public void shouldRemoveAndClearTasks() { - // given - LimboPlayer limboPlayer = mock(LimboPlayer.class); - String name = "abcdef"; - getCache().put(name, limboPlayer); - Player player = mock(Player.class); - given(player.getName()).willReturn(name); - - // when - limboCache.removeFromCache(player); - - // then - assertThat(getCache(), anEmptyMap()); - verify(limboPlayer).clearTasks(); - } - - @Test - public void shouldDeleteFromCacheAndStorage() { - // given - LimboPlayer limboPlayer = mock(LimboPlayer.class); - String name = "SomeName"; - getCache().put(name.toLowerCase(), limboPlayer); - getCache().put("othername", mock(LimboPlayer.class)); - Player player = mock(Player.class); - given(player.getName()).willReturn(name); - - // when - limboCache.deletePlayerData(player); - - // then - assertThat(getCache(), aMapWithSize(1)); - verify(limboPlayer).clearTasks(); - verify(limboPlayerStorage).removeData(player); - } - - @Test - public void shouldReturnIfHasData() { - // given - String name = "tester"; - getCache().put(name, mock(LimboPlayer.class)); - - // when / then - assertThat(limboCache.hasPlayerData(name), equalTo(true)); - assertThat(limboCache.hasPlayerData("someone_else"), equalTo(false)); - } - - private Map getCache() { - return ReflectionTestUtils.getFieldValue(LimboCache.class, limboCache, "cache"); - } -} diff --git a/src/test/java/fr/xephi/authme/data/limbo/LimboPlayerStorageTest.java b/src/test/java/fr/xephi/authme/data/limbo/LimboPlayerStorageTest.java deleted file mode 100644 index 6d346854..00000000 --- a/src/test/java/fr/xephi/authme/data/limbo/LimboPlayerStorageTest.java +++ /dev/null @@ -1,150 +0,0 @@ -package fr.xephi.authme.data.limbo; - -import ch.jalu.injector.testing.BeforeInjecting; -import ch.jalu.injector.testing.DelayedInjectionRunner; -import ch.jalu.injector.testing.InjectDelayed; -import fr.xephi.authme.TestHelper; -import fr.xephi.authme.initialization.DataFolder; -import fr.xephi.authme.permission.PermissionsManager; -import fr.xephi.authme.service.BukkitService; -import fr.xephi.authme.settings.SpawnLoader; -import fr.xephi.authme.util.FileUtils; -import org.bukkit.Location; -import org.bukkit.World; -import org.bukkit.entity.Player; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.TemporaryFolder; -import org.junit.runner.RunWith; -import org.mockito.Mock; - -import java.io.File; -import java.io.IOException; -import java.nio.file.Files; -import java.util.UUID; - -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.not; -import static org.hamcrest.Matchers.nullValue; -import static org.junit.Assert.assertThat; -import static org.mockito.BDDMockito.given; -import static org.mockito.Mockito.mock; - -/** - * Test for {@link LimboPlayerStorage}. - */ -@RunWith(DelayedInjectionRunner.class) -public class LimboPlayerStorageTest { - - private static final UUID SAMPLE_UUID = UUID.nameUUIDFromBytes("PlayerDataStorageTest".getBytes()); - private static final String SOURCE_FOLDER = TestHelper.PROJECT_ROOT + "data/backup/"; - - @InjectDelayed - private LimboPlayerStorage limboPlayerStorage; - - @Mock - private SpawnLoader spawnLoader; - - @Mock - private BukkitService bukkitService; - - @Mock - private PermissionsManager permissionsManager; - - @DataFolder - private File dataFolder; - - @Rule - public TemporaryFolder temporaryFolder = new TemporaryFolder(); - - @BeforeInjecting - public void copyTestFiles() throws IOException { - dataFolder = temporaryFolder.newFolder(); - File playerFolder = new File(dataFolder, FileUtils.makePath("playerdata", SAMPLE_UUID.toString())); - if (!playerFolder.mkdirs()) { - throw new IllegalStateException("Cannot create '" + playerFolder.getAbsolutePath() + "'"); - } - Files.copy(TestHelper.getJarPath(FileUtils.makePath(SOURCE_FOLDER, "sample-folder", "data.json")), - new File(playerFolder, "data.json").toPath()); - } - - @Test - public void shouldReadDataFromFile() { - // given - Player player = mock(Player.class); - given(player.getUniqueId()).willReturn(SAMPLE_UUID); - World world = mock(World.class); - given(bukkitService.getWorld("nether")).willReturn(world); - - // when - LimboPlayer data = limboPlayerStorage.readData(player); - - // then - assertThat(data, not(nullValue())); - assertThat(data.isOperator(), equalTo(true)); - assertThat(data.isCanFly(), equalTo(true)); - assertThat(data.getWalkSpeed(), equalTo(0.2f)); - assertThat(data.getFlySpeed(), equalTo(0.1f)); - assertThat(data.getGroup(), equalTo("players")); - Location location = data.getLocation(); - assertThat(location.getX(), equalTo(-113.219)); - assertThat(location.getY(), equalTo(72.0)); - assertThat(location.getZ(), equalTo(130.637)); - assertThat(location.getWorld(), equalTo(world)); - assertThat(location.getPitch(), equalTo(24.15f)); - assertThat(location.getYaw(), equalTo(-292.484f)); - } - - @Test - public void shouldReturnNullForUnavailablePlayer() { - // given - Player player = mock(Player.class); - given(player.getUniqueId()).willReturn(UUID.nameUUIDFromBytes("other-player".getBytes())); - - // when - LimboPlayer data = limboPlayerStorage.readData(player); - - // then - assertThat(data, nullValue()); - } - - @Test - public void shouldReturnIfHasData() { - // given - Player player1 = mock(Player.class); - given(player1.getUniqueId()).willReturn(SAMPLE_UUID); - Player player2 = mock(Player.class); - given(player2.getUniqueId()).willReturn(UUID.nameUUIDFromBytes("not-stored".getBytes())); - - // when / then - assertThat(limboPlayerStorage.hasData(player1), equalTo(true)); - assertThat(limboPlayerStorage.hasData(player2), equalTo(false)); - } - - @Test - public void shouldSavePlayerData() { - // given - Player player = mock(Player.class); - UUID uuid = UUID.nameUUIDFromBytes("New player".getBytes()); - given(player.getUniqueId()).willReturn(uuid); - given(permissionsManager.getPrimaryGroup(player)).willReturn("primary-grp"); - given(player.isOp()).willReturn(true); - given(player.getWalkSpeed()).willReturn(1.2f); - given(player.getFlySpeed()).willReturn(0.8f); - given(player.getAllowFlight()).willReturn(true); - - World world = mock(World.class); - given(world.getName()).willReturn("player-world"); - Location location = new Location(world, 0.2, 102.25, -89.28, 3.02f, 90.13f); - given(spawnLoader.getPlayerLocationOrSpawn(player)).willReturn(location); - - // when - limboPlayerStorage.saveData(player); - - // then - File playerFile = new File(dataFolder, FileUtils.makePath("playerdata", uuid.toString(), "data.json")); - assertThat(playerFile.exists(), equalTo(true)); - // TODO ljacqu 20160711: Check contents of file - } - -} diff --git a/src/test/java/fr/xephi/authme/process/unregister/AsynchronousUnregisterTest.java b/src/test/java/fr/xephi/authme/process/unregister/AsynchronousUnregisterTest.java index fac4917a..195de52c 100644 --- a/src/test/java/fr/xephi/authme/process/unregister/AsynchronousUnregisterTest.java +++ b/src/test/java/fr/xephi/authme/process/unregister/AsynchronousUnregisterTest.java @@ -14,7 +14,6 @@ import fr.xephi.authme.service.BukkitService; import fr.xephi.authme.service.CommonService; import fr.xephi.authme.service.TeleportationService; import fr.xephi.authme.settings.properties.RegistrationSettings; -import fr.xephi.authme.settings.properties.RestrictionSettings; import fr.xephi.authme.task.LimboPlayerTaskManager; import org.bukkit.command.CommandSender; import org.bukkit.entity.Player; @@ -104,8 +103,6 @@ public class AsynchronousUnregisterTest { given(passwordSecurity.comparePassword(userPassword, password, name)).willReturn(true); given(dataSource.removeAuth(name)).willReturn(true); given(service.getProperty(RegistrationSettings.FORCE)).willReturn(true); - given(service.getProperty(RegistrationSettings.APPLY_BLIND_EFFECT)).willReturn(true); - given(service.getProperty(RestrictionSettings.TIMEOUT)).willReturn(12); // when asynchronousUnregister.unregister(player, userPassword); @@ -117,7 +114,7 @@ public class AsynchronousUnregisterTest { verify(playerCache).removePlayer(name); verify(teleportationService).teleportOnJoin(player); verify(authGroupHandler).setGroup(player, AuthGroupType.UNREGISTERED); - verify(bukkitService).runTask(any(Runnable.class)); + verify(bukkitService).scheduleSyncTaskFromOptionallyAsyncTask(any(Runnable.class)); } @Test @@ -135,8 +132,6 @@ public class AsynchronousUnregisterTest { given(passwordSecurity.comparePassword(userPassword, password, name)).willReturn(true); given(dataSource.removeAuth(name)).willReturn(true); given(service.getProperty(RegistrationSettings.FORCE)).willReturn(true); - given(service.getProperty(RegistrationSettings.APPLY_BLIND_EFFECT)).willReturn(true); - given(service.getProperty(RestrictionSettings.TIMEOUT)).willReturn(0); // when asynchronousUnregister.unregister(player, userPassword); @@ -148,7 +143,7 @@ public class AsynchronousUnregisterTest { verify(playerCache).removePlayer(name); verify(teleportationService).teleportOnJoin(player); verify(authGroupHandler).setGroup(player, AuthGroupType.UNREGISTERED); - verify(bukkitService).runTask(any(Runnable.class)); + verify(bukkitService).scheduleSyncTaskFromOptionallyAsyncTask(any(Runnable.class)); } @Test @@ -237,8 +232,6 @@ public class AsynchronousUnregisterTest { given(player.isOnline()).willReturn(true); given(dataSource.removeAuth(name)).willReturn(true); given(service.getProperty(RegistrationSettings.FORCE)).willReturn(true); - given(service.getProperty(RegistrationSettings.APPLY_BLIND_EFFECT)).willReturn(true); - given(service.getProperty(RestrictionSettings.TIMEOUT)).willReturn(12); CommandSender initiator = mock(CommandSender.class); // when @@ -251,7 +244,7 @@ public class AsynchronousUnregisterTest { verify(playerCache).removePlayer(name); verify(teleportationService).teleportOnJoin(player); verify(authGroupHandler).setGroup(player, AuthGroupType.UNREGISTERED); - verify(bukkitService).runTask(any(Runnable.class)); + verify(bukkitService).scheduleSyncTaskFromOptionallyAsyncTask(any(Runnable.class)); } @Test diff --git a/src/test/java/fr/xephi/authme/task/LimboPlayerTaskManagerTest.java b/src/test/java/fr/xephi/authme/task/LimboPlayerTaskManagerTest.java index 5335da29..0d3d8f71 100644 --- a/src/test/java/fr/xephi/authme/task/LimboPlayerTaskManagerTest.java +++ b/src/test/java/fr/xephi/authme/task/LimboPlayerTaskManagerTest.java @@ -20,6 +20,11 @@ import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; import static fr.xephi.authme.service.BukkitService.TICKS_PER_SECOND; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.nullValue; +import static org.hamcrest.Matchers.sameInstance; +import static org.junit.Assert.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; @@ -112,9 +117,9 @@ public class LimboPlayerTaskManagerTest { @Test public void shouldCancelExistingMessageTask() { // given - LimboPlayer limboPlayer = mock(LimboPlayer.class); + LimboPlayer limboPlayer = new LimboPlayer(null, true, "grp", false, 0.1f, 0.0f); MessageTask existingMessageTask = mock(MessageTask.class); - given(limboPlayer.getMessageTask()).willReturn(existingMessageTask); + limboPlayer.setMessageTask(existingMessageTask); String name = "bobby"; given(limboService.getLimboPlayer(name)).willReturn(limboPlayer); @@ -124,7 +129,8 @@ public class LimboPlayerTaskManagerTest { limboPlayerTaskManager.registerMessageTask(name, false); // then - verify(limboPlayer).setMessageTask(any(MessageTask.class)); + assertThat(limboPlayer.getMessageTask(), not(nullValue())); + assertThat(limboPlayer.getMessageTask(), not(sameInstance(existingMessageTask))); verify(messages).retrieve(MessageKey.REGISTER_MESSAGE); verify(existingMessageTask).cancel(); } @@ -186,9 +192,9 @@ public class LimboPlayerTaskManagerTest { String name = "l33tPlayer"; Player player = mock(Player.class); given(player.getName()).willReturn(name); - LimboPlayer limboPlayer = mock(LimboPlayer.class); + LimboPlayer limboPlayer = new LimboPlayer(null, false, "", true, 0.3f, 0.1f); BukkitTask existingTask = mock(BukkitTask.class); - given(limboPlayer.getTimeoutTask()).willReturn(existingTask); + limboPlayer.setTimeoutTask(existingTask); given(limboService.getLimboPlayer(name)).willReturn(limboPlayer); given(settings.getProperty(RestrictionSettings.TIMEOUT)).willReturn(18); BukkitTask bukkitTask = mock(BukkitTask.class); @@ -199,7 +205,7 @@ public class LimboPlayerTaskManagerTest { // then verify(existingTask).cancel(); - verify(limboPlayer).setTimeoutTask(bukkitTask); + assertThat(limboPlayer.getTimeoutTask(), equalTo(bukkitTask)); verify(bukkitService).runTaskLater(any(TimeoutTask.class), eq(360L)); // 18 * TICKS_PER_SECOND verify(messages).retrieveSingle(MessageKey.LOGIN_TIMEOUT_ERROR); }