IN PROGRESS - BATCH-962: refactor TaskletStep's exception handling for better clarity

cleaned up redundant metadata updates from AbstractStep

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

@@ -200,31 +200,12 @@ public abstract class AbstractStep implements Step, InitializingBean, BeanNameAw
 
 			stepExecution.setStatus(BatchStatus.COMPLETED);
 			logger.debug("Step execution success: " + stepExecution);
-
-			try {
-				getJobRepository().update(stepExecution);
-				getJobRepository().updateExecutionContext(stepExecution);
-			}
-			catch (Exception e) {
-				commitException = e;
-				exitStatus = exitStatus.and(ExitStatus.UNKNOWN);
-			}
-
 		}
 		catch (Throwable e) {
-
 			logger.error("Encountered an error executing the step: " + e.getClass() + ": " + e.getMessage(), e);
 			stepExecution.setStatus(determineBatchStatus(e));
 			exitStatus = getDefaultExitStatusForFailure(e);
 			stepExecution.addFailureException(e);
-
-			try {
-				getJobRepository().updateExecutionContext(stepExecution);
-			}
-			catch (Exception ex) {
-				logger.error("Encountered an error on listener error callback.", ex);
-				stepExecution.addFailureException(ex);
-			}
 		}
 		finally {
 
@@ -235,21 +216,32 @@ public abstract class AbstractStep implements Step, InitializingBean, BeanNameAw
 				logger.error("Exception in afterStep callback", e);
 			}
 
-			stepExecution.setExitStatus(exitStatus);
+			try {
+				getJobRepository().updateExecutionContext(stepExecution);
+			}
+			catch (Exception e) {
+				stepExecution.setStatus(BatchStatus.UNKNOWN);
+				exitStatus = exitStatus.and(ExitStatus.UNKNOWN);
+				stepExecution.addFailureException(e);
+				logger.error("Encountered an error saving batch meta data."
+						+ "This job is now in an unknown state and should not be restarted.", commitException);
+			}
+
 			stepExecution.setEndTime(new Date());
 
 			try {
 				getJobRepository().update(stepExecution);
 			}
 			catch (Exception e) {
-				if (commitException == null) {
-					commitException = e;
-				}
-				else {
-					logger.error("Exception while updating step execution after commit exception", e);
-				}
+				stepExecution.setStatus(BatchStatus.UNKNOWN);
+				exitStatus = exitStatus.and(ExitStatus.UNKNOWN);
+				stepExecution.addFailureException(e);
+				logger.error("Encountered an error saving batch meta data."
+						+ "This job is now in an unknown state and should not be restarted.", commitException);
 			}
 
+			stepExecution.setExitStatus(exitStatus);
+
 			try {
 				close(stepExecution.getExecutionContext());
 			}
@@ -259,16 +251,8 @@ public abstract class AbstractStep implements Step, InitializingBean, BeanNameAw
 			}
 
 			StepSynchronizationManager.release();
-
-			if (commitException != null) {
-				stepExecution.setStatus(BatchStatus.UNKNOWN);
-				logger.error("Encountered an error saving batch meta data."
-						+ "This job is now in an unknown state and should not be restarted.", commitException);
-				stepExecution.addFailureException(commitException);
-			}
-
+			
 			logger.debug("Step execution complete: " + stepExecution);
-
 		}
 	}
 

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

@@ -291,7 +291,7 @@ public class AbstractStepTests {
 		assertEquals("close", events.get(i++));
 		assertEquals(3, events.size());
 
-		assertEquals(ExitStatus.UNKNOWN.getExitCode(), execution.getExitStatus().getExitCode());
+		assertEquals(ExitStatus.UNKNOWN, execution.getExitStatus());
 	}
 
 	/**

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

@@ -130,7 +130,7 @@ public class TaskletStepExceptionTests {
 		taskletStep.execute(stepExecution);
 		assertEquals(COMPLETED, stepExecution.getStatus());
 		assertFalse(stepExecution.getFailureExceptions().contains(exception));
-		assertEquals(4, jobRepository.getUpdateCount());
+		assertEquals(3, jobRepository.getUpdateCount());
 	}
 
 	@Test

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

@@ -298,8 +298,7 @@ public class TaskletStepTests {
 		step.execute(stepExecution);
 
 		// context saved before looping and updated once for every processing
-		// loop (once in this case) and finally in the abstract step (regardless
-		// of execution logic)
+		// loop (once in this case) 
 		assertEquals(3, list.size());
 	}