From a08069a9c6758bea2dc3074693dcf80b1f4152a1 Mon Sep 17 00:00:00 2001 From: dsyer Date: Sun, 3 Jan 2010 10:38:15 +0000 Subject: [PATCH] BATCH-1474: Added reload() feature to JobLoader. --- ...ClassPathXmlApplicationContextFactory.java | 40 ++++++++++- .../support/DefaultJobLoader.java | 68 +++++++++++++++++-- .../core/configuration/support/JobLoader.java | 29 ++++++-- .../support/JobRegistryBeanPostProcessor.java | 9 ++- ...sgiBundleXmlApplicationContextFactory.java | 21 ++++++ ...PathXmlApplicationContextFactoryTests.java | 46 +++++++++---- .../support/DefaultJobLoaderTests.java | 54 +++++++++++++++ ...ndleXmlApplicationContextFactoryTests.java | 10 +++ .../support/trivial-context-autoregister.xml | 17 +++++ 9 files changed, 266 insertions(+), 28 deletions(-) create mode 100644 spring-batch-core/src/test/java/org/springframework/batch/core/configuration/support/DefaultJobLoaderTests.java create mode 100644 spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/support/trivial-context-autoregister.xml diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/ClassPathXmlApplicationContextFactory.java b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/ClassPathXmlApplicationContextFactory.java index d00ae9990..b3c6f2713 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/ClassPathXmlApplicationContextFactory.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/ClassPathXmlApplicationContextFactory.java @@ -53,14 +53,27 @@ public class ClassPathXmlApplicationContextFactory implements ApplicationContext private Resource resource; private ConfigurableApplicationContext parent; - + private boolean copyConfiguration = true; private Collection> beanFactoryPostProcessorClasses; private Collection> beanPostProcessorExcludeClasses; + /** + * Convenient constructor for configuration purposes. + */ public ClassPathXmlApplicationContextFactory() { + this(null); + } + + /** + * Create a factory instance with the resource specified. The resource is a + * Spring XML configuration file. + */ + public ClassPathXmlApplicationContextFactory(Resource resource) { + + this.resource = resource; beanFactoryPostProcessorClasses = new ArrayList>(); beanFactoryPostProcessorClasses.add(PropertyPlaceholderConfigurer.class); beanFactoryPostProcessorClasses.add(CustomEditorConfigurer.class); @@ -80,7 +93,8 @@ public class ClassPathXmlApplicationContextFactory implements ApplicationContext * {@link ApplicationContext}. Use imports to centralise the configuration * in one file. * - * @param resource the resource path to the xml to load for the child context. + * @param resource the resource path to the xml to load for the child + * context. */ public void setResource(Resource resource) { this.resource = resource; @@ -156,7 +170,7 @@ public class ClassPathXmlApplicationContextFactory implements ApplicationContext * @see org.springframework.context.ApplicationContextAware#setApplicationContext(org.springframework.context.ApplicationContext) */ public void setApplicationContext(ApplicationContext applicationContext) throws BeansException { - if (applicationContext==null) { + if (applicationContext == null) { return; } Assert.isInstanceOf(ConfigurableApplicationContext.class, applicationContext); @@ -270,4 +284,24 @@ public class ClassPathXmlApplicationContextFactory implements ApplicationContext } } } + + @Override + public String toString() { + return "ClassPathXmlApplicationContextFactory [resource=" + resource + "]"; + } + + @Override + public int hashCode() { + return toString().hashCode(); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + return toString().equals(obj.toString()); + } + } 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 223786788..fed3ef92f 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 @@ -18,6 +18,8 @@ package org.springframework.batch.core.configuration.support; import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -29,6 +31,9 @@ import org.springframework.batch.core.launch.NoSuchJobException; import org.springframework.context.ConfigurableApplicationContext; /** + * 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 * */ @@ -38,10 +43,12 @@ public class DefaultJobLoader implements JobLoader { private JobRegistry jobRegistry; - private Collection contexts = new HashSet(); - + private Map contexts = new ConcurrentHashMap(); + + private Map> contextToJobNames = new ConcurrentHashMap>(); + /** - * Default constructor useful for declarative configuration. + * Default constructor useful for declarative configuration. */ public DefaultJobLoader() { this(null); @@ -71,7 +78,7 @@ public class DefaultJobLoader implements JobLoader { * @see JobLoader#clear() */ public void clear() { - for (ConfigurableApplicationContext context : contexts) { + for (ConfigurableApplicationContext context : contexts.values()) { if (context.isActive()) { context.close(); } @@ -82,22 +89,68 @@ public class DefaultJobLoader implements JobLoader { contexts.clear(); } + public Collection reload(ApplicationContextFactory factory) { + + // If the same factory is loaded twice the context can be closed + if (contexts.containsKey(factory)) { + ConfigurableApplicationContext context = contexts.get(factory); + for (String name : contextToJobNames.get(context)) { + logger.debug("Unregistering job: " + name + " from context: " + context.getDisplayName()); + jobRegistry.unregister(name); + } + context.close(); + } + + try { + return doLoad(factory, true); + } + catch (DuplicateJobException e) { + throw new IllegalStateException("Found duplicte job in reload (it should have been unregistered " + + "if it was previously registered in this loader)", e); + } + } + public Collection load(ApplicationContextFactory factory) throws DuplicateJobException { + return doLoad(factory, false); + } + + private Collection doLoad(ApplicationContextFactory factory, boolean unregister) throws DuplicateJobException { Collection jobNamesBefore = jobRegistry.getJobNames(); ConfigurableApplicationContext context = factory.createApplicationContext(); + Collection jobNamesAfter = jobRegistry.getJobNames(); // Try to detect auto-registration (e.g. through a bean post processor) - boolean autoRegistrationDetected = jobRegistry.getJobNames().size() > jobNamesBefore.size(); + boolean autoRegistrationDetected = jobNamesAfter.size() > jobNamesBefore.size(); - contexts.add(context); + Collection jobsRegistered = new HashSet(); + if (autoRegistrationDetected) { + for (String name : jobNamesAfter) { + if (!jobNamesBefore.contains(name)) { + jobsRegistered.add(name); + } + } + } + + contexts.put(factory, context); String[] names = context.getBeanNamesForType(Job.class); + Collection result = new ArrayList(); for (String name : names) { + if (!autoRegistrationDetected) { + + // On reload try to unregister first + if (unregister) { + logger.debug("Unregistering job: " + name + " from context: " + context.getDisplayName()); + jobRegistry.unregister(name); + } + logger.debug("Registering job: " + name + " from context: " + context.getDisplayName()); JobFactory jobFactory = new ReferenceJobFactory((Job) context.getBean(name)); jobRegistry.register(jobFactory); + jobsRegistered.add(name); + } try { result.add(jobRegistry.getJob(name)); @@ -106,8 +159,11 @@ public class DefaultJobLoader implements JobLoader { // should not happen; throw new IllegalStateException("Could not retrieve job that was should have been registered", e); } + } + contextToJobNames.put(context, jobsRegistered); + return result; } diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/JobLoader.java b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/JobLoader.java index b856ff0b4..be7b270c3 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/JobLoader.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/JobLoader.java @@ -20,25 +20,42 @@ import java.util.Collection; import org.springframework.batch.core.Job; import org.springframework.batch.core.configuration.DuplicateJobException; - /** * @author Dave Syer * * @since 2.1 */ public interface JobLoader { - + /** + * Load an application context and register all the jobs. + * * @param factory a factory for an application context (containing jobs) * @return a collection of the jobs created * - * @throws DuplicateJobException if a job with the same name was already registered + * @throws DuplicateJobException if a job with the same name was already + * registered */ Collection load(ApplicationContextFactory factory) throws DuplicateJobException; - + /** - * Unregister all the jobs and close all the contexts created by this loader. + * Load an application context and register all the jobs, having first + * unregistered them if already registered. Implementations should also take + * care to close and clean up the application context previously created if + * possible (either from this factory or from one with the same jobs). + * + * @param factory a factory for an application context (containing jobs) + * @return a collection of the jobs created + * + * @throws DuplicateJobException if a job with the same name was already + * registered + */ + Collection reload(ApplicationContextFactory factory); + + /** + * Unregister all the jobs and close all the contexts created by this + * loader. */ void clear(); - + } diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/JobRegistryBeanPostProcessor.java b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/JobRegistryBeanPostProcessor.java index eb676c913..f8dccf717 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/JobRegistryBeanPostProcessor.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/JobRegistryBeanPostProcessor.java @@ -18,6 +18,8 @@ package org.springframework.batch.core.configuration.support; import java.util.Collection; import java.util.HashSet; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.batch.core.Job; import org.springframework.batch.core.configuration.DuplicateJobException; import org.springframework.batch.core.configuration.JobLocator; @@ -45,6 +47,8 @@ import org.springframework.util.Assert; public class JobRegistryBeanPostProcessor implements BeanPostProcessor, BeanFactoryAware, InitializingBean, DisposableBean { + private static Log logger = LogFactory.getLog(JobRegistryBeanPostProcessor.class); + // It doesn't make sense for this to have a default value... private JobRegistry jobRegistry = null; @@ -106,6 +110,7 @@ public class JobRegistryBeanPostProcessor implements BeanPostProcessor, BeanFact */ public void destroy() throws Exception { for (String name : jobNames) { + logger.debug("Unregistering job: " + name); jobRegistry.unregister(name); } jobNames.clear(); @@ -128,8 +133,10 @@ public class JobRegistryBeanPostProcessor implements BeanPostProcessor, BeanFact } job = groupName==null ? job : new GroupAwareJob(groupName, job); ReferenceJobFactory jobFactory = new ReferenceJobFactory(job); + String name = jobFactory.getJobName(); + logger.debug("Registering job: " + name); jobRegistry.register(jobFactory); - jobNames.add(jobFactory.getJobName()); + jobNames.add(name); } catch (DuplicateJobException e) { throw new FatalBeanException("Cannot register job configuration", e); diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/OsgiBundleXmlApplicationContextFactory.java b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/OsgiBundleXmlApplicationContextFactory.java index 3d5734a0f..e83d7fc3c 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/OsgiBundleXmlApplicationContextFactory.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/OsgiBundleXmlApplicationContextFactory.java @@ -91,4 +91,25 @@ public class OsgiBundleXmlApplicationContextFactory implements BundleContextAwar return context; } + @Override + public String toString() { + String bundleId = bundleContext == null ? null : (bundleContext.getBundle() == null ? bundleContext.toString() + : "" + bundleContext.getBundle().getBundleId()); + return "OsgiBundleXmlApplicationContext [path=" + path + ", bundle=" + bundleId + "]"; + } + + @Override + public int hashCode() { + return toString().hashCode(); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + return toString().equals(obj.toString()); + } + } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/support/ClassPathXmlApplicationContextFactoryTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/support/ClassPathXmlApplicationContextFactoryTests.java index 25ccdaac1..6ee78cef7 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/support/ClassPathXmlApplicationContextFactoryTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/support/ClassPathXmlApplicationContextFactoryTests.java @@ -24,6 +24,7 @@ import org.springframework.beans.factory.config.BeanFactoryPostProcessor; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.support.ClassPathXmlApplicationContext; import org.springframework.core.io.ClassPathResource; +import org.springframework.core.io.Resource; import org.springframework.util.ClassUtils; /** @@ -36,30 +37,36 @@ public class ClassPathXmlApplicationContextFactoryTests { @Test public void testCreateJob() { - factory.setResource(new ClassPathResource(ClassUtils.addResourcePathToPackagePath(getClass(), "trivial-context.xml"))); + factory.setResource(new ClassPathResource(ClassUtils.addResourcePathToPackagePath(getClass(), + "trivial-context.xml"))); assertNotNull(factory.createApplicationContext()); } @Test public void testGetJobName() { - factory.setResource(new ClassPathResource(ClassUtils.addResourcePathToPackagePath(getClass(), "trivial-context.xml"))); + factory.setResource(new ClassPathResource(ClassUtils.addResourcePathToPackagePath(getClass(), + "trivial-context.xml"))); assertEquals("test-job", factory.createApplicationContext().getBeanNamesForType(Job.class)[0]); } @Test public void testParentConfigurationInherited() { - factory.setApplicationContext(new ClassPathXmlApplicationContext(ClassUtils.addResourcePathToPackagePath(getClass(), "parent-context.xml"))); - factory.setResource(new ClassPathResource(ClassUtils.addResourcePathToPackagePath(getClass(), "child-context.xml"))); + factory.setApplicationContext(new ClassPathXmlApplicationContext(ClassUtils.addResourcePathToPackagePath( + getClass(), "parent-context.xml"))); + factory.setResource(new ClassPathResource(ClassUtils.addResourcePathToPackagePath(getClass(), + "child-context.xml"))); ConfigurableApplicationContext context = factory.createApplicationContext(); assertEquals("test-job", context.getBeanNamesForType(Job.class)[0]); assertEquals("bar", ((Job) context.getBean("test-job", Job.class)).getName()); assertEquals(4, ((Foo) context.getBean("foo", Foo.class)).values[1], 0.01); } - + @Test public void testBeanFactoryPostProcessorsNotCopied() { - factory.setApplicationContext(new ClassPathXmlApplicationContext(ClassUtils.addResourcePathToPackagePath(getClass(), "parent-context.xml"))); - factory.setResource(new ClassPathResource(ClassUtils.addResourcePathToPackagePath(getClass(), "child-context.xml"))); + factory.setApplicationContext(new ClassPathXmlApplicationContext(ClassUtils.addResourcePathToPackagePath( + getClass(), "parent-context.xml"))); + factory.setResource(new ClassPathResource(ClassUtils.addResourcePathToPackagePath(getClass(), + "child-context.xml"))); @SuppressWarnings("unchecked") Class[] classes = (Class[]) new Class[0]; factory.setBeanFactoryPostProcessorClasses(classes); @@ -68,21 +75,36 @@ public class ClassPathXmlApplicationContextFactoryTests { assertEquals("${foo}", ((Job) context.getBean("test-job", Job.class)).getName()); assertEquals(4, ((Foo) context.getBean("foo", Foo.class)).values[1], 0.01); } - + @Test public void testBeanFactoryConfigurationNotCopied() { - factory.setApplicationContext(new ClassPathXmlApplicationContext(ClassUtils.addResourcePathToPackagePath(getClass(), "parent-context.xml"))); - factory.setResource(new ClassPathResource(ClassUtils.addResourcePathToPackagePath(getClass(), "child-context.xml"))); + factory.setApplicationContext(new ClassPathXmlApplicationContext(ClassUtils.addResourcePathToPackagePath( + getClass(), "parent-context.xml"))); + factory.setResource(new ClassPathResource(ClassUtils.addResourcePathToPackagePath(getClass(), + "child-context.xml"))); factory.setCopyConfiguration(false); ConfigurableApplicationContext context = factory.createApplicationContext(); assertEquals("test-job", context.getBeanNamesForType(Job.class)[0]); assertEquals("bar", ((Job) context.getBean("test-job", Job.class)).getName()); - // The CustomEditorConfigurer is a BeanFactoryPostProcessor so the editor gets copied anyway! + // The CustomEditorConfigurer is a BeanFactoryPostProcessor so the + // editor gets copied anyway! assertEquals(4, ((Foo) context.getBean("foo", Foo.class)).values[1], 0.01); } - + + @Test + public void testEquals() throws Exception { + Resource resource = new ClassPathResource(ClassUtils.addResourcePathToPackagePath(getClass(), + "child-context.xml")); + factory.setResource(resource); + ClassPathXmlApplicationContextFactory other = new ClassPathXmlApplicationContextFactory(); + other.setResource(resource); + assertEquals(other, factory); + assertEquals(other.hashCode(), factory.hashCode()); + } + public static class Foo { private double[] values; + public void setValues(double[] values) { this.values = values; } 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 new file mode 100644 index 000000000..93fb83231 --- /dev/null +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/support/DefaultJobLoaderTests.java @@ -0,0 +1,54 @@ +/* + * Copyright 2006-2010 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.core.configuration.support; + +import static org.junit.Assert.assertEquals; + +import org.junit.Test; +import org.springframework.batch.core.configuration.JobRegistry; +import org.springframework.core.io.ClassPathResource; + +/** + * @author Dave Syer + * + */ +public class DefaultJobLoaderTests { + + private JobRegistry registry = new MapJobRegistry(); + + private DefaultJobLoader jobLoader = new DefaultJobLoader(registry); + + @Test + public void testReload() throws Exception { + ClassPathXmlApplicationContextFactory factory = new ClassPathXmlApplicationContextFactory( + new ClassPathResource("trivial-context.xml", getClass())); + jobLoader.load(factory); + assertEquals(1, registry.getJobNames().size()); + jobLoader.reload(factory); + assertEquals(1, registry.getJobNames().size()); + } + + @Test + public void testReloadWithAutoRegister() throws Exception { + ClassPathXmlApplicationContextFactory factory = new ClassPathXmlApplicationContextFactory( + new ClassPathResource("trivial-context-autoregister.xml", getClass())); + jobLoader.load(factory); + assertEquals(1, registry.getJobNames().size()); + jobLoader.reload(factory); + assertEquals(1, registry.getJobNames().size()); + } + +} diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/support/OsgiBundleXmlApplicationContextFactoryTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/support/OsgiBundleXmlApplicationContextFactoryTests.java index 1c94c2673..24b3e8fde 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/support/OsgiBundleXmlApplicationContextFactoryTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/support/OsgiBundleXmlApplicationContextFactoryTests.java @@ -20,6 +20,7 @@ import static org.easymock.EasyMock.createNiceMock; import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.replay; import static org.easymock.EasyMock.verify; +import static org.junit.Assert.assertEquals; import org.junit.Test; import org.osgi.framework.Bundle; @@ -50,6 +51,15 @@ public class OsgiBundleXmlApplicationContextFactoryTests { verify(bundleContext, bundle); } + @Test + public void testEquals() throws Exception { + factory.setPath("child-context.xml"); + OsgiBundleXmlApplicationContextFactory other = new OsgiBundleXmlApplicationContextFactory(); + other.setPath("child-context.xml"); + assertEquals(other, factory); + assertEquals(other.hashCode(), factory.hashCode()); + } + /** * Test method for {@link org.springframework.batch.core.configuration.support.OsgiBundleXmlApplicationContextFactory#setApplicationContext(org.springframework.context.ApplicationContext)}. */ 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 new file mode 100644 index 000000000..6c8766611 --- /dev/null +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/support/trivial-context-autoregister.xml @@ -0,0 +1,17 @@ + + + + + + + + + + + +