From e9d2d9076463ef0aa8f773c60a417e9d3926f73c Mon Sep 17 00:00:00 2001 From: robokaso Date: Mon, 28 Jul 2008 10:56:35 +0000 Subject: [PATCH] IN PROGRESS - BATCH-518: clean up the *Or* repository methods saveOrUpdate(StepExecution) split to save and update --- .../batch/core/repository/JobRepository.java | 32 +++++++++------- .../support/SimpleJobRepository.java | 37 +++++++++++-------- .../batch/core/step/AbstractStep.java | 5 ++- .../core/step/item/ItemOrientedStep.java | 3 +- .../batch/core/job/SimpleJobTests.java | 2 +- .../SimpleJobRepositoryIntegrationTests.java | 8 ++-- .../support/SimpleJobRepositoryTests.java | 2 +- .../batch/core/step/JobRepositorySupport.java | 15 +++++--- .../integration/job/MessageOrientedStep.java | 2 +- .../integration/JobRepositorySupport.java | 6 +++ 10 files changed, 66 insertions(+), 46 deletions(-) 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 857546365..4929b32ef 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 @@ -39,8 +39,8 @@ public interface JobRepository { /** * Find or create a {@link JobExecution} for a given {@link Job} and - * {@link JobParameters}. If the {@link Job} was already executed with - * these {@link JobParameters}, its persisted values (including ID) will be + * {@link JobParameters}. If the {@link Job} was already executed with these + * {@link JobParameters}, its persisted values (including ID) will be * returned in a new {@link JobInstance}, associated with the * {@link JobExecution}. If no previous instance is found, the execution * will be associated with a new {@link JobInstance} @@ -63,7 +63,7 @@ public interface JobRepository { JobRestartException, JobInstanceAlreadyCompleteException; /** - * Update the {@link JobExecution}. + * Update the {@link JobExecution}. * * Preconditions: {@link JobExecution} must contain a valid * {@link JobInstance} and be saved (have an id assigned). @@ -73,25 +73,29 @@ public interface JobRepository { void updateJobExecution(JobExecution jobExecution); /** - * Save or update a {@link StepExecution}. If no ID is found a new instance - * will be created (and saved). If an ID does exist it will be updated. It - * is not advisable that an ID be assigned before calling this method. - * Instead, it should be left blank, to be assigned by a - * {@link JobRepository}. The {@link ExecutionContext} of the - * {@link StepExecution} is not saved: see - * {@link #saveOrUpdateExecutionContext(StepExecution)}. + * Save the {@link StepExecution}. ID will be assigned - it is not advisable + * that an ID be assigned before calling this method. Instead, it should be + * left blank, to be assigned by a {@link JobRepository}. The + * {@link ExecutionContext} of the {@link StepExecution} is not + * saved: see {@link #saveOrUpdateExecutionContext(StepExecution)}. * * Preconditions: {@link StepExecution} must have a valid {@link Step}. * * @param stepExecution */ - void saveOrUpdate(StepExecution stepExecution); + void save(StepExecution stepExecution); + + /** + * Update the {@link StepExecution}. + * + * Preconditions: {@link StepExecution} must be saved (have an id assigned). + * + * @param stepExecution + */ + void update(StepExecution stepExecution); /** * Save the {@link ExecutionContext} of the given {@link StepExecution}. - * Implementations are allowed to ensure that the {@link StepExecution} is - * already saved by calling {@link #saveOrUpdate(StepExecution)} before - * saving the {@link ExecutionContext}. * * @param stepExecution the {@link StepExecution} containing the * {@link ExecutionContext} to be saved. 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 225117ae1..4bfc806ce 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 @@ -213,27 +213,34 @@ public class SimpleJobRepository implements JobRepository { } /** - * Save or Update the given StepExecution. If it's id is null, it will be - * saved and an id will be set, otherwise it will be updated. It should be - * noted that assigning an ID randomly will likely cause an exception - * depending on the StepDao implementation. + * Save the {@link StepExecution}. * - * @param stepExecution to be saved. - * @throws IllegalArgumentException if stepExecution is null. + * Preconditions: step name must be given and associated + * {@link JobExecution} must already be saved (have an id assigned). */ - public void saveOrUpdate(StepExecution stepExecution) { + public void save(StepExecution stepExecution) { + validateStepExecution(stepExecution); + stepExecutionDao.saveStepExecution(stepExecution); + } + + /** + * Update the {@link StepExecution}. + * + * Preconditions: step name must be given and associated + * {@link JobExecution} must already be saved (have an id assigned). + */ + public void update(StepExecution stepExecution) { + validateStepExecution(stepExecution); + Assert.notNull(stepExecution.getId(), "StepExecution must already be saved (have an id assigned)"); + + stepExecutionDao.updateStepExecution(stepExecution); + } + + private void validateStepExecution(StepExecution stepExecution) { Assert.notNull(stepExecution, "StepExecution cannot be null."); Assert.notNull(stepExecution.getStepName(), "StepExecution's step name cannot be null."); Assert.notNull(stepExecution.getJobExecutionId(), "StepExecution must belong to persisted JobExecution"); - - if (stepExecution.getId() == null) { - stepExecutionDao.saveStepExecution(stepExecution); - } - else { - // existing execution, update - stepExecutionDao.updateStepExecution(stepExecution); - } } /* diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/AbstractStep.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/AbstractStep.java index 3d267aa30..d76007d85 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/AbstractStep.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/step/AbstractStep.java @@ -181,6 +181,7 @@ public abstract class AbstractStep implements Step, InitializingBean, BeanNameAw UnexpectedJobExecutionException { stepExecution.setStartTime(new Date()); stepExecution.setStatus(BatchStatus.STARTED); + getJobRepository().save(stepExecution); ExitStatus exitStatus = ExitStatus.FAILED; Exception commitException = null; @@ -205,7 +206,7 @@ public abstract class AbstractStep implements Step, InitializingBean, BeanNameAw exitStatus = exitStatus.and(getCompositeListener().afterStep(stepExecution)); try { - getJobRepository().saveOrUpdate(stepExecution); + getJobRepository().update(stepExecution); getJobRepository().saveOrUpdateExecutionContext(stepExecution); } catch (Exception e) { @@ -235,7 +236,7 @@ public abstract class AbstractStep implements Step, InitializingBean, BeanNameAw stepExecution.setEndTime(new Date()); try { - getJobRepository().saveOrUpdate(stepExecution); + getJobRepository().update(stepExecution); } catch (Exception e) { if (commitException == null) { diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/ItemOrientedStep.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/ItemOrientedStep.java index 5585a76e4..eb09677cd 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/ItemOrientedStep.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/ItemOrientedStep.java @@ -222,7 +222,6 @@ public class ItemOrientedStep extends AbstractStep { */ protected ExitStatus doExecute(final StepExecution stepExecution) throws Exception { stream.update(stepExecution.getExecutionContext()); - getJobRepository().saveOrUpdate(stepExecution); getJobRepository().saveOrUpdateExecutionContext(stepExecution); itemHandler.mark(); @@ -402,7 +401,7 @@ public class ItemOrientedStep extends AbstractStep { if (fatalException.hasException()) { // Try and give the system a chance to update the stepExecution with // the failure status. - getJobRepository().saveOrUpdate(stepExecution); + getJobRepository().update(stepExecution); } } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/job/SimpleJobTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/job/SimpleJobTests.java index 7fe43f59d..532579d20 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/job/SimpleJobTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/job/SimpleJobTests.java @@ -499,7 +499,7 @@ public class SimpleJobTests extends TestCase { passedInStepContext = new ExecutionContext(stepExecution.getExecutionContext()); stepExecution.getExecutionContext().putString("stepKey", "stepValue"); stepExecution.getJobExecution().getExecutionContext().putString("jobKey", "jobValue"); - jobRepository.saveOrUpdate(stepExecution); + jobRepository.save(stepExecution); jobRepository.saveOrUpdateExecutionContext(stepExecution); if (exception instanceof RuntimeException) { diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/repository/support/SimpleJobRepositoryIntegrationTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/repository/support/SimpleJobRepositoryIntegrationTests.java index 7a4b893d1..feb7324c6 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/repository/support/SimpleJobRepositoryIntegrationTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/repository/support/SimpleJobRepositoryIntegrationTests.java @@ -112,7 +112,7 @@ public class SimpleJobRepositoryIntegrationTests extends AbstractTransactionalDa JobExecution firstJobExec = jobRepository.createJobExecution(job, jobParameters); StepExecution firstStepExec = new StepExecution(step.getName(), firstJobExec); jobRepository.updateJobExecution(firstJobExec); - jobRepository.saveOrUpdate(firstStepExec); + jobRepository.save(firstStepExec); assertEquals(1, jobRepository.getStepExecutionCount(firstJobExec.getJobInstance(), step)); assertEquals(firstStepExec, jobRepository.getLastStepExecution(firstJobExec.getJobInstance(), step)); @@ -122,7 +122,7 @@ public class SimpleJobRepositoryIntegrationTests extends AbstractTransactionalDa firstStepExec.setStartTime(new Date(5)); firstStepExec.setStatus(BatchStatus.FAILED); firstStepExec.setEndTime(new Date(6)); - jobRepository.saveOrUpdate(firstStepExec); + jobRepository.update(firstStepExec); firstJobExec.setStatus(BatchStatus.FAILED); firstJobExec.setEndTime(new Date(7)); jobRepository.updateJobExecution(firstJobExec); @@ -131,7 +131,7 @@ public class SimpleJobRepositoryIntegrationTests extends AbstractTransactionalDa JobExecution secondJobExec = jobRepository.createJobExecution(job, jobParameters); StepExecution secondStepExec = new StepExecution(step.getName(), secondJobExec); jobRepository.updateJobExecution(secondJobExec); - jobRepository.saveOrUpdate(secondStepExec); + jobRepository.save(secondStepExec); assertEquals(2, jobRepository.getStepExecutionCount(secondJobExec.getJobInstance(), step)); assertEquals(secondStepExec, jobRepository.getLastStepExecution(secondJobExec.getJobInstance(), step)); @@ -153,7 +153,7 @@ public class SimpleJobRepositoryIntegrationTests extends AbstractTransactionalDa StepExecution stepExec = new StepExecution(step.getName(), jobExec); stepExec.setExecutionContext(ctx); - jobRepository.saveOrUpdate(stepExec); + jobRepository.save(stepExec); jobRepository.saveOrUpdateExecutionContext(stepExec); StepExecution retrievedStepExec = jobRepository.getLastStepExecution(jobExec.getJobInstance(), step); diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/repository/support/SimpleJobRepositoryTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/repository/support/SimpleJobRepositoryTests.java index 044001efa..8946ce662 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/repository/support/SimpleJobRepositoryTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/repository/support/SimpleJobRepositoryTests.java @@ -142,7 +142,7 @@ public class SimpleJobRepositoryTests extends TestCase { // failure scenario -- no step id set. try { - jobRepository.saveOrUpdate(stepExecution); + jobRepository.save(stepExecution); fail(); } catch (Exception ex) { 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 04a19fab9..825657e54 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 @@ -47,12 +47,6 @@ public class JobRepositorySupport implements JobRepository { */ public void saveOrUpdate(StepExecution stepExecution) { } - - /* (non-Javadoc) - * @see org.springframework.batch.core.repository.JobRepository#saveOrUpdateExecutionContext(org.springframework.batch.core.domain.StepExecution) - */ - public void saveOrUpdateExecutionContext(StepExecution stepExecution) { - } /* (non-Javadoc) * @see org.springframework.batch.container.common.repository.JobRepository#update(org.springframework.batch.container.common.domain.Job) @@ -76,4 +70,13 @@ public class JobRepositorySupport implements JobRepository { return null; } + public void save(StepExecution stepExecution) { + } + + public void update(StepExecution stepExecution) { + } + + public void saveOrUpdateExecutionContext(StepExecution stepExecution) { + } + } diff --git a/spring-batch-integration/src/main/java/org/springframework/batch/integration/job/MessageOrientedStep.java b/spring-batch-integration/src/main/java/org/springframework/batch/integration/job/MessageOrientedStep.java index 40e65e733..95b5fa7ca 100644 --- a/spring-batch-integration/src/main/java/org/springframework/batch/integration/job/MessageOrientedStep.java +++ b/spring-batch-integration/src/main/java/org/springframework/batch/integration/job/MessageOrientedStep.java @@ -111,7 +111,7 @@ public class MessageOrientedStep extends AbstractStep { else { executionContext.putString(WAITING, "true"); // TODO: need these two lines to be atomic - getJobRepository().saveOrUpdate(stepExecution); + getJobRepository().update(stepExecution); requestChannel.send(new GenericMessage(request)); waitForReply(request.getJobId()); } diff --git a/spring-batch-integration/src/test/java/org/springframework/batch/integration/JobRepositorySupport.java b/spring-batch-integration/src/test/java/org/springframework/batch/integration/JobRepositorySupport.java index b22b2b415..e1a8c14af 100644 --- a/spring-batch-integration/src/test/java/org/springframework/batch/integration/JobRepositorySupport.java +++ b/spring-batch-integration/src/test/java/org/springframework/batch/integration/JobRepositorySupport.java @@ -72,4 +72,10 @@ public class JobRepositorySupport implements JobRepository { public void saveOrUpdateExecutionContext(StepExecution stepExecution) { } + public void save(StepExecution stepExecution) { + } + + public void update(StepExecution stepExecution) { + } + }