From 22c08c9fc3c1ad3835eb83666b6158fd470b8374 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Tue, 11 Sep 2018 22:03:46 +0200 Subject: [PATCH] #1449 Fix copying of Commands, add tests with command delay --- .../settings/commandconfig/Command.java | 27 +++++++---------- .../commandconfig/CommandManager.java | 9 ++---- .../CommandMigrationService.java | 5 ++-- .../commandconfig/OnLoginCommand.java | 30 +++++++------------ .../commandconfig/CommandManagerTest.java | 18 +++++++++++ .../commandconfig/commands.complete.yml | 6 ++++ .../commandconfig/commands.incomplete.yml | 2 ++ 7 files changed, 53 insertions(+), 44 deletions(-) diff --git a/src/main/java/fr/xephi/authme/settings/commandconfig/Command.java b/src/main/java/fr/xephi/authme/settings/commandconfig/Command.java index 3a0eb012..5ac922b1 100644 --- a/src/main/java/fr/xephi/authme/settings/commandconfig/Command.java +++ b/src/main/java/fr/xephi/authme/settings/commandconfig/Command.java @@ -19,26 +19,21 @@ public class Command { } /** - * Constructor. + * Creates a copy of this Command object, setting the given command text on the copy. * - * @param command the command - * @param executor the executor of the command + * @param command the command text to use in the copy + * @return copy of the source with the new command */ - public Command(String command, Executor executor) { - this(command, executor, 0); + public Command copyWithCommand(String command) { + Command copy = new Command(); + setValuesToCopyWithNewCommand(copy, command); + return copy; } - /** - * Constructor. - * - * @param command the command - * @param executor the executor of the command - * @param delay the delay (in ticks) before executing command - */ - public Command(String command, Executor executor, long delay) { - this.command = command; - this.executor = executor; - this.delay = delay; + protected void setValuesToCopyWithNewCommand(Command copy, String newCommand) { + copy.command = newCommand; + copy.executor = this.executor; + copy.delay = this.delay; } public String getCommand() { diff --git a/src/main/java/fr/xephi/authme/settings/commandconfig/CommandManager.java b/src/main/java/fr/xephi/authme/settings/commandconfig/CommandManager.java index 1fad004b..1018da0a 100644 --- a/src/main/java/fr/xephi/authme/settings/commandconfig/CommandManager.java +++ b/src/main/java/fr/xephi/authme/settings/commandconfig/CommandManager.java @@ -174,15 +174,12 @@ public class CommandManager implements Reloadable { private WrappedTagReplacer newReplacer(Map commands) { return new WrappedTagReplacer<>(availableTags, commands.values(), Command::getCommand, - (cmd, text) -> new Command(text, cmd.getExecutor())); + Command::copyWithCommand); } - private WrappedTagReplacer newOnLoginCmdReplacer( - Map commands) { - + private WrappedTagReplacer newOnLoginCmdReplacer(Map commands) { return new WrappedTagReplacer<>(availableTags, commands.values(), Command::getCommand, - (cmd, text) -> new OnLoginCommand(text, cmd.getExecutor(), cmd.getIfNumberOfAccountsAtLeast(), - cmd.getIfNumberOfAccountsLessThan())); + OnLoginCommand::copyWithCommand); } private List> buildAvailableTags() { diff --git a/src/main/java/fr/xephi/authme/settings/commandconfig/CommandMigrationService.java b/src/main/java/fr/xephi/authme/settings/commandconfig/CommandMigrationService.java index b88e0c88..62925ff3 100644 --- a/src/main/java/fr/xephi/authme/settings/commandconfig/CommandMigrationService.java +++ b/src/main/java/fr/xephi/authme/settings/commandconfig/CommandMigrationService.java @@ -41,8 +41,9 @@ class CommandMigrationService implements MigrationService { private boolean moveOtherAccountsConfig(CommandConfig commandConfig) { if (settingsMigrationService.hasOldOtherAccountsCommand()) { - OnLoginCommand command = new OnLoginCommand( - replaceOldPlaceholdersWithNew(settingsMigrationService.getOldOtherAccountsCommand()), Executor.CONSOLE); + OnLoginCommand command = new OnLoginCommand(); + command.setCommand(replaceOldPlaceholdersWithNew(settingsMigrationService.getOldOtherAccountsCommand())); + command.setExecutor(Executor.CONSOLE); command.setIfNumberOfAccountsAtLeast( Optional.of(settingsMigrationService.getOldOtherAccountsCommandThreshold())); diff --git a/src/main/java/fr/xephi/authme/settings/commandconfig/OnLoginCommand.java b/src/main/java/fr/xephi/authme/settings/commandconfig/OnLoginCommand.java index 638e0838..13cc30fc 100644 --- a/src/main/java/fr/xephi/authme/settings/commandconfig/OnLoginCommand.java +++ b/src/main/java/fr/xephi/authme/settings/commandconfig/OnLoginCommand.java @@ -17,28 +17,18 @@ public class OnLoginCommand extends Command { } /** - * Constructor. + * Creates a copy of this object, using the given command as new {@link Command#command command}. * - * @param command the command to execute - * @param executor the executor of the command + * @param command the command text to use in the copy + * @return copy of the source with the new command */ - public OnLoginCommand(String command, Executor executor) { - super(command, executor); - } - - /** - * Constructor. - * - * @param command the command to execute - * @param executor the executor of the command - * @param ifNumberOfAccountsAtLeast required number of accounts for the command to run - * @param ifNumberOfAccountsLessThan max threshold of accounts, from which the command will not be run - */ - public OnLoginCommand(String command, Executor executor, Optional ifNumberOfAccountsAtLeast, - Optional ifNumberOfAccountsLessThan) { - super(command, executor); - this.ifNumberOfAccountsAtLeast = ifNumberOfAccountsAtLeast; - this.ifNumberOfAccountsLessThan = ifNumberOfAccountsLessThan; + @Override + public OnLoginCommand copyWithCommand(String command) { + OnLoginCommand copy = new OnLoginCommand(); + setValuesToCopyWithNewCommand(copy, command); + copy.ifNumberOfAccountsAtLeast = this.ifNumberOfAccountsAtLeast; + copy.ifNumberOfAccountsLessThan = this.ifNumberOfAccountsLessThan; + return copy; } public Optional getIfNumberOfAccountsAtLeast() { diff --git a/src/test/java/fr/xephi/authme/settings/commandconfig/CommandManagerTest.java b/src/test/java/fr/xephi/authme/settings/commandconfig/CommandManagerTest.java index a482eedb..828ec15d 100644 --- a/src/test/java/fr/xephi/authme/settings/commandconfig/CommandManagerTest.java +++ b/src/test/java/fr/xephi/authme/settings/commandconfig/CommandManagerTest.java @@ -3,6 +3,7 @@ package fr.xephi.authme.settings.commandconfig; import com.google.common.io.Files; import fr.xephi.authme.TestHelper; import fr.xephi.authme.service.BukkitService; +import fr.xephi.authme.service.BukkitServiceTestHelper; import fr.xephi.authme.service.GeoIpService; import fr.xephi.authme.settings.SettingsMigrationService; import org.bukkit.entity.Player; @@ -25,6 +26,7 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.only; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.verifyZeroInteractions; @@ -59,6 +61,7 @@ public class CommandManagerTest { public void setup() throws IOException { testFolder = temporaryFolder.newFolder(); player = mockPlayer(); + BukkitServiceTestHelper.setBukkitServiceToScheduleSyncDelayedTaskWithDelay(bukkitService); } @Test @@ -74,6 +77,8 @@ public class CommandManagerTest { verify(bukkitService).dispatchConsoleCommand("msg Bobby Welcome back"); verify(bukkitService).dispatchCommand(any(Player.class), eq("motd")); verify(bukkitService).dispatchCommand(any(Player.class), eq("list")); + verify(bukkitService).scheduleSyncDelayedTask(any(Runnable.class), eq(60L)); + verify(bukkitService).scheduleSyncDelayedTask(any(Runnable.class), eq(120L)); verifyNoMoreInteractions(bukkitService); verifyZeroInteractions(geoIpService); } @@ -92,6 +97,9 @@ public class CommandManagerTest { verify(bukkitService).dispatchCommand(player, "motd"); verify(bukkitService).dispatchCommand(player, "list"); verify(bukkitService).dispatchConsoleCommand("helpop Player Bobby has more than 1 account"); + verify(bukkitService).scheduleSyncDelayedTask(any(Runnable.class), eq(60L)); + verify(bukkitService).scheduleSyncDelayedTask(any(Runnable.class), eq(120L)); + verify(bukkitService).scheduleSyncDelayedTask(any(Runnable.class), eq(180L)); verifyNoMoreInteractions(bukkitService); verifyZeroInteractions(geoIpService); } @@ -111,6 +119,10 @@ public class CommandManagerTest { verify(bukkitService).dispatchCommand(player, "list"); verify(bukkitService).dispatchConsoleCommand("helpop Player Bobby has more than 1 account"); verify(bukkitService).dispatchConsoleCommand("log Bobby 127.0.0.3 many accounts"); + verify(bukkitService).scheduleSyncDelayedTask(any(Runnable.class), eq(60L)); + verify(bukkitService).scheduleSyncDelayedTask(any(Runnable.class), eq(120L)); + verify(bukkitService).scheduleSyncDelayedTask(any(Runnable.class), eq(180L)); + verify(bukkitService).scheduleSyncDelayedTask(any(Runnable.class), eq(240L)); verifyNoMoreInteractions(bukkitService); verifyZeroInteractions(geoIpService); } @@ -129,6 +141,9 @@ public class CommandManagerTest { verify(bukkitService).dispatchCommand(player, "motd"); verify(bukkitService).dispatchCommand(player, "list"); verify(bukkitService).dispatchConsoleCommand("helpop Player Bobby has more than 1 account"); + verify(bukkitService).scheduleSyncDelayedTask(any(Runnable.class), eq(60L)); + verify(bukkitService).scheduleSyncDelayedTask(any(Runnable.class), eq(120L)); + verify(bukkitService).scheduleSyncDelayedTask(any(Runnable.class), eq(180L)); verifyNoMoreInteractions(bukkitService); verifyZeroInteractions(geoIpService); } @@ -138,6 +153,7 @@ public class CommandManagerTest { // given copyJarFileAsCommandsYml(TEST_FILES_FOLDER + "commands.incomplete.yml"); initManager(); + BukkitServiceTestHelper.setBukkitServiceToScheduleSyncDelayedTaskWithDelay(bukkitService); // when manager.runCommandsOnLogin(player, Collections.emptyList()); @@ -145,6 +161,7 @@ public class CommandManagerTest { // then verify(bukkitService).dispatchConsoleCommand("msg Bobby Welcome back, bob"); verify(bukkitService).dispatchCommand(any(Player.class), eq("list")); + verify(bukkitService).scheduleSyncDelayedTask(any(Runnable.class), eq(100L)); verifyNoMoreInteractions(bukkitService); verifyZeroInteractions(geoIpService); } @@ -232,6 +249,7 @@ public class CommandManagerTest { // then verify(bukkitService).dispatchCommand(any(Player.class), eq("me I just registered")); verify(bukkitService).dispatchConsoleCommand("log Bobby (127.0.0.3, Syldavia) registered"); + verify(bukkitService, times(2)).scheduleSyncDelayedTask(any(Runnable.class), eq(100L)); verifyNoMoreInteractions(bukkitService); } diff --git a/src/test/resources/fr/xephi/authme/settings/commandconfig/commands.complete.yml b/src/test/resources/fr/xephi/authme/settings/commandconfig/commands.complete.yml index efd11db5..a2919ee7 100644 --- a/src/test/resources/fr/xephi/authme/settings/commandconfig/commands.complete.yml +++ b/src/test/resources/fr/xephi/authme/settings/commandconfig/commands.complete.yml @@ -8,9 +8,11 @@ onRegister: announce: command: 'me I just registered' executor: PLAYER + delay: 100 notify: command: 'log %p (%ip, %country) registered' executor: CONSOLE + delay: 100 onLogin: welcome: command: 'msg %p Welcome back' @@ -18,18 +20,22 @@ onLogin: show_motd: command: 'motd' executor: PLAYER + delay: 60 display_list: command: 'list' executor: PLAYER + delay: 120 warn_for_alts: command: 'helpop Player %p has more than 1 account' executor: CONSOLE ifNumberOfAccountsAtLeast: 2 + delay: 180 log_suspicious_user: command: 'log %p %ip many accounts' executor: CONSOLE ifNumberOfAccountsAtLeast: 5 ifNumberOfAccountsLessThan: 20 + delay: 240 onSessionLogin: welcome: command: 'msg %p Session login!' diff --git a/src/test/resources/fr/xephi/authme/settings/commandconfig/commands.incomplete.yml b/src/test/resources/fr/xephi/authme/settings/commandconfig/commands.incomplete.yml index 8bde4aab..2c2edaad 100644 --- a/src/test/resources/fr/xephi/authme/settings/commandconfig/commands.incomplete.yml +++ b/src/test/resources/fr/xephi/authme/settings/commandconfig/commands.incomplete.yml @@ -8,12 +8,14 @@ onLogin: welcome: command: 'msg %p Welcome back, %nick' executor: CONSOLE + delay: 0 show_motd: # command: 'motd' <-- mandatory property, so entry should be ignored executor: PLAYER display_list: command: 'list' executor: WRONG_EXECUTOR + delay: 100 doesNotExist: wrongEntry: command: 'should be ignored'