Minor: change bukkit service to return a plain collection of Player

- List is read-only and can be easily "upcast" to Collection<Player>
- Remove awkward hack that was needed for mocking the method previously (issues with generics)

src/main/java/fr/xephi/authme/command/executable/authme/VersionCommand.java

@@ -23,7 +23,7 @@ public class VersionCommand implements ExecutableCommand {
         sender.sendMessage(ChatColor.GOLD + "Version: " + ChatColor.WHITE + AuthMe.getPluginName()
             + " v" + AuthMe.getPluginVersion() + ChatColor.GRAY + " (build: " + AuthMe.getPluginBuildNumber() + ")");
         sender.sendMessage(ChatColor.GOLD + "Developers:");
-        Collection<? extends Player> onlinePlayers = bukkitService.getOnlinePlayers();
+        Collection<Player> onlinePlayers = bukkitService.getOnlinePlayers();
         printDeveloper(sender, "Alexandre Vanhecke", "xephi59", "Original Author", onlinePlayers);
         printDeveloper(sender, "Lucas J.", "ljacqu", "Main Developer", onlinePlayers);
         printDeveloper(sender, "Gnat008", "gnat008", "Developer", onlinePlayers);
@@ -49,7 +49,7 @@ public class VersionCommand implements ExecutableCommand {
      * @param onlinePlayers The list of online players
      */
     private static void printDeveloper(CommandSender sender, String name, String minecraftName, String function,
-                                       Collection<? extends Player> onlinePlayers) {
+                                       Collection<Player> onlinePlayers) {
         // Print the name
         StringBuilder msg = new StringBuilder();
         msg.append(" ")
@@ -77,7 +77,7 @@ public class VersionCommand implements ExecutableCommand {
      *
      * @return True if the player is online, false otherwise
      */
-    private static boolean isPlayerOnline(String minecraftName, Collection<? extends Player> onlinePlayers) {
+    private static boolean isPlayerOnline(String minecraftName, Collection<Player> onlinePlayers) {
         for (Player player : onlinePlayers) {
             if (player.getName().equalsIgnoreCase(minecraftName)) {
                 return true;

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

@@ -128,7 +128,7 @@ public class OnJoinVerifier implements Reloadable {
         }
 
         // Server is full and player is VIP; attempt to kick a non-VIP player to make room
-        Collection<? extends Player> onlinePlayers = bukkitService.getOnlinePlayers();
+        Collection<Player> onlinePlayers = bukkitService.getOnlinePlayers();
         if (onlinePlayers.size() < server.getMaxPlayers()) {
             event.allow();
             return false;
@@ -207,7 +207,7 @@ public class OnJoinVerifier implements Reloadable {
      *
      * @return the player to kick, or null if none applicable
      */
-    private Player generateKickPlayer(Collection<? extends Player> onlinePlayers) {
+    private Player generateKickPlayer(Collection<Player> onlinePlayers) {
         for (Player player : onlinePlayers) {
             if (!permissionsManager.hasPermission(player, PlayerStatePermission.IS_VIP)) {
                 return player;

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

@@ -233,8 +233,9 @@ public class BukkitService implements SettingsDependent {
      *
      * @return collection of online players
      */
-    public Collection<? extends Player> getOnlinePlayers() {
+    @SuppressWarnings("unchecked")
-        return Bukkit.getOnlinePlayers();
+    public Collection<Player> getOnlinePlayers() {
+        return (Collection<Player>) Bukkit.getOnlinePlayers();
     }
 
     /**

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

@@ -33,7 +33,6 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
-import static fr.xephi.authme.service.BukkitServiceTestHelper.returnGivenOnlinePlayers;
 import static org.hamcrest.Matchers.equalTo;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.fail;
@@ -123,7 +122,7 @@ public class OnJoinVerifierTest {
         List<Player> onlinePlayers = Arrays.asList(mock(Player.class), mock(Player.class));
         given(permissionsManager.hasPermission(onlinePlayers.get(0), PlayerStatePermission.IS_VIP)).willReturn(true);
         given(permissionsManager.hasPermission(onlinePlayers.get(1), PlayerStatePermission.IS_VIP)).willReturn(false);
-        returnGivenOnlinePlayers(bukkitService, onlinePlayers);
+        given(bukkitService.getOnlinePlayers()).willReturn(onlinePlayers);
         given(server.getMaxPlayers()).willReturn(onlinePlayers.size());
         given(messages.retrieveSingle(player, MessageKey.KICK_FOR_VIP)).willReturn("kick for vip");
 
@@ -147,7 +146,7 @@ public class OnJoinVerifierTest {
         given(permissionsManager.hasPermission(player, PlayerStatePermission.IS_VIP)).willReturn(true);
         List<Player> onlinePlayers = Collections.singletonList(mock(Player.class));
         given(permissionsManager.hasPermission(onlinePlayers.get(0), PlayerStatePermission.IS_VIP)).willReturn(true);
-        returnGivenOnlinePlayers(bukkitService, onlinePlayers);
+        given(bukkitService.getOnlinePlayers()).willReturn(onlinePlayers);
         given(server.getMaxPlayers()).willReturn(onlinePlayers.size());
         given(messages.retrieveSingle(player, MessageKey.KICK_FULL_SERVER)).willReturn("kick full server");
 

src/test/java/fr/xephi/authme/process/login/AsynchronousLoginTest.java

@@ -27,7 +27,6 @@ import org.mockito.stubbing.Answer;
 import java.util.Arrays;
 import java.util.List;
 
-import static fr.xephi.authme.service.BukkitServiceTestHelper.returnGivenOnlinePlayers;
 import static org.hamcrest.Matchers.equalTo;
 import static org.junit.Assert.assertThat;
 import static org.mockito.ArgumentMatchers.any;
@@ -266,7 +265,7 @@ public class AsynchronousLoginTest {
         TestHelper.mockPlayerIp(playerF, "3.3.3.3");
 
         List<Player> onlinePlayers = Arrays.asList(playerA, playerB, playerC, playerD, playerE, playerF);
-        returnGivenOnlinePlayers(bukkitService, onlinePlayers);
+        given(bukkitService.getOnlinePlayers()).willReturn(onlinePlayers);
     }
 
 }

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

@@ -19,7 +19,6 @@ import org.mockito.Mock;
 import java.util.Arrays;
 import java.util.List;
 
-import static fr.xephi.authme.service.BukkitServiceTestHelper.returnGivenOnlinePlayers;
 import static fr.xephi.authme.service.BukkitServiceTestHelper.setBukkitServiceToScheduleSyncDelayedTaskWithDelay;
 import static org.hamcrest.Matchers.equalTo;
 import static org.junit.Assert.assertThat;
@@ -158,7 +157,7 @@ public class AntiBotServiceTest {
     public void shouldInformPlayersOnActivation() {
         // given - listening antibot
         List<Player> players = Arrays.asList(mock(Player.class), mock(Player.class));
-        returnGivenOnlinePlayers(bukkitService, players);
+        given(bukkitService.getOnlinePlayers()).willReturn(players);
         given(permissionsManager.hasPermission(players.get(0), AdminPermission.ANTIBOT_MESSAGES)).willReturn(false);
         given(permissionsManager.hasPermission(players.get(1), AdminPermission.ANTIBOT_MESSAGES)).willReturn(true);
 

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

@@ -1,12 +1,7 @@
 package fr.xephi.authme.service;
 
-import org.bukkit.entity.Player;
-
-import java.util.Collection;
-
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyLong;
-import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.doAnswer;
 
 /**
@@ -86,17 +81,4 @@ public final class BukkitServiceTestHelper {
             return null;
         }).when(bukkitService).scheduleSyncDelayedTask(any(Runnable.class), anyLong());
     }
-
-    /**
-     * Sets a BukkitService mock to return the given players when its method
-     * {@link BukkitService#getOnlinePlayers()} is invoked.
-     *
-     * @param bukkitService the mock to set behavior on
-     * @param players the players to return
-     */
-    @SuppressWarnings("unchecked")
-    public static void returnGivenOnlinePlayers(BukkitService bukkitService, Collection<Player> players) {
-        // The compiler gets lost in generics because Collection<? extends Player> is returned from getOnlinePlayers()
-        given(bukkitService.getOnlinePlayers()).willReturn((Collection) players);
-    }
 }

src/test/java/fr/xephi/authme/settings/WelcomeMessageConfigurationTest.java

@@ -26,15 +26,12 @@ import java.nio.file.Files;
 import java.util.Arrays;
 import java.util.List;
 
-import static fr.xephi.authme.service.BukkitServiceTestHelper.returnGivenOnlinePlayers;
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasSize;
 import static org.junit.Assert.assertThat;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.only;
-import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyZeroInteractions;
 
 /**
@@ -115,7 +112,7 @@ public class WelcomeMessageConfigurationTest {
         // given
         String welcomeMessage = "{ONLINE}/{MAXPLAYERS} online\n{LOGINS} logged in\nYour world is {WORLD}\nServer: {VERSION}";
         setWelcomeMessageAndReload(welcomeMessage);
-        returnGivenOnlinePlayers(bukkitService, Arrays.asList(mock(Player.class), mock(Player.class)));
+        given(bukkitService.getOnlinePlayers()).willReturn(Arrays.asList(mock(Player.class), mock(Player.class)));
         given(server.getMaxPlayers()).willReturn(20);
         given(playerCache.getLogged()).willReturn(1);
         given(server.getBukkitVersion()).willReturn("Bukkit-456.77.8");