From 5e9915baa8b0c39951844b47ec23f21be40917ab Mon Sep 17 00:00:00 2001 From: dsyer Date: Thu, 23 Aug 2007 07:27:33 +0000 Subject: [PATCH] RESOLVED - issue BATCH-110: Potential simplification of DefaultJobExecutor by not having to include the StepExecutorFactory always http://opensource.atlassian.com/projects/spring/browse/BATCH-110 --- .../execution/job/DefaultJobExecutor.java | 15 ++-- .../step/SimpleStepExecutorFactory.java | 78 +++++++++++++++++++ .../execution/facade/SimpleJobTests.java | 2 +- .../job/DefaultJobExecutorTests.java | 60 ++++++++++---- .../step/SimpleStepExecutorFactoryTests.java | 66 ++++++++++++++++ .../resources/simple-container-definition.xml | 2 +- 6 files changed, 198 insertions(+), 25 deletions(-) create mode 100644 execution/src/main/java/org/springframework/batch/execution/step/SimpleStepExecutorFactory.java create mode 100644 execution/src/test/java/org/springframework/batch/execution/step/SimpleStepExecutorFactoryTests.java diff --git a/execution/src/main/java/org/springframework/batch/execution/job/DefaultJobExecutor.java b/execution/src/main/java/org/springframework/batch/execution/job/DefaultJobExecutor.java index 846c212c6..1191ea2de 100644 --- a/execution/src/main/java/org/springframework/batch/execution/job/DefaultJobExecutor.java +++ b/execution/src/main/java/org/springframework/batch/execution/job/DefaultJobExecutor.java @@ -33,13 +33,13 @@ import org.springframework.batch.core.executor.StepInterruptedException; import org.springframework.batch.core.repository.JobRepository; import org.springframework.batch.core.runtime.JobExecutionContext; import org.springframework.batch.core.runtime.StepExecutionContext; -import org.springframework.batch.execution.step.DefaultStepExecutorFactory; +import org.springframework.batch.execution.step.SimpleStepExecutorFactory; import org.springframework.batch.io.exception.BatchCriticalException; import org.springframework.batch.repeat.ExitStatus; import org.springframework.batch.repeat.RepeatContext; /** - * Default implementation of (@JobLifecycle) interface. Sequentially executes a + * Default implementation of (@link JobExecutor} interface. Sequentially executes a * job by iterating it's life of steps. Interruption of a job run is pluggable * by passing in various interruption policies. * @@ -47,9 +47,11 @@ import org.springframework.batch.repeat.RepeatContext; */ public class DefaultJobExecutor implements JobExecutor { + private static final SimpleStepExecutorFactory DEFAULT_STEP_EXECUTOR_FACTORY = new SimpleStepExecutorFactory(); + private JobRepository jobRepository; - private StepExecutorFactory stepExecutorResolver = new DefaultStepExecutorFactory(); + private StepExecutorFactory stepExecutorFactory = DEFAULT_STEP_EXECUTOR_FACTORY; public ExitStatus run(JobConfiguration configuration, JobExecutionContext jobExecutionContext) throws BatchCriticalException { @@ -69,7 +71,7 @@ public class DefaultJobExecutor implements JobExecutor { StepConfiguration stepConfiguration = (StepConfiguration) j.next(); if (shouldStart(step, stepConfiguration)) { updateStatus(jobExecutionContext, BatchStatus.STARTED); - StepExecutor stepExecutor = stepExecutorResolver.getExecutor(stepConfiguration); + StepExecutor stepExecutor = stepExecutorFactory.getExecutor(stepConfiguration); StepExecutionContext stepExecutionContext = new StepExecutionContext(jobExecutionContext, step); status = stepExecutor.process(stepConfiguration, stepExecutionContext); } @@ -145,10 +147,11 @@ public class DefaultJobExecutor implements JobExecutor { public void setJobRepository(JobRepository jobRepository) { this.jobRepository = jobRepository; + DEFAULT_STEP_EXECUTOR_FACTORY.setJobRepository(jobRepository); } - public void setStepExecutorResolver(StepExecutorFactory stepExecutorResolver) { - this.stepExecutorResolver = stepExecutorResolver; + public void setStepExecutorFactory(StepExecutorFactory stepExecutorResolver) { + this.stepExecutorFactory = stepExecutorResolver; } } diff --git a/execution/src/main/java/org/springframework/batch/execution/step/SimpleStepExecutorFactory.java b/execution/src/main/java/org/springframework/batch/execution/step/SimpleStepExecutorFactory.java new file mode 100644 index 000000000..d2540926b --- /dev/null +++ b/execution/src/main/java/org/springframework/batch/execution/step/SimpleStepExecutorFactory.java @@ -0,0 +1,78 @@ +/* + * Copyright 2006-2007 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.batch.execution.step; + +import org.springframework.batch.core.configuration.StepConfiguration; +import org.springframework.batch.core.executor.StepExecutor; +import org.springframework.batch.core.executor.StepExecutorFactory; +import org.springframework.batch.core.repository.JobRepository; +import org.springframework.batch.execution.step.simple.SimpleStepConfiguration; +import org.springframework.batch.execution.step.simple.SimpleStepExecutor; +import org.springframework.batch.repeat.RepeatOperations; +import org.springframework.batch.repeat.policy.SimpleCompletionPolicy; +import org.springframework.batch.repeat.support.RepeatTemplate; +import org.springframework.util.Assert; + +/** + * A {@link StepExecutorFactory} that only knows how to create + * {@link SimpleStepExecutor} instances. + * + * @author Dave Syer + * + */ +public class SimpleStepExecutorFactory implements StepExecutorFactory { + + private JobRepository jobRepository; + + /** + * Create a {@link SimpleStepExecutor} for this configuration. If the + * configuration is a {@link SimpleStepConfiguration} then a + * {@link StepExecutor} is created with policies matching the commit + * interval of the configuration.
+ * + * @throws IllegalStateException + * if the configuration is not a {@link SimpleStepConfiguration}. + * @throws IllegalStateException + * if the {@link JobRepository} is null. + * + * @see StepExecutorFactory#getExecutor(StepConfiguration) + */ + public StepExecutor getExecutor(StepConfiguration configuration) { + + Assert.notNull(jobRepository, "JobRepository cannot be null"); + Assert.state(configuration instanceof SimpleStepConfiguration, + "StepConfiguration must be instance of SimpleStepConfiguration - found: [" + + (configuration == null ? null : configuration + .getClass()) + "]"); + + SimpleStepExecutor executor = new SimpleStepExecutor(); + executor.setRepository(jobRepository); + RepeatTemplate template = new RepeatTemplate(); + RepeatOperations repeatOperations = template; + template.setCompletionPolicy(new SimpleCompletionPolicy( + ((SimpleStepConfiguration) configuration).getCommitInterval())); + + executor.setChunkOperations(repeatOperations); + + return executor; + + } + + public void setJobRepository(JobRepository jobRepository) { + this.jobRepository = jobRepository; + } + +} diff --git a/execution/src/test/java/org/springframework/batch/execution/facade/SimpleJobTests.java b/execution/src/test/java/org/springframework/batch/execution/facade/SimpleJobTests.java index 1a84f4474..6743d84ea 100644 --- a/execution/src/test/java/org/springframework/batch/execution/facade/SimpleJobTests.java +++ b/execution/src/test/java/org/springframework/batch/execution/facade/SimpleJobTests.java @@ -79,7 +79,7 @@ public class SimpleJobTests extends TestCase { super.setUp(); jobLifecycle.setJobRepository(repository); stepLifecycle.setRepository(repository); - jobLifecycle.setStepExecutorResolver(new StepExecutorFactory() { + jobLifecycle.setStepExecutorFactory(new StepExecutorFactory() { public StepExecutor getExecutor(StepConfiguration configuration) { return stepLifecycle; } diff --git a/execution/src/test/java/org/springframework/batch/execution/job/DefaultJobExecutorTests.java b/execution/src/test/java/org/springframework/batch/execution/job/DefaultJobExecutorTests.java index f9a94d38b..55ab7e043 100644 --- a/execution/src/test/java/org/springframework/batch/execution/job/DefaultJobExecutorTests.java +++ b/execution/src/test/java/org/springframework/batch/execution/job/DefaultJobExecutorTests.java @@ -34,6 +34,7 @@ import org.springframework.batch.core.repository.JobRepository; import org.springframework.batch.core.runtime.JobExecutionContext; import org.springframework.batch.core.runtime.SimpleJobIdentifier; import org.springframework.batch.core.runtime.StepExecutionContext; +import org.springframework.batch.core.tasklet.Tasklet; import org.springframework.batch.execution.repository.SimpleJobRepository; import org.springframework.batch.execution.repository.dao.JobDao; import org.springframework.batch.execution.repository.dao.MapJobDao; @@ -94,9 +95,9 @@ public class DefaultJobExecutorTests extends TestCase { private JobConfiguration jobConfiguration; - private SimpleJobIdentifier jobRuntimeInformation; + private SimpleJobIdentifier jobIdentifer; - private DefaultJobExecutor jobLifecycle; + private DefaultJobExecutor jobExecutor; protected void setUp() throws Exception { super.setUp(); @@ -106,10 +107,10 @@ public class DefaultJobExecutorTests extends TestCase { jobDao = new MapJobDao(); stepDao = new MapStepDao(); jobRepository = new SimpleJobRepository(jobDao, stepDao); - jobLifecycle = new DefaultJobExecutor(); - jobLifecycle.setJobRepository(jobRepository); + jobExecutor = new DefaultJobExecutor(); + jobExecutor.setJobRepository(jobRepository); - jobLifecycle.setStepExecutorResolver(new StepExecutorFactory() { + jobExecutor.setStepExecutorFactory(new StepExecutorFactory() { public StepExecutor getExecutor(StepConfiguration configuration) { return defaultStepLifecycle; } @@ -125,11 +126,11 @@ public class DefaultJobExecutorTests extends TestCase { jobConfiguration = new JobConfiguration(); jobConfiguration.setSteps(stepConfigurations); - jobRuntimeInformation = new SimpleJobIdentifier("TestJob"); + jobIdentifer = new SimpleJobIdentifier("TestJob"); - job = jobRepository.findOrCreateJob(jobConfiguration, jobRuntimeInformation); + job = jobRepository.findOrCreateJob(jobConfiguration, jobIdentifer); - jobExecutionContext = new JobExecutionContext(jobRuntimeInformation, job); + jobExecutionContext = new JobExecutionContext(jobIdentifer, job); List steps = job.getSteps(); step1 = (StepInstance) steps.get(0); @@ -143,18 +144,43 @@ public class DefaultJobExecutorTests extends TestCase { super.tearDown(); } - public void testRunWithDefaultLifecycle() throws Exception { + public void testRunNormally() throws Exception { stepConfiguration1.setStartLimit(5); stepConfiguration2.setStartLimit(5); - jobLifecycle.run(jobConfiguration, jobExecutionContext); + jobExecutor.run(jobConfiguration, jobExecutionContext); + assertEquals(2, list.size()); + checkRepository(BatchStatus.COMPLETED); + } + + public void testRunWithDefaultStepExecutor() throws Exception { + + jobExecutor = new DefaultJobExecutor(); + jobExecutor.setJobRepository(jobRepository); + // do not set StepExecutorFactory... + stepConfiguration1.setStartLimit(5); + stepConfiguration1.setTasklet(new Tasklet() { + public ExitStatus execute() throws Exception { + list.add("1"); + return ExitStatus.FINISHED; + } + }); + stepConfiguration2.setStartLimit(5); + stepConfiguration2.setTasklet(new Tasklet() { + public ExitStatus execute() throws Exception { + list.add("2"); + return ExitStatus.FINISHED; + } + }); + jobExecutor.run(jobConfiguration, jobExecutionContext); assertEquals(2, list.size()); checkRepository(BatchStatus.COMPLETED); } + public void testExecutionContextIsSet() throws Exception { - testRunWithDefaultLifecycle(); + testRunNormally(); assertEquals(job, jobExecutionContext.getJob()); assertEquals(step1, stepExecutionContext1.getStep()); assertEquals(step2, stepExecutionContext2.getStep()); @@ -162,7 +188,7 @@ public class DefaultJobExecutorTests extends TestCase { public void testRunWithNonDefaultExecutor() throws Exception { - jobLifecycle.setStepExecutorResolver(new StepExecutorFactory() { + jobExecutor.setStepExecutorFactory(new StepExecutorFactory() { public StepExecutor getExecutor(StepConfiguration configuration) { return configuration == stepConfiguration2 ? defaultStepLifecycle : configurationStepLifecycle; } @@ -170,7 +196,7 @@ public class DefaultJobExecutorTests extends TestCase { stepConfiguration1.setStartLimit(5); stepConfiguration2.setStartLimit(5); - jobLifecycle.run(jobConfiguration, jobExecutionContext); + jobExecutor.run(jobConfiguration, jobExecutionContext); assertEquals(2, list.size()); assertEquals("special", list.get(0)); @@ -189,7 +215,7 @@ public class DefaultJobExecutorTests extends TestCase { } }; try { - jobLifecycle.run(jobConfiguration, jobExecutionContext); + jobExecutor.run(jobConfiguration, jobExecutionContext); } catch (BatchCriticalException e) { assertEquals(exception, e.getCause()); @@ -209,7 +235,7 @@ public class DefaultJobExecutorTests extends TestCase { } }; try { - jobLifecycle.run(jobConfiguration, jobExecutionContext); + jobExecutor.run(jobConfiguration, jobExecutionContext); } catch (RuntimeException e) { assertEquals(exception, e); @@ -223,7 +249,7 @@ public class DefaultJobExecutorTests extends TestCase { stepConfiguration1.setStartLimit(0); try{ - jobLifecycle.run(jobConfiguration, jobExecutionContext); + jobExecutor.run(jobConfiguration, jobExecutionContext); fail(); } catch( Exception ex ){ @@ -235,7 +261,7 @@ public class DefaultJobExecutorTests extends TestCase { * Check JobRepository to ensure status is being saved. */ private void checkRepository(BatchStatus status) { - assertEquals(job, jobDao.findJobs(jobRuntimeInformation).get(0)); + assertEquals(job, jobDao.findJobs(jobIdentifer).get(0)); // because map dao stores in memory, it can be checked directly assertEquals(status, job.getStatus()); JobExecution jobExecution = (JobExecution) jobDao.findJobExecutions(job).get(0); diff --git a/execution/src/test/java/org/springframework/batch/execution/step/SimpleStepExecutorFactoryTests.java b/execution/src/test/java/org/springframework/batch/execution/step/SimpleStepExecutorFactoryTests.java new file mode 100644 index 000000000..f0b73d505 --- /dev/null +++ b/execution/src/test/java/org/springframework/batch/execution/step/SimpleStepExecutorFactoryTests.java @@ -0,0 +1,66 @@ +/* + * Copyright 2006-2007 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.batch.execution.step; + +import junit.framework.TestCase; + +import org.springframework.batch.core.configuration.StepConfigurationSupport; +import org.springframework.batch.execution.step.simple.JobRepositorySupport; +import org.springframework.batch.execution.step.simple.SimpleStepConfiguration; + +/** + * @author Dave Syer + * + */ +public class SimpleStepExecutorFactoryTests extends TestCase { + + private SimpleStepExecutorFactory factory = new SimpleStepExecutorFactory(); + + protected void setUp() throws Exception { + factory.setJobRepository(new JobRepositorySupport()); + } + + public void testSuccessfulStepExecutor() throws Exception { + assertNotNull(factory.getExecutor(new SimpleStepConfiguration())); + } + + public void testUnsuccessfulWrongConfiguration() throws Exception { + try { + factory.getExecutor(new StepConfigurationSupport()); + fail("Expected IllegalStateException"); + } catch (IllegalStateException e) { + // expected + assertTrue( + "Error message does not contain SimpleStepConfiguration: " + + e.getMessage(), e.getMessage().indexOf( + "SimpleStepConfiguration") >= 0); + } + } + + public void testUnsuccessfulNoJobRepository() throws Exception { + try { + factory = new SimpleStepExecutorFactory(); + factory.getExecutor(new SimpleStepConfiguration()); + fail("Expected IllegalArgumentException"); + } catch (IllegalArgumentException e) { + // expected + assertTrue( + "Error message does not contain JobRepository: " + + e.getMessage(), e.getMessage().indexOf( + "JobRepository") >= 0); + } + } +} diff --git a/execution/src/test/resources/simple-container-definition.xml b/execution/src/test/resources/simple-container-definition.xml index 96fd431aa..ed4a873c8 100644 --- a/execution/src/test/resources/simple-container-definition.xml +++ b/execution/src/test/resources/simple-container-definition.xml @@ -22,7 +22,7 @@ - +