Bug 5403 - Spec unclear on skipping part of an Exception hierarchy
Spec unclear on skipping part of an Exception hierarchy
Status: REOPENED
Product: jbatch
Classification: Unclassified
Component: TCK
1
All All
: P5 normal
: ---
Assigned To: ScottKurz
tck_1.0
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-09-20 19:18 UTC by mminella
Modified: 2015-09-03 15:24 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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.
Comment 7 ScottKurz 2015-09-03 15:24:00 UTC
Assigning to TCK.

As far as the proposed XSD change, will track along with the greater issue of
https://java.net/bugzilla/show_bug.cgi?id=7282