OPEN - issue BATCH-159: JobExecutor should return a JobExecution (which itself contains the ExitStatus)

http://opensource.atlassian.com/projects/spring/browse/BATCH-159

execution/src/main/java/org/springframework/batch/execution/bootstrap/AbstractJobLauncher.java

@@ -144,9 +144,14 @@ public abstract class AbstractJobLauncher implements JobLauncher,
 	 * Extension point for subclasses. Implementations might choose to start the
 	 * job in a new thread or in the current thread.<br/>
 	 * 
+	 * @param jobIdentifier
+	 *            the identifier of the job to run
 	 * @param exitCallback
 	 *            a callback that should be called by the implementation after
-	 *            the job has ended
+	 *            the job has ended (or failed)
+	 * 
+	 * @return an {@link ExitStatus} indicating the current knowledge of the
+	 *         state of the job.
 	 * 
 	 * @param runtimeInformation
 	 *            the {@link JobIdentifier} to start the launcher with.
@@ -302,8 +307,9 @@ public abstract class AbstractJobLauncher implements JobLauncher,
 	}
 
 	private boolean isInternalRunning(JobIdentifier jobIdentifier) {
-		synchronized(registry) {
-			return isRunning(jobIdentifier) && registry.containsKey(jobIdentifier);
+		synchronized (registry) {
+			return isRunning(jobIdentifier)
+					&& registry.containsKey(jobIdentifier);
 		}
 	}
 
@@ -312,8 +318,10 @@ public abstract class AbstractJobLauncher implements JobLauncher,
 	 * {@link JobIdentifier} to see if it is running. As long as at least one
 	 * job is running the launcher is deemed to be running.
 	 * 
-	 * @param jobIdentifier a {@link JobIdentifier}
-	 * @return always true.  Subclasses can override and provide more accurate information.
+	 * @param jobIdentifier
+	 *            a {@link JobIdentifier}
+	 * @return always true. Subclasses can override and provide more accurate
+	 *         information.
 	 */
 	protected boolean isRunning(JobIdentifier jobIdentifier) {
 		return true;

execution/src/main/java/org/springframework/batch/execution/bootstrap/JobLauncher.java

@@ -21,10 +21,9 @@ import org.springframework.batch.execution.facade.JobExecutorFacade;
 import org.springframework.batch.repeat.ExitStatus;
 
 /**
- * Simple interface for controlling jobs from the job configuration registry,
- * including possible ad-hoc executions, based on different runtime identifiers.
- * Implementations should concentrate on managing jobs and delegate the
- * launching to a {@link JobExecutorFacade}.
+ * Simple interface for controlling jobs, including possible ad-hoc executions,
+ * based on different runtime identifiers. Implementations should concentrate on
+ * managing jobs and delegate the launching to a {@link JobExecutorFacade}.
  * 
  * @author Lucas Ward
  * @author Dave Syer
@@ -32,25 +31,26 @@ import org.springframework.batch.repeat.ExitStatus;
 
 public interface JobLauncher {
 
-	public static final String NO_SUCH_JOB_CONFIGURATION = "NO_SUCH_JOB_CONFIGURATION";
-	public static final String JOB_CONFIGURATION_NOT_PROVIDED = "JOB_CONFIGURATION_NOT_PROVIDED";
-
 	/**
 	 * Start a job execution with default name and other runtime information
 	 * generated on the fly.<br/>
 	 * 
-	 * @return the exit code from the job if it returns synchronously.
+	 * @return the exit code from the job if it returns synchronously. If the
+	 *         implementation is asynchronous, the status might well be unknown.
 	 * 
 	 */
 	public ExitStatus run() throws NoSuchJobConfigurationException;
 
 	/**
 	 * Start a job execution with the given name and other runtime information
-	 * generated on the fly.
+	 * generated on the fly. The name is used to locate a job configuration, and
+	 * the other runtime information is used to identify the job instance.
 	 * 
 	 * @param name
-	 *            the name to assign to the job
-	 * @return the exit code from the job if it returns synchronously.
+	 *            the name to assign to the job configuration
+	 * @return the exit code from the job if it returns synchronously. If the
+	 *         implementation is asynchronous, the status might well be unknown.
+	 * 
 	 * @throws NoSuchJobConfigurationException
 	 */
 	public ExitStatus run(String jobName)
@@ -59,7 +59,9 @@ public interface JobLauncher {
 	/**
 	 * Start a job execution with the given runtime information.
 	 * 
-	 * @return the exit code from the job if it returns synchronously.
+	 * @return the exit code from the job if it returns synchronously. If the
+	 *         implementation is asynchronous, the status might well be unknown.
+	 * 
 	 * @throws NoSuchJobConfigurationException
 	 */
 	public ExitStatus run(JobIdentifier jobIdentifier)
@@ -74,7 +76,10 @@ public interface JobLauncher {
 	public void stop();
 
 	/**
-	 * Return whether or not a job execution is currently running.
+	 * Check whether or not any job execution is currently running.
+	 * 
+	 * @return true if this launcher started a job or jobs and one can be
+	 *         determined to be in an active state.
 	 */
 	public boolean isRunning();
 

execution/src/main/java/org/springframework/batch/execution/bootstrap/TaskExecutorJobLauncher.java

@@ -23,6 +23,7 @@ import javax.management.Notification;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.springframework.batch.core.configuration.NoSuchJobConfigurationException;
+import org.springframework.batch.core.domain.JobExecution;
 import org.springframework.batch.core.domain.JobIdentifier;
 import org.springframework.batch.execution.facade.JobExecutorFacade;
 import org.springframework.batch.execution.facade.NoSuchJobExecutionException;
@@ -45,7 +46,8 @@ import org.springframework.util.Assert;
  * 
  * <p>
  * This implementation can run jobs asynchronously. Jobs are stopped by calling
- * the stop method in the {@link JobExecutorFacade}, which is a graceful shutdown.
+ * the stop method in the {@link JobExecutorFacade}, which is a graceful
+ * shutdown.
  * </p>
  * 
  * @see JobExecutorFacade
@@ -99,10 +101,14 @@ public class TaskExecutorJobLauncher extends AbstractJobLauncher implements
 	}
 
 	/**
-	 * Start the job using the task executor provided.
+	 * Start the job using the task executor provided. An {@link Runnable} is
+	 * passed in by the caller which we need to call in a finally block.
 	 * 
 	 * @throws NoSuchJobConfigurationException
 	 *             if a job configuration cannot be located.
+	 *             
+	 * @see org.springframework.batch.execution.bootstrap.AbstractJobLauncher#doRun(org.springframework.batch.core.domain.JobIdentifier,
+	 *      java.lang.Runnable)
 	 */
 	protected ExitStatus doRun(final JobIdentifier jobIdentifier,
 			final Runnable exitCallback) {
@@ -118,17 +124,16 @@ public class TaskExecutorJobLauncher extends AbstractJobLauncher implements
 							.publishEvent(new RepeatOperationsApplicationEvent(
 									jobIdentifier, "No such job",
 									RepeatOperationsApplicationEvent.ERROR));
-					logger
-							.error(
-									"JobConfiguration could not be located inside Runnable for identifier: ["
-											+ jobIdentifier + "]", e);
+					logger.error(
+							"JobConfiguration could not be located inside Runnable for identifier: ["
+									+ jobIdentifier + "]", e);
 				} finally {
 					exitCallback.run();
 				}
 			}
 		});
 
-		return ExitStatus.RUNNING;
+		return ExitStatus.UNKNOWN;
 
 	}
 
@@ -177,9 +182,9 @@ public class TaskExecutorJobLauncher extends AbstractJobLauncher implements
 	 * be out of date by the time this method is called, so it should be used
 	 * for information purposes only.
 	 * 
-	 * @return Properties representing the last {@link JobExecutionContext}
-	 *         objects passed up from the underlying execution. If there are no
-	 *         jobs running it will be empty.
+	 * @return Properties representing the {@link JobExecution} objects passed
+	 *         up from the underlying execution. If there are no jobs running it
+	 *         will be empty.
 	 */
 	public Properties getStatistics() {
 		if (jobExecutorFacade instanceof StatisticsProvider) {
@@ -192,7 +197,8 @@ public class TaskExecutorJobLauncher extends AbstractJobLauncher implements
 	/**
 	 * Publish the provided message to an external listener if there is one.
 	 * 
-	 * @param message the message to publish
+	 * @param message
+	 *            the message to publish
 	 */
 	private void publish(String message) {
 		if (notificationPublisher != null) {

execution/src/main/java/org/springframework/batch/execution/bootstrap/support/BatchCommandLineLauncher.java

@@ -218,7 +218,7 @@ public class BatchCommandLineLauncher {
 			logger.fatal("Could not locate JobConfiguration \"" + jobName
 					+ "\"", e);
 			status = new ExitStatus(false,
-					JobLauncher.NO_SUCH_JOB_CONFIGURATION);
+					ExitCodeMapper.NO_SUCH_JOB_CONFIGURATION);
 		} catch (Throwable t) {
 			logger.fatal(t);
 			status = exceptionClassifier.classifyForExitCode(t);

execution/src/main/java/org/springframework/batch/execution/bootstrap/support/ExitCodeMapper.java

@@ -16,6 +16,8 @@ public interface ExitCodeMapper {
 	static int JVM_EXITCODE_COMPLETED = 0;
 	static int JVM_EXITCODE_GENERIC_ERROR = 1;
 	static int JVM_EXITCODE_JOB_CONFIGURATION_ERROR = 2;
+	public static final String NO_SUCH_JOB_CONFIGURATION = "NO_SUCH_JOB_CONFIGURATION";
+	public static final String JOB_CONFIGURATION_NOT_PROVIDED = "JOB_CONFIGURATION_NOT_PROVIDED";
 
 	/**
 	 * Transform the exitcode known by the batchframework into an exitcode in the

execution/src/main/java/org/springframework/batch/execution/bootstrap/support/SimpleJvmExitCodeMapper.java

@@ -21,7 +21,6 @@ import java.util.Map;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
-import org.springframework.batch.execution.bootstrap.JobLauncher;
 import org.springframework.batch.repeat.ExitStatus;
 
 
@@ -46,9 +45,9 @@ public class SimpleJvmExitCodeMapper implements ExitCodeMapper {
 				new Integer(JVM_EXITCODE_COMPLETED));
 		mapping.put(ExitStatus.FAILED.getExitCode(), 
 				new Integer(JVM_EXITCODE_GENERIC_ERROR));
-		mapping.put(JobLauncher.JOB_CONFIGURATION_NOT_PROVIDED, 
+		mapping.put(ExitCodeMapper.JOB_CONFIGURATION_NOT_PROVIDED, 
 				new Integer(JVM_EXITCODE_JOB_CONFIGURATION_ERROR));
-		mapping.put(JobLauncher.NO_SUCH_JOB_CONFIGURATION, 
+		mapping.put(ExitCodeMapper.NO_SUCH_JOB_CONFIGURATION, 
 				new Integer(JVM_EXITCODE_JOB_CONFIGURATION_ERROR));
 	}
 

execution/src/main/java/org/springframework/batch/execution/facade/JobExecutionListener.java

@@ -0,0 +1,43 @@
+/*
+ * Copyright 2006-2007 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.batch.execution.facade;
+
+import org.springframework.batch.core.domain.JobExecution;
+
+/**
+ * Listener interface for the job execution lifecycle.
+ * 
+ * @author Dave Syer
+ * 
+ */
+public interface JobExecutionListener {
+
+	/**
+	 * Callback for the start of a job, before any steps are processed.
+	 * 
+	 * @param execution
+	 *            the current {@link JobExecution}
+	 */
+	void before(JobExecution execution);
+
+	/**
+	 * Callback for the start of a job, after all steps are processed, or on an
+	 * error.
+	 * 
+	 * @param execution
+	 */
+	void after(JobExecution execution);
+}

execution/src/main/java/org/springframework/batch/execution/facade/JobExecutorFacade.java

@@ -24,8 +24,7 @@ import org.springframework.batch.repeat.ExitStatus;
  * Interface which defines a facade for running jobs. The interface is
  * intentionally minimal, and depends only on simple java types, so that the
  * facade can be used to launch a job from basic environments like a command
- * line or a JMX console. TODO: remove dependency on
- * {@link JobIdentifier}?
+ * line or a JMX console. TODO: remove dependency on {@link JobIdentifier}?
  * 
  * @author Lucas Ward
  * @author Dave Syer
@@ -35,21 +34,25 @@ public interface JobExecutorFacade {
 	/**
 	 * Start a job execution identifiable by the {@link JobIdentifier}.
 	 * Implementations normally require a job configuration to be locatable
-	 * corresponding to the {@link JobIdentifier}, preferably matching
-	 * them at least by name.
-	 * @param runtimeInformation
+	 * corresponding to the {@link JobIdentifier}, preferably matching them at
+	 * least by name.
+	 * 
+	 * @param jobIdentifier
 	 * 
 	 * @throws NoSuchJobConfigurationException
 	 */
-	ExitStatus start(JobIdentifier runtimeInformation) throws NoSuchJobConfigurationException;
+	ExitStatus start(JobIdentifier jobIdentifier)
+			throws NoSuchJobConfigurationException;
 
 	/**
 	 * Stop the job execution that was started with this runtime information.
-	 * @param runtimeInformation the {@link JobIdentifier}.
-	 * @throws NoSuchJobExecutionException if a job with this runtime
-	 * information is not running
+	 * 
+	 * @param jobIdentifier
+	 *            the {@link JobIdentifier}.
+	 * @throws NoSuchJobExecutionException
+	 *             if a job with this runtime information is not running
 	 */
-	void stop(JobIdentifier runtimeInformation) throws NoSuchJobExecutionException;
+	void stop(JobIdentifier jobIdentifier) throws NoSuchJobExecutionException;
 
 	/**
 	 * Simple check for whether or not there are jobs in progress. Can be used

execution/src/main/java/org/springframework/batch/execution/facade/SimpleJobExecutorFacade.java

@@ -48,7 +48,8 @@ import org.springframework.util.Assert;
  * @author Dave Syer
  * 
  */
-public class SimpleJobExecutorFacade implements JobExecutorFacade, StatisticsProvider {
+public class SimpleJobExecutorFacade implements JobExecutorFacade,
+		JobExecutionListener, StatisticsProvider {
 
 	private JobExecutor jobExecutor;
 
@@ -81,18 +82,23 @@ public class SimpleJobExecutorFacade implements JobExecutorFacade, StatisticsPro
 	/**
 	 * Setter for the job execution registry. The default should be adequate so
 	 * this setter method is mainly used for testing.
-	 * @param jobExecutionRegistry the jobExecutionRegistry to set
+	 * 
+	 * @param jobExecutionRegistry
+	 *            the jobExecutionRegistry to set
 	 */
-	public void setJobExecutionRegistry(JobExecutionRegistry jobExecutionRegistry) {
+	public void setJobExecutionRegistry(
+			JobExecutionRegistry jobExecutionRegistry) {
 		this.jobExecutionRegistry = jobExecutionRegistry;
 	}
 
 	/**
 	 * Setter for injection of {@link JobConfigurationLocator}.
 	 * 
-	 * @param jobConfigurationLocator the jobConfigurationLocator to set
+	 * @param jobConfigurationLocator
+	 *            the jobConfigurationLocator to set
 	 */
-	public void setJobConfigurationLocator(JobConfigurationLocator jobConfigurationLocator) {
+	public void setJobConfigurationLocator(
+			JobConfigurationLocator jobConfigurationLocator) {
 		this.jobConfigurationLocator = jobConfigurationLocator;
 	}
 
@@ -101,67 +107,101 @@ public class SimpleJobExecutorFacade implements JobExecutorFacade, StatisticsPro
 	 * {@link JobIdentifier} and the {@link JobConfigurationLocator}.
 	 * 
 	 * @see org.springframework.batch.execution.facade.JobExecutorFacade#start(org.springframework.batch.execution.common.domain.JobConfiguration,
-	 * org.springframework.batch.core.domain.JobIdentifier)
+	 *      org.springframework.batch.core.domain.JobIdentifier)
 	 * 
-	 * @throws IllegalArgumentException if the {@link JobIdentifier} is null or
-	 * its name is null
-	 * @throws IllegalStateException if the {@link JobConfigurationLocator} does
-	 * not contain a {@link JobConfiguration} with the name provided.
-	 * @throws IllegalStateException if the {@link JobExecutor} is null
-	 * @throws IllegalStateException if the {@link JobConfigurationLocator} is
-	 * null
+	 * @throws IllegalArgumentException
+	 *             if the {@link JobIdentifier} is null or its name is null
+	 * @throws IllegalStateException
+	 *             if the {@link JobConfigurationLocator} does not contain a
+	 *             {@link JobConfiguration} with the name provided.
+	 * @throws IllegalStateException
+	 *             if the {@link JobExecutor} is null
+	 * @throws IllegalStateException
+	 *             if the {@link JobConfigurationLocator} is null
 	 * 
 	 */
-	public ExitStatus start(JobIdentifier jobIdentifier) throws NoSuchJobConfigurationException {
+	public ExitStatus start(JobIdentifier jobIdentifier)
+			throws NoSuchJobConfigurationException {
 
 		Assert.notNull(jobIdentifier, "JobIdentifier must not be null.");
-		Assert.notNull(jobIdentifier.getName(), "JobIdentifier name must not be null.");
+		Assert.notNull(jobIdentifier.getName(),
+				"JobIdentifier name must not be null.");
 
-		Assert.state(!jobExecutionRegistry.isRegistered(jobIdentifier),
-				"A job with this JobRuntimeInformation is already executing in this container");
+		Assert
+				.state(!jobExecutionRegistry.isRegistered(jobIdentifier),
+						"A job with this JobRuntimeInformation is already executing in this container");
 
 		Assert.state(jobExecutor != null, "JobExecutor must be provided.");
-		Assert.state(jobConfigurationLocator != null, "JobConfigurationLocator must be provided.");
+		Assert.state(jobConfigurationLocator != null,
+				"JobConfigurationLocator must be provided.");
 
 		JobConfiguration jobConfiguration = jobConfigurationLocator
 				.getJobConfiguration(jobIdentifier.getName());
 
-		JobInstance job = jobRepository.findOrCreateJob(jobConfiguration, jobIdentifier);
+		JobInstance job = jobRepository.findOrCreateJob(jobConfiguration,
+				jobIdentifier);
 		JobExecution jobExecution = jobExecutionRegistry.register(job);
-		
-		ExitStatus exitStatus = ExitStatus.FAILED;
+
 		try {
-			synchronized (mutex) {
-				running++;
-			}
-			exitStatus = jobExecutor.run(jobConfiguration, jobExecution);
-		}
-		finally {
-			synchronized (mutex) {
-				// assume execution is synchronous so when we get to here we are
-				// not running any more
-				running--;
-			}
-			jobExecutionRegistry.unregister(jobIdentifier);
+
+			this.before(jobExecution);
+
+			jobExecutor.run(jobConfiguration, jobExecution);
+
+		} finally {
+
+			this.after(jobExecution);
+
 		}
 
-		return exitStatus;
+		return jobExecution.getExitStatus();
+	}
+
+	/**
+	 * Internal accounting for the job execution. Callback at start of job.
+	 * 
+	 * @param execution
+	 */
+	public void before(JobExecution execution) {
+		synchronized (mutex) {
+			running++;
+		}
+	}
+
+	/**
+	 * Internal accounting for the job execution. Callback at end of job.
+	 * 
+	 * @param execution
+	 */
+	public void after(JobExecution execution) {
+		synchronized (mutex) {
+			// assume execution is synchronous so when we get to here we are
+			// not running any more
+			running--;
+		}
+		jobExecutionRegistry.unregister(execution.getJobIdentifier());
 	}
 
 	/*
 	 * (non-Javadoc)
+	 * 
 	 * @see org.springframework.batch.container.BatchContainer#stop(org.springframework.batch.container.common.runtime.JobRuntimeInformation)
 	 */
-	public void stop(JobIdentifier runtimeInformation) throws NoSuchJobExecutionException {
-		JobExecution jobExecutionContext = jobExecutionRegistry.get(runtimeInformation);
+	public void stop(JobIdentifier runtimeInformation)
+			throws NoSuchJobExecutionException {
+		JobExecution jobExecutionContext = jobExecutionRegistry
+				.get(runtimeInformation);
 		if (jobExecutionContext == null) {
-			throw new NoSuchJobExecutionException("No such Job is executing: [" + runtimeInformation + "]");
+			throw new NoSuchJobExecutionException("No such Job is executing: ["
+					+ runtimeInformation + "]");
 		}
-		for (Iterator iter = jobExecutionContext.getStepContexts().iterator(); iter.hasNext();) {
+		for (Iterator iter = jobExecutionContext.getStepContexts().iterator(); iter
+				.hasNext();) {
 			RepeatContext context = (RepeatContext) iter.next();
 			context.setTerminateOnly();
 		}
-		for (Iterator iter = jobExecutionContext.getChunkContexts().iterator(); iter.hasNext();) {
+		for (Iterator iter = jobExecutionContext.getChunkContexts().iterator(); iter
+				.hasNext();) {
 			RepeatContext context = (RepeatContext) iter.next();
 			context.setTerminateOnly();
 		}
@@ -191,20 +231,23 @@ public class SimpleJobExecutorFacade implements JobExecutorFacade, StatisticsPro
 	public Properties getStatistics() {
 		int i = 0;
 		Properties props = new Properties();
-		for (Iterator iter = jobExecutionRegistry.findAll().iterator(); iter.hasNext();) {
+		for (Iterator iter = jobExecutionRegistry.findAll().iterator(); iter
+				.hasNext();) {
 			JobExecution element = (JobExecution) iter.next();
 			i++;
 			String runtime = "job" + i;
 			props.setProperty(runtime, "" + element.getJobIdentifier());
 			int j = 0;
-			for (Iterator iterator = element.getStepContexts().iterator(); iterator.hasNext();) {
+			for (Iterator iterator = element.getStepContexts().iterator(); iterator
+					.hasNext();) {
 				RepeatContext context = (RepeatContext) iterator.next();
 				j++;
 				props.setProperty(runtime + ".step" + j, "" + context);
 
 			}
 			j = 0;
-			for (Iterator iterator = element.getChunkContexts().iterator(); iterator.hasNext();) {
+			for (Iterator iterator = element.getChunkContexts().iterator(); iterator
+					.hasNext();) {
 				RepeatContext context = (RepeatContext) iterator.next();
 				j++;
 				props.setProperty(runtime + ".chunk" + j, "" + context);