REOPENED - BATCH-999: JobExecution ExecutionContext should not be persisted by Step?

persist the job execution context only after step completion

spring-batch-core/src/main/java/org/springframework/batch/core/JobExecution.java

@@ -218,7 +218,8 @@ public class JobExecution extends Entity {
 	}
 
 	/**
-	 * Returns the {@link ExecutionContext} for this execution
+	 * Returns the {@link ExecutionContext} for this execution. The content is
+	 * expected to be persisted after each step completion (successful or not).
 	 * 
 	 * @return the context
 	 */

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

@@ -319,6 +319,8 @@ public abstract class AbstractJob implements Job, BeanNameAware, InitializingBea
 			jobRepository.add(currentStepExecution);
 
 			step.execute(currentStepExecution);
+			
+			jobRepository.updateExecutionContext(execution);
 
 			if (currentStepExecution.getStatus() == BatchStatus.STOPPED
 					|| currentStepExecution.getStatus() == BatchStatus.STOPPING) {

spring-batch-core/src/main/java/org/springframework/batch/core/repository/JobRepository.java

@@ -126,12 +126,19 @@ public interface JobRepository {
 
 	/**
 	 * Persist the updated {@link ExecutionContext}s of the given
-	 * {@link StepExecution} and corresponding {@link JobExecution}.
+	 * {@link StepExecution}.
 	 * 
 	 * @param stepExecution
 	 */
 	void updateExecutionContext(StepExecution stepExecution);
 
+	/**
+	 * Persist the updated {@link ExecutionContext} of the given
+	 * {@link JobExecution}.
+	 * @param jobExecution
+	 */
+	void updateExecutionContext(JobExecution jobExecution);
+
 	/**
 	 * @param stepName the name of the step execution that might have run.
 	 * @return the last execution of step for the given job instance.

spring-batch-core/src/main/java/org/springframework/batch/core/repository/support/SimpleJobRepository.java

@@ -173,9 +173,12 @@ public class SimpleJobRepository implements JobRepository {
 	}
 
 	public void updateExecutionContext(StepExecution stepExecution) {
-		ecDao.updateExecutionContext(stepExecution.getJobExecution());
 		ecDao.updateExecutionContext(stepExecution);
 	}
+	
+	public void updateExecutionContext(JobExecution jobExecution) {
+		ecDao.updateExecutionContext(jobExecution);
+	}
 
 	public StepExecution getLastStepExecution(JobInstance jobInstance, String stepName) {
 		List<JobExecution> jobExecutions = jobExecutionDao.findJobExecutions(jobInstance);
@@ -242,8 +245,14 @@ public class SimpleJobRepository implements JobRepository {
 		if (jobInstance == null) {
 			return null;
 		}
-		return jobExecutionDao.getLastJobExecution(jobInstance);
+		JobExecution jobExecution = jobExecutionDao.getLastJobExecution(jobInstance);
+	
+		if (jobExecution != null) {
+			jobExecution.setExecutionContext(ecDao.getExecutionContext(jobExecution));
+		}
+		return jobExecution;
 
 	}
 
+
 }

spring-batch-core/src/test/java/org/springframework/batch/core/job/AbstractJobTests.java

@@ -15,16 +15,21 @@
  */
 package org.springframework.batch.core.job;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static org.junit.Assert.*;
+
+import java.util.Date;
 
 import org.junit.Test;
+import org.springframework.batch.core.BatchStatus;
 import org.springframework.batch.core.JobExecution;
 import org.springframework.batch.core.JobExecutionException;
+import org.springframework.batch.core.JobInterruptedException;
+import org.springframework.batch.core.JobParameters;
 import org.springframework.batch.core.StepExecution;
+import org.springframework.batch.core.repository.JobRepository;
+import org.springframework.batch.core.repository.support.MapJobRepositoryFactoryBean;
+import org.springframework.batch.core.step.StepSupport;
+import org.springframework.batch.support.transaction.ResourcelessTransactionManager;
 
 /**
  * @author Dave Syer
@@ -35,7 +40,8 @@ public class AbstractJobTests {
 	AbstractJob job = new StubJob("job");
 
 	/**
-	 * Test method for {@link org.springframework.batch.core.job.AbstractJob#getName()}.
+	 * Test method for
+	 * {@link org.springframework.batch.core.job.AbstractJob#getName()}.
 	 */
 	@Test
 	public void testGetName() {
@@ -44,7 +50,9 @@ public class AbstractJobTests {
 	}
 
 	/**
-	 * Test method for {@link org.springframework.batch.core.job.AbstractJob#setBeanName(java.lang.String)}.
+	 * Test method for
+	 * {@link org.springframework.batch.core.job.AbstractJob#setBeanName(java.lang.String)}
+	 * .
 	 */
 	@Test
 	public void testSetBeanName() {
@@ -53,7 +61,9 @@ public class AbstractJobTests {
 	}
 
 	/**
-	 * Test method for {@link org.springframework.batch.core.job.AbstractJob#setBeanName(java.lang.String)}.
+	 * Test method for
+	 * {@link org.springframework.batch.core.job.AbstractJob#setBeanName(java.lang.String)}
+	 * .
 	 */
 	@Test
 	public void testSetBeanNameWithNullName() {
@@ -64,7 +74,9 @@ public class AbstractJobTests {
 	}
 
 	/**
-	 * Test method for {@link org.springframework.batch.core.job.AbstractJob#setRestartable(boolean)}.
+	 * Test method for
+	 * {@link org.springframework.batch.core.job.AbstractJob#setRestartable(boolean)}
+	 * .
 	 */
 	@Test
 	public void testSetRestartable() {
@@ -78,7 +90,7 @@ public class AbstractJobTests {
 		String value = job.toString();
 		assertTrue("Should contain name: " + value, value.indexOf("name=") >= 0);
 	}
-	
+
 	@Test
 	public void testAfterPropertiesSet() throws Exception {
 		job.setJobRepository(null);
@@ -91,9 +103,52 @@ public class AbstractJobTests {
 		}
 	}
 
+	/**
+	 * Runs the step and persists job execution context.
+	 */
+	@Test
+	public void testHandleStep() throws Exception {
+		
+		class StubStep extends StepSupport {
+
+			static final String value = "message for next steps";
+
+			static final String key = "StubStep";
+
+			{
+				setName("StubStep");
+			}
+
+			public void execute(StepExecution stepExecution) throws JobInterruptedException {
+				stepExecution.getJobExecution().getExecutionContext().put(key, value);
+			}
+		}
+		
+		MapJobRepositoryFactoryBean.clear();
+		MapJobRepositoryFactoryBean factory = new MapJobRepositoryFactoryBean();
+		factory.setTransactionManager(new ResourcelessTransactionManager());
+		factory.afterPropertiesSet();
+		JobRepository repository = (JobRepository) factory.getObject();
+		job.setJobRepository(repository);
+		job.setRestartable(true);
+
+		JobExecution execution = repository.createJobExecution("testHandleStepJob", new JobParameters());
+		job.handleStep(new StubStep(), execution);
+
+		assertEquals(StubStep.value, execution.getExecutionContext().get(StubStep.key));
+
+		// simulate restart and check the job execution context's content survives
+		execution.setEndTime(new Date());
+		execution.setStatus(BatchStatus.FAILED);
+		repository.update(execution);
+
+		JobExecution restarted = repository.createJobExecution("testHandleStepJob", new JobParameters());
+		assertEquals(StubStep.value, restarted.getExecutionContext().get(StubStep.key));
+	}
+
 	/**
 	 * @author Dave Syer
-	 *
+	 * 
 	 */
 	private static class StubJob extends AbstractJob {
 		/**

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

@@ -82,4 +82,7 @@ public class JobRepositorySupport implements JobRepository {
 		return null;
 	}
 
+	public void updateExecutionContext(JobExecution jobExecution) {
+	}
+
 }

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

@@ -280,6 +280,9 @@ public class TaskletStepExceptionTests {
 		public JobExecution getLastJobExecution(String jobName, JobParameters jobParameters) {
 			return null;
 		}
+
+		public void updateExecutionContext(JobExecution jobExecution) {
+		}
 	}
 
 }