From a44f40a6cf4e335d849c5c994ab4993eb25334d5 Mon Sep 17 00:00:00 2001 From: robokaso Date: Thu, 22 Jan 2009 17:13:53 +0000 Subject: [PATCH] REOPENED - BATCH-999: JobExecution ExecutionContext should not be persisted by Step? persist the job execution context only after step completion --- .../batch/core/JobExecution.java | 3 +- .../batch/core/job/AbstractJob.java | 2 + .../batch/core/repository/JobRepository.java | 9 ++- .../support/SimpleJobRepository.java | 13 +++- .../batch/core/job/AbstractJobTests.java | 77 ++++++++++++++++--- .../batch/core/step/JobRepositorySupport.java | 3 + .../step/item/TaskletStepExceptionTests.java | 3 + 7 files changed, 95 insertions(+), 15 deletions(-) diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/JobExecution.java b/spring-batch-core/src/main/java/org/springframework/batch/core/JobExecution.java index 2146a83d6..db1a8edb6 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/JobExecution.java +++ b/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 */ diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/job/AbstractJob.java b/spring-batch-core/src/main/java/org/springframework/batch/core/job/AbstractJob.java index 299f11aac..b48b0260d 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/job/AbstractJob.java +++ b/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) { diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/JobRepository.java b/spring-batch-core/src/main/java/org/springframework/batch/core/repository/JobRepository.java index 1a16e2927..7876bacbd 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/JobRepository.java +++ b/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. diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/support/SimpleJobRepository.java b/spring-batch-core/src/main/java/org/springframework/batch/core/repository/support/SimpleJobRepository.java index 70d476573..bcde89292 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/support/SimpleJobRepository.java +++ b/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 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; } + } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/job/AbstractJobTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/job/AbstractJobTests.java index 680fa0051..8c859e72b 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/job/AbstractJobTests.java +++ b/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 { /** diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/JobRepositorySupport.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/JobRepositorySupport.java index 202ed7477..7387950fb 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/JobRepositorySupport.java +++ b/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) { + } + } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/TaskletStepExceptionTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/TaskletStepExceptionTests.java index 7efac2164..a9abeb9cf 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/TaskletStepExceptionTests.java +++ b/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) { + } } }