I have opened a challenge [1]. Hope we can exclude the test soon and move on with our batch impl soon.
[1] https://github.com/eclipse-ee4j/batch-tck/issues/71 Am Mittwoch, dem 28.02.2024 um 21:02 +0100 schrieb Richard Zowalla: > Given this comment [1] by Scott (Batch Spec), it looks like they are > just waiting for our formal challenge to exclude the test as our > discussions (as well as IBM internal discussions) now also lead to > CDI discussions and our arguments in the PR already convinced them. > > Guess we are good to go now but still want to wait a few more days, > if someone has a Show stopper here. > > Best > Richard > > [1] > https://github.com/eclipse-ee4j/batch-tck/pull/69#issuecomment-1969745918 > > 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 > > > > >
