From 5d12ec8b56a238b56c7748d6af47927d28b7ced9 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sun, 13 Mar 2016 11:09:27 +0100 Subject: [PATCH] Minor fixes + code householding - Fix SpawnCommandTest testing FirstSpawnCommand - Fix javadoc errors - Map TODO's to issue numbers where applicable - Fix trivial TODO's --- src/main/java/fr/xephi/authme/DataManager.java | 12 +++++++++--- .../fr/xephi/authme/command/CommandService.java | 2 ++ .../authme/ChangePasswordAdminCommand.java | 2 +- .../fr/xephi/authme/converter/RakamakConverter.java | 6 +++--- .../fr/xephi/authme/hooks/BungeeCordMessage.java | 3 ++- .../xephi/authme/listener/AuthMeEntityListener.java | 4 ++-- .../xephi/authme/listener/AuthMePlayerListener.java | 7 +++---- .../fr/xephi/authme/process/ProcessService.java | 12 ++++++++++-- .../process/unregister/AsynchronousUnregister.java | 13 +++++++------ .../fr/xephi/authme/security/pbkdf2/BinTools.java | 5 +++-- .../java/fr/xephi/authme/settings/SpawnLoader.java | 3 +-- .../command/executable/authme/SpawnCommandTest.java | 9 +++------ 12 files changed, 46 insertions(+), 32 deletions(-) diff --git a/src/main/java/fr/xephi/authme/DataManager.java b/src/main/java/fr/xephi/authme/DataManager.java index ca52f0d53..d513889bc 100644 --- a/src/main/java/fr/xephi/authme/DataManager.java +++ b/src/main/java/fr/xephi/authme/DataManager.java @@ -25,7 +25,8 @@ public class DataManager { /** * Constructor for DataManager. * - * @param plugin AuthMe + * @param plugin The plugin instance + * @param pluginHooks Plugin hooks instance */ public DataManager(AuthMe plugin, PluginHooks pluginHooks) { this.plugin = plugin; @@ -161,8 +162,13 @@ public class DataManager { */ public void purgeEssentials(List cleared) { int i = 0; - // FIXME: essentials data folder may be null - final File userDataFolder = new File(pluginHooks.getEssentialsDataFolder(), "userdata"); + File essentialsDataFolder = pluginHooks.getEssentialsDataFolder(); + if (essentialsDataFolder == null) { + ConsoleLogger.info("Cannot purge Essentials: plugin is not loaded"); + return; + } + + final File userDataFolder = new File(essentialsDataFolder, "userdata"); for (String name : cleared) { try { File playerFile = new File(userDataFolder, plugin.getServer().getOfflinePlayer(name).getUniqueId() + ".yml"); diff --git a/src/main/java/fr/xephi/authme/command/CommandService.java b/src/main/java/fr/xephi/authme/command/CommandService.java index a43a08028..9c3c55a53 100644 --- a/src/main/java/fr/xephi/authme/command/CommandService.java +++ b/src/main/java/fr/xephi/authme/command/CommandService.java @@ -46,6 +46,8 @@ public class CommandService { * @param permissionsManager The permissions manager * @param settings The settings manager * @param ipAddressManager The IP address manager + * @param pluginHooks The plugin hooks instance + * @param spawnLoader The spawn loader */ public CommandService(AuthMe authMe, CommandMapper commandMapper, HelpProvider helpProvider, Messages messages, PasswordSecurity passwordSecurity, PermissionsManager permissionsManager, NewSetting settings, diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommand.java index 10bb4432e..8ba9d740f 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommand.java @@ -40,7 +40,7 @@ public class ChangePasswordAdminCommand implements ExecutableCommand { commandService.send(sender, MessageKey.INVALID_PASSWORD_LENGTH); return; } - // TODO ljacqu 20160312: The UNSAFE_PASSWORDS should be all lowercase + // TODO #602 20160312: The UNSAFE_PASSWORDS should be all lowercase // -> introduce a lowercase String list property type if (commandService.getProperty(SecuritySettings.UNSAFE_PASSWORDS).contains(playerPassLowerCase)) { commandService.send(sender, MessageKey.PASSWORD_UNSAFE_ERROR); diff --git a/src/main/java/fr/xephi/authme/converter/RakamakConverter.java b/src/main/java/fr/xephi/authme/converter/RakamakConverter.java index 66e995c2a..289e11beb 100644 --- a/src/main/java/fr/xephi/authme/converter/RakamakConverter.java +++ b/src/main/java/fr/xephi/authme/converter/RakamakConverter.java @@ -22,9 +22,9 @@ import java.util.Map.Entry; */ public class RakamakConverter implements Converter { - public final AuthMe instance; - public final DataSource database; - public final CommandSender sender; + private final AuthMe instance; + private final DataSource database; + private final CommandSender sender; public RakamakConverter(AuthMe instance, CommandSender sender) { this.instance = instance; diff --git a/src/main/java/fr/xephi/authme/hooks/BungeeCordMessage.java b/src/main/java/fr/xephi/authme/hooks/BungeeCordMessage.java index 72f39b807..4b6534daa 100644 --- a/src/main/java/fr/xephi/authme/hooks/BungeeCordMessage.java +++ b/src/main/java/fr/xephi/authme/hooks/BungeeCordMessage.java @@ -22,7 +22,8 @@ public class BungeeCordMessage implements PluginMessageListener { /** * Constructor for BungeeCordMessage. * - * @param plugin AuthMe + * @param plugin The plugin instance + * @param ipAddressManager The IP address manager */ public BungeeCordMessage(AuthMe plugin, IpAddressManager ipAddressManager) { this.plugin = plugin; diff --git a/src/main/java/fr/xephi/authme/listener/AuthMeEntityListener.java b/src/main/java/fr/xephi/authme/listener/AuthMeEntityListener.java index c5c0b2a68..50b0e61ca 100644 --- a/src/main/java/fr/xephi/authme/listener/AuthMeEntityListener.java +++ b/src/main/java/fr/xephi/authme/listener/AuthMeEntityListener.java @@ -87,7 +87,7 @@ public class AuthMeEntityListener implements Listener { } } - // TODO: Need to check this, player can't throw snowball but the item is taken. + // TODO #568: Need to check this, player can't throw snowball but the item is taken. @EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST) public void onProjectileLaunch(ProjectileLaunchEvent event) { if (event.getEntity() == null) { @@ -103,7 +103,7 @@ public class AuthMeEntityListener implements Listener { } player = (Player) shooter; } else { - // TODO ljacqu 20151220: Invoking getShooter() with null but method isn't static + // TODO #568 20151220: Invoking getShooter() with null but method isn't static try { if (getShooter == null) { getShooter = Projectile.class.getMethod("getShooter"); diff --git a/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java b/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java index d563f33c0..7643de73c 100644 --- a/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java +++ b/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java @@ -80,11 +80,10 @@ public class AuthMePlayerListener implements Listener { } event.setCancelled(true); - sendLoginRegisterMSG(player); + sendLoginOrRegisterMessage(player); } - // TODO: new name - private void sendLoginRegisterMSG(final Player player) { + private void sendLoginOrRegisterMessage(final Player player) { plugin.getServer().getScheduler().runTaskAsynchronously(plugin, new Runnable() { @Override public void run() { @@ -117,7 +116,7 @@ public class AuthMePlayerListener implements Listener { return; } event.setCancelled(true); - sendLoginRegisterMSG(event.getPlayer()); + sendLoginOrRegisterMessage(event.getPlayer()); } @EventHandler(ignoreCancelled = true, priority = EventPriority.NORMAL) diff --git a/src/main/java/fr/xephi/authme/process/ProcessService.java b/src/main/java/fr/xephi/authme/process/ProcessService.java index 2ffbc8770..81c241d5d 100644 --- a/src/main/java/fr/xephi/authme/process/ProcessService.java +++ b/src/main/java/fr/xephi/authme/process/ProcessService.java @@ -2,6 +2,7 @@ package fr.xephi.authme.process; import fr.xephi.authme.AuthMe; import fr.xephi.authme.cache.IpAddressManager; +import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.hooks.PluginHooks; import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.output.Messages; @@ -22,16 +23,19 @@ public class ProcessService { private final NewSetting settings; private final Messages messages; private final AuthMe authMe; + private final DataSource dataSource; private final IpAddressManager ipAddressManager; private final PasswordSecurity passwordSecurity; private final PluginHooks pluginHooks; private final SpawnLoader spawnLoader; - public ProcessService(NewSetting settings, Messages messages, AuthMe authMe, IpAddressManager ipAddressManager, - PasswordSecurity passwordSecurity, PluginHooks pluginHooks, SpawnLoader spawnLoader) { + public ProcessService(NewSetting settings, Messages messages, AuthMe authMe, DataSource dataSource, + IpAddressManager ipAddressManager, PasswordSecurity passwordSecurity, PluginHooks pluginHooks, + SpawnLoader spawnLoader) { this.settings = settings; this.messages = messages; this.authMe = authMe; + this.dataSource = dataSource; this.ipAddressManager = ipAddressManager; this.passwordSecurity = passwordSecurity; this.pluginHooks = pluginHooks; @@ -98,4 +102,8 @@ public class ProcessService { return spawnLoader; } + public DataSource getDataSource() { + return dataSource; + } + } diff --git a/src/main/java/fr/xephi/authme/process/unregister/AsynchronousUnregister.java b/src/main/java/fr/xephi/authme/process/unregister/AsynchronousUnregister.java index 2e032cea3..29c2b8c3a 100644 --- a/src/main/java/fr/xephi/authme/process/unregister/AsynchronousUnregister.java +++ b/src/main/java/fr/xephi/authme/process/unregister/AsynchronousUnregister.java @@ -34,12 +34,13 @@ public class AsynchronousUnregister implements Process { private final ProcessService service; /** - * Constructor for AsynchronousUnregister. + * Constructor. * - * @param player Player - * @param password String - * @param force boolean - * @param plugin AuthMe + * @param player The player to perform the action for + * @param password The password + * @param force True to bypass password validation + * @param plugin The plugin instance + * @param service The process service */ public AsynchronousUnregister(Player player, String password, boolean force, AuthMe plugin, ProcessService service) { @@ -57,7 +58,7 @@ public class AsynchronousUnregister implements Process { PlayerAuth cachedAuth = PlayerCache.getInstance().getAuth(name); if (force || plugin.getPasswordSecurity().comparePassword( password, cachedAuth.getPassword(), player.getName())) { - if (!plugin.getDataSource().removeAuth(name)) { + if (!service.getDataSource().removeAuth(name)) { service.send(player, MessageKey.ERROR); return; } diff --git a/src/main/java/fr/xephi/authme/security/pbkdf2/BinTools.java b/src/main/java/fr/xephi/authme/security/pbkdf2/BinTools.java index 003b50bdd..daadcae8c 100644 --- a/src/main/java/fr/xephi/authme/security/pbkdf2/BinTools.java +++ b/src/main/java/fr/xephi/authme/security/pbkdf2/BinTools.java @@ -106,8 +106,9 @@ public class BinTools { throw new IllegalArgumentException("Input string may only contain hex digits, but found '" + c + "'"); } - // TODO ljacqu 20151219: Move to a BinToolsTest class - private static void testUtils(String[] args) { + // Note ljacqu 20160313: This appears to be a test method that was present in the third-party source. + // We can keep it for troubleshooting in the future. + private static void testUtils() { byte b[] = new byte[256]; byte bb = 0; for (int i = 0; i < 256; i++) { diff --git a/src/main/java/fr/xephi/authme/settings/SpawnLoader.java b/src/main/java/fr/xephi/authme/settings/SpawnLoader.java index 5deb5c16d..59693a98c 100644 --- a/src/main/java/fr/xephi/authme/settings/SpawnLoader.java +++ b/src/main/java/fr/xephi/authme/settings/SpawnLoader.java @@ -141,9 +141,8 @@ public class SpawnLoader { World world = player.getWorld(); Location spawnLoc = null; - // TODO ljacqu 20160312: We should trim() the entries for (String priority : spawnPriority) { - switch (priority.toLowerCase()) { + switch (priority.toLowerCase().trim()) { case "default": if (world.getSpawnLocation() != null) { spawnLoc = world.getSpawnLocation(); diff --git a/src/test/java/fr/xephi/authme/command/executable/authme/SpawnCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/authme/SpawnCommandTest.java index 305c41ba2..eebfd5af4 100644 --- a/src/test/java/fr/xephi/authme/command/executable/authme/SpawnCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/authme/SpawnCommandTest.java @@ -6,14 +6,13 @@ import fr.xephi.authme.settings.SpawnLoader; import org.bukkit.Location; import org.bukkit.entity.Player; import org.junit.Test; -import org.mockito.ArgumentCaptor; import java.util.Collections; import static org.hamcrest.Matchers.containsString; -import static org.junit.Assert.assertThat; import static org.mockito.BDDMockito.given; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.argThat; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -51,15 +50,13 @@ public class SpawnCommandTest { CommandService service = mock(CommandService.class); given(service.getSpawnLoader()).willReturn(spawnLoader); Player player = mock(Player.class); - ExecutableCommand command = new FirstSpawnCommand(); + ExecutableCommand command = new SpawnCommand(); // when command.executeCommand(player, Collections.EMPTY_LIST, service); // then - ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); - verify(player).sendMessage(captor.capture()); - assertThat(captor.getValue(), containsString("spawn has failed")); + verify(player).sendMessage(argThat(containsString("Spawn has failed"))); verify(player, never()).teleport(any(Location.class)); } }