diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/StepContribution.java b/spring-batch-core/src/main/java/org/springframework/batch/core/StepContribution.java index f589a18c6..4bbd2af77 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/StepContribution.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/StepContribution.java @@ -101,13 +101,20 @@ public class StepContribution { * Increment the counter for skipped reads */ public void incrementReadSkipCount() { - this.readSkipCount++; + readSkipCount++; } /** - * Clear the read skips and transfer them to the total skip count + * @return the read skip count */ - public void commitReadSkipCount() { + public int getReadSkipCount() { + return readSkipCount; + } + + /** + * Combine the skip counts and reset read skips to zero. + */ + public void combineSkipCounts() { skipCount += readSkipCount; readSkipCount = 0; } diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/StepExecution.java b/spring-batch-core/src/main/java/org/springframework/batch/core/StepExecution.java index e40c6eb30..38b10783f 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/StepExecution.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/StepExecution.java @@ -304,9 +304,8 @@ public class StepExecution extends Entity { */ public synchronized void apply(StepContribution contribution) { itemCount += contribution.getItemCount(); - // TODO: this should not be necessary - the step decides - // executionContext = contribution.getExecutionContext(); commitCount += contribution.getCommitCount(); + contribution.combineSkipCounts(); skipCount += contribution.getSkipCount(); } diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/ItemOrientedStep.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/ItemOrientedStep.java index acfce1a38..4cc28b2e2 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/ItemOrientedStep.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/ItemOrientedStep.java @@ -299,7 +299,6 @@ public class ItemOrientedStep extends AbstractStep { Thread.currentThread().interrupt(); } - contribution.commitReadSkipCount(); // Apply the contribution to the step // only if chunk was successful stepExecution.apply(contribution); diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/ItemSkipPolicyItemHandler.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/ItemSkipPolicyItemHandler.java index d4b0cf1db..da35a6561 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/ItemSkipPolicyItemHandler.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/ItemSkipPolicyItemHandler.java @@ -173,7 +173,7 @@ public class ItemSkipPolicyItemHandler extends SimpleItemHandler { catch (SkipLimitExceededException ex) { // we are headed for a abnormal ending so bake in the skip // count - contribution.commitReadSkipCount(); + contribution.combineSkipCounts(); throw ex; } } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/ItemSkipPolicyItemHandlerTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/ItemSkipPolicyItemHandlerTests.java index dfc2836fe..babace9df 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/ItemSkipPolicyItemHandlerTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/ItemSkipPolicyItemHandlerTests.java @@ -76,7 +76,7 @@ public class ItemSkipPolicyItemHandlerTests extends TestCase { handler.setItemSkipPolicy(new AlwaysSkipItemSkipPolicy()); assertEquals(new Holder("1"), handler.read(contribution)); assertEquals(new Holder("3"), handler.read(contribution)); - assertEquals(1, contribution.getSkipCount()); + assertEquals(1, contribution.getReadSkipCount()); assertEquals(new Holder("4"), handler.read(contribution)); } @@ -96,6 +96,7 @@ public class ItemSkipPolicyItemHandlerTests extends TestCase { handler.setItemSkipPolicy(new AlwaysSkipItemSkipPolicy()); handler.handle(contribution); handler.handle(contribution); + contribution.combineSkipCounts(); assertEquals(1, contribution.getSkipCount()); // 2 is skipped so 3 was last one processed and now we are at 4 try { @@ -121,6 +122,7 @@ public class ItemSkipPolicyItemHandlerTests extends TestCase { } handler.handle(contribution); handler.handle(contribution); + contribution.combineSkipCounts(); assertEquals(2, contribution.getSkipCount()); // 2 is skipped so 3 was last one processed and now we are at 4, which was previously skipped handler.handle(contribution); @@ -154,6 +156,7 @@ public class ItemSkipPolicyItemHandlerTests extends TestCase { assertEquals(1, contribution.getSkipCount()); assertEquals(new Holder("1"), handler.read(contribution)); assertEquals(new Holder("3"), handler.read(contribution)); + contribution.combineSkipCounts(); assertEquals(2, contribution.getSkipCount()); // No "4" because it was skipped on write assertEquals(new Holder("5"), handler.read(contribution)); @@ -165,6 +168,7 @@ public class ItemSkipPolicyItemHandlerTests extends TestCase { handler.setItemSkipPolicy(new AlwaysSkipItemSkipPolicy()); handler.handle(contribution); handler.handle(contribution); + contribution.combineSkipCounts(); assertEquals(1, contribution.getSkipCount()); // 2 is skipped so 3 was last one processed and now we are at 4 try {