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

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

Re-organise the Launcher base classes a bit more

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

@@ -52,7 +52,7 @@ public abstract class AbstractJobLauncher implements JobLauncher,
 	private static final Log logger = LogFactory
 			.getLog(AbstractJobLauncher.class);
 
-	protected JobExecutorFacade batchContainer;
+	protected JobExecutorFacade jobExecutorFacade;
 
 	private String jobConfigurationName;
 
@@ -103,8 +103,8 @@ public abstract class AbstractJobLauncher implements JobLauncher,
 	 * 
 	 * @param batchContainer
 	 */
-	public void setJobExecutorFacade(JobExecutorFacade batchContainer) {
-		this.batchContainer = batchContainer;
+	public void setJobExecutorFacade(JobExecutorFacade jobExecutorFacade) {
+		this.jobExecutorFacade = jobExecutorFacade;
 	}
 
 	/**
@@ -114,7 +114,7 @@ public abstract class AbstractJobLauncher implements JobLauncher,
 	 * @see org.springframework.beans.factory.InitializingBean#afterPropertiesSet()
 	 */
 	public void afterPropertiesSet() throws Exception {
-		Assert.notNull(batchContainer);
+		Assert.notNull(jobExecutorFacade);
 	}
 
 	/**
@@ -168,7 +168,7 @@ public abstract class AbstractJobLauncher implements JobLauncher,
 			throws NoSuchJobConfigurationException {
 
 		synchronized (monitor) {
-			if (isRunning(jobIdentifier)) {
+			if (isInternalRunning(jobIdentifier)) {
 				return ExitStatus.RUNNING;
 			}
 		}
@@ -181,10 +181,9 @@ public abstract class AbstractJobLauncher implements JobLauncher,
 		});
 
 		/*
-		 * Subclasses have to take care of unregistering the jobIdentifier - if
-		 * we do it here and doRun() is implemented to return immediately
-		 * without waiting for the job to finish, then we will have a job
-		 * running that is not in the registry.
+		 * Subclasses don't explicitly have to take care of unregistering the
+		 * jobIdentifier - they just have to call the exitCallback when the job
+		 * is finished.
 		 */
 	}
 
@@ -285,8 +284,9 @@ public abstract class AbstractJobLauncher implements JobLauncher,
 		this.stop(jobIdentifierFactory.getJobIdentifier(name));
 	}
 
-	/*
-	 * (non-Javadoc)
+	/**
+	 * Check each registered {@link JobIdentifier} to see if it is running (@see
+	 * {@link #isRunning(JobIdentifier)}), and if any are, then return true.
 	 * 
 	 * @see org.springframework.batch.container.bootstrap.BatchContainerLauncher#isRunning()
 	 */
@@ -294,19 +294,31 @@ public abstract class AbstractJobLauncher implements JobLauncher,
 		Collection jobs = new HashSet(registry.keySet());
 		for (Iterator iter = jobs.iterator(); iter.hasNext();) {
 			JobIdentifier jobIdentifier = (JobIdentifier) iter.next();
-			if (!isRunning(jobIdentifier)) {
-				return false;
+			if (isInternalRunning(jobIdentifier)) {
+				return true;
 			}
 		}
 		return !jobs.isEmpty();
 	}
 
-	protected boolean isRunning(JobIdentifier jobIdentifier) {
-		synchronized (registry) {
-			return registry.get(jobIdentifier) != null;
+	private boolean isInternalRunning(JobIdentifier jobIdentifier) {
+		synchronized(registry) {
+			return isRunning(jobIdentifier) && registry.containsKey(jobIdentifier);
 		}
 	}
 
+	/**
+	 * Extension point for subclasses to check an individual
+	 * {@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.
+	 */
+	protected boolean isRunning(JobIdentifier jobIdentifier) {
+		return true;
+	}
+
 	/**
 	 * Convenient synchronized accessor for the registry. Can be used by
 	 * subclasses if necessary (but it isn't likely).

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

@@ -16,12 +16,10 @@
 
 package org.springframework.batch.execution.bootstrap;
 
-import org.springframework.batch.core.configuration.JobConfiguration;
 import org.springframework.batch.core.configuration.NoSuchJobConfigurationException;
 import org.springframework.batch.core.domain.JobIdentifier;
-import org.springframework.batch.core.runtime.JobIdentifierFactory;
 import org.springframework.batch.execution.facade.JobExecutorFacade;
-import org.springframework.batch.execution.runtime.ScheduledJobIdentifierFactory;
+import org.springframework.batch.execution.facade.NoSuchJobExecutionException;
 import org.springframework.batch.repeat.ExitStatus;
 import org.springframework.util.Assert;
 
@@ -39,144 +37,65 @@ import org.springframework.util.Assert;
  * @author Dave Syer
  * @since 2.1
  */
-public class SimpleJobLauncher implements JobLauncher {
-	
+public class SimpleJobLauncher extends AbstractJobLauncher {
+
 	private volatile Thread processingThread;
-
-	private volatile boolean running = false;
-	
-	private JobExecutorFacade jobExecutorFacade;
-	
-	private JobIdentifierFactory jobIdentifierFactory = new ScheduledJobIdentifierFactory();;
-	
-	private String jobConfigurationName;
-	
-	/**
-	 * Check that mandatory properties are set.
-	 * 
-	 * @see #setJobExecutorFacade(JobExecutorFacade)
-	 * @see org.springframework.beans.factory.InitializingBean#afterPropertiesSet()
-	 */
-	public void afterPropertiesSet() throws Exception {
-		Assert.notNull(jobExecutorFacade);
-	}
+	private int running = 0;
 
 	/**
-	 * Return whether or not the container is currently running. This is done by
+	 * Check whether or not the container is currently running. This is done by
 	 * checking the thread to see if it is still alive.
 	 */
-	public boolean isRunning() {
-		return running && processingThread != null && processingThread.isAlive();
+	protected boolean isRunning(JobIdentifier jobIdentifier) {
+		return processingThread != null && processingThread.isAlive();
 	}
 
 	/**
-	 * Start the provided facade. The current thread will first be saved.
-	 * This may seem odd at first, however, this simple bootstrap requires that
-	 * only one thread can kick off a container, and that the first thread that
-	 * calls start is the 'processing thread'. If the container has already been
+	 * Start the provided facade. The current thread will first be saved. This
+	 * may seem odd at first, however, this simple bootstrap requires that only
+	 * one thread can kick off a container, and that the first thread that calls
+	 * start is the 'processing thread'. If the container has already been
 	 * started, no exception will be thrown.
-	 * @throws NoSuchJobConfigurationException 
+	 * 
+	 * @throws NoSuchJobConfigurationException
 	 * @see Lifecycle#start().
 	 * 
-	 * @throws IllegalStateException if JobConfiguration is null.
+	 * @throws IllegalStateException
+	 *             if JobConfiguration is null.
 	 */
-	public ExitStatus run(JobIdentifier jobIdentifier) throws NoSuchJobConfigurationException {
+	protected ExitStatus doRun(JobIdentifier jobIdentifier, Runnable exitCallback)
+			throws NoSuchJobConfigurationException {
 
 		Assert.notNull(jobIdentifier, "JobIdentifier must not be null.");
-		Assert.isTrue(!isRunning(), "SynchronousLaunchers can run only one job at at time.");
-		
+		Assert.isTrue(running==0,
+				"This launcher can run only one job at at time.");
+
 		/*
 		 * There is no reason to kick off a new thread, since only one thread
-		 * should be processing at once. However, a handle to the thread should
-		 * be maintained to allow for interrupt
+		 * should be processing at once. However, a handle to the thread is
+		 * maintained to allow for interrupt
 		 */
 		processingThread = Thread.currentThread();
-		
-		running = true;
 		try {
+			running++;
 			return jobExecutorFacade.start(jobIdentifier);
-		}
-		finally {
-			running = false;
+		} finally {
+			running--;
+			exitCallback.run();
 		}
 
 	}
-	
-	/**
-	 * Start a job execution with the given name. If a job is already running
-	 * has no effect.
-	 * 
-	 * @param name the name to assign to the job
-	 * @throws NoSuchJobConfigurationException 
-	 */
-	public ExitStatus run(String name) throws NoSuchJobConfigurationException {
-		if (name==null) {
-			throw new NoSuchJobConfigurationException("Null job name cannot be located.");
-		}
-		JobIdentifier runtimeInformation = jobIdentifierFactory.getJobIdentifier(name);
-		return this.run(runtimeInformation);
-	}
-	
-	/**
-	 * Start a job execution with default name and other runtime information
-	 * provided by the factory. If a job is already running has no effect. The
-	 * default name is taken from the enclosed {@link JobConfiguration}.
-	 * @throws NoSuchJobConfigurationException if the job configuration cannot be located
-	 * 
-	 * @see #setJobIdentifierFactory(JobIdentifierFactory)
-	 * @see org.springframework.context.Lifecycle#start()
-	 */
-	public ExitStatus run() throws NoSuchJobConfigurationException{
-		if (jobConfigurationName==null) {
-			throw new NoSuchJobConfigurationException("Null default job name cannot be located.");
-		}
-		return this.run(jobConfigurationName);
-	}
 
 	/**
-	 * Stop the job if it is running by interrupting its thread. If no job is
-	 * running, no action will be taken.
+	 * Interrupt the thread that is running the job.
 	 * 
-	 * (non-Javadoc)
-	 * @see org.springframework.context.Lifecycle#stop()
+	 * @see org.springframework.batch.execution.bootstrap.AbstractJobLauncher#doStop(org.springframework.batch.core.domain.JobIdentifier)
 	 */
-	public void stop() {
-
+	protected void doStop(JobIdentifier runtimeInformation)
+			throws NoSuchJobExecutionException {
 		if (isRunning()) {
 			processingThread.interrupt();
-			running = false;
 		}
 	}
-	
-	/**
-	 * The facade to which job launching will be delegated.
-	 *  
-	 * @param jobExecutorFacade a {@link JobExecutorFacade}.
-	 */
-	public void setJobExecutorFacade(JobExecutorFacade jobExecutorFacade) {
-		this.jobExecutorFacade = jobExecutorFacade;
-	}
 
-	/**
-	 * Public setter for injecting {@link JobIdentifierFactory}. When a job is
-	 * launched by name the factory needs to be used to create a new identifier
-	 * for it.  Defaults to a {@link ScheduledJobIdentifierFactory}.
-	 * 
-	 * @param jobIdentifierFactory
-	 *            the {@link JobIdentifierFactory} to use when constructing
-	 *            identifiers for jobs.
-	 */
-	public void setJobIdentifierFactory(
-			JobIdentifierFactory jobIdentifierFactory) {
-		this.jobIdentifierFactory = jobIdentifierFactory;
-	}
-	
-    /**
-     * Public setter for the default job configuration name to launch if none is specified.
-     * 
-     * @param jobConfigurationName the name of a {@link JobConfiguration} in the registry.
-     */
-    public void setJobConfigurationName(String jobConfigurationName) {
-		this.jobConfigurationName = jobConfigurationName;
-	}
 }

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

@@ -111,7 +111,7 @@ public class TaskExecutorJobLauncher extends AbstractJobLauncher implements
 		taskExecutor.execute(new Runnable() {
 			public void run() {
 				try {
-					batchContainer.start(runtimeInformation);
+					jobExecutorFacade.start(runtimeInformation);
 				} catch (NoSuchJobConfigurationException e) {
 					applicationEventPublisher
 							.publishEvent(new RepeatOperationsApplicationEvent(
@@ -141,7 +141,7 @@ public class TaskExecutorJobLauncher extends AbstractJobLauncher implements
 	 */
 	protected void doStop(JobIdentifier runtimeInformation)
 			throws NoSuchJobExecutionException {
-		batchContainer.stop(runtimeInformation);
+		jobExecutorFacade.stop(runtimeInformation);
 		// TODO: wait for the jobs to stop?
 	}
 
@@ -181,8 +181,8 @@ public class TaskExecutorJobLauncher extends AbstractJobLauncher implements
 	 *         jobs running it will be empty.
 	 */
 	public Properties getStatistics() {
-		if (batchContainer instanceof StatisticsProvider) {
-			return ((StatisticsProvider) batchContainer).getStatistics();
+		if (jobExecutorFacade instanceof StatisticsProvider) {
+			return ((StatisticsProvider) jobExecutorFacade).getStatistics();
 		} else {
 			return new Properties();
 		}