IN PROGRESS - BATCH-929: Deferrable Constraints cause unrecoverable errors

count non-fatal commit failure as rollback

Author: robokaso

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

@@ -297,9 +297,7 @@ public RepeatStatus doInStepContext(RepeatContext repeatContext, StepContext ste
 					}
 					catch (Exception e) {
 						if (nonFatalCommitExceptions.classify(e)) {
-							stepExecution.setExecutionContext(getJobRepository().getExecutionContext(stepExecution));
-							JobExecution jobExecution = stepExecution.getJobExecution();
-							jobExecution.setExecutionContext(getJobRepository().getExecutionContext(jobExecution));
+							rollbackExecutionContext(stepExecution);
 							throw new CommitException("non-fatal commit failure", e);
 						}
 						else {
@@ -326,10 +324,15 @@ public RepeatStatus doInStepContext(RepeatContext repeatContext, StepContext ste
 					throw e;
 				}
 				catch (Exception e) {
-					// if commit failed, calling rollback on tx manager would cause exception
+					// if commit failed, calling rollback on tx manager would
+					// cause exception
 					if (!(e instanceof CommitException)) {
 						processRollback(stepExecution, fatalException, transaction);
 					}
+					else {
+						// assume the failed commit caused rollback
+						stepExecution.rollback();
+					}
 					throw e;
 				}
 				finally {
@@ -348,6 +351,15 @@ public RepeatStatus doInStepContext(RepeatContext repeatContext, StepContext ste
 				return result;
 			}
 
+			/**
+			 * Load the saved value of ExecutionContext from repository.
+			 */
+			private void rollbackExecutionContext(StepExecution stepExecution) {
+				stepExecution.setExecutionContext(getJobRepository().getExecutionContext(stepExecution));
+				JobExecution jobExecution = stepExecution.getJobExecution();
+				jobExecution.setExecutionContext(getJobRepository().getExecutionContext(jobExecution));
+			}
+
 		});
 
 	}

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

@@ -199,7 +199,7 @@ public RepeatStatus execute(StepContribution contribution, AttributeAccessor att
 	 * status and execution context should be rolled back.
 	 */
 	@Test
-	public void testSkippableCommitError() throws Exception {
+	public void testNonFatalCommitError() throws Exception {
 
 		class TestItemStream extends ItemStreamSupport {
 			private boolean called = false;
@@ -240,6 +240,7 @@ public RepeatStatus execute(StepContribution contribution, AttributeAccessor att
 		assertEquals("step won't refuse to restart", FAILED, stepExecution.getStatus());
 		assertTrue("execution context modified", stream.called);
 		assertTrue("execution context rolled back", stepExecution.getExecutionContext().isEmpty());
+		assertEquals("failed commit counted as rollback", 1, stepExecution.getRollbackCount());
 	}
 
 	@Test

spring-batch-integration/src/test/java/org/springframework/batch/integration/JobRepositorySupport.java

@@ -23,6 +23,7 @@
 import org.springframework.batch.core.repository.JobInstanceAlreadyCompleteException;
 import org.springframework.batch.core.repository.JobRepository;
 import org.springframework.batch.core.repository.JobRestartException;
+import org.springframework.batch.item.ExecutionContext;
 
 /**
  * @author Dave Syer
@@ -87,4 +88,12 @@ public JobExecution getLastJobExecution(String jobName, JobParameters jobParamet
 		return null;
 	}
 
+	public ExecutionContext getExecutionContext(StepExecution stepExecution) {
+		return null;
+	}
+
+	public ExecutionContext getExecutionContext(JobExecution jobExecution) {
+		return null;
+	}
+
 }