diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/domain/ItemSkipPolicy.java b/spring-batch-core/src/main/java/org/springframework/batch/core/domain/ItemSkipPolicy.java index 6a47a4764..9f8bb8dd4 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/domain/ItemSkipPolicy.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/domain/ItemSkipPolicy.java @@ -27,10 +27,10 @@ public interface ItemSkipPolicy { * Returns true or false, indicating whether or not reading should * continue for the current step execution with the given throwable. * - * @param ex throwable encountered while reading - * @param stepContribution currently running execution + * @param exception throwable encountered while reading + * @param skipCount currently running count of skips * @return true if reading should continue, false otherwise. - * @throws IllegalArgumentException if t or stepExecution is null + * @throws IllegalArgumentException if the exception is null */ - boolean shouldSkip(Exception ex, StepContribution stepContribution); + boolean shouldSkip(Exception exception, int skipCount); } diff --git a/spring-batch-execution/src/main/java/org/springframework/batch/execution/step/ItemOrientedStep.java b/spring-batch-execution/src/main/java/org/springframework/batch/execution/step/ItemOrientedStep.java index 2fae057b6..ea2bf7112 100644 --- a/spring-batch-execution/src/main/java/org/springframework/batch/execution/step/ItemOrientedStep.java +++ b/spring-batch-execution/src/main/java/org/springframework/batch/execution/step/ItemOrientedStep.java @@ -444,7 +444,8 @@ public class ItemOrientedStep extends AbstractStep implements InitializingBean { } catch (Exception e) { - if (getItemSkipPolicy().shouldSkip(e, contribution)) { + if (getItemSkipPolicy().shouldSkip(e, contribution.getSkipCount())) { + contribution.incrementSkipCount(); skip(); } else { diff --git a/spring-batch-execution/src/main/java/org/springframework/batch/execution/step/support/AlwaysSkipItemSkipPolicy.java b/spring-batch-execution/src/main/java/org/springframework/batch/execution/step/support/AlwaysSkipItemSkipPolicy.java index 6d8cb3379..cea764c8b 100644 --- a/spring-batch-execution/src/main/java/org/springframework/batch/execution/step/support/AlwaysSkipItemSkipPolicy.java +++ b/spring-batch-execution/src/main/java/org/springframework/batch/execution/step/support/AlwaysSkipItemSkipPolicy.java @@ -16,7 +16,6 @@ package org.springframework.batch.execution.step.support; import org.springframework.batch.core.domain.ItemSkipPolicy; -import org.springframework.batch.core.domain.StepContribution; /** * Implementation of the {@link ItemSkipPolicy} interface that @@ -27,7 +26,7 @@ import org.springframework.batch.core.domain.StepContribution; */ public class AlwaysSkipItemSkipPolicy implements ItemSkipPolicy { - public boolean shouldSkip(Exception ex, StepContribution stepContribution) { + public boolean shouldSkip(Exception ex, int skipCount) { return true; } } diff --git a/spring-batch-execution/src/main/java/org/springframework/batch/execution/step/support/LimitCheckingItemSkipPolicy.java b/spring-batch-execution/src/main/java/org/springframework/batch/execution/step/support/LimitCheckingItemSkipPolicy.java index a214da05e..9d8f2a2bb 100644 --- a/spring-batch-execution/src/main/java/org/springframework/batch/execution/step/support/LimitCheckingItemSkipPolicy.java +++ b/spring-batch-execution/src/main/java/org/springframework/batch/execution/step/support/LimitCheckingItemSkipPolicy.java @@ -26,7 +26,6 @@ import org.springframework.batch.common.ExceptionClassifier; import org.springframework.batch.common.SubclassExceptionClassifier; import org.springframework.batch.core.domain.ItemSkipPolicy; import org.springframework.batch.core.domain.Step; -import org.springframework.batch.core.domain.StepContribution; import org.springframework.batch.core.domain.StepExecution; import org.springframework.batch.io.exception.FlatFileParsingException; @@ -76,17 +75,16 @@ public class LimitCheckingItemSkipPolicy implements ItemSkipPolicy { } /** - * Given the provided exception and StepExecution, determine whether or not + * Given the provided exception and skip count, determine whether or not * processing should continue for the given exception. If the exception * is not within the list of 'skippable exceptions', false will be returned. * If the exception is within the list, and {@link StepExecution} skipCount * is greater than the skipLimit, then a {@link SkipLimitExceededException} * will be thrown. */ - public boolean shouldSkip(Exception ex, StepContribution stepContribution){ + public boolean shouldSkip(Exception ex, int skipCount){ if(exceptionClassifier.classify(ex).equals(SKIP)){ - if(stepContribution.getSkipCount() < skipLimit){ - stepContribution.incrementSkipCount(); + if(skipCount < skipLimit){ return true; } else{ diff --git a/spring-batch-execution/src/main/java/org/springframework/batch/execution/step/support/NeverSkipItemSkipPolicy.java b/spring-batch-execution/src/main/java/org/springframework/batch/execution/step/support/NeverSkipItemSkipPolicy.java index 695515145..fb0fd31ee 100644 --- a/spring-batch-execution/src/main/java/org/springframework/batch/execution/step/support/NeverSkipItemSkipPolicy.java +++ b/spring-batch-execution/src/main/java/org/springframework/batch/execution/step/support/NeverSkipItemSkipPolicy.java @@ -16,7 +16,6 @@ package org.springframework.batch.execution.step.support; import org.springframework.batch.core.domain.ItemSkipPolicy; -import org.springframework.batch.core.domain.StepContribution; /** * {@link ItemSkipPolicy} implementation that always returns false, @@ -26,7 +25,7 @@ import org.springframework.batch.core.domain.StepContribution; */ public class NeverSkipItemSkipPolicy implements ItemSkipPolicy{ - public boolean shouldSkip(Exception ex, StepContribution stepContribution) { + public boolean shouldSkip(Exception ex, int skipCount) { return false; } diff --git a/spring-batch-execution/src/test/java/org/springframework/batch/execution/step/ItemOrientedStepTests.java b/spring-batch-execution/src/test/java/org/springframework/batch/execution/step/ItemOrientedStepTests.java index 10722d7bc..75d63aa7f 100644 --- a/spring-batch-execution/src/test/java/org/springframework/batch/execution/step/ItemOrientedStepTests.java +++ b/spring-batch-execution/src/test/java/org/springframework/batch/execution/step/ItemOrientedStepTests.java @@ -18,7 +18,6 @@ package org.springframework.batch.execution.step; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.List; import junit.framework.TestCase; @@ -39,8 +38,6 @@ import org.springframework.batch.execution.repository.dao.MapStepDao; import org.springframework.batch.execution.scope.StepScope; import org.springframework.batch.execution.scope.StepSynchronizationManager; import org.springframework.batch.execution.step.support.JobRepositorySupport; -import org.springframework.batch.execution.step.support.LimitCheckingItemSkipPolicy; -import org.springframework.batch.execution.step.support.SkipLimitExceededException; import org.springframework.batch.execution.step.support.StepInterruptionPolicy; import org.springframework.batch.item.ExecutionContext; import org.springframework.batch.item.ItemReader; @@ -368,29 +365,6 @@ public class ItemOrientedStepTests extends TestCase { assertEquals(1, list.size()); } - public void testApplyConfigurationWithZeroSkipLimit() throws Exception { - itemOrientedStep.setItemSkipPolicy(new LimitCheckingItemSkipPolicy(0, Collections.singletonList(Exception.class))); - itemOrientedStep.applyConfiguration(); - JobExecution jobExecution = new JobExecution(jobInstance); - StepExecution stepExecution = new StepExecution(itemOrientedStep, jobExecution); - try { - assertEquals(false, itemOrientedStep.getItemSkipPolicy().shouldSkip(new RuntimeException(), - stepExecution.createStepContribution())); - fail("Expected SkipLimitExceededException"); - } catch (SkipLimitExceededException e) { - // expected - } - } - - public void testApplyConfigurationWithNonZeroSkipLimit() throws Exception { - itemOrientedStep.setItemSkipPolicy(new LimitCheckingItemSkipPolicy(1, Collections.singletonList(Exception.class))); - itemOrientedStep.applyConfiguration(); - JobExecution jobExecution = new JobExecution(jobInstance); - StepExecution stepExecution = new StepExecution(itemOrientedStep, jobExecution); - assertEquals(true, itemOrientedStep.getItemSkipPolicy().shouldSkip(new RuntimeException(), - stepExecution.createStepContribution())); - } - public void testStreamManager() throws Exception { Step step = new StepSupport("stepName"); itemOrientedStep.setItemReader(new AbstractItemReader() { diff --git a/spring-batch-execution/src/test/java/org/springframework/batch/execution/step/support/SkipLimitReadFailurePolicyTests.java b/spring-batch-execution/src/test/java/org/springframework/batch/execution/step/support/SkipLimitReadFailurePolicyTests.java index f0116e29d..574876575 100644 --- a/spring-batch-execution/src/test/java/org/springframework/batch/execution/step/support/SkipLimitReadFailurePolicyTests.java +++ b/spring-batch-execution/src/test/java/org/springframework/batch/execution/step/support/SkipLimitReadFailurePolicyTests.java @@ -19,24 +19,17 @@ import java.io.FileNotFoundException; import java.util.ArrayList; import java.util.List; -import org.springframework.batch.core.domain.StepContribution; -import org.springframework.batch.core.domain.StepExecution; -import org.springframework.batch.core.domain.StepSupport; -import org.springframework.batch.execution.step.support.LimitCheckingItemSkipPolicy; -import org.springframework.batch.execution.step.support.SkipLimitExceededException; -import org.springframework.batch.io.exception.FlatFileParsingException; - import junit.framework.TestCase; +import org.springframework.batch.io.exception.FlatFileParsingException; + /** * @author Lucas Ward * */ public class SkipLimitReadFailurePolicyTests extends TestCase { - LimitCheckingItemSkipPolicy failurePolicy; - StepExecution stepExecution; - StepContribution stepContribution; + private LimitCheckingItemSkipPolicy failurePolicy; protected void setUp() throws Exception { super.setUp(); @@ -45,14 +38,11 @@ public class SkipLimitReadFailurePolicyTests extends TestCase { skippableExceptions.add(FlatFileParsingException.class); failurePolicy = new LimitCheckingItemSkipPolicy(1, skippableExceptions); - stepExecution = new StepExecution(new StepSupport("stepName"), null); - stepExecution.setSkipCount(2); - stepContribution = stepExecution.createStepContribution(); } public void testLimitExceed(){ try{ - failurePolicy.shouldSkip(new FlatFileParsingException("", ""), stepContribution); + failurePolicy.shouldSkip(new FlatFileParsingException("", ""), 2); fail(); } catch(SkipLimitExceededException ex){ @@ -61,12 +51,11 @@ public class SkipLimitReadFailurePolicyTests extends TestCase { } public void testNonSkippableException(){ - assertFalse(failurePolicy.shouldSkip(new FileNotFoundException(), stepContribution)); + assertFalse(failurePolicy.shouldSkip(new FileNotFoundException(), 2)); } public void testSkip(){ - stepExecution.setSkipCount(0); - assertTrue(failurePolicy.shouldSkip(new FlatFileParsingException("",""), stepContribution)); + assertTrue(failurePolicy.shouldSkip(new FlatFileParsingException("",""), 0)); } }