RESOLVED - issue BATCH-461: skip counting

http://jira.springframework.org/browse/BATCH-461

ItemOrientedStep now increments StepExecution's skipCount from StepContribution on rollback.
Simple assert checking the correct value of StepExecution#skipCount added to SkipLimitStepFactoryBeanTests.

Btw. renamed StepContribution#taskCount to itemCount

spring-batch-core/src/main/java/org/springframework/batch/core/StepContribution.java

@@ -25,7 +25,7 @@ package org.springframework.batch.core;
  */
 public class StepContribution {
 
-	private int taskCount = 0;
+	private int itemCount = 0;
 
 	private int parentSkipCount;
 
@@ -43,8 +43,8 @@ public class StepContribution {
 	/**
 	 * Increment the counter for the number of tasks executed.
 	 */
-	public void incrementTaskCount() {
-		taskCount++;
+	public void incrementItemCount() {
+		itemCount++;
 	}
 
 	/**
@@ -52,8 +52,8 @@ public class StepContribution {
 	 * 
 	 * @return the task execution counter.
 	 */
-	public int getTaskCount() {
-		return taskCount;
+	public int getItemCount() {
+		return itemCount;
 	}
 
 	/**

spring-batch-core/src/main/java/org/springframework/batch/core/StepExecution.java

@@ -303,7 +303,7 @@ public class StepExecution extends Entity {
 	 * @param contribution
 	 */
 	public synchronized void apply(StepContribution contribution) {
-		itemCount += contribution.getTaskCount();
+		itemCount += contribution.getItemCount();
 		// TODO: this should not be necessary - the step decides
 		// executionContext = contribution.getExecutionContext();
 		commitCount += contribution.getCommitCount();
@@ -336,6 +336,10 @@ public class StepExecution extends Entity {
 	public int getSkipCount() {
 		return skipCount;
 	}
+	
+	public void incrementSkipCountBy(int count) {
+		skipCount += count;
+	}
 
 	/**
 	 * Convenience method to get the current job parameters.

spring-batch-core/src/main/java/org/springframework/batch/core/step/item/ItemOrientedStep.java

@@ -333,10 +333,12 @@ public class ItemOrientedStep extends AbstractStep {
 
 					}
 					catch (Error e) {
+						stepExecution.incrementSkipCountBy(contribution.getContributionSkipCount());
 						processRollback(stepExecution, fatalException, transaction);
 						throw e;
 					}
 					catch (Exception e) {
+						stepExecution.incrementSkipCountBy(contribution.getContributionSkipCount());
 						processRollback(stepExecution, fatalException, transaction);
 						throw e;
 					}
@@ -467,7 +469,7 @@ public class ItemOrientedStep extends AbstractStep {
 				// check for interruption before each item as well
 				interruptionPolicy.checkInterrupted(execution);
 				ExitStatus exitStatus = itemHandler.handle(contribution);
-				contribution.incrementTaskCount();
+				contribution.incrementItemCount();
 				// check for interruption after each item as well
 				interruptionPolicy.checkInterrupted(execution);
 				return exitStatus;

spring-batch-core/src/test/java/org/springframework/batch/core/StepContributionTests.java

@@ -29,12 +29,12 @@ public class StepContributionTests extends TestCase {
 
 	/**
 	 * Test method for
-	 * {@link org.springframework.batch.core.StepContribution#incrementTaskCount()}.
+	 * {@link org.springframework.batch.core.StepContribution#incrementItemCount()}.
 	 */
 	public void testIncrementTaskCount() {
-		assertEquals(0, contribution.getTaskCount());
-		contribution.incrementTaskCount();
-		assertEquals(1, contribution.getTaskCount());
+		assertEquals(0, contribution.getItemCount());
+		contribution.incrementItemCount();
+		assertEquals(1, contribution.getItemCount());
 	}
 
 	/**

spring-batch-core/src/test/java/org/springframework/batch/core/step/item/ItemOrientedStepTests.java

@@ -140,7 +140,7 @@ public class ItemOrientedStepTests extends TestCase {
 		itemOrientedStep.processChunk(stepExecution, contribution);
 		assertEquals(1, processed.size());
 		assertEquals(0, stepExecution.getItemCount().intValue());
-		assertEquals(1, contribution.getTaskCount());
+		assertEquals(1, contribution.getItemCount());
 
 	}
 

spring-batch-core/src/test/java/org/springframework/batch/core/step/item/SkipLimitStepFactoryBeanTests.java

@@ -68,12 +68,13 @@ public class SkipLimitStepFactoryBeanTests extends TestCase {
 		StepExecution stepExecution = new StepExecution(step, jobExecution);
 		step.execute(stepExecution);
 
+		assertEquals(2, stepExecution.getSkipCount());
+		
 		assertTrue(reader.skipped.contains("2"));
 		assertTrue(reader.skipped.contains("4"));
 		// writer did not skip "2" as it never made it to writer, only "4" did
 		assertTrue(writer.skipped.contains("4"));
 
-//		TODO when reader throws exception on "2", it results in writer skipping "1"
 		String[] expectedOutput = { "1", "3", "5" };
 
 		for (int i = 0; i < expectedOutput.length; i++) {
@@ -81,6 +82,7 @@ public class SkipLimitStepFactoryBeanTests extends TestCase {
 					.contains(expectedOutput[i]));
 		}
 		assertTrue(writer.written.size() == expectedOutput.length);
+		
 	}
 
 	/**