Hi all, there is now also a discussion in CDI [1] about it and a follow up issue here: [2].
Given this current discussions, we are most likely save to raise this challenge as the behaviour tested isn't really defined in CDI 4 (CDI admitted it and they are discussing it for 5 now) + the arguments raised by Mark and Romain. If no one yells a loud stop, I am going to proceed with the formal steps next week. Gruß Richard [1] https://github.com/jakartaee/cdi/issues/779 [2] https://github.com/eclipse-ee4j/batch-tck/issues/70 Am 23. Februar 2024 11:07:21 MEZ schrieb Mark Struberg <[email protected]>: >I did dig into the details of the current latest spec and it's really clear >that CDI.current() to look up a BatchProperty is NOT supported by the spec! > >https://github.com/eclipse-ee4j/batch-api/blob/master/spec/src/main/asciidoc/batch_programming_model.adoc#batchproperty-definition > >"The @BatchProperty annotation identifies an injection as a batch property. A >batch property has a name (name) and, in case of a field injection, also has a >default value. @BatchProperty is used to assign batch artifact property values >from Job XML to the batch artifact itself. >Note that @BatchProperty is used with the standard @Inject annotation >(jakarta.inject.Inject) ..." > >This clearly states it: no BatchProperty without @Inject! > >Further down it is even clearer: > >https://github.com/eclipse-ee4j/batch-api/blob/master/spec/src/main/asciidoc/batch_programming_model.adoc#method-parameter-and-constructor-parameter-injection-with-explicit-name >"A batch runtime must support injection of the CDI Bean representing the batch >property via method parameter and constructor parameter injection, in addition >to supporting field injection." > >The valid options are: >* param injection >* ct injection >* field injection >but no CDI.current().select()... > >Feel free to reference this when creating a TCK challenge. > >txs and LieGrue, >strub > > >> Am 23.02.2024 um 09:01 schrieb Mark Struberg <[email protected]>: >> >> Please create a TCK Challenge and disable the test for now. >> >> txs and LieGrue, >> strub >> >> >>> Am 22.02.2024 um 20:09 schrieb Richard Zowalla <[email protected]>: >>> >>> Hi all, >>> >>> I want to get consensus on filling a TCK challenge against the Batch >>> TCK. As written in the other thread, I am currently working on getting >>> a Jakarta Batch 2.1 ready version of Batch EE for usage in TomEE 10 >>> [1]. The most significant change is, that CDI support is now required. >>> >>> == Details >>> >>> This implementation passes the Batch TCK with one exception [2] >>> >>> com.ibm.jbatch.tck.tests.jslxml.CDITests#testCDILookup(...) >>> >>> This test uses dynamic CDI injections to resolve batch properties in a >>> non CDI batchlet [3]. As Geronimo BatchEE is a fork of the reference >>> impl with some neat adjustments, we are more or less using a producer >>> bean more or less similar to the reference impl: [4, 5]. The major >>> difference between both impls is, that we are using OWB while the >>> reference impl uses Weld. >>> >>> So for the TCK test in question with this line >>> >>> cdi.select(String.class, new BatchPropertyLiteral("prop1")).get(); >>> >>> in [6], the following will happen: >>> >>> - OWB will invoke the producer in [4] with a NULL InjectionPoint >>> - Weld will invoke the producer in [5] with a "fake" InjectionPoint >>> with InjectionPoint#getMember == NULL. >>> >>> The JavaDoc of InjectionPoint [7] or more specifically of >>> InjectionPoint#getMember() does not tell us, if the return value can be >>> NULL. >>> >>> However, we find a code example in the interface docs: >>> >>> @Produces >>> Logger createLogger(InjectionPoint injectionPoint) { >>> return >>> Logger.getLogger(injectionPoint.getMember().getDeclaringClass().getName >>> ()); >>> } >>> >>> If InjectionPoint#getMember() can be NULL, this code example would >>> throw a NullPointerException. So from my point of view, Weld violates >>> the current JavaDoc of InjectionPoint by injecting a "fake" >>> InjectionPoint without a member. >>> >>> InjectionPoint#getBean can return NULL and it is stated, any other >>> method can't - intentionally cause an Instance only defines an >>> InjectionPoint when there is one. >>> >>> So from my point of view the test relies on weld-specific behaviour as >>> there is an ambiquity in the CDI 4.0 spec / javadocs regarding >>> InjectionPoint#getMember() here. KUDOS to Thomas & Romain for helping & >>> tracking it down to the actual ambiquity in the Javadocs ;-) >>> >>> In addition, the Jakarta Batch spec is rather explicit in its >>> definition in [8]. IMHO, it would be good for the spec to just delegate >>> the CDI spec and just write something like "As a consequence of the >>> previous section, an application must be able to get a CDI Bean with a >>> correct view of a batch property by using CDI" instead of adding >>> related details which require CDI knowledge. >>> >>> Before thinking about raising a challenge, I already had a quick >>> discussion with Thomas and Romain about it and Romain suggested to put >>> up a PR for an updated test, which does an indirection to a bean, which >>> is here (+ some discussion / details): [9]. >>> >>> Long story short: >>> >>> - Did we miss something obvious? >>> - Does this consistute for a valid challenge against the Batch TCK? >>> - Any other thoughts on it? >>> >>> Happy to get some opinions. >>> >>> Gruß >>> Richard >>> >>> >>> [1] https://github.com/apache/geronimo-batchee/pull/21 >>> [2] >>> https://github.com/eclipse-ee4j/batch-tck/blob/master/com.ibm.jbatch.tck/src/main/java/com/ibm/jbatch/tck/tests/jslxml/CDITests.java#L287 >>> [3] >>> https://github.com/eclipse-ee4j/batch-tck/blob/master/com.ibm.jbatch.tck/src/main/java/com/ibm/jbatch/tck/artifacts/cdi/NonCDIBeanBatchlet.java >>> [4] >>> https://github.com/rzo1/geronimo-batchee/blob/jakarta/jbatch/src/main/java/org/apache/batchee/container/cdi/BatchProducerBean.java#L67 >>> [5] >>> https://github.com/WASdev/standards.jsr352.jbatch/blob/master/com.ibm.jbatch.container/src/main/java/com/ibm/jbatch/container/cdi/BatchProducerBean.java#L96 >>> [6] >>> https://github.com/eclipse-ee4j/batch-tck/blob/master/com.ibm.jbatch.tck/src/main/java/com/ibm/jbatch/tck/artifacts/cdi/NonCDIBeanBatchlet.java#L51 >>> [7] >>> https://jakarta.ee/specifications/cdi/4.0/apidocs/jakarta.cdi/jakarta/enterprise/inject/spi/injectionpoint >>> [8] >>> https://jakarta.ee/specifications/batch/2.1/jakarta-batch-spec-2.1#consequences-and-suggested-patterns >>> [9] https://github.com/eclipse-ee4j/batch-tck/pull/69 >> >
