Bug 5403

Summary: Spec unclear on skipping part of an Exception hierarchy
Product: jbatch Reporter: mminella
Component: SPECAssignee: ScottKurz
Status: REOPENED ---    
Severity: normal CC: issues
Priority: P5    
Version: 1   
Target Milestone: ---   
Hardware: All   
OS: All   
Whiteboard: 1.0_mr_pending tck_1.0

Description mminella 2013-09-20 19:18:40 UTC
This bug is in reference to the email conversation started here: https://java.net/projects/jbatch/lists/public/archive/2013-09/message/12

In short, the spec is not clear on how the <include> and <exclude> logic works for the evaluation of an exception to be skipped/retried/etc.  In Spring Batch, the logic is based on this statement:

"For any exception encountered, the skippability will be determined by the nearest superclass in the class hierarchy. Any unclassifed exception will be treated as 'fatal'. The order of the <include/>and <exclude/> elements does not matter." [http://docs.spring.io/spring-batch/reference/html/configureStep.html#configuringSkip]

The TCK interprets the spec as apply include elements, then exclude elements.  So if we have the following exception hierarchy:

MyParentException --> MyChildException --> MyGrandchildException 

and we configure skip logic as follows:
<skippable-exception-classes>
    <include class="MyParentException"/>
    <include class="MyGrandchildException"/>
    <exclude class="MyChildException"/>
</skippable-exception-classes>

the job is skip MyParentException and NOT skip either MyChildException or MyGrandchildException.  My expectation would be that MyParentException and MyGrandchildException should be skipped and only MyChildException should be considered a fatal exception.
Comment 1 ScottKurz 2013-10-28 21:02:12 UTC
I agree that given the choice between the current algorithm used by the RI & TCK to combine include/exclude(s) and the alternative Michael mentioned, the alternative is better.   It's definitely more expressive and arguably more intuitive.

I would like to update both the TCK and the spec, then, accordingly.

My concern is to abide by whatever rules exist so as to not break anyone who's passing the TCK based on the current interpretation.  

Still trying to sort through these issues but plan to have an update shortly.
Comment 2 ScottKurz 2013-11-07 05:38:43 UTC
(In reply to ScottKurz from comment #1)
> I agree that given the choice between the current algorithm used by the RI &
> TCK to combine include/exclude(s) and the alternative Michael mentioned, the
> alternative is better.   It's definitely more expressive and arguably more
> intuitive.
> 
> I would like to update both the TCK and the spec, then, accordingly.
> 
> My concern is to abide by whatever rules exist so as to not break anyone
> who's passing the TCK based on the current interpretation.  
> 
> Still trying to sort through these issues but plan to have an update shortly.

Since this is only affects a test or two.. I think what we should do is:

1) exclude the test in the 1.0 release
2) update the spec in the (presumed) maintenance release to clarify include-exclude-include behavior as Michael suggested 
3) update the TCK by reworking this test at the same maintenance release.  

That leaves 1.0 a bit underspecified in this area, but hopefully those following this thread can set a precedent until 1.1.    

Thoughts?
Comment 3 ScottKurz 2013-11-09 19:06:30 UTC
Will mark with 'tck_1.0' to remind us to formally deliver the exclusion.
Comment 4 ScottKurz 2014-01-03 03:35:29 UTC
Would like to fix the behavior in a presumed 1.1 spec update and at that time, re-introduce a corrected form of this test.   I don't think I'll bother opening a bug just yet, when we haven't even started on 1.1.
Comment 5 ScottKurz 2014-05-09 17:15:31 UTC
Just remembered this change fell through the cracks.   When I first closed it out I thought it would be against JCP best practice to introduce a breaking change with a spec errata type of maintenance release.

However, we have other changes in our MR that are similarly disruptive (of edge cases).  

So I'm going to write this up and add it to the current MR change log.  I do remember we all agreed on the outcome here.
Comment 6 ScottKurz 2014-05-15 21:55:13 UTC
In Section 8.2.1.4.1 Skipping Exceptions:
(similar changes to retry exception paragraphs in Sections
8.2.1.4.2
8.2.1.4.5)

Change this text:
----------------

  The list of exception that will be skipped is specified in the skippable-exception-classes element on the child include element. One or more include child elements may be specified to specify the exception classes or superclasses that will be skipped. The list of exceptions to be skipped can be reduced by the exclude child element. 

To this text:
----------------

A given exception will be skipped if it "matches" an 'include' child element of the 'skippable-exception-classes' element, though this might be negated (and the exception not skipped) if it also "matches" an 'exclude' child element of 'skippable-exception-classes'.

The behavior is determined by the "nearest superclass" in the class hierarchy.

To elaborate, in this context, "matches" means the following:  For an include (or exclude) element C with @class attribute value T, an exception E "matches" C when either E is of type T or E's type is a subclass of T.

When an exception E "matches" both one or more 'include' and one or more 'exclude' elements, then there will be one type T1 among all the matching include/exclude elements such that all other distinct matching element types are superclasses of T1 (because of Java's single inheritance).    If T1 only occurs in a matching include element then include (skip) this exception.  If T1 appears in a matching exclude element (even if it also appears in a matching include element), then exclude (don't skip) this exception.

----------------

With this approach I think a more natural XSD would allow freedom to intermix the includes vs. excludes.. which we still don't allow, but that's a slightly more disruptive change and I'm not sure it's worth it until someone complains more.