Re: [jakartaee-platform-dev] Batch+CDI integration: proposal to continue

["Scott Kurz" <skurz@xxxxxxxxxx>]

So let me recap Ondro's earlier enumeration of the options.  

   Option 1: Artifacts not defined as CDI beans by the batch container, cannot be injected, other CDI beans can be injected into them
  Option 2: Artifacts not defined as CDI beans by the batch container, cannot be injected, other CDI beans cannot be injected into them (only accessesd via CDI.current())
  Option 3: Artifacts are defined as CDI beans by the batch container, can be injected, other CDI beans can be injected into them

(I added the "defined as CDI beans by the batch container" wording to clarify)

Romain filled in other important details but we can save them for the batch spec;  they were not the part that was still up for debate.
(I do think for the FQCN case, we can work out  the precise spec wording separately, on the Batch ML, and maybe survey existing implementations.)

So I agree with Romain.. I don't want to complicate things by adding another scanning or step within the CDI app lifecycle just for Batch.

Let me ask Ondro and anyone else... are we convincing you?  

Are we getting close to consensus or should we hold some kind of vote?  

And yes, it would be great to progress to more interesting things like the DSL idea but we did target CDI integration, we've spent a lot of time on it over the years, and it will be nice to finalize this at least.

Thanks,
------------------------------------------------------
Scott Kurz
WebSphere / Open Liberty Batch and Developer Experience
skurz@xxxxxxxxxx
--------------------------------------------------------

"Romain Manni-Bucau" ---11/16/2021 02:52:10 PM---Le mar. 16 nov. 2021 à 19:57, Emily Jiang via jakartaee-platform-dev < jakartaee-platform-dev@eclips

From: "Romain Manni-Bucau" <rmannibucau@xxxxxxxxx>
To: "jakartaee-platform developer discussions" <jakartaee-platform-dev@xxxxxxxxxxx>
Date: 11/16/2021 02:52 PM
Subject: [EXTERNAL] Re: [jakartaee-platform-dev] Batch+CDI integration: proposal to continue NOT injecting CDI beans into non-bean Batch artifacts
Sent by: "jakartaee-platform-dev" <jakartaee-platform-dev-bounces@xxxxxxxxxxx>

---

Le mar. 16 nov. 2021 à 19:57, Emily Jiang via jakartaee-platform-dev <jakartaee-platform-dev@xxxxxxxxxxx> a écrit :

My response is inline.


On Tue, Nov 16, 2021 at 4:14 PM Romain Manni-Bucau <rmannibucau@xxxxxxxxx> wrote:
Hi all, let me comment inline


Le mar. 16 nov. 2021 à 16:40, Scott Kurz <skurz@xxxxxxxxxx> a écrit :
So let me dive into  Ondro's idea for option 3. - make batch artifacts CDI beans by default.

So how would we implement: ?
> The Batch container would only need to supply a standard CDI extension to create such a CDI bean for non-CDI artifacts. 

It strikes me as wrong to try to parse the job XMLs at app startup (for one, JSL loading was left a bit open-ended so it is not guaranteed the JSL will be in META-INF/batch-jobs).


Agree, it also has other issues like the fact JBatch didn't define the application context/loader or anything like that so it is allowed to wrap jobOperator.start to set a custom classloader providing the job in memory so it will never be found with any scaninng - and it is actually used.
 


I don't think we can wait until job execution time...assuming that's too late to register with CDI.


Exactly, we have to be at startup during extension lifecycle before CDI ready.
 


So I guess we'd have to do something like scan for jakarta.batch.api.* classes at app startup time.


Which is a clear no-go at platform level IMHO since it means adding yet another concurrent scanning mecanism to CDI at the moment where it aims at unifying everything on top of CDI.
 


So Batch would do a separate round of scanning?   In addition to CDI scanning?   We couldn't rely on something like a Jandex scanning, right?


For me it goes against the platform so I hope not.
Jandex is an implement detail and is not a prerequisite of CDI at all.
There is also the fact classes can be disabled in CDI so even if you add all jakarta.* classes it can NOT be a cdi bean at runtime.
It is not uncommon to have a org.superbiz.batch.components and veto them all - either with an extension or @Vetoed - because you don't want the CDI overhead/consequences for standalone classes.

This is why I think the best of the options is the option 4:

1. if the bean exists in CDI context either by name (ref) or fqn with @Default qualifier (it is implicit by spec) then use it, you get all the goodness of CDI (lifecycle, injections everywhere, ...)

Do you mean the beans were made CDI beans via bean defining annotations or via beans.xml with bean-discovery-mode=all. I don't understand why they need to  have @Default Qualifier. Do you mean no other qualifier is allowed to be added to the batch artifacts?

Whatever is fine for CDI, can even be with a extension and addBean() DSL (configurators).
@Default comes from the fact JBatch descriptor will look like <anything ref="name" /> so you only have a name.
JBatch enables to put either a name (@Named) or a fully qualified class.
So when looking up a CDI bean we use @Default qualifier because we have no way to know if we should use others.

Side note: indeed there is the case you have a single bean matching with a custom qualifier but it is a very border case we don't care IMHO, other cases (> 1) would lead to a failure anyway so let's stick with the default while we use a XML -> Java lookup.

2. if 1 failed then just make the class managed by JBatch container.
In this JBatch managed artifacts, do they support CDI injections and the use of interceptors?

Nop, plain new MyArtifact() + @Inject @BatchProperty  + @inject StepContext + @inject JobContext *only* (= new + JBatch specific injections).
Exactly the same as in current version, no change there.

Thanks
Emily
 

It can be summarize as being 100% JBatch managed if not 100% CDI managed.

Indeed it lets the "1.5" case - standalone instance which would need injections - off the shelf but:

a. it is already the case by spec so no big deal
b. I don't think it is a realistic case. Look, you developped a bean which can't run by itself but expect an injection to be there and not settable through another way? weird no?
c. CDI.current() is more than fine for these cases anyway so there is a solution already

We would be discussing JBatch 1.0, I would probably agree 2 is useless but it s the behavior since 1.0 (and 2.0) and users adopted it because it enables to do libraries of reusable components in all environments (EE, CDI but also Spring, Guice, plain standalone) and very light batch deployments (a few megs only) so it is quite convenient.

Indeed we can complexify the spec, add some new qualifier etc but it will not bring any needed feature to end users, will not go in the simplification and a consistent direction of the platform IMHO so it would be useless work for everyone.

So just expliciting what was implicit is likely good enough.

Don't forget the big tracks for JBatch will likely not be that but how to have a Java DSL (XML-less), how to expose the persistence layer somehow to let it be pluggable etc...

So my 2cts would be to keep it simple, don't try to be over-technics or say "let's break it even if we don't gain any feature" - keep in mind the only value of JakartaEE is to NOT do that ;). So solutions to pb 2 and 3 which are "let it fail" are blockers for me.
 


Romain, am I capturing your concerns?


Yep, thanks a lot Scott!
 


------------------------------------------------------
Scott Kurz
WebSphere / Open Liberty Batch and Developer Experience
skurz@xxxxxxxxxx
--------------------------------------------------------

"Reza Rahman" ---11/15/2021 08:05:35 PM---I would certainly be curious to learn a bit more about what the EJB/JSF point is. To my knowledge, n

From: "Reza Rahman" <reza_rahman@xxxxxxxxx>
To: "jakartaee-platform developer discussions" <jakartaee-platform-dev@xxxxxxxxxxx>
Date: 11/15/2021 08:05 PM
Subject: [EXTERNAL] Re: [jakartaee-platform-dev] Batch+CDI integration: proposal to continue NOT injecting CDI beans into non-bean Batch artifacts
Sent by: "jakartaee-platform-dev" <jakartaee-platform-dev-bounces@xxxxxxxxxxx>


---




I would certainly be curious to learn a bit more about what the EJB/JSF point is. To my knowledge, neither API had the particular use case in question.

Reza Rahman
Jakarta EE Ambassador, Author, Blogger, Speaker

Please note views expressed here are my own as an individual community member and do not reflect the views of my employer.
 


---


From: jakartaee-platform-dev <jakartaee-platform-dev-bounces@xxxxxxxxxxx> on behalf of Scott Kurz <skurz@xxxxxxxxxx>
Sent: Monday, November 15, 2021 6:55 PM
To: jakartaee-platform developer discussions
Subject: Re: [jakartaee-platform-dev] Batch+CDI integration: proposal to continue NOT injecting CDI beans into non-bean Batch artifacts
 

Thanks Ondro,

I'm glad you broadened the discussion to add back the idea of treating all batch artifacts as beans (your option 3.).

Though generating a qualifier like `BatchArtifact` could address the problem of creating a new ambiguous dependency, my biggest concerns with 3. are Romain's suggestions that we'd be committing Batch to essentially re-implement CDI scanning or else end up with inconsistencies.

I'm reacting to these posts:
 https://eclipsefoundationhq.slack.com/archives/C02K9FX2ETA/p1636134043025000?thread_ts=1636126456.019800&cid=C02K9FX2ETA
 https://eclipsefoundationhq.slack.com/archives/C02K9FX2ETA/p1636127555021000?thread_ts=1636126456.019800&cid=C02K9FX2ETA

and this one suggesting there were some negative lessons learned with the Enterprise Beans and Server Faces specs:
 https://eclipsefoundationhq.slack.com/archives/C02K9FX2ETA/p1636128063022600?thread_ts=1636126456.019800&cid=C02K9FX2ETA

For what it's worth, another point I'd raised is that we can't know what batch artifacts will ultimately be used at application startup, (since we don't know what jobs will ultimately be executed), so we'd have to do something type-based like register all jakarta.batch.api.* impls.

But I fully admit not understanding this at the depth and detail necessary to have a truly informed opinion.. which is why I was hoping to tap into the collective wisdom of the platform list here.

Maybe we could ask Romain to elaborate more?

------------------------------------------------------
Scott Kurz
WebSphere / Open Liberty Batch and Developer Experience
skurz@xxxxxxxxxx
--------------------------------------------------------

"jakartaee-platform-dev" <jakartaee-platform-dev-bounces@xxxxxxxxxxx> wrote on 11/10/2021 11:25:51 AM:

> From: "Ondrej Mihályi" <ondrej.mihalyi@xxxxxxxxxxx>
> To: "jakartaee-platform developer discussions" <jakartaee-platform-
> dev@xxxxxxxxxxx>
> Date: 11/10/2021 11:26 AM
> Subject: [EXTERNAL] Re: [jakartaee-platform-dev] Batch+CDI
> integration: proposal to continue NOT injecting CDI beans into non-
> bean Batch artifacts
> Sent by: "jakartaee-platform-dev" <jakartaee-platform-dev-bounces@xxxxxxxxxxx>
>
> Hi,
>
> I followed the previous conversation and it's easy to get lost in
> the proposals and arguments so I'll try to summarize what are the
> proposals and questions/problems related to them. Scott and others,
> please correct me if I get it wrong.
>
> The 2 options discussed are whether:
> Batch artifacts would not become CDI beans by default (could be
> turned into CDI beans by a user as nowadays) but would allow injection
> of CDI artifacts. They wouldn't be injectable into other CDI beans.
> Similar to how Servlets, REST resources and other platform beans work now
> Batch artifacts would not become CDI beans and would not allow injection
> of CDI artifacts. They wouldn't be injectable. CDI artifacts would
> need to be accessed via CDI.current() mechanism unless the artifact
> is turned into a CDI bean by a user
> Batch artifacts should become CDI beans by default and thus allow injecting
> CDI beans into them. This in turn would also make them injectable
> into other CDI beans
> Summarized:
>
> Option 1: Artifacts not CDI beans, cannot be injected, other CDI beans can
> be injected into them
> Option 2: Artifacts not CDI beans, cannot be injected, other CDI beans cannot
> be injected into them (only accessesd via CDI.current())
> Option 3: Artifacts are CDI beans, can be injected, other CDI beans can
> be injected into them
>
> It looks like although the option 3 was mentioned earlier, it's not
> being considered now and we're choosing between 1 and 2.
>
> However, I think that the option number 3 might be the best as it
> combines best of both:
> CDI clearly defines the lifecycle (e.g. running PostConstruct
> method) and Batch spec can just build on this (specify the default
> scope of artifacts, inject Batch context objects as CDI beans,
> inject not only using field injection but also support other
> injection mechanisms)
> Batch artifacts could be injected and intercepted as CDI beans
> automatically, no need to turn them into CDI beans by the user as
> with solution 2
> Batch spec doesn't need to reinvent the wheel and just rely on the
> CDI mechanism for injection, so it's much easier to specify than
> with solution 1
>
> I think I have a good solution for all the drawbacks of this
> approach mentioned earlier, which I'll explain.
>
> AFAIR, the arguments against option 3 were:
> It could break current applications, because artifacts, which
> previously weren't CDI beans, could cause ambiguous dependency
> situations if they would match other existing beans and injection
> points that only expect a single bean. E.g. batch artifact B extends
> from a CDI bean A, and some other CDI bean C injects A. Now, A would
> just be injected to C, while B is not considered for injection. If B
> is turned into a CDI bean by default, then both A and B would be
> considered for injection into C -> Ambiguous
> It's possible that a batch artifact class doesn't meet the
> requirements for CDI and it couldn't be turned into a CDI bean (for
> example no noarg constructor)
> Batch would rely on CDI to work, even if it's used outside of a
> Jakarta EE runtime
> Solution for problem 1
>
> I think this can be easily resolved by creating a new @BatchArtifact
> CDI qualifier in the API and add this qualifier to all CDI beans
> created from the non-CDI Batch artifacts by the container. This
> would clearly separate CDI beans created by a Batch container and
> those created by a user, beans created by the container wouldn't be
> attempted to inject into user's code. It would still be possible to
> inject them into other CDI beans using the qualifier if needed
> (although this is rather edge case). And the Batch container can
> just use the CDI container directly to instantiate the beans, again
> using the qualifier. The Batch container would only need to supply a
> standard CDI extension to create such a CDI bean for non-CDI artifacts.
>
> Solution for problem 2
>
> This actually isn't a problem at all. In most cases, classes need to
> have a no-arg constructor so that they can be instantiated by their
> class name. So this is an edge case. And even if artifacts can't be
> turned into CDI beans, they could be treated as they are treated now
> - CDi injection wouldn't work, only simple non-CDI injection
> specified by the Batch spec already would take place. CDI beans
> wouldn't be injected. Runtimes can produce a warning about this and
> suggestion to make the class valid for CDI to make CDI injection work.
>
> Solution for problem 3
>
> This is similar situation as for problem 2 - if Batch shouldn't rely
> on CDI, artifacts can't be turned into CDI beans. Then they would be
> treated as they are treated now, with limited injection. This is OK
> because CDI isn't available and therefore not expected to inject beans.
>
> Is there anything wrong in my assumptions? What could break if we
> implement the option 3 in the way I suggest?
>
> Kind regards,
> Ondrej Mihályi
> Senior Payara Service Engineer
> Payara - Supported Enterprise Software for Jakarta EE and
> MicroProfile Applications
> US: +1 415 523 0175 | UK: +44 207 754 0481
> ----------------------------------------------------------------------------------------------------------------------
> Payara is a proud recipient of the prestigious Queen's Award for
> Enterprise: International Trade 2021
> Payara-Tech LDA, Registered Office: Rua Nova de São Pedro no. 54,
> 2nd floor, room “D”, 9000 048 Funchal, Ilha da Madeira, Portugal
> VAT: PT 515158674 | www.payara.fish | info@xxxxxxxxxxx | @Payara_Fish
>
> From: jakartaee-platform-dev <jakartaee-platform-dev-
> bounces@xxxxxxxxxxx> on behalf of Scott Kurz <skurz@xxxxxxxxxx>
> Sent: 09 November 2021 22:45
> To: jakartaee-platform developer discussions <jakartaee-platform-
> dev@xxxxxxxxxxx>
> Subject: Re: [jakartaee-platform-dev] Batch+CDI integration:
> proposal to continue NOT injecting CDI beans into non-bean Batch artifacts
>  
> Thanks Reza,
>
> Yes we're talking about batchlets, listeners, readers, writers and
> processors:  the spec-defined artifacts analogous to servlets, etc.
> in the table at: https://github.com/eclipse-ee4j/jakartaee-platform/
> blob/master/specification/src/main/asciidoc/platform/
> ResourcesNamingInjection.adoc#a651  
>
> My experience from Batch 1.0 is that users wanting to inject CDI
> beans were able to figure out, without too much confusion, that they
> could add a @Dependent on the batch artifact, or set BDM=all (yes
> the move from CDI 1.0 to 1.1 BDM may have required some re-learning
> but for all CDI users not Batch especially).
>
> The argument Romain and I are making is that this is easy enough for
> new/naive users.   For more advanced users, it is preferable to just
> defer to CDI rather than inventing (from the batch perspective), or
> continuing the pattern (from the platform perspective) the behavior
> for a hybrid, not-quite-CDI injection into a non-bean artifact use
> case, and define custom "hybrid" rules.
>
> I do see time constraints as an issue and don't want to rush.  
> However, the proposal now mostly would just standardize what people
> have been able to do since 2013.  
>
> True, at one point I was arguing the opposite case:  to support CDI
> injection into a non-bean batch artifact.     I was taking for
> granted that this was the expected norm that most would agree on
> when bringing a component spec into the Platform, and integrating
> with CDI, and a well-known pattern from integrating other specs.
>
> It does seem late to be defining any new behavior, though, but
> possibly not too late to ratify existing behavior, if we can reach
> consensus from both the Batch & Platform perspectives.
>
> Appreciate you trying to get more feedback,
> ------------------------------------------------------
> Scott Kurz
> WebSphere / Open Liberty Batch and Developer Experience
> skurz@xxxxxxxxxx
> --------------------------------------------------------
>
> [image removed] "Reza Rahman" ---11/09/2021 09:59:02 AM---I do hope
> there are other folks still left that can provide the institutional
> knowledge. If not, may
>
> From: "Reza Rahman" <reza_rahman@xxxxxxxxx>
> To: "jakartaee-platform developer discussions" <jakartaee-platform-
> dev@xxxxxxxxxxx>
> Date: 11/09/2021 09:59 AM
> Subject: [EXTERNAL] Re: [jakartaee-platform-dev] Batch+CDI
> integration: proposal to continue NOT injecting CDI beans into non-
> bean Batch artifacts
> Sent by: "jakartaee-platform-dev" <jakartaee-platform-dev-bounces@xxxxxxxxxxx>
>
>
>
> I do hope there are other folks still left that can provide the
> institutional knowledge. If not, maybe the answer is to form that
> institutional knowledge now.
>
> I prefer what I think would be easiest for users, which would be to
> just support injection by default rather than needing to convert
> over to CDI beans when needed. That said, I honestly have not seen
> that need in Bach usage in the wild that much and in general people
> seem to just make batch artifacts CDI beans anyway. So maybe in this
> case it’s fine. I may also be missing an understanding of all the
> cases where injection may be needed. Is it more than just batchlets,
> listeners, readers, writers and processors? I think it’s fine to
> expect those will probably be CDI beans, that’s what I have seen
> most people do anyway.
>
> If part of the issue is time constraints before the next release, is
> it better to just leave options open until we can get more Batch end
> user input? It’s surprising so few people have chimed in so far.
> I’ll try to see if I can change that.
>
> Reza Rahman
> Jakarta EE Ambassador, Author, Blogger, Speaker
>
> Please note views expressed here are my own as an individual
> community member and do not reflect the views of my employer.
>  
>
> From: jakartaee-platform-dev <jakartaee-platform-dev-
> bounces@xxxxxxxxxxx> on behalf of Scott Kurz <skurz@xxxxxxxxxx>
> Sent: Tuesday, November 9, 2021 9:37 AM
> To: jakartaee-platform-dev
> Subject: [jakartaee-platform-dev] Batch+CDI integration: proposal to
> continue NOT injecting CDI beans into non-bean Batch artifacts
>  
> Hi,
>
> I wanted to get some Platform-level input for the discussion we're
> having in Jakarta Batch re: CDI integration
>
> The key question is whether it's OK to continue only doing CDI bean
> injection into Batch artifacts that are themselves beans (via the
> CDI-defined standard mechanims:  BDA, etc.) , rather than injecting
> into any Batch artifact (as the Platform requires for other specs
> like Servlet, etc.).
>
> An easy starting point to read the argument laid out is the last message:
> https://www.eclipse.org/lists/jakartabatch-dev/msg00226.html
> plus the two previous embedded in this reply (from me and from
> Reza), but you maybe don't have to read the whole thread.
>
> So we're proposing formalizing the current behavior (which in EE 7
> in Batch 1.0 had been maybe sort of implied but not mandated as
> Batch was CDI-aware but tried to be DI-neutral for Spring Batch, etc.).
>
> I do think there's a counter-argument that says the way integration
> is done in the Jakarta platform is to support CDI injection into
> "special" artifact instances defined by the component specs, as
> described in this table:
> https://github.com/eclipse-ee4j/jakartaee-platform/blob/master/
> specification/src/main/asciidoc/platform/ResourcesNamingInjection.adoc#a651  
> and thus Batch should follow this pattern.
>
> In the thread you'll see more pros and cons (I won't rewrite them
> here for now).  
>
> But it does seem to me like the Platform should have a common
> approach or at least some "institutional knowledge" to guide each
> spec as it integrates with CDI.
>
> Thank you,
> ------------------------------------------------------
> Scott Kurz
> WebSphere / Open Liberty Batch and Developer Experience
> skurz@xxxxxxxxxx
> --------------------------------------------------------
> _______________________________________________
> jakartaee-platform-dev mailing list
> jakartaee-platform-dev@xxxxxxxxxxx
> To unsubscribe from this list, visit https://www.eclipse.org/
> mailman/listinfo/jakartaee-platform-dev
>
> _______________________________________________
> jakartaee-platform-dev mailing list
> jakartaee-platform-dev@xxxxxxxxxxx
> To unsubscribe from this list, visit https://www.eclipse.org/
> mailman/listinfo/jakartaee-platform-dev

_______________________________________________
jakartaee-platform-dev mailing list
jakartaee-platform-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jakartaee-platform-dev



_______________________________________________
jakartaee-platform-dev mailing list
jakartaee-platform-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jakartaee-platform-dev

_______________________________________________
jakartaee-platform-dev mailing list
jakartaee-platform-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jakartaee-platform-dev


--
Thanks
Emily

_______________________________________________
jakartaee-platform-dev mailing list
jakartaee-platform-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jakartaee-platform-dev_______________________________________________
jakartaee-platform-dev mailing list
jakartaee-platform-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jakartaee-platform-dev