From 3934d673305020515fbf72c9bee9affe32c749d7 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 21 Nov 2015 09:07:12 +0100 Subject: [PATCH 01/11] 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" (cherry picked from commit 45a50f3) --- .../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 e47311bf9..75d6e4aeb 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 6c3ffe458..e81c705a1 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 1d625d9c7..9fbcd1d9f 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 a627c9343..0e036b0d4 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 541154e70..da67fa82f 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 170685c01..04b4a0199 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 13555552d..a6401b0a3 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 19e1e67ca..0a9fd9444 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 1ca2a8518..4ee1fe066 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 f3b2700a4..343642c5a 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 58237ef80..b2c4c70f8 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 dc258362b..9b6956b25 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 34d6bba47..68f5e54b1 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 03a3c156b..d295512e3 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 748fed46f..44f007a95 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 d6b441642..e9b80cdfc 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 fcc86ce2e..e2e97806e 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 7484793d2..3c7f07ee4 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 987e38c5dfab200d88381883f51d8fcc5dcaf624 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 21 Nov 2015 09:49:39 +0100 Subject: [PATCH 02/11] 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. (cherry picked from commit f1a0022) --- 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 908dacc2b..3b8d9f142 100644 --- a/src/main/java/fr/xephi/authme/AuthMe.java +++ b/src/main/java/fr/xephi/authme/AuthMe.java @@ -1028,4 +1028,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 e9b80cdfc..2479a1058 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 000000000..6f4ca7a46 --- /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 000000000..b8c3c2f5e --- /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 e65319d42c086dcace1914d7cbe498542f328804 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 21 Nov 2015 10:29:40 +0100 Subject: [PATCH 03/11] Add tests for LogoutCommand and RegisterCommand. Add more generic mockUtil (cherry picked from commit 06cfd13) --- .../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 e2e97806e..60c14a268 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 3c7f07ee4..6e89fc62f 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 6f4ca7a46..905c46303 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 b8c3c2f5e..5ed42876c 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 000000000..8416b5fdc --- /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 000000000..0b2ba2cd0 --- /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 115680a363e81424158ac9c8c59c24223d4c4144 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 21 Nov 2015 11:16:31 +0100 Subject: [PATCH 04/11] Create test for HelpSyntaxHelperTest (cherry picked from commit 9a6e96d) --- .../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 51ce67b0b..4feec54d3 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 162d61d46..be5b28bf8 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 55f2581ac..5f564749b 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 05ede1ba1..69ed092ef 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 000000000..3fba70254 --- /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 4702a1b82d4774c50261f7791e658eb5b1cc88b2 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 21 Nov 2015 11:57:04 +0100 Subject: [PATCH 05/11] 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. (cherry picked from commit bb00be2) --- .../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 | 61 ++++++++++--- .../authme/command/CommandPartsTest.java | 38 ++++++++ .../fr/xephi/authme/util/StringUtilsTest.java | 87 +++++++++++++++++++ 6 files changed, 220 insertions(+), 102 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 create mode 100644 src/test/java/fr/xephi/authme/util/StringUtilsTest.java diff --git a/src/main/java/fr/xephi/authme/command/CommandParts.java b/src/main/java/fr/xephi/authme/command/CommandParts.java index 2c67f7b1b..9e50cc164 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 5f564749b..0ff99a5cf 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 69ed092ef..000000000 --- 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 c84fc74a5..f0dd3d9ce 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,60 @@ 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; } + + /** + * 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 000000000..62d22b5fa --- /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 new file mode 100644 index 000000000..51cd96776 --- /dev/null +++ b/src/test/java/fr/xephi/authme/util/StringUtilsTest.java @@ -0,0 +1,87 @@ +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}. + */ +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)); + } + + @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")); + } +} From b633b9a005bb5523ced605b60e3e7d01a140d444 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 21 Nov 2015 13:23:18 +0100 Subject: [PATCH 06/11] Create test for change password command --- .../changepassword/ChangePasswordCommand.java | 19 +- .../java/fr/xephi/authme/AuthMeMockUtil.java | 9 + .../ChangePasswordCommandTest.java | 166 ++++++++++++++++++ 3 files changed, 184 insertions(+), 10 deletions(-) create mode 100644 src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java 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 9b6956b25..e3ba71b4f 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 @@ -17,17 +17,17 @@ public class ChangePasswordCommand extends ExecutableCommand { @Override public boolean executeCommand(CommandSender sender, CommandParts commandReference, CommandParts commandArguments) { + // Make sure the current command executor is a player + if (!(sender instanceof Player)) { + return true; + } + final Messages m = Messages.getInstance(); // Get the passwords String playerPass = commandArguments.get(0); String playerPassVerify = commandArguments.get(1); - // Make sure the current command executor is a player - if(!(sender instanceof Player)) { - return true; - } - // Get the player instance and make sure it's authenticated Player player = (Player) sender; String name = player.getName().toLowerCase(); @@ -37,6 +37,7 @@ public class ChangePasswordCommand extends ExecutableCommand { } // Make sure the password is allowed + // TODO ljacqu 20151121: The password confirmation appears to be never verified String playerPassLowerCase = playerPass.toLowerCase(); if (playerPassLowerCase.contains("delete") || playerPassLowerCase.contains("where") || playerPassLowerCase.contains("insert") || playerPassLowerCase.contains("modify") || playerPassLowerCase.contains("from") || playerPassLowerCase.contains("select") || playerPassLowerCase.contains(";") || playerPassLowerCase.contains("null") || !playerPassLowerCase.matches(Settings.getPassRegex)) { m.send(player, "password_error"); @@ -50,11 +51,9 @@ public class ChangePasswordCommand extends ExecutableCommand { m.send(player, "pass_len"); return true; } - if (!Settings.unsafePasswords.isEmpty()) { - if (Settings.unsafePasswords.contains(playerPassLowerCase)) { - m.send(player, "password_error_unsafe"); - return true; - } + if (!Settings.unsafePasswords.isEmpty() && Settings.unsafePasswords.contains(playerPassLowerCase)) { + m.send(player, "password_error_unsafe"); + return true; } // Set the password diff --git a/src/test/java/fr/xephi/authme/AuthMeMockUtil.java b/src/test/java/fr/xephi/authme/AuthMeMockUtil.java index 905c46303..f3ab89719 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.cache.auth.PlayerCache; import fr.xephi.authme.settings.Messages; import org.mockito.Mockito; @@ -30,6 +31,14 @@ public final class AuthMeMockUtil { mockSingletonForClass(Messages.class, "singleton", mock); } + /** + * Creates a mock singleton for the player cache, retrievable from {@link PlayerCache#getInstance()}. + */ + public static void mockPlayerCacheInstance() { + PlayerCache mock = Mockito.mock(PlayerCache.class); + mockSingletonForClass(PlayerCache.class, "singleton", mock); + } + /** * Sets a field of a class to the given mock. * diff --git a/src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java new file mode 100644 index 000000000..d991797e1 --- /dev/null +++ b/src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java @@ -0,0 +1,166 @@ +package fr.xephi.authme.command.executable.changepassword; + +import fr.xephi.authme.AuthMeMockUtil; +import fr.xephi.authme.cache.auth.PlayerCache; +import fr.xephi.authme.command.CommandParts; +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 java.util.Arrays; + +import static org.mockito.Matchers.anyInt; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Test for {@link ChangePasswordCommand}. + */ +public class ChangePasswordCommandTest { + + private Messages messagesMock; + private PlayerCache cacheMock; + + @Before + public void setUpMocks() { + AuthMeMockUtil.mockAuthMeInstance(); + AuthMeMockUtil.mockPlayerCacheInstance(); + cacheMock = PlayerCache.getInstance(); + + AuthMeMockUtil.mockMessagesInstance(); + messagesMock = Messages.getInstance(); + + // Only allow passwords with alphanumerical characters for the test + Settings.getPassRegex = "[a-zA-Z0-9]+"; + Settings.getPasswordMinLen = 2; + Settings.passwordMaxLength = 50; + + // TODO ljacqu 20151121: Cannot mock getServer() as it's final + // Probably the Command class should delegate as the others do + /* + Server server = Mockito.mock(Server.class); + schedulerMock = Mockito.mock(BukkitScheduler.class); + Mockito.when(server.getScheduler()).thenReturn(schedulerMock); + Mockito.when(pluginMock.getServer()).thenReturn(server); + */ + } + + @Test + public void shouldRejectNonPlayerSender() { + // given + CommandSender sender = mock(BlockCommandSender.class); + ChangePasswordCommand command = new ChangePasswordCommand(); + CommandParts arguments = mock(CommandParts.class); + + // when + command.executeCommand(sender, new CommandParts(), arguments); + + // then + verify(arguments, never()).get(anyInt()); + //verify(pluginMock, never()).getServer(); + } + + @Test + public void shouldRejectNotLoggedInPlayer() { + // given + CommandSender sender = initPlayerWithName("name", false); + ChangePasswordCommand command = new ChangePasswordCommand(); + + // when + command.executeCommand(sender, new CommandParts(), new CommandParts("pass")); + + // then + verify(messagesMock).send(sender, "not_logged_in"); + //verify(pluginMock, never()).getServer(); + } + + @Test + public void shouldDenyInvalidPassword() { + // given + CommandSender sender = initPlayerWithName("name", true); + ChangePasswordCommand command = new ChangePasswordCommand(); + + // when + command.executeCommand(sender, new CommandParts(), new CommandParts("!pass")); + + // then + verify(messagesMock).send(sender, "password_error"); + //verify(pluginMock, never()).getServer(); + } + + + @Test + public void shouldRejectPasswordEqualToNick() { + // given + CommandSender sender = initPlayerWithName("tester", true); + ChangePasswordCommand command = new ChangePasswordCommand(); + + // when + command.executeCommand(sender, new CommandParts(), new CommandParts("Tester")); + + // then + verify(messagesMock).send(sender, "password_error_nick"); + //verify(pluginMock, never()).getServer(); + } + + @Test + public void shouldRejectTooLongPassword() { + // given + CommandSender sender = initPlayerWithName("abc12", true); + ChangePasswordCommand command = new ChangePasswordCommand(); + Settings.passwordMaxLength = 3; + + // when + command.executeCommand(sender, new CommandParts(), new CommandParts("test")); + + // then + verify(messagesMock).send(sender, "pass_len"); + //verify(pluginMock, never()).getServer(); + } + + @Test + public void shouldRejectTooShortPassword() { + // given + CommandSender sender = initPlayerWithName("abc12", true); + ChangePasswordCommand command = new ChangePasswordCommand(); + Settings.getPasswordMinLen = 7; + + // when + command.executeCommand(sender, new CommandParts(), new CommandParts("tester")); + + // then + verify(messagesMock).send(sender, "pass_len"); + //verify(pluginMock, never()).getServer(); + } + + @Test + public void shouldRejectUnsafeCustomPassword() { + // given + CommandSender sender = initPlayerWithName("player", true); + ChangePasswordCommand command = new ChangePasswordCommand(); + Settings.unsafePasswords = Arrays.asList("test", "abc123"); + + // when + command.executeCommand(sender, new CommandParts(), new CommandParts("abc123")); + + // then + verify(messagesMock).send(sender, "password_error_unsafe"); + //verify(pluginMock, never()).getServer(); + } + + private Player initPlayerWithName(String name, boolean loggedIn) { + Player player = mock(Player.class); + when(player.getName()).thenReturn(name); + when(cacheMock.isAuthenticated(name)).thenReturn(loggedIn); + return player; + } + +} From efb57989ed93d3766dc635d11cd250436ea0e656 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 21 Nov 2015 19:51:49 +0100 Subject: [PATCH 07/11] Start tests for email commands - Removed randomStringGenerator from Management as it is unused --- .../executable/email/AddEmailCommand.java | 10 +-- .../executable/email/ChangeEmailCommand.java | 10 +-- .../executable/email/RecoverEmailCommand.java | 8 +-- .../fr/xephi/authme/process/Management.java | 58 +---------------- .../executable/email/AddEmailCommandTest.java | 63 +++++++++++++++++++ .../email/ChangeEmailCommandTest.java | 63 +++++++++++++++++++ .../email/RecoverEmailCommandTest.java | 36 +++++++++++ 7 files changed, 178 insertions(+), 70 deletions(-) create mode 100644 src/test/java/fr/xephi/authme/command/executable/email/AddEmailCommandTest.java create mode 100644 src/test/java/fr/xephi/authme/command/executable/email/ChangeEmailCommandTest.java create mode 100644 src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java 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 68f5e54b1..a185fed98 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 @@ -14,19 +14,19 @@ public class AddEmailCommand extends ExecutableCommand { @Override public boolean executeCommand(CommandSender sender, CommandParts commandReference, CommandParts commandArguments) { - // Get the parameter values - String playerMail = commandArguments.get(0); - String playerMailVerify = commandArguments.get(1); - // Make sure the current command executor is a player if (!(sender instanceof Player)) { return true; } + // Get the parameter values + String playerMail = commandArguments.get(0); + String playerMailVerify = commandArguments.get(1); + // Get the player and perform email addition final AuthMe plugin = AuthMe.getInstance(); final Player player = (Player) sender; - plugin.management.performAddEmail(player, playerMail, playerMailVerify); + plugin.getManagement().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 d295512e3..2aa3972c0 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 @@ -13,19 +13,19 @@ public class ChangeEmailCommand extends ExecutableCommand { @Override public boolean executeCommand(CommandSender sender, CommandParts commandReference, CommandParts commandArguments) { - // Get the parameter values - String playerMailOld = commandArguments.get(0); - String playerMailNew = commandArguments.get(1); - // Make sure the current command executor is a player if (!(sender instanceof Player)) { return true; } + // Get the parameter values + String playerMailOld = commandArguments.get(0); + String playerMailNew = commandArguments.get(1); + // Get the player instance and execute action final AuthMe plugin = AuthMe.getInstance(); final Player player = (Player) sender; - plugin.management.performChangeEmail(player, playerMailOld, playerMailNew); + plugin.getManagement().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 44f007a95..5429a005a 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 @@ -22,14 +22,14 @@ public class RecoverEmailCommand extends ExecutableCommand { @Override public boolean executeCommand(CommandSender sender, CommandParts commandReference, CommandParts commandArguments) { - // Get the parameter values - String playerMail = commandArguments.get(0); - // Make sure the current command executor is a player - if(!(sender instanceof Player)) { + if (!(sender instanceof Player)) { return true; } + // Get the parameter values + String playerMail = commandArguments.get(0); + // Get the player instance and name final Player player = (Player) sender; final String playerName = player.getName(); diff --git a/src/main/java/fr/xephi/authme/process/Management.java b/src/main/java/fr/xephi/authme/process/Management.java index 1030eeddc..e917e352e 100644 --- a/src/main/java/fr/xephi/authme/process/Management.java +++ b/src/main/java/fr/xephi/authme/process/Management.java @@ -1,8 +1,5 @@ package fr.xephi.authme.process; -import org.bukkit.entity.Player; -import org.bukkit.scheduler.BukkitScheduler; - import fr.xephi.authme.AuthMe; import fr.xephi.authme.process.email.AsyncChangeEmail; import fr.xephi.authme.process.join.AsyncronousJoin; @@ -11,35 +8,21 @@ import fr.xephi.authme.process.logout.AsyncronousLogout; import fr.xephi.authme.process.quit.AsyncronousQuit; import fr.xephi.authme.process.register.AsyncRegister; import fr.xephi.authme.process.unregister.AsyncronousUnregister; -import fr.xephi.authme.security.RandomString; -import fr.xephi.authme.settings.Settings; +import org.bukkit.entity.Player; +import org.bukkit.scheduler.BukkitScheduler; /** - - * @author Gabriele - * @version $Revision: 1.0 $ */ public class Management { private final AuthMe plugin; private final BukkitScheduler sched; - public static RandomString rdm = new RandomString(Settings.captchaLength); - /** - * Constructor for Management. - * @param plugin AuthMe - */ public Management(AuthMe plugin) { this.plugin = plugin; this.sched = this.plugin.getServer().getScheduler(); } - /** - * Method performLogin. - * @param player Player - * @param password String - * @param forceLogin boolean - */ public void performLogin(final Player player, final String password, final boolean forceLogin) { sched.runTaskAsynchronously(plugin, new Runnable() { @@ -50,10 +33,6 @@ public class Management { }); } - /** - * Method performLogout. - * @param player Player - */ public void performLogout(final Player player) { sched.runTaskAsynchronously(plugin, new Runnable() { @@ -64,12 +43,6 @@ public class Management { }); } - /** - * Method performRegister. - * @param player Player - * @param password String - * @param email String - */ public void performRegister(final Player player, final String password, final String email) { sched.runTaskAsynchronously(plugin, new Runnable() { @@ -80,12 +53,6 @@ public class Management { }); } - /** - * Method performUnregister. - * @param player Player - * @param password String - * @param force boolean - */ public void performUnregister(final Player player, final String password, final boolean force) { sched.runTaskAsynchronously(plugin, new Runnable() { @@ -96,10 +63,6 @@ public class Management { }); } - /** - * Method performJoin. - * @param player Player - */ public void performJoin(final Player player) { sched.runTaskAsynchronously(plugin, new Runnable() { @@ -111,11 +74,6 @@ public class Management { }); } - /** - * Method performQuit. - * @param player Player - * @param isKick boolean - */ public void performQuit(final Player player, final boolean isKick) { sched.runTaskAsynchronously(plugin, new Runnable() { @@ -127,12 +85,6 @@ public class Management { }); } - /** - * Method performAddEmail. - * @param player Player - * @param newEmail String - * @param newEmailVerify String - */ public void performAddEmail(final Player player, final String newEmail, final String newEmailVerify) { sched.runTaskAsynchronously(plugin, new Runnable() { @Override @@ -142,12 +94,6 @@ public class Management { }); } - /** - * Method performChangeEmail. - * @param player Player - * @param oldEmail String - * @param newEmail String - */ public void performChangeEmail(final Player player, final String oldEmail, final String newEmail) { sched.runTaskAsynchronously(plugin, new Runnable() { @Override diff --git a/src/test/java/fr/xephi/authme/command/executable/email/AddEmailCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/email/AddEmailCommandTest.java new file mode 100644 index 000000000..795fe1bec --- /dev/null +++ b/src/test/java/fr/xephi/authme/command/executable/email/AddEmailCommandTest.java @@ -0,0 +1,63 @@ +package fr.xephi.authme.command.executable.email; + +import fr.xephi.authme.AuthMe; +import fr.xephi.authme.AuthMeMockUtil; +import fr.xephi.authme.command.CommandParts; +import fr.xephi.authme.process.Management; +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 java.util.Arrays; + +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Test for {@link AddEmailCommand}. + */ +public class AddEmailCommandTest { + + private AuthMe authMeMock; + private Management managementMock; + + @Before + public void setUpMocks() { + AuthMeMockUtil.mockAuthMeInstance(); + authMeMock = AuthMe.getInstance(); + managementMock = Mockito.mock(Management.class); + when(authMeMock.getManagement()).thenReturn(managementMock); + } + + @Test + public void shouldRejectNonPlayerSender() { + // given + CommandSender sender = Mockito.mock(BlockCommandSender.class); + AddEmailCommand command = new AddEmailCommand(); + + // when + command.executeCommand(sender, new CommandParts(), new CommandParts()); + + // then + verify(authMeMock, never()).getManagement(); + } + + @Test + public void shouldForwardData() { + // given + Player sender = Mockito.mock(Player.class); + AddEmailCommand command = new AddEmailCommand(); + + // when + command.executeCommand(sender, new CommandParts(), + new CommandParts(Arrays.asList("mail@example", "other_example"))); + + // then + verify(authMeMock).getManagement(); + verify(managementMock).performAddEmail(sender, "mail@example", "other_example"); + } +} diff --git a/src/test/java/fr/xephi/authme/command/executable/email/ChangeEmailCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/email/ChangeEmailCommandTest.java new file mode 100644 index 000000000..db6e6b84e --- /dev/null +++ b/src/test/java/fr/xephi/authme/command/executable/email/ChangeEmailCommandTest.java @@ -0,0 +1,63 @@ +package fr.xephi.authme.command.executable.email; + +import fr.xephi.authme.AuthMe; +import fr.xephi.authme.AuthMeMockUtil; +import fr.xephi.authme.command.CommandParts; +import fr.xephi.authme.process.Management; +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 java.util.Arrays; + +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Test for {@link ChangeEmailCommand}. + */ +public class ChangeEmailCommandTest { + + private AuthMe authMeMock; + private Management managementMock; + + @Before + public void setUpMocks() { + AuthMeMockUtil.mockAuthMeInstance(); + authMeMock = AuthMe.getInstance(); + managementMock = Mockito.mock(Management.class); + when(authMeMock.getManagement()).thenReturn(managementMock); + } + + @Test + public void shouldRejectNonPlayerSender() { + // given + CommandSender sender = Mockito.mock(BlockCommandSender.class); + ChangeEmailCommand command = new ChangeEmailCommand(); + + // when + command.executeCommand(sender, new CommandParts(), new CommandParts()); + + // then + verify(authMeMock, never()).getManagement(); + } + + @Test + public void shouldForwardData() { + // given + Player sender = Mockito.mock(Player.class); + ChangeEmailCommand command = new ChangeEmailCommand(); + + // when + command.executeCommand(sender, new CommandParts(), + new CommandParts(Arrays.asList("new.mail@example.org", "old_mail@example.org"))); + + // then + verify(authMeMock).getManagement(); + verify(managementMock).performChangeEmail(sender, "new.mail@example.org", "old_mail@example.org"); + } +} diff --git a/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java new file mode 100644 index 000000000..bd1e2e442 --- /dev/null +++ b/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java @@ -0,0 +1,36 @@ +package fr.xephi.authme.command.executable.email; + +import fr.xephi.authme.AuthMe; +import fr.xephi.authme.AuthMeMockUtil; +import fr.xephi.authme.command.CommandParts; +import org.bukkit.command.BlockCommandSender; +import org.bukkit.command.CommandSender; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mockito; + +/** + * Test for {@link RecoverEmailCommand}. + */ +public class RecoverEmailCommandTest { + + @Before + public void setUpMocks() { + AuthMeMockUtil.mockAuthMeInstance(); + } + + @Test + public void shouldRejectNonPlayerSender() { + // given + CommandSender sender = Mockito.mock(BlockCommandSender.class); + RecoverEmailCommand command = new RecoverEmailCommand(); + + // when + command.executeCommand(sender, new CommandParts(), new CommandParts()); + + // then + } + + // TODO ljacqu 20151121: Expand tests. This command doesn't use a scheduler and has all of its + // logic inside here. +} From cd728b569ee1c3f3392fd2ae60331da6739574d8 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 21 Nov 2015 22:32:16 +0100 Subject: [PATCH 08/11] Create test for CommandManager; fix javadoc in CommandDescription --- .../authme/command/CommandDescription.java | 132 ++++++++---------- .../authme/command/CommandManagerTest.java | 41 ++++++ 2 files changed, 99 insertions(+), 74 deletions(-) create mode 100644 src/test/java/fr/xephi/authme/command/CommandManagerTest.java diff --git a/src/main/java/fr/xephi/authme/command/CommandDescription.java b/src/main/java/fr/xephi/authme/command/CommandDescription.java index 4feec54d3..53054069e 100644 --- a/src/main/java/fr/xephi/authme/command/CommandDescription.java +++ b/src/main/java/fr/xephi/authme/command/CommandDescription.java @@ -96,27 +96,13 @@ public class CommandDescription { setArguments(arguments); } - /** - * Get the first relative command label. - * - - * @return First relative command label. */ - public String getLabel() { - // Ensure there's any item in the command list - if(this.labels.size() == 0) - return ""; - - // Return the first command on the list - return this.labels.get(0); - } - /** * Get the label most similar to the reference. The first label will be returned if no reference was supplied. * * @param reference The command reference. * - - * @return The most similar label, or the first label. An empty label will be returned if no label was set. */ + * @return The most similar label, or the first label. An empty label will be returned if no label was set. + */ public String getLabel(CommandParts reference) { // Ensure there's any item in the command list if(this.labels.size() == 0) @@ -147,8 +133,8 @@ public class CommandDescription { /** * Get all relative command labels. * - - * @return All relative labels labels. */ + * @return All relative labels labels. + */ public List getLabels() { return this.labels; } @@ -160,11 +146,11 @@ public class CommandDescription { */ public void setLabels(List labels) { // Check whether the command label list should be cleared - if(labels == null) + if (labels == null) { this.labels.clear(); - - else + } else { this.labels = labels; + } } /** @@ -307,8 +293,9 @@ public class CommandDescription { /** * Get the absolute command label, without a slash. - - * @return String */ + * + * @return the absolute label + */ public String getAbsoluteLabel() { return getAbsoluteLabel(false); } @@ -316,9 +303,9 @@ public class CommandDescription { /** * Get the absolute command label. * - * @param includeSlash boolean - * @return Absolute command label. */ + * @return Absolute command label. + */ public String getAbsoluteLabel(boolean includeSlash) { return getAbsoluteLabel(includeSlash, null); } @@ -326,10 +313,10 @@ public class CommandDescription { /** * Get the absolute command label. * - * @param includeSlash boolean * @param reference CommandParts - * @return Absolute command label. */ + * @return Absolute command label. + */ public String getAbsoluteLabel(boolean includeSlash, CommandParts reference) { // Get the command reference, and make sure it is valid CommandParts out = getCommandReference(reference); @@ -345,8 +332,8 @@ public class CommandDescription { * * @param reference The reference to use as template, which is used to choose the most similar reference. * - - * @return Command reference. */ + * @return Command reference. + */ public CommandParts getCommandReference(CommandParts reference) { // Build the reference List referenceList = new ArrayList<>(); @@ -367,8 +354,8 @@ public class CommandDescription { * * @param other The other command reference. * - - * @return The command difference. Zero if there's no difference. A negative number on error. */ + * @return The command difference. Zero if there's no difference. A negative number on error. + */ public double getCommandDifference(CommandParts other) { return getCommandDifference(other, false); } @@ -379,8 +366,8 @@ public class CommandDescription { * @param other The other command reference. * @param fullCompare True to fully compare both command references. * - - * @return The command difference. Zero if there's no difference. A negative number on error. */ + * @return The command difference. Zero if there's no difference. A negative number on error. + */ public double getCommandDifference(CommandParts other, boolean fullCompare) { // Make sure the reference is valid if(other == null) @@ -396,8 +383,8 @@ public class CommandDescription { /** * Get the executable command. * - - * @return The executable command. */ + * @return The executable command. + */ public ExecutableCommand getExecutableCommand() { return this.executableCommand; } @@ -414,8 +401,8 @@ public class CommandDescription { /** * Check whether this command is executable, based on the assigned executable command. * - - * @return True if this command is executable. */ + * @return True if this command is executable. + */ public boolean isExecutable() { return this.executableCommand != null; } @@ -427,8 +414,8 @@ public class CommandDescription { * @param commandReference The command reference. * @param commandArguments The command arguments. * - - * @return True on success, false on failure. */ + * @return True on success, false on failure. + */ public boolean execute(CommandSender sender, CommandParts commandReference, CommandParts commandArguments) { // Make sure the command is executable if(!isExecutable()) @@ -592,8 +579,8 @@ public class CommandDescription { /** * Get all command arguments. * - - * @return Command arguments. */ + * @return Command arguments. + */ public List getArguments() { return this.arguments; } @@ -605,11 +592,11 @@ public class CommandDescription { */ public void setArguments(List arguments) { // Convert null into an empty argument list - if(arguments == null) + if (arguments == null) { this.arguments.clear(); - - else + } else { this.arguments = arguments; + } } /** @@ -617,22 +604,17 @@ public class CommandDescription { * * @param argument The argument to check for. * - - * @return True if this argument already exists, false otherwise. */ + * @return True if this argument already exists, false otherwise. + */ public boolean hasArgument(CommandArgumentDescription argument) { - // Make sure the argument is valid - if(argument == null) - return false; - - // Check whether the argument exists, return the result - return this.arguments.contains(argument); + return argument != null && arguments.contains(argument); } /** * Check whether this command has any arguments. * - - * @return True if this command has any arguments. */ + * @return True if this command has any arguments. + */ public boolean hasArguments() { return (this.arguments.size() != 0); } @@ -640,8 +622,8 @@ public class CommandDescription { /** * The minimum number of arguments required for this command. * - - * @return The minimum number of required arguments. */ + * @return The minimum number of required arguments. + */ public int getMinimumArguments() { // Get the number of required and optional arguments int requiredArguments = 0; @@ -838,23 +820,26 @@ public class CommandDescription { * * @param commandReference The command reference. * - - * @return The difference in argument count between the reference and the actual command. */ + * @return The difference in argument count between the reference and the actual command. + */ public int getSuitableArgumentsDifference(CommandParts commandReference) { // Make sure the command reference is valid - if(commandReference.getCount() <= 0) + if (commandReference.getCount() <= 0) { return -1; + } // Get the remaining command reference element count int remainingElementCount = commandReference.getCount() - getParentCount() - 1; - // Check if there are too less arguments - if(getMinimumArguments() > remainingElementCount) + // Check if there are too few arguments + if (getMinimumArguments() > remainingElementCount) { return Math.abs(getMinimumArguments() - remainingElementCount); + } // Check if there are too many arguments - if(getMaximumArguments() < remainingElementCount && getMaximumArguments() >= 0) + if (getMaximumArguments() < remainingElementCount && getMaximumArguments() >= 0) { return Math.abs(remainingElementCount - getMaximumArguments()); + } // The arguments seem to be EQUALS, return the result return 0; @@ -863,8 +848,8 @@ public class CommandDescription { /** * Get the command permissions. * - - * @return The command permissions. */ + * @return The command permissions. + */ public CommandPermissions getCommandPermissions() { return this.permissions; } @@ -894,30 +879,29 @@ public class CommandDescription { * @param commandLabel The first command label. * @param otherCommandLabel The other command label. * - - * @return True if the labels are equal to each other. */ + * @return True if the labels are equal to each other. + */ private static boolean commandLabelEquals(String commandLabel, String otherCommandLabel) { - // Trim the command labels from unwanted whitespaces commandLabel = commandLabel.trim(); otherCommandLabel = otherCommandLabel.trim(); - - // Check whether the the two command labels are equal (case insensitive) - return (commandLabel.equalsIgnoreCase(otherCommandLabel)); + return commandLabel.equalsIgnoreCase(otherCommandLabel); } /** * Check whether the command description has been set up properly. * - - * @return True if the command description is valid, false otherwise. */ + * @return True if the command description is valid, false otherwise. + */ public boolean isValid() { // Make sure any command label is set - if(getLabels().size() == 0) + if (getLabels().size() == 0) { return false; + } // Make sure the permissions are set up properly - if(this.permissions == null) + if (this.permissions == null) { return false; + } // Everything seems to be correct, return the result return true; diff --git a/src/test/java/fr/xephi/authme/command/CommandManagerTest.java b/src/test/java/fr/xephi/authme/command/CommandManagerTest.java new file mode 100644 index 000000000..9309d4a00 --- /dev/null +++ b/src/test/java/fr/xephi/authme/command/CommandManagerTest.java @@ -0,0 +1,41 @@ +package fr.xephi.authme.command; + +import org.junit.Test; + +import java.util.List; + +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThat; + +/** + * Test for {@link CommandManager}. + */ +public class CommandManagerTest { + + @Test + public void shouldInitializeCommands() { + // given/when + CommandManager manager = new CommandManager(true); + int commandCount = manager.getCommandDescriptionCount(); + List commands = manager.getCommandDescriptions(); + + // then + // It obviously doesn't make sense to test much of the concrete data + // that is being initialized; we just want to guarantee with this test + // that data is indeed being initialized and we take a few "probes" + assertThat(commandCount, equalTo(9)); + assertThat(commandsIncludeLabel(commands, "authme"), equalTo(true)); + assertThat(commandsIncludeLabel(commands, "register"), equalTo(true)); + assertThat(commandsIncludeLabel(commands, "help"), equalTo(false)); + } + + private static boolean commandsIncludeLabel(Iterable commands, String label) { + for (CommandDescription command : commands) { + if (command.getLabels().contains(label)) { + return true; + } + } + return false; + } + +} From 43a60dc091038ce48e6624740f9e4fc741554b34 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Wed, 25 Nov 2015 22:21:37 +0100 Subject: [PATCH 09/11] Change lost in merge --- src/main/java/fr/xephi/authme/process/Management.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/fr/xephi/authme/process/Management.java b/src/main/java/fr/xephi/authme/process/Management.java index e6fe00220..58ce248f7 100644 --- a/src/main/java/fr/xephi/authme/process/Management.java +++ b/src/main/java/fr/xephi/authme/process/Management.java @@ -7,7 +7,7 @@ import fr.xephi.authme.process.login.AsynchronousLogin; import fr.xephi.authme.process.logout.AsynchronousLogout; import fr.xephi.authme.process.quit.AsynchronousQuit; import fr.xephi.authme.process.register.AsyncRegister; -import fr.xephi.authme.process.unregister.AsyncronousUnregister; +import fr.xephi.authme.process.unregister.AsynchronousUnregister; import org.bukkit.entity.Player; import org.bukkit.scheduler.BukkitScheduler; From 498c3342f25e90b2b46dc7f81b55b09defcd4013 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Wed, 25 Nov 2015 22:34:27 +0100 Subject: [PATCH 10/11] Create Wrapper as singleton; fix UtilsTest --- src/main/java/fr/xephi/authme/AntiBot.java | 2 +- .../java/fr/xephi/authme/ConsoleLogger.java | 2 +- .../java/fr/xephi/authme/util/GeoLiteAPI.java | 1 - src/main/java/fr/xephi/authme/util/Utils.java | 11 +-- .../java/fr/xephi/authme/util/Wrapper.java | 21 +++-- .../java/fr/xephi/authme/AuthMeMockUtil.java | 10 ++- .../java/fr/xephi/authme/util/UtilsTest.java | 81 +++++++++++-------- .../xephi/authme/{ => util}/WrapperMock.java | 10 +-- 8 files changed, 81 insertions(+), 57 deletions(-) rename src/test/java/fr/xephi/authme/{ => util}/WrapperMock.java (86%) diff --git a/src/main/java/fr/xephi/authme/AntiBot.java b/src/main/java/fr/xephi/authme/AntiBot.java index 3b8d0afc8..72be5ac7f 100644 --- a/src/main/java/fr/xephi/authme/AntiBot.java +++ b/src/main/java/fr/xephi/authme/AntiBot.java @@ -17,7 +17,7 @@ public class AntiBot { private static final AuthMe plugin = AuthMe.getInstance(); private static final Messages messages = plugin.getMessages(); - private static Wrapper wrapper = new Wrapper(plugin); + private static Wrapper wrapper = Wrapper.getInstance(); private static final List antibotPlayers = new ArrayList<>(); private static AntiBotStatus antiBotStatus = AntiBotStatus.DISABLED; diff --git a/src/main/java/fr/xephi/authme/ConsoleLogger.java b/src/main/java/fr/xephi/authme/ConsoleLogger.java index 6009bc3a1..890318046 100644 --- a/src/main/java/fr/xephi/authme/ConsoleLogger.java +++ b/src/main/java/fr/xephi/authme/ConsoleLogger.java @@ -17,7 +17,7 @@ import java.util.Date; */ public final class ConsoleLogger { - private static Wrapper wrapper = new Wrapper(AuthMe.getInstance()); + private static Wrapper wrapper = Wrapper.getInstance(); private static final DateFormat df = new SimpleDateFormat("[MM-dd HH:mm:ss]"); private ConsoleLogger() { diff --git a/src/main/java/fr/xephi/authme/util/GeoLiteAPI.java b/src/main/java/fr/xephi/authme/util/GeoLiteAPI.java index b9d730e7a..ebd0deb31 100644 --- a/src/main/java/fr/xephi/authme/util/GeoLiteAPI.java +++ b/src/main/java/fr/xephi/authme/util/GeoLiteAPI.java @@ -17,7 +17,6 @@ public class GeoLiteAPI { "available at http://www.maxmind.com"; private static final String GEOIP_URL = "http://geolite.maxmind.com/download/geoip/database/GeoLiteCountry" + "/GeoIP.dat.gz"; - private static final Wrapper wrapper = new Wrapper(AuthMe.getInstance()); private static final AuthMe plugin = AuthMe.getInstance(); private static LookupService lookupService; diff --git a/src/main/java/fr/xephi/authme/util/Utils.java b/src/main/java/fr/xephi/authme/util/Utils.java index 327fc43dc..33bf06d47 100644 --- a/src/main/java/fr/xephi/authme/util/Utils.java +++ b/src/main/java/fr/xephi/authme/util/Utils.java @@ -26,7 +26,7 @@ import java.util.Collections; */ public final class Utils { - private static final AuthMe plugin; + private static AuthMe plugin; private static Wrapper wrapper; private static boolean getOnlinePlayersIsCollection = false; @@ -34,7 +34,7 @@ public final class Utils { static { plugin = AuthMe.getInstance(); - wrapper = new Wrapper(plugin); + wrapper = Wrapper.getInstance(); initializeOnlinePlayersIsCollectionField(); } @@ -116,9 +116,10 @@ public final class Utils { // 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; + if (permsMan == null) { + ConsoleLogger.showError("Failed to access permissions manager instance, aborting."); + return false; + } // Remove old groups permsMan.removeGroups(player, Arrays.asList(Settings.unRegisteredGroup, diff --git a/src/main/java/fr/xephi/authme/util/Wrapper.java b/src/main/java/fr/xephi/authme/util/Wrapper.java index 1505a203e..978d0ab58 100644 --- a/src/main/java/fr/xephi/authme/util/Wrapper.java +++ b/src/main/java/fr/xephi/authme/util/Wrapper.java @@ -13,22 +13,31 @@ import java.util.logging.Logger; */ public class Wrapper { - private AuthMe authMe; + private static Wrapper singleton; - public Wrapper(AuthMe authMe) { - this.authMe = authMe; + /** + * Package-private constructor for testing purposes to inject a mock instance. + */ + Wrapper() { + } + + public static Wrapper getInstance() { + if (singleton == null) { + singleton = new Wrapper(); + } + return singleton; } public AuthMe getAuthMe() { - return authMe; + return AuthMe.getInstance(); } public Server getServer() { - return authMe.getServer(); + return getAuthMe().getServer(); } public Logger getLogger() { - return authMe.getLogger(); + return getAuthMe().getLogger(); } public BukkitScheduler getScheduler() { diff --git a/src/test/java/fr/xephi/authme/AuthMeMockUtil.java b/src/test/java/fr/xephi/authme/AuthMeMockUtil.java index ef22f4f59..1cc0ef7c8 100644 --- a/src/test/java/fr/xephi/authme/AuthMeMockUtil.java +++ b/src/test/java/fr/xephi/authme/AuthMeMockUtil.java @@ -3,6 +3,7 @@ package fr.xephi.authme; import fr.xephi.authme.cache.auth.PlayerCache; import fr.xephi.authme.settings.Messages; import fr.xephi.authme.util.Wrapper; +import fr.xephi.authme.util.WrapperMock; import org.mockito.Mockito; import java.lang.reflect.Field; @@ -43,6 +44,11 @@ public final class AuthMeMockUtil { mockSingletonForClass(PlayerCache.class, "singleton", mock); } + public static void initializeWrapperMock() { + WrapperMock wrapper = new WrapperMock(); + mockSingletonForClass(Wrapper.class, "singleton", wrapper); + } + /** * Set the given class' {@link Wrapper} field to a mock implementation. * @@ -59,7 +65,7 @@ public final class AuthMeMockUtil { } public static Wrapper insertMockWrapperInstance(Class clazz, String fieldName, AuthMe authMe) { - Wrapper wrapperMock = new WrapperMock(authMe); + Wrapper wrapperMock = new WrapperMock(); mockSingletonForClass(clazz, fieldName, wrapperMock); return wrapperMock; } @@ -78,7 +84,7 @@ public final class AuthMeMockUtil { * @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) { + public static void mockSingletonForClass(Class clazz, String fieldName, Object mock) { try { Field instance = clazz.getDeclaredField(fieldName); instance.setAccessible(true); diff --git a/src/test/java/fr/xephi/authme/util/UtilsTest.java b/src/test/java/fr/xephi/authme/util/UtilsTest.java index f228947f8..586b81f3e 100644 --- a/src/test/java/fr/xephi/authme/util/UtilsTest.java +++ b/src/test/java/fr/xephi/authme/util/UtilsTest.java @@ -2,63 +2,44 @@ package fr.xephi.authme.util; import fr.xephi.authme.AuthMe; import fr.xephi.authme.AuthMeMockUtil; -import fr.xephi.authme.WrapperMock; +import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.permission.PermissionsManager; -import org.bukkit.Server; +import fr.xephi.authme.settings.Settings; +import org.bukkit.GameMode; import org.bukkit.entity.Player; -import org.bukkit.plugin.Plugin; -import org.bukkit.scheduler.BukkitScheduler; -import org.bukkit.scheduler.BukkitTask; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; +import org.mockito.Mockito; import java.lang.reflect.Field; import java.util.Collection; +import java.util.logging.Logger; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; import static org.junit.Assert.assertThat; -import static org.mockito.Mockito.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.*; /** * Test for the {@link Utils} class. */ -@Ignore -// TODO ljacqu 20151123: Fix test setup public class UtilsTest { private AuthMe authMeMock; private PermissionsManager permissionsManagerMock; - private Wrapper wrapperMock; @Before public void setUpMocks() { authMeMock = AuthMeMockUtil.mockAuthMeInstance(); - - // We need to create the Wrapper mock before injecting it into Utils because it runs a lot of code in - // a static block which needs the proper mocks to be set up. - wrapperMock = new WrapperMock(authMeMock); - Server serverMock = wrapperMock.getServer(); - - BukkitScheduler schedulerMock = mock(BukkitScheduler.class); - when(serverMock.getScheduler()).thenReturn(schedulerMock); - - - when(schedulerMock.runTaskAsynchronously(any(Plugin.class), any(Runnable.class))) - .thenReturn(mock(BukkitTask.class)); - - System.out.println("Initialized scheduler mock for server mock"); - AuthMeMockUtil.insertMockWrapperInstance(Utils.class, "wrapper", (WrapperMock) wrapperMock); - System.out.println("Iniadfk"); - - permissionsManagerMock = mock(PermissionsManager.class); + AuthMeMockUtil.mockSingletonForClass(Utils.class, "plugin", authMeMock); + permissionsManagerMock = Mockito.mock(PermissionsManager.class); when(authMeMock.getPermissionsManager()).thenReturn(permissionsManagerMock); + + AuthMeMockUtil.initializeWrapperMock(); } - // TODO ljacques 20151122: The tests for Utils.forceGM somehow can't be set up with the mocks correctly - /*@Test + @Test public void shouldForceSurvivalGameMode() { // given Player player = mock(Player.class); @@ -68,6 +49,7 @@ public class UtilsTest { Utils.forceGM(player); // then + verify(authMeMock).getPermissionsManager(); verify(player).setGameMode(GameMode.SURVIVAL); } @@ -84,10 +66,40 @@ public class UtilsTest { verify(authMeMock).getPermissionsManager(); verify(permissionsManagerMock).hasPermission(player, "authme.bypassforcesurvival"); verify(player, never()).setGameMode(any(GameMode.class)); - }*/ + } @Test - // Note ljacqu 20151122: This is a heavy test setup with Reflections... If it causes trouble, skip it with @Ignore + public void shouldNotAddToNormalGroupIfPermissionsAreDisabled() { + // given + Settings.isPermissionCheckEnabled = false; + Player player = mock(Player.class); + + // when + boolean result = Utils.addNormal(player, "test_group"); + + // then + assertThat(result, equalTo(false)); + verify(authMeMock, never()).getPermissionsManager(); + } + + @Test + public void shouldNotAddToNormalGroupIfPermManagerIsNull() { + // given + Settings.isPermissionCheckEnabled = true; + given(authMeMock.getPermissionsManager()).willReturn(null); + Player player = mock(Player.class); + AuthMeMockUtil.mockSingletonForClass(ConsoleLogger.class, "wrapper", Wrapper.getInstance()); + + // when + boolean result = Utils.addNormal(player, "test_group"); + + // then + assertThat(result, equalTo(false)); + verify(authMeMock).getPermissionsManager(); + } + + @Test + // Note ljacqu 20151122: This is a heavy test setup with reflections... If it causes trouble, skip it with @Ignore public void shouldRetrieveListOfOnlinePlayersFromReflectedMethod() { // given setField("getOnlinePlayersIsCollection", false); @@ -114,6 +126,7 @@ public class UtilsTest { } } + // Note: This method is used through reflections public static Player[] onlinePlayersImpl() { return new Player[]{ mock(Player.class), mock(Player.class) diff --git a/src/test/java/fr/xephi/authme/WrapperMock.java b/src/test/java/fr/xephi/authme/util/WrapperMock.java similarity index 86% rename from src/test/java/fr/xephi/authme/WrapperMock.java rename to src/test/java/fr/xephi/authme/util/WrapperMock.java index 7812c6d33..1517d929b 100644 --- a/src/test/java/fr/xephi/authme/WrapperMock.java +++ b/src/test/java/fr/xephi/authme/util/WrapperMock.java @@ -1,6 +1,6 @@ -package fr.xephi.authme; +package fr.xephi.authme.util; -import fr.xephi.authme.util.Wrapper; +import fr.xephi.authme.AuthMe; import org.bukkit.Server; import org.bukkit.scheduler.BukkitScheduler; import org.mockito.Mockito; @@ -19,11 +19,7 @@ public class WrapperMock extends Wrapper { private static Map, Object> mocks = new HashMap<>(); public WrapperMock() { - this((AuthMe) getMock(AuthMe.class)); - } - - public WrapperMock(AuthMe authMe) { - super(authMe); + super(); } @Override From a46278005998a33b62a3ed0a3af66ba3f6596fb3 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Wed, 25 Nov 2015 23:22:43 +0100 Subject: [PATCH 11/11] Create tests for StringUtils#getStackTrace and StringUtils#getDifference - Create tests - Make StringUtils final with private constructor (utility class) --- .../fr/xephi/authme/util/StringUtils.java | 6 +++- .../fr/xephi/authme/util/StringUtilsTest.java | 30 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/main/java/fr/xephi/authme/util/StringUtils.java b/src/main/java/fr/xephi/authme/util/StringUtils.java index 395e4db1b..d712e3adf 100644 --- a/src/main/java/fr/xephi/authme/util/StringUtils.java +++ b/src/main/java/fr/xephi/authme/util/StringUtils.java @@ -10,10 +10,14 @@ import java.io.StringWriter; /** * Utility class for String operations. */ -public class StringUtils { +public final class StringUtils { public static final String newline = System.getProperty("line.separator"); + private StringUtils() { + // Utility class + } + /** * Get the difference of two strings. * diff --git a/src/test/java/fr/xephi/authme/util/StringUtilsTest.java b/src/test/java/fr/xephi/authme/util/StringUtilsTest.java index 47d183979..e2270b361 100644 --- a/src/test/java/fr/xephi/authme/util/StringUtilsTest.java +++ b/src/test/java/fr/xephi/authme/util/StringUtilsTest.java @@ -6,7 +6,9 @@ import java.net.MalformedURLException; import java.util.Arrays; import java.util.List; +import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.stringContainsInOrder; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; @@ -97,4 +99,32 @@ public class StringUtilsTest { // then assertThat(result, equalTo("[MalformedURLException]: Unrecognized URL format")); } + + @Test + public void shouldPrintStackTrace() { + // given + MalformedURLException ex = new MalformedURLException("Unrecognized URL format"); + + // when + String result = StringUtils.getStackTrace(ex); + + // then + assertThat(result, stringContainsInOrder(getClass().getName())); + } + + @Test + public void shouldGetDifferenceWithNullString() { + // given/when/then + assertThat(StringUtils.getDifference(null, "test"), equalTo(1.0)); + assertThat(StringUtils.getDifference("test", null), equalTo(1.0)); + assertThat(StringUtils.getDifference(null, null), equalTo(1.0)); + } + + @Test + public void shouldGetDifferenceBetweenTwoString() { + // given/when/then + assertThat(StringUtils.getDifference("test", "taste"), equalTo(0.4)); + assertThat(StringUtils.getDifference("test", "bear"), equalTo(0.75)); + assertThat(StringUtils.getDifference("test", "something"), greaterThan(0.88)); + } }