Code householding, add tests to TempbanManager

- Delegate event firing to BukkitService
- Write tests for IP banning function
- Update comments on tempban properties in config.yml

src/test/java/fr/xephi/authme/TestHelper.java

@@ -1,6 +1,7 @@
 package fr.xephi.authme;
 
 import fr.xephi.authme.util.BukkitService;
+import org.bukkit.entity.Player;
 import org.mockito.ArgumentCaptor;
 import org.mockito.Mockito;
 
@@ -8,6 +9,8 @@ import java.io.File;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Modifier;
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
@@ -15,7 +18,9 @@ import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.logging.Logger;
 
+import static org.mockito.BDDMockito.given;
 import static org.mockito.Matchers.anyLong;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 
 /**
@@ -144,4 +149,17 @@ public final class TestHelper {
         }
     }
 
+    /**
+     * Configures the player mock to return the given IP address.
+     *
+     * @param player the player mock
+     * @param ip the ip address it should return
+     */
+    public static void mockPlayerIp(Player player, String ip) {
+        InetAddress inetAddress = mock(InetAddress.class);
+        given(inetAddress.getHostAddress()).willReturn(ip);
+        InetSocketAddress inetSocketAddress = new InetSocketAddress(inetAddress, 8093);
+        given(player.getAddress()).willReturn(inetSocketAddress);
+    }
+
 }

src/test/java/fr/xephi/authme/cache/TempbanManagerTest.java

@@ -1,21 +1,31 @@
 package fr.xephi.authme.cache;
 
 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.settings.NewSetting;
 import fr.xephi.authme.settings.properties.SecuritySettings;
 import fr.xephi.authme.util.BukkitService;
+import org.bukkit.entity.Player;
 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.Calendar;
+import java.util.Date;
 import java.util.Map;
 
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.lessThan;
 import static org.junit.Assert.assertThat;
 import static org.mockito.BDDMockito.given;
+import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyZeroInteractions;
 
 /**
  * Test for {@link TempbanManager}.
@@ -23,11 +33,13 @@ import static org.mockito.Mockito.mock;
 @RunWith(MockitoJUnitRunner.class)
 public class TempbanManagerTest {
 
-    @Mock
-    BukkitService bukkitService;
+    private static final long DATE_TOLERANCE_MILLISECONDS = 100L;
 
     @Mock
-    Messages messages;
+    private BukkitService bukkitService;
+
+    @Mock
+    private Messages messages;
 
     @Test
     public void shouldAddCounts() {
@@ -109,6 +121,71 @@ public class TempbanManagerTest {
         assertThat(result, equalTo(false));
     }
 
+    @Test
+    public void shouldNotIssueBanIfDisabled() {
+        // given
+        NewSetting settings = mockSettings(0, 0);
+        given(settings.getProperty(SecuritySettings.TEMPBAN_ON_MAX_LOGINS)).willReturn(false);
+        Player player = mock(Player.class);
+        TempbanManager manager = new TempbanManager(bukkitService, messages, settings);
+
+        // when
+        manager.tempbanPlayer(player);
+
+        // then
+        verifyZeroInteractions(player, bukkitService);
+    }
+
+    @Test
+    public void shouldBanPlayerIp() {
+        // given
+        Player player = mock(Player.class);
+        String ip = "123.45.67.89";
+        TestHelper.mockPlayerIp(player, ip);
+        String banReason = "IP ban too many logins";
+        given(messages.retrieveSingle(MessageKey.TEMPBAN_MAX_LOGINS)).willReturn(banReason);
+        NewSetting settings = mockSettings(2, 100);
+        TempbanManager manager = new TempbanManager(bukkitService, messages, settings);
+
+        // when
+        manager.tempbanPlayer(player);
+        TestHelper.runSyncDelayedTask(bukkitService);
+
+        // then
+        verify(player).kickPlayer(banReason);
+        ArgumentCaptor<Date> captor = ArgumentCaptor.forClass(Date.class);
+        verify(bukkitService).banIp(eq(ip), eq(banReason), captor.capture(), eq("AuthMe"));
+
+        // Compute the expected expiration date and check that the actual date is within the difference tolerance
+        Calendar cal = Calendar.getInstance();
+        cal.add(Calendar.MINUTE, 100);
+        long expectedExpiration = cal.getTime().getTime();
+        assertThat(Math.abs(captor.getValue().getTime() - expectedExpiration), lessThan(DATE_TOLERANCE_MILLISECONDS));
+    }
+
+    @Test
+    public void shouldResetCountAfterBan() {
+        // given
+        Player player = mock(Player.class);
+        String ip = "22.44.66.88";
+        TestHelper.mockPlayerIp(player, ip);
+        String banReason = "kick msg";
+        given(messages.retrieveSingle(MessageKey.TEMPBAN_MAX_LOGINS)).willReturn(banReason);
+        NewSetting settings = mockSettings(10, 60);
+        TempbanManager manager = new TempbanManager(bukkitService, messages, settings);
+        manager.increaseCount(ip);
+        manager.increaseCount(ip);
+        manager.increaseCount(ip);
+
+        // when
+        manager.tempbanPlayer(player);
+        TestHelper.runSyncDelayedTask(bukkitService);
+
+        // then
+        verify(player).kickPlayer(banReason);
+        assertHasCount(manager, ip, null);
+    }
+
     private static NewSetting mockSettings(int maxTries, int tempbanLength) {
         NewSetting settings = mock(NewSetting.class);
         given(settings.getProperty(SecuritySettings.TEMPBAN_ON_MAX_LOGINS)).willReturn(true);

src/test/java/fr/xephi/authme/command/executable/authme/GetIpCommandTest.java

@@ -1,5 +1,6 @@
 package fr.xephi.authme.command.executable.authme;
 
+import fr.xephi.authme.TestHelper;
 import fr.xephi.authme.util.BukkitService;
 import org.bukkit.command.CommandSender;
 import org.bukkit.entity.Player;
@@ -9,8 +10,6 @@ import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.runners.MockitoJUnitRunner;
 
-import java.net.InetAddress;
-import java.net.InetSocketAddress;
 import java.util.Collections;
 
 import static org.hamcrest.Matchers.allOf;
@@ -68,10 +67,7 @@ public class GetIpCommandTest {
     private static Player mockPlayer(String name, String ip) {
         Player player = mock(Player.class);
         given(player.getName()).willReturn(name);
-        InetAddress inetAddress = mock(InetAddress.class);
-        given(inetAddress.getHostAddress()).willReturn(ip);
-        InetSocketAddress inetSocketAddress = new InetSocketAddress(inetAddress, 8093);
-        given(player.getAddress()).willReturn(inetSocketAddress);
+        TestHelper.mockPlayerIp(player, ip);
         return player;
     }
 }