Re: [jakartabatch-dev] Overview of Jakarta Batch 2.1 + CDI integration d

[Romain Manni-Bucau <rmannibucau@xxxxxxxxx>]

Le jeu. 14 oct. 2021 à 06:35, Scott Kurz <skurz@xxxxxxxxxx> a écrit :

<SK> Response below detailing  new direction:  loading batch artifacts as CDI beans (by default). </SK>

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

"Romain Manni-Bucau" ---10/07/2021 10:09:23 AM---Hi Scott, some comments inline Romain Manni-Bucau

From: "Romain Manni-Bucau" <rmannibucau@xxxxxxxxx>
To: "jakartabatch developer discussions" <jakartabatch-dev@xxxxxxxxxxx>
Date: 10/07/2021 10:09 AM
Subject: [EXTERNAL] Re: [jakartabatch-dev] Overview of Jakarta Batch 2.1 + CDI integration details
Sent by: "jakartabatch-dev" <jakartabatch-dev-bounces@xxxxxxxxxxx>

---

Hi Scott, some comments inline

Romain Manni-Bucau
@rmannibucau |  Blog | Old Blog | Github | LinkedIn | Book


Le jeu. 7 oct. 2021 à 15:55, Scott Kurz <skurz@xxxxxxxxxx> a écrit :

Since Batch + CDI integration overlaps a bunch of issues in our 2.1 milestone:  https://github.com/eclipse-ee4j/batch-api/milestone/1 ) let me level set with a thread here.

My take is that we specify a set of details like this:

1. Batch implementation required to perform CDI injection into batch artifacts  (https://github.com/eclipse-ee4j/batch-api/issues/46)

The Batch implementation MUST be able to load batch artifacts and inject CDI-managed beans (i.e. have the CDI engine inject them, however we'd state it), with or without the presence of BDA on the batch artifacts (i.e. without the batch artifacts needing to be themselves loaded as CDI managed beans), unless CDI is disabled altogether,  (e.g. bean discovery mode = none),

This loading with CDI bean injection MUST occur for any of the spec-mentioned artifact loading techniques (i.e. mapping from JSL @ref values):  a) CDI bean name(*)  b)   batch.xml entry  c) FQCN     (*) - IIRC, we would need to spec the bean name syntax

By default, with BDA=annotated, batch artifacts will not be managed as CDI beans.  

Not sure what you meant but annotated will still create cdi beans (as Bean<?>) so it will be compatible, bda=none will ignore the jar so think we should stick to "what is a cdi bean can be a batch component" which also includes producers btw, no?
 

This will be backed by a statement in the platform spec too (https://github.com/eclipse-ee4j/jakartaee-platform/issues/420),


2.  Instance scope for batch artifacts - CDI bean / non-bean

When a batch artifact is loaded but NOT as a CDI-managed bean, the batch implementation MUST ensure artifact instances are scoped to a single JSL reference.   This means, for one, that different methods within a given listener will use the same instance for a given step (e.g. StepListener beforeStep/afterStep).  

When a batch artifact is loaded as a CDI-managed bean there should be one instance request (TBD: figure out the correct wording) to the BeanManager for each JSL reference.    (This ensures again that using a bean with @Dependent-scoped, the default, doesn't result in different instances across e.g. beforeStep/afterStep).   However, since the BeanManager is control of instance scope it may not be one instance per JSL reference; e.g. using @ApplicationScoped for a batch artifact could result in one instance per application.

What about letting the user decide but encourage @Dependent? There are valid cases for @AppScoped (when reusing the same bean and storing the state elsewhere like a request scope bean or jobcontext.
 

<SK> After thinking some more, what if we just said that every batch artifact is loaded as a CDI bean? (even with  bean discovery mode = annotated, the default)

By default, the artifact without any scope annotation would be loaded with @Dependent scope.   So if you tried to inject a batch artifact type into some other bean, by default you'd get a new, separate instance (per @Dependent scope), rather than having to unexpectedly deal with reusing the same instance as the batch container.  (We'd clarify that "loading" happens once per JSL reference, i.e. the batch impl asks the BeanManager for a bean impl once per JSL reference, so e.g. StepListener beforeStep/afterStep use the same instance rather than two instances.    )

Issue there is some batch component can not be CDI beans so guess the behavior "try to lookup a CDI bean and if none if found, use a plain standalone instance" works well.

If we want to make it all CDI beans we must add components as CDI beans at startup which has multiple issues and one is to potentially break the app depending what it does - and we don't control it when embed.

But this would imply that with say a @StepScoped-annotated batch artifact, anyone injecting an artifact type instance during a step execution will get the same instance created by the batch container
(whether we get around to defining @StepScoped in the 2.1 standard or whether it remains only an extension then).

Yes, this is one of the reason we should let the user define stating the impact. It is not uncommon to use @ApplicationScoped when the state is 100% hosted by other beans.

We should likely make our contexts compatible with that - today think we are dependent so not compatible, no more sure it is state/spec-ed?

 

A weird aspect of this scenario, I admit, is that the batch properties may not get injected/populated correctly as defined in JSL.   E.g. if I configure two @JobScoped MyStepListeners but try to define different property sets, well, the CDI instance scope takes precedence and I only get the first one.   Even more strange could be an @ApplicationScoped bean, which might get stuck with 'null' values injected if populated before a job is run (if the bean is loaded that early for some reason).  

Right but this is the strength of that too: you can share a state by design between steps.

Note that it is the same when you have an endpoint @RequestScoped and inject an @ApplicationScoped inside, think it is ok for CDI users.

 

I imagine that these types of considerations could lead us down the path of adopting a spec behavior more like Restful services:
https://github.com/eclipse-ee4j/jakartaee-platform/blob/master/specification/src/main/asciidoc/platform/ResourcesNamingInjection.adoc#a651
where the spec container (servlet/restful container, etc.) manages the component instance, where the component instance can get other CDI beans injected into it, but where the spec container may create an unmanaged instance (so other beans injecting the type of the component instance may end up getting injected with a separate instance created by the CDI container).

We are already there actually in 1.0, it is just the root instance which is not so it is more an alignment.

 

I guess my intuition though is you will either think injecting a batch artifact into a bean is crazy / overly complicated... or you will want to inject the same exact instance loaded by the batch container.  I don't see what value there'd be in defining a type that serves as a batch artifact (impls that interface) and can also be injected into other beans, using completely separate instances.  

Avoiding to define a 3rd bean to get a service, controlling instantitations etc....

 

Back to the multiple property set issue.. could we instead just have the batch container go set (overwrite) the properties on the instance?   For Job/StepScoped beans, there won't be any other thread using this instance (well, we have to worth through partitions but ignoring that for now).  

Not, it must respect the scope so the container provides the properties as a dependent bean, if the scope is compatible it works, else it does not.

It is not different from microprofile-config or deltaspike-config for example.

 

OK.. for an ApplicationScoped bean you could have another thread using this but then you probably shouldn't have configured batch properties on it, or they should be immutable.   So only the CDI container creates instances but the batch container still might set values on these instances.

And you could disable all this by setting BDM=none and revert to non-CDI, one artifact instance per JSL reference.

Or vetoing the bean too.

 

It would be interesting to hear feedback too from other CDI experts, e.g. those maybe responsible for defining the Restful+CDI instance behavior.
</SK>



3. Inject JobOperator - (straightforward:  https://github.com/eclipse-ee4j/batch-api/issues/17)

4. The Batch implementation MAY provide custom, proprietary techniques for disabling CDI

Strictly speaking there's probably no need to explicitly state this but it does provide continuity speaking to our legacy of trying to define a generic, non/pre-CDI specified behavior.  

A jakarta.batch.cdi.enabled=false can be neat in properties for portability.

 <SK> Though maybe with bean discovery mode = none we already have a standard way to disable CDI, so little need to add another. </SK>

There is the embed case where it can help because if you dev a batch as a standalone you can then import it in a jakarta container and there it can become a CDI bean.

Generally it will not break anything but behavior will change.

That said no big deal to not add it now.

 

5. (A miscellaneous  consequence worth mentioning) - Don't rely on field initializers  https://github.com/eclipse-ee4j/batch-api/issues/73   
Since the CDI engine may be in control of @BatchProperty injection, field initializers can not be relied upon to provide a default value in the absence of a corresponding JSL property, and so should be considered a non-best practice (since CDI is a better practice).

I think these artifacts already exist so it's too late to try to ban them.   But we should say something in the spec, since the question has come up.  Perhaps an implementation would want to issue a warning.

Think we can say that we register a generic batch property producer with dependent scope so any injection must comply with it. We can also  mention it is only valid during bean lookup (before it is used) and not after clarifying the behavior for normal scope proxies and expliciting the risk for not dependent beans.

<SK> My concern is if we have an artifact that works in some implementation today which is not using CDI producer to inject batch properties.   With Batch 2.1 and the above proposal since we're saying batch artifacts are beans by default and therefore would have batch property injection handled via a CDI producer.   Maybe the answer here is simply to disable CDI with BDM=none.  This doesn't seem like an "obvious" implication of Batch + CDI spec integration, to me, so I think mentioning in spec could be helpful.  </SK

Yes, worse case user can veto all its component through a CDI extension to get back the old behavior during the integration/import. But as an impl there is no way to know it is a warning or not I think. 

6. Batch defined custom scope

Still on the table:  Job/Step-scoped ((https://github.com/eclipse-ee4j/batch-api/issues/79)    One issue to clarify is whether we should offer both the "thread local" behavior of JobContext/StepContext and non-thread-affinity scope variants, and what exactly to name them, etc.
(Not sure if we need to specify when standard CDI scope lifecycle event callbacks are made relative to parts of the batch lifecycle or not)

7. Injection of one Batch artifact into another  https://github.com/eclipse-ee4j/batch-api/issues/171

I think we've effectivley covered this now.  Item 2.,  above describes instance creation from the batch container artifact loading, and so at this point we can just say "normal CDI rules apply".  And of course item 6. elaborates on those rules.
---

I think that's it.   Did I miss anything?   Appreciate your comments/feedback.
------------------------------------------------------
Scott Kurz
WebSphere / Open Liberty Batch and Developer Experience
skurz@xxxxxxxxxx
--------------------------------------------------------

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

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