RESOLVED - issue BATCH-706: Commit exception is lost if update fails as well

Check to see if commit already failed before storing exception.  Also ensure that step starts continuable, and only hits UNKNOWN if a commit failure occurs

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

@@ -88,7 +88,7 @@ public class StepExecutionTests extends TestCase {
 	 * {@link org.springframework.batch.core.JobExecution#getExitStatus()}.
 	 */
 	public void testGetExitCode() {
-		assertEquals(ExitStatus.UNKNOWN, execution.getExitStatus());
+		assertEquals(ExitStatus.CONTINUABLE, execution.getExitStatus());
 		execution.setExitStatus(ExitStatus.FINISHED);
 		assertEquals(ExitStatus.FINISHED, execution.getExitStatus());
 	}

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

@@ -170,11 +170,45 @@ public class AbstractStepTests extends TestCase {
 		assertEquals(7, events.size());
 
 		assertEquals(ExitStatus.FAILED.getExitCode(), execution.getExitStatus().getExitCode());
-		
+
 		assertTrue("Execution context modifications made by listener should be persisted", repository.saved
 				.containsKey("onErrorInStep"));
 	}
 
+	/**
+	 * Exception during business processing.
+	 */
+	public void testFailureInSavingExecutionContext() throws Exception {
+		tested = new EventTrackingStep() {
+			protected ExitStatus doExecute(StepExecution stepExecution) throws Exception {
+				super.doExecute(stepExecution);
+				return ExitStatus.FINISHED;
+			}
+		};
+		repository = new JobRepositoryStub() {
+			public void saveOrUpdateExecutionContext(StepExecution stepExecution) {
+				throw new RuntimeException("Bad context!");
+			}
+		};
+		tested.setJobRepository(repository);
+
+		try {
+			tested.execute(execution);
+			fail();
+		}
+		catch (RuntimeException expected) {
+			assertEquals("Bad context!", expected.getCause().getMessage());
+		}
+
+		int i = 0;
+		assertEquals("open", events.get(i++));
+		assertEquals("doExecute", events.get(i++));
+		assertEquals("close", events.get(i++));
+		assertEquals(3, events.size());
+
+		assertEquals(ExitStatus.UNKNOWN.getExitCode(), execution.getExitStatus().getExitCode());
+	}
+
 	/**
 	 * JobRepository is a required property.
 	 */

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

@@ -76,7 +76,7 @@ public class ItemOrientedStepTests extends TestCase {
 	private ItemOrientedStep itemOrientedStep;
 
 	private Job job;
-	
+
 	private JobInstance jobInstance;
 
 	private ResourcelessTransactionManager transactionManager;
@@ -314,7 +314,6 @@ public class ItemOrientedStepTests extends TestCase {
 		assertEquals(BatchStatus.UNKNOWN, stepExecution.getStatus());
 	}
 
-
 	/*
 	 * Test that a job that is being restarted, but has saveExecutionAttributes
 	 * set to false, doesn't have restore or getExecutionAttributes called on
@@ -539,7 +538,8 @@ public class ItemOrientedStepTests extends TestCase {
 		catch (JobInterruptedException ex) {
 			assertEquals(BatchStatus.STOPPED, stepExecution.getStatus());
 			String msg = stepExecution.getExitStatus().getExitDescription();
-			assertTrue("Message does not contain 'JobInterruptedException': " + msg, contains(msg, "JobInterruptedException"));
+			assertTrue("Message does not contain 'JobInterruptedException': " + msg, contains(msg,
+					"JobInterruptedException"));
 		}
 	}
 
@@ -664,14 +664,12 @@ public class ItemOrientedStepTests extends TestCase {
 
 	public void testStatusForFinalUpdateFailedException() throws Exception {
 
-		itemOrientedStep.setJobRepository(new JobRepositorySupport() {
-			public void saveOrUpdate(StepExecution stepExecution) {
-				if (stepExecution.getEndTime() != null) {
-					throw new RuntimeException("Bar");
-				}
-				super.saveOrUpdate(stepExecution);
+		itemOrientedStep.setJobRepository(new JobRepositorySupport());
+		itemOrientedStep.setStreams(new ItemStream[] { new ItemStreamSupport() {
+			public void close(ExecutionContext executionContext) throws ItemStreamException {
+				throw new RuntimeException("Bar");
 			}
-		});
+		} });
 
 		JobExecution jobExecutionContext = new JobExecution(jobInstance);
 		StepExecution stepExecution = new StepExecution(itemOrientedStep.getName(), jobExecutionContext);
@@ -681,12 +679,13 @@ public class ItemOrientedStepTests extends TestCase {
 			fail("Expected RuntimeException");
 		}
 		catch (RuntimeException ex) {
-			// The job actually completeed, but teh streams couldn't be closed.
+			// The job actually completed, but the streams couldn't be closed.
 			assertEquals(BatchStatus.COMPLETED, stepExecution.getStatus());
 			String msg = stepExecution.getExitStatus().getExitDescription();
 			assertEquals("", msg);
 			msg = ex.getMessage();
-			assertTrue("Message does not contain 'saving batch meta data': " + msg, contains(msg, "saving batch meta data"));
+			assertTrue("Message does not contain 'closing step': " + msg, contains(msg,
+					"closing step"));
 			// The original rollback was caused by this one:
 			assertEquals("Bar", ex.getCause().getMessage());
 		}
@@ -754,7 +753,7 @@ public class ItemOrientedStepTests extends TestCase {
 			assertTrue(stepExecution.getExecutionContext().getString("spam").equals("bucket"));
 		}
 	}
-	
+
 	public void testStepToCompletion() throws Exception {
 
 		RepeatTemplate template = new RepeatTemplate();
@@ -769,7 +768,7 @@ public class ItemOrientedStepTests extends TestCase {
 		itemOrientedStep.execute(stepExecution);
 		assertEquals(3, processed.size());
 		assertEquals(3, stepExecution.getItemCount().intValue());
-		}
+	}
 
 	/**
 	 * Exception in {@link StepExecutionListener#afterStep(StepExecution)}