Add CDI support #54
Add CDI support #54
Conversation
Resolves #12 There are several changes here: First, this adds an `@Inject` annotation to each CDI-managed bean (unless there is already a no-arg constructor). Second, each package with CDI-managed beans has a `beans.xml` file in the appropriate resources directory. Third, component configuration has been consolidated to use Apache Tamaya. Fourth, constructors have been cleaned up to work better in CDI environments while remaining compatible with non-CDI environments. Fifth, constructors that had been using generic types (e.g. the various cache-related types) now use concrete types. E.g.: CacheService<String, String> becomes: ProfileCacheService
|
Will def review, @ us2ts.org today/tomorrow-- can I review before Monday? |
|
First question: looks like the |
|
@ajs6f my thinking here was just to rely on module/JAR management, but you raise a good point about the content of the beans.xml files. The current approach is definitely pretty simplistic, and if you have a suggestion for improving it, I'm all ears. |
|
I think it's legit for now-- step by step. For example, the next big step would be to introduce the use of an actual CDI framework in |
| dependencies { | ||
|
|
||
| api group: 'org.apache.commons', name: 'commons-rdf-api', version: commonsRdfVersion | ||
| api group: 'org.apache.servicemix.bundles', name: 'org.apache.servicemix.bundles.javax-inject', version: javaxInjectVersion |
There was a problem hiding this comment.
Is it this (and not the vanilla API JAR) in order to get OSGi metadata?
There was a problem hiding this comment.
Yes, it's just to make it easier to deploy in Karaf.
|
Is the use of concrete types specifically to make typesafe injection work without colliding caches? |
|
Yes, the concrete types are created specifically to make it easier to distinguish between the different caches, since those caches make use of different generic types and they also will typically have very different configurations. Another option would be to use annotations, but this seemed more straight-forward. |
|
Yeah, I probably would have gone with annotations, but I couldn't give you a good reason for it. But it might be good to comment on those new types that they exist for the purpose of injection and aren't intended to actually extend the generic types in any way. |
|
@ajs6f I am going to update this PR with annotations rather than using specialized types for the various caches. |
|
I'm in no way against that, but just to be clear, I'm not demanding that. I think we're okay to go either way... but happy to review again once you've sprinkled the annotation flavor on top! |
|
@ajs6f: yes, either way is fine with me, too. I just think the types would have a cleaner design with annotations. |
|
That's probably true. And they might be reusable if the types of caches change, which would be nice. |
|
@ajs6f I have removed the concrete types in favor of qualifiers for use with CDI injection. I am using |
|
I hate injection-by-name (nuts to Spring!) but I agree that it's a sensible move here. We can use this and work on our meta-annotation hype as we go. I'm 👍 to this PR. |
|
@ajs6f sounds good. We can always refine the qualifiers as we go, but this gets us a long ways there. |
Resolves #12
There are several changes here:
First, this adds an
@Injectannotation to each CDI-managed bean (unless there is already a no-arg constructor).Second, each package with CDI-managed beans has a
beans.xmlfile in the appropriate resources directory.Third, component configuration has been consolidated to use Apache Tamaya.
Fourth, constructors have been cleaned up to work better in CDI environments while remaining compatible with non-CDI environments.
Fifth, constructors that had been using generic types (e.g. the various cache-related types) now use concrete types. E.g.:
becomes:
I will note that I am not actually testing this change in a CDI environment, but that will come. Once that happens, there may be additional changes, but this PR lays the foundation for all of that.