-
-
Notifications
You must be signed in to change notification settings - Fork 565
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
added possibility to overwrite DefaultPluginManager (to create f.i. a JarPluginManager) #66
Conversation
* added possibility to overwrite DefaultPluginManager (to create f.i. a JarPluginManager)
* Returns the PluginClasspath used by this plugin manager | ||
* @return the classpath used by this manager | ||
*/ | ||
protected PluginClasspath getPluginClasspath() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that we need this method. From your gist code I see that you use this method in
protected PluginDescriptorFinder createPluginDescriptorFinder() {
if (RuntimeMode.DEVELOPMENT.equals(getRuntimeMode())) {
return new PropertiesPluginDescriptorFinder();
}
return new DefaultPluginDescriptorFinder(getPluginClasspath());
}
but your code is exactly the same with the default implementation:
protected PluginDescriptorFinder createPluginDescriptorFinder() {
if (RuntimeMode.DEVELOPMENT.equals(getRuntimeMode())) {
return new PropertiesPluginDescriptorFinder();
}
return new DefaultPluginDescriptorFinder(pluginClasspath);
}
Other comments for your gist.
public class JarPluginClasspath extends PluginClasspath {
public JarPluginClasspath() {
super();
}
protected void addResources() {
classesDirectories.add(".");
}
} |
To be honest I don't see the value for a big jar plugin (jar with dependencies) versus a zip plugin (with the default layout: |
You're absolutely right. It's enough if pluginsDirectory is protected. That would be sufficient for overwriting the DefaultPluginManager. The method "getPluginClasspath()" is also not required. It's a try&error-relict while trying to hack the JarPluginManager. Sorry for the confusion. I updated the gist sample files + added a partial pom.xml for a plugin.
The advantage is:
which enables you to use the plugin, f.i. in another assembly (which packages the main application plus required packages into a distribution zip) like this: assembly.xml
With that, I'm now in the position of specifiying the version of my main-application plus the factory-default-plugins with its version in the pom of the "distribution" project and bundle all the stuff together to big distribution zip. Maybe there are other ways of handling .zip artifacts of projects and bundle them together with maven assembly plugin. But that's the default way the maven documentation describes. Beside that: The ZIP files worked well. I don't see the requirement of having the JarPluginManager in PF4J project. But there should be the option to overwrite the DefaultPluginManager to create a custom manager. |
Probably the best option is to extend |
If you need more help please tell me. Don't forget to close this PR if everything is OK. |
That was also my feeling. Maybe it's also a good idea to set a few other fields to "protected" instead of private? Otherwise overwriting other createXYZ methods could result in the same issue... For me and my "special case" it's okay now. Thanks for changing the code. |
Sure, I will do this step by step (on request). |
I'm working on an JarPluginManager which does not need to extract the jar archive. Will see if this works and what else needs to be changed in visibility. |
To create an "JarPluginManager" that uses JARs (f.i. created with Maven Assembly "jar-with-dependencies") instead of ZIP, I needed to add two get-methods to DefaultPluginManager.
And while adding those two get-methods, I also updated javadoc to keep maven javadoc generation happy.
Here is my JarPluginManager implementation incl. demo program:
https://gist.github.com/tuxedo0801/0bc4ad265131b2d46643
Instead of the long configuration part in plugin's pom,xml + assembly xml, you now only need this in the pom (just an example):
That creates an JAR archive with "-jar-with-dependencies.jar" suffix/extension which includes all classes from dependencies.
One could also think of skipping the plugin-extraction-process and just pass the JAR to the classloader. But for improved startup-speed on small systems (Raspberry Pi f.i.) it could be a good idea to extract the JAR.