TCK Challenge: com.ibm.jbatch.tck.tests.jslxml.CDITests#testCDILookup(...)

[rzo1]

The relevant specification version and section number(s)

• https://jakarta.ee/specifications/batch/2.1/jakarta-batch-spec-2.1#batchproperty-definition
• https://jakarta.ee/specifications/batch/2.1/jakarta-batch-spec-2.1#method-parameter-and-constructor-parameter-injection-with-explicit-name
• https://jakarta.ee/specifications/batch/2.1/jakarta-batch-spec-2.1#consequences-and-suggested-patterns

The coordinates of the challenged test(s)

• com.ibm.jbatch.tck.tests.jslxml.CDITests#testCDILookup(...)

The exact TCK version

2.1.1

The implementation being tested, including name and company

Apache Tomee 10.0.0-M1-SNAPSHOT (specifcally: Geronimo BatchEE 2.0.0-SNAPSHOT), The Apache Software Foundation

A full description of why the test is invalid and what the correct behavior is believed to be

Argument 1: Undefined CDI behaviour

As dicussed in #69, the test com.ibm.jbatch.tck.tests.jslxml.CDITests#testCDILookup(...) above relies on undefined (vendor-specific) CDI behaviour.

The test stated above contains the following code:

cdi.select(String.class, new BatchPropertyLiteral("prop1")).get();

depending on the CDI implementation (OWB or Weld), the following will happen:

• OWB will invoke the producer with a null InjectionPoint
• Weld will invoke the producer with a "fake" InjectionPoint with InjectionPoint#getMember == null.

The JavaDoc of InjectionPoint 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 our 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.

This is now also discussed in jakartaee/cdi#779

Argument 2: Definition of @BatchProperty

The spec says the following about @BatchProperty quoting from

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) ...

from https://jakarta.ee/specifications/batch/2.1/jakarta-batch-spec-2.1#batchproperty-definition

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.

from https://jakarta.ee/specifications/batch/2.1/jakarta-batch-spec-2.1#method-parameter-and-constructor-parameter-injection-with-explicit-name

Given those two quotes, the valid options for injecting a @BatchProperty are:

• param injection
• constructor injection
• field injection

but no CDI.current().select(...) / dynamic CDI lookups as tested in com.ibm.jbatch.tck.tests.jslxml.CDITests#testCDILookup(...) although dynamic CDI lookups are mentioned in https://jakarta.ee/specifications/batch/2.1/jakarta-batch-spec-2.1#consequences-and-suggested-patterns - this contradiction could maybe adressed with jakartaee/batch#209

Resolution / Solution

Maybe allow an exclusion of the test for now and than follow up with #70

Any supporting material; debug logs, test output, test logs, run scripts, etc.

• Make com.ibm.jbatch.tck.tests.jslxml.CDITests#testCDILookup(...) portable by using a bean indirection instead of jbatch annotations directly #69
• Define how InjectionPoint works when a bean is obtained via CDI.current() cdi#779

[rzo1]

Is there anything we need to do here? @scottkurz

[scottkurz]

Is there anything we need to do here? @scottkurz

No, I was actually just working on this. I had to grab some time to be able to do a few related tasks, building up to getting a release out the door soon.

I will ask you to review the specification text change as well when I complete that.

[scottkurz]

Posted PR for related spec change at jakartaee/batch#211