From 2a4cda070922d604ab9a629dea438f39c3a268b0 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sun, 17 Jul 2016 11:54:22 +0200 Subject: [PATCH] #784 Perform bypass purge permission check with OfflinePlayer objects - Move permission check inside PurgeTask to perform it with OfflinePlayer objects instead of lowercase names - Move purge members into child "purge" package - Unify online and offline default permission behavior in DefaultPermission --- src/main/java/fr/xephi/authme/AuthMe.java | 2 +- .../authme/PurgeBannedPlayersCommand.java | 2 +- .../executable/authme/PurgeCommand.java | 2 +- .../authme/permission/DefaultPermission.java | 36 ++-------- .../authme/permission/PermissionsManager.java | 24 +++++-- .../handlers/BPermissionsHandler.java | 2 +- .../handlers/GroupManagerHandler.java | 2 +- .../handlers/PermissionHandler.java | 2 +- .../handlers/PermissionsBukkitHandler.java | 2 +- .../handlers/PermissionsExHandler.java | 2 +- .../permission/handlers/VaultHandler.java | 2 +- .../handlers/ZPermissionsHandler.java | 2 +- .../authme/task/{ => purge}/PurgeService.java | 70 ++++--------------- .../authme/task/{ => purge}/PurgeTask.java | 42 ++++++++--- .../authme/AuthMeInitializationTest.java | 4 +- .../authme/PurgeBannedPlayersCommandTest.java | 2 +- .../executable/authme/PurgeCommandTest.java | 2 +- .../task/{ => purge}/PurgeServiceTest.java | 51 +++++--------- 18 files changed, 102 insertions(+), 149 deletions(-) rename src/main/java/fr/xephi/authme/task/{ => purge}/PurgeService.java (79%) rename src/main/java/fr/xephi/authme/task/{ => purge}/PurgeTask.java (66%) rename src/test/java/fr/xephi/authme/task/{ => purge}/PurgeServiceTest.java (82%) diff --git a/src/main/java/fr/xephi/authme/AuthMe.java b/src/main/java/fr/xephi/authme/AuthMe.java index 6c86945c8..bb9d06a67 100644 --- a/src/main/java/fr/xephi/authme/AuthMe.java +++ b/src/main/java/fr/xephi/authme/AuthMe.java @@ -47,7 +47,7 @@ import fr.xephi.authme.settings.properties.RestrictionSettings; import fr.xephi.authme.settings.properties.SecuritySettings; import fr.xephi.authme.settings.properties.SettingsFieldRetriever; import fr.xephi.authme.settings.propertymap.PropertyMap; -import fr.xephi.authme.task.PurgeService; +import fr.xephi.authme.task.purge.PurgeService; import fr.xephi.authme.util.BukkitService; import fr.xephi.authme.util.FileUtils; import fr.xephi.authme.util.GeoLiteAPI; diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/PurgeBannedPlayersCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/PurgeBannedPlayersCommand.java index 30e3bd530..b4e9e545e 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/PurgeBannedPlayersCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/PurgeBannedPlayersCommand.java @@ -1,7 +1,7 @@ package fr.xephi.authme.command.executable.authme; import fr.xephi.authme.command.ExecutableCommand; -import fr.xephi.authme.task.PurgeService; +import fr.xephi.authme.task.purge.PurgeService; import fr.xephi.authme.util.BukkitService; import org.bukkit.OfflinePlayer; import org.bukkit.command.CommandSender; diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/PurgeCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/PurgeCommand.java index 1885dc76c..21552d81f 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/PurgeCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/PurgeCommand.java @@ -1,7 +1,7 @@ package fr.xephi.authme.command.executable.authme; import fr.xephi.authme.command.ExecutableCommand; -import fr.xephi.authme.task.PurgeService; +import fr.xephi.authme.task.purge.PurgeService; import org.bukkit.ChatColor; import org.bukkit.command.CommandSender; diff --git a/src/main/java/fr/xephi/authme/permission/DefaultPermission.java b/src/main/java/fr/xephi/authme/permission/DefaultPermission.java index d4b542cd9..96e979a6d 100644 --- a/src/main/java/fr/xephi/authme/permission/DefaultPermission.java +++ b/src/main/java/fr/xephi/authme/permission/DefaultPermission.java @@ -1,6 +1,6 @@ package fr.xephi.authme.permission; -import org.bukkit.command.CommandSender; +import org.bukkit.permissions.ServerOperator; /** * The default permission to fall back to if there is no support for permission nodes. @@ -10,12 +10,7 @@ public enum DefaultPermission { /** No one has permission. */ NOT_ALLOWED("No permission") { @Override - public boolean evaluate(CommandSender sender) { - return false; - } - - @Override - public boolean evaluateOffline(String name) { + public boolean evaluate(ServerOperator sender) { return false; } }, @@ -23,26 +18,15 @@ public enum DefaultPermission { /** Only players with OP status have permission. */ OP_ONLY("OP's only") { @Override - public boolean evaluate(CommandSender sender) { - return sender.isOp(); - } - - @Override - public boolean evaluateOffline(String name) { - // TODO #784: Check if there is an elegant way to evaluate OP status - return false; + public boolean evaluate(ServerOperator sender) { + return sender != null && sender.isOp(); } }, /** Everyone is granted permission. */ ALLOWED("Everyone allowed") { @Override - public boolean evaluate(CommandSender sender) { - return true; - } - - @Override - public boolean evaluateOffline(String name) { + public boolean evaluate(ServerOperator sender) { return true; } }; @@ -64,15 +48,7 @@ public enum DefaultPermission { * @param sender the sender to process * @return true if the sender has permission, false otherwise */ - public abstract boolean evaluate(CommandSender sender); - - /** - * Evaluate whether permission is granted to an offline user. - * - * @param name The name to check - * @return True if the user has permission, false otherwise - */ - public abstract boolean evaluateOffline(String name); + public abstract boolean evaluate(ServerOperator sender); /** * Return the textual representation. diff --git a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java index efa1c66eb..52ec20dbf 100644 --- a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java +++ b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java @@ -12,6 +12,7 @@ import fr.xephi.authme.permission.handlers.VaultHandler; import fr.xephi.authme.permission.handlers.ZPermissionsHandler; import fr.xephi.authme.util.StringUtils; import org.anjocaido.groupmanager.GroupManager; +import org.bukkit.OfflinePlayer; import org.bukkit.Server; import org.bukkit.command.CommandSender; import org.bukkit.entity.Player; @@ -210,22 +211,33 @@ public class PermissionsManager implements Reloadable { * Check if a player has permission for the given permission node. This is for offline player checks. If no permissions * system is used, then the player will not have permission. * - * @param name The name of the player. - * @param permissionNode The permission node to verify. + * @param player The offline player + * @param permissionNode The permission node to verify * - * @return + * @return true if the player has permission, false otherwise */ - public boolean hasPermissionOffline(String name, PermissionNode permissionNode) { + public boolean hasPermissionOffline(OfflinePlayer player, PermissionNode permissionNode) { // Check if the permission node is null if (permissionNode == null) { return true; } if (!isEnabled()) { - return permissionNode.getDefaultPermission().evaluateOffline(name); + return permissionNode.getDefaultPermission().evaluate(player); } - return handler.hasPermission(name, permissionNode); + return handler.hasPermissionOffline(player.getName(), permissionNode); + } + + public boolean hasPermissionOffline(String name, PermissionNode permissionNode) { + if (permissionNode == null) { + return true; + } + if (!isEnabled()) { + return permissionNode.getDefaultPermission().evaluate(null); + } + + return handler.hasPermissionOffline(name, permissionNode); } /** diff --git a/src/main/java/fr/xephi/authme/permission/handlers/BPermissionsHandler.java b/src/main/java/fr/xephi/authme/permission/handlers/BPermissionsHandler.java index 71a6772b8..c448d7b7c 100644 --- a/src/main/java/fr/xephi/authme/permission/handlers/BPermissionsHandler.java +++ b/src/main/java/fr/xephi/authme/permission/handlers/BPermissionsHandler.java @@ -28,7 +28,7 @@ public class BPermissionsHandler implements PermissionHandler { } @Override - public boolean hasPermission(String name, PermissionNode node) { + public boolean hasPermissionOffline(String name, PermissionNode node) { return ApiLayer.hasPermission(null, CalculableType.USER, name, node.getNode()); } diff --git a/src/main/java/fr/xephi/authme/permission/handlers/GroupManagerHandler.java b/src/main/java/fr/xephi/authme/permission/handlers/GroupManagerHandler.java index 383f109bb..d8980f87e 100644 --- a/src/main/java/fr/xephi/authme/permission/handlers/GroupManagerHandler.java +++ b/src/main/java/fr/xephi/authme/permission/handlers/GroupManagerHandler.java @@ -36,7 +36,7 @@ public class GroupManagerHandler implements PermissionHandler { } @Override - public boolean hasPermission(String name, PermissionNode node) { + public boolean hasPermissionOffline(String name, PermissionNode node) { final AnjoPermissionsHandler handler = groupManager.getWorldsHolder().getWorldPermissionsByPlayerName(name); List perms = handler.getAllPlayersPermissions(name); return perms.contains(node.getNode()); diff --git a/src/main/java/fr/xephi/authme/permission/handlers/PermissionHandler.java b/src/main/java/fr/xephi/authme/permission/handlers/PermissionHandler.java index b47261bb6..bf294c522 100644 --- a/src/main/java/fr/xephi/authme/permission/handlers/PermissionHandler.java +++ b/src/main/java/fr/xephi/authme/permission/handlers/PermissionHandler.java @@ -47,7 +47,7 @@ public interface PermissionHandler { * * @return True if the player has permission. */ - boolean hasPermission(String name, PermissionNode node); + boolean hasPermissionOffline(String name, PermissionNode node); /** * Check whether the player is in the specified group. diff --git a/src/main/java/fr/xephi/authme/permission/handlers/PermissionsBukkitHandler.java b/src/main/java/fr/xephi/authme/permission/handlers/PermissionsBukkitHandler.java index bb351c80a..22edea699 100644 --- a/src/main/java/fr/xephi/authme/permission/handlers/PermissionsBukkitHandler.java +++ b/src/main/java/fr/xephi/authme/permission/handlers/PermissionsBukkitHandler.java @@ -26,7 +26,7 @@ public class PermissionsBukkitHandler implements PermissionHandler { } @Override - public boolean hasPermission(String name, PermissionNode node) { + public boolean hasPermissionOffline(String name, PermissionNode node) { return false; } diff --git a/src/main/java/fr/xephi/authme/permission/handlers/PermissionsExHandler.java b/src/main/java/fr/xephi/authme/permission/handlers/PermissionsExHandler.java index c7e40171d..7b1096f9e 100644 --- a/src/main/java/fr/xephi/authme/permission/handlers/PermissionsExHandler.java +++ b/src/main/java/fr/xephi/authme/permission/handlers/PermissionsExHandler.java @@ -44,7 +44,7 @@ public class PermissionsExHandler implements PermissionHandler { } @Override - public boolean hasPermission(String name, PermissionNode node) { + public boolean hasPermissionOffline(String name, PermissionNode node) { PermissionUser user = permissionManager.getUser(name); return user.has(node.getNode()); } diff --git a/src/main/java/fr/xephi/authme/permission/handlers/VaultHandler.java b/src/main/java/fr/xephi/authme/permission/handlers/VaultHandler.java index b680d47e4..f8e322d6b 100644 --- a/src/main/java/fr/xephi/authme/permission/handlers/VaultHandler.java +++ b/src/main/java/fr/xephi/authme/permission/handlers/VaultHandler.java @@ -50,7 +50,7 @@ public class VaultHandler implements PermissionHandler { } @Override - public boolean hasPermission(String name, PermissionNode node) { + public boolean hasPermissionOffline(String name, PermissionNode node) { return vaultProvider.has("", name, node.getNode()); } diff --git a/src/main/java/fr/xephi/authme/permission/handlers/ZPermissionsHandler.java b/src/main/java/fr/xephi/authme/permission/handlers/ZPermissionsHandler.java index 27dca5a3a..b4f3198f2 100644 --- a/src/main/java/fr/xephi/authme/permission/handlers/ZPermissionsHandler.java +++ b/src/main/java/fr/xephi/authme/permission/handlers/ZPermissionsHandler.java @@ -43,7 +43,7 @@ public class ZPermissionsHandler implements PermissionHandler { } @Override - public boolean hasPermission(String name, PermissionNode node) { + public boolean hasPermissionOffline(String name, PermissionNode node) { Map perms = zPermissionsService.getPlayerPermissions(null, null, name); if (perms.containsKey(node.getNode())) return perms.get(node.getNode()); diff --git a/src/main/java/fr/xephi/authme/task/PurgeService.java b/src/main/java/fr/xephi/authme/task/purge/PurgeService.java similarity index 79% rename from src/main/java/fr/xephi/authme/task/PurgeService.java rename to src/main/java/fr/xephi/authme/task/purge/PurgeService.java index 8d30d7a18..103bace53 100644 --- a/src/main/java/fr/xephi/authme/task/PurgeService.java +++ b/src/main/java/fr/xephi/authme/task/purge/PurgeService.java @@ -1,11 +1,9 @@ -package fr.xephi.authme.task; +package fr.xephi.authme.task.purge; import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.hooks.PluginHooks; -import fr.xephi.authme.initialization.Reloadable; import fr.xephi.authme.permission.PermissionsManager; -import fr.xephi.authme.permission.PlayerStatePermission; import fr.xephi.authme.settings.NewSetting; import fr.xephi.authme.settings.properties.PurgeSettings; import fr.xephi.authme.util.BukkitService; @@ -17,16 +15,14 @@ import org.bukkit.Server; import org.bukkit.command.CommandSender; import org.bukkit.command.ConsoleCommandSender; -import javax.annotation.PostConstruct; import javax.inject.Inject; import java.io.File; import java.util.Calendar; -import java.util.HashSet; import java.util.Set; import static fr.xephi.authme.util.StringUtils.makePath; -public class PurgeService implements Reloadable { +public class PurgeService { @Inject private BukkitService bukkitService; @@ -48,17 +44,6 @@ public class PurgeService implements Reloadable { private boolean isPurging = false; - // Settings - private int daysBeforePurge; - - /** - * Return whether a purge is in progress. - * - * @return True if purging. - */ - public boolean isPurging() { - return this.isPurging; - } /** * Set if a purge is currently in progress. @@ -70,9 +55,10 @@ public class PurgeService implements Reloadable { } /** - * Purges players from the database. Run on startup if enabled. + * Purges players from the database. Runs on startup if enabled. */ public void runAutoPurge() { + int daysBeforePurge = settings.getProperty(PurgeSettings.DAYS_BEFORE_REMOVE_PLAYER); if (!settings.getProperty(PurgeSettings.USE_AUTO_PURGE)) { return; } else if (daysBeforePurge <= 0) { @@ -89,29 +75,27 @@ public class PurgeService implements Reloadable { } /** - * Run a purge with a specified time. + * Runs a purge with a specified last login threshold. Players who haven't logged in since the threshold + * will be purged. * - * @param sender Sender running the command. - * @param until The minimum last login. + * @param sender Sender running the command + * @param until The last login threshold in milliseconds */ public void runPurge(CommandSender sender, long until) { //todo: note this should may run async because it may executes a SQL-Query - Set initialPurge = dataSource.getRecordsToPurge(until); - if (CollectionUtils.isEmpty(initialPurge)) { + Set toPurge = dataSource.getRecordsToPurge(until); + if (CollectionUtils.isEmpty(toPurge)) { logAndSendMessage(sender, "No players to purge"); return; } - Set toPurge = getFinalPurgeList(initialPurge); purgePlayers(sender, toPurge, bukkitService.getOfflinePlayers()); } /** - * Purges the given list of player names. + * Purges all banned players. * - * @param sender Sender running the command. - * @param names The names to remove. - * @param players Collection of OfflinePlayers (including those with the given names). + * @param sender Sender running the command */ public void purgePlayers(CommandSender sender, Set names, OfflinePlayer[] players) { //todo: note this should may run async because it may executes a SQL-Query @@ -120,35 +104,17 @@ public class PurgeService implements Reloadable { return; } + // FIXME #784: We can no longer delete records here -> permission check happens inside PurgeTask dataSource.purgeRecords(names); + // TODO ljacqu 20160717: We shouldn't output namedBanned.size() but the actual total that was deleted logAndSendMessage(sender, ChatColor.GOLD + "Deleted " + names.size() + " user accounts"); logAndSendMessage(sender, ChatColor.GOLD + "Purging user accounts..."); isPurging = true; - PurgeTask purgeTask = new PurgeTask(this, sender, names, players); + PurgeTask purgeTask = new PurgeTask(this, permissionsManager, sender, names, players); bukkitService.runTaskAsynchronously(purgeTask); } - /** - * Check each name in the initial purge findings to remove any player from the purge list - * that has the bypass permission. - * - * @param initial The initial list of players to purge. - * - * @return The list of players to purge after permission check. - */ - private Set getFinalPurgeList(Set initial) { - Set toPurge = new HashSet<>(); - - for (String name : initial) { - if (!permissionsManager.hasPermissionOffline(name, PlayerStatePermission.BYPASS_PURGE)) { - toPurge.add(name); - } - } - - return toPurge; - } - synchronized void purgeAntiXray(Set cleared) { if (!settings.getProperty(PurgeSettings.REMOVE_ANTI_XRAY_FILE)) { return; @@ -287,10 +253,4 @@ public class PurgeService implements Reloadable { sender.sendMessage(message); } } - - @PostConstruct - @Override - public void reload() { - this.daysBeforePurge = settings.getProperty(PurgeSettings.DAYS_BEFORE_REMOVE_PLAYER); - } } diff --git a/src/main/java/fr/xephi/authme/task/PurgeTask.java b/src/main/java/fr/xephi/authme/task/purge/PurgeTask.java similarity index 66% rename from src/main/java/fr/xephi/authme/task/PurgeTask.java rename to src/main/java/fr/xephi/authme/task/purge/PurgeTask.java index d06367592..d356fc2ae 100644 --- a/src/main/java/fr/xephi/authme/task/PurgeTask.java +++ b/src/main/java/fr/xephi/authme/task/purge/PurgeTask.java @@ -1,6 +1,8 @@ -package fr.xephi.authme.task; +package fr.xephi.authme.task.purge; import fr.xephi.authme.ConsoleLogger; +import fr.xephi.authme.permission.PermissionsManager; +import fr.xephi.authme.permission.PlayerStatePermission; import org.bukkit.Bukkit; import org.bukkit.ChatColor; import org.bukkit.OfflinePlayer; @@ -15,9 +17,10 @@ import java.util.UUID; public class PurgeTask extends BukkitRunnable { //how many players we should check for each tick - private static final int INTERVALL_CHECK = 5; + private static final int INTERVAL_CHECK = 5; private final PurgeService purgeService; + private final PermissionsManager permissionsManager; private final UUID sender; private final Set toPurge; @@ -26,8 +29,20 @@ public class PurgeTask extends BukkitRunnable { private int currentPage = 0; - public PurgeTask(PurgeService service, CommandSender sender, Set toPurge, OfflinePlayer[] offlinePlayers) { + /** + * Constructor. + * + * @param service the purge service + * @param permissionsManager the permissions manager + * @param sender the sender who initiated the purge, or null + * @param toPurge lowercase names to purge + * @param offlinePlayers offline players to map to the names + */ + public PurgeTask(PurgeService service, PermissionsManager permissionsManager, CommandSender sender, + Set toPurge, OfflinePlayer[] offlinePlayers) { this.purgeService = service; + this.permissionsManager = permissionsManager; + if (sender instanceof Player) { this.sender = ((Player) sender).getUniqueId(); } else { @@ -47,20 +62,21 @@ public class PurgeTask extends BukkitRunnable { return; } - Set playerPortion = new HashSet(INTERVALL_CHECK); - Set namePortion = new HashSet(INTERVALL_CHECK); - for (int i = 0; i < INTERVALL_CHECK; i++) { - int nextPosition = (currentPage * INTERVALL_CHECK) + i; + Set playerPortion = new HashSet<>(INTERVAL_CHECK); + Set namePortion = new HashSet<>(INTERVAL_CHECK); + for (int i = 0; i < INTERVAL_CHECK; i++) { + int nextPosition = (currentPage * INTERVAL_CHECK) + i; if (offlinePlayers.length <= nextPosition) { //no more offline players on this page break; } OfflinePlayer offlinePlayer = offlinePlayers[nextPosition]; - //remove to speed up later lookups if (toPurge.remove(offlinePlayer.getName().toLowerCase())) { - playerPortion.add(offlinePlayer); - namePortion.add(offlinePlayer.getName()); + if (!permissionsManager.hasPermissionOffline(offlinePlayer, PlayerStatePermission.BYPASS_PURGE)) { + playerPortion.add(offlinePlayer); + namePortion.add(offlinePlayer.getName()); + } } } @@ -68,7 +84,11 @@ public class PurgeTask extends BukkitRunnable { ConsoleLogger.info("Finished lookup up offlinePlayers. Begin looking purging player names only"); //we went through all offlineplayers but there are still names remaining - namePortion.addAll(toPurge); + for (String name : toPurge) { + if (!permissionsManager.hasPermissionOffline(name, PlayerStatePermission.BYPASS_PURGE)) { + namePortion.add(name); + } + } toPurge.clear(); } diff --git a/src/test/java/fr/xephi/authme/AuthMeInitializationTest.java b/src/test/java/fr/xephi/authme/AuthMeInitializationTest.java index 722089925..c5c199db5 100644 --- a/src/test/java/fr/xephi/authme/AuthMeInitializationTest.java +++ b/src/test/java/fr/xephi/authme/AuthMeInitializationTest.java @@ -12,7 +12,7 @@ import fr.xephi.authme.process.Management; import fr.xephi.authme.process.login.ProcessSyncPlayerLogin; import fr.xephi.authme.security.PasswordSecurity; import fr.xephi.authme.settings.NewSetting; -import fr.xephi.authme.task.PurgeService; +import fr.xephi.authme.task.purge.PurgeService; import org.bukkit.Bukkit; import org.bukkit.Server; import org.bukkit.plugin.PluginDescriptionFile; @@ -93,7 +93,7 @@ public class AuthMeInitializationTest { // given NewSetting settings = new NewSetting(settingsFile, dataFolder, getAllPropertyFields(), alwaysFulfilled()); - // TODO ljacqu 20160619: At some point setting the "plugin" field should not longer be necessary + // TODO ljacqu 20160619: At some point setting the "plugin" field should no longer be necessary // We only require it right now because of usages of AuthMe#getInstance() ReflectionTestUtils.setField(AuthMe.class, null, "plugin", authMe); diff --git a/src/test/java/fr/xephi/authme/command/executable/authme/PurgeBannedPlayersCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/authme/PurgeBannedPlayersCommandTest.java index 295186cfe..01d9c89d9 100644 --- a/src/test/java/fr/xephi/authme/command/executable/authme/PurgeBannedPlayersCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/authme/PurgeBannedPlayersCommandTest.java @@ -1,6 +1,6 @@ package fr.xephi.authme.command.executable.authme; -import fr.xephi.authme.task.PurgeService; +import fr.xephi.authme.task.purge.PurgeService; import fr.xephi.authme.util.BukkitService; import org.bukkit.OfflinePlayer; import org.bukkit.command.CommandSender; diff --git a/src/test/java/fr/xephi/authme/command/executable/authme/PurgeCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/authme/PurgeCommandTest.java index 05074adf6..68a34db76 100644 --- a/src/test/java/fr/xephi/authme/command/executable/authme/PurgeCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/authme/PurgeCommandTest.java @@ -1,6 +1,6 @@ package fr.xephi.authme.command.executable.authme; -import fr.xephi.authme.task.PurgeService; +import fr.xephi.authme.task.purge.PurgeService; import org.bukkit.command.CommandSender; import org.junit.Test; import org.junit.runner.RunWith; diff --git a/src/test/java/fr/xephi/authme/task/PurgeServiceTest.java b/src/test/java/fr/xephi/authme/task/purge/PurgeServiceTest.java similarity index 82% rename from src/test/java/fr/xephi/authme/task/PurgeServiceTest.java rename to src/test/java/fr/xephi/authme/task/purge/PurgeServiceTest.java index a631476ec..0b80a3936 100644 --- a/src/test/java/fr/xephi/authme/task/PurgeServiceTest.java +++ b/src/test/java/fr/xephi/authme/task/purge/PurgeServiceTest.java @@ -1,14 +1,13 @@ -package fr.xephi.authme.task; +package fr.xephi.authme.task.purge; import fr.xephi.authme.ReflectionTestUtils; import fr.xephi.authme.TestHelper; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.hooks.PluginHooks; import fr.xephi.authme.permission.PermissionsManager; -import fr.xephi.authme.permission.PlayerStatePermission; import fr.xephi.authme.runner.BeforeInjecting; -import fr.xephi.authme.runner.InjectDelayed; import fr.xephi.authme.runner.DelayedInjectionRunner; +import fr.xephi.authme.runner.InjectDelayed; import fr.xephi.authme.settings.NewSetting; import fr.xephi.authme.settings.properties.PurgeSettings; import fr.xephi.authme.util.BukkitService; @@ -32,7 +31,6 @@ import static com.google.common.collect.Sets.newHashSet; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.equalToIgnoringCase; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertThat; @@ -40,7 +38,6 @@ import static org.mockito.BDDMockito.given; import static org.mockito.Matchers.anyLong; import static org.mockito.Matchers.anySet; import static org.mockito.Matchers.argThat; -import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -95,7 +92,6 @@ public class PurgeServiceTest { // given given(settings.getProperty(PurgeSettings.USE_AUTO_PURGE)).willReturn(true); given(settings.getProperty(PurgeSettings.DAYS_BEFORE_REMOVE_PLAYER)).willReturn(0); - purgeService.reload(); // when purgeService.runAutoPurge(); @@ -109,10 +105,9 @@ public class PurgeServiceTest { // given given(settings.getProperty(PurgeSettings.USE_AUTO_PURGE)).willReturn(true); given(settings.getProperty(PurgeSettings.DAYS_BEFORE_REMOVE_PLAYER)).willReturn(60); - String[] playerNames = {"alpha", "bravo", "charlie", "delta"}; - given(dataSource.getRecordsToPurge(anyLong())).willReturn(newHashSet(playerNames)); + Set playerNames = newHashSet("alpha", "bravo", "charlie", "delta"); + given(dataSource.getRecordsToPurge(anyLong())).willReturn(playerNames); mockReturnedOfflinePlayers(); - mockHasBypassPurgePermission("bravo", "delta"); // when purgeService.runAutoPurge(); @@ -121,13 +116,14 @@ public class PurgeServiceTest { ArgumentCaptor captor = ArgumentCaptor.forClass(Long.class); verify(dataSource).getRecordsToPurge(captor.capture()); assertCorrectPurgeTimestamp(captor.getValue(), 60); - verify(dataSource).purgeRecords(newHashSet("alpha", "charlie")); - assertThat(purgeService.isPurging(), equalTo(true)); - verifyScheduledPurgeTask(null, "alpha", "charlie"); + verify(dataSource).purgeRecords(playerNames); + assertThat(Boolean.TRUE.equals( + ReflectionTestUtils.getFieldValue(PurgeService.class, purgeService, "isPurging")), equalTo(true)); + verifyScheduledPurgeTask(null, playerNames); } - @SuppressWarnings("unchecked") @Test + @SuppressWarnings("unchecked") public void shouldRecognizeNoPlayersToPurge() { // given long delay = 123012301L; @@ -148,9 +144,9 @@ public class PurgeServiceTest { public void shouldRunPurge() { // given long delay = 1809714L; - given(dataSource.getRecordsToPurge(delay)).willReturn(newHashSet("charlie", "delta", "echo", "foxtrot")); + Set playerNames = newHashSet("charlie", "delta", "echo", "foxtrot"); + given(dataSource.getRecordsToPurge(delay)).willReturn(playerNames); mockReturnedOfflinePlayers(); - mockHasBypassPurgePermission("echo"); Player sender = mock(Player.class); UUID uuid = UUID.randomUUID(); given(sender.getUniqueId()).willReturn(uuid); @@ -160,13 +156,14 @@ public class PurgeServiceTest { // then verify(dataSource).getRecordsToPurge(delay); - verify(dataSource).purgeRecords(newHashSet("charlie", "delta", "foxtrot")); - verify(sender).sendMessage(argThat(containsString("Deleted 3 user accounts"))); - verifyScheduledPurgeTask(uuid, "charlie", "delta", "foxtrot"); + verify(dataSource).purgeRecords(playerNames); + // FIXME #784: Deleting accounts needs to be handled differently + verify(sender).sendMessage(argThat(containsString("Deleted 4 user accounts"))); + verifyScheduledPurgeTask(uuid, playerNames); } @Test - public void shouldRunPurgeIfProcessIsAlreadyRunning() { + public void shouldNotRunPurgeIfProcessIsAlreadyRunning() { // given purgeService.setPurging(true); CommandSender sender = mock(CommandSender.class); @@ -198,18 +195,6 @@ public class PurgeServiceTest { return players; } - /** - * Mocks the permission manager to say that the given names have the bypass purge permission. - * - * @param names the names - */ - private void mockHasBypassPurgePermission(String... names) { - for (String name : names) { - given(permissionsManager.hasPermissionOffline( - argThat(equalToIgnoringCase(name)), eq(PlayerStatePermission.BYPASS_PURGE))).willReturn(true); - } - } - private void assertCorrectPurgeTimestamp(long timestamp, int configuredDays) { final long toleranceMillis = 100L; Calendar cal = Calendar.getInstance(); @@ -221,7 +206,7 @@ public class PurgeServiceTest { } @SuppressWarnings("unchecked") - private void verifyScheduledPurgeTask(UUID uuid, String... names) { + private void verifyScheduledPurgeTask(UUID uuid, Set names) { ArgumentCaptor captor = ArgumentCaptor.forClass(PurgeTask.class); verify(bukkitService).runTaskAsynchronously(captor.capture()); PurgeTask task = captor.getValue(); @@ -229,6 +214,6 @@ public class PurgeServiceTest { Object senderInTask = ReflectionTestUtils.getFieldValue(PurgeTask.class, task, "sender"); Set namesInTask = (Set) ReflectionTestUtils.getFieldValue(PurgeTask.class, task, "toPurge"); assertThat(senderInTask, Matchers.equalTo(uuid)); - assertThat(namesInTask, containsInAnyOrder(names)); + assertThat(namesInTask, containsInAnyOrder(names.toArray())); } }