| Re: [jakartabatch-dev] Overview of Jakarta Batch 2.1 + CDI integration details |
Emily,
Thanks for chiming in. I'm going to merge your post into this single thread and reply.
You wrote:
Late to the party... I had some conversation with Scott previously regarding this issue. Let me ask a few questions to see whether I understand the issues.
Since you want to inject Batch Artifact to other beans, the batch spec needs to specify Batch Artifacts to be CDI beans (such as Bean Validator providing Validator bean etc). By default, it is Dependent scoped. Are you saying sometimes the artifacts need to be ApplicationScoped? Can ApplicationScoped annotation be added to the Batch Artifact to override the default? My apologies if I understand the conversation wrongly. It might help if Scott can provide a couple of use cases with code snippets.
----
Yes, I think in general users should be able to add scope annotations to the batch artifacts... AND the batch container should honor these scopes and load scoped contextual instances when loading artifacts during the batch lifecycle.
This maybe isn't quite as useful with @ApplicationScoped (though possibly useful) as it would be with other scoped like @JobScoped/@StepScoped, (now defined only via extension but possibly to be added to the Jakarta Batch spec).
That said,these use cases raise some questions of when/how batch properties should be injected... the values might not be known until job execution time, after app startup.
Finally, there was the question of should batch artifacts be treated as (dependent-scoped) beans by default (without the @Dependent annotation)... I was willing to concede "no" as Romain was mentioning startup issues, but still was curious.
OK,...possibly some examples are still needed but thread merged at least.
------------------------------------------------------
Scott Kurz
WebSphere / Open Liberty Batch and Developer Experience
skurz@xxxxxxxxxx
--------------------------------------------------------"Scott Kurz" ---10/20/2021 01:59:14 PM---Some more comments ------------------------------------------------------
From: "Scott Kurz" <skurz@xxxxxxxxxx>
To: "jakartabatch developer discussions" <jakartabatch-dev@xxxxxxxxxxx>
Date: 10/20/2021 01:59 PM
Subject: [EXTERNAL] Re: [jakartabatch-dev] Overview of Jakarta Batch 2.1 + CDI integration details
Sent by: "jakartabatch-dev" <jakartabatch-dev-bounces@xxxxxxxxxxx>
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.
Romain, you do have more experience here and so I'm inclined to better trust your preference.
I had partly been motivated by the direction being discussed in Jakarta Restful 4.0 to making all Restful classes beans, see:
https://www.eclipse.org/lists/jaxrs-dev/msg00974.html
https://github.com/spericas/jaxrs-api/blob/release-4.0/JakartaRest40.pdf
Perhaps it's different though in that they already have annotations like @Path that they can leverage and interpret as bean-defining annotations (though couldn't we achieve something similar by looking at all instance of jakarta.batch.* interface types for artifacts?).
But I agree "try to lookup a CDI bean and if none if found, use a plain standalone instance" would work, with the additional requirement that the batch artifacts support contextual CDI injection and interceptors even if the batch artifact isn't itself a bean. (If you want everything to be a bean you can always use BDM=all).
I'm curious though. How do you think treating all batch artifacts as beans would potentially "break" an application? (I suppose there is a performance concern maybe, in the cost of scanning, but that's not the concern you mentioned.)
----
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?
I'm not sure what you're saying here. Maybe this overlaps the concern about injecting batch properties???
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.
It's true that to some extent ... the integration between Batch + CDI was somewhat "de facto" implied when CDI was present, as in an EE 7 container. But it was very confusing to say at the same time that CDI was "optional"... (and we've spent many words trying to sort through this). But yes, in some ways we were already there in 1.0.
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.
Are you saying non-dependent scoped beans shouldn't be allowed to inject batch properties? It seems important to me that Job/Step scoped beans should be allowed to use properties.
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_______________________________________________
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
