From 00ee45854eb935f44e4b09acb1616735f1b1de35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ste=CC=81phane=20Nicoll?= Date: Sat, 24 Nov 2012 10:43:35 +0100 Subject: [PATCH] Added the concept of a StepRegistry that tracks the Step instance attached to a Job. - Added a simple MapStepRegistry that keeps track of the step in a simple map per job - MapJobRegistry implements StepRegistry as well using the delegate pattern - DefaultJobLoader makes sure to initialize both registries now. Auto-detects that the JobRegistry implements StepRegistry - JobSupport now implements StepLocator so that such job can be loaded by the DefaultJobLoader - Also registering any "detached" step available for a particular job to that job if that wasn't done yet. Since a partition handler may link to any step by name, the step registry must be filled with any step that are available in the context of a job. If each job is located in its own application context, this is just fine. If more than one job is defined in the same context, the step registry may be able to return a step that is not related to that job but it shouldn't hurt as this happens only if a component is explicitly asking to execute a step by name for a particular job. --- .../core/configuration/StepRegistry.java | 47 ++++ .../support/DefaultJobLoader.java | 96 ++++++- .../configuration/support/MapJobRegistry.java | 100 ++++---- .../support/MapStepRegistry.java | 66 +++++ .../support/DefaultJobLoaderTests.java | 220 +++++++++++++--- .../support/MapStepRegistryTests.java | 234 ++++++++++++++++++ .../batch/core/job/JobSupport.java | 27 +- .../job-context-with-separate-steps.xml | 17 ++ .../support/job-context-with-steps.xml | 27 ++ .../support/trivial-context-autoregister.xml | 17 +- .../configuration/support/trivial-context.xml | 23 +- 11 files changed, 764 insertions(+), 110 deletions(-) create mode 100644 spring-batch-core/src/main/java/org/springframework/batch/core/configuration/StepRegistry.java create mode 100644 spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/MapStepRegistry.java create mode 100644 spring-batch-core/src/test/java/org/springframework/batch/core/configuration/support/MapStepRegistryTests.java create mode 100644 spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/support/job-context-with-separate-steps.xml create mode 100644 spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/support/job-context-with-steps.xml diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/StepRegistry.java b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/StepRegistry.java new file mode 100644 index 000000000..291682eec --- /dev/null +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/StepRegistry.java @@ -0,0 +1,47 @@ +package org.springframework.batch.core.configuration; + +import org.springframework.batch.core.Step; +import org.springframework.batch.core.launch.NoSuchJobException; +import org.springframework.batch.core.step.NoSuchStepException; + +import java.util.Collection; + +/** + * Registry keeping track of all the {@link Step} defined in a + * {@link org.springframework.batch.core.Job}. + * + * @author Sebastien Gerard + * @author Stephane Nicoll + */ +public interface StepRegistry { + + /** + * Registers all the step of the given job. If the job is already registered, + * the method {@link #unregisterStepsFromJob(String)} is called before registering + * the given steps. + * + * @param jobName the give job name + * @param steps the job steps + */ + void register(String jobName, Collection steps); + + /** + * Unregisters all the steps of the given job. If the job is not registered, + * nothing happens. + * + * @param jobName the given job name + */ + void unregisterStepsFromJob(String jobName); + + /** + * Returns the {@link Step} of the specified job based on its name. + * + * @param jobName the name of the job + * @param stepName the name of the step to retrieve + * @return the step with the given name belonging to the mentioned job + * @throws NoSuchJobException no such job with that name exists + * @throws NoSuchStepException no such step with that name for that job exists + */ + Step getStep(String jobName, String stepName) throws NoSuchJobException, NoSuchStepException; + +} diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/DefaultJobLoader.java b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/DefaultJobLoader.java index b79ab9c07..9c0d44dbf 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/DefaultJobLoader.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/DefaultJobLoader.java @@ -24,24 +24,31 @@ import java.util.concurrent.ConcurrentHashMap; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.batch.core.Job; +import org.springframework.batch.core.Step; import org.springframework.batch.core.configuration.DuplicateJobException; import org.springframework.batch.core.configuration.JobFactory; import org.springframework.batch.core.configuration.JobRegistry; +import org.springframework.batch.core.configuration.StepRegistry; import org.springframework.batch.core.launch.NoSuchJobException; +import org.springframework.batch.core.step.StepLocator; +import org.springframework.beans.factory.InitializingBean; +import org.springframework.context.ApplicationContext; import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.util.Assert; /** * Default implementation of {@link JobLoader}. Uses a {@link JobRegistry} to * manage a population of loaded jobs and clears them up when asked. - * + * * @author Dave Syer - * + * */ -public class DefaultJobLoader implements JobLoader { +public class DefaultJobLoader implements JobLoader, InitializingBean { private static Log logger = LogFactory.getLog(DefaultJobLoader.class); private JobRegistry jobRegistry; + private StepRegistry stepRegistry; private Map contexts = new ConcurrentHashMap(); @@ -51,30 +58,57 @@ public class DefaultJobLoader implements JobLoader { * Default constructor useful for declarative configuration. */ public DefaultJobLoader() { - this(null); + this(null, null); } + /** + * Creates a job loader with the job registry provided. + *

+ * If the specified {@link JobRegistry} is also a {@link StepRegistry} it + * is registered as the step registry to use for this instance. + * + * @param jobRegistry a {@link JobRegistry} + */ + public DefaultJobLoader(JobRegistry jobRegistry) { + this(jobRegistry, jobRegistry instanceof StepRegistry ? (StepRegistry) jobRegistry : null); + } + /** - * Create a job loader with the job registry provided. + * Creates a job loader with the job and step registries provided. + * * @param jobRegistry a {@link JobRegistry} + * @param stepRegistry a {@link StepRegistry} */ - public DefaultJobLoader(JobRegistry jobRegistry) { + public DefaultJobLoader(JobRegistry jobRegistry, StepRegistry stepRegistry) { this.jobRegistry = jobRegistry; + this.stepRegistry = stepRegistry; } /** * The {@link JobRegistry} to use for jobs created. - * - * @param jobRegistry + * + * @param jobRegistry the job registry */ public void setJobRegistry(JobRegistry jobRegistry) { this.jobRegistry = jobRegistry; + if (stepRegistry == null && jobRegistry instanceof StepRegistry) { + setStepRegistry((StepRegistry) jobRegistry); + } } + /** + * The {@link StepRegistry} to use for the steps of created jobs. + * + * @param stepRegistry the step registry + */ + public void setStepRegistry(StepRegistry stepRegistry) { + this.stepRegistry = stepRegistry; + } + /** * Unregister all the jobs and close all the contexts created by this * loader. - * + * * @see JobLoader#clear() */ public void clear() { @@ -85,6 +119,7 @@ public class DefaultJobLoader implements JobLoader { } for (String jobName : jobRegistry.getJobNames()) { jobRegistry.unregister(jobName); + stepRegistry.unregisterStepsFromJob(jobName); } contexts.clear(); } @@ -97,6 +132,7 @@ public class DefaultJobLoader implements JobLoader { for (String name : contextToJobNames.get(context)) { logger.debug("Unregistering job: " + name + " from context: " + context.getDisplayName()); jobRegistry.unregister(name); + stepRegistry.unregisterStepsFromJob(name); } context.close(); } @@ -105,7 +141,7 @@ public class DefaultJobLoader implements JobLoader { return doLoad(factory, true); } catch (DuplicateJobException e) { - throw new IllegalStateException("Found duplicte job in reload (it should have been unregistered " + throw new IllegalStateException("Found duplicate job in reload (it should have been unregistered " + "if it was previously registered in this loader)", e); } } @@ -151,6 +187,7 @@ public class DefaultJobLoader implements JobLoader { JobFactory jobFactory = new ReferenceJobFactory(job); jobRegistry.register(jobFactory); jobsRegistered.add(jobName); + stepRegistry.register(job.getName(), getSteps(job, context)); } @@ -174,4 +211,43 @@ public class DefaultJobLoader implements JobLoader { } + /** + * Returns all the {@link Step} instances defined by the specified {@link Job}. + * + * @param job the given job + * @param jobApplicationContext the application context of the job + * @return all the {@link Step} defined in the given job + * @see StepLocator + */ + private Collection getSteps(final Job job, final ApplicationContext jobApplicationContext) { + // TODO: that sounds like we need a stronger contract here + if (!(job instanceof StepLocator)) { + throw new UnsupportedOperationException("Cannot locate step from a Job that is not a StepLocator: job=" + + job.getName() + " does not implement StepLocator"); + } + final StepLocator stepLocator = (StepLocator) job; + final Collection stepNames = stepLocator.getStepNames(); + final Collection result = new ArrayList(); + for (String stepName : stepNames) { + result.add(stepLocator.getStep(stepName)); + } + + // Because some steps are referenced by name, we need to look in the context to see if there + // are more Step instances defined. Right now they are registered as being available in the + // context of the job but we have no idea if they are linked to that Job or not. + @SuppressWarnings("unchecked") + final Map allSteps = jobApplicationContext.getBeansOfType(Step.class); + for (Map.Entry entry : allSteps.entrySet()) { + if (!stepNames.contains(entry.getKey())) { + result.add(entry.getValue()); + } + } + return result; + } + + public void afterPropertiesSet() { + Assert.notNull(jobRegistry, "Job registry could not be null."); + Assert.notNull(stepRegistry, "Step registry could not be null. Should be set if the Job registry " + + "implementation does not implement StepRegistry."); + } } diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/MapJobRegistry.java b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/MapJobRegistry.java index 12d61f573..14e2271ea 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/MapJobRegistry.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/MapJobRegistry.java @@ -15,64 +15,78 @@ */ package org.springframework.batch.core.configuration.support; -import java.util.Collections; -import java.util.Set; - -import java.util.concurrent.ConcurrentMap; -import java.util.concurrent.ConcurrentHashMap; - import org.springframework.batch.core.Job; +import org.springframework.batch.core.Step; import org.springframework.batch.core.configuration.DuplicateJobException; import org.springframework.batch.core.configuration.JobFactory; import org.springframework.batch.core.configuration.JobRegistry; +import org.springframework.batch.core.configuration.StepRegistry; import org.springframework.batch.core.launch.NoSuchJobException; import org.springframework.util.Assert; +import java.util.Collection; +import java.util.Collections; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; + /** - * Simple, thread-safe, map-based implementation of {@link JobRegistry}. - * + * Simple, thread-safe, map-based implementation of {@link JobRegistry}. + * * @author Dave Syer * @author Robert Fischer - * */ -public class MapJobRegistry implements JobRegistry { +public class MapJobRegistry implements JobRegistry, StepRegistry { - /** - * The map holding the registered job factories. - */ - // The "final" ensures that it is visible and initialized when the constructor resolves. - private final ConcurrentMap map = new ConcurrentHashMap(); + /** + * The map holding the registered job factories. + */ + // The "final" ensures that it is visible and initialized when the constructor resolves. + private final ConcurrentMap map = new ConcurrentHashMap(); + private final MapStepRegistry stepRegistry = new MapStepRegistry(); - public void register(JobFactory jobFactory) throws DuplicateJobException { - Assert.notNull(jobFactory); - String name = jobFactory.getJobName(); - Assert.notNull(name, "Job configuration must have a name."); - JobFactory previousValue = map.putIfAbsent(name, jobFactory); - if(previousValue != null) { - throw new DuplicateJobException("A job configuration with this name [" + name - + "] was already registered"); - } - } + public void register(JobFactory jobFactory) throws DuplicateJobException { + Assert.notNull(jobFactory); + String name = jobFactory.getJobName(); + Assert.notNull(name, "Job configuration must have a name."); + JobFactory previousValue = map.putIfAbsent(name, jobFactory); + if (previousValue != null) { + throw new DuplicateJobException("A job configuration with this name [" + name + + "] was already registered"); + } + } - public void unregister(String name) { - Assert.notNull(name, "Job configuration must have a name."); - map.remove(name); - } + public void unregister(String name) { + Assert.notNull(name, "Job configuration must have a name."); + map.remove(name); + } - public Job getJob(String name) throws NoSuchJobException { - JobFactory factory = map.get(name); - if(factory == null) { - throw new NoSuchJobException("No job configuration with the name [" + name + "] was registered"); - } else { - return factory.createJob(); - } - } + public Job getJob(String name) throws NoSuchJobException { + JobFactory factory = map.get(name); + if (factory == null) { + throw new NoSuchJobException("No job configuration with the name [" + name + "] was registered"); + } else { + return factory.createJob(); + } + } - /** - * Provides an unmodifiable view of the job names. - */ - public Set getJobNames() { - return Collections.unmodifiableSet(map.keySet()); - } + /** + * Provides an unmodifiable view of the job names. + */ + public Set getJobNames() { + return Collections.unmodifiableSet(map.keySet()); + } + + public void register(String jobName, Collection steps) { + stepRegistry.register(jobName, steps); + } + + public void unregisterStepsFromJob(String jobName) { + stepRegistry.unregisterStepsFromJob(jobName); + } + + public Step getStep(String jobName, String stepName) throws NoSuchJobException { + return stepRegistry.getStep(jobName, stepName); + } } diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/MapStepRegistry.java b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/MapStepRegistry.java new file mode 100644 index 000000000..a1cea6c93 --- /dev/null +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/MapStepRegistry.java @@ -0,0 +1,66 @@ +package org.springframework.batch.core.configuration.support; + +import org.springframework.batch.core.Step; +import org.springframework.batch.core.configuration.StepRegistry; +import org.springframework.batch.core.launch.NoSuchJobException; +import org.springframework.batch.core.step.NoSuchStepException; +import org.springframework.util.Assert; + +import java.util.Collection; +import java.util.HashMap; +import java.util.Map; + +/** + * Simple map-based implementation of {@link StepRegistry}. Access to the map is + * synchronized, guarded by an internal lock. + * + * @author Sebastien Gerard + * @author Stephane Nicoll + */ +public class MapStepRegistry implements StepRegistry { + + private final Map> map = new HashMap>(); + + public void register(String jobName, Collection steps) { + Assert.notNull(jobName, "The job name cannot be null."); + Assert.notNull(steps, "The job steps cannot be null."); + + unregisterStepsFromJob(jobName); + + synchronized (this.map) { + final Map jobSteps = new HashMap(); + for (Step step : steps) { + jobSteps.put(step.getName(), step); + } + + this.map.put(jobName, jobSteps); + } + } + + public void unregisterStepsFromJob(String jobName) { + Assert.notNull(jobName, "Job configuration must have a name."); + synchronized (map) { + map.remove(jobName); + } + } + + public Step getStep(String jobName, String stepName) throws NoSuchJobException { + Assert.notNull(jobName, "The job name cannot be null."); + Assert.notNull(stepName, "The step name cannot be null."); + + synchronized (map) { + if (!map.containsKey(jobName)) { + throw new NoSuchJobException("No job configuration with the name [" + jobName + "] was registered"); + } else { + final Map jobSteps = map.get(jobName); + if (jobSteps.containsKey(stepName)) { + return jobSteps.get(stepName); + } else { + throw new NoSuchStepException("The step called [" + stepName + "] does not exist in the job [" + + jobName + "]"); + } + } + } + } + +} diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/support/DefaultJobLoaderTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/support/DefaultJobLoaderTests.java index f898d52a9..3e2170223 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/support/DefaultJobLoaderTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/support/DefaultJobLoaderTests.java @@ -16,82 +16,234 @@ package org.springframework.batch.core.configuration.support; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +import java.util.Collection; +import java.util.Collections; import org.junit.Test; import org.springframework.batch.core.Job; import org.springframework.batch.core.JobExecution; import org.springframework.batch.core.JobParametersIncrementer; import org.springframework.batch.core.JobParametersValidator; +import org.springframework.batch.core.Step; +import org.springframework.batch.core.configuration.DuplicateJobException; +import org.springframework.batch.core.configuration.JobFactory; import org.springframework.batch.core.configuration.JobRegistry; +import org.springframework.batch.core.configuration.StepRegistry; +import org.springframework.batch.core.launch.NoSuchJobException; +import org.springframework.batch.core.step.NoSuchStepException; +import org.springframework.batch.core.step.StepLocator; import org.springframework.core.io.ByteArrayResource; import org.springframework.core.io.ClassPathResource; /** * @author Dave Syer - * + * @author Stephane Nicoll */ public class DefaultJobLoaderTests { - private JobRegistry registry = new MapJobRegistry(); + /** + * The name of the job as defined in the test context used in this test. + */ + private static final String TEST_JOB_NAME = "test-job"; - private DefaultJobLoader jobLoader = new DefaultJobLoader(registry); + /** + * The name of the step as defined in the test context used in this test. + */ + private static final String TEST_STEP_NAME = "test-step"; + + private JobRegistry jobRegistry = new MapJobRegistry(); + private StepRegistry stepRegistry = new MapStepRegistry(); + + private DefaultJobLoader jobLoader = new DefaultJobLoader(jobRegistry, stepRegistry); @Test public void testLoadWithExplicitName() throws Exception { GenericApplicationContextFactory factory = new GenericApplicationContextFactory(new ByteArrayResource( JOB_XML.getBytes())); jobLoader.load(factory); - assertEquals(1, registry.getJobNames().size()); + assertEquals(1, jobRegistry.getJobNames().size()); jobLoader.reload(factory); - assertEquals(1, registry.getJobNames().size()); + assertEquals(1, jobRegistry.getJobNames().size()); } - @Test - public void testReload() throws Exception { + @Test + public void createWithBothRegistries() { + final DefaultJobLoader loader = new DefaultJobLoader(); + loader.setJobRegistry(jobRegistry); + loader.setStepRegistry(stepRegistry); + + loader.afterPropertiesSet(); + } + + @Test + public void createWithJobRegistryWhichIsAStepRegistry() { + final DefaultJobLoader loader = new DefaultJobLoader(); + loader.setJobRegistry(jobRegistry); + + loader.afterPropertiesSet(); + } + + @Test + public void createWithSimpleJobRegistry() { + final DefaultJobLoader loader = new DefaultJobLoader(); + loader.setJobRegistry(new JobRegistryMock()); + + try { + loader.afterPropertiesSet(); + fail("Should have failed to create job loader without a step registry (" + + "and the job registry could not fulfill that role)"); + } catch (IllegalArgumentException e) { + // OK + } + } + + @Test + public void testRegistryUpdated() throws DuplicateJobException { + GenericApplicationContextFactory factory = new GenericApplicationContextFactory( + new ClassPathResource("trivial-context.xml", getClass())); + jobLoader.load(factory); + assertEquals(1, jobRegistry.getJobNames().size()); + assertStepExist(TEST_JOB_NAME, TEST_STEP_NAME); + } + + @Test + public void testMultipleJobsInTheSameContext() throws DuplicateJobException { + GenericApplicationContextFactory factory = new GenericApplicationContextFactory( + new ClassPathResource("job-context-with-steps.xml", getClass())); + jobLoader.load(factory); + assertEquals(2, jobRegistry.getJobNames().size()); + assertStepExist("job1", "step11", "step12"); + assertStepDoNotExist("job1", "step21", "step22"); + assertStepExist("job2", "step21", "step22"); + assertStepDoNotExist("job2", "step11", "step12"); + } + + @Test + public void testMultipleJobsInTheSameContextWithSeparateSteps() throws DuplicateJobException { + GenericApplicationContextFactory factory = new GenericApplicationContextFactory( + new ClassPathResource("job-context-with-separate-steps.xml", getClass())); + jobLoader.load(factory); + assertEquals(2, jobRegistry.getJobNames().size()); + assertStepExist("job1", "step11", "step12", "genericStep1", "genericStep2"); + assertStepDoNotExist("job1", "step21", "step22"); + assertStepExist("job2", "step21", "step22", "genericStep1", "genericStep2"); + assertStepDoNotExist("job2", "step11", "step12"); + } + + @Test + public void testReload() throws Exception { GenericApplicationContextFactory factory = new GenericApplicationContextFactory(new ClassPathResource( "trivial-context.xml", getClass())); - jobLoader.load(factory); - assertEquals(1, registry.getJobNames().size()); - jobLoader.reload(factory); - assertEquals(1, registry.getJobNames().size()); - } + jobLoader.load(factory); + assertEquals(1, jobRegistry.getJobNames().size()); + assertStepExist(TEST_JOB_NAME, TEST_STEP_NAME); + jobLoader.reload(factory); + assertEquals(1, jobRegistry.getJobNames().size()); + assertStepExist(TEST_JOB_NAME, TEST_STEP_NAME); + } - @Test - public void testReloadWithAutoRegister() throws Exception { + @Test + public void testReloadWithAutoRegister() throws Exception { GenericApplicationContextFactory factory = new GenericApplicationContextFactory(new ClassPathResource( "trivial-context-autoregister.xml", getClass())); - jobLoader.load(factory); - assertEquals(1, registry.getJobNames().size()); - jobLoader.reload(factory); - assertEquals(1, registry.getJobNames().size()); - } + jobLoader.load(factory); + assertEquals(1, jobRegistry.getJobNames().size()); + assertStepExist(TEST_JOB_NAME, TEST_STEP_NAME); + jobLoader.reload(factory); + assertEquals(1, jobRegistry.getJobNames().size()); + assertStepExist(TEST_JOB_NAME, TEST_STEP_NAME); + } - private static final String JOB_XML = String - .format("", - DefaultJobLoaderTests.class.getName()); + protected void assertStepExist(String jobName, String... stepNames) { + for (String stepName : stepNames) { + try { + stepRegistry.getStep(jobName, stepName); + } catch (NoSuchJobException e) { + fail("Job with name [" + jobName + "] should have been found."); + } catch (NoSuchStepException e) { + fail("Step with name [" + stepName + "] for job [" + jobName + "] should have been found."); + } + } + } - public static class StubJob implements Job { + protected void assertStepDoNotExist(String jobName, String... stepNames) { + for (String stepName : stepNames) { + try { + final Step step = stepRegistry.getStep(jobName, stepName); + fail("Step with name [" + stepName + "] for job [" + jobName + "] should " + + "not have been found but got [" + step + "]"); + } catch (NoSuchJobException e) { + fail("Job with name [" + jobName + "] should have been found."); + } catch (NoSuchStepException e) { + // OK + } + } + } + private static final String JOB_XML = String + .format( + "", + DefaultJobLoaderTests.class.getName()); + + public static class StubJob implements Job, StepLocator { + + @Override public void execute(JobExecution execution) { - } + } + @Override public JobParametersIncrementer getJobParametersIncrementer() { - return null; - } + return null; + } + @Override public String getName() { - return "job"; - } + return "job"; + } + @Override public boolean isRestartable() { - return false; - } + return false; + } + @Override public JobParametersValidator getJobParametersValidator() { - return null; - } + return null; + } - } + @Override + public Collection getStepNames() { + return Collections.emptyList(); + } + @Override + public Step getStep(String stepName) throws NoSuchStepException { + throw new NoSuchStepException("Step [" + stepName + "] does not exist"); + } + } + + private static class JobRegistryMock implements JobRegistry { + @Override + public void register(JobFactory jobFactory) throws DuplicateJobException { + // dummy + } + + @Override + public void unregister(String jobName) { + // dummy + } + + @Override + public Collection getJobNames() { + return Collections.emptyList(); + } + + @Override + public Job getJob(String name) throws NoSuchJobException { + throw new NoSuchJobException("Mock implementation does not hold any job."); + } + } } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/support/MapStepRegistryTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/support/MapStepRegistryTests.java new file mode 100644 index 000000000..146143986 --- /dev/null +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/support/MapStepRegistryTests.java @@ -0,0 +1,234 @@ +package org.springframework.batch.core.configuration.support; + +import org.junit.Assert; +import org.junit.Test; +import org.springframework.batch.core.Step; +import org.springframework.batch.core.configuration.StepRegistry; +import org.springframework.batch.core.launch.NoSuchJobException; +import org.springframework.batch.core.step.NoSuchStepException; +import org.springframework.batch.core.step.tasklet.TaskletStep; + +import java.util.Arrays; +import java.util.Collection; +import java.util.HashSet; + +/** + * @author Sebastien Gerard + */ +public class MapStepRegistryTests { + + private static final String EXCEPTION_NOT_THROWN_MSG = "An exception should have been thrown"; + + @Test + public void registerStepEmptyCollection() { + final StepRegistry stepRegistry = createRegistry(); + + launchRegisterGetRegistered(stepRegistry, "myJob", getStepCollection()); + } + + @Test + public void registerStepNullJobName() { + final StepRegistry stepRegistry = createRegistry(); + + try { + stepRegistry.register(null, new HashSet()); + Assert.fail(EXCEPTION_NOT_THROWN_MSG); + } catch (IllegalArgumentException e) { + } + } + + @Test + public void registerStepNullSteps() { + final StepRegistry stepRegistry = createRegistry(); + + try { + stepRegistry.register("fdsfsd", null); + Assert.fail(EXCEPTION_NOT_THROWN_MSG); + } catch (IllegalArgumentException e) { + } + } + + @Test + public void registerStepGetStep() { + final StepRegistry stepRegistry = createRegistry(); + + launchRegisterGetRegistered(stepRegistry, "myJob", + getStepCollection( + createStep("myStep"), + createStep("myOtherStep"), + createStep("myThirdStep") + )); + } + + @Test + public void getJobNotRegistered() { + final StepRegistry stepRegistry = createRegistry(); + + final String aStepName = "myStep"; + launchRegisterGetRegistered(stepRegistry, "myJob", + getStepCollection( + createStep(aStepName), + createStep("myOtherStep"), + createStep("myThirdStep") + )); + + assertJobNotRegistered(stepRegistry, "a ghost"); + } + + @Test + public void getJobNotRegisteredNoRegistration() { + final StepRegistry stepRegistry = createRegistry(); + + assertJobNotRegistered(stepRegistry, "a ghost"); + } + + @Test + public void getStepNotRegistered() { + final StepRegistry stepRegistry = createRegistry(); + + final String jobName = "myJob"; + launchRegisterGetRegistered(stepRegistry, jobName, + getStepCollection( + createStep("myStep"), + createStep("myOtherStep"), + createStep("myThirdStep") + )); + + assertStepNameNotRegistered(stepRegistry, jobName, "fsdfsdfsdfsd"); + } + + @Test + public void registerRegisterAgainAndGet() { + final StepRegistry stepRegistry = createRegistry(); + + final String jobName = "myJob"; + final Collection stepsFirstRegistration = getStepCollection( + createStep("myStep"), + createStep("myOtherStep"), + createStep("myThirdStep") + ); + + // first registration + launchRegisterGetRegistered(stepRegistry, jobName, stepsFirstRegistration); + + // register again the job + launchRegisterGetRegistered(stepRegistry, jobName, + getStepCollection( + createStep("myFourthStep"), + createStep("lastOne") + )); + + assertStepsNotRegistered(stepRegistry, jobName, stepsFirstRegistration); + } + + @Test + public void getStepNullJobName() throws NoSuchJobException { + final StepRegistry stepRegistry = createRegistry(); + + try { + stepRegistry.getStep(null, "a step"); + Assert.fail(EXCEPTION_NOT_THROWN_MSG); + } catch (IllegalArgumentException e) { + } + } + + @Test + public void getStepNullStepName() throws NoSuchJobException { + final StepRegistry stepRegistry = createRegistry(); + + final String stepName = "myStep"; + launchRegisterGetRegistered(stepRegistry, "myJob", getStepCollection(createStep(stepName))); + + try { + stepRegistry.getStep(null, stepName); + Assert.fail(EXCEPTION_NOT_THROWN_MSG); + } catch (IllegalArgumentException e) { + } + } + + @Test + public void registerStepUnregisterJob() { + final StepRegistry stepRegistry = createRegistry(); + + final Collection steps = getStepCollection( + createStep("myStep"), + createStep("myOtherStep"), + createStep("myThirdStep") + ); + + final String jobName = "myJob"; + launchRegisterGetRegistered(stepRegistry, jobName, steps); + + stepRegistry.unregisterStepsFromJob(jobName); + assertJobNotRegistered(stepRegistry, jobName); + } + + @Test + public void unregisterJobNameNull() { + final StepRegistry stepRegistry = createRegistry(); + + try { + stepRegistry.unregisterStepsFromJob(null); + Assert.fail(EXCEPTION_NOT_THROWN_MSG); + } catch (IllegalArgumentException e) { + } + } + + @Test + public void unregisterNoRegistration() { + final StepRegistry stepRegistry = createRegistry(); + + assertJobNotRegistered(stepRegistry, "a job"); + } + + protected StepRegistry createRegistry() { + return new MapStepRegistry(); + } + + protected Step createStep(String stepName) { + return new TaskletStep(stepName); + } + + protected Collection getStepCollection(Step... steps) { + return Arrays.asList(steps); + } + + protected void launchRegisterGetRegistered(StepRegistry stepRegistry, String jobName, Collection steps) { + stepRegistry.register(jobName, steps); + assertStepsRegistered(stepRegistry, jobName, steps); + } + + protected void assertJobNotRegistered(StepRegistry stepRegistry, String jobName) { + try { + stepRegistry.getStep(jobName, "a step"); + Assert.fail(EXCEPTION_NOT_THROWN_MSG); + } catch (NoSuchJobException e) { + } + } + + protected void assertStepsRegistered(StepRegistry stepRegistry, String jobName, Collection steps) { + for (Step step : steps) { + try { + stepRegistry.getStep(jobName, step.getName()); + } catch (NoSuchJobException e) { + Assert.fail("Unexpected exception " + e); + } + } + } + + protected void assertStepsNotRegistered(StepRegistry stepRegistry, String jobName, Collection steps) { + for (Step step : steps) { + assertStepNameNotRegistered(stepRegistry, jobName, step.getName()); + } + } + + protected void assertStepNameNotRegistered(StepRegistry stepRegistry, String jobName, String stepName) { + try { + stepRegistry.getStep(jobName, stepName); + Assert.fail(EXCEPTION_NOT_THROWN_MSG); + } catch (NoSuchJobException e) { + Assert.fail("Unexpected exception"); + } catch (NoSuchStepException e) { + } + } +} \ No newline at end of file diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/job/JobSupport.java b/spring-batch-core/src/test/java/org/springframework/batch/core/job/JobSupport.java index a55bdde09..9ece5b763 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/job/JobSupport.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/job/JobSupport.java @@ -16,8 +16,10 @@ package org.springframework.batch.core.job; -import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; import java.util.List; +import java.util.Map; import org.springframework.batch.core.Job; import org.springframework.batch.core.JobExecution; @@ -25,6 +27,8 @@ import org.springframework.batch.core.JobParametersIncrementer; import org.springframework.batch.core.JobParametersValidator; import org.springframework.batch.core.Step; import org.springframework.batch.core.UnexpectedJobExecutionException; +import org.springframework.batch.core.step.NoSuchStepException; +import org.springframework.batch.core.step.StepLocator; import org.springframework.beans.factory.BeanNameAware; import org.springframework.util.ClassUtils; @@ -37,9 +41,9 @@ import org.springframework.util.ClassUtils; * @author Lucas Ward * @author Dave Syer */ -public class JobSupport implements BeanNameAware, Job { +public class JobSupport implements BeanNameAware, Job, StepLocator { - private List steps = new ArrayList(); + private Map steps = new HashMap(); private String name; @@ -110,11 +114,13 @@ public class JobSupport implements BeanNameAware, Job { public void setSteps(List steps) { this.steps.clear(); - this.steps.addAll(steps); + for (Step step : steps) { + this.steps.put(step.getName(), step); + } } public void addStep(Step step) { - this.steps.add(step); + this.steps.put(step.getName(), step); } /* @@ -172,4 +178,15 @@ public class JobSupport implements BeanNameAware, Job { return jobParametersValidator; } + public Collection getStepNames() { + return steps.keySet(); + } + + public Step getStep(String stepName) throws NoSuchStepException { + final Step step = steps.get(stepName); + if (step == null) { + throw new NoSuchStepException("Step ["+stepName+"] does not exist for job with name ["+getName()+"]"); + } + return step; + } } diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/support/job-context-with-separate-steps.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/support/job-context-with-separate-steps.xml new file mode 100644 index 000000000..c8f9292c5 --- /dev/null +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/support/job-context-with-separate-steps.xml @@ -0,0 +1,17 @@ + + + + + Declares two jobs with a set of steps. Also declares two steps that are not attached to any job + but could be linked to them through a partition handler configuration for instance. + + + + + + + + diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/support/job-context-with-steps.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/support/job-context-with-steps.xml new file mode 100644 index 000000000..bb3bc8ba1 --- /dev/null +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/support/job-context-with-steps.xml @@ -0,0 +1,27 @@ + + + + + + + + + + + + + + + + + + + + + + diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/support/trivial-context-autoregister.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/support/trivial-context-autoregister.xml index eef60a7a4..98505c6c1 100644 --- a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/support/trivial-context-autoregister.xml +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/support/trivial-context-autoregister.xml @@ -1,12 +1,15 @@ - + - + + + + + diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/support/trivial-context.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/support/trivial-context.xml index cad2a6ed0..17f09758d 100644 --- a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/support/trivial-context.xml +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/support/trivial-context.xml @@ -1,15 +1,16 @@ + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" + xsi:schemaLocation="http://www.springframework.org/schema/beans + http://www.springframework.org/schema/beans/spring-beans-2.0.xsd"> + + + + + + + - -