Bug 4085 - Missing spec detail on problematic inheritance cases
Missing spec detail on problematic inheritance cases
Status: CLOSED FIXED
Product: jbatch
Classification: Unclassified
Component: source
1
All All
: P5 blocker
: ---
Assigned To: cvignola
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-31 18:12 UTC by ctedlock
Modified: 2013-01-16 18:29 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 ctedlock 2012-08-31 18:12:10 UTC
Hi,

Below are a few problematic paths that came up while implementing inheritance section 5.8 of the spec. The examples given are narrow in scope, and it's not clear what should happen in the general case.

Thanks,
Collin

Problematic Inheritance Cases:


1. Inherit leads to schema invalid result

parent:
<step id="parent">
	<chunk reader="r" writer="w" processor="p" chunk-size="1"/>
</step>

child:
<step id="child" parent="parent">
	<batchlet ref="b"/>
</step>

anticipated merge:

<step id="child">
	<batchlet ref="b"/>
	<chunk reader="r" writer="w" processor="p" chunk-size="1"/>
</step>




2. Inherited subelement id collides with existing element

parent:
<job id="parent">
	<step id="step" next="next">
		<batchlet ref="b"/>
	</step>
</job>

child:
<job id="child" parent="parent">
	<step id="step" start-limit="1">
		<properties><property name="n" value="v"/></properties>
	</step>
</job>

Merge option 1, child overrides:
<job id="child">
	<step id="step" start-limit="1">
		<properties><property name="n" value="v"/></properties>
	</step>
</job>

Merge option 2, parent overrides:
<job id="child">
	<step id="step" next="next">
		<batchlet ref="b"/>
	</step>
</job>

Merge option 3, merge attributes only:
<job id="child">
	<step id="step" start-limit="1" next="next">
		<properties><property name="n" value="v"/></properties>
	</step>
</job>

Merge option 4, full recursive merge of subelements:
<job id="child">
	<step id="step" start-limit="1" next="next">
		<properties><property name="n" value="v"/></properties>
		<batchlet ref="b"/>
	</step>
</job>


3. Inherit colliding id from dissimilar element

parent:
<job id="parent">
	<step id="collide"/>
</job>

child:
<job id="child" parent="parent">
	<split id="collide"/>
</job>

Merge option 1, error:
Throw an exception on id collision.

Merge option 2, loose merge without validation:
<job id="child">
	<step id="collide"/>
	<split id="collide"/>
</job>

Merge option 3, child overrides:
<job id="child">
	<split id="collide"/>
</job>

Merge option 4, parent overrides:
<job id="child">
	<step id="collide"/>
</job>
Comment 1 cvignola 2012-09-07 15:10:32 UTC
Good questions.  Let me respond by first asserting the general rule and then applying to your questions.  

General rule: 

Inheritance combines the elements and attributes of the child and parent, with the child overriding parent attributes. 

Aside: I presume inheritance occurs first, and validation of the resultant job definition second. 

Applied to your questions:

1. Combine the elements, allow validation to flag error - i.e. mutually exclusive elements.

2. Merge option 4 (i.e. combine parent and child elements) 

3. Merge option 2 (i.e. combine parent and child elements, allow validation to flag error - i.e. duplicate ids)

Additional note:  remember that lists (<listeners> and <properties>) get special treatment with the merge=true|false attribute.  True means combine the child and parent lists;  false means child list overrides parent list.
Comment 2 ctedlock 2012-09-10 15:54:43 UTC
Thanks Chris, that helps clear some things up.

There's a problem #4 here as well, as relates to ordering of elements when inheriting. See below.

Thanks,
Collin


4. How to order inherited elements. There are three subcases:
4a. Inherited properties at any level

The spec imposes no ordering, and intuitively it would seem that property order doesn't matter. That is:

<properties>
  <property name="prop1" value="prop1"/>
  <property name="prop2" value="prop2"/>
</properties>

is equivalent to:

<properties>
  <property name="prop2" value="prop2"/>
  <property name="prop1" value="prop1"/>
</properties>

Is this true?


4b. Inherited listeners at any level
Per the spec, e.g.; section 5.1.1: "If multiple job listeners are configured on the same job,  they are invoked in the order they are specified."
Therefore listener order affects code execution.

Parent listener XML:
<job id="parent">
  <listeners>
    <listener ref="parentListener"/>
  </listeners>
</job>

Child listener XML:
<job id="child" parent="parent">
  <listeners merge="true">
    <listener ref="childListener"/>
  </listeners>
</job>

Potential merge 1 (parent comes first):
<job id="child">
  <listeners>
    <listener ref="parentListener"/>
    <listener ref="childListener"/>
  </listeners>
</job>

Potential merge 2 (child comes first):
<job id="child">
  <listeners>
    <listener ref="childListener"/>
    <listener ref="parentListener"/>
  </listeners>
</job>

Which should happen? The example in section 5.1.1 of the spec suggests, but does not explicitly state, that option 1 is correct.


4c. Inherited execution elements
Execution element is not a term formally defined by the spec, but section 6.4.1.2 uses it to describe a step, split, or flow. I use it here loosely to describe the logical flow of a job.

Per 5.2.5: "The first step, flow, or split defines the first step (flow or split) to execute for a given Job XML.  The 'next' attribute on the step, flow, or split defines what executes next."
Per 5.2, the 'next' attribute "specifies the next step, flow, split, or decision to run after this step is complete.  It must be a valid XML string value.  This is an optional attribute.   The default is this step is the last step in the job."

<job id="parent">
  <step id="parentStep"/>
</job>

<job id="child" parent="parent">
  <step id="childStep"/>
</job>

Merge option 1 (parent goes first):
<job id="child">
  <step id="parentStep"/>
  <step id="childStep"/>
</job>
Here, parentStep goes first by virtue of being first element, executes first. The childStep, by virtue of 1) being the only other step, and 2) being the only step without a next attribute, will go last. (Side note: what happens if there is a third step here, also without a next attribute?)


Merge option 2 (parent goes last):
<job id="child">
  <step id="childStep"/>
  <step id="parentStep"/>
</job>
Here, the reverse happens. Which is correct?
Comment 3 cvignola 2012-09-10 16:17:59 UTC
(In reply to comment #2) 
Good questions.  Answers: 
> 4. How to order inherited elements. There are three subcases:
The principle is natural order.  Let's see how that affects outcomes below:
> 4a. Inherited properties at any level
> The spec imposes no ordering, and intuitively it would seem that property order
> doesn't matter. 
Correct:  property order does not matter.  However, when merge=true, properties should be ordered parent-first, child-second - i.e. natural order.
> 4b. Inherited listeners at any level
Parent-first, child-second (i.e. your option 1)
> 4c. Inherited execution elements
Parent-first, child-second (i.e. your option 1)
Comment 4 cvignola 2012-10-10 13:52:30 UTC
After further discussion with the EG,  we have decided to match Spring Batch inheritance rules.  I believe that means:

1. jobs can inherit jobs, but not parent's steps, flows, splits 

2. steps can inherit steps 

3. non-intersecting elements are merged 

4. non-intersecting attributes on intersecting elements are merged 

5. intersecting attributes on intersecting elements result in child overrides parent attribute

6. lists support merge=true|false attribute.  Note: the order in which listeners are invokes is not guaranteed, so the order in which they are merged does not matter.
Comment 5 ctedlock 2012-10-10 14:17:35 UTC
(In reply to comment #4)

Does #1 mean that when inheriting from a job we do not pull in its steps, flows, and splits into the final merged job? i.e.; we only merge properties and listeners?

#2 only mentions steps explicitly; combined with #1 then are flows and splits no longer inheritable at all?

#3-#5 seem to imply no changes to direction except that we are restricting what can be merged by #1 and #2.

For #6:
In the 3.0 spec, (Sec 5.1.1), the order of the listeners is stated to matter:

"Multiple listeners may be configured on a job.  A job listener is invoked according to its relationship to the job life cycle.  If multiple job listeners are configured on the same job,  they are invoked in the order they are specified.  "

Is that changing or should this be reconsidered?
Comment 6 cvignola 2012-10-10 14:30:30 UTC
Does #1 mean that when inheriting from a job we do not pull in its steps,
flows, and splits into the final merged job? i.e.; we only merge properties and
listeners?

Correct.

#2 only mentions steps explicitly; combined with #1 then are flows and splits
no longer inheritable at all?

Correct.

#3-#5 seem to imply no changes to direction except that we are restricting what
can be merged by #1 and #2.

Correct. 

For #6:
In the 3.0 spec, (Sec 5.1.1), the order of the listeners is stated to matter:

"Multiple listeners may be configured on a job.  A job listener is invoked
according to its relationship to the job life cycle.  If multiple job listeners
are configured on the same job,  they are invoked in the order they are
specified.  "

Is that changing or should this be reconsidered?  

Listeners order is not guaranteed.  The spec draft was in error.