diff --git a/spring-batch-core/.settings/com.springsource.sts.config.flow.prefs b/spring-batch-core/.settings/com.springsource.sts.config.flow.prefs index 5789486ba..7fd2dae1e 100644 --- a/spring-batch-core/.settings/com.springsource.sts.config.flow.prefs +++ b/spring-batch-core/.settings/com.springsource.sts.config.flow.prefs @@ -1,6 +1,10 @@ -#Tue Mar 15 13:48:56 GMT 2011 +#Wed May 04 11:15:42 BST 2011 //com.springsource.sts.config.flow.coordinates\:http\://www.springframework.org/schema/batch\:/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/JobExecutionListenerMethodAttributeParserTests-context.xml=\n\n\n\n\n\n //com.springsource.sts.config.flow.coordinates\:http\://www.springframework.org/schema/batch\:/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/JobRepositoryDefaultParserTests-context.xml=\n +//com.springsource.sts.config.flow.coordinates\:http\://www.springframework.org/schema/batch\:/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/ParentRetryableLateBindingStepFactoryBeanParserTests-context.xml=\n\n\n\n\n\n\n\n\n\n +//com.springsource.sts.config.flow.coordinates\:http\://www.springframework.org/schema/batch\:/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/ParentRetryableStepFactoryBeanParserTests-context.xml=\n\n\n\n\n\n\n\n\n\n +//com.springsource.sts.config.flow.coordinates\:http\://www.springframework.org/schema/batch\:/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/ParentSkippableLateBindingStepFactoryBeanParserTests-context.xml=\n\n\n\n\n\n\n\n\n\n +//com.springsource.sts.config.flow.coordinates\:http\://www.springframework.org/schema/batch\:/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/ParentSkippableStepFactoryBeanParserTests-context.xml=\n\n\n\n\n\n\n\n\n\n //com.springsource.sts.config.flow.coordinates\:http\://www.springframework.org/schema/batch\:/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/PartitionStepParserTests-context.xml=\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n //com.springsource.sts.config.flow.coordinates\:http\://www.springframework.org/schema/batch\:/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/StepListenerInStepParserTests-context.xml=\n\n\n\n\n\n //com.springsource.sts.config.flow.coordinates\:http\://www.springframework.org/schema/batch\:/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/StepListenerMethodAttributeParserTests-context.xml=\n\n\n\n\n\n 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 4358a76d6..bc6b43a1c 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 @@ -124,11 +124,11 @@ public class ChunkElementParser { ManagedMap skippableExceptions = handleExceptionElement(element, parserContext, "skippable-exception-classes"); boolean hasSkipPolicy = false; if (StringUtils.hasText(skipLimit)) { - if (skippableExceptions == null) { - parserContext.getReaderContext().error( - "The element must have skippable-exceptions if a skip-limit is specified.", element); - } if (skipLimit.startsWith("#")) { + if (skippableExceptions == null) { + parserContext.getReaderContext().error( + "The element must have skippable-exceptions if a skip-limit is specified.", element); + } // It's a late binding expression, so we need step scope... BeanDefinitionBuilder skipPolicy = BeanDefinitionBuilder .genericBeanDefinition(LimitCheckingItemSkipPolicy.class); @@ -139,6 +139,10 @@ public class ChunkElementParser { hasSkipPolicy = true; } else { + if (skippableExceptions == null) { + skippableExceptions = new ManagedMap(); + skippableExceptions.setMergeEnabled(true); + } propertyValues.addPropertyValue("skipLimit", skipLimit); } } @@ -155,11 +159,11 @@ public class ChunkElementParser { ManagedMap retryableExceptions = handleExceptionElement(element, parserContext, "retryable-exception-classes"); boolean hasRetryPolicy = false; if (StringUtils.hasText(retryLimit)) { - if (retryableExceptions == null) { - parserContext.getReaderContext().error( - "The element must have retryable-exceptions if a retry-limit is specified.", element); - } if (retryLimit.startsWith("#")) { + if (retryableExceptions == null) { + parserContext.getReaderContext().error( + "The element must have retryable-exceptions if a retry-limit is specified.", element); + } // It's a late binding expression, so we need step scope... BeanDefinitionBuilder retryPolicy = BeanDefinitionBuilder .genericBeanDefinition(SimpleRetryPolicy.class); @@ -170,6 +174,10 @@ public class ChunkElementParser { hasRetryPolicy = true; } else { + if (retryableExceptions == null) { + retryableExceptions = new ManagedMap(); + retryableExceptions.setMergeEnabled(true); + } propertyValues.addPropertyValue("retryLimit", retryLimit); } } diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/FaultTolerantStepFactoryBean.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/FaultTolerantStepFactoryBean.java index a3f9565be..40083a880 100755 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/FaultTolerantStepFactoryBean.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/FaultTolerantStepFactoryBean.java @@ -59,6 +59,7 @@ import org.springframework.core.task.TaskExecutor; import org.springframework.transaction.TransactionException; import org.springframework.transaction.interceptor.DefaultTransactionAttribute; import org.springframework.transaction.interceptor.TransactionAttribute; +import org.springframework.util.Assert; /** * Factory bean for step that provides options for configuring skip behaviour. @@ -393,9 +394,13 @@ public class FaultTolerantStepFactoryBean extends SimpleStepFactoryBean, Boolean> map = new HashMap, Boolean>( + skippableExceptionClasses); + map.put(ForceRollbackForWriteSkipException.class, true); + LimitCheckingItemSkipPolicy limitCheckingItemSkipPolicy = new LimitCheckingItemSkipPolicy(skipLimit, map); if (skipPolicy == null) { + Assert.state(!(skippableExceptionClasses.isEmpty() && skipLimit > 0), + "If a skip limit is provided then skippable exceptions must also be specified"); skipPolicy = limitCheckingItemSkipPolicy; } else if (limitCheckingItemSkipPolicy != null) { @@ -429,16 +434,6 @@ public class FaultTolerantStepFactoryBean extends SimpleStepFactoryBean, Boolean> getSkippableExceptionClasses() { - Map, Boolean> map = new HashMap, Boolean>( - skippableExceptionClasses); - map.put(ForceRollbackForWriteSkipException.class, true); - return map; - } - /** * @return fully configured retry template for item processing phase. */ @@ -453,6 +448,8 @@ public class FaultTolerantStepFactoryBean extends SimpleStepFactoryBean 0), + "If a retry limit is provided then retryable exceptions must also be specified"); retryPolicy = simpleRetryPolicy; } else if ((!retryableExceptionClasses.isEmpty() && retryLimit > 0)) { 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 1f734905d..1f777b9b1 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 @@ -38,7 +38,6 @@ import org.springframework.batch.retry.listener.RetryListenerSupport; import org.springframework.batch.retry.policy.SimpleRetryPolicy; import org.springframework.beans.PropertyAccessorUtils; import org.springframework.beans.factory.BeanCreationException; -import org.springframework.beans.factory.parsing.BeanDefinitionParsingException; import org.springframework.context.ApplicationContext; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.support.ClassPathXmlApplicationContext; @@ -91,8 +90,8 @@ public class ChunkElementParserTests { "org/springframework/batch/core/configuration/xml/ChunkElementIllegalSkipAndRetryAttributeParserTests-context.xml"); Step step = (Step) context.getBean("s1", Step.class); assertNotNull("Step not parsed", step); - fail("Expected BeanDefinitionParsingException"); - } catch (BeanDefinitionParsingException e) { + fail("Expected BeanCreationException"); + } catch (BeanCreationException e) { // expected } } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/ParentStepFactoryBeanParserTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/ParentStepFactoryBeanParserTests.java index 2ec531b2e..c0d1cc961 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/ParentStepFactoryBeanParserTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/ParentStepFactoryBeanParserTests.java @@ -18,6 +18,7 @@ package org.springframework.batch.core.configuration.xml; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; +import org.junit.Ignore; import org.junit.Test; import org.springframework.batch.core.Step; import org.springframework.batch.core.step.item.FaultTolerantChunkProcessor; @@ -53,4 +54,39 @@ public class ParentStepFactoryBeanParserTests { assertTrue("Wrong processor type", chunkProcessor instanceof FaultTolerantChunkProcessor); } + @Test + public void testRetryableAttributes() throws Exception { + ConfigurableApplicationContext context = new ClassPathXmlApplicationContext( + "org/springframework/batch/core/configuration/xml/ParentRetryableStepFactoryBeanParserTests-context.xml"); + Object step = context.getBean("s1", Step.class); + assertNotNull("Step not parsed", step); + Object tasklet = ReflectionTestUtils.getField(step, "tasklet"); + Object chunkProcessor = ReflectionTestUtils.getField(tasklet, "chunkProcessor"); + assertTrue("Wrong processor type", chunkProcessor instanceof FaultTolerantChunkProcessor); + } + + @Test + @Ignore // Fix this BATCH-1739 + public void testRetryableLateBindingAttributes() throws Exception { + ConfigurableApplicationContext context = new ClassPathXmlApplicationContext( + "org/springframework/batch/core/configuration/xml/ParentRetryableLateBindingStepFactoryBeanParserTests-context.xml"); + Object step = context.getBean("s1", Step.class); + assertNotNull("Step not parsed", step); + Object tasklet = ReflectionTestUtils.getField(step, "tasklet"); + Object chunkProcessor = ReflectionTestUtils.getField(tasklet, "chunkProcessor"); + assertTrue("Wrong processor type", chunkProcessor instanceof FaultTolerantChunkProcessor); + } + + @Test + @Ignore // Fix this BATCH-1739 + public void testSkippableLateBindingAttributes() throws Exception { + ConfigurableApplicationContext context = new ClassPathXmlApplicationContext( + "org/springframework/batch/core/configuration/xml/ParentSkippableLateBindingStepFactoryBeanParserTests-context.xml"); + Object step = context.getBean("s1", Step.class); + assertNotNull("Step not parsed", step); + Object tasklet = ReflectionTestUtils.getField(step, "tasklet"); + Object chunkProcessor = ReflectionTestUtils.getField(tasklet, "chunkProcessor"); + assertTrue("Wrong processor type", chunkProcessor instanceof FaultTolerantChunkProcessor); + } + } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/StepParserStepFactoryBeanTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/StepParserStepFactoryBeanTests.java index c5f14a64c..e4a49608d 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/StepParserStepFactoryBeanTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/StepParserStepFactoryBeanTests.java @@ -20,6 +20,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import java.util.HashMap; +import java.util.Map; import org.junit.Test; import org.springframework.aop.framework.Advised; @@ -226,8 +227,10 @@ public class StepParserStepFactoryBeanTests { fb.setSkipLimit(100); fb.setThrottleLimit(10); fb.setRetryListeners(new RetryListenerSupport()); - fb.setSkippableExceptionClasses(new HashMap, Boolean>()); - fb.setRetryableExceptionClasses(new HashMap, Boolean>()); + @SuppressWarnings("unchecked") + Map, Boolean> exceptionMap = getExceptionMap(Exception.class); + fb.setSkippableExceptionClasses(exceptionMap); + fb.setRetryableExceptionClasses(exceptionMap); Object step = fb.getObject(); assertTrue(step instanceof TaskletStep); @@ -297,4 +300,12 @@ public class StepParserStepFactoryBeanTests { assertTrue(handler instanceof SimpleFlow); } + private Map, Boolean> getExceptionMap(Class... args) { + Map, Boolean> map = new HashMap, Boolean>(); + for (Class arg : args) { + map.put(arg, true); + } + return map; + } + } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/FaultTolerantStepFactoryBeanRollbackTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/FaultTolerantStepFactoryBeanRollbackTests.java index dadf353ee..5ef7a10ee 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/FaultTolerantStepFactoryBeanRollbackTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/FaultTolerantStepFactoryBeanRollbackTests.java @@ -149,7 +149,7 @@ public class FaultTolerantStepFactoryBeanRollbackTests { reader.setExceptionType(SkippableException.class); // No skips by default - factory.setSkippableExceptionClasses(getExceptionMap()); + factory.setSkippableExceptionClasses(getExceptionMap(RuntimeException.class)); // But this one is explicit in the tx-attrs so it should be skipped factory.setNoRollbackExceptionClasses(getExceptionList(SkippableException.class)); diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/FaultTolerantStepFactoryBeanTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/FaultTolerantStepFactoryBeanTests.java index 84958f091..8016b05f0 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/FaultTolerantStepFactoryBeanTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/FaultTolerantStepFactoryBeanTests.java @@ -127,7 +127,7 @@ public class FaultTolerantStepFactoryBeanTests { reader.setFailures("2"); // nothing is skippable - factory.setSkippableExceptionClasses(getExceptionMap()); + factory.setSkippableExceptionClasses(getExceptionMap(NonExistentException.class)); Step step = (Step) factory.getObject(); @@ -144,7 +144,7 @@ public class FaultTolerantStepFactoryBeanTests { @Test public void testNonSkippableException() throws Exception { // nothing is skippable - factory.setSkippableExceptionClasses(getExceptionMap()); + factory.setSkippableExceptionClasses(getExceptionMap(NonExistentException.class)); factory.setCommitInterval(1); // no failures on read @@ -1066,5 +1066,9 @@ public class FaultTolerantStepFactoryBeanTests { } return map; } + + public static class NonExistentException extends Exception { + + } } diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/ParentRetryableLateBindingStepFactoryBeanParserTests-context.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/ParentRetryableLateBindingStepFactoryBeanParserTests-context.xml new file mode 100644 index 000000000..dedc7f117 --- /dev/null +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/ParentRetryableLateBindingStepFactoryBeanParserTests-context.xml @@ -0,0 +1,28 @@ + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/ParentRetryableStepFactoryBeanParserTests-context.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/ParentRetryableStepFactoryBeanParserTests-context.xml new file mode 100644 index 000000000..1da9d0f07 --- /dev/null +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/ParentRetryableStepFactoryBeanParserTests-context.xml @@ -0,0 +1,26 @@ + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/ParentSkippableLateBindingStepFactoryBeanParserTests-context.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/ParentSkippableLateBindingStepFactoryBeanParserTests-context.xml new file mode 100644 index 000000000..90e264ff1 --- /dev/null +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/ParentSkippableLateBindingStepFactoryBeanParserTests-context.xml @@ -0,0 +1,28 @@ + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/ParentSkippableStepFactoryBeanParserTests-context.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/ParentSkippableStepFactoryBeanParserTests-context.xml index 409433076..6e13336fa 100644 --- a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/ParentSkippableStepFactoryBeanParserTests-context.xml +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/ParentSkippableStepFactoryBeanParserTests-context.xml @@ -5,20 +5,20 @@ - - - - - - - + + + + + + + + + - - - +