Add strategy for handling the recovery of a plugin that could not be resolved (#564)

5 files changed, 306 insertions, 81 deletions

diff --git a/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java b/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java
index 2f0d210..f3a7231 100644
--- a/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java
+++ b/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java
@@ -31,6 +31,7 @@ import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -109,6 +110,7 @@ public abstract class AbstractPluginManager implements PluginManager {
     protected boolean exactVersionAllowed = false;
 
     protected VersionManager versionManager;
+    protected ResolveRecoveryStrategy resolveRecoveryStrategy;
 
     /**
      * The plugins roots are supplied as comma-separated list by {@code System.getProperty("pf4j.pluginsDir", "plugins")}.
@@ -280,56 +282,55 @@ public abstract class AbstractPluginManager implements PluginManager {
      * @return true if the plugin was unloaded, otherwise false
      */
     protected boolean unloadPlugin(String pluginId, boolean unloadDependents) {
-        try {
-            if (unloadDependents) {
-                List<String> dependents = dependencyResolver.getDependents(pluginId);
-                while (!dependents.isEmpty()) {
-                    String dependent = dependents.remove(0);
-                    unloadPlugin(dependent, false);
-                    dependents.addAll(0, dependencyResolver.getDependents(dependent));
-                }
+        if (unloadDependents) {
+            List<String> dependents = dependencyResolver.getDependents(pluginId);
+            while (!dependents.isEmpty()) {
+                String dependent = dependents.remove(0);
+                unloadPlugin(dependent, false);
+                dependents.addAll(0, dependencyResolver.getDependents(dependent));
             }
-            PluginWrapper pluginWrapper = getPlugin(pluginId);
-            PluginState pluginState;
-            try {
-                pluginState = stopPlugin(pluginId, false);
-                if (PluginState.STARTED == pluginState) {
-                    return false;
-                }
+        }
 
-                log.info("Unload plugin '{}'", getPluginLabel(pluginWrapper.getDescriptor()));
-            } catch (Exception e) {
-                if (pluginWrapper == null) {
-                    return false;
-                }
-                pluginState = PluginState.FAILED;
+        if (!plugins.containsKey(pluginId)) {
+            // nothing to do
+            return false;
+        }
+
+        PluginWrapper pluginWrapper = getPlugin(pluginId);
+        PluginState pluginState;
+        try {
+            pluginState = stopPlugin(pluginId, false);
+            if (PluginState.STARTED == pluginState) {
+                return false;
             }
 
-            // remove the plugin
-            plugins.remove(pluginId);
-            getResolvedPlugins().remove(pluginWrapper);
-
-            firePluginStateEvent(new PluginStateEvent(this, pluginWrapper, pluginState));
-
-            // remove the classloader
-            Map<String, ClassLoader> pluginClassLoaders = getPluginClassLoaders();
-            if (pluginClassLoaders.containsKey(pluginId)) {
-                ClassLoader classLoader = pluginClassLoaders.remove(pluginId);
-                if (classLoader instanceof Closeable) {
-                    try {
-                        ((Closeable) classLoader).close();
-                    } catch (IOException e) {
-                        throw new PluginRuntimeException(e, "Cannot close classloader");
-                    }
+            log.info("Unload plugin '{}'", getPluginLabel(pluginWrapper.getDescriptor()));
+        } catch (Exception e) {
+            log.error("Cannot stop plugin '{}'", getPluginLabel(pluginWrapper.getDescriptor()), e);
+            pluginState = PluginState.FAILED;
+        }
+
+        // remove the plugin
+        plugins.remove(pluginId);
+        getResolvedPlugins().remove(pluginWrapper);
+        getUnresolvedPlugins().remove(pluginWrapper);
+
+        firePluginStateEvent(new PluginStateEvent(this, pluginWrapper, pluginState));
+
+        // remove the classloader
+        Map<String, ClassLoader> pluginClassLoaders = getPluginClassLoaders();
+        if (pluginClassLoaders.containsKey(pluginId)) {
+            ClassLoader classLoader = pluginClassLoaders.remove(pluginId);
+            if (classLoader instanceof Closeable) {
+                try {
+                    ((Closeable) classLoader).close();
+                } catch (IOException e) {
+                    throw new PluginRuntimeException(e, "Cannot close classloader");
                 }
             }
-
-            return true;
-        } catch (IllegalArgumentException e) {
-            // ignore not found exceptions because this method is recursive
         }
 
-        return false;
+        return true;
     }
 
     @Override
@@ -506,11 +507,11 @@ public abstract class AbstractPluginManager implements PluginManager {
      * Check if the plugin exists in the list of plugins.
      *
      * @param pluginId the pluginId to check
-     * @throws IllegalArgumentException if the plugin does not exist
+     * @throws PluginNotFoundException if the plugin does not exist
      */
     protected void checkPluginId(String pluginId) {
         if (!plugins.containsKey(pluginId)) {
-            throw new IllegalArgumentException(String.format("Unknown pluginId %s", pluginId));
+            throw new PluginNotFoundException(pluginId);
         }
     }
 
@@ -749,6 +750,7 @@ public abstract class AbstractPluginManager implements PluginManager {
 
         versionManager = createVersionManager();
         dependencyResolver = new DependencyResolver(versionManager);
+        resolveRecoveryStrategy = ResolveRecoveryStrategy.THROW_EXCEPTION;
     }
 
     /**
@@ -814,27 +816,7 @@ public abstract class AbstractPluginManager implements PluginManager {
      * @throws PluginRuntimeException if something goes wrong
      */
     protected void resolvePlugins() {
-        // retrieves the plugins descriptors
-        List<PluginDescriptor> descriptors = plugins.values().stream()
-            .map(PluginWrapper::getDescriptor)
-            .collect(Collectors.toList());
-
-        DependencyResolver.Result result = dependencyResolver.resolve(descriptors);
-
-        if (result.hasCyclicDependency()) {
-            throw new DependencyResolver.CyclicDependencyException();
-        }
-
-        List<String> notFoundDependencies = result.getNotFoundDependencies();
-        if (!notFoundDependencies.isEmpty()) {
-            throw new DependencyResolver.DependenciesNotFoundException(notFoundDependencies);
-        }
-
-        List<DependencyResolver.WrongDependencyVersion> wrongVersionDependencies = result.getWrongVersionDependencies();
-        if (!wrongVersionDependencies.isEmpty()) {
-            throw new DependencyResolver.DependenciesWrongVersionException(wrongVersionDependencies);
-        }
-
+        DependencyResolver.Result result = resolveDependencies();
         List<String> sortedPlugins = result.getSortedPlugins();
 
         // move plugins from "unresolved" to "resolved"
@@ -1063,4 +1045,89 @@ public abstract class AbstractPluginManager implements PluginManager {
         return extensions;
     }
 
+    protected DependencyResolver.Result resolveDependencies() {
+        // retrieves the plugins descriptors
+        List<PluginDescriptor> descriptors = plugins.values().stream()
+            .map(PluginWrapper::getDescriptor)
+            .collect(Collectors.toList());
+
+        DependencyResolver.Result result = dependencyResolver.resolve(descriptors);
+
+        if (result.isOK()) {
+            return result;
+        }
+
+        if (result.hasCyclicDependency()) {
+            // cannot recover from cyclic dependency
+            throw new DependencyResolver.CyclicDependencyException();
+        }
+
+        List<String> notFoundDependencies = result.getNotFoundDependencies();
+        if (result.hasNotFoundDependencies() && resolveRecoveryStrategy.equals(ResolveRecoveryStrategy.THROW_EXCEPTION)) {
+            throw new DependencyResolver.DependenciesNotFoundException(notFoundDependencies);
+        }
+
+        List<DependencyResolver.WrongDependencyVersion> wrongVersionDependencies = result.getWrongVersionDependencies();
+        if (result.hasWrongVersionDependencies() && resolveRecoveryStrategy.equals(ResolveRecoveryStrategy.THROW_EXCEPTION)) {
+            throw new DependencyResolver.DependenciesWrongVersionException(wrongVersionDependencies);
+        }
+
+        List<PluginDescriptor> resolvedDescriptors = new ArrayList<>(descriptors);
+
+        for (String notFoundDependency : notFoundDependencies) {
+            List<String> dependents = dependencyResolver.getDependents(notFoundDependency);
+            dependents.forEach(dependent -> resolvedDescriptors.removeIf(descriptor -> descriptor.getPluginId().equals(dependent)));
+        }
+
+        for (DependencyResolver.WrongDependencyVersion wrongVersionDependency : wrongVersionDependencies) {
+            resolvedDescriptors.removeIf(descriptor -> descriptor.getPluginId().equals(wrongVersionDependency.getDependencyId()));
+        }
+
+        List<PluginDescriptor> unresolvedDescriptors = new ArrayList<>(descriptors);
+        unresolvedDescriptors.removeAll(resolvedDescriptors);
+
+        for (PluginDescriptor unresolvedDescriptor : unresolvedDescriptors) {
+            unloadPlugin(unresolvedDescriptor.getPluginId(), false);
+        }
+
+        return resolveDependencies();
+    }
+
+    /**
+     * Retrieve the strategy for handling the recovery of a plugin resolve (load) failure.
+     * Default is {@link ResolveRecoveryStrategy#THROW_EXCEPTION}.
+     *
+     * @return the strategy
+     */
+    protected final ResolveRecoveryStrategy getResolveRecoveryStrategy() {
+        return resolveRecoveryStrategy;
+    }
+
+    /**
+     * Set the strategy for handling the recovery of a plugin resolve (load) failure.
+     *
+     * @param resolveRecoveryStrategy the strategy
+     */
+    protected void setResolveRecoveryStrategy(ResolveRecoveryStrategy resolveRecoveryStrategy) {
+        Objects.requireNonNull(resolveRecoveryStrategy, "resolveRecoveryStrategy cannot be null");
+        this.resolveRecoveryStrategy = resolveRecoveryStrategy;
+    }
+
+    /**
+     * Strategy for handling the recovery of a plugin that could not be resolved
+     * (loaded) due to a dependency problem.
+     */
+    public enum ResolveRecoveryStrategy {
+
+        /**
+         * Throw an exception when a resolve (load) failure occurs.
+         */
+        THROW_EXCEPTION,
+        /**
+         * Ignore the plugin with the resolve (load) failure and continue.
+         * The plugin with problems will be removed/unloaded from the plugins list.
+         */
+        IGNORE_PLUGIN_AND_CONTINUE
+    }
+
 }
diff --git a/pf4j/src/main/java/org/pf4j/DependencyResolver.java b/pf4j/src/main/java/org/pf4j/DependencyResolver.java
index 092b58a..abe9d29 100644
--- a/pf4j/src/main/java/org/pf4j/DependencyResolver.java
+++ b/pf4j/src/main/java/org/pf4j/DependencyResolver.java
@@ -218,6 +218,15 @@ public class DependencyResolver {
         }
 
         /**
+         * Returns true if there are dependencies required that were not found.
+         *
+         * @return true if there are dependencies required that were not found
+         */
+        public boolean hasNotFoundDependencies() {
+            return !notFoundDependencies.isEmpty();
+        }
+
+        /**
          * Returns a list with dependencies required that were not found.
          *
          * @return a list with dependencies required that were not found
@@ -227,6 +236,15 @@ public class DependencyResolver {
         }
 
         /**
+         * Returns true if there are dependencies with wrong version.
+         *
+         * @return true if there are dependencies with wrong version
+         */
+        public boolean hasWrongVersionDependencies() {
+            return !wrongVersionDependencies.isEmpty();
+        }
+
+        /**
          * Returns a list with dependencies with wrong version.
          *
          * @return a list with dependencies with wrong version
@@ -236,6 +254,15 @@ public class DependencyResolver {
         }
 
         /**
+         * Returns true if the result is OK (no cyclic dependency, no not found dependencies, no wrong version dependencies).
+         *
+         * @return true if the result is OK
+         */
+        public boolean isOK() {
+            return !hasCyclicDependency() && !hasNotFoundDependencies() && !hasWrongVersionDependencies();
+        }
+
+        /**
          * Get the list of plugins in dependency sorted order.
          *
          * @return the list of plugins in dependency sorted order
@@ -333,7 +360,7 @@ public class DependencyResolver {
      */
     public static class DependenciesWrongVersionException extends PluginRuntimeException {
 
-        private List<WrongDependencyVersion> dependencies;
+        private final List<WrongDependencyVersion> dependencies;
 
         public DependenciesWrongVersionException(List<WrongDependencyVersion> dependencies) {
             super("Dependencies '{}' have wrong version", dependencies);
diff --git a/pf4j/src/main/java/org/pf4j/PluginNotFoundException.java b/pf4j/src/main/java/org/pf4j/PluginNotFoundException.java
new file mode 100644
index 0000000..003aaf6
--- /dev/null
+++ b/pf4j/src/main/java/org/pf4j/PluginNotFoundException.java
@@ -0,0 +1,37 @@
+/*
+ * Copyright (C) 2012-present the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.pf4j;
+
+/**
+ * Thrown when a plugin is not found.
+ *
+ * @author Decebal Suiu
+ */
+public class PluginNotFoundException extends PluginRuntimeException {
+
+    private final String pluginId;
+
+    public PluginNotFoundException(String pluginId) {
+        super("Plugin '{}' not found", pluginId);
+
+        this.pluginId = pluginId;
+    }
+
+    public String getPluginId() {
+        return pluginId;
+    }
+
+}
diff --git a/pf4j/src/test/java/org/pf4j/AbstractPluginManagerTest.java b/pf4j/src/test/java/org/pf4j/AbstractPluginManagerTest.java
index 70a9312..07a083b 100644
--- a/pf4j/src/test/java/org/pf4j/AbstractPluginManagerTest.java
+++ b/pf4j/src/test/java/org/pf4j/AbstractPluginManagerTest.java
@@ -18,12 +18,10 @@ package org.pf4j;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.io.TempDir;
 import org.pf4j.test.TestExtension;
 import org.pf4j.test.TestExtensionPoint;
 
 import java.io.IOException;
-import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -118,7 +116,7 @@ public class AbstractPluginManagerTest {
 
     @Test
     void checkNotExistedPluginId() {
-        assertThrows(IllegalArgumentException.class, () -> pluginManager.checkPluginId("plugin1"));
+        assertThrows(PluginNotFoundException.class, () -> pluginManager.checkPluginId("plugin1"));
     }
 
     private PluginWrapper createPluginWrapper(String pluginId) {
@@ -127,7 +125,7 @@ public class AbstractPluginManagerTest {
         pluginDescriptor.setPluginVersion("1.2.3");
 
         PluginWrapper pluginWrapper = new PluginWrapper(pluginManager, pluginDescriptor, Paths.get("plugin1"), getClass().getClassLoader());
-        Plugin plugin= mock(Plugin.class);
+        Plugin plugin = mock(Plugin.class);
         pluginWrapper.setPluginFactory(wrapper -> plugin);
 
         return pluginWrapper;
diff --git a/pf4j/src/test/java/org/pf4j/DefaultPluginManagerTest.java b/pf4j/src/test/java/org/pf4j/DefaultPluginManagerTest.java
index 87c5e3d..2e78ad2 100644
--- a/pf4j/src/test/java/org/pf4j/DefaultPluginManagerTest.java
+++ b/pf4j/src/test/java/org/pf4j/DefaultPluginManagerTest.java
@@ -28,8 +28,6 @@ import java.nio.file.Path;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertNotEquals;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -194,24 +192,122 @@ public class DefaultPluginManagerTest {
 
     @Test
     public void loadedPluginWithMissingDependencyCanBeUnloaded() throws IOException {
+        pluginManager.setResolveRecoveryStrategy(AbstractPluginManager.ResolveRecoveryStrategy.IGNORE_PLUGIN_AND_CONTINUE);
+
         PluginZip pluginZip = new PluginZip.Builder(pluginsPath.resolve("my-plugin-1.1.1.zip"), "myPlugin")
             .pluginVersion("1.1.1")
             .pluginDependencies("myBasePlugin")
             .build();
 
-        try {
-            pluginManager.loadPlugin(pluginZip.path());
-        } catch (DependencyResolver.DependenciesNotFoundException e) {
-            // expected
-        }
+        // try to load the plugin with a missing dependency
+        pluginManager.loadPlugin(pluginZip.path());
+
+        // the plugin is unloaded automatically
+        assertTrue(pluginManager.getPlugins().isEmpty());
+
+        // start plugins
+        pluginManager.startPlugins();
+
+        assertTrue(pluginManager.getStartedPlugins().isEmpty());
+        assertTrue(pluginManager.getUnresolvedPlugins().isEmpty());
+        assertTrue(pluginManager.getResolvedPlugins().isEmpty());
+        assertTrue(pluginManager.getPlugins().isEmpty());
+    }
+
+    @Test
+    void loadingPluginWithMissingDependencyDoesNotBreakOtherPlugins() throws IOException {
+        pluginManager.setResolveRecoveryStrategy(AbstractPluginManager.ResolveRecoveryStrategy.IGNORE_PLUGIN_AND_CONTINUE);
+
+        // Load 2 plugins, one with a dependency that is missing and one without any dependencies.
+        PluginZip pluginZip1 = new PluginZip.Builder(pluginsPath.resolve("my-first-plugin-1.1.1.zip"), "myPlugin1")
+            .pluginVersion("1.1.1")
+            .pluginDependencies("myBasePlugin")
+            .build();
+
+        PluginZip pluginZip2 = new PluginZip.Builder(pluginsPath.resolve("my-second-plugin-2.2.2.zip"), "myPlugin2")
+            .pluginVersion("2.2.2")
+            .build();
 
+        pluginManager.loadPlugins();
         pluginManager.startPlugins();
 
-        PluginWrapper myPlugin = pluginManager.getPlugin(pluginZip.pluginId());
-        assertNotNull(myPlugin);
-        assertNotEquals(PluginState.STARTED, myPlugin.getPluginState());
+        // myPlugin2 should have been started at this point.
+        assertEquals(PluginState.STARTED, pluginManager.getPlugin(pluginZip2.pluginId()).getPluginState());
+
+        pluginManager.unloadPlugin(pluginZip1.pluginId());
+
+        // No traces should remain of myPlugin1.
+        assertTrue(
+            pluginManager.getUnresolvedPlugins().stream()
+                .noneMatch(pluginWrapper -> pluginWrapper.getPluginId().equals(pluginZip1.pluginId()))
+        );
+
+        pluginManager.unloadPlugin(pluginZip2.pluginId());
+
+        // Load the missing dependency, everything should start working.
+        PluginZip pluginZipBase = new PluginZip.Builder(pluginsPath.resolve("my-base-plugin-3.0.0.zip"), "myBasePlugin")
+            .pluginVersion("3.0.0")
+            .build();
+
+        pluginManager.loadPlugins();
+        pluginManager.startPlugins();
+
+        assertEquals(PluginState.STARTED, pluginManager.getPlugin(pluginZip1.pluginId()).getPluginState());
+        assertEquals(PluginState.STARTED, pluginManager.getPlugin(pluginZip2.pluginId()).getPluginState());
+        assertEquals(PluginState.STARTED, pluginManager.getPlugin(pluginZipBase.pluginId()).getPluginState());
+
+        assertTrue(pluginManager.getUnresolvedPlugins().isEmpty());
+    }
+
+    @Test
+    void loadingPluginWithMissingDependencyFails() throws IOException {
+        pluginManager.setResolveRecoveryStrategy(AbstractPluginManager.ResolveRecoveryStrategy.THROW_EXCEPTION);
+
+        PluginZip pluginZip = new PluginZip.Builder(pluginsPath.resolve("my-plugin-1.1.1.zip"), "myPlugin")
+            .pluginVersion("1.1.1")
+            .pluginDependencies("myBasePlugin")
+            .build();
+
+        // try to load the plugin with a missing dependency
+        Path pluginPath = pluginZip.path();
+        assertThrows(DependencyResolver.DependenciesNotFoundException.class, () -> pluginManager.loadPlugin(pluginPath));
+    }
+
+    @Test
+    void loadingPluginWithWrongDependencyVersionFails() throws IOException {
+        pluginManager.setResolveRecoveryStrategy(AbstractPluginManager.ResolveRecoveryStrategy.THROW_EXCEPTION);
+
+        PluginZip pluginZip1 = new PluginZip.Builder(pluginsPath.resolve("my-first-plugin-1.1.1.zip"), "myPlugin1")
+            .pluginVersion("1.1.1")
+            .pluginDependencies("myPlugin2@3.0.0")
+            .build();
+
+        PluginZip pluginZip2 = new PluginZip.Builder(pluginsPath.resolve("my-second-plugin-2.2.2.zip"), "myPlugin2")
+            .pluginVersion("2.2.2")
+            .build();
+
+        // try to load the plugins with cyclic dependencies
+        Path pluginPath1 = pluginZip1.path();
+        Path pluginPath2 = pluginZip2.path();
+        assertThrows(DependencyResolver.DependenciesWrongVersionException.class, () -> pluginManager.loadPlugins());
+    }
+
+    @Test
+    void loadingPluginsWithCyclicDependenciesFails() throws IOException {
+        PluginZip pluginZip1 = new PluginZip.Builder(pluginsPath.resolve("my-first-plugin-1.1.1.zip"), "myPlugin1")
+            .pluginVersion("1.1.1")
+            .pluginDependencies("myPlugin2")
+            .build();
+
+        PluginZip pluginZip2 = new PluginZip.Builder(pluginsPath.resolve("my-second-plugin-2.2.2.zip"), "myPlugin2")
+            .pluginVersion("2.2.2")
+            .pluginDependencies("myPlugin1")
+            .build();
 
-        assertTrue(pluginManager.unloadPlugin(pluginZip.pluginId()));
+        // try to load the plugins with cyclic dependencies
+        Path pluginPath1 = pluginZip1.path();
+        Path pluginPath2 = pluginZip2.path();
+        assertThrows(DependencyResolver.CyclicDependencyException.class, () -> pluginManager.loadPlugins());
     }
 
 }