Fix wrong logical condition (optional dependencies are always skipped) (#386)

3 files changed, 35 insertions, 36 deletions

diff --git a/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java b/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java
index 13ae937..5b8b82c 100644
--- a/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java
+++ b/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java
@@ -15,6 +15,10 @@
  */
 package org.pf4j;
 
+import org.pf4j.util.StringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import java.io.Closeable;
 import java.io.IOException;
 import java.nio.file.Files;
@@ -28,10 +32,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
-import org.pf4j.util.StringUtils;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
 /**
  * This class implements the boilerplate plugin code that any {@link PluginManager}
  * implementation would have to support.
@@ -243,7 +243,7 @@ public abstract class AbstractPluginManager implements PluginManager {
         return unloadPlugin(pluginId, true);
     }
 
-    private boolean unloadPlugin(String pluginId, boolean unloadDependents) {
+    protected boolean unloadPlugin(String pluginId, boolean unloadDependents) {
         try {
             if (unloadDependents) {
                 List<String> dependents = dependencyResolver.getDependents(pluginId);
@@ -332,10 +332,10 @@ public abstract class AbstractPluginManager implements PluginManager {
                     pluginWrapper.setPluginState(PluginState.STARTED);
                     pluginWrapper.setFailedException(null);
                     startedPlugins.add(pluginWrapper);
-                } catch (Exception e) {
+                } catch (Exception | LinkageError e) {
                     pluginWrapper.setPluginState(PluginState.FAILED);
                     pluginWrapper.setFailedException(e);
-                    log.error(e.getMessage(), e);
+                    log.error("Unable to start plugin '{}'", getPluginLabel(pluginWrapper.getDescriptor()), e);
                 } finally {
                     firePluginStateEvent(new PluginStateEvent(this, pluginWrapper, pluginState));
                 }
@@ -371,7 +371,10 @@ public abstract class AbstractPluginManager implements PluginManager {
         }
 
         for (PluginDependency dependency : pluginDescriptor.getDependencies()) {
-            startPlugin(dependency.getPluginId());
+            // start dependency only if it marked as required (non optional) or if it optional and loaded
+            if (!dependency.isOptional() || plugins.containsKey(dependency.getPluginId())) {
+                startPlugin(dependency.getPluginId());
+            }
         }
 
         log.info("Start plugin '{}'", getPluginLabel(pluginDescriptor));
@@ -418,7 +421,7 @@ public abstract class AbstractPluginManager implements PluginManager {
         return stopPlugin(pluginId, true);
     }
 
-    private PluginState stopPlugin(String pluginId, boolean stopDependents) {
+    protected PluginState stopPlugin(String pluginId, boolean stopDependents) {
         checkPluginId(pluginId);
 
         PluginWrapper pluginWrapper = getPlugin(pluginId);
@@ -454,7 +457,7 @@ public abstract class AbstractPluginManager implements PluginManager {
         return pluginWrapper.getPluginState();
     }
 
-    private void checkPluginId(String pluginId) {
+    protected void checkPluginId(String pluginId) {
         if (!plugins.containsKey(pluginId)) {
             throw new IllegalArgumentException(String.format("Unknown pluginId %s", pluginId));
         }
@@ -913,7 +916,7 @@ public abstract class AbstractPluginManager implements PluginManager {
     }
 
     @SuppressWarnings("unchecked")
-    private <T> List<Class<? extends T>> getExtensionClasses(List<ExtensionWrapper<T>> extensionsWrapper) {
+    protected <T> List<Class<? extends T>> getExtensionClasses(List<ExtensionWrapper<T>> extensionsWrapper) {
         List<Class<? extends T>> extensionClasses = new ArrayList<>(extensionsWrapper.size());
         for (ExtensionWrapper<T> extensionWrapper : extensionsWrapper) {
             Class<T> c = (Class<T>) extensionWrapper.getDescriptor().extensionClass;
@@ -923,7 +926,7 @@ public abstract class AbstractPluginManager implements PluginManager {
         return extensionClasses;
     }
 
-    private <T> List<T> getExtensions(List<ExtensionWrapper<T>> extensionsWrapper) {
+    protected <T> List<T> getExtensions(List<ExtensionWrapper<T>> extensionsWrapper) {
         List<T> extensions = new ArrayList<>(extensionsWrapper.size());
         for (ExtensionWrapper<T> extensionWrapper : extensionsWrapper) {
             try {
diff --git a/pf4j/src/main/java/org/pf4j/PluginClassLoader.java b/pf4j/src/main/java/org/pf4j/PluginClassLoader.java
index 4cf59ae..6d609c1 100644
--- a/pf4j/src/main/java/org/pf4j/PluginClassLoader.java
+++ b/pf4j/src/main/java/org/pf4j/PluginClassLoader.java
@@ -15,6 +15,9 @@
  */
 package org.pf4j;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import java.io.File;
 import java.io.IOException;
 import java.net.URL;
@@ -26,9 +29,6 @@ import java.util.Enumeration;
 import java.util.List;
 import java.util.Objects;
 
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
 /**
  * One instance of this class should be created by plugin manager for every available plug-in.
  * By default, this class loader is a Parent Last ClassLoader - it loads the classes from the plugin's jars
@@ -177,7 +177,7 @@ public class PluginClassLoader extends URLClassLoader {
                 log.trace("Found resource '{}' in plugin classpath", name);
                 return url;
             }
-            
+
             url = findResourceFromDependencies(name);
             if (url != null) {
                 log.trace("Found resource '{}' in plugin dependencies", name);
@@ -197,41 +197,36 @@ public class PluginClassLoader extends URLClassLoader {
             log.trace("Couldn't find resource '{}' in parent", name);
 
             url = findResourceFromDependencies(name);
-           
+
             if (url != null) {
                log.trace("Found resource '{}' in dependencies", name);
                return url;
-            }  
-            
+            }
+
             return findResource(name);
         }
     }
 
     @Override
-    public Enumeration<URL> getResources(String name) throws IOException {  
+    public Enumeration<URL> getResources(String name) throws IOException {
     	List<URL> resources = new ArrayList<>();
 
     	if (!parentFirst) {
-            
             resources.addAll(Collections.list(findResources(name)));
-
             resources.addAll(findResourcesFromDependencies(name));
-            
+
             if (getParent() != null) {
                 resources.addAll(Collections.list(getParent().getResources(name)));
             }
-
         } else {
-        	
         	if (getParent() != null) {
                 resources.addAll(Collections.list(getParent().getResources(name)));
             }
-        	
+
         	resources.addAll(findResourcesFromDependencies(name));
-        	
         	resources.addAll(Collections.list(super.findResources(name)));
         }
-    	
+
     	return Collections.enumeration(resources);
     }
 
@@ -242,7 +237,7 @@ public class PluginClassLoader extends URLClassLoader {
             ClassLoader classLoader = pluginManager.getPluginClassLoader(dependency.getPluginId());
 
             // If the dependency is marked as optional, its class loader might not be available.
-            if (classLoader == null || dependency.isOptional()) {
+            if (classLoader == null && dependency.isOptional()) {
                 continue;
             }
 
@@ -255,7 +250,7 @@ public class PluginClassLoader extends URLClassLoader {
 
         return null;
     }
-    
+
     private URL findResourceFromDependencies(String name) {
         log.trace("Search in dependencies for resource '{}'", name);
         List<PluginDependency> dependencies = pluginDescriptor.getDependencies();
@@ -263,7 +258,7 @@ public class PluginClassLoader extends URLClassLoader {
             PluginClassLoader classLoader = (PluginClassLoader) pluginManager.getPluginClassLoader(dependency.getPluginId());
 
             // If the dependency is marked as optional, its class loader might not be available.
-            if (classLoader == null || dependency.isOptional()) {
+            if (classLoader == null && dependency.isOptional()) {
                 continue;
             }
 
@@ -275,7 +270,7 @@ public class PluginClassLoader extends URLClassLoader {
 
         return null;
     }
-    
+
     private Collection<URL> findResourcesFromDependencies(String name) throws IOException {
         log.trace("Search in dependencies for resources '{}'", name);
         List<URL> results = new ArrayList<>();
@@ -284,9 +279,10 @@ public class PluginClassLoader extends URLClassLoader {
             PluginClassLoader classLoader = (PluginClassLoader) pluginManager.getPluginClassLoader(dependency.getPluginId());
 
             // If the dependency is marked as optional, its class loader might not be available.
-            if (classLoader == null || dependency.isOptional()) {
+            if (classLoader == null && dependency.isOptional()) {
                 continue;
             }
+
             results.addAll(Collections.list(classLoader.findResources(name)));
         }
 
diff --git a/pf4j/src/main/java/org/pf4j/PluginWrapper.java b/pf4j/src/main/java/org/pf4j/PluginWrapper.java
index 8e472f8..64a14d2 100644
--- a/pf4j/src/main/java/org/pf4j/PluginWrapper.java
+++ b/pf4j/src/main/java/org/pf4j/PluginWrapper.java
@@ -32,7 +32,7 @@ public class PluginWrapper {
     private PluginState pluginState;
     private RuntimeMode runtimeMode;
 
-    private Exception failedException;
+    private Throwable failedException;
 
     Plugin plugin; // cache
 
@@ -145,11 +145,11 @@ public class PluginWrapper {
      * Returns the exception with which the plugin fails to start.
      * See @{link PluginStatus#FAILED}.
      */
-    public Exception getFailedException() {
+    public Throwable getFailedException() {
         return failedException;
     }
 
-    public void setFailedException(Exception failedException) {
+    public void setFailedException(Throwable failedException) {
         this.failedException = failedException;
     }