RESOLVED - issue BATCH-511: read failures cause rollback

Renamed "commit*" in StepContribution to "combine*".  Fixed broken test (forgot to combine skip counts).

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;
 	}

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();
 	}
 

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);

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;
 				}
 			}

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 {