From e75cff5fb8bdbbda226d0decd2f2b7cf5edab092 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Thu, 2 Jun 2016 12:46:54 +0200 Subject: [PATCH] Remove static injections in ListenerService - Get other classes via Inject annotation - Remove no longer needed Utils methods (relates to #736) - Create tests for ListenerService and AuthMeBlockListener - Performance improvement: keep unrestricted names as Set instead of List for faster contains() method --- src/main/java/fr/xephi/authme/api/API.java | 2 +- .../authme/listener/AuthMeBlockListener.java | 9 +- .../authme/listener/AuthMeEntityListener.java | 26 +- .../authme/listener/AuthMePlayerListener.java | 42 +-- .../listener/AuthMePlayerListener16.java | 7 +- .../listener/AuthMePlayerListener18.java | 7 +- .../authme/listener/ListenerService.java | 93 +++++-- src/main/java/fr/xephi/authme/util/Utils.java | 24 +- .../listener/AuthMeBlockListenerTest.java | 95 +++++++ .../authme/listener/ListenerServiceTest.java | 254 ++++++++++++++++++ .../authme/listener/OnJoinVerifierTest.java | 1 - .../authme/settings/SpawnLoaderTest.java | 3 - 12 files changed, 480 insertions(+), 83 deletions(-) create mode 100644 src/test/java/fr/xephi/authme/listener/AuthMeBlockListenerTest.java create mode 100644 src/test/java/fr/xephi/authme/listener/ListenerServiceTest.java diff --git a/src/main/java/fr/xephi/authme/api/API.java b/src/main/java/fr/xephi/authme/api/API.java index 26f5ea8f7..ca75cee11 100644 --- a/src/main/java/fr/xephi/authme/api/API.java +++ b/src/main/java/fr/xephi/authme/api/API.java @@ -179,7 +179,7 @@ public class API { */ @Deprecated public boolean isNPC(Player player) { - return Utils.isNPC(player); + return instance.getPluginHooks().isNpc(player); } } diff --git a/src/main/java/fr/xephi/authme/listener/AuthMeBlockListener.java b/src/main/java/fr/xephi/authme/listener/AuthMeBlockListener.java index 9ffdd4831..7fe23dd96 100644 --- a/src/main/java/fr/xephi/authme/listener/AuthMeBlockListener.java +++ b/src/main/java/fr/xephi/authme/listener/AuthMeBlockListener.java @@ -5,18 +5,23 @@ import org.bukkit.event.Listener; import org.bukkit.event.block.BlockBreakEvent; import org.bukkit.event.block.BlockPlaceEvent; +import javax.inject.Inject; + public class AuthMeBlockListener implements Listener { + @Inject + private ListenerService listenerService; + @EventHandler(ignoreCancelled = true) public void onBlockPlace(BlockPlaceEvent event) { - if (ListenerService.shouldCancelEvent(event.getPlayer())) { + if (listenerService.shouldCancelEvent(event.getPlayer())) { event.setCancelled(true); } } @EventHandler(ignoreCancelled = true) public void onBlockBreak(BlockBreakEvent event) { - if (ListenerService.shouldCancelEvent(event.getPlayer())) { + if (listenerService.shouldCancelEvent(event.getPlayer())) { event.setCancelled(true); } } diff --git a/src/main/java/fr/xephi/authme/listener/AuthMeEntityListener.java b/src/main/java/fr/xephi/authme/listener/AuthMeEntityListener.java index 616416798..ce8afa408 100644 --- a/src/main/java/fr/xephi/authme/listener/AuthMeEntityListener.java +++ b/src/main/java/fr/xephi/authme/listener/AuthMeEntityListener.java @@ -16,17 +16,19 @@ import org.bukkit.event.entity.EntityTargetEvent; import org.bukkit.event.entity.FoodLevelChangeEvent; import org.bukkit.event.entity.ProjectileLaunchEvent; +import javax.inject.Inject; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import static fr.xephi.authme.listener.ListenerService.shouldCancelEvent; - public class AuthMeEntityListener implements Listener { + private final ListenerService listenerService; private Method getShooter; private boolean shooterIsLivingEntity; - public AuthMeEntityListener() { + @Inject + AuthMeEntityListener(ListenerService listenerService) { + this.listenerService = listenerService; try { getShooter = Projectile.class.getDeclaredMethod("getShooter"); shooterIsLivingEntity = getShooter.getReturnType() == LivingEntity.class; @@ -38,7 +40,7 @@ public class AuthMeEntityListener implements Listener { // Note #360: npc status can be used to bypass security!!! @EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST) public void onEntityDamage(EntityDamageEvent event) { - if (shouldCancelEvent(event)) { + if (listenerService.shouldCancelEvent(event)) { event.getEntity().setFireTicks(0); event.setDamage(0); event.setCancelled(true); @@ -47,7 +49,7 @@ public class AuthMeEntityListener implements Listener { @EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST) public void onEntityTarget(EntityTargetEvent event) { - if (shouldCancelEvent(event)) { + if (listenerService.shouldCancelEvent(event)) { event.setTarget(null); event.setCancelled(true); } @@ -55,21 +57,21 @@ public class AuthMeEntityListener implements Listener { @EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST) public void onDamage(EntityDamageByEntityEvent event) { - if (shouldCancelEvent(event)) { + if (listenerService.shouldCancelEvent(event)) { event.setCancelled(true); } } @EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST) public void onFoodLevelChange(FoodLevelChangeEvent event) { - if (shouldCancelEvent(event)) { + if (listenerService.shouldCancelEvent(event)) { event.setCancelled(true); } } @EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST) public void entityRegainHealthEvent(EntityRegainHealthEvent event) { - if (shouldCancelEvent(event)) { + if (listenerService.shouldCancelEvent(event)) { event.setAmount(0); event.setCancelled(true); } @@ -77,14 +79,14 @@ public class AuthMeEntityListener implements Listener { @EventHandler(ignoreCancelled = true, priority = EventPriority.HIGHEST) public void onEntityInteract(EntityInteractEvent event) { - if (shouldCancelEvent(event)) { + if (listenerService.shouldCancelEvent(event)) { event.setCancelled(true); } } @EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST) public void onLowestEntityInteract(EntityInteractEvent event) { - if (shouldCancelEvent(event)) { + if (listenerService.shouldCancelEvent(event)) { event.setCancelled(true); } } @@ -111,14 +113,14 @@ public class AuthMeEntityListener implements Listener { } else { shooterRaw = projectile.getShooter(); } - if (shooterRaw instanceof Player && shouldCancelEvent((Player) shooterRaw)) { + if (shooterRaw instanceof Player && listenerService.shouldCancelEvent((Player) shooterRaw)) { event.setCancelled(true); } } @EventHandler(ignoreCancelled = true, priority = EventPriority.NORMAL) public void onShoot(EntityShootBowEvent event) { - if (shouldCancelEvent(event)) { + if (listenerService.shouldCancelEvent(event)) { event.setCancelled(true); } } diff --git a/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java b/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java index a974421a5..67e57ee8b 100644 --- a/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java +++ b/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java @@ -45,7 +45,6 @@ import java.util.Iterator; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; -import static fr.xephi.authme.listener.ListenerService.shouldCancelEvent; import static fr.xephi.authme.settings.properties.RestrictionSettings.ALLOWED_MOVEMENT_RADIUS; import static fr.xephi.authme.settings.properties.RestrictionSettings.ALLOW_UNAUTHED_MOVEMENT; @@ -72,6 +71,8 @@ public class AuthMePlayerListener implements Listener { private SpawnLoader spawnLoader; @Inject private OnJoinVerifier onJoinVerifier; + @Inject + private ListenerService listenerService; @EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST) public void onPlayerCommandPreprocess(PlayerCommandPreprocessEvent event) { @@ -83,11 +84,10 @@ public class AuthMePlayerListener implements Listener { return; } final Player player = event.getPlayer(); - if (!shouldCancelEvent(player)) { - return; + if (listenerService.shouldCancelEvent(player)) { + event.setCancelled(true); + m.send(player, MessageKey.DENIED_COMMAND); } - event.setCancelled(true); - m.send(player, MessageKey.DENIED_COMMAND); } @EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST) @@ -97,7 +97,7 @@ public class AuthMePlayerListener implements Listener { } final Player player = event.getPlayer(); - if (shouldCancelEvent(player)) { + if (listenerService.shouldCancelEvent(player)) { event.setCancelled(true); m.send(player, MessageKey.DENIED_CHAT); } else if (settings.getProperty(RestrictionSettings.HIDE_CHAT)) { @@ -105,7 +105,7 @@ public class AuthMePlayerListener implements Listener { Iterator iter = recipients.iterator(); while (iter.hasNext()) { Player p = iter.next(); - if (shouldCancelEvent(p)) { + if (listenerService.shouldCancelEvent(p)) { iter.remove(); } } @@ -134,7 +134,7 @@ public class AuthMePlayerListener implements Listener { } Player player = event.getPlayer(); - if (!shouldCancelEvent(player)) { + if (!listenerService.shouldCancelEvent(player)) { return; } @@ -280,21 +280,21 @@ public class AuthMePlayerListener implements Listener { @EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST) public void onPlayerPickupItem(PlayerPickupItemEvent event) { - if (shouldCancelEvent(event)) { + if (listenerService.shouldCancelEvent(event)) { event.setCancelled(true); } } @EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST) public void onPlayerInteract(PlayerInteractEvent event) { - if (shouldCancelEvent(event)) { + if (listenerService.shouldCancelEvent(event)) { event.setCancelled(true); } } @EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST) public void onPlayerConsumeItem(PlayerItemConsumeEvent event) { - if (shouldCancelEvent(event)) { + if (listenerService.shouldCancelEvent(event)) { event.setCancelled(true); } } @@ -303,7 +303,7 @@ public class AuthMePlayerListener implements Listener { public void onPlayerInventoryOpen(InventoryOpenEvent event) { final Player player = (Player) event.getPlayer(); - if (!shouldCancelEvent(player)) { + if (!listenerService.shouldCancelEvent(player)) { return; } event.setCancelled(true); @@ -329,7 +329,7 @@ public class AuthMePlayerListener implements Listener { return; } Player player = (Player) event.getWhoClicked(); - if (!shouldCancelEvent(player)) { + if (!listenerService.shouldCancelEvent(player)) { return; } event.setCancelled(true); @@ -337,28 +337,28 @@ public class AuthMePlayerListener implements Listener { @EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST) public void onPlayerHitPlayerEvent(EntityDamageByEntityEvent event) { - if (shouldCancelEvent(event)) { + if (listenerService.shouldCancelEvent(event)) { event.setCancelled(true); } } @EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST) public void onPlayerInteractEntity(PlayerInteractEntityEvent event) { - if (shouldCancelEvent(event)) { + if (listenerService.shouldCancelEvent(event)) { event.setCancelled(true); } } @EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST) public void onPlayerDropItem(PlayerDropItemEvent event) { - if (shouldCancelEvent(event)) { + if (listenerService.shouldCancelEvent(event)) { event.setCancelled(true); } } @EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST) public void onPlayerBedEnter(PlayerBedEnterEvent event) { - if (shouldCancelEvent(event)) { + if (listenerService.shouldCancelEvent(event)) { event.setCancelled(true); } } @@ -366,7 +366,7 @@ public class AuthMePlayerListener implements Listener { @EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST) public void onSignChange(SignChangeEvent event) { Player player = event.getPlayer(); - if (shouldCancelEvent(player)) { + if (listenerService.shouldCancelEvent(player)) { event.setCancelled(true); } } @@ -377,7 +377,7 @@ public class AuthMePlayerListener implements Listener { if (settings.getProperty(RestrictionSettings.NO_TELEPORT)) { return; } - if (!shouldCancelEvent(event)) { + if (!listenerService.shouldCancelEvent(event)) { return; } Player player = event.getPlayer(); @@ -398,14 +398,14 @@ public class AuthMePlayerListener implements Listener { @EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST) public void onPlayerShear(PlayerShearEntityEvent event) { - if (shouldCancelEvent(event)) { + if (listenerService.shouldCancelEvent(event)) { event.setCancelled(true); } } @EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST) public void onPlayerFish(PlayerFishEvent event) { - if (shouldCancelEvent(event)) { + if (listenerService.shouldCancelEvent(event)) { event.setCancelled(true); } } diff --git a/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener16.java b/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener16.java index ee0e581c1..871757f4e 100644 --- a/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener16.java +++ b/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener16.java @@ -5,14 +5,19 @@ import org.bukkit.event.EventPriority; import org.bukkit.event.Listener; import org.bukkit.event.player.PlayerEditBookEvent; +import javax.inject.Inject; + /** * Listener of player events for events introduced in Minecraft 1.6. */ public class AuthMePlayerListener16 implements Listener { + @Inject + private ListenerService listenerService; + @EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST) public void onPlayerEditBook(PlayerEditBookEvent event) { - if (ListenerService.shouldCancelEvent(event)) { + if (listenerService.shouldCancelEvent(event)) { event.setCancelled(true); } } diff --git a/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener18.java b/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener18.java index 560f6e8bd..b6cbf2a7c 100644 --- a/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener18.java +++ b/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener18.java @@ -5,14 +5,19 @@ import org.bukkit.event.EventPriority; import org.bukkit.event.Listener; import org.bukkit.event.player.PlayerInteractAtEntityEvent; +import javax.inject.Inject; + /** * Listener of player events for events introduced in Minecraft 1.8. */ public class AuthMePlayerListener18 implements Listener { + @Inject + private ListenerService listenerService; + @EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST) public void onPlayerInteractAtEntity(PlayerInteractAtEntityEvent event) { - if (ListenerService.shouldCancelEvent(event)) { + if (listenerService.shouldCancelEvent(event)) { event.setCancelled(true); } } diff --git a/src/main/java/fr/xephi/authme/listener/ListenerService.java b/src/main/java/fr/xephi/authme/listener/ListenerService.java index 449dec67c..c5de1bc3b 100644 --- a/src/main/java/fr/xephi/authme/listener/ListenerService.java +++ b/src/main/java/fr/xephi/authme/listener/ListenerService.java @@ -1,26 +1,48 @@ package fr.xephi.authme.listener; -import fr.xephi.authme.util.Utils; +import fr.xephi.authme.cache.auth.PlayerCache; +import fr.xephi.authme.datasource.DataSource; +import fr.xephi.authme.hooks.PluginHooks; +import fr.xephi.authme.initialization.SettingsDependent; +import fr.xephi.authme.settings.NewSetting; +import fr.xephi.authme.settings.properties.RegistrationSettings; +import fr.xephi.authme.settings.properties.RestrictionSettings; import org.bukkit.entity.Entity; import org.bukkit.entity.Player; import org.bukkit.event.entity.EntityEvent; import org.bukkit.event.player.PlayerEvent; -/** - * Service class for the AuthMe listeners. - */ -final class ListenerService { +import javax.inject.Inject; +import java.util.HashSet; +import java.util.Set; - private ListenerService() { +/** + * Service class for the AuthMe listeners to determine whether an event should be canceled. + */ +class ListenerService implements SettingsDependent { + + private final DataSource dataSource; + private final PluginHooks pluginHooks; + private final PlayerCache playerCache; + + private boolean isRegistrationForced; + private Set unrestrictedNames; + + @Inject + ListenerService(NewSetting settings, DataSource dataSource, PluginHooks pluginHooks, PlayerCache playerCache) { + this.dataSource = dataSource; + this.pluginHooks = pluginHooks; + this.playerCache = playerCache; + loadSettings(settings); } /** - * Return whether an event should be canceled (for unauthenticated, non-NPC players). + * Returns whether an event should be canceled (for unauthenticated, non-NPC players). * - * @param event The event to process - * @return True if the event should be canceled, false otherwise + * @param event the event to process + * @return true if the event should be canceled, false otherwise */ - public static boolean shouldCancelEvent(EntityEvent event) { + public boolean shouldCancelEvent(EntityEvent event) { Entity entity = event.getEntity(); if (entity == null || !(entity instanceof Player)) { return false; @@ -31,24 +53,57 @@ final class ListenerService { } /** - * Return whether an event should be canceled (for unauthenticated, non-NPC players). + * Returns whether an event should be canceled (for unauthenticated, non-NPC players). * - * @param event The event to process - * @return True if the event should be canceled, false otherwise + * @param event the event to process + * @return true if the event should be canceled, false otherwise */ - public static boolean shouldCancelEvent(PlayerEvent event) { + public boolean shouldCancelEvent(PlayerEvent event) { Player player = event.getPlayer(); return shouldCancelEvent(player); } /** - * Return, based on the player associated with the event, whether or not the event should be canceled. + * Returns, based on the player associated with the event, whether or not the event should be canceled. * - * @param player The player to verify - * @return True if the associated event should be canceled, false otherwise + * @param player the player to verify + * @return true if the associated event should be canceled, false otherwise */ - public static boolean shouldCancelEvent(Player player) { - return player != null && !Utils.checkAuth(player) && !Utils.isNPC(player); + public boolean shouldCancelEvent(Player player) { + return player != null && !checkAuth(player.getName()) && !pluginHooks.isNpc(player); } + @Override + public void loadSettings(NewSetting settings) { + isRegistrationForced = settings.getProperty(RegistrationSettings.FORCE); + // Keep unrestricted names as Set for more efficient contains() + unrestrictedNames = new HashSet<>(settings.getProperty(RestrictionSettings.UNRESTRICTED_NAMES)); + } + + /** + * Checks whether the player is allowed to perform actions (i.e. whether he is logged in + * or if other settings permit playing). + * + * @param name the name of the player to verify + * @return true if the player may play, false otherwise + */ + private boolean checkAuth(String name) { + if (isUnrestricted(name) || playerCache.isAuthenticated(name)) { + return true; + } + if (!isRegistrationForced && !dataSource.isAuthAvailable(name)) { + return true; + } + return false; + } + + /** + * Checks if the name is unrestricted according to the configured settings. + * + * @param name the name to verify + * @return true if unrestricted, false otherwise + */ + private boolean isUnrestricted(String name) { + return unrestrictedNames.contains(name.toLowerCase()); + } } diff --git a/src/main/java/fr/xephi/authme/util/Utils.java b/src/main/java/fr/xephi/authme/util/Utils.java index 5d8c08eea..32cbeb9ab 100644 --- a/src/main/java/fr/xephi/authme/util/Utils.java +++ b/src/main/java/fr/xephi/authme/util/Utils.java @@ -2,7 +2,6 @@ package fr.xephi.authme.util; import fr.xephi.authme.AuthMe; import fr.xephi.authme.ConsoleLogger; -import fr.xephi.authme.cache.auth.PlayerCache; import fr.xephi.authme.cache.limbo.LimboCache; import fr.xephi.authme.cache.limbo.LimboPlayer; import fr.xephi.authme.events.AuthMeTeleportEvent; @@ -118,23 +117,9 @@ public final class Utils { return permsMan.addGroup(player, group); } - // TODO: Move to a Manager - public static boolean checkAuth(Player player) { - if (player == null || Utils.isUnrestricted(player)) { - return true; - } - - if (PlayerCache.getInstance().isAuthenticated(player.getName())) { - return true; - } - - if (!Settings.isForcedRegistrationEnabled && !plugin.getDataSource().isAuthAvailable(player.getName())) { - return true; - } - return false; - } - public static boolean isUnrestricted(Player player) { + // TODO ljacqu 20160602: Checking for Settings.isAllowRestrictedIp is wrong! Nothing in the config suggests + // that this setting has anything to do with unrestricted names return Settings.isAllowRestrictedIp && Settings.getUnrestrictedName.contains(player.getName().toLowerCase()); } @@ -165,11 +150,6 @@ public final class Utils { }); } - @Deprecated - public static boolean isNPC(Player player) { - return plugin.getPluginHooks().isNpc(player); - } - public static void teleportToSpawn(Player player) { if (Settings.isTeleportToSpawnEnabled && !Settings.noTeleport) { Location spawn = plugin.getSpawnLocation(player); diff --git a/src/test/java/fr/xephi/authme/listener/AuthMeBlockListenerTest.java b/src/test/java/fr/xephi/authme/listener/AuthMeBlockListenerTest.java new file mode 100644 index 000000000..e96a30223 --- /dev/null +++ b/src/test/java/fr/xephi/authme/listener/AuthMeBlockListenerTest.java @@ -0,0 +1,95 @@ +package fr.xephi.authme.listener; + +import org.bukkit.entity.Player; +import org.bukkit.event.block.BlockBreakEvent; +import org.bukkit.event.block.BlockPlaceEvent; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; + +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; + +/** + * Test for {@link AuthMeBlockListener}. + */ +@RunWith(MockitoJUnitRunner.class) +public class AuthMeBlockListenerTest { + + @InjectMocks + private AuthMeBlockListener listener; + + @Mock + private ListenerService listenerService; + + @Test + public void shouldAllowPlaceEvent() { + // given + Player player = mock(Player.class); + BlockPlaceEvent event = mock(BlockPlaceEvent.class); + given(event.getPlayer()).willReturn(player); + given(listenerService.shouldCancelEvent(player)).willReturn(false); + + // when + listener.onBlockPlace(event); + + // then + verify(event).getPlayer(); + verifyNoMoreInteractions(event); + } + + @Test + public void shouldDenyPlaceEvent() { + // given + Player player = mock(Player.class); + BlockPlaceEvent event = mock(BlockPlaceEvent.class); + given(event.getPlayer()).willReturn(player); + given(listenerService.shouldCancelEvent(player)).willReturn(true); + + // when + listener.onBlockPlace(event); + + // then + verify(event).setCancelled(true); + verify(event).getPlayer(); + verifyNoMoreInteractions(event); + } + + @Test + public void shouldAllowBreakEvent() { + // given + Player player = mock(Player.class); + BlockBreakEvent event = mock(BlockBreakEvent.class); + given(event.getPlayer()).willReturn(player); + given(listenerService.shouldCancelEvent(player)).willReturn(false); + + // when + listener.onBlockBreak(event); + + // then + verify(event).getPlayer(); + verifyNoMoreInteractions(event); + } + + @Test + public void shouldDenyBreakEvent() { + // given + Player player = mock(Player.class); + BlockBreakEvent event = mock(BlockBreakEvent.class); + given(event.getPlayer()).willReturn(player); + given(listenerService.shouldCancelEvent(player)).willReturn(true); + + // when + listener.onBlockBreak(event); + + // then + verify(event).setCancelled(true); + verify(event).getPlayer(); + verifyNoMoreInteractions(event); + } + +} diff --git a/src/test/java/fr/xephi/authme/listener/ListenerServiceTest.java b/src/test/java/fr/xephi/authme/listener/ListenerServiceTest.java new file mode 100644 index 000000000..c9413e17e --- /dev/null +++ b/src/test/java/fr/xephi/authme/listener/ListenerServiceTest.java @@ -0,0 +1,254 @@ +package fr.xephi.authme.listener; + +import fr.xephi.authme.cache.auth.PlayerCache; +import fr.xephi.authme.datasource.DataSource; +import fr.xephi.authme.hooks.PluginHooks; +import fr.xephi.authme.settings.NewSetting; +import fr.xephi.authme.settings.properties.RegistrationSettings; +import fr.xephi.authme.settings.properties.RestrictionSettings; +import org.bukkit.entity.Entity; +import org.bukkit.entity.Player; +import org.bukkit.event.HandlerList; +import org.bukkit.event.entity.EntityEvent; +import org.bukkit.event.player.PlayerEvent; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.stubbing.Answer; + +import java.lang.reflect.Method; +import java.util.Arrays; + +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThat; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; + +/** + * Test for {@link ListenerService}. + */ +@RunWith(MockitoJUnitRunner.class) +public class ListenerServiceTest { + + private ListenerService listenerService; + + @Mock + private NewSetting settings; + + @Mock + private DataSource dataSource; + + @Mock + private PluginHooks pluginHooks; + + @Mock + private PlayerCache playerCache; + + @Before + public void initializeTestSetup() { + given(settings.getProperty(RegistrationSettings.FORCE)).willReturn(true); + given(settings.getProperty(RestrictionSettings.UNRESTRICTED_NAMES)).willReturn( + Arrays.asList("npc1", "npc2", "npc3")); + + // Note ljacqu 20160602: We use a hacky way to avoid having to instantiate the service in each test: + // the listenerService test is initialized as a mock that will answer to any method invocation by creating an + // actual service object (with the @Mock fields) and then invoking the method on that actual service. + // As long as there is no interaction with listenerService all of the mock setups will have effect. + listenerService = mock(ListenerService.class, new Answer() { + @Override + public Object answer(InvocationOnMock invocation) throws Exception { + Method method = invocation.getMethod(); + ListenerService service = new ListenerService(settings, dataSource, pluginHooks, playerCache); + return method.invoke(service, invocation.getArguments()); + } + }); + } + + @Test + public void shouldHandleEventWithNullEntity() { + // given + EntityEvent event = mock(EntityEvent.class); + given(event.getEntity()).willReturn(null); + + // when + boolean result = listenerService.shouldCancelEvent(event); + + // then + assertThat(result, equalTo(false)); + } + + @Test + public void shouldHandleEntityEventWithNonPlayerEntity() { + // given + EntityEvent event = mock(EntityEvent.class); + given(event.getEntity()).willReturn(mock(Entity.class)); + + // when + boolean result = listenerService.shouldCancelEvent(event); + + // then + assertThat(result, equalTo(false)); + } + + @Test + public void shouldAllowAuthenticatedPlayer() { + // given + String playerName = "Bobby"; + Player player = mockPlayerWithName(playerName); + given(playerCache.isAuthenticated(playerName)).willReturn(true); + EntityEvent event = mock(EntityEvent.class); + given(event.getEntity()).willReturn(player); + + // when + boolean result = listenerService.shouldCancelEvent(event); + + // then + assertThat(result, equalTo(false)); + verify(playerCache).isAuthenticated(playerName); + verifyZeroInteractions(dataSource); + } + + @Test + public void shouldDenyUnLoggedPlayer() { + // given + String playerName = "Tester"; + Player player = mockPlayerWithName(playerName); + given(playerCache.isAuthenticated(playerName)).willReturn(false); + EntityEvent event = mock(EntityEvent.class); + given(event.getEntity()).willReturn(player); + + // when + boolean result = listenerService.shouldCancelEvent(event); + + // then + assertThat(result, equalTo(true)); + verify(playerCache).isAuthenticated(playerName); + // makes sure the setting is checked first = avoid unnecessary DB operation + verifyZeroInteractions(dataSource); + } + + @Test + public void shouldAllowUnloggedPlayerForOptionalRegistration() { + // given + String playerName = "myPlayer1"; + Player player = mockPlayerWithName(playerName); + given(playerCache.isAuthenticated(playerName)).willReturn(false); + given(settings.getProperty(RegistrationSettings.FORCE)).willReturn(false); + EntityEvent event = mock(EntityEvent.class); + given(event.getEntity()).willReturn(player); + + // when + boolean result = listenerService.shouldCancelEvent(event); + + // then + assertThat(result, equalTo(false)); + verify(playerCache).isAuthenticated(playerName); + verify(dataSource).isAuthAvailable(playerName); + } + + @Test + public void shouldAllowUnrestrictedName() { + // given + String playerName = "Npc2"; + Player player = mockPlayerWithName(playerName); + EntityEvent event = mock(EntityEvent.class); + given(event.getEntity()).willReturn(player); + + // when + boolean result = listenerService.shouldCancelEvent(event); + + // then + assertThat(result, equalTo(false)); + verifyZeroInteractions(dataSource); + } + + @Test + public void shouldAllowNpcPlayer() { + // given + String playerName = "other_npc"; + Player player = mockPlayerWithName(playerName); + EntityEvent event = mock(EntityEvent.class); + given(event.getEntity()).willReturn(player); + given(pluginHooks.isNpc(player)).willReturn(true); + + // when + boolean result = listenerService.shouldCancelEvent(event); + + // then + assertThat(result, equalTo(false)); + verify(pluginHooks).isNpc(player); + } + + @Test + // This simply forwards to shouldCancelEvent(Player), so the rest is already tested + public void shouldHandlePlayerEvent() { + // given + String playerName = "example"; + Player player = mockPlayerWithName(playerName); + PlayerEvent event = new TestPlayerEvent(player); + given(playerCache.isAuthenticated(playerName)).willReturn(true); + + // when + boolean result = listenerService.shouldCancelEvent(event); + + // then + assertThat(result, equalTo(false)); + verify(playerCache).isAuthenticated(playerName); + verifyZeroInteractions(dataSource); + } + + @Test + public void shouldHandlePlayerEventWithNullPlayer() { + // given + PlayerEvent event = new TestPlayerEvent(null); + + // when + boolean result = listenerService.shouldCancelEvent(event); + + // then + assertThat(result, equalTo(false)); + } + + @Test + // The previous tests verify most of shouldCancelEvent(Player) + public void shouldVerifyBasedOnPlayer() { + // given + String playerName = "player"; + Player player = mockPlayerWithName(playerName); + + // when + boolean result = listenerService.shouldCancelEvent(player); + + // then + assertThat(result, equalTo(true)); + verify(playerCache).isAuthenticated(playerName); + verifyZeroInteractions(dataSource); + verify(pluginHooks).isNpc(player); + } + + private static Player mockPlayerWithName(String name) { + Player player = mock(Player.class); + given(player.getName()).willReturn(name); + return player; + } + + /** + * Test implementation of {@link PlayerEvent} (necessary because + * {@link PlayerEvent#getPlayer()} is declared final). + */ + private static final class TestPlayerEvent extends PlayerEvent { + public TestPlayerEvent(Player player) { + super(player); + } + + @Override + public HandlerList getHandlers() { + return null; + } + } +} diff --git a/src/test/java/fr/xephi/authme/listener/OnJoinVerifierTest.java b/src/test/java/fr/xephi/authme/listener/OnJoinVerifierTest.java index eb7e15112..1cf9d0c1e 100644 --- a/src/test/java/fr/xephi/authme/listener/OnJoinVerifierTest.java +++ b/src/test/java/fr/xephi/authme/listener/OnJoinVerifierTest.java @@ -396,7 +396,6 @@ public class OnJoinVerifierTest { } private void expectValidationExceptionWith(MessageKey messageKey, String... args) { - //expectedException.expect(FailedVerificationException.class); expectedException.expect(exceptionWithData(messageKey, args)); } diff --git a/src/test/java/fr/xephi/authme/settings/SpawnLoaderTest.java b/src/test/java/fr/xephi/authme/settings/SpawnLoaderTest.java index 7bd0d40ec..06088e194 100644 --- a/src/test/java/fr/xephi/authme/settings/SpawnLoaderTest.java +++ b/src/test/java/fr/xephi/authme/settings/SpawnLoaderTest.java @@ -12,8 +12,6 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; -import org.junit.runner.RunWith; -import org.mockito.runners.MockitoJUnitRunner; import java.io.File; import java.io.IOException; @@ -26,7 +24,6 @@ import static org.mockito.Mockito.mock; /** * Test for {@link SpawnLoader}. */ -@RunWith(MockitoJUnitRunner.class) public class SpawnLoaderTest { @Rule