Create unit tests for antibot refactoring

src/main/java/fr/xephi/authme/listener/PlayerListener.java

@@ -223,7 +223,6 @@ public class PlayerListener implements Listener {
             // Slow stuff
             final PlayerAuth auth = dataSource.getAuth(name);
             final boolean isAuthAvailable = (auth != null);
-            final String lowerName = name.toLowerCase();
             onJoinVerifier.checkAntibot(player, isAuthAvailable);
             onJoinVerifier.checkKickNonRegistered(isAuthAvailable);
             onJoinVerifier.checkNameCasing(player, auth);

src/main/java/fr/xephi/authme/service/AntiBotService.java

@@ -35,10 +35,11 @@ public class AntiBotService implements SettingsDependent {
     private AntiBotStatus antiBotStatus;
     private BukkitTask disableTask;
     private int antibotPlayers;
-    private final CopyOnWriteArrayList<String> antibotKicked = new CopyOnWriteArrayList();
+    private final CopyOnWriteArrayList<String> antibotKicked = new CopyOnWriteArrayList<>();
 
     @Inject
-    AntiBotService(Settings settings, Messages messages, PermissionsManager permissionsManager, BukkitService bukkitService) {
+    AntiBotService(Settings settings, Messages messages, PermissionsManager permissionsManager,
+                   BukkitService bukkitService) {
         // Instances
         this.messages = messages;
         this.permissionsManager = permissionsManager;
@@ -75,7 +76,7 @@ public class AntiBotService implements SettingsDependent {
         }, 90 * TICKS_PER_SECOND);
     }
 
-    protected void startProtection() {
+    private void startProtection() {
         // Disable existing antibot session
         stopProtection();
         // Enable the new session
@@ -98,7 +99,7 @@ public class AntiBotService implements SettingsDependent {
     }
 
     private void stopProtection() {
-        if(antiBotStatus != AntiBotStatus.ACTIVE) {
+        if (antiBotStatus != AntiBotStatus.ACTIVE) {
             return;
         }
 
@@ -134,13 +135,12 @@ public class AntiBotService implements SettingsDependent {
      * @param started the new protection status
      */
     public void overrideAntiBotStatus(boolean started) {
-        if (antiBotStatus == AntiBotStatus.DISABLED) {
+        if (antiBotStatus != AntiBotStatus.DISABLED) {
-            return;
+            if (started) {
-        }
+                startProtection();
-        if (started) {
+            } else {
-            startProtection();
+                stopProtection();
-        } else {
+            }
-            stopProtection();
         }
     }
 

src/test/java/fr/xephi/authme/listener/OnJoinVerifierTest.java

@@ -1,6 +1,5 @@
 package fr.xephi.authme.listener;
 
-import fr.xephi.authme.service.AntiBotService;
 import fr.xephi.authme.TestHelper;
 import fr.xephi.authme.cache.auth.PlayerAuth;
 import fr.xephi.authme.datasource.DataSource;
@@ -8,6 +7,7 @@ import fr.xephi.authme.output.MessageKey;
 import fr.xephi.authme.output.Messages;
 import fr.xephi.authme.permission.PermissionsManager;
 import fr.xephi.authme.permission.PlayerStatePermission;
+import fr.xephi.authme.service.AntiBotService;
 import fr.xephi.authme.settings.Settings;
 import fr.xephi.authme.settings.properties.ProtectionSettings;
 import fr.xephi.authme.settings.properties.RegistrationSettings;
@@ -376,33 +376,53 @@ public class OnJoinVerifierTest {
     }
 
     @Test
-    public void shouldCheckAntiBot() throws FailedVerificationException {
+    public void shouldAllowUser() throws FailedVerificationException {
         // given
-        Player player = newPlayerWithName("test123");
+        Player player = newPlayerWithName("Bobby");
-        boolean hasAuth = false;
+        boolean isAuthAvailable = false;
         given(permissionsManager.hasPermission(player, PlayerStatePermission.BYPASS_ANTIBOT)).willReturn(false);
-        given(antiBotService.getAntiBotStatus()).willReturn(AntiBotService.AntiBotStatus.LISTENING);
+        given(antiBotService.shouldKick(isAuthAvailable)).willReturn(false);
 
         // when
-        onJoinVerifier.checkAntibot(player, hasAuth);
+        onJoinVerifier.checkAntibot(player, isAuthAvailable);
 
         // then
-        verify(antiBotService).shouldKick(hasAuth);
+        verify(permissionsManager).hasPermission(player, PlayerStatePermission.BYPASS_ANTIBOT);
+        verify(antiBotService).shouldKick(isAuthAvailable);
     }
 
     @Test
-    public void shouldAllowUserWithAuth() throws FailedVerificationException {
+    public void shouldAllowUserWithBypassPermission() throws FailedVerificationException {
         // given
-        Player player = newPlayerWithName("Bobby");
+        Player player = newPlayerWithName("Steward");
-        boolean hasAuth = true;
+        boolean isAuthAvailable = false;
-        given(permissionsManager.hasPermission(player, PlayerStatePermission.BYPASS_ANTIBOT)).willReturn(false);
+        given(permissionsManager.hasPermission(player, PlayerStatePermission.BYPASS_ANTIBOT)).willReturn(true);
-        given(antiBotService.getAntiBotStatus()).willReturn(AntiBotService.AntiBotStatus.ACTIVE);
+        given(antiBotService.shouldKick(isAuthAvailable)).willReturn(true);
 
         // when
-        onJoinVerifier.checkAntibot(player, hasAuth);
+        onJoinVerifier.checkAntibot(player, isAuthAvailable);
 
         // then
-        verify(antiBotService).shouldKick(hasAuth);
+        verify(permissionsManager).hasPermission(player, PlayerStatePermission.BYPASS_ANTIBOT);
+    }
+
+    @Test
+    public void shouldKickUserForFailedAntibotCheck() throws FailedVerificationException {
+        // given
+        Player player = newPlayerWithName("D3");
+        boolean isAuthAvailable = false;
+        given(permissionsManager.hasPermission(player, PlayerStatePermission.BYPASS_ANTIBOT)).willReturn(false);
+        given(antiBotService.shouldKick(isAuthAvailable)).willReturn(true);
+
+        // when / then
+        try {
+            onJoinVerifier.checkAntibot(player, isAuthAvailable);
+            fail("Expected exception to be thrown");
+        } catch (FailedVerificationException e) {
+            verify(permissionsManager).hasPermission(player, PlayerStatePermission.BYPASS_ANTIBOT);
+            verify(antiBotService).shouldKick(isAuthAvailable);
+        }
+
     }
 
     /**
@@ -473,7 +493,7 @@ public class OnJoinVerifierTest {
         return player;
     }
 
-    @SuppressWarnings({ "unchecked", "rawtypes" })
+    @SuppressWarnings("unchecked")
     private void returnOnlineListFromBukkitServer(Collection<Player> onlineList) {
         // Note ljacqu 20160529: The compiler gets lost in generics because Collection<? extends Player> is returned
         // from getOnlinePlayers(). We need to uncheck onlineList to a simple Collection or it will refuse to compile.

src/test/java/fr/xephi/authme/service/AntiBotServiceTest.java

@@ -0,0 +1,211 @@
+package fr.xephi.authme.service;
+
+import ch.jalu.injector.testing.BeforeInjecting;
+import ch.jalu.injector.testing.DelayedInjectionRunner;
+import ch.jalu.injector.testing.InjectDelayed;
+import fr.xephi.authme.ReflectionTestUtils;
+import fr.xephi.authme.output.MessageKey;
+import fr.xephi.authme.output.Messages;
+import fr.xephi.authme.permission.AdminPermission;
+import fr.xephi.authme.permission.PermissionsManager;
+import fr.xephi.authme.settings.Settings;
+import fr.xephi.authme.settings.properties.ProtectionSettings;
+import fr.xephi.authme.util.BukkitService;
+import org.bukkit.entity.Player;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+
+import java.util.Arrays;
+import java.util.List;
+
+import static fr.xephi.authme.TestHelper.runSyncDelayedTaskWithDelay;
+import static org.hamcrest.Matchers.equalTo;
+import static org.junit.Assert.assertThat;
+import static org.mockito.BDDMockito.given;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.only;
+import static org.mockito.Mockito.reset;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyZeroInteractions;
+
+/**
+ * Test for {@link AntiBotService}.
+ */
+@RunWith(DelayedInjectionRunner.class)
+public class AntiBotServiceTest {
+
+    @InjectDelayed
+    private AntiBotService antiBotService;
+
+    @Mock
+    private Settings settings;
+    @Mock
+    private Messages messages;
+    @Mock
+    private PermissionsManager permissionsManager;
+    @Mock
+    private BukkitService bukkitService;
+
+    @BeforeInjecting
+    public void initSettings() {
+        given(settings.getProperty(ProtectionSettings.ANTIBOT_DURATION)).willReturn(10);
+        given(settings.getProperty(ProtectionSettings.ANTIBOT_SENSIBILITY)).willReturn(5);
+        given(settings.getProperty(ProtectionSettings.ENABLE_ANTIBOT)).willReturn(true);
+    }
+
+    @Test
+    public void shouldStartListenerOnStartup() {
+        // given / when
+        runSyncDelayedTaskWithDelay(bukkitService);
+
+        // then
+        assertThat(antiBotService.getAntiBotStatus(), equalTo(AntiBotService.AntiBotStatus.LISTENING));
+    }
+
+    @Test
+    public void shouldNotListenForDisabledSetting() {
+        // given
+        reset(bukkitService);
+        given(settings.getProperty(ProtectionSettings.ENABLE_ANTIBOT)).willReturn(false);
+
+        // when
+        AntiBotService antiBotService = new AntiBotService(settings, messages, permissionsManager, bukkitService);
+
+        // then
+        assertThat(antiBotService.getAntiBotStatus(), equalTo(AntiBotService.AntiBotStatus.DISABLED));
+        verifyZeroInteractions(bukkitService);
+    }
+
+    @Test
+    public void shouldActivateAntibot() {
+        // given - listening antibot
+        runSyncDelayedTaskWithDelay(bukkitService);
+
+        // when
+        antiBotService.overrideAntiBotStatus(true);
+
+        // then
+        assertThat(antiBotService.getAntiBotStatus(), equalTo(AntiBotService.AntiBotStatus.ACTIVE));
+        // Check that a task is scheduled to disable again
+        runSyncDelayedTaskWithDelay(bukkitService);
+        assertThat(antiBotService.getAntiBotStatus(), equalTo(AntiBotService.AntiBotStatus.LISTENING));
+    }
+
+    @Test
+    public void shouldNotActivateAntibotForDisabledSetting() {
+        // given - disabled antibot
+        reset(bukkitService);
+        assertThat(antiBotService.getAntiBotStatus(), equalTo(AntiBotService.AntiBotStatus.DISABLED));
+        given(settings.getProperty(ProtectionSettings.ENABLE_ANTIBOT)).willReturn(false);
+
+        // when
+        antiBotService.overrideAntiBotStatus(true);
+
+        // then
+        assertThat(antiBotService.getAntiBotStatus(), equalTo(AntiBotService.AntiBotStatus.DISABLED));
+        verifyZeroInteractions(bukkitService);
+    }
+
+    @Test
+    public void shouldKeepTrackOfKickedPlayers() {
+        // given
+        String name = "eratic";
+        antiBotService.addPlayerKick(name);
+
+        // when
+        boolean result1 = antiBotService.wasPlayerKicked(name);
+        boolean result2 = antiBotService.wasPlayerKicked("other");
+
+        // then
+        assertThat(result1, equalTo(true));
+        assertThat(result2, equalTo(false));
+    }
+
+    @Test
+    public void shouldAcceptPlayerToJoin() {
+        // given - listening antibot
+        runSyncDelayedTaskWithDelay(bukkitService);
+
+        // when
+        boolean result = antiBotService.shouldKick(false);
+
+        // then
+        assertThat(result, equalTo(false));
+    }
+
+    @Test
+    public void shouldRejectPlayerWithoutAuth() {
+        // given - active antibot
+        runSyncDelayedTaskWithDelay(bukkitService);
+        antiBotService.overrideAntiBotStatus(true);
+
+        // when
+        boolean kickWithoutAuth = antiBotService.shouldKick(false);
+        boolean kickWithAuth = antiBotService.shouldKick(true);
+
+        // then
+        assertThat(kickWithoutAuth, equalTo(true));
+        assertThat(kickWithAuth, equalTo(false));
+    }
+
+    @Test
+    public void shouldIncreaseCountAndDecreaseAfterDelay() {
+        // given - listening antibot
+        runSyncDelayedTaskWithDelay(bukkitService);
+        reset(bukkitService);
+        assertThat(getAntiBotCount(antiBotService), equalTo(0));
+
+        // when
+        antiBotService.handlePlayerJoin();
+
+        // then
+        assertThat(getAntiBotCount(antiBotService), equalTo(1));
+        runSyncDelayedTaskWithDelay(bukkitService);
+        assertThat(getAntiBotCount(antiBotService), equalTo(0));
+    }
+
+    @Test
+    public void shouldActivateAntibotAfterThreshold() {
+        // given
+        int sensitivity = 10;
+        given(settings.getProperty(ProtectionSettings.ANTIBOT_SENSIBILITY)).willReturn(sensitivity);
+        reset(bukkitService);
+        AntiBotService antiBotService = new AntiBotService(settings, messages, permissionsManager, bukkitService);
+        runSyncDelayedTaskWithDelay(bukkitService);
+
+        for (int i = 0; i < sensitivity; ++i) {
+            antiBotService.handlePlayerJoin();
+        }
+        assertThat(antiBotService.getAntiBotStatus(), equalTo(AntiBotService.AntiBotStatus.LISTENING));
+
+        // when
+        antiBotService.handlePlayerJoin();
+
+        // then
+        assertThat(antiBotService.getAntiBotStatus(), equalTo(AntiBotService.AntiBotStatus.ACTIVE));
+    }
+
+    @Test
+    @SuppressWarnings("unchecked")
+    public void shouldInformPlayersOnActivation() {
+        // given - listening antibot
+        runSyncDelayedTaskWithDelay(bukkitService);
+        List<Player> players = Arrays.asList(mock(Player.class), mock(Player.class));
+        given(bukkitService.getOnlinePlayers()).willReturn((List) players);
+        given(permissionsManager.hasPermission(players.get(0), AdminPermission.ANTIBOT_MESSAGES)).willReturn(false);
+        given(permissionsManager.hasPermission(players.get(1), AdminPermission.ANTIBOT_MESSAGES)).willReturn(true);
+
+        // when
+        antiBotService.overrideAntiBotStatus(true);
+
+        // then
+        verify(permissionsManager).hasPermission(players.get(0), AdminPermission.ANTIBOT_MESSAGES);
+        verify(permissionsManager).hasPermission(players.get(1), AdminPermission.ANTIBOT_MESSAGES);
+        verify(messages, only()).send(players.get(1), MessageKey.ANTIBOT_AUTO_ENABLED_MESSAGE);
+    }
+
+    private static int getAntiBotCount(AntiBotService antiBotService) {
+        return ReflectionTestUtils.getFieldValue(AntiBotService.class, antiBotService, "antibotPlayers");
+    }
+}

src/test/java/fr/xephi/authme/service/AntiBotTest.java

@@ -1,175 +0,0 @@
-package fr.xephi.authme.service;
-
-import fr.xephi.authme.ReflectionTestUtils;
-import fr.xephi.authme.TestHelper;
-import fr.xephi.authme.output.MessageKey;
-import fr.xephi.authme.output.Messages;
-import fr.xephi.authme.permission.AdminPermission;
-import fr.xephi.authme.permission.PermissionsManager;
-import fr.xephi.authme.permission.PlayerStatePermission;
-import fr.xephi.authme.service.AntiBotService;
-import fr.xephi.authme.settings.Settings;
-import fr.xephi.authme.settings.properties.ProtectionSettings;
-import fr.xephi.authme.util.BukkitService;
-import org.bukkit.entity.Player;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.ArgumentCaptor;
-import org.mockito.Mock;
-import org.mockito.runners.MockitoJUnitRunner;
-
-import java.util.Arrays;
-import java.util.List;
-
-import static fr.xephi.authme.util.BukkitService.TICKS_PER_MINUTE;
-import static fr.xephi.authme.util.BukkitService.TICKS_PER_SECOND;
-import static org.hamcrest.Matchers.contains;
-import static org.hamcrest.Matchers.empty;
-import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.hasSize;
-import static org.junit.Assert.assertThat;
-import static org.mockito.BDDMockito.given;
-import static org.mockito.Matchers.any;
-import static org.mockito.Matchers.anyLong;
-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 AntiBotService}.
- */
-@RunWith(MockitoJUnitRunner.class)
-public class AntiBotTest {
-
-    @Mock
-    private Settings settings;
-    @Mock
-    private Messages messages;
-    @Mock
-    private PermissionsManager permissionsManager;
-    @Mock
-    private BukkitService bukkitService;
-
-    @Before
-    public void setDefaultSettingValues() {
-        given(settings.getProperty(ProtectionSettings.ENABLE_ANTIBOT)).willReturn(true);
-    }
-
-    @Test
-    public void shouldKeepAntiBotDisabled() {
-        // given / when
-        given(settings.getProperty(ProtectionSettings.ENABLE_ANTIBOT)).willReturn(false);
-        AntiBotService antiBot = new AntiBotService(settings, messages, permissionsManager, bukkitService);
-
-        // then
-        verify(bukkitService, never()).scheduleSyncDelayedTask(any(Runnable.class), anyLong());
-        assertThat(antiBot.getAntiBotStatus(), equalTo(AntiBotService.AntiBotStatus.DISABLED));
-    }
-
-    @Test
-    public void shouldTransitionToListening() {
-        // given / when
-        AntiBotService antiBot = new AntiBotService(settings, messages, permissionsManager, bukkitService);
-        TestHelper.runSyncDelayedTaskWithDelay(bukkitService);
-
-        // then
-        assertThat(antiBot.getAntiBotStatus(), equalTo(AntiBotService.AntiBotStatus.LISTENING));
-    }
-
-    @Test
-    public void shouldSetStatusToActive() {
-        // given
-        AntiBotService antiBot = createListeningAntiBot();
-
-        // when
-        antiBot.overrideAntiBotStatus(true);
-
-        // then
-        assertThat(antiBot.getAntiBotStatus(), equalTo(AntiBotService.AntiBotStatus.ACTIVE));
-    }
-
-    @Test
-    public void shouldSetStatusToListening() {
-        // given
-        AntiBotService antiBot = createListeningAntiBot();
-
-        // when
-        antiBot.overrideAntiBotStatus(false);
-
-        // then
-        assertThat(antiBot.getAntiBotStatus(), equalTo(AntiBotService.AntiBotStatus.LISTENING));
-    }
-
-    @Test
-    public void shouldRemainDisabled() {
-        // given
-        given(settings.getProperty(ProtectionSettings.ENABLE_ANTIBOT)).willReturn(false);
-        AntiBotService antiBot = new AntiBotService(settings, messages, permissionsManager, bukkitService);
-
-        // when
-        antiBot.overrideAntiBotStatus(true);
-
-        // then
-        assertThat(antiBot.getAntiBotStatus(), equalTo(AntiBotService.AntiBotStatus.DISABLED));
-    }
-
-    @Test
-    public void shouldActivateAntiBot() {
-        // given
-        int duration = 300;
-        given(settings.getProperty(ProtectionSettings.ANTIBOT_DURATION)).willReturn(duration);
-        AntiBotService antiBot = createListeningAntiBot();
-        List<Player> onlinePlayers = Arrays.asList(mock(Player.class), mock(Player.class), mock(Player.class));
-        given(bukkitService.getOnlinePlayers()).willReturn((List) onlinePlayers);
-        given(permissionsManager.hasPermission(onlinePlayers.get(0), AdminPermission.ANTIBOT_MESSAGES)).willReturn(true);
-        given(permissionsManager.hasPermission(onlinePlayers.get(1), AdminPermission.ANTIBOT_MESSAGES)).willReturn(false);
-        given(permissionsManager.hasPermission(onlinePlayers.get(2), AdminPermission.ANTIBOT_MESSAGES)).willReturn(true);
-
-        // when
-        antiBot.startProtection();
-
-        // then
-        assertThat(antiBot.getAntiBotStatus(), equalTo(AntiBotService.AntiBotStatus.ACTIVE));
-        verify(bukkitService).getOnlinePlayers();
-        verify(permissionsManager, times(3)).hasPermission(any(Player.class), eq(AdminPermission.ANTIBOT_MESSAGES));
-        verify(messages).send(onlinePlayers.get(0), MessageKey.ANTIBOT_AUTO_ENABLED_MESSAGE);
-        verify(messages, never()).send(onlinePlayers.get(1), MessageKey.ANTIBOT_AUTO_ENABLED_MESSAGE);
-        verify(messages).send(onlinePlayers.get(2), MessageKey.ANTIBOT_AUTO_ENABLED_MESSAGE);
-        long expectedTicks = duration * TICKS_PER_MINUTE;
-        verify(bukkitService).scheduleSyncDelayedTask(any(Runnable.class), eq(expectedTicks));
-    }
-
-    @Test
-    public void shouldDisableAntiBotAfterSetDuration() {
-        // given
-        given(messages.retrieve(MessageKey.ANTIBOT_AUTO_ENABLED_MESSAGE)).willReturn(new String[0]);
-        given(messages.retrieve(MessageKey.ANTIBOT_AUTO_DISABLED_MESSAGE))
-            .willReturn(new String[]{"Disabled...", "Placeholder: %m."});
-        given(settings.getProperty(ProtectionSettings.ANTIBOT_DURATION)).willReturn(4);
-        AntiBotService antiBot = createListeningAntiBot();
-
-        // when
-        antiBot.startProtection();
-        TestHelper.runSyncDelayedTaskWithDelay(bukkitService);
-
-        // then
-        assertThat(antiBot.getAntiBotStatus(), equalTo(AntiBotService.AntiBotStatus.LISTENING));
-        verify(bukkitService).scheduleSyncDelayedTask(any(Runnable.class), eq((long) 4800));
-        ArgumentCaptor<String> captor = ArgumentCaptor.forClass(String.class);
-        verify(bukkitService, times(2)).broadcastMessage(captor.capture());
-        assertThat(captor.getAllValues(), contains("Disabled...", "Placeholder: 4."));
-    }
-
-    private AntiBotService createListeningAntiBot() {
-        AntiBotService antiBot = new AntiBotService(settings, messages, permissionsManager, bukkitService);
-        TestHelper.runSyncDelayedTaskWithDelay(bukkitService);
-        // Make BukkitService forget about all interactions up to here
-        reset(bukkitService);
-        return antiBot;
-    }
-
-}