RESOLVED - issue BATCH-465: The term "exit code" is overloaded

RESOLVED - issue BATCH-468: ExitStatusExceptionClassifier, ExitCodeMapper and simple versions thereof (and others?) incorrectly moved into step.item package

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

@@ -15,7 +15,6 @@
  */
 package org.springframework.batch.core;
 
-import org.springframework.batch.item.ExecutionContext;
 
 /**
  * Represents a contribution to a {@link StepExecution}, buffering changes
@@ -28,9 +27,7 @@ public class StepContribution {
 
 	private int taskCount = 0;
 
-	private StepExecution execution;
-
-	private ExecutionContext executionContext;
+	private int parentSkipCount;
 
 	private int commitCount;
 
@@ -40,7 +37,7 @@ public class StepContribution {
 	 * @param execution
 	 */
 	public StepContribution(StepExecution execution) {
-		this.execution = execution;
+		this.parentSkipCount = execution.getSkipCount();
 	}
 
 	/**
@@ -66,23 +63,6 @@ public class StepContribution {
 		commitCount++;
 	}
 
-	/**
-	 * Set the statistics properties.
-	 * 
-	 * @param executionContext
-	 */
-	public void setExecutionContext(ExecutionContext executionContext) {
-		this.executionContext = executionContext;
-	}
-
-	/**
-	 * Public getter for the {@link ExecutionContext}.
-	 * @return the stream context
-	 */
-	public ExecutionContext getExecutionContext() {
-		return executionContext;
-	}
-
 	/**
 	 * Public getter for the commit counter.
 	 * @return the commitCount
@@ -91,20 +71,12 @@ public class StepContribution {
 		return commitCount;
 	}
 
-	/**
-	 * Delegate call to the {@link StepExecution}.
-	 * @return the flag from the underlying execution
-	 */
-	public boolean isTerminateOnly() {
-		return execution.isTerminateOnly();
-	}
-
 	/**
 	 * @return the sum of skips accumulated in the parent {@link StepExecution}
 	 * and this <code>StepContribution</code>.
 	 */
 	public int getStepSkipCount() {
-		return skipCount + execution.getSkipCount();
+		return skipCount + parentSkipCount;
 	}
 
 	/**

spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/CommandLineJobRunner.java

@@ -195,11 +195,11 @@ public class CommandLineJobRunner {
 							.splitArrayElementsIntoProperties(parameters, "="));
 
 			JobExecution jobExecution = launcher.run(job, jobParameters);
-			return exitCodeMapper.getExitCode(jobExecution.getExitStatus()
+			return exitCodeMapper.intValue(jobExecution.getExitStatus()
 					.getExitCode());
 		} catch (Throwable e) {
 			logger.error("Job Terminated in error:", e);
-			return exitCodeMapper.getExitCode(ExitStatus.FAILED.getExitCode());
+			return exitCodeMapper.intValue(ExitStatus.FAILED.getExitCode());
 		}
 	}
 

spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/ExitCodeMapper.java

@@ -1,12 +1,12 @@
 package org.springframework.batch.core.launch.support;
 
-
 /**
  * 
- * This interface should be implemented when an environment calling the batch famework has specific
- * requirements regarding the process return codes. 
- *
- * @param The type of returncode expected by the environment
+ * This interface should be implemented when an environment calling the batch
+ * framework has specific requirements regarding the operating system process
+ * return status.
+ * 
+ * @param The type of return status expected by the environment
  * @author Stijn Maller
  * @author Lucas Ward
  * @author Dave Syer
@@ -14,17 +14,22 @@ package org.springframework.batch.core.launch.support;
 public interface ExitCodeMapper {
 
 	static int JVM_EXITCODE_COMPLETED = 0;
+
 	static int JVM_EXITCODE_GENERIC_ERROR = 1;
+
 	static int JVM_EXITCODE_JOB_ERROR = 2;
+
 	public static final String NO_SUCH_JOB = "NO_SUCH_JOB";
+
 	public static final String JOB_NOT_PROVIDED = "JOB_NOT_PROVIDED";
 
 	/**
-	 * Transform the exitcode known by the batchframework into an exitcode in the
-	 * format of the calling environment.
-	 * @param exitCode	The exitcode which is used internally by the batch framework.
-	 * @return			The corresponding exitcode as known by the calling environment.
+	 * Convert the exit code from String into an integer that the calling
+	 * environment as an operating system can interpret as an exit status.
+	 * @param exitCode The exit code which is used internally.
+	 * @return The corresponding exit status as known by the calling
+	 * environment.
 	 */
-	public int getExitCode(String exitCode);
-	
+	public int intValue(String exitCode);
+
 }

spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/SimpleJvmExitCodeMapper.java

@@ -23,10 +23,10 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.springframework.batch.repeat.ExitStatus;
 
-
 /**
- * An implementation of {@link ExitCodeMapper} that can be configured 
- * through a map from batch exit codes (String) to integer results.
+ * An implementation of {@link ExitCodeMapper} that can be configured through a
+ * map from batch exit codes (String) to integer results. Some default entries
+ * are set up to recognise common cases.  Any that are injected are added to these.
  * 
  * @author Stijn Maller
  * @author Lucas Ward
@@ -36,19 +36,15 @@ import org.springframework.batch.repeat.ExitStatus;
 public class SimpleJvmExitCodeMapper implements ExitCodeMapper {
 
 	protected Log logger = LogFactory.getLog(getClass());
-	
+
 	private Map mapping;
-	
-	public SimpleJvmExitCodeMapper(){
+
+	public SimpleJvmExitCodeMapper() {
 		mapping = new HashMap();
-		mapping.put(ExitStatus.FINISHED.getExitCode(), 
-				new Integer(JVM_EXITCODE_COMPLETED));
-		mapping.put(ExitStatus.FAILED.getExitCode(), 
-				new Integer(JVM_EXITCODE_GENERIC_ERROR));
-		mapping.put(ExitCodeMapper.JOB_NOT_PROVIDED, 
-				new Integer(JVM_EXITCODE_JOB_ERROR));
-		mapping.put(ExitCodeMapper.NO_SUCH_JOB, 
-				new Integer(JVM_EXITCODE_JOB_ERROR));
+		mapping.put(ExitStatus.FINISHED.getExitCode(), new Integer(JVM_EXITCODE_COMPLETED));
+		mapping.put(ExitStatus.FAILED.getExitCode(), new Integer(JVM_EXITCODE_GENERIC_ERROR));
+		mapping.put(ExitCodeMapper.JOB_NOT_PROVIDED, new Integer(JVM_EXITCODE_JOB_ERROR));
+		mapping.put(ExitCodeMapper.NO_SUCH_JOB, new Integer(JVM_EXITCODE_JOB_ERROR));
 	}
 
 	public Map getMapping() {
@@ -56,33 +52,36 @@ public class SimpleJvmExitCodeMapper implements ExitCodeMapper {
 	}
 
 	/**
-	 * Supply the ExitCodeMappings 
-	 * @param exitCodeMap A set of mappings between environment specific exit codes
-	 * 					  and batch framework internal exit codes
+	 * Supply the ExitCodeMappings
+	 * @param exitCodeMap A set of mappings between environment specific exit
+	 * codes and batch framework internal exit codes
 	 */
 	public void setMapping(Map exitCodeMap) {
 		mapping.putAll(exitCodeMap);
 	}
 
 	/**
-	 * Get the JVM exitcode that matches a certain Batch Framework Exitcode
-	 * @param exitCode The exitcode of the Batch Job as known by the Batch Framework
-	 * @return The exitCode of the Batch Job as known by the JVM 
+	 * Get the operating system exit status that matches a certain Batch
+	 * Framework Exitcode
+	 * @param exitCode The exitcode of the Batch Job as known by the Batch
+	 * Framework
+	 * @return The exitCode of the Batch Job as known by the JVM
 	 */
-	public int getExitCode(String exitCode) {
-		
+	public int intValue(String exitCode) {
+
 		Integer statusCode = null;
-		
-		try{
-			statusCode = (Integer)mapping.get(exitCode);
+
+		try {
+			statusCode = (Integer) mapping.get(exitCode);
 		}
-		catch(RuntimeException ex){ 
-			//We still need to return an exit code, even if there is an issue with
-			//the mapper.
-			logger.fatal("Error mapping exit code, generic exit code returned.", ex);
+		catch (RuntimeException ex) {
+			// We still need to return an exit code, even if there is an issue
+			// with
+			// the mapper.
+			logger.fatal("Error mapping exit code, generic exit status returned.", ex);
 		}
-		
+
 		return (statusCode != null) ? statusCode.intValue() : JVM_EXITCODE_GENERIC_ERROR;
 	}
-	
+
 }

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

@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.springframework.batch.core.step.item;
+package org.springframework.batch.core.step;
 
 import org.springframework.batch.core.StepExecution;
 

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

@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.springframework.batch.core.step.item;
+package org.springframework.batch.core.step;
 
 import org.springframework.batch.repeat.ExitStatus;
 import org.springframework.batch.support.ExceptionClassifier;

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

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-package org.springframework.batch.core.step.item;
+package org.springframework.batch.core.step;
 
 import java.util.concurrent.Semaphore;
 

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

@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.springframework.batch.core.step.item;
+package org.springframework.batch.core.step;
 
 import java.io.PrintWriter;
 import java.io.StringWriter;

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

@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.springframework.batch.core.step.item;
+package org.springframework.batch.core.step;
 
 import org.springframework.batch.core.StepExecution;
 

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

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-package org.springframework.batch.core.step.item;
+package org.springframework.batch.core.step;
 
 import org.springframework.core.JdkVersion;
 import org.springframework.util.ClassUtils;
@@ -25,7 +25,7 @@ import org.springframework.util.ClassUtils;
  * 
  * @author Ben Hale
  */
-class StepExecutionSyncronizerFactory {
+public class StepExecutionSyncronizerFactory {
 
 	/** Whether the backport-concurrent library is present on the classpath */
 	private static final boolean backportConcurrentAvailable = ClassUtils.isPresent(
@@ -34,7 +34,7 @@ class StepExecutionSyncronizerFactory {
 
 	private final StepExecutionSynchronizer synchronizer;
 
-	StepExecutionSyncronizerFactory() {
+	public StepExecutionSyncronizerFactory() {
 		if (JdkVersion.isAtLeastJava15()) {
 			synchronizer = new JdkConcurrentStepExecutionSynchronizer();
 		} else if (backportConcurrentAvailable) {

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

@@ -14,11 +14,11 @@
  * limitations under the License.
  */
 
-package org.springframework.batch.core.step.item;
+package org.springframework.batch.core.step;
 
 import org.springframework.batch.core.JobInterruptedException;
 import org.springframework.batch.core.Step;
-import org.springframework.batch.repeat.RepeatContext;
+import org.springframework.batch.core.StepExecution;
 
 /**
  * Strategy interface for an interruption policy. This policy allows
@@ -32,9 +32,9 @@ public interface StepInterruptionPolicy {
 	/**
 	 * Has the job been interrupted? If so then throw a
 	 * {@link JobInterruptedException}.
-	 * @param context the current context of the running step.
+	 * @param stepExecution the current context of the running step.
 	 * 
 	 * @throws JobInterruptedException when the job has been interrupted.
 	 */
-	void checkInterrupted(RepeatContext context) throws JobInterruptedException;
+	void checkInterrupted(StepExecution stepExecution) throws JobInterruptedException;
 }

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

@@ -14,12 +14,12 @@
  * limitations under the License.
  */
 
-package org.springframework.batch.core.step.item;
+package org.springframework.batch.core.step;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.springframework.batch.core.JobInterruptedException;
-import org.springframework.batch.repeat.RepeatContext;
+import org.springframework.batch.core.StepExecution;
 
 /**
  * Policy that checks the current thread to see if it has been interrupted.
@@ -38,20 +38,20 @@ public class ThreadStepInterruptionPolicy implements StepInterruptionPolicy {
 	 * Returns if the current job lifecycle has been interrupted by checking if
 	 * the current thread is interrupted.
 	 */
-	public void checkInterrupted(RepeatContext context) throws JobInterruptedException {
+	public void checkInterrupted(StepExecution stepExecution) throws JobInterruptedException {
 
-		if (isInterrupted(context)) {
+		if (isInterrupted(stepExecution)) {
 			throw new JobInterruptedException("Job interrupted status detected.");
 		}
 		
 	}
 
 	/**
-	 * @param context the current context
+	 * @param stepExecution the current context
 	 * @return true if the job has been interrupted
 	 */
-	private boolean isInterrupted(RepeatContext context) {
-		boolean interrupted = (Thread.currentThread().isInterrupted() || context.isTerminateOnly());
+	private boolean isInterrupted(StepExecution stepExecution) {
+		boolean interrupted = (Thread.currentThread().isInterrupted() || stepExecution.isTerminateOnly());
 		if(interrupted){
 			logger.error("Step interrupted");
 		}

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

@@ -28,7 +28,12 @@ import org.springframework.batch.core.UnexpectedJobExecutionException;
 import org.springframework.batch.core.listener.CompositeStepListener;
 import org.springframework.batch.core.repository.JobRepository;
 import org.springframework.batch.core.step.AbstractStep;
-import org.springframework.batch.core.step.tasklet.Tasklet;
+import org.springframework.batch.core.step.ExitStatusExceptionClassifier;
+import org.springframework.batch.core.step.SimpleExitStatusExceptionClassifier;
+import org.springframework.batch.core.step.StepExecutionSynchronizer;
+import org.springframework.batch.core.step.StepExecutionSyncronizerFactory;
+import org.springframework.batch.core.step.StepInterruptionPolicy;
+import org.springframework.batch.core.step.ThreadStepInterruptionPolicy;
 import org.springframework.batch.item.ItemReader;
 import org.springframework.batch.item.ItemStream;
 import org.springframework.batch.item.ItemWriter;
@@ -268,10 +273,12 @@ public class ItemOrientedStep extends AbstractStep {
 				public ExitStatus doInIteration(final RepeatContext context) throws Exception {
 
 					final StepContribution contribution = stepExecution.createStepContribution();
-					contribution.setExecutionContext(stepExecution.getExecutionContext());
 					// Before starting a new transaction, check for
 					// interruption.
-					interruptionPolicy.checkInterrupted(context);
+					if (stepExecution.isTerminateOnly()) {
+						context.setTerminateOnly();
+					}
+					interruptionPolicy.checkInterrupted(stepExecution);
 
 					ExitStatus result = ExitStatus.CONTINUABLE;
 
@@ -281,7 +288,7 @@ public class ItemOrientedStep extends AbstractStep {
 					try {
 
 						itemHandler.mark();
-						result = processChunk(contribution);
+						result = processChunk(stepExecution, contribution);
 						contribution.incrementCommitCount();
 
 						// If the step operations are asynchronous then we need
@@ -340,7 +347,7 @@ public class ItemOrientedStep extends AbstractStep {
 					// Check for interruption after transaction as well, so that
 					// the interrupted exception is correctly propagated up to
 					// caller
-					interruptionPolicy.checkInterrupted(context);
+					interruptionPolicy.checkInterrupted(stepExecution);
 
 					return result;
 
@@ -444,23 +451,25 @@ public class ItemOrientedStep extends AbstractStep {
 	 * transaction. The transaction is programmatically started and stopped
 	 * outside this method, so subclasses that override do not need to create a
 	 * transaction.
+	 * @param execution the current {@link StepExecution} which should be
+	 * treated as read-only for the purposes of this method.
+	 * @param contribution the current {@link StepContribution} which can accept
+	 * changes to be aggregated later into the step execution.
 	 * 
-	 * @param step the current step containing the {@link Tasklet} with the
-	 * business logic.
 	 * @return true if there is more data to process.
 	 */
-	protected ExitStatus processChunk(final StepContribution contribution) {
+	protected ExitStatus processChunk(final StepExecution execution, final StepContribution contribution) {
 		ExitStatus result = chunkOperations.iterate(new RepeatCallback() {
 			public ExitStatus doInIteration(final RepeatContext context) throws Exception {
-				if (contribution.isTerminateOnly()) {
+				if (execution.isTerminateOnly()) {
 					context.setTerminateOnly();
 				}
 				// check for interruption before each item as well
-				interruptionPolicy.checkInterrupted(context);
+				interruptionPolicy.checkInterrupted(execution);
 				ExitStatus exitStatus = itemHandler.handle(contribution);
 				contribution.incrementTaskCount();
 				// check for interruption after each item as well
-				interruptionPolicy.checkInterrupted(context);
+				interruptionPolicy.checkInterrupted(execution);
 				return exitStatus;
 			}
 		});
@@ -494,8 +503,8 @@ public class ItemOrientedStep extends AbstractStep {
 	private void processRollback(final StepExecution stepExecution, final ExceptionHolder fatalException,
 			TransactionStatus transaction) {
 		/*
-		 * Any exception thrown within the transaction should
-		 * automatically cause the transaction to rollback.
+		 * Any exception thrown within the transaction should automatically
+		 * cause the transaction to rollback.
 		 */
 		stepExecution.rollback();
 
@@ -506,9 +515,8 @@ public class ItemOrientedStep extends AbstractStep {
 		}
 		catch (Exception e) {
 			/*
-			 * If we already failed to commit, it doesn't help
-			 * to do this again - it's better to allow the
-			 * CommitFailedException to propagate
+			 * If we already failed to commit, it doesn't help to do this again -
+			 * it's better to allow the CommitFailedException to propagate
 			 */
 			if (!fatalException.hasException()) {
 				fatalException.setException(e);

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

@@ -15,8 +15,9 @@
  */
 package org.springframework.batch.core.step.item;
 
-import org.springframework.batch.core.ItemSkipPolicy;
 import org.springframework.batch.core.StepContribution;
+import org.springframework.batch.core.step.skip.ItemSkipPolicy;
+import org.springframework.batch.core.step.skip.NeverSkipItemSkipPolicy;
 import org.springframework.batch.item.ItemReader;
 import org.springframework.batch.item.ItemWriter;
 import org.springframework.batch.item.Skippable;

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

@@ -2,6 +2,8 @@ package org.springframework.batch.core.step.item;
 
 import java.util.Arrays;
 
+import org.springframework.batch.core.step.skip.LimitCheckingItemSkipPolicy;
+import org.springframework.batch.core.step.skip.NeverSkipItemSkipPolicy;
 import org.springframework.batch.repeat.exception.SimpleLimitExceptionHandler;
 
 /**

spring-batch-core/src/main/java/org/springframework/batch/core/step/skip/AlwaysSkipItemSkipPolicy.java

@@ -13,9 +13,8 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.springframework.batch.core.step.item;
+package org.springframework.batch.core.step.skip;
 
-import org.springframework.batch.core.ItemSkipPolicy;
 
 /**
  * Implementation of the {@link ItemSkipPolicy} interface that

spring-batch-core/src/main/java/org/springframework/batch/core/step/skip/ItemSkipPolicy.java

@@ -13,25 +13,26 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.springframework.batch.core;
-
+package org.springframework.batch.core.step.skip;
 
 /**
- * Policy for determining whether or not an item should be skipped.
+ * Policy for determining whether or not some processing should be skipped.
  * 
  * @author Lucas Ward
+ * @author Dave Syer
  */
 public interface ItemSkipPolicy {
 
 	/**
-	 * Returns true or false, indicating whether or not reading should
-	 * continue for the current step execution with the given throwable.
+	 * Returns true or false, indicating whether or not processing should
+	 * continue with the given throwable.
 	 * 
-	 * @param t throwable encountered while reading
+	 * @param t exception encountered while reading
 	 * @param skipCount currently running count of skips
 	 * @return true if reading should continue, false otherwise.
+	 * @throws SkipLimitExceededException if a limit is breached
 	 * @throws IllegalArgumentException if the exception is null
 	 */
-	boolean shouldSkip(Throwable t, int skipCount);
-	
+	boolean shouldSkip(Throwable t, int skipCount) throws SkipLimitExceededException;
+
 }

spring-batch-core/src/main/java/org/springframework/batch/core/step/skip/LimitCheckingItemSkipPolicy.java

@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.springframework.batch.core.step.item;
+package org.springframework.batch.core.step.skip;
 
 import java.io.FileNotFoundException;
 import java.util.Collections;
@@ -22,7 +22,6 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
-import org.springframework.batch.core.ItemSkipPolicy;
 import org.springframework.batch.core.Step;
 import org.springframework.batch.core.StepExecution;
 import org.springframework.batch.item.file.FlatFileParseException;

spring-batch-core/src/main/java/org/springframework/batch/core/step/skip/NeverSkipItemSkipPolicy.java

@@ -13,9 +13,8 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.springframework.batch.core.step.item;
+package org.springframework.batch.core.step.skip;
 
-import org.springframework.batch.core.ItemSkipPolicy;
 
 /**
  * {@link ItemSkipPolicy} implementation that always returns false,

spring-batch-core/src/main/java/org/springframework/batch/core/step/skip/SkipLimitExceededException.java

@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.springframework.batch.core.step.item;
+package org.springframework.batch.core.step.skip;
 
 import org.springframework.batch.core.UnexpectedJobExecutionException;
 

spring-batch-core/src/main/java/org/springframework/batch/core/step/skip/package.html

@@ -0,0 +1,7 @@
+<html>
+<body>
+<p>
+Specific implementations of skip concerns for items in a step.
+</p>
+</body>
+</html>