From 49645285d4434d7f7e48d246cbdc7f2ee76f81fd Mon Sep 17 00:00:00 2001
From: Damien Hollis <damien.hollis@unimarket.com>
Date: Fri, 2 Mar 2012 15:22:39 +1300
Subject: [PATCH] BATCH-1841 ChunkElementParser Inheritance Error

Fixes bug in ChunkElementParser that caused the
retryable-exception-classes and skippable-exception-classes
defined on parent bean to be ignored.

Polishing

Reverted spaces to tabs and added another test.
---
 .../configuration/xml/ChunkElementParser.java | 20 ++++++----
 .../xml/ChunkElementParserTests.java          | 37 ++++++++++++-----
 ...mentParentAttributeParserTests-context.xml | 40 ++++++++++++++++++-
 3 files changed, 77 insertions(+), 20 deletions(-)

diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/ChunkElementParser.java b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/ChunkElementParser.java
index 013714e88..f9b2ef44b 100644
--- a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/ChunkElementParser.java
+++ b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/ChunkElementParser.java
@@ -15,8 +15,6 @@
  */
 package org.springframework.batch.core.configuration.xml;
 
-import java.util.List;
-
 import org.springframework.batch.core.listener.StepListenerMetaData;
 import org.springframework.batch.core.step.item.ForceRollbackForWriteSkipException;
 import org.springframework.batch.repeat.policy.SimpleCompletionPolicy;
@@ -36,6 +34,8 @@ import org.springframework.util.StringUtils;
 import org.springframework.util.xml.DomUtils;
 import org.w3c.dom.Element;
 
+import java.util.List;
+
 /**
  * Internal parser for the &lt;chunk/&gt; element inside a step.
  * 
@@ -127,9 +127,11 @@ public class ChunkElementParser {
 			}
 			propertyValues.addPropertyValue("skipLimit", skipLimit);
 		}
-		// Even if there is no retryLimit, we can still accept exception
-		// classes for an abstract parent bean definition
-		propertyValues.addPropertyValue("skippableExceptionClasses", skippableExceptions);
+		if (skippableExceptions != null) {
+			// Even if there is no retryLimit, we can still accept exception
+			// classes for an abstract parent bean definition
+			propertyValues.addPropertyValue("skippableExceptionClasses", skippableExceptions);
+		}
 
 		handleItemHandler(bd, "skip-policy", "skipPolicy", null, false, element, parserContext, propertyValues,
 				underspecified);
@@ -143,9 +145,11 @@ public class ChunkElementParser {
 			}
 			propertyValues.addPropertyValue("retryLimit", retryLimit);
 		}
-		// Even if there is no retryLimit, we can still accept exception
-		// classes for an abstract parent bean definition
-		propertyValues.addPropertyValue("retryableExceptionClasses", retryableExceptions);
+		if (retryableExceptions != null) {
+			// Even if there is no retryLimit, we can still accept exception
+			// classes for an abstract parent bean definition
+			propertyValues.addPropertyValue("retryableExceptionClasses", retryableExceptions);
+		}
 
 		handleItemHandler(bd, "retry-policy", "retryPolicy", null, false, element, parserContext, propertyValues,
 				underspecified);
diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/ChunkElementParserTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/ChunkElementParserTests.java
index 1f777b9b1..33d2edc66 100644
--- a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/ChunkElementParserTests.java
+++ b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/ChunkElementParserTests.java
@@ -15,16 +15,6 @@
  */
 package org.springframework.batch.core.configuration.xml;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Map;
-
 import org.junit.Test;
 import org.springframework.batch.classify.SubclassClassifier;
 import org.springframework.batch.core.Step;
@@ -49,6 +39,17 @@ import org.springframework.dao.PessimisticLockingFailureException;
 import org.springframework.test.util.ReflectionTestUtils;
 import org.springframework.util.StringUtils;
 
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
 /**
  * @author Dan Garrette
  * @author Dave Syer
@@ -183,6 +184,22 @@ public class ChunkElementParserTests {
 		containsClassified(retryable, CannotSerializeTransactionException.class, false);
 	}
 
+	@Test
+	public void testRetryableInherited() throws Exception {
+		Map<Class<? extends Throwable>, Boolean> retryable = getRetryableExceptionClasses("s3", getContext());
+		System.err.println(retryable);
+		assertEquals(2, retryable.size());
+		containsClassified(retryable, IOException.class, true);
+	}
+
+	@Test
+	public void testRetryableInheritedMerge() throws Exception {
+		Map<Class<? extends Throwable>, Boolean> retryable = getRetryableExceptionClasses("s4", getContext());
+		System.err.println(retryable);
+		assertEquals(3, retryable.size());
+		containsClassified(retryable, IOException.class, true);
+	}
+
 	@Test
 	public void testInheritSkippable() throws Exception {
 		Map<Class<? extends Throwable>, Boolean> skippable = getSkippableExceptionClasses("s1", getContext());
diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/ChunkElementParentAttributeParserTests-context.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/ChunkElementParentAttributeParserTests-context.xml
index aef8df686..77fcf0f6a 100644
--- a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/ChunkElementParentAttributeParserTests-context.xml
+++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/ChunkElementParentAttributeParserTests-context.xml
@@ -30,7 +30,7 @@
 			</tasklet>
 		</step>
 
-		<step id="s2" parent="baseStep">
+		<step id="s2" parent="baseStep" next="s3">
 			<tasklet>
 				<chunk reader="reader" writer="writer" commit-interval="5" skip-limit="5" retry-limit="3">
 					<skippable-exception-classes>
@@ -51,15 +51,51 @@
 				</chunk>
 			</tasklet>
 		</step>
+
+		<step id="s3" parent="baseStep" next="s4">
+			<tasklet>
+				<chunk reader="reader" writer="writer" commit-interval="5">
+					<streams>
+						<stream ref="stream1"/>
+					</streams>
+					<retry-listeners>
+						<listener>
+							<beans:bean class="org.springframework.batch.core.configuration.xml.DummyRetryListener"/>
+						</listener>
+					</retry-listeners>
+				</chunk>
+			</tasklet>
+		</step>
+
+		<step id="s4" parent="baseStep">
+			<tasklet>
+				<chunk reader="reader" writer="writer" commit-interval="5">
+					<retryable-exception-classes merge="true">
+						<include class="java.lang.RuntimeException"/>
+					</retryable-exception-classes>
+					<streams>
+						<stream ref="stream1"/>
+					</streams>
+					<retry-listeners>
+						<listener>
+							<beans:bean class="org.springframework.batch.core.configuration.xml.DummyRetryListener"/>
+						</listener>
+					</retry-listeners>
+				</chunk>
+			</tasklet>
+		</step>
 	</job>
 
 	<step id="baseStep" abstract="true">
 		<tasklet>
-				<chunk>
+				<chunk retry-limit="6" skip-limit="7">
 					<skippable-exception-classes>
 						<include class="java.lang.ArithmeticException"/>
 						<exclude class="org.springframework.dao.DeadlockLoserDataAccessException"/>
 					</skippable-exception-classes>
+					<retryable-exception-classes>
+						<include class="java.io.IOException"/>
+					</retryable-exception-classes>
 					<streams>
 						<stream ref="stream2"/>
 					</streams>