search

spring-projects / spring-plugin

Apache License 2.0
441 stars 117 forks source link

@EnablePluginRegistries not discovering plugins on classpath #23

Closed jjparsons closed 8 years ago

jjparsons commented 8 years ago

Hi,

I'm using @EnablePluginRegistries({ Feed.class }) to discovery plugins (feeds) on the classpath and then InitializingBean to process plugins using the afterPropertiesSet() method. The plugins have been added using a separate dependency (jar), however, they are not being picked up for some reason. Any ideas?

Note, a PluginRegistry bean is added to the application context but includes no plugins. I've tried copying the plugins directly into the project and ensuring that @EnablePluginRegistries is being used within the root of the project.

Thanks,

Jordan

odrotbohm commented 8 years ago

Can you provide an example project showing the issue? Are you sure the plugin implementations of the separate JAR are picked up by component scanning or explicit means of bean registration?

jjparsons commented 8 years ago

Will do. Quick question, I assume I can use a multi-Maven module project to provide default plugin implementations and simply ensure they are added to the classpath? I reviewed the source and it seems that any subclass of the plugin should be wired and included.

Note, the custom plugin registry bean is created correctly as expected.

jjparsons commented 8 years ago

Done. This illustrates the problem. Pull Request's welcome.

https://github.com/jjparsons/spring-plugin

odrotbohm commented 8 years ago

I took a quick look an it's exactly like I expected. The @SpringBootApplication annotation enables component scanning for the com.github.poc.app package. However, your DefaultFeed implementation is not even becoming a Spring bean in the first place, as it doesn't carry an @Component annotation and resides in an unscanned package com.github.poc.feed.

If you fix that — e.g. by setting up a component scan for com.github.poc and annotating DefaultFeed with @Component — you should see your example working.

odrotbohm commented 8 years ago

PR submitted which makes your app bootstrap as expected.

jjparsons commented 8 years ago

Thanks, that fixed the issue. Saw your PR - agreed, I think you need to enhance the auto config such that it is not prescriptive and you can include any disparate packages/plugins. Referring to the plugin interface is what confused me. You may want to consider using annotations or something of that nature. It's different that component scanning other packages because they are disparate in nature and the relationship is an inversion of ownership. I will follow-up with any suggestions after I have cycles to think about it...

odrotbohm commented 8 years ago

I think the key aspect to realize is that the library basically assembles plugin registries from an application context that you provide. So how plugin implementations get into that application context is completely under your control and thus at the same time something, you have to take care of.

Agreed, using component scanning directly with the application defining the package is not quite fitting the model as it basically forces all plugins to reside under that package. This might be something undesirable but might not even be a problem if all you want to do is build a modular monolith.

If you want to be more open, you can follow the pattern used in the reference documentation: agreeing on an XML file location to import, e.g. by letting the app use a wildcarded import like classpath*:META-INF/plugins.xml. With that in place, the plugins gain control about their contribution to the application context: they could set up a component scan for their package, explicitly declare configuration classes or XML based plugin bean declarations. I just mentioned the component scanning approach primarily as using XML files doesn't seem to be en vogue these days.