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

Require artifact loading via CDI when CDI is "available" (beans.xml is present)? #46

Closed
follis opened this issue Jul 15, 2020 · 0 comments · Fixed by #181
Closed

Require artifact loading via CDI when CDI is "available" (beans.xml is present)? #46

follis opened this issue Jul 15, 2020 · 0 comments · Fixed by #181

Comments

@follis
Copy link
Contributor

follis commented Jul 15, 2020

Originally opened as bug 5416 by ScottKurz

--------------Original Comment History----------------------------
Comment from = ScottKurz on 2013-09-26 13:53:58 +0000

The JSR 352 specification was developed with the intent to leave open the question of what technology would be used for:

a) dynamic injection - of batch properties and job/step context
b) artifact loading - mapping @ref values to classes and object instances

In working on the EE TCK (in CTS), we realized the following dilemna: the presence of beans.xml in the application archive forces the batch runtime to be able to satisfy a) via CDI.

How should the spec be updated to reflect this de facto requirement for CDI support?


Comment from = waynexlund on 2013-10-28 15:11:36 +0000

Our position from Spring Batch was to ensure that CDI was not the assumption but I was never really happy with how far we swung in saying no assumption about DI. I had proposed a similar approach to the JSR-107, but cannot recall the details right now. I'd have to reinvestigate. I'm not for implying CDI as the DI.


Comment from = ScottKurz on 2013-10-28 16:04:54 +0000

Wayne,

Again, the core problem is this scenario: in an EE-environment, if you have a beans.xml in your app archive, then you must be able to satisfy the @Inject for properties and contexts via CDI. (Not sure if SE has an analogous case with beans.xml or not). CDI doesn't care that you wanted to use some other DI technology at that point in your batch app.. you blow up since CDI can't satisfy the injection.

Yes, another DI could put its own extra burden on the batch implementation, like CDI does, and as you know we have worked to keep the 352 language DI technology-neutral.

But CDI is part of the EE platform... so could a batch implementation really choose to blow up completely just because the batch artifacts are part of a bigger application using CDI? That's not very friendly from the EE platform viewpoint.

That's where I was headed in wondering if we could "require CDI support for injection when beans.xml is present" (note we wouldn't necessarily have to require its use for artifact loading, i.e. mapping @ref values to artifact instances). I'd probably need some help phrasing it correctly from a CDI expert. Another thing I wondered is if this is really for the EE 7 platform spec to clarify..not for our individual spec (352).

In any case, I'd be curious to hear if you'd developed any new angles (e.g. in JSR 107) that might be helpful in resolving this.


Comment from = mminella on 2013-10-29 16:18:24 +0000

It seems to me that we've gone down a path that has backfired on us. The use of @Inject was intended to be a portable way to allow for properties/contexts to be injected.

What I think Scott is saying is that it, is in fact, not portable and ends up tying us to CDI. I was already planning on suggesting the removal of the @Inject annotation from properties (it's redundant to the @BatchProperty annotation). The only other place the @Inject is used (off the top of my head) is the contexts. I'd advocate for an update that addresses the injection of contexts without @Inject than require CDI support in any scenario.

As I've noted in other issues, the expert group was clear on this. The spec should not require DI but be open to it's use. To not only require DI, but a specific implementation of it goes directly against this.


Comment from = simas_ch on 2013-10-29 16:33:40 +0000

I'm currently using the RI in one of my projects.

My problem is that in the Java EE environment where CDI is present I can use @nAmed for my artifacts and in the JSL file I use that name as the reference.
I also could use CDI interceptors, decorators etc.

But when my batch runs in Java SE this wouldn't work.
Therefor my batch will not be portable anymore.

CDI is based on JSR-330 javax.inject and Google Guice and Spring Framework implement JSR-330.

Could it be an option to allow everything that is in javax.inject?


Comment from = chrisschaefer on 2013-10-29 21:22:35 +0000

(In reply to mminella from comment #3)

It seems to me that we've gone down a path that has backfired on us. The
use of @Inject was intended to be a portable way to allow for
properties/contexts to be injected.

What I think Scott is saying is that it, is in fact, not portable and ends
up tying us to CDI. I was already planning on suggesting the removal of the
@Inject annotation from properties (it's redundant to the @BatchProperty
annotation). The only other place the @Inject is used (off the top of my
head) is the contexts. I'd advocate for an update that addresses the
injection of contexts without @Inject than require CDI support in any
scenario.

As I've noted in other issues, the expert group was clear on this. The spec
should not require DI but be open to it's use. To not only require DI, but
a specific implementation of it goes directly against this.

+1 on the removal of @Inject annotation from properties already marked w/ @BatchProperty.


Comment from = ScottKurz on 2013-10-29 22:05:40 +0000

(In reply to mminella from comment #3)

We've collectively spent a lot of time and effort on this nuanced approach to DI, and I don't see why we now have to throw up our hands.

We had originally failed to recognize, however, that there is such thing as a CDI app, i.e. an EE app archive (someone feel free to state more precisely) that includes beans.xml, that will trigger CDI-managed injection.

(This case could exist in some form in SE but without CDI in the platform, we don't have to address it today.)

As it stands, we've left open that a 352 impl can inject into a non-CDI Batch application however it sees fit. There's no reason to now view this case any differently.

Requiring CDI support for a CDI app, in an EE server that presumably already has support for CDI in general, is a particular case, and I don't think strays from the original goal of not being unnecessarily tied to CDI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants