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 d5d52f0df..6acefcfc6 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 @@ -38,10 +38,12 @@ 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. + * manage a population of loaded jobs and clears them up when asked. An optional + * {@link StepRegistry} might also be set to register the step(s) available for + * each registered job. * * @author Dave Syer - * + * @author Stephane Nicoll */ public class DefaultJobLoader implements JobLoader, InitializingBean { @@ -63,14 +65,11 @@ public class DefaultJobLoader implements JobLoader, InitializingBean { /** * 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); + this(jobRegistry, null); } /** @@ -91,9 +90,6 @@ public class DefaultJobLoader implements JobLoader, InitializingBean { */ public void setJobRegistry(JobRegistry jobRegistry) { this.jobRegistry = jobRegistry; - if (stepRegistry == null && jobRegistry instanceof StepRegistry) { - setStepRegistry((StepRegistry) jobRegistry); - } } /** @@ -118,8 +114,7 @@ public class DefaultJobLoader implements JobLoader, InitializingBean { } } for (String jobName : jobRegistry.getJobNames()) { - jobRegistry.unregister(jobName); - stepRegistry.unregisterStepsFromJob(jobName); + doUnregister(jobName); } contexts.clear(); } @@ -131,8 +126,7 @@ public class DefaultJobLoader implements JobLoader, InitializingBean { ConfigurableApplicationContext context = contexts.get(factory); for (String name : contextToJobNames.get(context)) { logger.debug("Unregistering job: " + name + " from context: " + context.getDisplayName()); - jobRegistry.unregister(name); - stepRegistry.unregisterStepsFromJob(name); + doUnregister(name); } context.close(); } @@ -180,16 +174,12 @@ public class DefaultJobLoader implements JobLoader, InitializingBean { // On reload try to unregister first if (unregister) { logger.debug("Unregistering job: " + jobName + " from context: " + context.getDisplayName()); - jobRegistry.unregister(jobName); - stepRegistry.unregisterStepsFromJob(jobName); + doUnregister(jobName); } logger.debug("Registering job: " + jobName + " from context: " + context.getDisplayName()); - JobFactory jobFactory = new ReferenceJobFactory(job); - jobRegistry.register(jobFactory); + doRegister(context, job); jobsRegistered.add(jobName); - stepRegistry.register(job.getName(), getSteps(job, context)); - } } @@ -213,20 +203,17 @@ public class DefaultJobLoader implements JobLoader, InitializingBean { } /** - * Returns all the {@link Step} instances defined by the specified {@link Job}. + * Returns all the {@link Step} instances defined by the specified {@link StepLocator}. + *

+ * The specified jobApplicationContext is used to collect additional steps that + * are not exposed by the step locator * - * @param job the given job + * @param stepLocator the given step locator * @param jobApplicationContext the application context of the job - * @return all the {@link Step} defined in the given job + * @return all the {@link Step} defined by the given step locator and context * @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; + private Collection getSteps(final StepLocator stepLocator, final ApplicationContext jobApplicationContext) { final Collection stepNames = stepLocator.getStepNames(); final Collection result = new ArrayList(); for (String stepName : stepNames) { @@ -246,9 +233,42 @@ public class DefaultJobLoader implements JobLoader, InitializingBean { return result; } + /** + * Registers the specified {@link Job} defined in the specified {@link ConfigurableApplicationContext}. + *

+ * Makes sure to update the {@link StepRegistry} if it is available. + * + * @param context the context in which the job is defined + * @param job the job to register + * @throws DuplicateJobException if that job is already registered + */ + private void doRegister(ConfigurableApplicationContext context, Job job) throws DuplicateJobException { + final JobFactory jobFactory = new ReferenceJobFactory(job); + jobRegistry.register(jobFactory); + + if (stepRegistry != null) { + if (!(job instanceof StepLocator)) { + throw new UnsupportedOperationException("Cannot locate steps from a Job that is not a StepLocator: job=" + + job.getName() + " does not implement StepLocator"); + } + stepRegistry.register(job.getName(), getSteps((StepLocator) job, context)); + } + } + + /** + * Unregisters the job identified by the specified jobName. + * + * @param jobName the name of the job to unregister + */ + private void doUnregister(String jobName) { + jobRegistry.unregister(jobName); + if (stepRegistry != null) { + stepRegistry.unregisterStepsFromJob(jobName); + } + + } + 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 b4cc29e9c..0787440d0 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 @@ -16,15 +16,12 @@ package org.springframework.batch.core.configuration.support; 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; @@ -36,14 +33,13 @@ import java.util.concurrent.ConcurrentMap; * @author Dave Syer * @author Robert Fischer */ -public class MapJobRegistry implements JobRegistry, StepRegistry { +public class MapJobRegistry implements JobRegistry { /** * 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); @@ -77,16 +73,4 @@ public class MapJobRegistry implements JobRegistry, StepRegistry { return Collections.unmodifiableSet(map.keySet()); } - public void register(String jobName, Collection steps) throws DuplicateJobException { - 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/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 3e2170223..d33fcbf3f 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 @@ -78,7 +78,7 @@ public class DefaultJobLoaderTests { } @Test - public void createWithJobRegistryWhichIsAStepRegistry() { + public void createWithOnlyJobRegistry() { final DefaultJobLoader loader = new DefaultJobLoader(); loader.setJobRegistry(jobRegistry); @@ -132,6 +132,43 @@ public class DefaultJobLoaderTests { assertStepDoNotExist("job2", "step11", "step12"); } + @Test + public void testNoStepRegistryAvailable() throws DuplicateJobException { + final JobLoader loader = new DefaultJobLoader(jobRegistry); + ClassPathXmlApplicationContextFactory factory = new ClassPathXmlApplicationContextFactory( + new ClassPathResource("job-context-with-steps.xml", getClass())); + loader.load(factory); + // No step registry available so just registering the jobs + assertEquals(2, jobRegistry.getJobNames().size()); + } + + @Test + public void testLoadWithJobThatIsNotAStepLocator() throws DuplicateJobException { + ClassPathXmlApplicationContextFactory factory = new ClassPathXmlApplicationContextFactory( + new ByteArrayResource(BASIC_JOB_XML.getBytes())); + try { + jobLoader.load(factory); + fail("Should have failed with a ["+UnsupportedOperationException.class.getName()+"] as job does not" + + "implement StepLocator."); + } catch (UnsupportedOperationException e) { + // Job is not a step locator, can't register steps + } + + } + + @Test + public void testLoadWithJobThatIsNotAStepLocatorNoStepRegistry() throws DuplicateJobException { + final JobLoader loader = new DefaultJobLoader(jobRegistry); + ClassPathXmlApplicationContextFactory factory = new ClassPathXmlApplicationContextFactory( + new ByteArrayResource(BASIC_JOB_XML.getBytes())); + try { + loader.load(factory); + } catch (UnsupportedOperationException e) { + fail("Should not have failed with a [" + UnsupportedOperationException.class.getName() + "] as " + + "stepRegistry is not available for this JobLoader instance."); + } + } + @Test public void testReload() throws Exception { GenericApplicationContextFactory factory = new GenericApplicationContextFactory(new ClassPathResource( @@ -182,13 +219,19 @@ public class DefaultJobLoaderTests { } } + private static final String BASIC_JOB_XML = String + .format( + "", + DefaultJobLoaderTests.class.getName()); + private static final String JOB_XML = String .format( "", DefaultJobLoaderTests.class.getName()); - public static class StubJob implements Job, StepLocator { + public static class BasicStubJob implements Job { @Override public void execute(JobExecution execution) { @@ -213,6 +256,9 @@ public class DefaultJobLoaderTests { public JobParametersValidator getJobParametersValidator() { return null; } + } + + public static class StubJob extends BasicStubJob implements StepLocator { @Override public Collection getStepNames() { diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/support/MapJobRegistryTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/support/MapJobRegistryTests.java index ab0928868..8e99211b3 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/support/MapJobRegistryTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/support/MapJobRegistryTests.java @@ -21,8 +21,6 @@ import junit.framework.TestCase; import org.springframework.batch.core.configuration.DuplicateJobException; import org.springframework.batch.core.configuration.JobFactory; -import org.springframework.batch.core.configuration.support.MapJobRegistry; -import org.springframework.batch.core.configuration.support.ReferenceJobFactory; import org.springframework.batch.core.job.JobSupport; import org.springframework.batch.core.launch.NoSuchJobException;