From 3b9b7a96bacef33692891d9f1e07965d3968df24 Mon Sep 17 00:00:00 2001
From: dsyer <dsyer@2e8e966d-6415-0410-94b9-dc4e65435d03>
Date: Sun, 28 Jun 2009 07:31:51 +0000
Subject: [PATCH] Clearer logging and comments

---
 .../batch/core/step/tasklet/TaskletStep.java  | 37 ++++++++++++-------
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/tasklet/TaskletStep.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/tasklet/TaskletStep.java
index 941f26b42..2cd86e8d4 100644
--- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/tasklet/TaskletStep.java
+++ b/spring-batch-core/src/main/java/org/springframework/batch/core/step/tasklet/TaskletStep.java
@@ -106,14 +106,17 @@ public class TaskletStep extends AbstractStep {
 	public TaskletStep(String name) {
 		super(name);
 	}
-	
-	/* (non-Javadoc)
-	 * @see org.springframework.batch.core.step.AbstractStep#afterPropertiesSet()
+
+	/*
+	 * (non-Javadoc)
+	 * 
+	 * @see
+	 * org.springframework.batch.core.step.AbstractStep#afterPropertiesSet()
 	 */
 	@Override
 	public void afterPropertiesSet() throws Exception {
 		super.afterPropertiesSet();
-		Assert.notNull(transactionManager, "TransactionManager is mandatory");		
+		Assert.notNull(transactionManager, "TransactionManager is mandatory");
 	}
 
 	/**
@@ -239,7 +242,8 @@ public class TaskletStep extends AbstractStep {
 		stepOperations.iterate(new StepContextRepeatCallback(stepExecution) {
 
 			@Override
-			public RepeatStatus doInChunkContext(RepeatContext repeatContext, ChunkContext chunkContext) throws Exception {
+			public RepeatStatus doInChunkContext(RepeatContext repeatContext, ChunkContext chunkContext)
+					throws Exception {
 
 				StepExecution stepExecution = chunkContext.getStepContext().getStepExecution();
 
@@ -252,7 +256,7 @@ public class TaskletStep extends AbstractStep {
 				RepeatStatus result = RepeatStatus.CONTINUABLE;
 
 				TransactionStatus transaction = transactionManager.getTransaction(transactionAttribute);
-				
+
 				chunkListener.beforeChunk();
 
 				boolean locked = false;
@@ -262,7 +266,8 @@ public class TaskletStep extends AbstractStep {
 					try {
 						try {
 							result = tasklet.execute(contribution, chunkContext);
-						} catch (Exception e) {
+						}
+						catch (Exception e) {
 							if (transactionAttribute.rollbackOn(e)) {
 								throw e;
 							}
@@ -277,7 +282,6 @@ public class TaskletStep extends AbstractStep {
 
 					}
 
-
 					// If the step operations are asynchronous then we need
 					// to synchronize changes to the step execution (at a
 					// minimum).
@@ -306,19 +310,24 @@ public class TaskletStep extends AbstractStep {
 				}
 				catch (FatalException e) {
 					try {
-						logger.debug("Rollback for FatalException: "+e.getClass().getName()+": "+e.getMessage());
+						logger.debug("Rollback for FatalException: " + e.getClass().getName() + ": " + e.getMessage());
 						rollback(stepExecution, transaction);
 					}
 					catch (Exception rollbackException) {
-						// propagate the original fatal failure, only log the
-						// failed rollback
-						logger.error("Rollback caused by fatal failure failed", rollbackException);
+						/*
+						 * Propagate the original fatal failure; only log the
+						 * failed rollback. The failure can be caused by
+						 * attempting a rollback when the commit has already
+						 * succeeded (which is normal so only logged at debug
+						 * level)
+						 */
+						logger.debug("Rollback caused by fatal failure failed", rollbackException);
 					}
 					throw e;
 				}
 				catch (Error e) {
 					try {
-						logger.debug("Rollback for Error: "+e.getClass().getName()+": "+e.getMessage());
+						logger.debug("Rollback for Error: " + e.getClass().getName() + ": " + e.getMessage());
 						rollback(stepExecution, transaction);
 					}
 					catch (Exception rollbackException) {
@@ -330,7 +339,7 @@ public class TaskletStep extends AbstractStep {
 				}
 				catch (Exception e) {
 					try {
-						logger.debug("Rollback for Exception: "+e.getClass().getName()+": "+e.getMessage());
+						logger.debug("Rollback for Exception: " + e.getClass().getName() + ": " + e.getMessage());
 						rollback(stepExecution, transaction);
 					}
 					catch (Exception rollbackException) {