Create Wrapper as singleton; fix UtilsTest

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<String> antibotPlayers = new ArrayList<>();
     private static AntiBotStatus antiBotStatus = AntiBotStatus.DISABLED;
 

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() {

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;
 

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)
+        if (permsMan == null) {
-            ConsoleLogger.showError("Failed to access permissions manager instance, shutting down.");
+            ConsoleLogger.showError("Failed to access permissions manager instance, aborting.");
-        assert permsMan != null;
+            return false;
+        }
 
         // Remove old groups
         permsMan.removeGroups(player, Arrays.asList(Settings.unRegisteredGroup,

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() {

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);

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.BDDMockito.given;
-import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.*;
-import static org.mockito.Mockito.when;
 
 /**
  * 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();
-
+        AuthMeMockUtil.mockSingletonForClass(Utils.class, "plugin", authMeMock);
-        // We need to create the Wrapper mock before injecting it into Utils because it runs a lot of  code in
+        permissionsManagerMock = Mockito.mock(PermissionsManager.class);
-        // 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);
         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)

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<Class<?>, Object> mocks = new HashMap<>();
 
     public WrapperMock() {
-        this((AuthMe) getMock(AuthMe.class));
+        super();
-    }
-
-    public WrapperMock(AuthMe authMe) {
-        super(authMe);
     }
 
     @Override