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)

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;

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()}.
 	 */

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();
 				}

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;

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);
 

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(){

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() {