Move #isNpc method to PlayerUtils

- After dropping our hook to CombatTagPlus it is not relevant for it to be in PluginHooksService anymore

src/main/java/fr/xephi/authme/api/NewAPI.java

@@ -9,8 +9,8 @@ import fr.xephi.authme.process.register.executors.ApiPasswordRegisterParams;
 import fr.xephi.authme.process.register.executors.RegistrationMethod;
 import fr.xephi.authme.security.PasswordSecurity;
 import fr.xephi.authme.security.crypts.HashedPassword;
-import fr.xephi.authme.service.PluginHookService;
 import fr.xephi.authme.service.ValidationService;
+import fr.xephi.authme.util.PlayerUtils;
 import org.bukkit.Bukkit;
 import org.bukkit.Location;
 import org.bukkit.entity.Player;
@@ -35,7 +35,6 @@ public class NewAPI {
 
     private static NewAPI singleton;
     private final AuthMe plugin;
-    private final PluginHookService pluginHookService;
     private final DataSource dataSource;
     private final PasswordSecurity passwordSecurity;
     private final Management management;
@@ -46,10 +45,9 @@ public class NewAPI {
      * Constructor for NewAPI.
      */
     @Inject
-    NewAPI(AuthMe plugin, PluginHookService pluginHookService, DataSource dataSource, PasswordSecurity passwordSecurity,
+    NewAPI(AuthMe plugin, DataSource dataSource, PasswordSecurity passwordSecurity,
            Management management, ValidationService validationService, PlayerCache playerCache) {
         this.plugin = plugin;
-        this.pluginHookService = pluginHookService;
         this.dataSource = dataSource;
         this.passwordSecurity = passwordSecurity;
         this.management = management;
@@ -108,7 +106,7 @@ public class NewAPI {
      * @return true if the player is an npc
      */
     public boolean isNPC(Player player) {
-        return pluginHookService.isNpc(player);
+        return PlayerUtils.isNpc(player);
     }
 
     /**

src/main/java/fr/xephi/authme/api/v3/AuthMeApi.java

@@ -10,8 +10,8 @@ import fr.xephi.authme.process.register.executors.RegistrationMethod;
 import fr.xephi.authme.security.PasswordSecurity;
 import fr.xephi.authme.security.crypts.HashedPassword;
 import fr.xephi.authme.service.GeoIpService;
-import fr.xephi.authme.service.PluginHookService;
 import fr.xephi.authme.service.ValidationService;
+import fr.xephi.authme.util.PlayerUtils;
 import org.bukkit.Bukkit;
 import org.bukkit.Location;
 import org.bukkit.entity.Player;
@@ -33,7 +33,6 @@ public class AuthMeApi {
 
     private static AuthMeApi singleton;
     private final AuthMe plugin;
-    private final PluginHookService pluginHookService;
     private final DataSource dataSource;
     private final PasswordSecurity passwordSecurity;
     private final Management management;
@@ -45,11 +44,9 @@ public class AuthMeApi {
      * Constructor for AuthMeApi.
      */
     @Inject
-    AuthMeApi(AuthMe plugin, PluginHookService pluginHookService, DataSource dataSource, PlayerCache playerCache,
+    AuthMeApi(AuthMe plugin, DataSource dataSource, PlayerCache playerCache, PasswordSecurity passwordSecurity,
-              PasswordSecurity passwordSecurity, Management management, ValidationService validationService,
+              Management management, ValidationService validationService, GeoIpService geoIpService) {
-              GeoIpService geoIpService) {
         this.plugin = plugin;
-        this.pluginHookService = pluginHookService;
         this.dataSource = dataSource;
         this.passwordSecurity = passwordSecurity;
         this.management = management;
@@ -109,7 +106,7 @@ public class AuthMeApi {
      * @return true if the player is an npc
      */
     public boolean isNpc(Player player) {
-        return pluginHookService.isNpc(player);
+        return PlayerUtils.isNpc(player);
     }
 
     /**

src/main/java/fr/xephi/authme/initialization/OnShutdownPlayerSaver.java

@@ -5,11 +5,11 @@ import fr.xephi.authme.data.auth.PlayerCache;
 import fr.xephi.authme.data.limbo.LimboService;
 import fr.xephi.authme.datasource.DataSource;
 import fr.xephi.authme.service.BukkitService;
-import fr.xephi.authme.service.PluginHookService;
 import fr.xephi.authme.service.ValidationService;
 import fr.xephi.authme.settings.Settings;
 import fr.xephi.authme.settings.SpawnLoader;
 import fr.xephi.authme.settings.properties.RestrictionSettings;
+import fr.xephi.authme.util.PlayerUtils;
 import org.bukkit.Location;
 import org.bukkit.entity.Player;
 
@@ -31,8 +31,6 @@ public class OnShutdownPlayerSaver {
     @Inject
     private SpawnLoader spawnLoader;
     @Inject
-    private PluginHookService pluginHookService;
-    @Inject
     private PlayerCache playerCache;
     @Inject
     private LimboService limboService;
@@ -51,7 +49,7 @@ public class OnShutdownPlayerSaver {
 
     private void savePlayer(Player player) {
         final String name = player.getName().toLowerCase();
-        if (pluginHookService.isNpc(player) || validationService.isUnrestricted(name)) {
+        if (PlayerUtils.isNpc(player) || validationService.isUnrestricted(name)) {
             return;
         }
         if (limboService.hasLimboPlayer(name)) {

src/main/java/fr/xephi/authme/listener/ListenerService.java

@@ -2,11 +2,11 @@ package fr.xephi.authme.listener;
 
 import fr.xephi.authme.data.auth.PlayerCache;
 import fr.xephi.authme.datasource.DataSource;
-import fr.xephi.authme.service.PluginHookService;
 import fr.xephi.authme.initialization.SettingsDependent;
+import fr.xephi.authme.service.ValidationService;
 import fr.xephi.authme.settings.Settings;
 import fr.xephi.authme.settings.properties.RegistrationSettings;
-import fr.xephi.authme.service.ValidationService;
+import fr.xephi.authme.util.PlayerUtils;
 import org.bukkit.entity.Entity;
 import org.bukkit.entity.Player;
 import org.bukkit.event.entity.EntityEvent;
@@ -20,17 +20,15 @@ import javax.inject.Inject;
 class ListenerService implements SettingsDependent {
 
     private final DataSource dataSource;
-    private final PluginHookService pluginHookService;
     private final PlayerCache playerCache;
     private final ValidationService validationService;
 
     private boolean isRegistrationForced;
 
     @Inject
-    ListenerService(Settings settings, DataSource dataSource, PluginHookService pluginHookService,
+    ListenerService(Settings settings, DataSource dataSource, PlayerCache playerCache,
-                    PlayerCache playerCache, ValidationService validationService) {
+                    ValidationService validationService) {
         this.dataSource = dataSource;
-        this.pluginHookService = pluginHookService;
         this.playerCache = playerCache;
         this.validationService = validationService;
         reload(settings);
@@ -79,7 +77,7 @@ class ListenerService implements SettingsDependent {
      * @return true if the associated event should be canceled, false otherwise
      */
     public boolean shouldCancelEvent(Player player) {
-        return player != null && !checkAuth(player.getName()) && !pluginHookService.isNpc(player);
+        return player != null && !checkAuth(player.getName()) && !PlayerUtils.isNpc(player);
     }
 
     @Override

src/main/java/fr/xephi/authme/service/PluginHookService.java

@@ -76,16 +76,6 @@ public class PluginHookService {
         return null;
     }
 
-    /**
-     * Checks whether the player is an NPC.
-     *
-     * @param player The player to process
-     * @return True if player is NPC, false otherwise
-     */
-    public boolean isNpc(Player player) {
-        return player.hasMetadata("NPC");
-    }
-
 
     // ------
     // "Is plugin available" methods

src/main/java/fr/xephi/authme/util/PlayerUtils.java

@@ -39,4 +39,15 @@ public final class PlayerUtils {
     public static String getPlayerIp(Player p) {
         return p.getAddress().getAddress().getHostAddress();
     }
+
+    /**
+     * Returns if the player is an NPC or not.
+     *
+     * @param player The player to check
+     *
+     * @return True if the player is an NPC, false otherwise
+     */
+    public static boolean isNpc(Player player) {
+       return player.hasMetadata("NPC");
+    }
 }

src/test/java/fr/xephi/authme/api/NewAPITest.java

@@ -7,7 +7,6 @@ import fr.xephi.authme.data.auth.PlayerCache;
 import fr.xephi.authme.datasource.DataSource;
 import fr.xephi.authme.process.Management;
 import fr.xephi.authme.security.PasswordSecurity;
-import fr.xephi.authme.service.PluginHookService;
 import fr.xephi.authme.service.ValidationService;
 import org.bukkit.Bukkit;
 import org.bukkit.Location;
@@ -43,8 +42,6 @@ public class NewAPITest {
     @InjectMocks
     private NewAPI api;
 
-    @Mock
-    private PluginHookService pluginHookService;
     @Mock
     private ValidationService validationService;
     @Mock
@@ -84,13 +81,14 @@ public class NewAPITest {
     public void shouldReturnIfPlayerIsNpc() {
         // given
         Player player = mock(Player.class);
-        given(pluginHookService.isNpc(player)).willReturn(true);
+        given(player.hasMetadata("NPC")).willReturn(true);
 
         // when
         boolean result = api.isNPC(player);
 
         // then
         assertThat(result, equalTo(true));
+        verify(player).hasMetadata("NPC");
     }
 
     @Test

src/test/java/fr/xephi/authme/api/v3/AuthMeApiTest.java

@@ -6,7 +6,6 @@ import fr.xephi.authme.data.auth.PlayerCache;
 import fr.xephi.authme.datasource.DataSource;
 import fr.xephi.authme.process.Management;
 import fr.xephi.authme.security.PasswordSecurity;
-import fr.xephi.authme.service.PluginHookService;
 import fr.xephi.authme.service.ValidationService;
 import org.bukkit.Bukkit;
 import org.bukkit.Location;
@@ -43,8 +42,6 @@ public class AuthMeApiTest {
     @InjectMocks
     private AuthMeApi api;
 
-    @Mock
-    private PluginHookService pluginHookService;
     @Mock
     private ValidationService validationService;
     @Mock
@@ -84,13 +81,14 @@ public class AuthMeApiTest {
     public void shouldReturnIfPlayerIsNpc() {
         // given
         Player player = mock(Player.class);
-        given(pluginHookService.isNpc(player)).willReturn(true);
+        given(player.hasMetadata("NPC")).willReturn(true);
 
         // when
         boolean result = api.isNpc(player);
 
         // then
         assertThat(result, equalTo(true));
+        verify(player).hasMetadata("NPC");
     }
 
     @Test

src/test/java/fr/xephi/authme/listener/ListenerServiceTest.java

@@ -5,10 +5,9 @@ import ch.jalu.injector.testing.DelayedInjectionRunner;
 import ch.jalu.injector.testing.InjectDelayed;
 import fr.xephi.authme.data.auth.PlayerCache;
 import fr.xephi.authme.datasource.DataSource;
-import fr.xephi.authme.service.PluginHookService;
+import fr.xephi.authme.service.ValidationService;
 import fr.xephi.authme.settings.Settings;
 import fr.xephi.authme.settings.properties.RegistrationSettings;
-import fr.xephi.authme.service.ValidationService;
 import org.bukkit.entity.Entity;
 import org.bukkit.entity.Player;
 import org.bukkit.event.HandlerList;
@@ -40,9 +39,6 @@ public class ListenerServiceTest {
     @Mock
     private DataSource dataSource;
 
-    @Mock
-    private PluginHookService pluginHookService;
-
     @Mock
     private PlayerCache playerCache;
 
@@ -161,14 +157,14 @@ public class ListenerServiceTest {
         Player player = mockPlayerWithName(playerName);
         EntityEvent event = mock(EntityEvent.class);
         given(event.getEntity()).willReturn(player);
-        given(pluginHookService.isNpc(player)).willReturn(true);
+        given(player.hasMetadata("NPC")).willReturn(true);
 
         // when
         boolean result = listenerService.shouldCancelEvent(event);
 
         // then
         assertThat(result, equalTo(false));
-        verify(pluginHookService).isNpc(player);
+        verify(player).hasMetadata("NPC");
     }
 
     @Test
@@ -215,7 +211,6 @@ public class ListenerServiceTest {
         assertThat(result, equalTo(true));
         verify(playerCache).isAuthenticated(playerName);
         verifyZeroInteractions(dataSource);
-        verify(pluginHookService).isNpc(player);
     }
 
     private static Player mockPlayerWithName(String name) {

src/test/java/fr/xephi/authme/service/PluginHookServiceTest.java

@@ -58,8 +58,6 @@ public class PluginHookServiceTest {
         assertThat(pluginHookService.isEssentialsAvailable(), equalTo(true));
     }
 
-    // Note ljacqu 20160312: Cannot test with CombatTagPlus because its class is declared final
-
     @Test
     public void shouldHookIntoEssentialsAtInitialization() {
         // given

src/test/java/fr/xephi/authme/util/PlayerUtilsTest.java

@@ -70,4 +70,21 @@ public class PlayerUtilsTest {
         // given / when / then
         TestHelper.validateHasOnlyPrivateEmptyConstructor(PlayerUtils.class);
     }
+
+    @Test
+    public void shouldCheckIfIsNpc() {
+        // given
+        Player player1 = mock(Player.class);
+        given(player1.hasMetadata("NPC")).willReturn(false);
+        Player player2 = mock(Player.class);
+        given(player2.hasMetadata("NPC")).willReturn(true);
+
+        // when
+        boolean result1 = PlayerUtils.isNpc(player1);
+        boolean result2 = PlayerUtils.isNpc(player2);
+
+        // then
+        assertThat(result1, equalTo(false));
+        assertThat(result2, equalTo(true));
+    }
 }