Delay registering flag listeners until plugin is loaded.

Reworked test classes to avoid running the Bukkit server.

src/main/java/us/tastybento/bskyblock/BSkyBlock.java

@@ -67,7 +67,7 @@ public class BSkyBlock extends JavaPlugin {
     @Override
     public void onEnable(){
         // Not loaded
-        setLoaded(false);
+        isLoaded = false;
         // Store the current millis time so we can tell how many ms it took for BSB to fully load.
         final long startMillis = System.currentTimeMillis();
 
@@ -139,14 +139,15 @@ public class BSkyBlock extends JavaPlugin {
                     playersManager.save(true);
                     islandsManager.save(true);
                 }, getSettings().getDatabaseBackupPeriod() * 20 * 60L, getSettings().getDatabaseBackupPeriod() * 20 * 60L);
-
+                isLoaded = true;
+                flagsManager.registerListeners();
                 instance.log("#############################################");
                 instance.log(instance.getDescription().getFullName() + " has been fully enabled.");
                 instance.log("It took: " + (System.currentTimeMillis() - startMillis + "ms"));
                 instance.log("Thanks for using our plugin !");
                 instance.log("- Tastybento and Poslovitch, 2017-2018");
                 instance.log("#############################################");
-                instance.setLoaded(true);
+
             });
         });
     }
@@ -350,20 +351,11 @@ public class BSkyBlock extends JavaPlugin {
 
 
     /**
+     * True if the plugin is loaded and ready
      * @return the isLoaded
      */
     public boolean isLoaded() {
         return isLoaded;
     }
 
-
-
-    /**
-     * @param isLoaded the isLoaded to set
-     */
-    public void setLoaded(boolean isLoaded) {
-        this.isLoaded = isLoaded;
-    }
-
-
 }

src/main/java/us/tastybento/bskyblock/api/commands/CompositeCommand.java

@@ -472,7 +472,6 @@ public abstract class CompositeCommand extends Command implements PluginIdentifi
         }
 
         String lastArg = args.length != 0 ? args[args.length - 1] : "";
-
         return Util.tabLimit(options, lastArg).stream().sorted().collect(Collectors.toList());
     }
 

src/main/java/us/tastybento/bskyblock/api/user/User.java

@@ -319,7 +319,6 @@ public class User {
      * Forces an update of the user's complete inventory.
      * Deprecated, but there is no current alternative.
      */
-    @SuppressWarnings("deprecation")
     public void updateInventory() {
         player.updateInventory();
 

src/main/java/us/tastybento/bskyblock/listeners/flags/CleanSuperFlatListener.java

@@ -24,9 +24,6 @@ public class CleanSuperFlatListener extends AbstractFlagListener {
     @EventHandler(priority = EventPriority.LOW, ignoreCancelled = true)
     public void onChunkLoad(ChunkLoadEvent e) {
         BSkyBlock plugin = BSkyBlock.getInstance();
-        if (!plugin.isLoaded()) {
-            return;
-        }
         World world = e.getWorld();
         if (!e.getChunk().getBlock(0, 0, 0).getType().equals(Material.BEDROCK)
                 || !Flags.CLEAN_SUPER_FLAT.isSetForWorld(world)

src/main/java/us/tastybento/bskyblock/managers/FlagsManager.java

@@ -1,7 +1,9 @@
 package us.tastybento.bskyblock.managers;
 
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 import org.bukkit.Bukkit;
 import org.bukkit.event.Listener;
@@ -21,8 +23,9 @@ public class FlagsManager {
 
     /**
      * Stores the flag listeners that have already been registered into Bukkit's API to avoid duplicates.
+     * Value is true if the listener has been registered already
      */
-    private ArrayList<Listener> registeredListeners = new ArrayList<>();
+    private Map<Listener, Boolean> registeredListeners = new HashMap<>();
 
     public FlagsManager(BSkyBlock plugin) {
         this.plugin = plugin;
@@ -44,16 +47,30 @@ public class FlagsManager {
             }
         }
         flags.add(flag);
-        // If there is a listener which is not already registered, register it into Bukkit.
+        // If there is a listener which is not already registered, register it into Bukkit if the plugin is fully loaded
-        flag.getListener().ifPresent(l -> {
+        flag.getListener().ifPresent(this::registerListener);
-            if (!registeredListeners.contains(l)) {
-                Bukkit.getServer().getPluginManager().registerEvents(l, plugin);
-                registeredListeners.add(l);
-            }
-        });
         return true;
     }
 
+    /**
+     * Register any unregistered listeners - called after the plugin is fully loaded
+     */
+    public void registerListeners() {
+        registeredListeners.entrySet().stream().filter(Map.Entry::getValue).map(Map.Entry::getKey).forEach(this::registerListener);
+    }
+
+    /**
+     * Tries to register a listener if the plugin is loaded
+     * @param l
+     */
+    private void registerListener(Listener l) {
+        registeredListeners.putIfAbsent(l, false);
+        if (BSkyBlock.getInstance().isLoaded() && !registeredListeners.get(l)) {
+            Bukkit.getServer().getPluginManager().registerEvents(l, plugin);
+            registeredListeners.put(l, true);
+        }
+    }
+
     /**
      * @return list of all flags
      */

src/test/java/bskyblock/TestBSkyBlock.java

@@ -38,7 +38,7 @@ import org.bukkit.inventory.ItemFactory;
 import org.bukkit.inventory.meta.SkullMeta;
 import org.bukkit.plugin.PluginManager;
 import org.junit.Assert;
-import org.junit.BeforeClass;
+import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Matchers;
@@ -67,7 +67,7 @@ import us.tastybento.bskyblock.managers.RanksManager;
 import us.tastybento.bskyblock.util.Util;
 
 @RunWith(PowerMockRunner.class)
-@PrepareForTest({ BSkyBlock.class, Flags.class, Util.class})
+@PrepareForTest({ BSkyBlock.class, Flags.class, Util.class, Bukkit.class})
 public class TestBSkyBlock {
     private static final UUID MEMBER_UUID = UUID.randomUUID();
     private static final UUID OWNER_UUID = UUID.randomUUID();
@@ -82,8 +82,8 @@ public class TestBSkyBlock {
     private static Player ownerOfIsland;
     private static Player visitorToIsland;
 
-    @BeforeClass
+    @Before
-    public static void setUpBeforeClass() {
+    public void setUp() {
         // Set up plugin
         plugin = mock(BSkyBlock.class);
         when(plugin.getCommandsManager()).thenReturn(mock(CommandsManager.class));
@@ -101,7 +101,8 @@ public class TestBSkyBlock {
         ItemFactory itemFactory = mock(ItemFactory.class);
         when(server.getItemFactory()).thenReturn(itemFactory);
 
-        Bukkit.setServer(server);
+        PowerMockito.mockStatic(Bukkit.class);
+        when(Bukkit.getServer()).thenReturn(server);
 
         SkullMeta skullMeta = mock(SkullMeta.class);
         when(itemFactory.getItemMeta(any())).thenReturn(skullMeta);
@@ -117,8 +118,9 @@ public class TestBSkyBlock {
         player = mock(Player.class);
         ownerOfIsland = mock(Player.class);
         visitorToIsland = mock(Player.class);
-        Mockito.when(player.hasPermission("default.permission")).thenReturn(true);
+        Mockito.when(player.hasPermission(Mockito.anyString())).thenReturn(true);
-
+        User.getInstance(player);
+        when(Bukkit.getPlayer(Mockito.any(UUID.class))).thenReturn(player);
 
         location = mock(Location.class);
         Mockito.when(location.getWorld()).thenReturn(world);
@@ -136,7 +138,6 @@ public class TestBSkyBlock {
 
         PowerMockito.mockStatic(Flags.class);
 
-        plugin = Mockito.mock(BSkyBlock.class);
         flagsManager = new FlagsManager(plugin);
         Mockito.when(plugin.getFlagsManager()).thenReturn(flagsManager);
 
@@ -151,6 +152,8 @@ public class TestBSkyBlock {
         when(iwm.inWorld(any())).thenReturn(true);
         PowerMockito.mockStatic(Util.class);
         when(Util.getWorld(Mockito.any())).thenReturn(world);
+        // We want the read tabLimit call here
+        when(Util.tabLimit(Mockito.any(), Mockito.anyString())).thenCallRealMethod();
 
         // Islands
         IslandsManager im = mock(IslandsManager.class);
@@ -337,7 +340,6 @@ public class TestBSkyBlock {
 
         @Override
         public boolean execute(User user, List<String> args) {
-            Bukkit.getLogger().info("args are " + args.toString());
 
             return args.size() == 3;
         }
@@ -361,7 +363,6 @@ public class TestBSkyBlock {
 
         @Override
         public boolean execute(User user, List<String> args) {
-            Bukkit.getLogger().info("args are " + args.toString());
             return args.size() == 3;
         }
 
@@ -471,20 +472,16 @@ public class TestBSkyBlock {
         // Test events
 
         FlagListener fl = new FlagListener(plugin);
-        Bukkit.getLogger().info("SETUP: owner UUID = " + ownerOfIsland.getUniqueId());
-        Bukkit.getLogger().info("SETUP: member UUID = " + player.getUniqueId());
-        Bukkit.getLogger().info("SETUP: visitor UUID = " + visitorToIsland.getUniqueId());
 
-        Bukkit.getLogger().info("DEBUG: checking events - vistor");
+        // checking events - vistor
         Event e3 = new BlockBreakEvent(block, visitorToIsland);
         Assert.assertFalse(fl.checkIsland(e3, location, Flags.BREAK_BLOCKS, true));
 
-        Bukkit.getLogger().info("DEBUG: checking events - owner");
+        // checking events - owner
         Event e = new BlockBreakEvent(block, ownerOfIsland);
         Assert.assertTrue(fl.checkIsland(e, location, Flags.BREAK_BLOCKS, true));
 
-        Bukkit.getLogger().info("DEBUG: checking events - member");
+        // checking events - member
-
         Event e2 = new BlockBreakEvent(block, player);
         Assert.assertTrue(fl.checkIsland(e2, location, Flags.BREAK_BLOCKS, true));
 

src/test/java/us/tastybento/bskyblock/listeners/flags/CleanSuperFlatListenerTest.java

@@ -107,17 +107,6 @@ public class CleanSuperFlatListenerTest {
         Mockito.verify(world, Mockito.never()).regenerateChunk(Mockito.anyInt(), Mockito.anyInt());
     }
 
-    /**
-     * Test method for {@link us.tastybento.bskyblock.listeners.flags.CleanSuperFlatListener#onChunkLoad(org.bukkit.event.world.ChunkLoadEvent)}.
-     */
-    @Test
-    public void testOnChunkLoadNotLoaded() {
-        when(plugin.isLoaded()).thenReturn(false);
-        ChunkLoadEvent e = new ChunkLoadEvent(chunk, false);
-        new CleanSuperFlatListener().onChunkLoad(e);
-        Mockito.verify(world, Mockito.never()).regenerateChunk(Mockito.anyInt(), Mockito.anyInt());
-    }
-
     /**
      * Test method for {@link us.tastybento.bskyblock.listeners.flags.CleanSuperFlatListener#onChunkLoad(org.bukkit.event.world.ChunkLoadEvent)}.
      */

src/test/java/us/tastybento/bskyblock/listeners/flags/MobSpawnListenerTest.java

@@ -123,7 +123,17 @@ public class MobSpawnListenerTest {
         Map<String, Boolean> worldFlags = new HashMap<>();
         when(ws.getWorldFlags()).thenReturn(worldFlags);
 
+        // Default - plugin is loaded
+        when(plugin.isLoaded()).thenReturn(true);
+    }
 
+    @Test
+    public void testNotLoaded() {
+        when(plugin.isLoaded()).thenReturn(false);
+        CreatureSpawnEvent e = new CreatureSpawnEvent(null, SpawnReason.NATURAL);
+        MobSpawnListener l = new MobSpawnListener();
+        assertFalse(l.onNaturalMobSpawn(e));
+        assertFalse(e.isCancelled());
     }
 
     @Test

src/test/java/us/tastybento/bskyblock/managers/FlagsManagerTest.java

@@ -18,12 +18,14 @@ import org.bukkit.Bukkit;
 import org.bukkit.Material;
 import org.bukkit.Server;
 import org.bukkit.World;
+import org.bukkit.event.Listener;
 import org.bukkit.inventory.ItemFactory;
 import org.bukkit.inventory.meta.SkullMeta;
 import org.bukkit.plugin.PluginManager;
-import org.junit.BeforeClass;
+import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Mockito;
 import org.powermock.api.mockito.PowerMockito;
 import org.powermock.core.classloader.annotations.PrepareForTest;
 import org.powermock.modules.junit4.PowerMockRunner;
@@ -36,23 +38,27 @@ import us.tastybento.bskyblock.listeners.flags.BreakBlocksListener;
 import us.tastybento.bskyblock.lists.Flags;
 
 @RunWith(PowerMockRunner.class)
-@PrepareForTest( {BSkyBlock.class, Flags.class} )
+@PrepareForTest( {BSkyBlock.class, Flags.class, Bukkit.class} )
 public class FlagsManagerTest {
 
 
     private static BSkyBlock plugin;
+    private static Server server;
 
-    @BeforeClass
+    @Before
-    public static void setUpBeforeClass() throws Exception {
+    public void setUp() throws Exception {
         // Set up plugin
         plugin = mock(BSkyBlock.class);
         Whitebox.setInternalState(BSkyBlock.class, "instance", plugin);
 
+        // Plugin is loaded
+        when(plugin.isLoaded()).thenReturn(true);
+
         IslandsManager im = mock(IslandsManager.class);
         when(plugin.getIslands()).thenReturn(im);
 
 
-        Server server = mock(Server.class);
+        server = mock(Server.class);
         World world = mock(World.class);
         when(server.getLogger()).thenReturn(Logger.getAnonymousLogger());
         when(server.getWorld("world")).thenReturn(world);
@@ -64,7 +70,8 @@ public class FlagsManagerTest {
         ItemFactory itemFactory = mock(ItemFactory.class);
         when(server.getItemFactory()).thenReturn(itemFactory);
 
-        Bukkit.setServer(server);
+        PowerMockito.mockStatic(Bukkit.class);
+        when(Bukkit.getServer()).thenReturn(server);
 
         SkullMeta skullMeta = mock(SkullMeta.class);
         when(itemFactory.getItemMeta(any())).thenReturn(skullMeta);
@@ -80,10 +87,15 @@ public class FlagsManagerTest {
     }
 
     @Test
-    public void testRegisterFlag() {
+    public void testRegisterDuplicateFlag() {
         FlagsManager fm = new FlagsManager(plugin);
         // Try to register every single flag - it should fail every time
         Flags.values().forEach(dupe -> assertFalse(fm.registerFlag(dupe)));
+    }
+
+    @Test
+    public void testRegisterDuplicateFlagIcons() {
+        FlagsManager fm = new FlagsManager(plugin);
         // Change the ID to something random, but use every icon that is already used
         Flags.values().forEach(dupe -> {
             assertFalse(fm.registerFlag(new FlagBuilder()
@@ -92,11 +104,42 @@ public class FlagsManagerTest {
                     .listener(new BreakBlocksListener())
                     .build()));
         });
+    }
+
+    @Test
+    public void testRegisteroriginalFlagPluginNotLoaded() {
+        when(plugin.isLoaded()).thenReturn(false);
+        FlagsManager fm = new FlagsManager(plugin);
         // This should pass
         Flag originalFlag = new FlagBuilder().id("ORIGINAL").icon(Material.EMERALD_BLOCK).listener(new BreakBlocksListener()).build();
         assertTrue(fm.registerFlag(originalFlag));
+        // Verify no Bukkit listener registered
+        Mockito.verify(server, Mockito.never()).getPluginManager();
     }
 
+
+    @Test
+    public void testRegisteroriginalFlagPluginLoadedOriginalListener() {
+        when(plugin.isLoaded()).thenReturn(true);
+        FlagsManager fm = new FlagsManager(plugin);
+        // This should pass
+        OriginalListener ol = new OriginalListener();
+        Flag originalFlag = new FlagBuilder().id("ORIGINAL").icon(Material.EMERALD_BLOCK).listener(ol).build();
+        assertTrue(fm.registerFlag(originalFlag));
+        // Verify registered
+        Mockito.verify(server).getPluginManager();
+        // Register another flag with same listener
+        Flag originalFlag2 = new FlagBuilder().id("ORIGINAL2").icon(Material.COAL_ORE).listener(ol).build();
+        assertTrue(fm.registerFlag(originalFlag2));
+        // Verify registered only once
+        Mockito.verify(server).getPluginManager();
+    }
+
+    class OriginalListener implements Listener {
+        // nothing here
+    }
+
+
     @Test
     public void testGetFlags() {
         FlagsManager fm = new FlagsManager(plugin);
@@ -111,4 +154,6 @@ public class FlagsManagerTest {
         Flags.values().stream().sorted(Comparator.reverseOrder()).forEach(flag -> assertEquals(flag, fm.getFlagByID(flag.getID())));
 
     }
+
+
 }