From 38cc217cff374e3a33bf3d61b7c794d936c0f148 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 21 Nov 2015 07:41:56 +0100 Subject: [PATCH 01/26] Revert certain JavaDoc changes Ideally JavaDoc should provide additional information to the developer as to the method's purpose and usage. Typically you do not add the return type of the method and the parameter's types since this can be seen in the code. A short description of what the parameter really is (e.g. a String can hold many types of information) is a lot more beneficial. A JavaDoc statement simply restating the parameter types and the method name is, put bluntly, simply noise, since all of these things are already contained in the code itself. Similarly, @see references are great for pointing to other, related methods but aren't very helpful to point to a superclass method (the implemented or overriden method) since it is implied by @Override. A developer can navigate easily to the superclass method with any reasonable IDE. --- pom.xml | 2 +- src/main/java/fr/xephi/authme/AuthMe.java | 118 ++++-------------- .../java/fr/xephi/authme/Log4JFilter.java | 56 +-------- 3 files changed, 28 insertions(+), 148 deletions(-) diff --git a/pom.xml b/pom.xml index 93fdb5f2..1a9a6439 100644 --- a/pom.xml +++ b/pom.xml @@ -606,7 +606,7 @@ true - + junit junit diff --git a/src/main/java/fr/xephi/authme/AuthMe.java b/src/main/java/fr/xephi/authme/AuthMe.java index 908dacc2..29d7091c 100644 --- a/src/main/java/fr/xephi/authme/AuthMe.java +++ b/src/main/java/fr/xephi/authme/AuthMe.java @@ -60,6 +60,7 @@ import net.milkbowl.vault.permission.Permission; import net.minelink.ctplus.CombatTagPlus; /** + * The AuthMe main class. */ public class AuthMe extends JavaPlugin { @@ -506,8 +507,8 @@ public class AuthMe extends JavaPlugin { /** * Get the permissions manager instance. * - - * @return Permissions Manager instance. */ + * @return Permissions Manager instance. + */ public PermissionsManager getPermissionsManager() { return this.permsMan; } @@ -649,10 +650,6 @@ public class AuthMe extends JavaPlugin { } // Save Player Data - /** - * Method savePlayer. - * @param player Player - */ public void savePlayer(Player player) { if ((Utils.isNPC(player)) || (Utils.isUnrestricted(player))) { return; @@ -681,11 +678,6 @@ public class AuthMe extends JavaPlugin { } // Select the player to kick when a vip player join the server when full - /** - * Method generateKickPlayer. - * @param collection Collection - - * @return Player */ public Player generateKickPlayer(Collection collection) { Player player = null; for (Player p : collection) { @@ -730,11 +722,6 @@ public class AuthMe extends JavaPlugin { } // Return the spawn location of a player - /** - * Method getSpawnLocation. - * @param player Player - - * @return Location */ public Location getSpawnLocation(Player player) { World world = player.getWorld(); String[] spawnPriority = Settings.spawnPriority.split(","); @@ -757,21 +744,11 @@ public class AuthMe extends JavaPlugin { } // Return the default spawnpoint of a world - /** - * Method getDefaultSpawn. - * @param world World - - * @return Location */ private Location getDefaultSpawn(World world) { return world.getSpawnLocation(); } // Return the multiverse spawnpoint of a world - /** - * Method getMultiverseSpawn. - * @param world World - - * @return Location */ private Location getMultiverseSpawn(World world) { if (multiverse != null && Settings.multiverse) { try { @@ -784,10 +761,6 @@ public class AuthMe extends JavaPlugin { } // Return the essentials spawnpoint - /** - * Method getEssentialsSpawn. - - * @return Location */ private Location getEssentialsSpawn() { if (essentialsSpawn != null) { return essentialsSpawn; @@ -796,11 +769,6 @@ public class AuthMe extends JavaPlugin { } // Return the authme soawnpoint - /** - * Method getAuthMeSpawn. - * @param player Player - - * @return Location */ private Location getAuthMeSpawn(Player player) { if ((!database.isAuthAvailable(player.getName().toLowerCase()) || !player.hasPlayedBefore()) && (Spawn.getInstance().getFirstSpawn() != null)) { return Spawn.getInstance().getFirstSpawn(); @@ -811,19 +779,11 @@ public class AuthMe extends JavaPlugin { return player.getWorld().getSpawnLocation(); } - /** - * Method switchAntiBotMod. - * @param mode boolean - */ public void switchAntiBotMod(boolean mode) { this.antibotMod = mode; Settings.switchAntiBotMod(mode); } - /** - * Method getAntiBotModMode. - - * @return boolean */ public boolean getAntiBotModMode() { return this.antibotMod; } @@ -850,12 +810,7 @@ public class AuthMe extends JavaPlugin { }, 1, 1200 * Settings.delayRecall); } - /** - * Method replaceAllInfos. - * @param message String - * @param player Player - - * @return String */ + public String replaceAllInfos(String message, Player player) { int playersOnline = Utils.getOnlinePlayers().size(); message = message.replace("&", "\u00a7"); @@ -871,11 +826,7 @@ public class AuthMe extends JavaPlugin { return message; } - /** - * Method getIP. - * @param player Player - - * @return String */ + public String getIP(Player player) { String name = player.getName().toLowerCase(); String ip = player.getAddress().getAddress().getHostAddress(); @@ -889,12 +840,7 @@ public class AuthMe extends JavaPlugin { return ip; } - /** - * Method isLoggedIp. - * @param name String - * @param ip String - - * @return boolean */ + public boolean isLoggedIp(String name, String ip) { int count = 0; for (Player player : Utils.getOnlinePlayers()) { @@ -904,12 +850,7 @@ public class AuthMe extends JavaPlugin { return count >= Settings.getMaxLoginPerIp; } - /** - * Method hasJoinedIp. - * @param name String - * @param ip String - - * @return boolean */ + public boolean hasJoinedIp(String name, String ip) { int count = 0; for (Player player : Utils.getOnlinePlayers()) { @@ -919,21 +860,18 @@ public class AuthMe extends JavaPlugin { return count >= Settings.getMaxJoinPerIp; } - /** - * Method getModuleManager. - - * @return ModuleManager */ + public ModuleManager getModuleManager() { return moduleManager; } /** - * Get Player real IP through VeryGames method + * Gets a player's real IP through VeryGames method. * - * @param player - * player - - * @return String */ + * @param player the player to process + * + * @return the real IP of the player + */ @Deprecated public String getVeryGamesIP(Player player) { String realIP = player.getAddress().getAddress().getHostAddress(); @@ -952,31 +890,19 @@ public class AuthMe extends JavaPlugin { return realIP; } - /** - * Method getCountryCode. - * @param ip String - - * @return String */ + @Deprecated public String getCountryCode(String ip) { return Utils.getCountryCode(ip); } - /** - * Method getCountryName. - * @param ip String - - * @return String */ + @Deprecated public String getCountryName(String ip) { return Utils.getCountryName(ip); } - /** - * Get the command handler instance. - * - - * @return Command handler. */ + public CommandHandler getCommandHandler() { return this.commandHandler; } @@ -993,9 +919,7 @@ public class AuthMe extends JavaPlugin { * @param args * The command arguments (Bukkit). * - - - * @return True if the command was executed, false otherwise. * @see org.bukkit.command.CommandExecutor#onCommand(CommandSender, Command, String, String[]) * @see org.bukkit.command.CommandExecutor#onCommand(CommandSender, Command, String, String[]) + * @return True if the command was executed, false otherwise. */ @Override public boolean onCommand(CommandSender sender, Command cmd, @@ -1012,9 +936,9 @@ public class AuthMe extends JavaPlugin { /** * Get the current installed AuthMeReloaded version name. * - * @return The version name of the currently installed AuthMeReloaded - * instance. */ + * instance. + */ public static String getVersionName() { return PLUGIN_VERSION_NAME; } @@ -1022,9 +946,9 @@ public class AuthMe extends JavaPlugin { /** * Get the current installed AuthMeReloaded version code. * - * @return The version code of the currently installed AuthMeReloaded - * instance. */ + * instance. + */ public static int getVersionCode() { return PLUGIN_VERSION_CODE; } diff --git a/src/main/java/fr/xephi/authme/Log4JFilter.java b/src/main/java/fr/xephi/authme/Log4JFilter.java index 5eebc7cb..917a7536 100644 --- a/src/main/java/fr/xephi/authme/Log4JFilter.java +++ b/src/main/java/fr/xephi/authme/Log4JFilter.java @@ -24,12 +24,6 @@ public class Log4JFilter implements org.apache.logging.log4j.core.Filter { public Log4JFilter() { } - /** - * Method filter. - * @param record LogEvent - - - * @return Result * @see org.apache.logging.log4j.core.Filter#filter(LogEvent) */ @Override public Result filter(LogEvent record) { if (record == null) { @@ -38,32 +32,12 @@ public class Log4JFilter implements org.apache.logging.log4j.core.Filter { return validateMessage(record.getMessage()); } - /** - * Method filter. - * @param arg0 Logger - * @param arg1 Level - * @param arg2 Marker - * @param message String - * @param arg4 Object[] - - - * @return Result * @see org.apache.logging.log4j.core.Filter#filter(Logger, Level, Marker, String, Object[]) */ @Override public Result filter(Logger arg0, Level arg1, Marker arg2, String message, Object... arg4) { return validateMessage(message); } - /** - * Method filter. - * @param arg0 Logger - * @param arg1 Level - * @param arg2 Marker - * @param message Object - * @param arg4 Throwable - - - * @return Result * @see org.apache.logging.log4j.core.Filter#filter(Logger, Level, Marker, Object, Throwable) */ @Override public Result filter(Logger arg0, Level arg1, Marker arg2, Object message, Throwable arg4) { @@ -73,37 +47,17 @@ public class Log4JFilter implements org.apache.logging.log4j.core.Filter { return validateMessage(message.toString()); } - /** - * Method filter. - * @param arg0 Logger - * @param arg1 Level - * @param arg2 Marker - * @param message Message - * @param arg4 Throwable - - - * @return Result * @see org.apache.logging.log4j.core.Filter#filter(Logger, Level, Marker, Message, Throwable) */ @Override public Result filter(Logger arg0, Level arg1, Marker arg2, Message message, Throwable arg4) { return validateMessage(message); } - /** - * Method getOnMatch. - - - * @return Result * @see org.apache.logging.log4j.core.Filter#getOnMatch() */ @Override public Result getOnMatch() { return Result.NEUTRAL; } - /** - * Method getOnMismatch. - - - * @return Result * @see org.apache.logging.log4j.core.Filter#getOnMismatch() */ @Override public Result getOnMismatch() { return Result.NEUTRAL; @@ -115,8 +69,9 @@ public class Log4JFilter implements org.apache.logging.log4j.core.Filter { * data. * * @param message the Message object to verify - - * @return the Result value */ + * + * @return the Result value + */ private static Result validateMessage(Message message) { if (message == null) { return Result.NEUTRAL; @@ -129,8 +84,9 @@ public class Log4JFilter implements org.apache.logging.log4j.core.Filter { * depending on whether the message contains sensitive AuthMe data. * * @param message the message to verify - - * @return the Result value */ + * + * @return the Result value + */ private static Result validateMessage(String message) { if (message == null) { return Result.NEUTRAL; From 9a68aa5517afa61efdaec465808e219c1ccaf24d Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 21 Nov 2015 08:28:53 +0100 Subject: [PATCH 02/26] Proper Javadoc example / add test for StringUtils - Proper example for the purpose of javadoc and how it could look like - Fix containsAny to be null safe - Add tests --- .../fr/xephi/authme/util/StringUtils.java | 26 ++++++----- .../fr/xephi/authme/util/StringUtilsTest.java | 46 +++++++++++++++++++ 2 files changed, 61 insertions(+), 11 deletions(-) create mode 100644 src/test/java/fr/xephi/authme/util/StringUtilsTest.java diff --git a/src/main/java/fr/xephi/authme/util/StringUtils.java b/src/main/java/fr/xephi/authme/util/StringUtils.java index c84fc74a..e9b495bc 100644 --- a/src/main/java/fr/xephi/authme/util/StringUtils.java +++ b/src/main/java/fr/xephi/authme/util/StringUtils.java @@ -5,17 +5,18 @@ import net.ricecode.similarity.StringSimilarityService; import net.ricecode.similarity.StringSimilarityServiceImpl; /** + * Utility class for String operations. */ public class StringUtils { /** * Get the difference of two strings. * - * @param first First string. - * @param second Second string. + * @param first First string + * @param second Second string * - - * @return The difference value. */ + * @return The difference value + */ public static double getDifference(String first, String second) { // Make sure the strings are valid. if(first == null || second == null) @@ -27,22 +28,25 @@ public class StringUtils { // Determine the difference value, return the result return Math.abs(service.score(first, second) - 1.0); } - + /** - * Method containsAny. - * @param str String - * @param pieces String[] - - * @return boolean */ + * Returns whether the given string contains any of the provided elements. + * + * @param str the string to analyze + * @param pieces the items to check the string for + * + * @return true if the string contains at least one of the items + */ public static boolean containsAny(String str, String... pieces) { if (str == null) { return false; } for (String piece : pieces) { - if (str.contains(piece)) { + if (piece != null && str.contains(piece)) { return true; } } return false; } + } diff --git a/src/test/java/fr/xephi/authme/util/StringUtilsTest.java b/src/test/java/fr/xephi/authme/util/StringUtilsTest.java new file mode 100644 index 00000000..668f2059 --- /dev/null +++ b/src/test/java/fr/xephi/authme/util/StringUtilsTest.java @@ -0,0 +1,46 @@ +package fr.xephi.authme.util; + +import org.junit.Test; + +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThat; + +/** + * Test for {@link StringUtils}. + */ +public class StringUtilsTest { + + @Test + public void shouldFindContainedItem() { + // given + String text = "This is a test of containsAny()"; + String piece = "test"; + + // when + boolean result = StringUtils.containsAny(text, "some", "words", "that", "do not", "exist", piece); + + // then + assertThat(result, equalTo(true)); + } + + @Test + public void shouldReturnFalseIfNoneFound() { + // given + String text = "This is a test string"; + + // when + boolean result = StringUtils.containsAny(text, "some", "other", "words", null); + + // then + assertThat(result, equalTo(false)); + } + + @Test + public void shouldReturnFalseForNullString() { + // given/when + boolean result = StringUtils.containsAny(null, "some", "words", "to", "check"); + + // then + assertThat(result, equalTo(false)); + } +} From 400d014e7b9fe22fb54e718001904f7bf02295c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20Vis=C3=A9e?= Date: Sat, 21 Nov 2015 14:41:14 +0100 Subject: [PATCH 03/26] Fixed setGroup in PermissionsManager for Essentials Group Manager and zPermissions --- .../xephi/authme/permission/PermissionsManager.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java index 104a986c..45f4e8cd 100644 --- a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java +++ b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java @@ -480,16 +480,17 @@ public class PermissionsManager { case ESSENTIALS_GROUP_MANAGER: // Essentials Group Manager - final AnjoPermissionsHandler handler = groupManagerPerms.getWorldsHolder().getWorldPermissions(player); + /*final AnjoPermissionsHandler handler = groupManagerPerms.getWorldsHolder().getWorldPermissions(player); if(handler == null) - return false; - // TODO: Write proper code here! - //return Arrays.asList(handler.getGroups(player.getName())); + return false;*/ + // Add the user to the group + // TODO: Clear the current list of groups? + return Bukkit.dispatchCommand(Bukkit.getConsoleSender(), "manuadd " + player.getName() + " " + groupName); case Z_PERMISSIONS: //zPermissions - // TODO: Write proper code here! - //return new ArrayList(zPermissionsService.getPlayerGroups(player.getName())); + // Set the players group through the plugin commands + return Bukkit.dispatchCommand(Bukkit.getConsoleSender(), "permissions player " + player.getName() + " setgroup " + groupName); case VAULT: // Vault From 42dee2e10125ab560b16a85a07c10f49829518b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20Vis=C3=A9e?= Date: Sat, 21 Nov 2015 14:44:43 +0100 Subject: [PATCH 04/26] Created addGroup method in permissions manager --- .../authme/permission/PermissionsManager.java | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java index 45f4e8cd..cacf74d2 100644 --- a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java +++ b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java @@ -442,6 +442,63 @@ public class PermissionsManager { } } + /** + * Add the permission group of a player, if supported. + * + * @param player The player + * @param groupName The name of the group. + * + * @return True if succeed, false otherwise. + */ + @SuppressWarnings({"unchecked", "rawtypes", "deprecation"}) + public boolean addGroup(Player player, String groupName) { + if(!isEnabled()) + // No permissions system is used, return false + return false; + + // Set the group the proper way + switch(this.permsType) { + case PERMISSIONS_EX: + // Permissions Ex + PermissionUser user = PermissionsEx.getUser(player); + user.addGroup(groupName); + return true; + + case PERMISSIONS_BUKKIT: + // Permissions Bukkit + // Permissions Bukkit doesn't support groups, return false + return false; + + case B_PERMISSIONS: + // bPermissions + ApiLayer.addGroup(player.getWorld().getName(), CalculableType.USER, player.getName(), groupName); + return true; + + case ESSENTIALS_GROUP_MANAGER: + // Essentials Group Manager + // Add the group to the user using a command + return Bukkit.dispatchCommand(Bukkit.getConsoleSender(), "manuadd " + player.getName() + " " + groupName); + + case Z_PERMISSIONS: + // zPermissions + // Add the group to the user using a command + return Bukkit.dispatchCommand(Bukkit.getConsoleSender(), "permissions player " + player.getName() + " addgroup " + groupName); + + case VAULT: + // Vault + vaultPerms.playerAddGroup(player, groupName); + return true; + + case NONE: + // Not hooked into any permissions system, return false + return false; + + default: + // Something went wrong, return false + return false; + } + } + /** * Set the permission group of a player, if supported. * From f4da63fee6ca04244ef04417aa45485bb8dcee0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20Vis=C3=A9e?= Date: Sat, 21 Nov 2015 14:57:34 +0100 Subject: [PATCH 05/26] Added note to getGroups method in permissions manager for PermissionsBukkit --- .../java/fr/xephi/authme/permission/PermissionsManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java index cacf74d2..55d5dc58 100644 --- a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java +++ b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java @@ -410,7 +410,7 @@ public class PermissionsManager { case PERMISSIONS_BUKKIT: // Permissions Bukkit - // Permissions Bukkit doesn't support group, return an empty list + // FIXME: Add support for this! return new ArrayList<>(); case B_PERMISSIONS: From a6fe728d796ccfd674b458b1e8ead3c89a32b340 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20Vis=C3=A9e?= Date: Sat, 21 Nov 2015 15:00:14 +0100 Subject: [PATCH 06/26] Fixed addGroup method for PermissionsBukkit in permissions manager --- .../java/fr/xephi/authme/permission/PermissionsManager.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java index 55d5dc58..ef7bda49 100644 --- a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java +++ b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java @@ -466,8 +466,8 @@ public class PermissionsManager { case PERMISSIONS_BUKKIT: // Permissions Bukkit - // Permissions Bukkit doesn't support groups, return false - return false; + // Add the group to the user using a command + return Bukkit.dispatchCommand(Bukkit.getConsoleSender(), "permissions player addgroup " + player.getName() + " " + groupName); case B_PERMISSIONS: // bPermissions From b07e4b62ccc04ff18030ad488a11d447a5e80c60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20Vis=C3=A9e?= Date: Sat, 21 Nov 2015 15:07:46 +0100 Subject: [PATCH 07/26] Created addGroups method in permissions manager --- .../authme/permission/PermissionsManager.java | 37 ++++++++++++++++--- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java index ef7bda49..6830622a 100644 --- a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java +++ b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java @@ -339,8 +339,8 @@ public class PermissionsManager { * @return True if the player has permission. */ public boolean hasPermission(Player player, String permsNode, boolean def) { + // If no permissions system is used, return the default value if(!isEnabled()) - // No permissions system is used, return default return def; switch(this.permsType) { @@ -394,12 +394,12 @@ public class PermissionsManager { * * @param player The player. * - * @return Permission groups. + * @return Permission groups, or an empty list if this feature is not supported. */ @SuppressWarnings({"unchecked", "rawtypes", "deprecation"}) public List getGroups(Player player) { + // If no permissions system is used, return an empty list if(!isEnabled()) - // No permissions system is used, return an empty list return new ArrayList<>(); switch(this.permsType) { @@ -449,11 +449,12 @@ public class PermissionsManager { * @param groupName The name of the group. * * @return True if succeed, false otherwise. + * False is also returned if this feature isn't supported for the current permissions system. */ @SuppressWarnings({"unchecked", "rawtypes", "deprecation"}) public boolean addGroup(Player player, String groupName) { + // If no permissions system is used, return false if(!isEnabled()) - // No permissions system is used, return false return false; // Set the group the proper way @@ -499,6 +500,31 @@ public class PermissionsManager { } } + /** + * Add the permission group of a player, if supported. + * + * @param player The player + * @param groupNames The name of the groups to add. + * + * @return True if succeed, false otherwise. + * False is also returned if this feature isn't supported for the current permissions system. + */ + @SuppressWarnings({"unchecked", "rawtypes", "deprecation"}) + public boolean addGroups(Player player, List groupNames) { + // If no permissions system is used, return false + if(!isEnabled()) + return false; + + // Add each group to the user + boolean result = true; + for(String groupName : groupNames) + if(!addGroup(player, groupName)) + result = false; + + // Return the result + return result; + } + /** * Set the permission group of a player, if supported. * @@ -506,11 +532,12 @@ public class PermissionsManager { * @param groupName The name of the group. * * @return True if succeed, false otherwise. + * False is also returned if this feature isn't supported for the current permissions system. */ @SuppressWarnings({"unchecked", "rawtypes", "deprecation"}) public boolean setGroup(Player player, String groupName) { + // If no permissions system is used, return false if(!isEnabled()) - // No permissions system is used, return false return false; // Create a list of group names From bcf4eeab00631133ba2c52d904e8d39973e985a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20Vis=C3=A9e?= Date: Sat, 21 Nov 2015 15:15:04 +0100 Subject: [PATCH 08/26] Created removeGroup method in permissions manager --- .../authme/permission/PermissionsManager.java | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java index 6830622a..0dbfc52f 100644 --- a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java +++ b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java @@ -525,6 +525,64 @@ public class PermissionsManager { return result; } + /** + * Remove the permission group of a player, if supported. + * + * @param player The player + * @param groupName The name of the group. + * + * @return True if succeed, false otherwise. + * False is also returned if this feature isn't supported for the current permissions system. + */ + @SuppressWarnings({"unchecked", "rawtypes", "deprecation"}) + public boolean removeGroup(Player player, String groupName) { + // If no permissions system is used, return false + if(!isEnabled()) + return false; + + // Set the group the proper way + switch(this.permsType) { + case PERMISSIONS_EX: + // Permissions Ex + PermissionUser user = PermissionsEx.getUser(player); + user.removeGroup(groupName); + return true; + + case PERMISSIONS_BUKKIT: + // Permissions Bukkit + // Remove the group to the user using a command + return Bukkit.dispatchCommand(Bukkit.getConsoleSender(), "permissions player removegroup " + player.getName() + " " + groupName); + + case B_PERMISSIONS: + // bPermissions + ApiLayer.removeGroup(player.getWorld().getName(), CalculableType.USER, player.getName(), groupName); + return true; + + case ESSENTIALS_GROUP_MANAGER: + // Essentials Group Manager + // Remove the group to the user using a command + return Bukkit.dispatchCommand(Bukkit.getConsoleSender(), "manudelsub " + player.getName() + " " + groupName); + + case Z_PERMISSIONS: + // zPermissions + // Remove the group to the user using a command + return Bukkit.dispatchCommand(Bukkit.getConsoleSender(), "permissions player " + player.getName() + " removegroup " + groupName); + + case VAULT: + // Vault + vaultPerms.playerRemoveGroup(player, groupName); + return true; + + case NONE: + // Not hooked into any permissions system, return false + return false; + + default: + // Something went wrong, return false + return false; + } + } + /** * Set the permission group of a player, if supported. * From 462a2e9878d159d22af8ace31f8620f2753e5712 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20Vis=C3=A9e?= Date: Sat, 21 Nov 2015 15:16:19 +0100 Subject: [PATCH 09/26] Created removeGroups method in permissions manager --- .../authme/permission/PermissionsManager.java | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java index 0dbfc52f..89e01241 100644 --- a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java +++ b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java @@ -501,7 +501,7 @@ public class PermissionsManager { } /** - * Add the permission group of a player, if supported. + * Add the permission groups of a player, if supported. * * @param player The player * @param groupNames The name of the groups to add. @@ -583,6 +583,31 @@ public class PermissionsManager { } } + /** + * Remove the permission groups of a player, if supported. + * + * @param player The player + * @param groupNames The name of the groups to add. + * + * @return True if succeed, false otherwise. + * False is also returned if this feature isn't supported for the current permissions system. + */ + @SuppressWarnings({"unchecked", "rawtypes", "deprecation"}) + public boolean removeGroups(Player player, List groupNames) { + // If no permissions system is used, return false + if(!isEnabled()) + return false; + + // Add each group to the user + boolean result = true; + for(String groupName : groupNames) + if(!removeGroup(player, groupName)) + result = false; + + // Return the result + return result; + } + /** * Set the permission group of a player, if supported. * From a84e219899a886834d222f88028ebaed3ebeb02f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20Vis=C3=A9e?= Date: Sat, 21 Nov 2015 15:18:01 +0100 Subject: [PATCH 10/26] Fixed minor Essentials Group Manager issue in addGroup method of permissions manager --- .../java/fr/xephi/authme/permission/PermissionsManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java index 89e01241..15d86a66 100644 --- a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java +++ b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java @@ -478,7 +478,7 @@ public class PermissionsManager { case ESSENTIALS_GROUP_MANAGER: // Essentials Group Manager // Add the group to the user using a command - return Bukkit.dispatchCommand(Bukkit.getConsoleSender(), "manuadd " + player.getName() + " " + groupName); + return Bukkit.dispatchCommand(Bukkit.getConsoleSender(), "manuaddsub " + player.getName() + " " + groupName); case Z_PERMISSIONS: // zPermissions From f7f455a56ab49dd7f2488fe2802dac27f5d2b651 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20Vis=C3=A9e?= Date: Sat, 21 Nov 2015 15:24:53 +0100 Subject: [PATCH 11/26] Created setGroups method in permissions manager --- .../authme/permission/PermissionsManager.java | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java index 15d86a66..1edda2ea 100644 --- a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java +++ b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java @@ -610,6 +610,7 @@ public class PermissionsManager { /** * Set the permission group of a player, if supported. + * This clears the current groups of the player. * * @param player The player * @param groupName The name of the group. @@ -675,6 +676,41 @@ public class PermissionsManager { } } + /** + * Set the permission groups of a player, if supported. + * This clears the current groups of the player. + * + * @param player The player + * @param groupNames The name of the groups to set. + * + * @return True if succeed, false otherwise. + * False is also returned if this feature isn't supported for the current permissions system. + */ + @SuppressWarnings({"unchecked", "rawtypes", "deprecation"}) + public boolean setGroups(Player player, List groupNames) { + // If no permissions system is used or if there's no group supplied, return false + if(!isEnabled() || groupNames.size() <= 0) + return false; + + // Set the main group + if(!setGroup(player, groupNames.get(0))) + return false; + + // Add the rest of the groups + boolean result = true; + for(int i = 1; i < groupNames.size(); i++) { + // Get the group name + String groupName = groupNames.get(0); + + // Add this group + if(!addGroup(player, groupName)) + result = false; + } + + // Return the result + return result; + } + public enum PermissionsSystemType { NONE("None"), PERMISSIONS_EX("PermissionsEx"), From 1091db0e15a4f10db2828a7d294e895110035a6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20Vis=C3=A9e?= Date: Sat, 21 Nov 2015 15:34:38 +0100 Subject: [PATCH 12/26] Created removeAllGroups method in permissions manager --- .../authme/permission/PermissionsManager.java | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java index 1edda2ea..11b18475 100644 --- a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java +++ b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java @@ -711,6 +711,28 @@ public class PermissionsManager { return result; } + /** + * Remove all groups of the specified player, if supported. + * Systems like Essentials GroupManager don't allow all groups to be removed from a player, thus the user will stay + * in it's primary group. All the subgroups are removed just fine. + * + * @param player The player to remove all groups from. + * + * @return True if succeed, false otherwise. + * False will also be returned if this feature isn't supported for the used permissions system. + */ + public boolean removeAllGroups(Player player) { + // If no permissions system is used, return false + if(!isEnabled()) + return false; + + // Get a list of current groups + List groupNames = getGroups(player); + + // Remove each group + return removeGroups(player, groupNames); + } + public enum PermissionsSystemType { NONE("None"), PERMISSIONS_EX("PermissionsEx"), From a05a97a0a69f2dea8e70dfd7451c0acbd690ab89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20Vis=C3=A9e?= Date: Sat, 21 Nov 2015 15:38:20 +0100 Subject: [PATCH 13/26] Fixed setGroup method in permissions manager --- .../authme/permission/PermissionsManager.java | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java index 11b18475..240f0381 100644 --- a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java +++ b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java @@ -638,8 +638,8 @@ public class PermissionsManager { case PERMISSIONS_BUKKIT: // Permissions Bukkit - // Permissions Bukkit doesn't support groups, return false - return false; + // Set the user's group using a command + return Bukkit.dispatchCommand(Bukkit.getConsoleSender(), "permissions player setgroup " + player.getName() + " " + groupName); case B_PERMISSIONS: // bPermissions @@ -648,11 +648,8 @@ public class PermissionsManager { case ESSENTIALS_GROUP_MANAGER: // Essentials Group Manager - /*final AnjoPermissionsHandler handler = groupManagerPerms.getWorldsHolder().getWorldPermissions(player); - if(handler == null) - return false;*/ - // Add the user to the group - // TODO: Clear the current list of groups? + // Clear the list of groups, add the player to the specified group afterwards using a command + removeAllGroups(player); return Bukkit.dispatchCommand(Bukkit.getConsoleSender(), "manuadd " + player.getName() + " " + groupName); case Z_PERMISSIONS: @@ -662,7 +659,8 @@ public class PermissionsManager { case VAULT: // Vault - // TODO: Clear the current list of groups? + // Remove all current groups, add the player to the specified group afterwards + removeAllGroups(player); vaultPerms.playerAddGroup(player, groupName); return true; From f8cf9e2e48bcf0d6776d7f9cbc5a0b4c48addb7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20Vis=C3=A9e?= Date: Sat, 21 Nov 2015 15:44:57 +0100 Subject: [PATCH 14/26] Created inGroup method in permissions manager --- .../authme/permission/PermissionsManager.java | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java index 240f0381..098082d8 100644 --- a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java +++ b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java @@ -442,6 +442,62 @@ public class PermissionsManager { } } + /** + * Check whether the player is in the specified group. + * + * @param player The player. + * @param groupName The group name. + * + * @return True if the player is in the specified group, false otherwise. + * False is also returned if groups aren't supported by the used permissions system. + */ + public boolean inGroup(Player player, String groupName) { + // If no permissions system is used, return false + if(!isEnabled()) + return false; + + switch(this.permsType) { + case PERMISSIONS_EX: + // Permissions Ex + PermissionUser user = PermissionsEx.getUser(player); + return user.inGroup(groupName); + + case PERMISSIONS_BUKKIT: + case Z_PERMISSIONS: + // Get the current list of groups + List groupNames = getGroups(player); + + // Check whether the list contains the group name, return the result + for(String entry : groupNames) + if(entry.equals(groupName)) + return true; + return false; + + case B_PERMISSIONS: + // bPermissions + return ApiLayer.hasGroup(player.getWorld().getName(), CalculableType.USER, player.getName(), groupName); + + case ESSENTIALS_GROUP_MANAGER: + // Essentials Group Manager + final AnjoPermissionsHandler handler = groupManagerPerms.getWorldsHolder().getWorldPermissions(player); + if(handler == null) + return false; + return handler.inGroup(player.getName(), groupName); + + case VAULT: + // Vault + return vaultPerms.playerInGroup(player, groupName); + + case NONE: + // Not hooked into any permissions system, return an empty list + return false; + + default: + // Something went wrong, return an empty list to prevent problems + return false; + } + } + /** * Add the permission group of a player, if supported. * From cfaece3eae8737122b77d0ed6229b34fef2f38bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20Vis=C3=A9e?= Date: Sat, 21 Nov 2015 15:46:10 +0100 Subject: [PATCH 15/26] Simplefied some code --- .../java/fr/xephi/authme/permission/PermissionsManager.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java index 098082d8..a66f6dca 100644 --- a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java +++ b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java @@ -480,9 +480,7 @@ public class PermissionsManager { case ESSENTIALS_GROUP_MANAGER: // Essentials Group Manager final AnjoPermissionsHandler handler = groupManagerPerms.getWorldsHolder().getWorldPermissions(player); - if(handler == null) - return false; - return handler.inGroup(player.getName(), groupName); + return handler != null && handler.inGroup(player.getName(), groupName); case VAULT: // Vault From 973c683c90fc943a4de13c23d9feacb83209b74a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20Vis=C3=A9e?= Date: Sat, 21 Nov 2015 15:47:59 +0100 Subject: [PATCH 16/26] Minor fix for PermissionsBukkit support in permissions manager --- .../java/fr/xephi/authme/permission/PermissionsManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java index a66f6dca..890b85e8 100644 --- a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java +++ b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java @@ -415,7 +415,7 @@ public class PermissionsManager { case B_PERMISSIONS: // bPermissions - return Arrays.asList(ApiLayer.getGroups(player.getName(), CalculableType.USER, player.getName())); + return Arrays.asList(ApiLayer.getGroups(player.getWorld().getName(), CalculableType.USER, player.getName())); case ESSENTIALS_GROUP_MANAGER: // Essentials Group Manager From 69d6518b30a8950e0f490e16652c0f724ecf1d95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20Vis=C3=A9e?= Date: Sat, 21 Nov 2015 16:03:00 +0100 Subject: [PATCH 17/26] Improved setGroup method in Utils class, to replace legacy permissions code --- src/main/java/fr/xephi/authme/util/Utils.java | 69 ++++++++++--------- 1 file changed, 36 insertions(+), 33 deletions(-) diff --git a/src/main/java/fr/xephi/authme/util/Utils.java b/src/main/java/fr/xephi/authme/util/Utils.java index 330e8d8e..af15b8e5 100644 --- a/src/main/java/fr/xephi/authme/util/Utils.java +++ b/src/main/java/fr/xephi/authme/util/Utils.java @@ -7,6 +7,7 @@ import fr.xephi.authme.cache.auth.PlayerCache; import fr.xephi.authme.cache.limbo.LimboCache; import fr.xephi.authme.cache.limbo.LimboPlayer; import fr.xephi.authme.events.AuthMeTeleportEvent; +import fr.xephi.authme.permission.PermissionsManager; import fr.xephi.authme.settings.Settings; import org.bukkit.Bukkit; import org.bukkit.GameMode; @@ -125,47 +126,49 @@ public class Utils { * @param group GroupType */ public static void setGroup(Player player, GroupType group) { - if (!Settings.isPermissionCheckEnabled) + if(!Settings.isPermissionCheckEnabled) return; - if (plugin.vaultGroupManagement == null) - return; - String currentGroup; - try { - currentGroup = plugin.vaultGroupManagement.getPrimaryGroup(player); - } catch (UnsupportedOperationException e) { - ConsoleLogger.showError("Your permission plugin (" + plugin.vaultGroupManagement.getName() + ") doesn't support the Group system... unhook!"); - plugin.vaultGroupManagement = null; - return; - } - switch (group) { - case UNREGISTERED: { - plugin.vaultGroupManagement.playerRemoveGroup(player, currentGroup); - plugin.vaultGroupManagement.playerAddGroup(player, Settings.unRegisteredGroup); + + // TODO: Make sure a groups system is used! + + // 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, shutting down."); + assert permsMan != null; + + switch(group) { + case UNREGISTERED: + // Remove the other group type groups, set the current group + permsMan.removeGroups(player, Arrays.asList(Settings.getRegisteredGroup, Settings.getUnloggedinGroup)); + permsMan.addGroup(player, Settings.unRegisteredGroup); break; - } - case REGISTERED: { - plugin.vaultGroupManagement.playerRemoveGroup(player, currentGroup); - plugin.vaultGroupManagement.playerAddGroup(player, Settings.getRegisteredGroup); + + case REGISTERED: + // Remove the other group type groups, set the current group + permsMan.removeGroups(player, Arrays.asList(Settings.unRegisteredGroup, Settings.getUnloggedinGroup)); + permsMan.addGroup(player, Settings.getRegisteredGroup); break; - } - case NOTLOGGEDIN: { - if (!useGroupSystem()) - break; - plugin.vaultGroupManagement.playerRemoveGroup(player, currentGroup); - plugin.vaultGroupManagement.playerAddGroup(player, Settings.getUnloggedinGroup); + + case NOTLOGGEDIN: + // Remove the other group type groups, set the current group + permsMan.removeGroups(player, Arrays.asList(Settings.unRegisteredGroup, Settings.getRegisteredGroup)); + permsMan.addGroup(player, Settings.getUnloggedinGroup); break; - } - case LOGGEDIN: { - if (!useGroupSystem()) - break; + + case LOGGEDIN: + // Get the limbo player data LimboPlayer limbo = LimboCache.getInstance().getLimboPlayer(player.getName().toLowerCase()); - if (limbo == null) + if(limbo == null) break; + + // Get the players group String realGroup = limbo.getGroup(); - plugin.vaultGroupManagement.playerRemoveGroup(player, currentGroup); - plugin.vaultGroupManagement.playerAddGroup(player, realGroup); + + // Remove the other group types groups, set the real group + permsMan.removeGroups(player, Arrays.asList(Settings.unRegisteredGroup, Settings.getRegisteredGroup, Settings.getUnloggedinGroup)); + permsMan.addGroup(player, realGroup); break; - } } } From eaba2765fa2197221365d055388a9d79e4fce931 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20Vis=C3=A9e?= Date: Sat, 21 Nov 2015 16:07:02 +0100 Subject: [PATCH 18/26] Created hasGroupSupport method in permissions manager --- .../authme/permission/PermissionsManager.java | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java index 890b85e8..0b075d45 100644 --- a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java +++ b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java @@ -389,6 +389,40 @@ public class PermissionsManager { } } + /** + * Check whether the current permissions system has group support. + * If no permissions system is hooked, false will be returned. + * + * @return True if the current permissions system supports groups, false otherwise. + */ + public boolean hasGroupSupport() { + // If no permissions system is used, return false + if(!isEnabled()) + return false; + + switch(this.permsType) { + case PERMISSIONS_EX: + case PERMISSIONS_BUKKIT: + case B_PERMISSIONS: + case ESSENTIALS_GROUP_MANAGER: + case Z_PERMISSIONS: + case PERMISSIONS: + return true; + + case VAULT: + // Vault + return vaultPerms.hasGroupSupport(); + + case NONE: + // Not hooked into any permissions system, return false + return false; + + default: + // Something went wrong, return false to prevent problems + return false; + } + } + /** * Get the permission groups of a player, if available. * From 6dc406656312c511fa526936ce37ea47c6e88168 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20Vis=C3=A9e?= Date: Sat, 21 Nov 2015 16:08:41 +0100 Subject: [PATCH 19/26] Added group support check to setGroups method in Utils class --- src/main/java/fr/xephi/authme/util/Utils.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/java/fr/xephi/authme/util/Utils.java b/src/main/java/fr/xephi/authme/util/Utils.java index af15b8e5..278cf41a 100644 --- a/src/main/java/fr/xephi/authme/util/Utils.java +++ b/src/main/java/fr/xephi/authme/util/Utils.java @@ -129,14 +129,16 @@ public class Utils { if(!Settings.isPermissionCheckEnabled) return; - // TODO: Make sure a groups system is used! - // 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, shutting down."); assert permsMan != null; + // Make sure group support is available + if(!permsMan.hasGroupSupport()) + ConsoleLogger.showError("The current permissions system doesn't have group support, unable to set group!"); + switch(group) { case UNREGISTERED: // Remove the other group type groups, set the current group From bb22daab336384e520b9cf5c8e8893811797b380 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20Vis=C3=A9e?= Date: Sat, 21 Nov 2015 16:21:38 +0100 Subject: [PATCH 20/26] Added some missing support for Nijikokun's Permissions in permissions manager --- .../authme/permission/PermissionsManager.java | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java index 0b075d45..b0e8700d 100644 --- a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java +++ b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java @@ -1,5 +1,6 @@ package fr.xephi.authme.permission; +import com.nijiko.permissions.Group; import com.nijiko.permissions.PermissionHandler; import com.nijikokun.bukkit.Permissions.Permissions; import de.bananaco.bpermissions.api.ApiLayer; @@ -466,6 +467,18 @@ public class PermissionsManager { // Vault return Arrays.asList(vaultPerms.getPlayerGroups(player)); + case PERMISSIONS: + // Permissions + // Create a list to put the groups in + List groups = new ArrayList<>(); + + // Get the groups and add each to the list + for(Group group : this.defaultPerms.getGroups(player.getName())) + groups.add(group.getName()); + + // Return the groups + return groups; + case NONE: // Not hooked into any permissions system, return an empty list return new ArrayList<>(); @@ -520,6 +533,10 @@ public class PermissionsManager { // Vault return vaultPerms.playerInGroup(player, groupName); + case PERMISSIONS: + // Permissions + return this.defaultPerms.inGroup(player.getWorld().getName(), player.getName(), groupName); + case NONE: // Not hooked into any permissions system, return an empty list return false; @@ -578,6 +595,11 @@ public class PermissionsManager { vaultPerms.playerAddGroup(player, groupName); return true; + case PERMISSIONS: + // Permissions + // FIXME: Add this method! + //return this.defaultPerms.group + case NONE: // Not hooked into any permissions system, return false return false; @@ -661,6 +683,11 @@ public class PermissionsManager { vaultPerms.playerRemoveGroup(player, groupName); return true; + case PERMISSIONS: + // Permissions + // FIXME: Add this method! + //return this.defaultPerms.group + case NONE: // Not hooked into any permissions system, return false return false; @@ -752,6 +779,11 @@ public class PermissionsManager { vaultPerms.playerAddGroup(player, groupName); return true; + case PERMISSIONS: + // Permissions + // FIXME: Add this method! + //return this.defaultPerms.group + case NONE: // Not hooked into any permissions system, return false return false; From 8181bda762ca4685e3676115e6bb12ac8dc2d62c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20Vis=C3=A9e?= Date: Sat, 21 Nov 2015 16:26:05 +0100 Subject: [PATCH 21/26] Minor update to hasGroupSupport method in permissions manager --- .../java/fr/xephi/authme/permission/PermissionsManager.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java index b0e8700d..8640d3e1 100644 --- a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java +++ b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java @@ -407,13 +407,17 @@ public class PermissionsManager { case B_PERMISSIONS: case ESSENTIALS_GROUP_MANAGER: case Z_PERMISSIONS: - case PERMISSIONS: return true; case VAULT: // Vault return vaultPerms.hasGroupSupport(); + case PERMISSIONS: + // Legacy permissions + // FIXME: Supported by plugin, but addGroup and removeGroup haven't been implemented correctly yet! + return false; + case NONE: // Not hooked into any permissions system, return false return false; From d81ef3168e0152107a8340539d353d775c547b8b Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 21 Nov 2015 09:07:12 +0100 Subject: [PATCH 22/26] Command refactor - remove unused fields, reduce variable "scope" Minor refactorings in the command section for familiarization. 1. Removed suppressWarning("Deprecated") - the method is deprecated for a reason and we should be made aware of that. 2. Removed same javadoc on ExecutableCommand implementation that just had the same as the interface (this is just clutter; @Override signals that it's an implementing class and a developer can view the superclass javadoc) 3. In places where the AuthMe instance was retrieved at the top but used at the very bottom, moved it to the bottom to reduce its "scope" --- .../command/CommandArgumentDescription.java | 12 +++++----- .../authme/command/ExecutableCommand.java | 5 ++-- .../command/executable/HelpCommand.java | 20 ++++------------ .../executable/authme/AccountsCommand.java | 9 -------- .../executable/authme/AuthMeCommand.java | 9 -------- .../authme/ChangePasswordCommand.java | 13 +---------- .../executable/authme/FirstSpawnCommand.java | 10 +------- .../executable/authme/ForceLoginCommand.java | 10 -------- .../executable/authme/GetEmailCommand.java | 8 ++----- .../executable/authme/GetIpCommand.java | 10 -------- .../executable/authme/LastLoginCommand.java | 18 +++------------ .../changepassword/ChangePasswordCommand.java | 14 +---------- .../executable/email/AddEmailCommand.java | 20 ++-------------- .../executable/email/ChangeEmailCommand.java | 21 ++--------------- .../executable/email/RecoverEmailCommand.java | 18 +++------------ .../executable/login/LoginCommand.java | 23 ++++--------------- .../executable/logout/LogoutCommand.java | 15 ++---------- .../executable/register/RegisterCommand.java | 14 +---------- 18 files changed, 36 insertions(+), 213 deletions(-) diff --git a/src/main/java/fr/xephi/authme/command/CommandArgumentDescription.java b/src/main/java/fr/xephi/authme/command/CommandArgumentDescription.java index e47311bf..75d6e4ae 100644 --- a/src/main/java/fr/xephi/authme/command/CommandArgumentDescription.java +++ b/src/main/java/fr/xephi/authme/command/CommandArgumentDescription.java @@ -39,8 +39,8 @@ public class CommandArgumentDescription { /** * Get the argument label. * - - * @return Argument label. */ + * @return Argument label. + */ public String getLabel() { return this.label; } @@ -57,8 +57,8 @@ public class CommandArgumentDescription { /** * Get the argument description. * - - * @return Argument description. */ + * @return Argument description. + */ public String getDescription() { return description; } @@ -75,8 +75,8 @@ public class CommandArgumentDescription { /** * Check whether the argument is optional. * - - * @return True if the argument is optional, false otherwise. */ + * @return True if the argument is optional, false otherwise. + */ public boolean isOptional() { return optional; } diff --git a/src/main/java/fr/xephi/authme/command/ExecutableCommand.java b/src/main/java/fr/xephi/authme/command/ExecutableCommand.java index 6c3ffe45..e81c705a 100644 --- a/src/main/java/fr/xephi/authme/command/ExecutableCommand.java +++ b/src/main/java/fr/xephi/authme/command/ExecutableCommand.java @@ -3,6 +3,7 @@ package fr.xephi.authme.command; import org.bukkit.command.CommandSender; /** + * Base class for AuthMe commands that can be executed. */ public abstract class ExecutableCommand { @@ -13,7 +14,7 @@ public abstract class ExecutableCommand { * @param commandReference The command reference. * @param commandArguments The command arguments. * - - * @return True if the command was executed successfully, false otherwise. */ + * @return True if the command was executed successfully, false otherwise. + */ public abstract boolean executeCommand(CommandSender sender, CommandParts commandReference, CommandParts commandArguments); } diff --git a/src/main/java/fr/xephi/authme/command/executable/HelpCommand.java b/src/main/java/fr/xephi/authme/command/executable/HelpCommand.java index 1d625d9c..9fbcd1d9 100644 --- a/src/main/java/fr/xephi/authme/command/executable/HelpCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/HelpCommand.java @@ -10,31 +10,19 @@ import fr.xephi.authme.command.help.HelpProvider; */ public class HelpCommand extends ExecutableCommand { - /** - * Execute the command. - * - * @param sender The command sender. - * @param commandReference The command reference. - * @param commandArguments The command arguments. - * - - * @return True if the command was executed successfully, false otherwise. */ @Override public boolean executeCommand(CommandSender sender, CommandParts commandReference, CommandParts commandArguments) { // Check whether quick help should be shown boolean quickHelp = commandArguments.getCount() == 0; - // Set the proper command arguments for the quick help - if(quickHelp) + // Set the proper command arguments for the quick help and show it + if (quickHelp) { commandArguments = new CommandParts(commandReference.get(0)); - - // Show the new help - if(quickHelp) HelpProvider.showHelp(sender, commandReference, commandArguments, false, false, false, false, false, true); - else + } else { HelpProvider.showHelp(sender, commandReference, commandArguments); + } - // Return the result return true; } } diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/AccountsCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/AccountsCommand.java index a627c934..0e036b0d 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/AccountsCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/AccountsCommand.java @@ -16,15 +16,6 @@ import fr.xephi.authme.settings.Messages; */ public class AccountsCommand extends ExecutableCommand { - /** - * Execute the command. - * - * @param sender The command sender. - * @param commandReference The command reference. - * @param commandArguments The command arguments. - * - - * @return True if the command was executed successfully, false otherwise. */ @Override public boolean executeCommand(final CommandSender sender, CommandParts commandReference, CommandParts commandArguments) { // AuthMe plugin instance diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/AuthMeCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/AuthMeCommand.java index 541154e7..da67fa82 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/AuthMeCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/AuthMeCommand.java @@ -11,15 +11,6 @@ import fr.xephi.authme.command.ExecutableCommand; */ public class AuthMeCommand extends ExecutableCommand { - /** - * Execute the command. - * - * @param sender The command sender. - * @param commandReference The command reference. - * @param commandArguments The command arguments. - * - - * @return True if the command was executed successfully, false otherwise. */ @Override public boolean executeCommand(CommandSender sender, CommandParts commandReference, CommandParts commandArguments) { // Show some version info diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordCommand.java index 170685c0..04b4a019 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordCommand.java @@ -19,20 +19,8 @@ import fr.xephi.authme.settings.Settings; */ public class ChangePasswordCommand extends ExecutableCommand { - /** - * Execute the command. - * - * @param sender The command sender. - * @param commandReference The command reference. - * @param commandArguments The command arguments. - * - - * @return True if the command was executed successfully, false otherwise. */ @Override public boolean executeCommand(final CommandSender sender, CommandParts commandReference, CommandParts commandArguments) { - // AuthMe plugin instance - final AuthMe plugin = AuthMe.getInstance(); - // Messages instance final Messages m = Messages.getInstance(); @@ -62,6 +50,7 @@ public class ChangePasswordCommand extends ExecutableCommand { } // Set the password + final AuthMe plugin = AuthMe.getInstance(); final String playerNameLowerCase = playerName.toLowerCase(); Bukkit.getScheduler().runTaskAsynchronously(plugin, new Runnable() { diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/FirstSpawnCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/FirstSpawnCommand.java index 13555552..a6401b0a 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/FirstSpawnCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/FirstSpawnCommand.java @@ -12,15 +12,6 @@ import fr.xephi.authme.settings.Spawn; */ public class FirstSpawnCommand extends ExecutableCommand { - /** - * Execute the command. - * - * @param sender The command sender. - * @param commandReference The command reference. - * @param commandArguments The command arguments. - * - - * @return True if the command was executed successfully, false otherwise. */ @Override public boolean executeCommand(CommandSender sender, CommandParts commandReference, CommandParts commandArguments) { // Make sure the command executor is a player @@ -33,6 +24,7 @@ public class FirstSpawnCommand extends ExecutableCommand { sender.sendMessage("[AuthMe] Please use that command in game"); } } catch (NullPointerException ex) { + // TODO ljacqu 20151119: Catching NullPointerException is never a good idea. Find what can cause one instead ConsoleLogger.showError(ex.getMessage()); } return true; diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/ForceLoginCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/ForceLoginCommand.java index 19e1e67c..0a9fd944 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/ForceLoginCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/ForceLoginCommand.java @@ -12,15 +12,6 @@ import fr.xephi.authme.command.ExecutableCommand; */ public class ForceLoginCommand extends ExecutableCommand { - /** - * Execute the command. - * - * @param sender The command sender. - * @param commandReference The command reference. - * @param commandArguments The command arguments. - * - - * @return True if the command was executed successfully, false otherwise. */ @Override public boolean executeCommand(CommandSender sender, CommandParts commandReference, CommandParts commandArguments) { // AuthMe plugin instance @@ -33,7 +24,6 @@ public class ForceLoginCommand extends ExecutableCommand { // Command logic try { - @SuppressWarnings("deprecation") Player player = Bukkit.getPlayer(playerName); if (player == null || !player.isOnline()) { sender.sendMessage("Player needs to be online!"); diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/GetEmailCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/GetEmailCommand.java index 1ca2a851..4ee1fe06 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/GetEmailCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/GetEmailCommand.java @@ -23,18 +23,14 @@ public class GetEmailCommand extends ExecutableCommand { * @return True if the command was executed successfully, false otherwise. */ @Override public boolean executeCommand(CommandSender sender, CommandParts commandReference, CommandParts commandArguments) { - // AuthMe plugin instance - AuthMe plugin = AuthMe.getInstance(); - - // Messages instance - Messages m = Messages.getInstance(); - // Get the player name String playerName = sender.getName(); if(commandArguments.getCount() >= 1) playerName = commandArguments.get(0); // Get the authenticated user + AuthMe plugin = AuthMe.getInstance(); + Messages m = Messages.getInstance(); PlayerAuth auth = plugin.database.getAuth(playerName.toLowerCase()); if (auth == null) { m.send(sender, "unknown_user"); diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/GetIpCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/GetIpCommand.java index f3b2700a..343642c5 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/GetIpCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/GetIpCommand.java @@ -12,15 +12,6 @@ import fr.xephi.authme.command.ExecutableCommand; */ public class GetIpCommand extends ExecutableCommand { - /** - * Execute the command. - * - * @param sender The command sender. - * @param commandReference The command reference. - * @param commandArguments The command arguments. - * - - * @return True if the command was executed successfully, false otherwise. */ @Override public boolean executeCommand(CommandSender sender, CommandParts commandReference, CommandParts commandArguments) { // AuthMe plugin instance @@ -31,7 +22,6 @@ public class GetIpCommand extends ExecutableCommand { if(commandArguments.getCount() >= 1) playerName = commandArguments.get(0); - @SuppressWarnings("deprecation") Player player = Bukkit.getPlayer(playerName); if (player == null) { sender.sendMessage("This player is not actually online"); diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/LastLoginCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/LastLoginCommand.java index 58237ef8..b2c4c70f 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/LastLoginCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/LastLoginCommand.java @@ -14,29 +14,17 @@ import fr.xephi.authme.settings.Messages; */ public class LastLoginCommand extends ExecutableCommand { - /** - * Execute the command. - * - * @param sender The command sender. - * @param commandReference The command reference. - * @param commandArguments The command arguments. - * - - * @return True if the command was executed successfully, false otherwise. */ @Override public boolean executeCommand(CommandSender sender, CommandParts commandReference, CommandParts commandArguments) { - // AuthMe plugin instance - AuthMe plugin = AuthMe.getInstance(); - - // Messages instance - Messages m = Messages.getInstance(); - // Get the player String playerName = sender.getName(); if(commandArguments.getCount() >= 1) playerName = commandArguments.get(0); // Validate the player + AuthMe plugin = AuthMe.getInstance(); + Messages m = Messages.getInstance(); + PlayerAuth auth; try { auth = plugin.database.getAuth(playerName.toLowerCase()); diff --git a/src/main/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommand.java b/src/main/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommand.java index dc258362..9b6956b2 100644 --- a/src/main/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommand.java @@ -15,21 +15,8 @@ import fr.xephi.authme.task.ChangePasswordTask; */ public class ChangePasswordCommand extends ExecutableCommand { - /** - * Execute the command. - * - * @param sender The command sender. - * @param commandReference The command reference. - * @param commandArguments The command arguments. - * - - * @return True if the command was executed successfully, false otherwise. */ @Override public boolean executeCommand(CommandSender sender, CommandParts commandReference, CommandParts commandArguments) { - // AuthMe plugin instance - final AuthMe plugin = AuthMe.getInstance(); - - // Messages instance final Messages m = Messages.getInstance(); // Get the passwords @@ -71,6 +58,7 @@ public class ChangePasswordCommand extends ExecutableCommand { } // Set the password + final AuthMe plugin = AuthMe.getInstance(); plugin.getServer().getScheduler().runTaskAsynchronously(plugin, new ChangePasswordTask(plugin, player, playerPass, playerPassVerify)); return true; } diff --git a/src/main/java/fr/xephi/authme/command/executable/email/AddEmailCommand.java b/src/main/java/fr/xephi/authme/command/executable/email/AddEmailCommand.java index 34d6bba4..68f5e54b 100644 --- a/src/main/java/fr/xephi/authme/command/executable/email/AddEmailCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/email/AddEmailCommand.java @@ -12,22 +12,8 @@ import fr.xephi.authme.settings.Messages; */ public class AddEmailCommand extends ExecutableCommand { - /** - * Execute the command. - * - * @param sender The command sender. - * @param commandReference The command reference. - * @param commandArguments The command arguments. - - * @return True if the command was executed successfully, false otherwise. */ @Override public boolean executeCommand(CommandSender sender, CommandParts commandReference, CommandParts commandArguments) { - // AuthMe plugin instance - final AuthMe plugin = AuthMe.getInstance(); - - // Messages instance - final Messages m = Messages.getInstance(); - // Get the parameter values String playerMail = commandArguments.get(0); String playerMailVerify = commandArguments.get(1); @@ -37,11 +23,9 @@ public class AddEmailCommand extends ExecutableCommand { return true; } - // Get the player instance and name + // Get the player and perform email addition + final AuthMe plugin = AuthMe.getInstance(); final Player player = (Player) sender; - final String playerName = player.getName().toLowerCase(); - - // Command logic plugin.management.performAddEmail(player, playerMail, playerMailVerify); return true; } diff --git a/src/main/java/fr/xephi/authme/command/executable/email/ChangeEmailCommand.java b/src/main/java/fr/xephi/authme/command/executable/email/ChangeEmailCommand.java index 03a3c156..d295512e 100644 --- a/src/main/java/fr/xephi/authme/command/executable/email/ChangeEmailCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/email/ChangeEmailCommand.java @@ -6,28 +6,13 @@ import org.bukkit.entity.Player; import fr.xephi.authme.AuthMe; import fr.xephi.authme.command.CommandParts; import fr.xephi.authme.command.ExecutableCommand; -import fr.xephi.authme.settings.Messages; /** */ public class ChangeEmailCommand extends ExecutableCommand { - /** - * Execute the command. - * - * @param sender The command sender. - * @param commandReference The command reference. - * @param commandArguments The command arguments. - - * @return True if the command was executed successfully, false otherwise. */ @Override public boolean executeCommand(CommandSender sender, CommandParts commandReference, CommandParts commandArguments) { - // AuthMe plugin instance - final AuthMe plugin = AuthMe.getInstance(); - - // Messages instance - final Messages m = Messages.getInstance(); - // Get the parameter values String playerMailOld = commandArguments.get(0); String playerMailNew = commandArguments.get(1); @@ -37,11 +22,9 @@ public class ChangeEmailCommand extends ExecutableCommand { return true; } - // Get the player instance and name + // Get the player instance and execute action + final AuthMe plugin = AuthMe.getInstance(); final Player player = (Player) sender; - final String playerName = player.getName(); - - // Command logic plugin.management.performChangeEmail(player, playerMailOld, playerMailNew); return true; } diff --git a/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java b/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java index 748fed46..44f007a9 100644 --- a/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java @@ -20,23 +20,8 @@ import fr.xephi.authme.settings.Settings; */ public class RecoverEmailCommand extends ExecutableCommand { - /** - * Execute the command. - * - * @param sender The command sender. - * @param commandReference The command reference. - * @param commandArguments The command arguments. - * - - * @return True if the command was executed successfully, false otherwise. */ @Override public boolean executeCommand(CommandSender sender, CommandParts commandReference, CommandParts commandArguments) { - // AuthMe plugin instance - final AuthMe plugin = AuthMe.getInstance(); - - // Messages instance - final Messages m = Messages.getInstance(); - // Get the parameter values String playerMail = commandArguments.get(0); @@ -50,6 +35,9 @@ public class RecoverEmailCommand extends ExecutableCommand { final String playerName = player.getName(); // Command logic + final AuthMe plugin = AuthMe.getInstance(); + final Messages m = Messages.getInstance(); + if (plugin.mail == null) { m.send(player, "error"); return true; diff --git a/src/main/java/fr/xephi/authme/command/executable/login/LoginCommand.java b/src/main/java/fr/xephi/authme/command/executable/login/LoginCommand.java index d6b44164..e9b80cdf 100644 --- a/src/main/java/fr/xephi/authme/command/executable/login/LoginCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/login/LoginCommand.java @@ -11,32 +11,19 @@ import fr.xephi.authme.command.ExecutableCommand; */ public class LoginCommand extends ExecutableCommand { - /** - * Execute the command. - * - * @param sender The command sender. - * @param commandReference The command reference. - * @param commandArguments The command arguments. - * - - * @return True if the command was executed successfully, false otherwise. */ @Override public boolean executeCommand(CommandSender sender, CommandParts commandReference, CommandParts commandArguments) { - // AuthMe plugin instance - final AuthMe plugin = AuthMe.getInstance(); - // Make sure the current command executor is a player - if(!(sender instanceof Player)) { + if (!(sender instanceof Player)) { return true; } - // Get the player instance + // Get the necessary objects + final AuthMe plugin = AuthMe.getInstance(); final Player player = (Player) sender; + final String playerPass = commandArguments.get(0); - // Get the password - String playerPass = commandArguments.get(0); - - // Login the player + // Log the player in plugin.management.performLogin(player, playerPass, false); return true; } diff --git a/src/main/java/fr/xephi/authme/command/executable/logout/LogoutCommand.java b/src/main/java/fr/xephi/authme/command/executable/logout/LogoutCommand.java index fcc86ce2..e2e97806 100644 --- a/src/main/java/fr/xephi/authme/command/executable/logout/LogoutCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/logout/LogoutCommand.java @@ -11,26 +11,15 @@ import fr.xephi.authme.command.ExecutableCommand; */ public class LogoutCommand extends ExecutableCommand { - /** - * Execute the command. - * - * @param sender The command sender. - * @param commandReference The command reference. - * @param commandArguments The command arguments. - * - - * @return True if the command was executed successfully, false otherwise. */ @Override public boolean executeCommand(CommandSender sender, CommandParts commandReference, CommandParts commandArguments) { - // AuthMe plugin instance - final AuthMe plugin = AuthMe.getInstance(); - // Make sure the current command executor is a player - if(!(sender instanceof Player)) { + if (!(sender instanceof Player)) { return true; } // Get the player instance + final AuthMe plugin = AuthMe.getInstance(); final Player player = (Player) sender; // Logout the player diff --git a/src/main/java/fr/xephi/authme/command/executable/register/RegisterCommand.java b/src/main/java/fr/xephi/authme/command/executable/register/RegisterCommand.java index 7484793d..3c7f07ee 100644 --- a/src/main/java/fr/xephi/authme/command/executable/register/RegisterCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/register/RegisterCommand.java @@ -14,21 +14,8 @@ import fr.xephi.authme.settings.Settings; */ public class RegisterCommand extends ExecutableCommand { - /** - * Execute the command. - * - * @param sender The command sender. - * @param commandReference The command reference. - * @param commandArguments The command arguments. - * - - * @return True if the command was executed successfully, false otherwise. */ @Override public boolean executeCommand(CommandSender sender, CommandParts commandReference, CommandParts commandArguments) { - // AuthMe plugin instance - final AuthMe plugin = AuthMe.getInstance(); - - // Messages instance final Messages m = Messages.getInstance(); // Make sure the sender is a player @@ -44,6 +31,7 @@ public class RegisterCommand extends ExecutableCommand { return true; } + final AuthMe plugin = AuthMe.getInstance(); if (Settings.emailRegistration && !Settings.getmailAccount.isEmpty()) { if (Settings.doubleEmailCheck) { if (commandArguments.getCount() < 2 || !commandArguments.get(0).equals(commandArguments.get(1))) { From 4e8614fdf7cfa909179f48e1c5723f03cb627ee1 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 21 Nov 2015 09:49:39 +0100 Subject: [PATCH 23/26] Add test for LoginCommand; create AuthMe mock test util Had to create a getter for the Management instance in the AuthMe class for mocking, but fields should generally not be accessed globally. Hopefully soon we will be able to make the field private. --- src/main/java/fr/xephi/authme/AuthMe.java | 5 ++ .../executable/login/LoginCommand.java | 2 +- .../java/fr/xephi/authme/AuthMeMockUtil.java | 30 ++++++++ .../executable/login/LoginCommandTest.java | 77 +++++++++++++++++++ 4 files changed, 113 insertions(+), 1 deletion(-) create mode 100644 src/test/java/fr/xephi/authme/AuthMeMockUtil.java create mode 100644 src/test/java/fr/xephi/authme/command/executable/login/LoginCommandTest.java diff --git a/src/main/java/fr/xephi/authme/AuthMe.java b/src/main/java/fr/xephi/authme/AuthMe.java index 29d7091c..69112ea2 100644 --- a/src/main/java/fr/xephi/authme/AuthMe.java +++ b/src/main/java/fr/xephi/authme/AuthMe.java @@ -952,4 +952,9 @@ public class AuthMe extends JavaPlugin { public static int getVersionCode() { return PLUGIN_VERSION_CODE; } + + /** Returns the management instance. */ + public Management getManagement() { + return management; + } } diff --git a/src/main/java/fr/xephi/authme/command/executable/login/LoginCommand.java b/src/main/java/fr/xephi/authme/command/executable/login/LoginCommand.java index e9b80cdf..2479a105 100644 --- a/src/main/java/fr/xephi/authme/command/executable/login/LoginCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/login/LoginCommand.java @@ -24,7 +24,7 @@ public class LoginCommand extends ExecutableCommand { final String playerPass = commandArguments.get(0); // Log the player in - plugin.management.performLogin(player, playerPass, false); + plugin.getManagement().performLogin(player, playerPass, false); return true; } } diff --git a/src/test/java/fr/xephi/authme/AuthMeMockUtil.java b/src/test/java/fr/xephi/authme/AuthMeMockUtil.java new file mode 100644 index 00000000..6f4ca7a4 --- /dev/null +++ b/src/test/java/fr/xephi/authme/AuthMeMockUtil.java @@ -0,0 +1,30 @@ +package fr.xephi.authme; + +import org.mockito.Mockito; + +import java.lang.reflect.Field; + +/** + * Creates a mock implementation of AuthMe for testing purposes. + */ +public final class AuthMeMockUtil { + + private AuthMeMockUtil() { + // Util class + } + + /** + * Set the AuthMe plugin instance to a mock object. Use {@link AuthMe#getInstance()} to retrieve the mock. + */ + public static void initialize() { + AuthMe mock = Mockito.mock(AuthMe.class); + + try { + Field instance = AuthMe.class.getDeclaredField("plugin"); + instance.setAccessible(true); + instance.set(null, mock); + } catch (NoSuchFieldException | IllegalAccessException e) { + throw new RuntimeException("Could not initialize AuthMe mock", e); + } + } +} diff --git a/src/test/java/fr/xephi/authme/command/executable/login/LoginCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/login/LoginCommandTest.java new file mode 100644 index 00000000..b8c3c2f5 --- /dev/null +++ b/src/test/java/fr/xephi/authme/command/executable/login/LoginCommandTest.java @@ -0,0 +1,77 @@ +package fr.xephi.authme.command.executable.login; + +import fr.xephi.authme.AuthMe; +import fr.xephi.authme.AuthMeMockUtil; +import fr.xephi.authme.command.CommandParts; +import fr.xephi.authme.process.Management; +import fr.xephi.authme.settings.Settings; +import org.bukkit.command.BlockCommandSender; +import org.bukkit.command.CommandSender; +import org.bukkit.entity.Player; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mockito; + + +import static org.mockito.Matchers.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; + +/** + * Test for {@link LoginCommand}. + */ +public class LoginCommandTest { + + private static Management managementMock; + + @Before + public void initializeAuthMeMock() { + AuthMeMockUtil.initialize(); + AuthMe pluginMock = AuthMe.getInstance(); + + Settings.captchaLength = 10; + managementMock = mock(Management.class); + Mockito.when(pluginMock.getManagement()).thenReturn(managementMock); + } + + @Test + public void shouldStopIfSenderIsNotAPlayer() { + // given + CommandSender sender = mock(BlockCommandSender.class); + LoginCommand command = new LoginCommand(); + + // when + command.executeCommand(sender, new CommandParts(), new CommandParts()); + + // then + Mockito.verify(managementMock, never()).performLogin(any(Player.class), anyString(), anyBoolean()); + } + + @Test + public void shouldCallManagementForPlayerCaller() { + // given + Player sender = mock(Player.class); + LoginCommand command = new LoginCommand(); + + // when + command.executeCommand(sender, new CommandParts(), new CommandParts("password")); + + // then + Mockito.verify(managementMock).performLogin(eq(sender), eq("password"), eq(false)); + } + + @Test + public void shouldHandleMissingPassword() { + // given + Player sender = mock(Player.class); + LoginCommand command = new LoginCommand(); + + // when + command.executeCommand(sender, new CommandParts(), new CommandParts()); + + // then + // TODO ljacqu 20151121: May make sense to handle null password in LoginCommand instead of forwarding the call + String password = null; + Mockito.verify(managementMock).performLogin(eq(sender), eq(password), eq(false)); + } +} From 58dc15123c4d4e19a48c4a4294c15487891c730d Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 21 Nov 2015 10:29:40 +0100 Subject: [PATCH 24/26] Add tests for LogoutCommand and RegisterCommand. Add more generic mockUtil --- .../executable/logout/LogoutCommand.java | 2 +- .../executable/register/RegisterCommand.java | 7 +- .../java/fr/xephi/authme/AuthMeMockUtil.java | 27 +++++- .../executable/login/LoginCommandTest.java | 8 +- .../executable/logout/LogoutCommandTest.java | 62 +++++++++++++ .../register/RegisterCommandTest.java | 88 +++++++++++++++++++ 6 files changed, 183 insertions(+), 11 deletions(-) create mode 100644 src/test/java/fr/xephi/authme/command/executable/logout/LogoutCommandTest.java create mode 100644 src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java diff --git a/src/main/java/fr/xephi/authme/command/executable/logout/LogoutCommand.java b/src/main/java/fr/xephi/authme/command/executable/logout/LogoutCommand.java index e2e97806..60c14a26 100644 --- a/src/main/java/fr/xephi/authme/command/executable/logout/LogoutCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/logout/LogoutCommand.java @@ -23,7 +23,7 @@ public class LogoutCommand extends ExecutableCommand { final Player player = (Player) sender; // Logout the player - plugin.management.performLogout(player); + plugin.getManagement().performLogout(player); return true; } } diff --git a/src/main/java/fr/xephi/authme/command/executable/register/RegisterCommand.java b/src/main/java/fr/xephi/authme/command/executable/register/RegisterCommand.java index 3c7f07ee..6e89fc62 100644 --- a/src/main/java/fr/xephi/authme/command/executable/register/RegisterCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/register/RegisterCommand.java @@ -1,5 +1,6 @@ package fr.xephi.authme.command.executable.register; +import fr.xephi.authme.process.Management; import org.bukkit.command.CommandSender; import org.bukkit.entity.Player; @@ -31,7 +32,7 @@ public class RegisterCommand extends ExecutableCommand { return true; } - final AuthMe plugin = AuthMe.getInstance(); + final Management management = AuthMe.getInstance().getManagement(); if (Settings.emailRegistration && !Settings.getmailAccount.isEmpty()) { if (Settings.doubleEmailCheck) { if (commandArguments.getCount() < 2 || !commandArguments.get(0).equals(commandArguments.get(1))) { @@ -46,7 +47,7 @@ public class RegisterCommand extends ExecutableCommand { } RandomString rand = new RandomString(Settings.getRecoveryPassLength); final String thePass = rand.nextString(); - plugin.management.performRegister(player, thePass, email); + management.performRegister(player, thePass, email); return true; } if (commandArguments.getCount() > 1 && Settings.getEnablePasswordVerifier) @@ -54,7 +55,7 @@ public class RegisterCommand extends ExecutableCommand { m.send(player, "password_error"); return true; } - plugin.management.performRegister(player, commandArguments.get(0), ""); + management.performRegister(player, commandArguments.get(0), ""); return true; } } diff --git a/src/test/java/fr/xephi/authme/AuthMeMockUtil.java b/src/test/java/fr/xephi/authme/AuthMeMockUtil.java index 6f4ca7a4..905c4630 100644 --- a/src/test/java/fr/xephi/authme/AuthMeMockUtil.java +++ b/src/test/java/fr/xephi/authme/AuthMeMockUtil.java @@ -1,5 +1,6 @@ package fr.xephi.authme; +import fr.xephi.authme.settings.Messages; import org.mockito.Mockito; import java.lang.reflect.Field; @@ -14,17 +15,35 @@ public final class AuthMeMockUtil { } /** - * Set the AuthMe plugin instance to a mock object. Use {@link AuthMe#getInstance()} to retrieve the mock. + * Sets the AuthMe plugin instance to a mock object. Use {@link AuthMe#getInstance()} to retrieve the mock. */ - public static void initialize() { + public static void mockAuthMeInstance() { AuthMe mock = Mockito.mock(AuthMe.class); + mockSingletonForClass(AuthMe.class, "plugin", mock); + } + /** + * Creates a mock Messages object for the instance returned from {@link Messages#getInstance()}. + */ + public static void mockMessagesInstance() { + Messages mock = Mockito.mock(Messages.class); + mockSingletonForClass(Messages.class, "singleton", mock); + } + + /** + * Sets a field of a class to the given mock. + * + * @param clazz the class to modify + * @param fieldName the field name + * @param mock the mock to set for the given field + */ + private static void mockSingletonForClass(Class clazz, String fieldName, Object mock) { try { - Field instance = AuthMe.class.getDeclaredField("plugin"); + Field instance = clazz.getDeclaredField(fieldName); instance.setAccessible(true); instance.set(null, mock); } catch (NoSuchFieldException | IllegalAccessException e) { - throw new RuntimeException("Could not initialize AuthMe mock", e); + throw new RuntimeException("Could not set mock instance for class " + clazz.getName(), e); } } } diff --git a/src/test/java/fr/xephi/authme/command/executable/login/LoginCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/login/LoginCommandTest.java index b8c3c2f5..5ed42876 100644 --- a/src/test/java/fr/xephi/authme/command/executable/login/LoginCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/login/LoginCommandTest.java @@ -12,8 +12,10 @@ import org.junit.Before; import org.junit.Test; import org.mockito.Mockito; - -import static org.mockito.Matchers.*; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyBoolean; +import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -26,7 +28,7 @@ public class LoginCommandTest { @Before public void initializeAuthMeMock() { - AuthMeMockUtil.initialize(); + AuthMeMockUtil.mockAuthMeInstance(); AuthMe pluginMock = AuthMe.getInstance(); Settings.captchaLength = 10; diff --git a/src/test/java/fr/xephi/authme/command/executable/logout/LogoutCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/logout/LogoutCommandTest.java new file mode 100644 index 00000000..8416b5fd --- /dev/null +++ b/src/test/java/fr/xephi/authme/command/executable/logout/LogoutCommandTest.java @@ -0,0 +1,62 @@ +package fr.xephi.authme.command.executable.logout; + +import fr.xephi.authme.AuthMe; +import fr.xephi.authme.AuthMeMockUtil; +import fr.xephi.authme.command.CommandParts; +import fr.xephi.authme.process.Management; +import fr.xephi.authme.settings.Settings; +import org.bukkit.command.BlockCommandSender; +import org.bukkit.command.CommandSender; +import org.bukkit.entity.Player; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mockito; + +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; + +/** + * Test for {@link LogoutCommand}. + */ +public class LogoutCommandTest { + + private static Management managementMock; + + @Before + public void initializeAuthMeMock() { + AuthMeMockUtil.mockAuthMeInstance(); + AuthMe pluginMock = AuthMe.getInstance(); + + Settings.captchaLength = 10; + managementMock = mock(Management.class); + Mockito.when(pluginMock.getManagement()).thenReturn(managementMock); + } + + @Test + public void shouldStopIfSenderIsNotAPlayer() { + // given + CommandSender sender = mock(BlockCommandSender.class); + LogoutCommand command = new LogoutCommand(); + + // when + command.executeCommand(sender, new CommandParts(), new CommandParts()); + + // then + Mockito.verify(managementMock, never()).performLogout(any(Player.class)); + } + + @Test + public void shouldCallManagementForPlayerCaller() { + // given + Player sender = mock(Player.class); + LogoutCommand command = new LogoutCommand(); + + // when + command.executeCommand(sender, new CommandParts(), new CommandParts("password")); + + // then + Mockito.verify(managementMock).performLogout(sender); + } + +} diff --git a/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java new file mode 100644 index 00000000..0b2ba2cd --- /dev/null +++ b/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java @@ -0,0 +1,88 @@ +package fr.xephi.authme.command.executable.register; + +import fr.xephi.authme.AuthMe; +import fr.xephi.authme.AuthMeMockUtil; +import fr.xephi.authme.command.CommandParts; +import fr.xephi.authme.process.Management; +import fr.xephi.authme.settings.Messages; +import fr.xephi.authme.settings.Settings; +import org.bukkit.command.BlockCommandSender; +import org.bukkit.command.CommandSender; +import org.bukkit.entity.Player; +import org.junit.Before; +import org.junit.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Mockito; + +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThat; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +/** + * Test for {@link RegisterCommand}. + */ +public class RegisterCommandTest { + + private static Management managementMock; + private static Messages messagesMock; + + @Before + public void initializeAuthMeMock() { + AuthMeMockUtil.mockMessagesInstance(); + messagesMock = Messages.getInstance(); + + AuthMeMockUtil.mockAuthMeInstance(); + AuthMe pluginMock = AuthMe.getInstance(); + + Settings.captchaLength = 10; + managementMock = mock(Management.class); + Mockito.when(pluginMock.getManagement()).thenReturn(managementMock); + } + + @Test + public void shouldNotRunForNonPlayerSender() { + // given + CommandSender sender = mock(BlockCommandSender.class); + RegisterCommand command = new RegisterCommand(); + ArgumentCaptor messageCaptor = ArgumentCaptor.forClass(String.class); + + // when + command.executeCommand(sender, new CommandParts(), new CommandParts()); + + // then + verify(sender).sendMessage(messageCaptor.capture()); + assertThat(messageCaptor.getValue().contains("Player Only!"), equalTo(true)); + verify(managementMock, never()).performRegister(any(Player.class), anyString(), anyString()); + } + + @Test + public void shouldFailForEmptyArguments() { + // given + CommandSender sender = mock(Player.class); + RegisterCommand command = new RegisterCommand(); + + // when + command.executeCommand(sender, new CommandParts(), new CommandParts()); + + // then + verify(messagesMock).send(sender, "usage_reg"); + verify(managementMock, never()).performRegister(any(Player.class), anyString(), anyString()); + } + + @Test + public void shouldForwardRegister() { + // given + Player sender = mock(Player.class); + RegisterCommand command = new RegisterCommand(); + + // when + command.executeCommand(sender, new CommandParts(), new CommandParts("password")); + + // then + verify(managementMock).performRegister(sender, "password", ""); + } +} From a3f24bcb9a38473f04018d98f3635e794aec634a Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 21 Nov 2015 11:16:31 +0100 Subject: [PATCH 25/26] Create test for HelpSyntaxHelperTest --- .../authme/command/CommandDescription.java | 1 + .../xephi/authme/command/CommandManager.java | 1 + .../authme/command/help/HelpSyntaxHelper.java | 16 +- .../java/fr/xephi/authme/util/ListUtils.java | 19 --- .../command/help/HelpSyntaxHelperTest.java | 152 ++++++++++++++++++ 5 files changed, 164 insertions(+), 25 deletions(-) create mode 100644 src/test/java/fr/xephi/authme/command/help/HelpSyntaxHelperTest.java diff --git a/src/main/java/fr/xephi/authme/command/CommandDescription.java b/src/main/java/fr/xephi/authme/command/CommandDescription.java index 51ce67b0..4feec54d 100644 --- a/src/main/java/fr/xephi/authme/command/CommandDescription.java +++ b/src/main/java/fr/xephi/authme/command/CommandDescription.java @@ -681,6 +681,7 @@ public class CommandDescription { * * @param maximumArguments True if there is an argument maximum, based on the number of registered arguments. */ + // TODO ljacqu 20151121: Rename the setter public void setMaximumArguments(boolean maximumArguments) { this.noArgumentMaximum = !maximumArguments; } diff --git a/src/main/java/fr/xephi/authme/command/CommandManager.java b/src/main/java/fr/xephi/authme/command/CommandManager.java index 162d61d4..be5b28bf 100644 --- a/src/main/java/fr/xephi/authme/command/CommandManager.java +++ b/src/main/java/fr/xephi/authme/command/CommandManager.java @@ -54,6 +54,7 @@ public class CommandManager { /** * Register all commands. */ + // TODO ljacqu 20151121: Create a builder class for CommandDescription @SuppressWarnings({ "serial" }) public void registerCommands() { // Register the base AuthMe Reloaded command diff --git a/src/main/java/fr/xephi/authme/command/help/HelpSyntaxHelper.java b/src/main/java/fr/xephi/authme/command/help/HelpSyntaxHelper.java index 55f2581a..5f564749 100644 --- a/src/main/java/fr/xephi/authme/command/help/HelpSyntaxHelper.java +++ b/src/main/java/fr/xephi/authme/command/help/HelpSyntaxHelper.java @@ -9,7 +9,11 @@ import fr.xephi.authme.util.ListUtils; /** */ -public class HelpSyntaxHelper { +public final class HelpSyntaxHelper { + + private HelpSyntaxHelper() { + // Helper class + } /** * Get the proper syntax for a command. @@ -19,8 +23,8 @@ public class HelpSyntaxHelper { * @param alternativeLabel The alternative label to use for this command syntax. * @param highlight True to highlight the important parts of this command. * - - * @return The command with proper syntax. */ + * @return The command with proper syntax. + */ @SuppressWarnings("StringConcatenationInsideStringBufferAppend") public static String getCommandSyntax(CommandDescription commandDescription, CommandParts commandReference, String alternativeLabel, boolean highlight) { // Create a string builder to build the command @@ -35,9 +39,9 @@ public class HelpSyntaxHelper { String commandLabel = helpCommandReference.get(helpCommandReference.getCount() - 1); // Check whether the alternative label should be used - if(alternativeLabel != null) - if(alternativeLabel.trim().length() > 0) - commandLabel = alternativeLabel; + if (alternativeLabel != null && alternativeLabel.trim().length() > 0) { + commandLabel = alternativeLabel; + } // Show the important bit of the command, highlight this part if required sb.append(ListUtils.implode(parentCommand, (highlight ? ChatColor.YELLOW + "" + ChatColor.BOLD : "") + commandLabel, " ")); diff --git a/src/main/java/fr/xephi/authme/util/ListUtils.java b/src/main/java/fr/xephi/authme/util/ListUtils.java index 05ede1ba..69ed092e 100644 --- a/src/main/java/fr/xephi/authme/util/ListUtils.java +++ b/src/main/java/fr/xephi/authme/util/ListUtils.java @@ -37,25 +37,6 @@ public class ListUtils { return sb.toString(); } - /** - * Implode two lists of elements into a single string, with a specified separator. - * - * @param elements The first list of elements to implode. - * @param otherElements The second list of elements to implode. - * @param separator The separator to use. - * - - * @return The result string. */ - public static String implode(List elements, List otherElements, String separator) { - // Combine the lists - List combined = new ArrayList<>(); - combined.addAll(elements); - combined.addAll(otherElements); - - // Implode and return the result - return implode(combined, separator); - } - /** * Implode two elements into a single string, with a specified separator. * diff --git a/src/test/java/fr/xephi/authme/command/help/HelpSyntaxHelperTest.java b/src/test/java/fr/xephi/authme/command/help/HelpSyntaxHelperTest.java new file mode 100644 index 00000000..3fba7025 --- /dev/null +++ b/src/test/java/fr/xephi/authme/command/help/HelpSyntaxHelperTest.java @@ -0,0 +1,152 @@ +package fr.xephi.authme.command.help; + +import fr.xephi.authme.command.CommandArgumentDescription; +import fr.xephi.authme.command.CommandDescription; +import fr.xephi.authme.command.CommandParts; +import fr.xephi.authme.command.executable.authme.AuthMeCommand; +import fr.xephi.authme.command.executable.authme.RegisterCommand; +import org.junit.Test; + +import java.util.ArrayList; +import java.util.Arrays; + +import static java.util.Collections.singletonList; +import static org.bukkit.ChatColor.BOLD; +import static org.bukkit.ChatColor.ITALIC; +import static org.bukkit.ChatColor.WHITE; +import static org.bukkit.ChatColor.YELLOW; +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThat; + +/** + * Test for {@link HelpSyntaxHelper}. + */ +public class HelpSyntaxHelperTest { + + @Test + public void shouldFormatSimpleCommand() { + // given + CommandDescription description = getDescription(); + + // when + String result = HelpSyntaxHelper.getCommandSyntax( + description, new CommandParts(), "", false); + + // then + assertThat(result, equalTo(WHITE + "/authme register" + ITALIC + " [name]")); + } + + @Test + public void shouldFormatSimpleCommandWithOptionalParam() { + // given + CommandDescription description = getDescription(); + description.setArguments(singletonList( + new CommandArgumentDescription("test", "", false))); + + // when + String result = HelpSyntaxHelper.getCommandSyntax( + description, new CommandParts(), null, false); + + // then + assertThat(result, equalTo(WHITE + "/authme register" + ITALIC + " ")); + } + + @Test + public void shouldFormatCommandWithMultipleParams() { + // given + CommandDescription description = getDescription(); + description.setArguments(Arrays.asList( + new CommandArgumentDescription("name", "", true), + new CommandArgumentDescription("test", "", false))); + + // when + String result = HelpSyntaxHelper.getCommandSyntax( + description, new CommandParts(), "", false); + + // then + assertThat(result, equalTo(WHITE + "/authme register" + ITALIC + " [name]" + ITALIC + " ")); + } + + @Test + public void shouldHighlightCommandWithMultipleParams() { + // given + CommandDescription description = getDescription(); + description.setArguments(Arrays.asList( + new CommandArgumentDescription("name", "", true), + new CommandArgumentDescription("test", "", false))); + + // when + String result = HelpSyntaxHelper.getCommandSyntax( + description, new CommandParts(), "", true); + + // then + assertThat(result, equalTo(WHITE + "/authme " + + YELLOW + BOLD + "register" + + YELLOW + ITALIC + " [name]" + ITALIC + " ")); + } + + @Test + public void shouldHighlightCommandWithNoParams() { + // given + CommandDescription description = getDescription(); + description.setArguments(new ArrayList()); + + // when + String result = HelpSyntaxHelper.getCommandSyntax( + description, new CommandParts(), null, true); + + // then + assertThat(result, equalTo(WHITE + "/authme " + YELLOW + BOLD + "register" + YELLOW)); + } + + @Test + public void shouldFormatSimpleCommandWithAlternativeLabel() { + // given + CommandDescription description = getDescription(); + + // when + String result = HelpSyntaxHelper.getCommandSyntax( + description, new CommandParts(), "alt", false); + + // then + assertThat(result, equalTo(WHITE + "/authme alt" + ITALIC + " [name]")); + } + + @Test + public void shouldHighlightCommandWithAltLabelAndUnlimitedArguments() { + // given + CommandDescription description = getDescription(); + description.setArguments(Arrays.asList( + new CommandArgumentDescription("name", "", true), + new CommandArgumentDescription("test", "", false))); + description.setMaximumArguments(false); + + // when + String result = HelpSyntaxHelper.getCommandSyntax( + description, new CommandParts(), "test", true); + + // then + assertThat(result, equalTo(WHITE + "/authme " + + YELLOW + BOLD + "test" + + YELLOW + ITALIC + " [name]" + ITALIC + " " + ITALIC + " ...")); + } + + + private static CommandDescription getDescription() { + CommandDescription base = new CommandDescription(new AuthMeCommand(), + singletonList("authme"), + "Base command", + "AuthMe base command", + null); + CommandArgumentDescription userArg = new CommandArgumentDescription( + "name", "", true); + + return new CommandDescription( + new RegisterCommand(), + Arrays.asList("register", "r"), + "Register a player", + "Register the specified player with the specified password.", + base, + singletonList(userArg)); + } +} From b3d0a71dec4c1f3c3b00f815a05003fb5e8c33f6 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 21 Nov 2015 11:57:04 +0100 Subject: [PATCH 26/26] Merge ListUtil into StringUtil; refactor HelpSyntaxHelper + create test The HelpSyntaxHelper had suppressed warnings for string concatenation within StringBuilder - the point of the StringBuilder is that it is faster when you use it to concatenate many elements. If you still use string concatenation with + within these calls it beats the purpose. --- .../fr/xephi/authme/command/CommandParts.java | 15 +++-- .../authme/command/help/HelpSyntaxHelper.java | 63 +++++++++++-------- .../java/fr/xephi/authme/util/ListUtils.java | 58 ----------------- .../fr/xephi/authme/util/StringUtils.java | 35 +++++++++++ .../authme/command/CommandPartsTest.java | 38 +++++++++++ .../fr/xephi/authme/util/StringUtilsTest.java | 41 ++++++++++++ 6 files changed, 159 insertions(+), 91 deletions(-) delete mode 100644 src/main/java/fr/xephi/authme/util/ListUtils.java create mode 100644 src/test/java/fr/xephi/authme/command/CommandPartsTest.java diff --git a/src/main/java/fr/xephi/authme/command/CommandParts.java b/src/main/java/fr/xephi/authme/command/CommandParts.java index 2c67f7b1..9e50cc16 100644 --- a/src/main/java/fr/xephi/authme/command/CommandParts.java +++ b/src/main/java/fr/xephi/authme/command/CommandParts.java @@ -3,7 +3,6 @@ package fr.xephi.authme.command; import java.util.ArrayList; import java.util.List; -import fr.xephi.authme.util.ListUtils; import fr.xephi.authme.util.StringUtils; /** @@ -165,8 +164,8 @@ public class CommandParts { * * @param other The other reference. * - - * @return The result from zero to above. A negative number will be returned on error. */ + * @return The result from zero to above. A negative number will be returned on error. + */ public double getDifference(CommandParts other) { return getDifference(other, false); } @@ -177,8 +176,8 @@ public class CommandParts { * @param other The other reference. * @param fullCompare True to compare the full references as far as the range reaches. * - - * @return The result from zero to above. A negative number will be returned on error. */ + * @return The result from zero to above. A negative number will be returned on error. + */ public double getDifference(CommandParts other, boolean fullCompare) { // Make sure the other reference is correct if(other == null) @@ -196,10 +195,10 @@ public class CommandParts { /** * Convert the parts to a string. * - - * @return The part as a string. */ + * @return The part as a string. + */ @Override public String toString() { - return ListUtils.implode(this.parts, " "); + return StringUtils.join(" ", this.parts); } } diff --git a/src/main/java/fr/xephi/authme/command/help/HelpSyntaxHelper.java b/src/main/java/fr/xephi/authme/command/help/HelpSyntaxHelper.java index 5f564749..0ff99a5c 100644 --- a/src/main/java/fr/xephi/authme/command/help/HelpSyntaxHelper.java +++ b/src/main/java/fr/xephi/authme/command/help/HelpSyntaxHelper.java @@ -1,13 +1,15 @@ package fr.xephi.authme.command.help; +import fr.xephi.authme.util.StringUtils; import org.bukkit.ChatColor; import fr.xephi.authme.command.CommandArgumentDescription; import fr.xephi.authme.command.CommandDescription; import fr.xephi.authme.command.CommandParts; -import fr.xephi.authme.util.ListUtils; /** + * Helper class for formatting a command's structure (name and arguments) + * for a Minecraft user. */ public final class HelpSyntaxHelper { @@ -16,52 +18,63 @@ public final class HelpSyntaxHelper { } /** - * Get the proper syntax for a command. + * Get the formatted syntax for a command. * - * @param commandDescription The command to get the syntax for. + * @param commandDescription The command to build the syntax for. * @param commandReference The reference of the command. * @param alternativeLabel The alternative label to use for this command syntax. * @param highlight True to highlight the important parts of this command. * * @return The command with proper syntax. */ - @SuppressWarnings("StringConcatenationInsideStringBufferAppend") - public static String getCommandSyntax(CommandDescription commandDescription, CommandParts commandReference, String alternativeLabel, boolean highlight) { - // Create a string builder to build the command - StringBuilder sb = new StringBuilder(); - - // Set the color and prefix a slash - sb.append(ChatColor.WHITE + "/"); + public static String getCommandSyntax(CommandDescription commandDescription, CommandParts commandReference, + String alternativeLabel, boolean highlight) { + // Create a string builder with white color and prefixed slash + StringBuilder sb = new StringBuilder() + .append(ChatColor.WHITE) + .append("/"); // Get the help command reference, and the command label CommandParts helpCommandReference = commandDescription.getCommandReference(commandReference); - final String parentCommand = (new CommandParts(helpCommandReference.getRange(0, helpCommandReference.getCount() - 1))).toString(); - String commandLabel = helpCommandReference.get(helpCommandReference.getCount() - 1); + final String parentCommand = new CommandParts( + helpCommandReference.getRange(0, helpCommandReference.getCount() - 1)).toString(); // Check whether the alternative label should be used - if (alternativeLabel != null && alternativeLabel.trim().length() > 0) { + String commandLabel; + if (StringUtils.isEmpty(alternativeLabel)) { + commandLabel = helpCommandReference.get(helpCommandReference.getCount() - 1); + } else { commandLabel = alternativeLabel; } // Show the important bit of the command, highlight this part if required - sb.append(ListUtils.implode(parentCommand, (highlight ? ChatColor.YELLOW + "" + ChatColor.BOLD : "") + commandLabel, " ")); - if(highlight) - sb.append(ChatColor.YELLOW); + sb.append(parentCommand) + .append(" ") + .append(highlight ? ChatColor.YELLOW.toString() + ChatColor.BOLD : "") + .append(commandLabel); - // Add each command arguments - for(CommandArgumentDescription arg : commandDescription.getArguments()) { - // Add the argument as optional or non-optional argument - if(!arg.isOptional()) - sb.append(ChatColor.ITALIC + " <" + arg.getLabel() + ">"); - else - sb.append(ChatColor.ITALIC + " [" + arg.getLabel() + "]"); + if (highlight) { + sb.append(ChatColor.YELLOW); + } + + // Add each command argument + for (CommandArgumentDescription arg : commandDescription.getArguments()) { + sb.append(ChatColor.ITALIC).append(formatArgument(arg)); } // Add some dots if the command allows unlimited arguments - if(commandDescription.getMaximumArguments() < 0) - sb.append(ChatColor.ITALIC + " ..."); + if (commandDescription.getMaximumArguments() < 0) { + sb.append(ChatColor.ITALIC).append(" ..."); + } // Return the build command syntax return sb.toString(); } + + private static String formatArgument(CommandArgumentDescription argument) { + if (argument.isOptional()) { + return " [" + argument.getLabel() + "]"; + } + return " <" + argument.getLabel() + ">"; + } } diff --git a/src/main/java/fr/xephi/authme/util/ListUtils.java b/src/main/java/fr/xephi/authme/util/ListUtils.java deleted file mode 100644 index 69ed092e..00000000 --- a/src/main/java/fr/xephi/authme/util/ListUtils.java +++ /dev/null @@ -1,58 +0,0 @@ -package fr.xephi.authme.util; - -import java.util.ArrayList; -import java.util.List; - -/** - */ -public class ListUtils { - - /** - * Implode a list of elements into a single string, with a specified separator. - * - * @param elements The elements to implode. - * @param separator The separator to use. - * - - * @return The result string. */ - public static String implode(List elements, String separator) { - // Create a string builder - StringBuilder sb = new StringBuilder(); - - // Append each element - for(String element : elements) { - // Make sure the element isn't empty - if(element.trim().length() == 0) - continue; - - // Prefix the separator if it isn't the first element - if(sb.length() > 0) - sb.append(separator); - - // Append the element - sb.append(element); - } - - // Return the result - return sb.toString(); - } - - /** - * Implode two elements into a single string, with a specified separator. - * - * @param element The first element to implode. - * @param otherElement The second element to implode. - * @param separator The separator to use. - * - - * @return The result string. */ - public static String implode(String element, String otherElement, String separator) { - // Combine the lists - List combined = new ArrayList<>(); - combined.add(element); - combined.add(otherElement); - - // Implode and return the result - return implode(combined, separator); - } -} diff --git a/src/main/java/fr/xephi/authme/util/StringUtils.java b/src/main/java/fr/xephi/authme/util/StringUtils.java index e9b495bc..f0dd3d9c 100644 --- a/src/main/java/fr/xephi/authme/util/StringUtils.java +++ b/src/main/java/fr/xephi/authme/util/StringUtils.java @@ -49,4 +49,39 @@ public class StringUtils { return false; } + /** + * Null-safe method for checking whether a string is empty. Note that the string + * is trimmed, so this method also considers a string with whitespace as empty. + * + * @param str the string to verify + * + * @return true if the string is empty, false otherwise + */ + public static boolean isEmpty(String str) { + return str == null || str.trim().isEmpty(); + } + + /** + * Joins a list of elements into a single string with the specified delimiter. + * + * @param delimiter the delimiter to use + * @param elements the elements to join + * + * @return a new String that is composed of the elements separated by the delimiter + */ + public static String join(String delimiter, Iterable elements) { + StringBuilder sb = new StringBuilder(); + for (String element : elements) { + if (!isEmpty(element)) { + // Add the separator if it isn't the first element + if (sb.length() > 0) { + sb.append(delimiter); + } + sb.append(element); + } + } + + return sb.toString(); + } + } diff --git a/src/test/java/fr/xephi/authme/command/CommandPartsTest.java b/src/test/java/fr/xephi/authme/command/CommandPartsTest.java new file mode 100644 index 00000000..62d22b5f --- /dev/null +++ b/src/test/java/fr/xephi/authme/command/CommandPartsTest.java @@ -0,0 +1,38 @@ +package fr.xephi.authme.command; + +import org.junit.Test; + +import java.util.Arrays; + +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThat; + +/** + * Test for {@link CommandParts}. + */ +public class CommandPartsTest { + + @Test + public void shouldPrintPartsForStringRepresentation() { + // given + CommandParts parts = new CommandParts(Arrays.asList("some", "parts", "for", "test")); + + // when + String str = parts.toString(); + + // then + assertThat(str, equalTo("some parts for test")); + } + + @Test + public void shouldPrintEmptyStringForNoArguments() { + // given + CommandParts parts = new CommandParts(); + + // when + String str = parts.toString(); + + // then + assertThat(str, equalTo("")); + } +} diff --git a/src/test/java/fr/xephi/authme/util/StringUtilsTest.java b/src/test/java/fr/xephi/authme/util/StringUtilsTest.java index 668f2059..51cd9677 100644 --- a/src/test/java/fr/xephi/authme/util/StringUtilsTest.java +++ b/src/test/java/fr/xephi/authme/util/StringUtilsTest.java @@ -2,8 +2,13 @@ package fr.xephi.authme.util; import org.junit.Test; +import java.util.Arrays; +import java.util.List; + import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; /** * Test for {@link StringUtils}. @@ -43,4 +48,40 @@ public class StringUtilsTest { // then assertThat(result, equalTo(false)); } + + @Test + public void shouldCheckIsEmptyUtil() { + // Should be true for null/empty/whitespace + assertTrue(StringUtils.isEmpty(null)); + assertTrue(StringUtils.isEmpty("")); + assertTrue(StringUtils.isEmpty(" \t")); + + // Should be false if string has content + assertFalse(StringUtils.isEmpty("P")); + assertFalse(StringUtils.isEmpty(" test")); + } + + @Test + public void shouldJoinString() { + // given + List elements = Arrays.asList("test", "for", null, "join", "StringUtils"); + + // when + String result = StringUtils.join(", ", elements); + + // then + assertThat(result, equalTo("test, for, join, StringUtils")); + } + + @Test + public void shouldNotHaveDelimiter() { + // given + List elements = Arrays.asList(" ", null, "\t", "hello", null); + + // when + String result = StringUtils.join("-", elements); + + // then + assertThat(result, equalTo("hello")); + } }