RESOLVED - issue BATCH-1326: Restart after <stop/> doesn't work if any previous steps have allowStartIfComplete=true

spring-batch-core/src/main/java/org/springframework/batch/core/job/AbstractJob.java

@@ -353,6 +353,8 @@ public abstract class AbstractJob implements Job, StepLocator, BeanNameAware, In
 				throw new JobInterruptedException("Job interrupted by step execution");
 			}
 
+		} else {
+			// currentStepExecution.setExitStatus(ExitStatus.NOOP);
 		}
 
 		return currentStepExecution;

spring-batch-core/src/main/java/org/springframework/batch/core/job/flow/FlowJob.java

@@ -32,10 +32,10 @@ import org.springframework.batch.core.repository.JobRestartException;
 import org.springframework.batch.core.step.StepHolder;
 
 /**
- * Implementation of the {@link Job} interface that allows for complex flows
- * of steps, rather than requiring sequential execution.  In general, this
- * job implementation was designed to be used behind a parser, allowing for
- * a namespace to abstract away details.
+ * Implementation of the {@link Job} interface that allows for complex flows of
+ * steps, rather than requiring sequential execution. In general, this job
+ * implementation was designed to be used behind a parser, allowing for a
+ * namespace to abstract away details.
  * 
  * @author Dave Syer
  * @since 2.0
@@ -61,8 +61,7 @@ public class FlowJob extends AbstractJob {
 	/**
 	 * Public setter for the flow.
 	 * 
-	 * @param flow
-	 *            the flow to set
+	 * @param flow the flow to set
 	 */
 	public void setFlow(Flow flow) {
 		this.flow = flow;
@@ -79,14 +78,14 @@ public class FlowJob extends AbstractJob {
 		}
 		return null;
 	}
-	
+
 	/**
 	 * {@inheritDoc}
 	 */
 	@Override
 	public Collection<String> getStepNames() {
 		Collection<String> steps = new HashSet<String>();
-		for (State state: flow.getStates()) {
+		for (State state : flow.getStates()) {
 			if (state instanceof StepHolder) {
 				steps.add(state.getName());
 			}
@@ -98,17 +97,16 @@ public class FlowJob extends AbstractJob {
 	 * @see AbstractJob#doExecute(JobExecution)
 	 */
 	@Override
-	protected void doExecute(final JobExecution execution)
-			throws JobExecutionException {
+	protected void doExecute(final JobExecution execution) throws JobExecutionException {
 		try {
 			JobFlowExecutor executor = new JobFlowExecutor(execution);
 			executor.updateJobExecutionStatus(flow.start(executor).getStatus());
-		} catch (FlowExecutionException e) {
+		}
+		catch (FlowExecutionException e) {
 			if (e.getCause() instanceof JobExecutionException) {
 				throw (JobExecutionException) e.getCause();
 			}
-			throw new JobExecutionException(
-					"Flow execution ended unexpectedly", e);
+			throw new JobExecutionException("Flow execution ended unexpectedly", e);
 		}
 	}
 
@@ -132,24 +130,22 @@ public class FlowJob extends AbstractJob {
 			stepExecutionHolder.set(null);
 		}
 
-		public String executeStep(Step step) throws JobInterruptedException,
-				JobRestartException, StartLimitExceededException {
+		public String executeStep(Step step) throws JobInterruptedException, JobRestartException,
+				StartLimitExceededException {
 			StepExecution stepExecution = handleStep(step, execution);
 			stepExecutionHolder.set(stepExecution);
-			return stepExecution == null ? ExitStatus.COMPLETED.getExitCode()
-					: stepExecution.getExitStatus().getExitCode();
+			return stepExecution == null ? ExitStatus.COMPLETED.getExitCode() : stepExecution.getExitStatus()
+					.getExitCode();
 		}
 
 		public void abandonStepExecution() {
 			StepExecution lastStepExecution = stepExecutionHolder.get();
-			if (lastStepExecution != null
-					&& lastStepExecution.getStatus().isGreaterThan(
-							BatchStatus.STOPPING)) {
+			if (lastStepExecution != null && lastStepExecution.getStatus().isGreaterThan(BatchStatus.STOPPING)) {
 				lastStepExecution.upgradeStatus(BatchStatus.ABANDONED);
 				updateStepExecution(lastStepExecution);
 			}
 		}
-		
+
 		public void updateJobExecutionStatus(FlowExecutionStatus status) {
 			execution.setStatus(findBatchStatus(status));
 			exitStatus = exitStatus.and(new ExitStatus(status.getName()));
@@ -167,11 +163,19 @@ public class FlowJob extends AbstractJob {
 		public void close(FlowExecution result) {
 			stepExecutionHolder.set(null);
 		}
-		
+
 		public boolean isRestart() {
+			if (getStepExecution() != null && getStepExecution().getStatus() == BatchStatus.ABANDONED) {
+				/*
+				 * This is assumed to be the last step execution and it was
+				 * marked abandoned, so we are in a restart of a stopped step.
+				 * TODO: mark the step execution in some more definitive way?
+				 */
+				return true;
+			}
 			return execution.getStepExecutions().isEmpty();
 		}
-		
+
 		public void addExitStatus(String code) {
 			exitStatus = exitStatus.and(new ExitStatus(code));
 		}

spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/StopAndRestartFailedJobParserTests.java

@@ -43,7 +43,7 @@ public class StopAndRestartFailedJobParserTests extends AbstractJobParserTests {
 		//
 		// First Launch
 		//
-		JobExecution jobExecution = launchAndAssert("[s1]");
+		JobExecution jobExecution = launchAndAssert("[s0, s1]");
 		StepExecution stepExecution = getStepExecution(jobExecution, "s1");
 		assertEquals(BatchStatus.ABANDONED, stepExecution.getStatus());
 		assertEquals(ExitStatus.FAILED.getExitCode(), stepExecution.getExitStatus().getExitCode());
@@ -52,7 +52,7 @@ public class StopAndRestartFailedJobParserTests extends AbstractJobParserTests {
 		// Second Launch
 		//
 		stepNamesList.clear();
-		jobExecution = launchAndAssert("[s2]");
+		jobExecution = launchAndAssert("[s0, s2]");
 		stepExecution = getStepExecution(jobExecution, "s2");
 		assertEquals(BatchStatus.COMPLETED, stepExecution.getStatus());
 		assertEquals(ExitStatus.COMPLETED.getExitCode(), stepExecution.getExitStatus().getExitCode());

spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/StopAndRestartFailedJobParserTests-context.xml

@@ -1,17 +1,21 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<beans:beans xmlns="http://www.springframework.org/schema/batch" xmlns:beans="http://www.springframework.org/schema/beans"
+<beans xmlns="http://www.springframework.org/schema/beans"
 	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+	xmlns:p="http://www.springframework.org/schema/p"
 	xsi:schemaLocation="http://www.springframework.org/schema/batch http://www.springframework.org/schema/batch/spring-batch-2.0.xsd
-	http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-2.5.xsd">
+		http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-2.5.xsd">
 
-	<beans:import resource="common-context.xml" />
-	
-	<job id="job">
+	<import resource="common-context.xml" />
+
+	<job id="job" xmlns="http://www.springframework.org/schema/batch">
+		<step id="s0" parent="step0" next="s1"/>
 		<step id="s1" parent="failingStep">
-			<stop on="FAILED" restart="s2"/>
-			<end on="*"/>
+			<stop on="FAILED" restart="s2" />
+			<end on="*" />
 		</step>
-		<step id="s2" parent="step2"/>
+		<step id="s2" parent="step2" />
 	</job>
+	
+	<bean id="step0" parent="step1" p:allowStartIfComplete="true"/>
 
-</beans:beans>
+</beans>