#844 Run PurgeTask as repeating task and #784 write tests for PurgeTask

src/main/java/fr/xephi/authme/task/purge/PurgeExecutor.java

@@ -49,7 +49,7 @@ class PurgeExecutor {
      * players and names.
      *
      * @param players the players to purge
-     * @param names names to purge (lowercase)
+     * @param names names to purge
      */
     public void executePurge(Collection<OfflinePlayer> players, Collection<String> names) {
         // Purge other data

src/main/java/fr/xephi/authme/task/purge/PurgeService.java

@@ -91,7 +91,7 @@ public class PurgeService {
 
         isPurging = true;
         PurgeTask purgeTask = new PurgeTask(this, permissionsManager, sender, names, players);
-        bukkitService.runTaskAsynchronously(purgeTask);
+        bukkitService.runTaskTimer(purgeTask, 0, 1);
     }
 
     /**

src/main/java/fr/xephi/authme/task/purge/PurgeTask.java

@@ -104,9 +104,9 @@ class PurgeTask extends BukkitRunnable {
         cancel();
 
         // Show a status message
-        sendMessage(ChatColor.GREEN + "[AuthMe] Database has been purged correctly");
+        sendMessage(ChatColor.GREEN + "[AuthMe] Database has been purged successfully");
 
-        ConsoleLogger.info("Purge Finished!");
+        ConsoleLogger.info("Purge finished!");
         purgeService.setPurging(false);
     }
 

src/main/java/fr/xephi/authme/util/BukkitService.java

@@ -9,6 +9,9 @@ import org.bukkit.OfflinePlayer;
 import org.bukkit.World;
 import org.bukkit.entity.Player;
 import org.bukkit.event.Event;
+import org.bukkit.plugin.Plugin;
+import org.bukkit.scheduler.BukkitRunnable;
+import org.bukkit.scheduler.BukkitScheduler;
 import org.bukkit.scheduler.BukkitTask;
 
 import javax.inject.Inject;
@@ -123,6 +126,22 @@ public class BukkitService {
         return Bukkit.getScheduler().runTaskLaterAsynchronously(authMe, task, delay);
     }
 
+    /**
+     * Schedules the given task to repeatedly run until cancelled, starting after the
+     * specified number of server ticks.
+     *
+     * @param task the task to schedule
+     * @param delay the ticks to wait before running the task
+     * @param period the ticks to wait between runs
+     * @return a BukkitTask that contains the id number
+     * @throws IllegalArgumentException if plugin is null
+     * @throws IllegalStateException if this was already scheduled
+     * @see BukkitScheduler#runTaskTimer(Plugin, Runnable, long, long)
+     */
+    public BukkitTask runTaskTimer(BukkitRunnable task, long delay, long period) {
+        return task.runTaskTimer(authMe, period, delay);
+    }
+
     /**
      * Broadcast a message to all players.
      *

src/test/java/fr/xephi/authme/task/purge/PurgeServiceTest.java

@@ -4,9 +4,6 @@ import fr.xephi.authme.ReflectionTestUtils;
 import fr.xephi.authme.TestHelper;
 import fr.xephi.authme.datasource.DataSource;
 import fr.xephi.authme.permission.PermissionsManager;
-import fr.xephi.authme.runner.BeforeInjecting;
-import fr.xephi.authme.runner.DelayedInjectionRunner;
-import fr.xephi.authme.runner.InjectDelayed;
 import fr.xephi.authme.settings.NewSetting;
 import fr.xephi.authme.settings.properties.PurgeSettings;
 import fr.xephi.authme.util.BukkitService;
@@ -18,10 +15,14 @@ import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
+import org.mockito.InjectMocks;
 import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
 
+import java.util.Arrays;
 import java.util.Calendar;
 import java.util.Collections;
+import java.util.List;
 import java.util.Set;
 import java.util.UUID;
 
@@ -36,6 +37,7 @@ import static org.mockito.BDDMockito.given;
 import static org.mockito.Matchers.anyLong;
 import static org.mockito.Matchers.anySet;
 import static org.mockito.Matchers.argThat;
+import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
@@ -44,10 +46,10 @@ import static org.mockito.Mockito.verifyZeroInteractions;
 /**
  * Test for {@link PurgeService}.
  */
-@RunWith(DelayedInjectionRunner.class)
+@RunWith(MockitoJUnitRunner.class)
 public class PurgeServiceTest {
 
-    @InjectDelayed
+    @InjectMocks
     private PurgeService purgeService;
 
     @Mock
@@ -66,15 +68,11 @@ public class PurgeServiceTest {
         TestHelper.setupLogger();
     }
 
-    @BeforeInjecting
-    public void initSettingDefaults() {
-        given(settings.getProperty(PurgeSettings.DAYS_BEFORE_REMOVE_PLAYER)).willReturn(60);
-    }
-
     @Test
     public void shouldNotRunAutoPurge() {
         // given
         given(settings.getProperty(PurgeSettings.USE_AUTO_PURGE)).willReturn(false);
+        given(settings.getProperty(PurgeSettings.DAYS_BEFORE_REMOVE_PLAYER)).willReturn(60);
 
         // when
         purgeService.runAutoPurge();
@@ -112,8 +110,8 @@ public class PurgeServiceTest {
         ArgumentCaptor<Long> captor = ArgumentCaptor.forClass(Long.class);
         verify(dataSource).getRecordsToPurge(captor.capture());
         assertCorrectPurgeTimestamp(captor.getValue(), 60);
-        assertThat(Boolean.TRUE.equals(
-            ReflectionTestUtils.getFieldValue(PurgeService.class, purgeService, "isPurging")), equalTo(true));
+        assertThat(Boolean.TRUE, equalTo(
+            ReflectionTestUtils.getFieldValue(PurgeService.class, purgeService, "isPurging")));
         verifyScheduledPurgeTask(null, playerNames);
     }
 
@@ -169,6 +167,19 @@ public class PurgeServiceTest {
         verifyZeroInteractions(bukkitService, dataSource, permissionsManager);
     }
 
+    @Test
+    public void shouldExecutePurgeActions() {
+        // given
+        List<OfflinePlayer> players = Arrays.asList(mockReturnedOfflinePlayers());
+        List<String> names = Arrays.asList("alpha", "bravo", "foxtrot");
+
+        // when
+        purgeService.executePurge(players, names);
+
+        // then
+        verify(executor).executePurge(players, names);
+    }
+
     /**
      * Returns mock OfflinePlayer objects with names corresponding to A - G of the NATO phonetic alphabet,
      * in various casing.
@@ -198,14 +209,14 @@ public class PurgeServiceTest {
     }
 
     @SuppressWarnings("unchecked")
-    private void verifyScheduledPurgeTask(UUID uuid, Set<String> names) {
+    private void verifyScheduledPurgeTask(UUID senderUuid, Set<String> names) {
         ArgumentCaptor<PurgeTask> captor = ArgumentCaptor.forClass(PurgeTask.class);
-        verify(bukkitService).runTaskAsynchronously(captor.capture());
+        verify(bukkitService).runTaskTimer(captor.capture(), eq(0L), eq(1L));
         PurgeTask task = captor.getValue();
 
         Object senderInTask = ReflectionTestUtils.getFieldValue(PurgeTask.class, task, "sender");
         Set<String> namesInTask = (Set<String>) ReflectionTestUtils.getFieldValue(PurgeTask.class, task, "toPurge");
-        assertThat(senderInTask, Matchers.<Object>equalTo(uuid));
+        assertThat(senderInTask, Matchers.<Object>equalTo(senderUuid));
         assertThat(namesInTask, containsInAnyOrder(names.toArray()));
     }
 }

src/test/java/fr/xephi/authme/task/purge/PurgeTaskTest.java

@@ -0,0 +1,221 @@
+package fr.xephi.authme.task.purge;
+
+import fr.xephi.authme.ReflectionTestUtils;
+import fr.xephi.authme.TestHelper;
+import fr.xephi.authme.permission.PermissionNode;
+import fr.xephi.authme.permission.PermissionsManager;
+import fr.xephi.authme.permission.PlayerStatePermission;
+import org.bukkit.Bukkit;
+import org.bukkit.OfflinePlayer;
+import org.bukkit.Server;
+import org.bukkit.command.ConsoleCommandSender;
+import org.bukkit.entity.Player;
+import org.bukkit.scheduler.BukkitRunnable;
+import org.bukkit.scheduler.BukkitScheduler;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Captor;
+import org.mockito.Mock;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.runners.MockitoJUnitRunner;
+import org.mockito.stubbing.Answer;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.UUID;
+
+import static com.google.common.collect.Sets.newHashSet;
+import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.empty;
+import static org.junit.Assert.assertThat;
+import static org.mockito.BDDMockito.given;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Matchers.argThat;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.reset;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+
+/**
+ * Test for {@link PurgeTask}.
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class PurgeTaskTest {
+
+    private static final PermissionNode BYPASS_NODE = PlayerStatePermission.BYPASS_PURGE;
+
+    private Map<OfflinePlayer, Boolean> playerBypassAssignments = new HashMap<>();
+
+    @Mock
+    private PermissionsManager permissionsManager;
+
+    @Mock
+    private PurgeService purgeService;
+
+    @Captor
+    private ArgumentCaptor<Collection<OfflinePlayer>> playerCaptor;
+
+    @Captor
+    private ArgumentCaptor<Collection<String>> namesCaptor;
+
+    @BeforeClass
+    public static void initLogger() {
+        TestHelper.setupLogger();
+    }
+
+    @Test
+    public void shouldRunTask() {
+        // given
+        Set<String> names =
+            newHashSet("alpha", "bravo", "charlie", "delta", "echo", "foxtrot", "golf", "hotel", "india");
+        // alpha and echo have bypass permission
+        // Foxtrot and india are not present as OfflinePlayer
+        // Additionally, BOGUS and 123456 are not present in the names list
+        OfflinePlayer[] players = asArray(
+            mockOfflinePlayer("Alpha", true),  mockOfflinePlayer("BOGUS", false),  mockOfflinePlayer("charlie", false),
+            mockOfflinePlayer("Delta", false), mockOfflinePlayer("BRAVO", false),  mockOfflinePlayer("Echo", true),
+            mockOfflinePlayer("Golf", false),  mockOfflinePlayer("123456", false), mockOfflinePlayer("HOTEL", false));
+        reset(purgeService, permissionsManager);
+        setPermissionsBehavior();
+        PurgeTask task = new PurgeTask(purgeService, permissionsManager, null, names, players);
+
+        // when (1 - first run, 5 players per run)
+        task.run();
+
+        // then (1)
+        // In the first run, Alpha to BRAVO (see players list above) went through. One of those players is not present
+        // in the names list, so expect the permission manager to have been called four times
+        verify(permissionsManager, times(4)).hasPermissionOffline(any(OfflinePlayer.class), eq(BYPASS_NODE));
+        // Alpha has the bypass permission, so we expect charlie, Delta and BRAVO to be purged
+        assertRanPurgeWithPlayers(players[2], players[3], players[4]);
+
+        // when (2)
+        reset(purgeService, permissionsManager);
+        setPermissionsBehavior();
+        task.run();
+
+        // then (2)
+        // Echo, Golf, HOTEL
+        verify(permissionsManager, times(3)).hasPermissionOffline(any(OfflinePlayer.class), eq(BYPASS_NODE));
+        assertRanPurgeWithPlayers(players[6], players[8]);
+
+        // given (3)
+        // Third round: no more OfflinePlayer objects, but some names remain
+        reset(purgeService, permissionsManager);
+        given(permissionsManager.hasPermissionOffline("india", BYPASS_NODE)).willReturn(true);
+
+        // when (3)
+        task.run();
+
+        // then (3)
+        // We no longer have any OfflinePlayers, so lookup of permissions was done with the names
+        verify(permissionsManager, times(2)).hasPermissionOffline(anyString(), eq(BYPASS_NODE));
+        verify(permissionsManager, never()).hasPermissionOffline(any(OfflinePlayer.class), any(PermissionNode.class));
+        assertRanPurgeWithNames("foxtrot");
+    }
+
+    @Test
+    public void shouldStopTaskAndInformSenderUponCompletion() {
+        // given
+        Set<String> names = newHashSet("name1", "name2");
+        Player sender = mock(Player.class);
+        UUID uuid = UUID.randomUUID();
+        given(sender.getUniqueId()).willReturn(uuid);
+        PurgeTask task = new PurgeTask(purgeService, permissionsManager, sender, names, new OfflinePlayer[0]);
+
+        ReflectionTestUtils.setField(BukkitRunnable.class, task, "taskId", 10049);
+        Server server = mock(Server.class);
+        BukkitScheduler scheduler = mock(BukkitScheduler.class);
+        given(server.getScheduler()).willReturn(scheduler);
+        ReflectionTestUtils.setField(Bukkit.class, null, "server", server);
+        given(server.getPlayer(uuid)).willReturn(sender);
+
+        task.run(); // Run for the first time -> results in empty names list
+
+        // when
+        task.run();
+
+        // then
+        verify(scheduler).cancelTask(task.getTaskId());
+        verify(sender).sendMessage(argThat(containsString("Database has been purged successfully")));
+    }
+
+    @Test
+    public void shouldStopTaskAndInformConsoleUser() {
+        // given
+        Set<String> names = newHashSet("name1", "name2");
+        PurgeTask task = new PurgeTask(purgeService, permissionsManager, null, names, new OfflinePlayer[0]);
+
+        ReflectionTestUtils.setField(BukkitRunnable.class, task, "taskId", 10049);
+        Server server = mock(Server.class);
+        BukkitScheduler scheduler = mock(BukkitScheduler.class);
+        given(server.getScheduler()).willReturn(scheduler);
+        ReflectionTestUtils.setField(Bukkit.class, null, "server", server);
+        ConsoleCommandSender consoleSender = mock(ConsoleCommandSender.class);
+        given(server.getConsoleSender()).willReturn(consoleSender);
+
+        task.run(); // Run for the first time -> results in empty names list
+
+        // when
+        task.run();
+
+        // then
+        verify(scheduler).cancelTask(task.getTaskId());
+        verify(consoleSender).sendMessage(argThat(containsString("Database has been purged successfully")));
+    }
+
+
+    private OfflinePlayer mockOfflinePlayer(String name, boolean hasBypassPermission) {
+        OfflinePlayer player = mock(OfflinePlayer.class);
+        given(player.getName()).willReturn(name);
+        playerBypassAssignments.put(player, hasBypassPermission);
+        return player;
+    }
+
+    private OfflinePlayer[] asArray(OfflinePlayer... players) {
+        return players;
+    }
+
+    private void setPermissionsBehavior() {
+        given(permissionsManager.hasPermissionOffline(any(OfflinePlayer.class), eq(BYPASS_NODE)))
+            .willAnswer(new Answer<Boolean>() {
+                @Override
+                public Boolean answer(InvocationOnMock invocationOnMock) throws Throwable {
+                    OfflinePlayer player = (OfflinePlayer) invocationOnMock.getArguments()[0];
+                    Boolean hasPermission = playerBypassAssignments.get(player);
+                    if (hasPermission == null) {
+                        throw new IllegalStateException("Unexpected check of '" + BYPASS_NODE
+                            + "' with player = " + player);
+                    }
+                    return hasPermission;
+                }
+            });
+    }
+
+    private void assertRanPurgeWithPlayers(OfflinePlayer... players) {
+        List<String> names = new ArrayList<>(players.length);
+        for (OfflinePlayer player : players) {
+            names.add(player.getName());
+        }
+        verify(purgeService).executePurge(playerCaptor.capture(), namesCaptor.capture());
+        assertThat(namesCaptor.getValue(), containsInAnyOrder(names.toArray()));
+        assertThat(playerCaptor.getValue(), containsInAnyOrder(players));
+    }
+
+    private void assertRanPurgeWithNames(String... names) {
+        verify(purgeService).executePurge(playerCaptor.capture(), namesCaptor.capture());
+        assertThat(namesCaptor.getValue(), containsInAnyOrder(names));
+        assertThat(playerCaptor.getValue(), empty());
+    }
+
+}