Fixes tests. Also fixes a bug.

https://github.com/BentoBoxWorld/bentobox/issues/258

src/main/java/world/bentobox/bentobox/database/objects/Island.java

@@ -190,6 +190,7 @@ public class Island implements DataObject {
     }
 
     /**
+     * Members > MEMBER_RANK
      * @return the members of the island (owner included)
      */
     public ImmutableSet<UUID> getMemberSet(){

src/main/java/world/bentobox/bentobox/managers/IslandsManager.java

@@ -285,7 +285,9 @@ public class IslandsManager {
 
     /**
      * Returns the island at the location or Optional empty if there is none.
-     * This includes the full island space, not just the protected area
+     * This includes the full island space, not just the protected area.
+     * Use {@link #getProtectedIslandAt(Location)} for only the protected
+     * island space.
      *
      * @param location - the location
      * @return Optional Island object
@@ -334,7 +336,9 @@ public class IslandsManager {
     }
 
     /**
-     * Returns the island being public at the location or Optional Empty if there is none
+     * Returns the island at the location or Optional empty if there is none.
+     * This includes only the protected area. Use {@link #getIslandAt(Location)}
+     * for the full island space.
      *
      * @param location - the location
      * @return Optional Island object
@@ -631,7 +635,11 @@ public class IslandsManager {
         if (user == null) {
             return false;
         }
-        return Optional.ofNullable(getIsland(world, user)).map(i -> i.onIsland(user.getLocation())).orElse(false);
+        return getProtectedIslandAt(user.getLocation())
+                .map(i -> i.getMembers().entrySet().stream()
+                        .map(en -> en.getKey().equals(user.getUniqueId()) && en.getValue() > RanksManager.VISITOR_RANK)
+                        .findAny().orElse(false))
+                .orElse(false);
     }
 
     /**

src/main/java/world/bentobox/bentobox/panels/SettingsPanel.java

@@ -4,7 +4,6 @@ import java.util.Comparator;
 
 import org.bukkit.Material;
 import org.bukkit.World;
-import org.bukkit.inventory.ItemStack;
 
 import world.bentobox.bentobox.BentoBox;
 import world.bentobox.bentobox.api.flags.Flag;

src/test/java/world/bentobox/bentobox/listeners/flags/PVPListenerTest.java

@@ -64,6 +64,7 @@ import world.bentobox.bentobox.api.configuration.WorldSettings;
 import world.bentobox.bentobox.api.flags.Flag;
 import world.bentobox.bentobox.api.panels.Panel;
 import world.bentobox.bentobox.api.panels.PanelItem;
+import world.bentobox.bentobox.api.user.Notifier;
 import world.bentobox.bentobox.api.user.User;
 import world.bentobox.bentobox.database.objects.Island;
 import world.bentobox.bentobox.lists.Flags;
@@ -91,6 +92,7 @@ public class PVPListenerTest {
     private Zombie zombie;
     private Creeper creeper;
     private World world;
+    private Notifier notifier;
 
     /**
      * @throws java.lang.Exception
@@ -100,6 +102,8 @@ public class PVPListenerTest {
         // Set up plugin
         BentoBox plugin = mock(BentoBox.class);
         Whitebox.setInternalState(BentoBox.class, "instance", plugin);
+        // Make sure you set the plung for the User class otherwise it'll use an old object
+        User.setPlugin(plugin);
         // Island World Manager
         iwm = mock(IslandWorldManager.class);
         when(iwm.inWorld(Mockito.any())).thenReturn(true);
@@ -184,6 +188,10 @@ public class PVPListenerTest {
         when(iwm.getWorldSettings(Mockito.any())).thenReturn(ws);
         Map<String, Boolean> worldFlags = new HashMap<>();
         when(ws.getWorldFlags()).thenReturn(worldFlags);
+
+        // Notifier
+        notifier = mock(Notifier.class);
+        when(plugin.getNotifier()).thenReturn(notifier);
     }
 
     /**
@@ -387,7 +395,6 @@ public class PVPListenerTest {
      */
     @Test
     public void testOnEntityDamagePVPNotAllowed() {
-
         // No visitor protection
         EntityDamageByEntityEvent e = new EntityDamageByEntityEvent(player, player2, EntityDamageEvent.DamageCause.ENTITY_ATTACK,
                 new EnumMap<>(ImmutableMap.of(DamageModifier.BASE, 0D)),
@@ -395,7 +402,18 @@ public class PVPListenerTest {
         new PVPListener().onEntityDamage(e);
         // PVP should be banned
         assertTrue(e.isCancelled());
-        Mockito.verify(player).sendMessage(Flags.PVP_OVERWORLD.getHintReference());
+        Mockito.verify(notifier).notify(Mockito.any(), Mockito.eq(Flags.PVP_OVERWORLD.getHintReference()));
+
+    }
+
+    /**
+     * Test method for {@link world.bentobox.bentobox.listeners.flags.PVPListener#onEntityDamage(org.bukkit.event.entity.EntityDamageByEntityEvent)}.
+     */
+    @Test
+    public void testOnEntityDamagePVPNotAllowedInvVisitor() {
+        EntityDamageByEntityEvent e = new EntityDamageByEntityEvent(player, player2, EntityDamageEvent.DamageCause.ENTITY_ATTACK,
+                new EnumMap<>(ImmutableMap.of(DamageModifier.BASE, 0D)),
+                new EnumMap<DamageModifier, Function<? super Double, Double>>(ImmutableMap.of(DamageModifier.BASE, Functions.constant(-0.0))));
 
         // Enable visitor protection
         // This player is a visitor and any damage is not allowed
@@ -404,7 +422,7 @@ public class PVPListenerTest {
         new PVPListener().onEntityDamage(e);
         // visitor should be protected
         assertTrue(e.isCancelled());
-        Mockito.verify(player).sendMessage(Flags.INVINCIBLE_VISITORS.getHintReference());
+        Mockito.verify(notifier).notify(Mockito.any(), Mockito.eq(Flags.INVINCIBLE_VISITORS.getHintReference()));
     }
 
     /**
@@ -429,7 +447,7 @@ public class PVPListenerTest {
         new PVPListener().onEntityDamage(e);
         // visitor should be protected
         assertTrue(e.isCancelled());
-        Mockito.verify(player).sendMessage(Flags.INVINCIBLE_VISITORS.getHintReference());
+        Mockito.verify(notifier).notify(Mockito.any(), Mockito.eq(Flags.INVINCIBLE_VISITORS.getHintReference()));
 
     }
 
@@ -447,7 +465,7 @@ public class PVPListenerTest {
         new PVPListener().onEntityDamage(e);
         // PVP should be banned
         assertTrue(e.isCancelled());
-        Mockito.verify(player).sendMessage(Flags.PVP_OVERWORLD.getHintReference());
+        Mockito.verify(notifier).notify(Mockito.any(), Mockito.eq(Flags.PVP_OVERWORLD.getHintReference()));
 
         // Visitor protection
         // This player is a visitor and any damage is not allowed
@@ -457,7 +475,7 @@ public class PVPListenerTest {
         // visitor should be protected
         assertTrue(e.isCancelled());
         // PVP trumps visitor protection
-        Mockito.verify(player).sendMessage(Flags.PVP_OVERWORLD.getHintReference());
+        Mockito.verify(notifier).notify(Mockito.any(), Mockito.eq(Flags.PVP_OVERWORLD.getHintReference()));
 
     }
 
@@ -502,7 +520,7 @@ public class PVPListenerTest {
         new PVPListener().onEntityDamage(e);
         // visitor should be protected
         assertTrue(e.isCancelled());
-        Mockito.verify(player).sendMessage(Flags.INVINCIBLE_VISITORS.getHintReference());
+        Mockito.verify(notifier).notify(Mockito.any(), Mockito.eq(Flags.INVINCIBLE_VISITORS.getHintReference()));
 
     }
 
@@ -525,7 +543,7 @@ public class PVPListenerTest {
 
         // PVP should be banned
         assertTrue(pfe.isCancelled());
-        Mockito.verify(player).sendMessage(Flags.PVP_OVERWORLD.getHintReference());
+        Mockito.verify(notifier).notify(Mockito.any(), Mockito.eq(Flags.PVP_OVERWORLD.getHintReference()));
         // Hook should be removed
         Mockito.verify(hook).remove();
 
@@ -571,7 +589,7 @@ public class PVPListenerTest {
         new PVPListener().onFishing(pfe);
         // visitor should be protected
         assertTrue(pfe.isCancelled());
-        Mockito.verify(player).sendMessage(Flags.INVINCIBLE_VISITORS.getHintReference());
+        Mockito.verify(notifier).notify(Mockito.any(), Mockito.eq(Flags.INVINCIBLE_VISITORS.getHintReference()));
     }
 
     /**
@@ -607,7 +625,7 @@ public class PVPListenerTest {
         new PVPListener().onFishing(pfe);
         // visitor should be protected
         assertTrue(pfe.isCancelled());
-        Mockito.verify(player).sendMessage(Flags.INVINCIBLE_VISITORS.getHintReference());
+        Mockito.verify(notifier).notify(Mockito.any(), Mockito.eq(Flags.INVINCIBLE_VISITORS.getHintReference()));
     }
 
     /**
@@ -659,7 +677,7 @@ public class PVPListenerTest {
         PotionSplashEvent e = new PotionSplashEvent(tp, map);
         new PVPListener().onSplashPotionSplash(e);
         assertTrue(e.isCancelled());
-        Mockito.verify(player).sendMessage(Flags.PVP_OVERWORLD.getHintReference());
+        Mockito.verify(notifier).notify(Mockito.any(), Mockito.eq(Flags.PVP_OVERWORLD.getHintReference()));
 
         // Wrong world
         when(iwm.inWorld(Mockito.any())).thenReturn(false);
@@ -742,7 +760,7 @@ public class PVPListenerTest {
         new PVPListener().onSplashPotionSplash(e);
         // visitor should be protected
         assertTrue(e.isCancelled());
-        Mockito.verify(player).sendMessage(Flags.INVINCIBLE_VISITORS.getHintReference());
+        Mockito.verify(notifier).notify(Mockito.any(), Mockito.eq(Flags.INVINCIBLE_VISITORS.getHintReference()));
 
         // Wrong world
         when(iwm.inWorld(Mockito.any())).thenReturn(false);
@@ -813,14 +831,14 @@ public class PVPListenerTest {
         listener.onLingeringPotionDamage(ae);
         assertEquals(3, ae.getAffectedEntities().size());
         assertFalse(ae.getAffectedEntities().contains(player2));
-        Mockito.verify(player).sendMessage(Flags.PVP_OVERWORLD.getHintReference());
+        Mockito.verify(notifier).notify(Mockito.any(), Mockito.eq(Flags.PVP_OVERWORLD.getHintReference()));
         // Wrong world
         when(iwm.inWorld(Mockito.any())).thenReturn(false);
         listener.onLingeringPotionSplash(e);
         // No change to results
         assertEquals(3, ae.getAffectedEntities().size());
         assertFalse(ae.getAffectedEntities().contains(player2));
-        Mockito.verify(player).sendMessage(Flags.PVP_OVERWORLD.getHintReference());
+        Mockito.verify(notifier).notify(Mockito.any(), Mockito.eq(Flags.PVP_OVERWORLD.getHintReference()));
     }
 
     /**
@@ -888,13 +906,13 @@ public class PVPListenerTest {
         listener.onLingeringPotionDamage(ae);
         assertEquals(3, ae.getAffectedEntities().size());
         assertFalse(ae.getAffectedEntities().contains(player2));
-        Mockito.verify(player).sendMessage(Flags.INVINCIBLE_VISITORS.getHintReference());
+        Mockito.verify(notifier).notify(Mockito.any(), Mockito.eq(Flags.INVINCIBLE_VISITORS.getHintReference()));
         // Wrong world
         when(iwm.inWorld(Mockito.any())).thenReturn(false);
         listener.onLingeringPotionSplash(e);
         assertEquals(3, ae.getAffectedEntities().size());
         assertFalse(ae.getAffectedEntities().contains(player2));
-        Mockito.verify(player).sendMessage(Flags.INVINCIBLE_VISITORS.getHintReference());
+        Mockito.verify(notifier).notify(Mockito.any(), Mockito.eq(Flags.INVINCIBLE_VISITORS.getHintReference()));
     }
 
     /**
@@ -928,12 +946,12 @@ public class PVPListenerTest {
         listener.onLingeringPotionDamage(ae);
         assertEquals(3, ae.getAffectedEntities().size());
         assertFalse(ae.getAffectedEntities().contains(player2));
-        Mockito.verify(player).sendMessage(Flags.INVINCIBLE_VISITORS.getHintReference());
+        Mockito.verify(notifier).notify(Mockito.any(), Mockito.eq(Flags.INVINCIBLE_VISITORS.getHintReference()));
         // Wrong world
         when(iwm.inWorld(Mockito.any())).thenReturn(false);
         listener.onLingeringPotionSplash(e);
         assertEquals(3, ae.getAffectedEntities().size());
         assertFalse(ae.getAffectedEntities().contains(player2));
-        Mockito.verify(player).sendMessage(Flags.INVINCIBLE_VISITORS.getHintReference());
+        Mockito.verify(notifier).notify(Mockito.any(), Mockito.eq(Flags.INVINCIBLE_VISITORS.getHintReference()));
     }
 }

src/test/java/world/bentobox/bentobox/managers/IslandsManagerTest.java

@@ -78,6 +78,7 @@ public class IslandsManagerTest {
     private IslandWorldManager iwm;
     private IslandCache islandCache;
     private Optional<Island> optionalIsland;
+    private Island is;
 
     /**
      * @throws java.lang.Exception
@@ -163,7 +164,7 @@ public class IslandsManagerTest {
 
         // Mock island cache
         islandCache = mock(IslandCache.class);
-        Island is = mock(Island.class);
+        is = mock(Island.class);
         when(islandCache.getIslandAt(Mockito.any(Location.class))).thenReturn(is);
         optionalIsland = Optional.ofNullable(is);
 
@@ -704,26 +705,30 @@ public class IslandsManagerTest {
     }
 
     /**
-     * Test method for .
+     * Test method for user is on island
      * @throws Exception
      */
     @Test
     public void testUserIsOnIsland() throws Exception {
-        // Mock island cache
-        Island is = mock(Island.class);
-
-        when(islandCache.get(Mockito.any(), Mockito.any(UUID.class))).thenReturn(is);
-
         IslandsManager im = new IslandsManager(plugin);
         im.setIslandCache(islandCache);
 
+        // Null user
         assertFalse(im.userIsOnIsland(world, null));
 
-        when(is.onIsland(Mockito.any())).thenReturn(false);
+
+        // User is on island is determined by whether the user's location is on
+        // an island that has them as a memebr (rank > 0)
+        when(is.onIsland(Mockito.any())).thenReturn(true);
+        Map<UUID, Integer> members = new HashMap<>();
+        when(is.getMembers()).thenReturn(members);
         assertFalse(im.userIsOnIsland(world, user));
 
-        when(is.onIsland(Mockito.any())).thenReturn(true);
+        members.put(user.getUniqueId(), RanksManager.MEMBER_RANK);
         assertTrue(im.userIsOnIsland(world, user));
+
+        members.put(user.getUniqueId(), RanksManager.BANNED_RANK);
+        assertFalse(im.userIsOnIsland(world, user));
     }
 
     /**