Browse Source

PluginClassLoader does not resolve classpath resources from plugin dependencies (#365)

pull/370/head
Lee David Painter 5 years ago committed by GitHub
parent
commit
2f8343cfda
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 8
      pf4j/src/main/java/org/pf4j/AbstractPluginManager.java
  2. 83
      pf4j/src/main/java/org/pf4j/PluginClassLoader.java
  3. 127
      pf4j/src/test/java/org/pf4j/PluginClassLoaderTest.java

8
pf4j/src/main/java/org/pf4j/AbstractPluginManager.java

@ -15,10 +15,6 @@
*/ */
package org.pf4j; package org.pf4j;
import org.pf4j.util.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.Closeable; import java.io.Closeable;
import java.io.IOException; import java.io.IOException;
import java.nio.file.Files; import java.nio.file.Files;
@ -32,6 +28,10 @@ import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Set; 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} * This class implements the boilerplate plugin code that any {@link PluginManager}
* implementation would have to support. * implementation would have to support.

83
pf4j/src/main/java/org/pf4j/PluginClassLoader.java

@ -15,17 +15,19 @@
*/ */
package org.pf4j; package org.pf4j;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.File; import java.io.File;
import java.io.IOException; import java.io.IOException;
import java.net.URL; import java.net.URL;
import java.net.URLClassLoader; import java.net.URLClassLoader;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections; import java.util.Collections;
import java.util.Enumeration; import java.util.Enumeration;
import java.util.List; 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. * One instance of this class should be created by plugin manager for every available plug-in.
@ -175,8 +177,14 @@ public class PluginClassLoader extends URLClassLoader {
log.trace("Found resource '{}' in plugin classpath", name); log.trace("Found resource '{}' in plugin classpath", name);
return url; return url;
} }
url = findResourceFromDependencies(name);
if (url != null) {
log.trace("Found resource '{}' in plugin dependencies", name);
return url;
}
log.trace("Couldn't find resource '{}' in plugin classpath. Delegating to parent", name); log.trace("Couldn't find resource '{}' in plugin or dependencies classpath. Delegating to parent", name);
return super.getResource(name); return super.getResource(name);
} else { } else {
@ -188,25 +196,43 @@ public class PluginClassLoader extends URLClassLoader {
log.trace("Couldn't find resource '{}' in parent", name); 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); return findResource(name);
} }
} }
@Override @Override
public Enumeration<URL> getResources(String name) throws IOException { public Enumeration<URL> getResources(String name) throws IOException {
if (!parentFirst) { List<URL> resources = new ArrayList<>();
List<URL> resources = new ArrayList<>();
if (!parentFirst) {
resources.addAll(Collections.list(findResources(name))); resources.addAll(Collections.list(findResources(name)));
resources.addAll(findResourcesFromDependencies(name));
if (getParent() != null) { if (getParent() != null) {
resources.addAll(Collections.list(getParent().getResources(name))); resources.addAll(Collections.list(getParent().getResources(name)));
} }
return Collections.enumeration(resources);
} else { } else {
return super.getResources(name);
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);
} }
private Class<?> loadClassFromDependencies(String className) { private Class<?> loadClassFromDependencies(String className) {
@ -216,7 +242,7 @@ public class PluginClassLoader extends URLClassLoader {
ClassLoader classLoader = pluginManager.getPluginClassLoader(dependency.getPluginId()); ClassLoader classLoader = pluginManager.getPluginClassLoader(dependency.getPluginId());
// If the dependency is marked as optional, its class loader might not be available. // 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; continue;
} }
@ -229,5 +255,42 @@ public class PluginClassLoader extends URLClassLoader {
return null; return null;
} }
private URL findResourceFromDependencies(String name) {
log.trace("Search in dependencies for resource '{}'", name);
List<PluginDependency> dependencies = pluginDescriptor.getDependencies();
for (PluginDependency dependency : dependencies) {
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()) {
continue;
}
URL url = classLoader.findResource(name);
if (Objects.nonNull(url)) {
return url;
}
}
return null;
}
private Collection<URL> findResourcesFromDependencies(String name) throws IOException {
log.trace("Search in dependencies for resources '{}'", name);
List<URL> results = new ArrayList<>();
List<PluginDependency> dependencies = pluginDescriptor.getDependencies();
for (PluginDependency dependency : dependencies) {
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()) {
continue;
}
results.addAll(Collections.list(classLoader.findResources(name)));
}
return results;
}
} }

127
pf4j/src/test/java/org/pf4j/PluginClassLoaderTest.java

@ -42,11 +42,16 @@ import static org.junit.jupiter.api.Assertions.*;
*/ */
public class PluginClassLoaderTest { public class PluginClassLoaderTest {
private DefaultPluginManager pluginManager; private TestPluginManager pluginManager;
private TestPluginManager pluginManagerParentFirst;
private DefaultPluginDescriptor pluginDependencyDescriptor;
private DefaultPluginDescriptor pluginDescriptor; private DefaultPluginDescriptor pluginDescriptor;
private PluginClassLoader parentLastPluginClassLoader; private PluginClassLoader parentLastPluginClassLoader;
private PluginClassLoader parentFirstPluginClassLoader; private PluginClassLoader parentFirstPluginClassLoader;
private PluginClassLoader parentLastPluginDependencyClassLoader;
private PluginClassLoader parentFirstPluginDependencyClassLoader;
@TempDir @TempDir
Path pluginsPath; Path pluginsPath;
@ -62,6 +67,8 @@ public class PluginClassLoaderTest {
} }
createFile(parentClassPathBase.resolve("META-INF").resolve("file-only-in-parent")); createFile(parentClassPathBase.resolve("META-INF").resolve("file-only-in-parent"));
createFile(parentClassPathBase.resolve("META-INF").resolve("file-in-both-parent-and-dependency-and-plugin"));
createFile(parentClassPathBase.resolve("META-INF").resolve("file-in-both-parent-and-dependency"));
createFile(parentClassPathBase.resolve("META-INF").resolve("file-in-both-parent-and-plugin")); createFile(parentClassPathBase.resolve("META-INF").resolve("file-in-both-parent-and-plugin"));
} }
@ -69,7 +76,7 @@ public class PluginClassLoaderTest {
File file = pathToFile.toFile(); File file = pathToFile.toFile();
file.deleteOnExit(); file.deleteOnExit();
assertTrue(file.createNewFile(), "failed to create '" + pathToFile + "'"); assertTrue(file.exists() || file.createNewFile(), "failed to create '" + pathToFile + "'");
try (PrintWriter printWriter = new PrintWriter(file)) { try (PrintWriter printWriter = new PrintWriter(file)) {
printWriter.write("parent"); printWriter.write("parent");
} }
@ -77,13 +84,57 @@ public class PluginClassLoaderTest {
@BeforeEach @BeforeEach
void setUp() throws IOException { void setUp() throws IOException {
pluginManager = new DefaultPluginManager(pluginsPath); pluginManager = new TestPluginManager(pluginsPath);
pluginManagerParentFirst = new TestPluginManager(pluginsPath);
pluginDependencyDescriptor = new DefaultPluginDescriptor();
pluginDependencyDescriptor.setPluginId("myDependency");
pluginDependencyDescriptor.setPluginVersion("1.2.3");
pluginDependencyDescriptor.setPluginDescription("My plugin");
pluginDependencyDescriptor.setDependencies("");
pluginDependencyDescriptor.setProvider("Me");
pluginDependencyDescriptor.setRequires("5.0.0");
Path pluginDependencyPath = pluginsPath.resolve(pluginDependencyDescriptor.getPluginId() + "-" + pluginDependencyDescriptor.getVersion() + ".zip");
PluginZip pluginDependencyZip = new PluginZip.Builder(pluginDependencyPath, pluginDependencyDescriptor.getPluginId())
.pluginVersion(pluginDependencyDescriptor.getVersion())
.addFile(Paths.get("classes/META-INF/dependency-file"), "dependency")
.addFile(Paths.get("classes/META-INF/file-in-both-parent-and-dependency-and-plugin"), "dependency")
.addFile(Paths.get("classes/META-INF/file-in-both-parent-and-dependency"), "dependency")
.build();
FileUtils.expandIfZip(pluginDependencyZip.path());
PluginClasspath pluginDependencyClasspath = new DefaultPluginClasspath();
parentLastPluginDependencyClassLoader = new PluginClassLoader(pluginManager, pluginDependencyDescriptor, PluginClassLoaderTest.class.getClassLoader());
parentFirstPluginDependencyClassLoader = new PluginClassLoader(pluginManagerParentFirst, pluginDependencyDescriptor, PluginClassLoaderTest.class.getClassLoader(), true);
pluginManager.addClassLoader(pluginDependencyDescriptor.getPluginId(), parentLastPluginDependencyClassLoader);
pluginManagerParentFirst.addClassLoader(pluginDependencyDescriptor.getPluginId(), parentFirstPluginDependencyClassLoader);
for (String classesDirectory : pluginDependencyClasspath.getClassesDirectories()) {
File classesDirectoryFile = pluginDependencyZip.unzippedPath().resolve(classesDirectory).toFile();
parentLastPluginDependencyClassLoader.addFile(classesDirectoryFile);
parentFirstPluginDependencyClassLoader.addFile(classesDirectoryFile);
}
for (String jarsDirectory : pluginDependencyClasspath.getJarsDirectories()) {
Path jarsDirectoryPath = pluginDependencyZip.unzippedPath().resolve(jarsDirectory);
List<File> jars = FileUtils.getJars(jarsDirectoryPath);
for (File jar : jars) {
parentLastPluginDependencyClassLoader.addFile(jar);
parentFirstPluginDependencyClassLoader.addFile(jar);
}
}
pluginDescriptor = new DefaultPluginDescriptor(); pluginDescriptor = new DefaultPluginDescriptor();
pluginDescriptor.setPluginId("myPlugin"); pluginDescriptor.setPluginId("myPlugin");
pluginDescriptor.setPluginVersion("1.2.3"); pluginDescriptor.setPluginVersion("1.2.3");
pluginDescriptor.setPluginDescription("My plugin"); pluginDescriptor.setPluginDescription("My plugin");
pluginDescriptor.setDependencies("bar, baz"); pluginDescriptor.setDependencies("myDependency");
pluginDescriptor.setProvider("Me"); pluginDescriptor.setProvider("Me");
pluginDescriptor.setRequires("5.0.0"); pluginDescriptor.setRequires("5.0.0");
@ -91,6 +142,7 @@ public class PluginClassLoaderTest {
PluginZip pluginZip = new PluginZip.Builder(pluginPath, pluginDescriptor.getPluginId()) PluginZip pluginZip = new PluginZip.Builder(pluginPath, pluginDescriptor.getPluginId())
.pluginVersion(pluginDescriptor.getVersion()) .pluginVersion(pluginDescriptor.getVersion())
.addFile(Paths.get("classes/META-INF/plugin-file"), "plugin") .addFile(Paths.get("classes/META-INF/plugin-file"), "plugin")
.addFile(Paths.get("classes/META-INF/file-in-both-parent-and-dependency-and-plugin"), "plugin")
.addFile(Paths.get("classes/META-INF/file-in-both-parent-and-plugin"), "plugin") .addFile(Paths.get("classes/META-INF/file-in-both-parent-and-plugin"), "plugin")
.build(); .build();
@ -101,6 +153,9 @@ public class PluginClassLoaderTest {
parentLastPluginClassLoader = new PluginClassLoader(pluginManager, pluginDescriptor, PluginClassLoaderTest.class.getClassLoader()); parentLastPluginClassLoader = new PluginClassLoader(pluginManager, pluginDescriptor, PluginClassLoaderTest.class.getClassLoader());
parentFirstPluginClassLoader = new PluginClassLoader(pluginManager, pluginDescriptor, PluginClassLoaderTest.class.getClassLoader(), true); parentFirstPluginClassLoader = new PluginClassLoader(pluginManager, pluginDescriptor, PluginClassLoaderTest.class.getClassLoader(), true);
pluginManager.addClassLoader(pluginDescriptor.getPluginId(), parentLastPluginClassLoader);
pluginManagerParentFirst.addClassLoader(pluginDescriptor.getPluginId(), parentFirstPluginClassLoader);
for (String classesDirectory : pluginClasspath.getClassesDirectories()) { for (String classesDirectory : pluginClasspath.getClassesDirectories()) {
File classesDirectoryFile = pluginZip.unzippedPath().resolve(classesDirectory).toFile(); File classesDirectoryFile = pluginZip.unzippedPath().resolve(classesDirectory).toFile();
parentLastPluginClassLoader.addFile(classesDirectoryFile); parentLastPluginClassLoader.addFile(classesDirectoryFile);
@ -120,7 +175,10 @@ public class PluginClassLoaderTest {
@AfterEach @AfterEach
void tearDown() { void tearDown() {
pluginManager = null; pluginManager = null;
pluginDependencyDescriptor = null;
pluginDescriptor = null; pluginDescriptor = null;
parentLastPluginClassLoader = null;
parentFirstPluginClassLoader = null;
} }
@Test @Test
@ -138,7 +196,7 @@ public class PluginClassLoaderTest {
URL resource = parentLastPluginClassLoader.getResource("META-INF/file-only-in-parent"); URL resource = parentLastPluginClassLoader.getResource("META-INF/file-only-in-parent");
assertFirstLine("parent", resource); assertFirstLine("parent", resource);
} }
@Test @Test
void parentFirstGetResourceExistsInParent() throws IOException, URISyntaxException { void parentFirstGetResourceExistsInParent() throws IOException, URISyntaxException {
URL resource = parentFirstPluginClassLoader.getResource("META-INF/file-only-in-parent"); URL resource = parentFirstPluginClassLoader.getResource("META-INF/file-only-in-parent");
@ -156,6 +214,18 @@ public class PluginClassLoaderTest {
URL resource = parentFirstPluginClassLoader.getResource("META-INF/plugin-file"); URL resource = parentFirstPluginClassLoader.getResource("META-INF/plugin-file");
assertFirstLine("plugin", resource); assertFirstLine("plugin", resource);
} }
@Test
void parentLastGetResourceExistsOnlyInDependnecy() throws IOException, URISyntaxException {
URL resource = parentLastPluginClassLoader.getResource("META-INF/dependency-file");
assertFirstLine("dependency", resource);
}
@Test
void parentFirstGetResourceExistsOnlyInDependency() throws IOException, URISyntaxException {
URL resource = parentFirstPluginClassLoader.getResource("META-INF/dependency-file");
assertFirstLine("dependency", resource);
}
@Test @Test
void parentLastGetResourceExistsInBothParentAndPlugin() throws URISyntaxException, IOException { void parentLastGetResourceExistsInBothParentAndPlugin() throws URISyntaxException, IOException {
@ -168,6 +238,18 @@ public class PluginClassLoaderTest {
URL resource = parentFirstPluginClassLoader.getResource("META-INF/file-in-both-parent-and-plugin"); URL resource = parentFirstPluginClassLoader.getResource("META-INF/file-in-both-parent-and-plugin");
assertFirstLine("parent", resource); assertFirstLine("parent", resource);
} }
@Test
void parentLastGetResourceExistsInParentAndDependencyAndPlugin() throws URISyntaxException, IOException {
URL resource = parentLastPluginClassLoader.getResource("META-INF/file-in-both-parent-and-dependency-and-plugin");
assertFirstLine("plugin", resource);
}
@Test
void parentFirstGetResourceExistsInParentAndDependencyAndPlugin() throws URISyntaxException, IOException {
URL resource = parentFirstPluginClassLoader.getResource("META-INF/file-in-both-parent-and-dependency-and-plugin");
assertFirstLine("parent", resource);
}
@Test @Test
void parentLastGetResourcesNonExisting() throws IOException { void parentLastGetResourcesNonExisting() throws IOException {
@ -191,6 +273,18 @@ public class PluginClassLoaderTest {
assertNumberOfResourcesAndFirstLineOfFirstElement(1, "parent", resources); assertNumberOfResourcesAndFirstLineOfFirstElement(1, "parent", resources);
} }
@Test
void parentLastGetResourcesExistsOnlyInDependency() throws IOException, URISyntaxException {
Enumeration<URL> resources = parentLastPluginClassLoader.getResources("META-INF/dependency-file");
assertNumberOfResourcesAndFirstLineOfFirstElement(1, "dependency", resources);
}
@Test
void parentFirstGetResourcesExistsOnlyInDependency() throws IOException, URISyntaxException {
Enumeration<URL> resources = parentFirstPluginClassLoader.getResources("META-INF/dependency-file");
assertNumberOfResourcesAndFirstLineOfFirstElement(1, "dependency", resources);
}
@Test @Test
void parentLastGetResourcesExistsOnlyInPlugin() throws IOException, URISyntaxException { void parentLastGetResourcesExistsOnlyInPlugin() throws IOException, URISyntaxException {
Enumeration<URL> resources = parentLastPluginClassLoader.getResources("META-INF/plugin-file"); Enumeration<URL> resources = parentLastPluginClassLoader.getResources("META-INF/plugin-file");
@ -214,6 +308,18 @@ public class PluginClassLoaderTest {
Enumeration<URL> resources = parentFirstPluginClassLoader.getResources("META-INF/file-in-both-parent-and-plugin"); Enumeration<URL> resources = parentFirstPluginClassLoader.getResources("META-INF/file-in-both-parent-and-plugin");
assertNumberOfResourcesAndFirstLineOfFirstElement(2, "parent", resources); assertNumberOfResourcesAndFirstLineOfFirstElement(2, "parent", resources);
} }
@Test
void parentLastGetResourcesExistsInParentAndDependencyAndPlugin() throws URISyntaxException, IOException {
Enumeration<URL> resources = parentLastPluginClassLoader.getResources("META-INF/file-in-both-parent-and-dependency-and-plugin");
assertNumberOfResourcesAndFirstLineOfFirstElement(3, "plugin", resources);
}
@Test
void parentFirstGetResourcesExistsInParentAndDependencyAndPlugin() throws URISyntaxException, IOException {
Enumeration<URL> resources = parentFirstPluginClassLoader.getResources("META-INF/file-in-both-parent-and-dependency-and-plugin");
assertNumberOfResourcesAndFirstLineOfFirstElement(3, "parent", resources);
}
private static void assertFirstLine(String expected, URL resource) throws URISyntaxException, IOException { private static void assertFirstLine(String expected, URL resource) throws URISyntaxException, IOException {
assertNotNull(resource); assertNotNull(resource);
@ -227,4 +333,15 @@ public class PluginClassLoaderTest {
URL firstResource = list.get(0); URL firstResource = list.get(0);
assertEquals(expectedFirstLine, Files.readAllLines(Paths.get(firstResource.toURI())).get(0)); assertEquals(expectedFirstLine, Files.readAllLines(Paths.get(firstResource.toURI())).get(0));
} }
class TestPluginManager extends DefaultPluginManager {
public TestPluginManager(Path pluginsPath) {
super(pluginsPath);
}
void addClassLoader(String pluginId, PluginClassLoader classLoader) {
getPluginClassLoaders().put(pluginId, classLoader);
}
}
} }

Loading…
Cancel
Save