diff --git a/src/main/java/fr/xephi/authme/AntiBot.java b/src/main/java/fr/xephi/authme/AntiBot.java index 3b8d0afc8..72be5ac7f 100644 --- a/src/main/java/fr/xephi/authme/AntiBot.java +++ b/src/main/java/fr/xephi/authme/AntiBot.java @@ -17,7 +17,7 @@ public class AntiBot { private static final AuthMe plugin = AuthMe.getInstance(); private static final Messages messages = plugin.getMessages(); - private static Wrapper wrapper = new Wrapper(plugin); + private static Wrapper wrapper = Wrapper.getInstance(); private static final List antibotPlayers = new ArrayList<>(); private static AntiBotStatus antiBotStatus = AntiBotStatus.DISABLED; diff --git a/src/main/java/fr/xephi/authme/ConsoleLogger.java b/src/main/java/fr/xephi/authme/ConsoleLogger.java index 6009bc3a1..890318046 100644 --- a/src/main/java/fr/xephi/authme/ConsoleLogger.java +++ b/src/main/java/fr/xephi/authme/ConsoleLogger.java @@ -17,7 +17,7 @@ import java.util.Date; */ public final class ConsoleLogger { - private static Wrapper wrapper = new Wrapper(AuthMe.getInstance()); + private static Wrapper wrapper = Wrapper.getInstance(); private static final DateFormat df = new SimpleDateFormat("[MM-dd HH:mm:ss]"); private ConsoleLogger() { diff --git a/src/main/java/fr/xephi/authme/util/GeoLiteAPI.java b/src/main/java/fr/xephi/authme/util/GeoLiteAPI.java index b9d730e7a..ebd0deb31 100644 --- a/src/main/java/fr/xephi/authme/util/GeoLiteAPI.java +++ b/src/main/java/fr/xephi/authme/util/GeoLiteAPI.java @@ -17,7 +17,6 @@ public class GeoLiteAPI { "available at http://www.maxmind.com"; private static final String GEOIP_URL = "http://geolite.maxmind.com/download/geoip/database/GeoLiteCountry" + "/GeoIP.dat.gz"; - private static final Wrapper wrapper = new Wrapper(AuthMe.getInstance()); private static final AuthMe plugin = AuthMe.getInstance(); private static LookupService lookupService; diff --git a/src/main/java/fr/xephi/authme/util/Utils.java b/src/main/java/fr/xephi/authme/util/Utils.java index 327fc43dc..33bf06d47 100644 --- a/src/main/java/fr/xephi/authme/util/Utils.java +++ b/src/main/java/fr/xephi/authme/util/Utils.java @@ -26,7 +26,7 @@ import java.util.Collections; */ public final class Utils { - private static final AuthMe plugin; + private static AuthMe plugin; private static Wrapper wrapper; private static boolean getOnlinePlayersIsCollection = false; @@ -34,7 +34,7 @@ public final class Utils { static { plugin = AuthMe.getInstance(); - wrapper = new Wrapper(plugin); + wrapper = Wrapper.getInstance(); initializeOnlinePlayersIsCollectionField(); } @@ -116,9 +116,10 @@ public final class Utils { // Get the permissions manager, and make sure it's valid PermissionsManager permsMan = plugin.getPermissionsManager(); - if (permsMan == null) - ConsoleLogger.showError("Failed to access permissions manager instance, shutting down."); - assert permsMan != null; + if (permsMan == null) { + ConsoleLogger.showError("Failed to access permissions manager instance, aborting."); + return false; + } // Remove old groups permsMan.removeGroups(player, Arrays.asList(Settings.unRegisteredGroup, diff --git a/src/main/java/fr/xephi/authme/util/Wrapper.java b/src/main/java/fr/xephi/authme/util/Wrapper.java index 1505a203e..978d0ab58 100644 --- a/src/main/java/fr/xephi/authme/util/Wrapper.java +++ b/src/main/java/fr/xephi/authme/util/Wrapper.java @@ -13,22 +13,31 @@ import java.util.logging.Logger; */ public class Wrapper { - private AuthMe authMe; + private static Wrapper singleton; - public Wrapper(AuthMe authMe) { - this.authMe = authMe; + /** + * Package-private constructor for testing purposes to inject a mock instance. + */ + Wrapper() { + } + + public static Wrapper getInstance() { + if (singleton == null) { + singleton = new Wrapper(); + } + return singleton; } public AuthMe getAuthMe() { - return authMe; + return AuthMe.getInstance(); } public Server getServer() { - return authMe.getServer(); + return getAuthMe().getServer(); } public Logger getLogger() { - return authMe.getLogger(); + return getAuthMe().getLogger(); } public BukkitScheduler getScheduler() { diff --git a/src/test/java/fr/xephi/authme/AuthMeMockUtil.java b/src/test/java/fr/xephi/authme/AuthMeMockUtil.java index ef22f4f59..1cc0ef7c8 100644 --- a/src/test/java/fr/xephi/authme/AuthMeMockUtil.java +++ b/src/test/java/fr/xephi/authme/AuthMeMockUtil.java @@ -3,6 +3,7 @@ package fr.xephi.authme; import fr.xephi.authme.cache.auth.PlayerCache; import fr.xephi.authme.settings.Messages; import fr.xephi.authme.util.Wrapper; +import fr.xephi.authme.util.WrapperMock; import org.mockito.Mockito; import java.lang.reflect.Field; @@ -43,6 +44,11 @@ public final class AuthMeMockUtil { mockSingletonForClass(PlayerCache.class, "singleton", mock); } + public static void initializeWrapperMock() { + WrapperMock wrapper = new WrapperMock(); + mockSingletonForClass(Wrapper.class, "singleton", wrapper); + } + /** * Set the given class' {@link Wrapper} field to a mock implementation. * @@ -59,7 +65,7 @@ public final class AuthMeMockUtil { } public static Wrapper insertMockWrapperInstance(Class clazz, String fieldName, AuthMe authMe) { - Wrapper wrapperMock = new WrapperMock(authMe); + Wrapper wrapperMock = new WrapperMock(); mockSingletonForClass(clazz, fieldName, wrapperMock); return wrapperMock; } @@ -78,7 +84,7 @@ public final class AuthMeMockUtil { * @param fieldName The field name * @param mock The mock to set for the given field */ - private static void mockSingletonForClass(Class clazz, String fieldName, Object mock) { + public static void mockSingletonForClass(Class clazz, String fieldName, Object mock) { try { Field instance = clazz.getDeclaredField(fieldName); instance.setAccessible(true); diff --git a/src/test/java/fr/xephi/authme/util/UtilsTest.java b/src/test/java/fr/xephi/authme/util/UtilsTest.java index f228947f8..586b81f3e 100644 --- a/src/test/java/fr/xephi/authme/util/UtilsTest.java +++ b/src/test/java/fr/xephi/authme/util/UtilsTest.java @@ -2,63 +2,44 @@ package fr.xephi.authme.util; import fr.xephi.authme.AuthMe; import fr.xephi.authme.AuthMeMockUtil; -import fr.xephi.authme.WrapperMock; +import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.permission.PermissionsManager; -import org.bukkit.Server; +import fr.xephi.authme.settings.Settings; +import org.bukkit.GameMode; import org.bukkit.entity.Player; -import org.bukkit.plugin.Plugin; -import org.bukkit.scheduler.BukkitScheduler; -import org.bukkit.scheduler.BukkitTask; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; +import org.mockito.Mockito; import java.lang.reflect.Field; import java.util.Collection; +import java.util.logging.Logger; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; import static org.junit.Assert.assertThat; -import static org.mockito.Mockito.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.*; /** * Test for the {@link Utils} class. */ -@Ignore -// TODO ljacqu 20151123: Fix test setup public class UtilsTest { private AuthMe authMeMock; private PermissionsManager permissionsManagerMock; - private Wrapper wrapperMock; @Before public void setUpMocks() { authMeMock = AuthMeMockUtil.mockAuthMeInstance(); - - // We need to create the Wrapper mock before injecting it into Utils because it runs a lot of code in - // a static block which needs the proper mocks to be set up. - wrapperMock = new WrapperMock(authMeMock); - Server serverMock = wrapperMock.getServer(); - - BukkitScheduler schedulerMock = mock(BukkitScheduler.class); - when(serverMock.getScheduler()).thenReturn(schedulerMock); - - - when(schedulerMock.runTaskAsynchronously(any(Plugin.class), any(Runnable.class))) - .thenReturn(mock(BukkitTask.class)); - - System.out.println("Initialized scheduler mock for server mock"); - AuthMeMockUtil.insertMockWrapperInstance(Utils.class, "wrapper", (WrapperMock) wrapperMock); - System.out.println("Iniadfk"); - - permissionsManagerMock = mock(PermissionsManager.class); + AuthMeMockUtil.mockSingletonForClass(Utils.class, "plugin", authMeMock); + permissionsManagerMock = Mockito.mock(PermissionsManager.class); when(authMeMock.getPermissionsManager()).thenReturn(permissionsManagerMock); + + AuthMeMockUtil.initializeWrapperMock(); } - // TODO ljacques 20151122: The tests for Utils.forceGM somehow can't be set up with the mocks correctly - /*@Test + @Test public void shouldForceSurvivalGameMode() { // given Player player = mock(Player.class); @@ -68,6 +49,7 @@ public class UtilsTest { Utils.forceGM(player); // then + verify(authMeMock).getPermissionsManager(); verify(player).setGameMode(GameMode.SURVIVAL); } @@ -84,10 +66,40 @@ public class UtilsTest { verify(authMeMock).getPermissionsManager(); verify(permissionsManagerMock).hasPermission(player, "authme.bypassforcesurvival"); verify(player, never()).setGameMode(any(GameMode.class)); - }*/ + } @Test - // Note ljacqu 20151122: This is a heavy test setup with Reflections... If it causes trouble, skip it with @Ignore + public void shouldNotAddToNormalGroupIfPermissionsAreDisabled() { + // given + Settings.isPermissionCheckEnabled = false; + Player player = mock(Player.class); + + // when + boolean result = Utils.addNormal(player, "test_group"); + + // then + assertThat(result, equalTo(false)); + verify(authMeMock, never()).getPermissionsManager(); + } + + @Test + public void shouldNotAddToNormalGroupIfPermManagerIsNull() { + // given + Settings.isPermissionCheckEnabled = true; + given(authMeMock.getPermissionsManager()).willReturn(null); + Player player = mock(Player.class); + AuthMeMockUtil.mockSingletonForClass(ConsoleLogger.class, "wrapper", Wrapper.getInstance()); + + // when + boolean result = Utils.addNormal(player, "test_group"); + + // then + assertThat(result, equalTo(false)); + verify(authMeMock).getPermissionsManager(); + } + + @Test + // Note ljacqu 20151122: This is a heavy test setup with reflections... If it causes trouble, skip it with @Ignore public void shouldRetrieveListOfOnlinePlayersFromReflectedMethod() { // given setField("getOnlinePlayersIsCollection", false); @@ -114,6 +126,7 @@ public class UtilsTest { } } + // Note: This method is used through reflections public static Player[] onlinePlayersImpl() { return new Player[]{ mock(Player.class), mock(Player.class) diff --git a/src/test/java/fr/xephi/authme/WrapperMock.java b/src/test/java/fr/xephi/authme/util/WrapperMock.java similarity index 86% rename from src/test/java/fr/xephi/authme/WrapperMock.java rename to src/test/java/fr/xephi/authme/util/WrapperMock.java index 7812c6d33..1517d929b 100644 --- a/src/test/java/fr/xephi/authme/WrapperMock.java +++ b/src/test/java/fr/xephi/authme/util/WrapperMock.java @@ -1,6 +1,6 @@ -package fr.xephi.authme; +package fr.xephi.authme.util; -import fr.xephi.authme.util.Wrapper; +import fr.xephi.authme.AuthMe; import org.bukkit.Server; import org.bukkit.scheduler.BukkitScheduler; import org.mockito.Mockito; @@ -19,11 +19,7 @@ public class WrapperMock extends Wrapper { private static Map, Object> mocks = new HashMap<>(); public WrapperMock() { - this((AuthMe) getMock(AuthMe.class)); - } - - public WrapperMock(AuthMe authMe) { - super(authMe); + super(); } @Override