Browse Source

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

pull/387/head
Valeriy Kucherenko 4 years ago committed by GitHub
parent
commit
b72b75ae26
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 27
      pf4j/src/main/java/org/pf4j/AbstractPluginManager.java
  2. 38
      pf4j/src/main/java/org/pf4j/PluginClassLoader.java
  3. 6
      pf4j/src/main/java/org/pf4j/PluginWrapper.java

27
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 {

38
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)));
}

6
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;
}

Loading…
Cancel
Save