Use Collection instead of List; Fix plugin duplication in getDependingPlugins results

Bungee/src/main/java/net/frankheijden/serverutils/bungee/managers/BungeePluginManager.java

@@ -145,7 +145,7 @@ public class BungeePluginManager extends AbstractPluginManager<Plugin, BungeePlu
     }
 
     @Override
-    protected Optional<Plugin> checkPluginStates(List<Plugin> plugins, boolean enabled) {
+    protected Optional<Plugin> checkPluginStates(Collection<Plugin> plugins, boolean enabled) {
         return Optional.empty(); // Bungee can't differentiate between "loaded" and "enabled"
     }
 

Common/src/main/java/net/frankheijden/serverutils/common/commands/CommandServerUtils.java

@@ -7,9 +7,12 @@ import cloud.commandframework.context.CommandContext;
 import java.io.File;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Optional;
 import java.util.function.Consumer;
 import java.util.function.Function;
@@ -202,7 +205,7 @@ public abstract class CommandServerUtils<U extends ServerUtilsPlugin<P, ?, C, ?,
 
     private void handleUnloadPlugin(CommandContext<C> context) {
         C sender = context.getSender();
-        List<P> plugins = Arrays.asList(context.get("plugins"));
+        List<P> plugins = new ArrayList<>(Arrays.asList(context.get("plugins")));
 
         if (checkProtectedPlugins(sender, plugins)) {
             return;
@@ -210,7 +213,7 @@ public abstract class CommandServerUtils<U extends ServerUtilsPlugin<P, ?, C, ?,
 
         boolean recursive = context.flags().contains("recursive");
 
-        List<P> dependingPlugins = getDependingPlugins(context, sender, plugins, "unloadplugin");
+        Collection<P> dependingPlugins = getDependingPlugins(context, sender, plugins, "unloadplugin");
         if (!recursive) {
             if (!dependingPlugins.isEmpty()) return;
         } else {
@@ -240,7 +243,7 @@ public abstract class CommandServerUtils<U extends ServerUtilsPlugin<P, ?, C, ?,
 
         boolean recursive = context.flags().contains("recursive");
 
-        List<P> dependingPlugins = getDependingPlugins(context, sender, plugins, "reloadplugin");
+        Collection<P> dependingPlugins = getDependingPlugins(context, sender, plugins, "reloadplugin");
         if (!recursive && !dependingPlugins.isEmpty()) {
             return;
         }
@@ -297,15 +300,16 @@ public abstract class CommandServerUtils<U extends ServerUtilsPlugin<P, ?, C, ?,
         return !getDependingPlugins(context, sender, plugins, subcommand).isEmpty();
     }
 
-    protected List<P> getDependingPlugins(CommandContext<C> context, C sender, List<P> plugins, String subcommand) {
-        if (context.flags().contains("force")) return Collections.emptyList();
+    protected Collection<P> getDependingPlugins(CommandContext<C> context, C sender, List<P> plugins,
+            String subcommand) {
+        if (context.flags().contains("force")) return Collections.emptySet();
 
         boolean recursive = context.flags().contains("recursive");
 
         AbstractPluginManager<P, ?> pluginManager = plugin.getPluginManager();
         MessagesResource messages = plugin.getMessagesResource();
 
-        List<P> dependingPluginsAll = new ArrayList<>();
+        Map<String, P> dependingPluginsAll = new HashMap<>();
         boolean hasDependingPlugins = false;
         for (P plugin : plugins) {
             String pluginId = pluginManager.getPluginId(plugin);
@@ -328,7 +332,9 @@ public abstract class CommandServerUtils<U extends ServerUtilsPlugin<P, ?, C, ?,
                 }
 
                 hasDependingPlugins = true;
-                dependingPluginsAll.addAll(dependingPlugins);
+                for (P dependingPlugin : dependingPlugins) {
+                    dependingPluginsAll.putIfAbsent(pluginManager.getPluginId(dependingPlugin), dependingPlugin);
+                }
             }
         }
 
@@ -350,7 +356,7 @@ public abstract class CommandServerUtils<U extends ServerUtilsPlugin<P, ?, C, ?,
             ));
         }
 
-        return dependingPluginsAll;
+        return dependingPluginsAll.values();
     }
 
     protected boolean checkServerUtils(CommandContext<C> context, C sender, List<P> plugins) {

Common/src/main/java/net/frankheijden/serverutils/common/managers/AbstractPluginManager.java

@@ -39,7 +39,7 @@ public abstract class AbstractPluginManager<P, D extends ServerUtilsPluginDescri
      * Loads a list of files as plugins.
      */
     public PluginResults<P> loadPlugins(List<File> files) {
-        List<D> descriptions = new ArrayList<>(files.size());
+        Set<D> descriptions = new HashSet<>(files.size());
 
         for (File file : files) {
             D description;
@@ -117,7 +117,7 @@ public abstract class AbstractPluginManager<P, D extends ServerUtilsPluginDescri
      * Finds the first plugin where the enabled state does not match the given state it must be in.
      * This method can be overridden by implementations which don't support the enabled state.
      */
-    protected Optional<P> checkPluginStates(List<P> plugins, boolean enabled) {
+    protected Optional<P> checkPluginStates(Collection<P> plugins, boolean enabled) {
         for (P plugin : plugins) {
             if (isPluginEnabled(plugin) != enabled) {
                 return Optional.of(plugin);
@@ -142,7 +142,7 @@ public abstract class AbstractPluginManager<P, D extends ServerUtilsPluginDescri
     /**
      * Disables a list of plugins.
      */
-    public PluginResults<P> disablePlugins(List<P> plugins) {
+    public PluginResults<P> disablePlugins(Collection<P> plugins) {
         Optional<P> pluginOptional = checkPluginStates(plugins, true);
         if (pluginOptional.isPresent()) {
             return new PluginResults<P>().addResult(getPluginId(pluginOptional.get()), Result.ALREADY_DISABLED);
@@ -235,7 +235,7 @@ public abstract class AbstractPluginManager<P, D extends ServerUtilsPluginDescri
     /**
      * Unloads a list of plugins.
      */
-    public CloseablePluginResults<P> unloadPlugins(List<P> plugins) {
+    public CloseablePluginResults<P> unloadPlugins(Collection<P> plugins) {
         List<P> orderedPlugins;
         try {
             orderedPlugins = determineLoadOrder(plugins);
@@ -259,7 +259,7 @@ public abstract class AbstractPluginManager<P, D extends ServerUtilsPluginDescri
     /**
      * Determines the load order of a list of plugins.
      */
-    public List<P> determineLoadOrder(List<P> plugins) throws IllegalStateException {
+    public List<P> determineLoadOrder(Collection<P> plugins) throws IllegalStateException {
         Map<D, P> descriptionMap = new HashMap<>(plugins.size());
         for (P plugin : plugins) {
             descriptionMap.put(getLoadedPluginDescription(plugin), plugin);
@@ -276,7 +276,7 @@ public abstract class AbstractPluginManager<P, D extends ServerUtilsPluginDescri
      * Determines the load order for a given collection of descriptions.
      * @throws IllegalStateException Iff circular dependency
      */
-    public List<D> determineLoadOrder(Collection<? extends D> descriptions) throws IllegalStateException {
+    public List<D> determineLoadOrder(Set<? extends D> descriptions) throws IllegalStateException {
         Map<String, D> pluginIdToDescriptionMap = new HashMap<>();
         for (D description : descriptions) {
             pluginIdToDescriptionMap.put(description.getId(), description);