From 8e1ad48aa6637f94cc550bbd2f4f0a3b1a8d78a4 Mon Sep 17 00:00:00 2001 From: robokaso Date: Mon, 25 Feb 2008 12:37:24 +0000 Subject: [PATCH] IN PROGRESS - issue BATCH-340: Refactor JobRepository for greater clarity and consistency. http://jira.springframework.org/browse/BATCH-340 removed redundant stepNames property from JobInstance (StepInstance heritage) --- .../batch/core/domain/JobInstance.java | 16 --------- .../batch/core/domain/JobInstanceTests.java | 19 ----------- .../batch/execution/job/simple/SimpleJob.java | 9 ++--- .../repository/SimpleJobRepository.java | 33 ++++++++----------- .../execution/job/simple/SimpleJobTests.java | 7 ++-- .../dao/JdbcStepDaoPrefixTests.java | 2 -- .../StepExecutorInterruptionTests.java | 9 ++--- 7 files changed, 25 insertions(+), 70 deletions(-) diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/domain/JobInstance.java b/spring-batch-core/src/main/java/org/springframework/batch/core/domain/JobInstance.java index f9808406e..018f8342a 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/domain/JobInstance.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/domain/JobInstance.java @@ -16,8 +16,6 @@ package org.springframework.batch.core.domain; -import java.util.ArrayList; -import java.util.List; /** @@ -31,8 +29,6 @@ import java.util.List; */ public class JobInstance extends Entity { - private List stepNames = new ArrayList(); - private JobParameters jobParameters; private Job job; @@ -58,18 +54,6 @@ public class JobInstance extends Entity { public JobExecution getLastExecution() { return lastExecution; } - - public List getStepNames() { - return stepNames; - } - - public void setStepNames(List stepInstances) { - this.stepNames = stepInstances; - } - - public void addStepName(String stepName) { - this.stepNames.add(stepName); - } public int getJobExecutionCount() { return jobExecutionCount; diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/domain/JobInstanceTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/domain/JobInstanceTests.java index df7520801..c68088cec 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/domain/JobInstanceTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/domain/JobInstanceTests.java @@ -15,8 +15,6 @@ */ package org.springframework.batch.core.domain; -import java.util.Collections; - import junit.framework.TestCase; /** @@ -34,23 +32,6 @@ public class JobInstanceTests extends TestCase { assertEquals(lastExecution, instance.getLastExecution()); } - /** - * Test method for {@link org.springframework.batch.core.domain.JobInstance#getStepNames()}. - */ - public void testGetSteps() { - assertEquals(0, instance.getStepNames().size()); - instance.setStepNames(Collections.singletonList("")); - assertEquals(1, instance.getStepNames().size()); - } - - /** - * Test method for {@link org.springframework.batch.core.domain.JobInstance#addStepName(org.springframework.batch.core.domain.StepInstance)}. - */ - public void testAddStep() { - instance.addStepName(""); - assertEquals(1, instance.getStepNames().size()); - } - /** * Test method for {@link org.springframework.batch.core.domain.JobInstance#getJobExecutionCount()}. */ diff --git a/spring-batch-execution/src/main/java/org/springframework/batch/execution/job/simple/SimpleJob.java b/spring-batch-execution/src/main/java/org/springframework/batch/execution/job/simple/SimpleJob.java index bdd7bd84f..51e7d404d 100644 --- a/spring-batch-execution/src/main/java/org/springframework/batch/execution/job/simple/SimpleJob.java +++ b/spring-batch-execution/src/main/java/org/springframework/batch/execution/job/simple/SimpleJob.java @@ -59,8 +59,6 @@ public class SimpleJob extends JobSupport { JobInstance jobInstance = execution.getJobInstance(); jobInstance.setLastExecution(execution); - List stepNames = jobInstance.getStepNames(); - ExitStatus status = ExitStatus.FAILED; try { @@ -76,15 +74,14 @@ public class SimpleJob extends JobSupport { int startedCount = 0; List steps = getSteps(); - for (Iterator i = stepNames.iterator(), j = steps.iterator(); i.hasNext() && j.hasNext();) { + for (Iterator i = steps.iterator(); i.hasNext();) { - String stepInstance = (String) i.next(); - Step step = (Step) j.next(); + Step step = (Step) i.next(); if (shouldStart(jobInstance, step)) { startedCount++; updateStatus(execution, BatchStatus.STARTED); - StepExecution stepExecution = execution.createStepExecution(stepInstance); + StepExecution stepExecution = execution.createStepExecution(step.getName()); step.execute(stepExecution); status = stepExecution.getExitStatus(); } diff --git a/spring-batch-execution/src/main/java/org/springframework/batch/execution/repository/SimpleJobRepository.java b/spring-batch-execution/src/main/java/org/springframework/batch/execution/repository/SimpleJobRepository.java index 3fc4e90ac..dff95d7d4 100644 --- a/spring-batch-execution/src/main/java/org/springframework/batch/execution/repository/SimpleJobRepository.java +++ b/spring-batch-execution/src/main/java/org/springframework/batch/execution/repository/SimpleJobRepository.java @@ -25,7 +25,6 @@ import org.springframework.batch.core.domain.JobExecution; import org.springframework.batch.core.domain.JobInstance; import org.springframework.batch.core.domain.JobParameters; import org.springframework.batch.core.domain.JobSupport; -import org.springframework.batch.core.domain.Step; import org.springframework.batch.core.domain.StepExecution; import org.springframework.batch.core.repository.BatchRestartException; import org.springframework.batch.core.repository.JobExecutionAlreadyRunningException; @@ -146,7 +145,7 @@ public class SimpleJobRepository implements JobRepository { Assert.notNull(job, "Job must not be null."); Assert.notNull(jobParameters, "JobParameters must not be null."); - List jobs = new ArrayList(); + List jobInstances = new ArrayList(); JobInstance jobInstance; // Check if a job is restartable, if not, create and return a new job @@ -161,12 +160,12 @@ public class SimpleJobRepository implements JobRepository { * thread or process will block until this transaction has finished. */ - jobs = jobInstanceDao.findJobInstances(job.getName(), jobParameters); + jobInstances = jobInstanceDao.findJobInstances(job.getName(), jobParameters); } - if (jobs.size() == 1) { + if (jobInstances.size() == 1) { // One job was found - jobInstance = (JobInstance) jobs.get(0); + jobInstance = (JobInstance) jobInstances.get(0); jobInstance.setJobExecutionCount(jobExecutionDao.getJobExecutionCount(jobInstance)); if (jobInstance.getJobExecutionCount() > job.getStartLimit()) { throw new BatchRestartException("Restart Max exceeded for Job: " + jobInstance.toString()); @@ -189,10 +188,9 @@ public class SimpleJobRepository implements JobRepository { } } jobInstance.setLastExecution(lastExecution); - jobInstance.setStepNames(getStepNames(job)); jobInstance.setJob(job); } - else if (jobs.size() == 0) { + else if (jobInstances.size() == 0) { // no job found, create one jobInstance = createJobInstance(job, jobParameters); } @@ -206,14 +204,14 @@ public class SimpleJobRepository implements JobRepository { } - private List getStepNames(Job job) { - List stepNames = new ArrayList(job.getSteps().size()); - for (Iterator iterator = job.getSteps().iterator(); iterator.hasNext();) { - Step step = (Step) iterator.next(); - stepNames.add(step.getName()); - } - return stepNames; - } +// private List getStepNames(Job job) { +// List stepNames = new ArrayList(job.getSteps().size()); +// for (Iterator iterator = job.getSteps().iterator(); iterator.hasNext();) { +// Step step = (Step) iterator.next(); +// stepNames.add(step.getName()); +// } +// return stepNames; +// } @@ -290,7 +288,6 @@ public class SimpleJobRepository implements JobRepository { JobInstance jobInstance = jobInstanceDao.createJobInstance(job.getName(), jobParameters); jobInstance.setJob(job); - jobInstance.setStepNames(getStepNames(job)); return jobInstance; } @@ -316,10 +313,6 @@ public class SimpleJobRepository implements JobRepository { } return latest; } - - private JobExecution getLastJobExecution(JobInstance jobInstance) { - return jobExecutionDao.getLastJobExecution(jobInstance); - } public int getStepExecutionCount(JobInstance jobInstance, String stepName) { int count = 0; diff --git a/spring-batch-execution/src/test/java/org/springframework/batch/execution/job/simple/SimpleJobTests.java b/spring-batch-execution/src/test/java/org/springframework/batch/execution/job/simple/SimpleJobTests.java index 1978bf530..c088b603c 100644 --- a/spring-batch-execution/src/test/java/org/springframework/batch/execution/job/simple/SimpleJobTests.java +++ b/spring-batch-execution/src/test/java/org/springframework/batch/execution/job/simple/SimpleJobTests.java @@ -26,6 +26,7 @@ import org.springframework.batch.core.domain.JobExecution; import org.springframework.batch.core.domain.JobInstance; import org.springframework.batch.core.domain.JobInterruptedException; import org.springframework.batch.core.domain.JobParameters; +import org.springframework.batch.core.domain.Step; import org.springframework.batch.core.domain.StepExecution; import org.springframework.batch.core.repository.JobRepository; import org.springframework.batch.core.runtime.ExitStatusExceptionClassifier; @@ -114,9 +115,9 @@ public class SimpleJobTests extends TestCase { jobExecution = jobRepository.createJobExecution(job, jobParameters); jobInstance = jobExecution.getJobInstance(); - List steps = jobInstance.getStepNames(); - step1 = (String) steps.get(0); - step2 = (String) steps.get(1); + List steps = jobInstance.getJob().getSteps(); + step1 = ((Step) steps.get(0)).getName(); + step2 = ((Step) steps.get(1)).getName(); stepExecution1 = new StepExecution(step1, jobExecution, null); stepExecution2 = new StepExecution(step2, jobExecution, null); diff --git a/spring-batch-execution/src/test/java/org/springframework/batch/execution/repository/dao/JdbcStepDaoPrefixTests.java b/spring-batch-execution/src/test/java/org/springframework/batch/execution/repository/dao/JdbcStepDaoPrefixTests.java index 2b1bc70fd..0b6e1b361 100644 --- a/spring-batch-execution/src/test/java/org/springframework/batch/execution/repository/dao/JdbcStepDaoPrefixTests.java +++ b/spring-batch-execution/src/test/java/org/springframework/batch/execution/repository/dao/JdbcStepDaoPrefixTests.java @@ -50,8 +50,6 @@ public class JdbcStepDaoPrefixTests extends TestCase { stepExecution.setId(new Long(1)); stepExecution.incrementVersion(); - job.addStepName(step); - } public void testModifiedUpdateStepExecution(){ diff --git a/spring-batch-execution/src/test/java/org/springframework/batch/execution/step/support/StepExecutorInterruptionTests.java b/spring-batch-execution/src/test/java/org/springframework/batch/execution/step/support/StepExecutorInterruptionTests.java index 370cd1be7..126c7e15d 100644 --- a/spring-batch-execution/src/test/java/org/springframework/batch/execution/step/support/StepExecutorInterruptionTests.java +++ b/spring-batch-execution/src/test/java/org/springframework/batch/execution/step/support/StepExecutorInterruptionTests.java @@ -26,6 +26,7 @@ import org.springframework.batch.core.domain.JobInstance; import org.springframework.batch.core.domain.JobInterruptedException; import org.springframework.batch.core.domain.JobParameters; import org.springframework.batch.core.domain.JobSupport; +import org.springframework.batch.core.domain.Step; import org.springframework.batch.core.domain.StepExecution; import org.springframework.batch.core.repository.JobRepository; import org.springframework.batch.execution.repository.SimpleJobRepository; @@ -51,7 +52,7 @@ public class StepExecutorInterruptionTests extends TestCase { private StepExecutionDao stepExecutionDao = new MapStepDao(); - private JobInstance job; + private JobInstance jobInstance; private RepeatOperationsStep step; @@ -64,7 +65,7 @@ public class StepExecutorInterruptionTests extends TestCase { step.setName("stepName"); jobConfiguration.addStep(step); jobConfiguration.setBeanName("testJob"); - job = jobRepository.createJobExecution(jobConfiguration, new JobParameters()).getJobInstance(); + jobInstance = jobRepository.createJobExecution(jobConfiguration, new JobParameters()).getJobInstance(); step.setJobRepository(jobRepository); step.setTransactionManager(new ResourcelessTransactionManager()); step.setItemReader(new ItemReaderAdapter()); @@ -76,8 +77,8 @@ public class StepExecutorInterruptionTests extends TestCase { public void testInterruptChunk() throws Exception { - List steps = job.getStepNames(); - final String stepName = (String) steps.get(0); + List steps = jobInstance.getJob().getSteps(); + final String stepName = ((Step)steps.get(0)).getName(); JobExecution jobExecutionContext = new JobExecution(new JobInstance(new Long(0L), new JobParameters())); final StepExecution stepExecution = new StepExecution(stepName, jobExecutionContext); step.setItemReader(new AbstractItemReader() {