Skip to content
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

Make it easier to co-host zip and jar plugins in same system #143

Closed
janhoy opened this issue Apr 19, 2017 · 7 comments
Closed

Make it easier to co-host zip and jar plugins in same system #143

janhoy opened this issue Apr 19, 2017 · 7 comments

Comments

@janhoy
Copy link
Member

janhoy commented Apr 19, 2017

A host system may want to let developers choose between zip plugins or jar plugins. Or perhaps in the future Jigsaw module plugins. This is hard today since the distinction is on a PluginManager level (JarPluginManager).

Provide an out of the box SmartPluginLoader that can will decide based on path whether this is a jar plugin or an unpacked zip plugin and load using correct loader.
Also a SmartPluginDescriptorFinder that based on path will decide whether to use JarPluginDescriptorFinder, and in case of folder, will first try Manifest, then if exception fall back to properties.

I have done just that in my own code, but it would be better to have framework support?

@decebals
Copy link
Member

I think that it could be a solution but I have doubt that someone wish to use multiple plugin packaging systems (.JAR, .ZIP, ... => different PluginLoader, PluginDescrptorFinder, ...). But if it's a demand in this sense I am ready to add support for this feature in PF4J.

@janhoy
Copy link
Member Author

janhoy commented Apr 20, 2017

If it turns out to be useful for us, I'll contribute a solution later. For now I'll keep it custom and get some experience with it.

@janhoy
Copy link
Member Author

janhoy commented Sep 8, 2017

See https://github.com/cominvent/lucene-solr/blob/pf4j/solr/core/src/java/org/apache/solr/util/plugin/bundle/SolrPf4jPluginManager.java#L146 for how I solved it. Simply implementing a AutoPluginLoader, AutoPluginDescriptorFinder, and AutoPluginRepository to replace the default ones.

@decebals
Copy link
Member

decebals commented Sep 8, 2017

The solution is good for now but it's a little bit static for the future. The idea is to use a Compound(PluginLoader/DescriptorFinder/PluginRepository) and to pass the entries (a list with PluginLoader for example) in constructor. In CompundPluginLoader.loadPlugin(...) we can iterate over all entities. The missing piece is a new method boolean isApplicable(Path pluginPath) that returns true if a pluginPath can be loader/resolved by a PluginLoader.
My little pseudo code:

public CompoundPluginLoader implements PluginLoader {

    private List<PluginLoader> items;

    public CompoundPluginLoader(List<PluginLoader> items) {
        this.items = items;
    }

    @Override
    public boolean isApplicable(Path pluginPath) {
        for (PluginLoader item : items) {
            if (item.isApplicable(pluginPath)) {
                return true;
            }
        }

        return false;
    }

    @Override
    public ClassLoader loadPlugin(Path pluginPath, PluginDescriptor pluginDescriptor) {
        for (PluginLoader item : items) {
            if (item.isApplicable(pluginPath)) {
                return item.loadPlugin(pluginPath, pluginDescriptor);
            }
        }

        throw new PluginRuntimeException("Something is very bad"); // no happening
    }

}

It's only an idea.

@decebals
Copy link
Member

decebals commented Sep 26, 2017

The concept described by me in the previous comment is taken from Pippo, see explanations for MethodParmaterExtractor concept.
We can eliminate isApplicable method and sign that a PluginLoader is not applicable for a pluginPath returned null in loadPlugin method or throws an exception.
In this class above class becomes:

public CompoundPluginLoader implements PluginLoader {

    private List<PluginLoader> items;

    public CompoundPluginLoader(List<PluginLoader> items) {
        this.items = items;
    }

    @Override
    public ClassLoader loadPlugin(Path pluginPath, PluginDescriptor pluginDescriptor) {
        ClassLoader classLoader = null;
        for (PluginLoader item : items) {
            try {
                classLoader = item.loadPlugin(pluginPath, pluginDescriptor);
            } catch (Throwable t) {
                // ignore or log
            }

            if (classLoader != null) {
                return classLoader;
            }
        }

        throw new PluginRuntimeException("Something is very bad"); // no happening
    }

}

I see the same pattern applicable for PluginLoader, PluginDescriptorFinder and PluginRepository.
In this mode, the uses cases as described in #170 is very easy to implement. JarPluginManager will be useless because we already have that functionality included in DefaultPluginManager.

@decebals
Copy link
Member

decebals commented Oct 4, 2017

The CompoundPluginDescriptorFinder is already available on master.
The next step is to implement CompoundPluginLoader.

@decebals
Copy link
Member

decebals commented Oct 5, 2017

I finished the implementation. DefaultPluginManager has built-in support for Jar plugins (fat jars).
Now we can mix plugin repositories (zip, jars, directories, ...), plugin loaders, plugin finders.
I removed JarPluginManager because is obsolete.
I am very satisfied with the result. PF4J is very flexible, very customizable.

@decebals decebals closed this as completed Oct 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants