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

Support any interface as an ExtensionPoint #289

Closed
dmitry-timofeev opened this issue Mar 1, 2019 · 13 comments
Closed

Support any interface as an ExtensionPoint #289

dmitry-timofeev opened this issue Mar 1, 2019 · 13 comments

Comments

@dmitry-timofeev
Copy link
Contributor

Currently a plugin cannot provide an extension implementing an interface that does not extend ExtensionPoint. That prevents the applications from using the interfaces they do not control as extension points (e.g., from the standard library or from any third-party library).

For example, if one needs to provide an implementation of HashFunction interface from Guava, they could write

@Extension
public final class MyHashFunction implements HashFunction {
 …
}

A similar functionality is allowed in OSGi declarative services with @Component annotation.

Would it be possible to support any interfaces as extension points?

@dmitry-timofeev dmitry-timofeev mentioned this issue Mar 1, 2019
3 tasks
@pinhead84
Copy link
Contributor

pinhead84 commented Mar 2, 2019

As far as I understand, the @Extension annotation is only used by the ExtensionAnnotationProcessor at compile time in order to avoid writing META-INF/extensions.idx or META-INF/services files manually. It is a convenience feature, that I don't really like to miss. ;)

Have you already tested to add an extension without this annotation to META-INF/extensions.idx or META-INF/services? - I'm not 100% sure about it. But PF4J should be able to load those extensions without the annotation, if you register these extensions manually.


Edit: Sorry, I guess, that I understood your question wrong. I guess my answer points to the wrong direction...

@decebals
Copy link
Member

decebals commented Mar 2, 2019

The idea with the interface marker ExtensionPoint is that I want:

  • to have a strict control in my application/plugins, where I can "inject" extensions. The list of extensions points is defined with care in the modularization process of application. I do not think that I want any interface or abstract class to be a possible extension point.
  • to have the possibility to create automatically a documentation for a plugin where to specify what extensions points and extensions provides that plugin. I think that this aspect is not covered well in the current version of PF4J and may be it's a subject for the PF4J 3.x roadmap (PF4J 3 roadmap #282). For example I think that it will be nice to have an extension-points.idx file, similar with extensions.idx, where all extensions points to be listed. Also, I think that it will be nice to include the extension point (class name) for each extension listed currently in extensions.idx, in this mode we will avoid the reflection in the ExtensionFinder implementations supplied by PF4J.

On the other hand, if you insist to use any interface or abstract class as an extension point, without to extends the marker interface ExtensionPoint (the use case from issue's description), I think that you can do it very well with what we have already.
For example, in the ExtensionFinder implementations that come with PF4J, we didn't check that an extension point extends the ExtensionPoint interface. The only one place where this check is made is in ExtensionAnnotationProcessor (see isExtension and getExtensionPointType). Maybe we can add a parameter (optional) that can disable the check.

@dmitry-timofeev
Copy link
Contributor Author

Thanks for the responses,

I see, extending the ExtensionPoint interface definitily makes the provided extension points more explicit in the application code. But in cases where the app does not control the interface it needs to have as an extension point, it requires extending that interface just to be able to use it as an extension point. Or, if the interface in a separate module from the application, adds an additional dependency on PF4J just to get to ExtensionPoint.

Regarding the control over available extensions: isn't it true that the application already controls which extensions a plugin can provide by explicitly invoking PluginManager#getExtensions methods? Or is there some mechanism that will instantiate any extension in a plugin without the application request (all of them)?

An automatic documentation (something like Foo implements Bar) sounds cool! Will allowing the users to specify any interface as an implemented extension point make that harder to achieve?

I think it would also be nice if this feature, if implemented, requires as little configuration as possible and is easy to use.

@decebals
Copy link
Member

decebals commented Nov 15, 2019

@dmitry-timofeev
I think that I solved this feature request (see PR #350). I published a new SNAPSHOT that contains this feature.
Can you test it please?
If you use Maven in your project(s), please take a look at doc to find how to use SNAPSHOT in your project(s).

@dmitry-timofeev
Copy link
Contributor Author

dmitry-timofeev commented Nov 20, 2019

Hi, @decebals,
Thank you for the feature!

I tried it, and bumped into a limitation that indirectly implemented interfaces (e.g., through a super class) cannot be used as an extension point.

Would it make sense to allow explicit specification to support this case? Currently it is not possible because Extension#points accepts only classes implementing ExtensionPoint:

/* Doesn’t work unless HashFunction < ExtensionPoint */
@Extension(points = HashFunction.class)
public final class MyHashFunction extends AbstractHashFunction {
 …
}

That would also allow multiple interfaces (whether directly implemented by a class, or indirectly):

@Extension(points = {Foo.class, Bar.class})
public final class MyFooBar implements Foo, Bar {
 …
}

Also, would it be reasonable to just register an extension for all directly implemented classes; and require the explicitly specified points when some are indirectly present (through superclasses or superinterfaces)?

@dmitry-timofeev
Copy link
Contributor Author

Is the compiler argument a temporary switch, or will it remain required?

@decebals
Copy link
Member

decebals commented Nov 20, 2019

@dmitry-timofeev

Would it make sense to allow explicit specification to support this case? Currently it is not possible because Extension#points accepts only classes implementing ExtensionPoint:

If I relax a little bit the type for points field, then everything is OK (both scenarios described by you).

image

But for the moment, I will consider this case (with some values for Extension#points), a particular situation and I will not relax (yet) the type of points field.
We will see in the next versions if we can improve the situation. The idea is that I don't want to obtain the same result using two different methods (an extension point would extends or not ExtensionPoint marker interface). In the end what does ExtensionPoint put on table if we allow any interface as an ExtensionPoint? I think that in this situation, ExtensionPoint is useless as concept.

Also, would it be reasonable to just register an extension for all directly implemented classes; and require the explicitly specified points when some are indirectly present (through superclasses or superinterfaces)?

Yes. I think so. But in my projects I didn't encounter situations that require a feature described by you in this issue. Maybe you can answer to this question. Is it enough for you the current implementation that already exists on master (without support for Extension#point)?

@decebals
Copy link
Member

@dmitry-timofeev

Is the compiler argument a temporary switch, or will it remain required?

I opt for the current version (compiler argument). The idea is to not add extra work/check in processor. Sure, if other people have other opinions we can change this behavior.

@dmitry-timofeev
Copy link
Contributor Author

I also don't see the immediate need for registering an Extension for multiple directly present interfaces automatically. There are no such scenarious in our project.

Regarding the indirectly present interfaces (either a single one or multiple), I think it would be useful. I agree that the place of ExtensionPoint interface in this case shall likely be reconsidered, if it's no longer used to define an extension point.

@decebals
Copy link
Member

@dmitry-timofeev
My intention is to release a new version soon, that mainly will contain this feature and #347. In this context, do you think this problem is solved (with what we have on the master's branch) or do you expect more?
If everything is OK (useful enough) for you, I propose you to close this issue. In the future we can create other issue(s) that will improve the current concept/code.

@dmitry-timofeev
Copy link
Contributor Author

I think it is good, thank you very much!

@decebals
Copy link
Member

This feature is included in the latest release (3.2.0).

@decebals
Copy link
Member

The #360 comes to improve the support for any interface/class as an extension point.

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

3 participants