diff --git a/spring-batch-core/.springBeans b/spring-batch-core/.springBeans index ae68998df..0729082d2 100644 --- a/spring-batch-core/.springBeans +++ b/spring-batch-core/.springBeans @@ -1,7 +1,7 @@ 1 - + @@ -101,6 +101,8 @@ src/test/resources/org/springframework/batch/core/configuration/xml/FlowStepParserTests-context.xml src/test/resources/org/springframework/batch/core/configuration/xml/JobParserWrongSchemaInRootTests-context.xml src/test/resources/org/springframework/batch/core/configuration/xml/PartitionStepParserTests-context.xml + src/test/resources/org/springframework/batch/core/configuration/xml/ChunkElementSkipPolicyParserTests-context.xml + src/test/resources/org/springframework/batch/core/configuration/xml/JobStepParserTests-context.xml 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 8e9c41aa6..c50016bb1 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 @@ -103,6 +103,9 @@ public class ChunkElementParser { propertyValues.addPropertyValue("skipLimit", skipLimit); } + handleItemHandler("skip-policy", "skipPolicy", null, false, element, parserContext, + propertyValues, underspecified); + String retryLimit = element.getAttribute("retry-limit"); if (StringUtils.hasText(retryLimit)) { propertyValues.addPropertyValue("retryLimit", retryLimit); diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/StepParserStepFactoryBean.java b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/StepParserStepFactoryBean.java index 8329ec52c..46d5158b2 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/StepParserStepFactoryBean.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/StepParserStepFactoryBean.java @@ -40,6 +40,7 @@ import org.springframework.batch.core.step.item.FaultTolerantStepFactoryBean; import org.springframework.batch.core.step.item.SimpleStepFactoryBean; import org.springframework.batch.core.step.job.JobParametersExtractor; import org.springframework.batch.core.step.job.JobStep; +import org.springframework.batch.core.step.skip.SkipPolicy; import org.springframework.batch.core.step.tasklet.Tasklet; import org.springframework.batch.core.step.tasklet.TaskletStep; import org.springframework.batch.item.ItemProcessor; @@ -151,6 +152,8 @@ class StepParserStepFactoryBean implements FactoryBean, BeanNameAware { private Integer skipLimit; + private SkipPolicy skipPolicy; + private TaskExecutor taskExecutor; private Integer throttleLimit; @@ -342,6 +345,9 @@ class StepParserStepFactoryBean implements FactoryBean, BeanNameAware { if (skipLimit != null) { fb.setSkipLimit(skipLimit); } + if (skipPolicy != null) { + fb.setSkipPolicy(skipPolicy); + } if (retryListeners != null) { fb.setRetryListeners(retryListeners); @@ -429,7 +435,17 @@ class StepParserStepFactoryBean implements FactoryBean, BeanNameAware { } } - private void validateAtLeastOneDependency(String dependantName, Boolean dependantValue, String name, + /** + * Check if a field is present then a second (at least one taken from a + * list) is also. + * + * @param dependantName the name of the first field + * @param dependantValue the value of the first field + * @param names the names of the other fields (used to construct an exception message) + * @param values the other field values (one of which must be set if the + * first field is) + */ + private void validateAtLeastOneDependency(String dependantName, Boolean dependantValue, String names, Object... values) { boolean oneIsPresent = false; for (Object value : values) { @@ -440,22 +456,42 @@ class StepParserStepFactoryBean implements FactoryBean, BeanNameAware { } if (isPresent(dependantValue) && !oneIsPresent) { throw new IllegalArgumentException("The field '" + dependantName + "' is not permitted on the step [" - + this.name + "] because " + name + " is not present."); + + this.name + "] because " + names + " is not present."); } } - private void validateDependency(String dependantName, Object dependantValue, String name, Object value, + /** + * Check if a field is present then a second is also. If the + * twoWayDependency flag is set then the opposite must also be true: if the + * second value is present, the first must also be. + * + * @param dependentName the name of the first field + * @param dependentValue the value of the first field + * @param name the name of the other field (which should be absent if the + * first is present) + * @param value the value of the other field + * @param twoWayDependency true if both depend on each other + * + * @throws IllegalArgumentException if eiether condition is violated + */ + private void validateDependency(String dependentName, Object dependentValue, String name, Object value, boolean twoWayDependency) { - if (isPresent(dependantValue) && !isPresent(value)) { - throw new IllegalArgumentException("The field '" + dependantName + "' is not permitted on the step [" + if (isPresent(dependentValue) && !isPresent(value)) { + throw new IllegalArgumentException("The field '" + dependentName + "' is not permitted on the step [" + this.name + "] because there is no '" + name + "'."); } - if (twoWayDependency && isPresent(value) && !isPresent(dependantValue)) { + if (twoWayDependency && isPresent(value) && !isPresent(dependentValue)) { throw new IllegalArgumentException("The field '" + name + "' is not permitted on the step [" + this.name - + "] because there is no '" + dependantName + "'."); + + "] because there is no '" + dependentName + "'."); } } + /** + * Is the object non-null (or if an Integer, non-zero)? + * + * @param o an object + * @return true if the object has a value + */ private boolean isPresent(Object o) { if (o instanceof Integer) { return isPositive((Integer) o); @@ -464,7 +500,7 @@ class StepParserStepFactoryBean implements FactoryBean, BeanNameAware { } private boolean isFaultTolerant() { - return isPositive(skipLimit) || isPositive(retryLimit) || isPositive(cacheCapacity) + return skipPolicy != null || isPositive(skipLimit) || isPositive(retryLimit) || isPositive(cacheCapacity) || isTrue(readerTransactionalQueue); } @@ -521,11 +557,11 @@ class StepParserStepFactoryBean implements FactoryBean, BeanNameAware { public void setJob(Job job) { this.job = job; } - + public void setJobParametersExtractor(JobParametersExtractor jobParametersExtractor) { this.jobParametersExtractor = jobParametersExtractor; } - + public void setJobLauncher(JobLauncher jobLauncher) { this.jobLauncher = jobLauncher; } @@ -762,6 +798,16 @@ class StepParserStepFactoryBean implements FactoryBean, BeanNameAware { this.skipLimit = skipLimit; } + /** + * Public setter for a skip policy. If this value is set then the skip limit + * and skippable exceptions are ignored. + * + * @param skipPolicy the {@link SkipPolicy} to set + */ + public void setSkipPolicy(SkipPolicy skipPolicy) { + this.skipPolicy = skipPolicy; + } + /** * Public setter for the {@link TaskExecutor}. If this is set, then it will * be used to execute the chunk processing inside the {@link Step}. 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 93501562d..50e8b725b 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 @@ -367,6 +367,15 @@ public class FaultTolerantStepFactoryBean extends SimpleStepFactoryBean configureChunkProvider() { + if (skipPolicy != null) { + if (!skippableExceptionClasses.isEmpty()) { + logger.info("Skippable exceptions will be ignored because a SkipPolicy was specified explicitly"); + } + if (skipLimit>0) { + logger.info("Skip limit will be ignored because a SkipPolicy was specified explicitly"); + } + } + SkipPolicy readSkipPolicy = skipPolicy != null ? skipPolicy : new LimitCheckingItemSkipPolicy(skipLimit, getSkippableExceptionClasses()); readSkipPolicy = getFatalExceptionAwareProxy(readSkipPolicy); diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/skip/ExceptionClassifierSkipPolicy.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/skip/ExceptionClassifierSkipPolicy.java index 6108927eb..3f759138c 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/skip/ExceptionClassifierSkipPolicy.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/step/skip/ExceptionClassifierSkipPolicy.java @@ -15,6 +15,9 @@ */ package org.springframework.batch.core.step.skip; +import java.util.Map; + +import org.springframework.batch.classify.Classifier; import org.springframework.batch.classify.SubclassClassifier; /** @@ -38,6 +41,21 @@ public class ExceptionClassifierSkipPolicy implements SkipPolicy { this.classifier = classifier; } + /** + * Setter for policy map. This property should not be changed dynamically - + * set it once, e.g. in configuration, and then don't change it during a + * running application. Either this property or the exception classifier + * directly should be set, but not both. + * + * @param policyMap a map of String to {@link SkipPolicy} that will be used + * to create a {@link Classifier} to locate a policy. + */ + public void setPolicyMap(Map, SkipPolicy> policyMap) { + SubclassClassifier subclassClassifier = new SubclassClassifier( + policyMap, new NeverSkipItemSkipPolicy()); + this.classifier = subclassClassifier; + } + /** * Consult the classifier and find a delegate policy, and then use that to * determine the outcome. diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/skip/LimitCheckingItemSkipPolicy.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/skip/LimitCheckingItemSkipPolicy.java index 5a81f4785..dc545976b 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/skip/LimitCheckingItemSkipPolicy.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/step/skip/LimitCheckingItemSkipPolicy.java @@ -52,9 +52,9 @@ import org.springframework.batch.item.file.FlatFileParseException; */ public class LimitCheckingItemSkipPolicy implements SkipPolicy { - private final int skipLimit; + private int skipLimit; - private final Classifier skippableExceptionClassifier; + private Classifier skippableExceptionClassifier; /** * Convenience constructor that assumes all exception types are fatal. @@ -84,6 +84,37 @@ public class LimitCheckingItemSkipPolicy implements SkipPolicy { this.skippableExceptionClassifier = skippableExceptionClassifier; } + /** + * The absolute number of skips (of skippable exceptions) that can be + * tolerated before a failure. + * + * @param skipLimit the skip limit to set + */ + public void setSkipLimit(int skipLimit) { + this.skipLimit = skipLimit; + } + + /** + * The classifier that will be used to decide on skippability. If an + * exception classifies as "true" then it is skippable, and otherwise not. + * + * @param skippableExceptionClassifier the skippableExceptionClassifier to + * set + */ + public void setSkippableExceptionClassifier(Classifier skippableExceptionClassifier) { + this.skippableExceptionClassifier = skippableExceptionClassifier; + } + + /** + * Set up the classifier through a convenient map from throwable class to + * boolean (true if skippable). + * + * @param skippableExceptions the skippable exceptions to set + */ + public void setSkippableExceptionMap(Map, Boolean> skippableExceptions) { + this.skippableExceptionClassifier = new BinaryExceptionClassifier(skippableExceptions); + } + /** * Given the provided exception and skip count, determine whether or not * processing should continue for the given exception. If the exception is diff --git a/spring-batch-core/src/main/resources/org/springframework/batch/core/configuration/xml/spring-batch-2.1.xsd b/spring-batch-core/src/main/resources/org/springframework/batch/core/configuration/xml/spring-batch-2.1.xsd index a359eb856..da48e2dcb 100644 --- a/spring-batch-core/src/main/resources/org/springframework/batch/core/configuration/xml/spring-batch-2.1.xsd +++ b/spring-batch-core/src/main/resources/org/springframework/batch/core/configuration/xml/spring-batch-2.1.xsd @@ -738,6 +738,19 @@ + + + + + + + + + + @@ -864,6 +877,17 @@ ]]> + + + + + + + + + , Boolean> skippable = getExceptionClasses("s1", context); + assertEquals(2, skippable.size()); + containsClassified(skippable, NullPointerException.class, true); + containsClassified(skippable, ArithmeticException.class, true); + } + + @Test + public void testSkipPolicyElement() throws Exception { + ConfigurableApplicationContext context = new ClassPathXmlApplicationContext( + "org/springframework/batch/core/configuration/xml/ChunkElementSkipPolicyParserTests-context.xml"); + Map, Boolean> skippable = getExceptionClasses("s2", context); + assertEquals(1, skippable.size()); + containsClassified(skippable, ArithmeticException.class, true); + } + @Test public void testProcessorTransactionalAttributes() throws Exception { - ConfigurableApplicationContext chunkElementAttributeParserTestsContext = new ClassPathXmlApplicationContext( + ConfigurableApplicationContext context = new ClassPathXmlApplicationContext( "org/springframework/batch/core/configuration/xml/ChunkElementTransactionalAttributeParserTests-context.xml"); - Object step = chunkElementAttributeParserTestsContext.getBean("s1", Step.class); + Object step = context.getBean("s1", Step.class); assertNotNull("Step not parsed", step); Object tasklet = ReflectionTestUtils.getField(step, "tasklet"); Object chunkProcessor = ReflectionTestUtils.getField(tasklet, "chunkProcessor"); @@ -179,12 +198,6 @@ public class ChunkElementParserTests { assertTrue(h); } - private void containsClassified(Map, Boolean> classified, - Class cls, boolean include) { - assertTrue(classified.containsKey(cls)); - assertEquals(include, classified.get(cls)); - } - @SuppressWarnings("unchecked") private Map, Boolean> getExceptionClasses(String stepName, ApplicationContext ctx) throws Exception { @@ -203,6 +216,12 @@ public class ChunkElementParserTests { return (Map, Boolean>) ReflectionTestUtils.getField(limitClassifier, "classified"); } + private void containsClassified(Map, Boolean> classified, + Class cls, boolean include) { + assertTrue(classified.containsKey(cls)); + assertEquals(include, classified.get(cls)); + } + @SuppressWarnings("unchecked") private Collection getStreams(String stepName, ApplicationContext ctx) throws Exception { Map beans = ctx.getBeansOfType(Step.class); diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/ChunkElementSkipPolicyParserTests-context.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/ChunkElementSkipPolicyParserTests-context.xml new file mode 100644 index 000000000..e4a474303 --- /dev/null +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/ChunkElementSkipPolicyParserTests-context.xml @@ -0,0 +1,42 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file