@BatchProperty injection should keep field default value in case of no matching batch property

Originally opened as bug 6796 by cf126330

--------------Original Comment History---------------------------- Comment from = cf126330 on 2015-03-07 17:08:02 +0000

Section 9.3.1 states:

The value of the annotated field is assigned by the batch runtime if a corresponding property element with a matching name is specified in the JSL in the scope that applies to the batch artifact in question.

So if no matching batch property is found for a @BatchProperty field, batch runtime should not attempt to inject anything, and thus preserving the java default value for the field.

I did a quick test in GlassFish, and it shows a null value is injected, and the java default value is overwritten.

@Inject @BatchProperty String undefinedProperty = "default value for undefinedProperty";

Comment from = BrentDouglas on 2015-03-07 18:41:01 +0000

If I'm reading your post correctly then I disagree that this is a spec compliance issue. Later in section 9.3.1 is the following:

The resulting value might simply be the Java default value, however using various dependency injection technologies may produce different results. The resultant behavior may be defined by the particular dependency injection technology used in the runtime environment and so is outside the scope of this > specification.

And of particular relevance:

If no value is defined in JSL, the Java default (null) is assigned or some other default is provided by a particular dependency injection technology.

So the behavior is:

If "undefinedProperty" is not defined in your JSL 'the Java default (null) is assigned' and the current behavior is compliant.
If you have a property "undefinedProperty" defined in your JSL which resolves to "" then 'no assignment is made'.

So if by 'no matching batch property is found' you mean that "undefinedProperty" is not defined in your JSL then the behavior you have described is compliant with the spec.

Comment from = cf126330 on 2015-03-07 19:16:10 +0000

I think the spec makes a distinction between: (1) a batch property is not defined in job xml at all; and (2) a batch property appears in job xml but resolves to non-meaningful value (empty string). My reading is, what you've quoted applies to the latter case. The first sentense (see quote my original comment) establish the context for the subsequent long and detailed explanation.

For case 1, since the intended batch property does not exist, default java behavior should be preserved, and no batch property injection should be happening. In my example above, undefinedProperty field should hold value "default value for undefinedProperty" (the java default value) after the artifact is created.

This is consistent with resource injections in Java EE.

Comment from = BrentDouglas on 2015-03-07 20:20:05 +0000

I think the spec makes a distinction between: (1) a batch property is not defined in job xml at all; and (2) a batch property appears in job xml but resolves to non-meaningful value (empty string).

Yes.

My reading is, what you've quoted applies to the latter case.

I quoted both cases.

The first sentense (see quote my original comment) establish the context for the subsequent long and detailed explanation.

If you post the JSL document used it would remove any area for misinterpretation.

For case 1, since the intended batch property does not exist, default java behavior should be preserved, and no batch property injection should be happening.

From here down I will only be discussing case 1: "a batch property is not defined in job xml at all"

The spec does not require that "no batch property injection should be happening". The spec area that applies is:

If no value is defined in JSL, the Java default (null) is assigned or some other default is provided by a particular dependency injection technology.

Which could also be expressed "If you are using a DI framework the field can be anything at all but if not the field will be assigned null". Assuming you are not using a DI framework in your example, the value of null is the correct value to be assigned.

In my example above, undefinedProperty field should hold value "default value for undefinedProperty" (the java default value) after the artifact is created.

The spec clearly indicates that in this case the term "Java default" refers to null. I would say calling it the java default is a reference to section 4.12.5 of the JLS "For all reference types (�4.3), the default value is null" (http://docs.oracle.com/javase/specs/jls/se7/html/jls-4.html#jls-4.12.5).

This is consistent with resource injections in Java EE.

That sounds like you are arguing that the spec needs changing rather than the RI is not implementing it correctly which is a different issue.

Comment from = ScottKurz on 2015-03-11 21:55:38 +0000

Cheng,

I'd originally thought the spec wording was clear and covered all cases, and had the same interpretation that Brent suggested across all these cases.

But it sounds like you're taking words such as: "If no value is defined in JSL, ..." (beginning of last sentence of 9.3.1)

to imply that we are already in the case where there is a JSL property, which has been "resolved" to the empty string. It sounds like you're drawing a distinction between this case and the case where there is no JSL property name matching the @BatchProperty name.

OK, let's assume the wording could have been clearer.

As the next step, I'd like to ask whether you are suggesting the spec should be updated or whether the RI behavior should be different?

You mentioned "This is consistent with resource injections in Java EE." Could you elaborate on that?

Thanks, Scott

Comment from = cf126330 on 2015-03-12 14:38:09 +0000

For Java EE resource injections, please see Java EE 7 Platform spec, section EE.5.4.1.3 Declaration of Simple Environment Entries. To quote the relevant part:

It�s often convenient to declare a field or method as an injection target, but specify a default value in the code, as illustrated in the following example. // The maximum number of tax exemptions, configured by the Deployer. @Resource int maxExemptions = 4; // defaults to 4 To support this case, the container must only inject a value for this resource if the deployer has specified a value to override the default value.

I think that's the most intuitive defaulting behavior from a developer's perspective. Of course, batch spec doesn't have to follow Java EE spec, especially it's something specific to batch domain. But since this case is more about general property injection, my personal perference is to align with Java EE spec.

The current batch spec gives quite some leeway for implementations in this area. I hope RI could be updated towards a more sensible injection default. To take it one step further, the spec could also benefit from some clarification and remove some ambiguity.

Related discussion on cdi-dev (using cdi as one mechanism of injection) http://lists.jboss.org/pipermail/cdi-dev/2015-March/thread.html

Comment from = ScottKurz on 2015-03-18 21:19:34 +0000

(Sorry for dragging out the conversation here..)

OK, let's talk about the RI first then, and consider the spec later.

We tried to optimize our "custom batch injection" for compatibility with CDI. (This in spite of the fact we didn't do some simple things like honoring the injection on fields defined on a batch artifact superclass).

Cheng, were you able to solve the problem of NOT having CDI overwrite the initializer value with a 'null'? I couldn't tell from the cdi-dev thread what the conclusion was.

Now I'm realizing I'd been reading the situation as "CDI injects a null..". Actually, of course, the CDI is acting through the batch implementation's producer, so maybe there's room for batch to be doing things differently.

If so, I think preserving the intializer value is clearly more useful in batch, in general, so would be interested to revisit.

I'm also observing your EE platform quote referred to @Resource, not @Inject (not sure if that matters, just noting as I haven't had time to research myself).

Comment from = cf126330 on 2015-03-18 23:44:11 +0000

From cdi-dev discussion, it seems there is no portable way to opt out CDI injection within producer methods. Therefore, if we try to get property value for non-existent batch property (and we will get null), and if the producer method returns null to CDI, CDI will just inject null into that field, effectively overwriting whatever value is already in that field.

I recently added some fix to JBeret to get the field value (best-effort attemp) if the requested property does not exist, and have the producer method return that value to CDI to essentially let CDI to re-inject it. This is not as good as avoiding injecting, but still better than injecting null.

@Resource was introduced in Java EE 5, predating CDI. Now I think all injections in Java EE are trying to converge into CDI, at least it appears so. So I would think the requirement for @Resource also apply to @Inject, however it is implemented.

Comment from = ScottKurz on 2015-09-01 21:23:35 +0000

Whatever we decide for the RI, the spec should be amended to clarify that using the value of the Java initializer is allowed.

I think it is the most useful approach in the non-CDI case. Plus it sounds like JBeret is doing this (in contrast to the RI). WebSphere is keeping the initializer value as well.

jakartaee / batch