From b72b75ae26e4bbab7ce4a597484da4494825b655 Mon Sep 17 00:00:00 2001 From: Valeriy Kucherenko Date: Fri, 17 Jul 2020 13:24:02 +0300 Subject: [PATCH] Fix wrong logical condition (optional dependencies are always skipped) (#386) --- .../java/org/pf4j/AbstractPluginManager.java | 27 +++++++------ .../main/java/org/pf4j/PluginClassLoader.java | 38 +++++++++---------- .../src/main/java/org/pf4j/PluginWrapper.java | 6 +-- 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 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 List> getExtensionClasses(List> extensionsWrapper) { + protected List> getExtensionClasses(List> extensionsWrapper) { List> extensionClasses = new ArrayList<>(extensionsWrapper.size()); for (ExtensionWrapper extensionWrapper : extensionsWrapper) { Class c = (Class) extensionWrapper.getDescriptor().extensionClass; @@ -923,7 +926,7 @@ public abstract class AbstractPluginManager implements PluginManager { return extensionClasses; } - private List getExtensions(List> extensionsWrapper) { + protected List getExtensions(List> extensionsWrapper) { List extensions = new ArrayList<>(extensionsWrapper.size()); for (ExtensionWrapper 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 getResources(String name) throws IOException { + public Enumeration getResources(String name) throws IOException { List 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 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 findResourcesFromDependencies(String name) throws IOException { log.trace("Search in dependencies for resources '{}'", name); List 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; } -- 2.39.5