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 
<strub...@yahoo.de.INVALID>:
>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 <strub...@yahoo.de.INVALID>:
>> 
>> 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 <r...@apache.org>:
>>> 
>>> 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
>> 
>

Reply via email to