From fb9b662160e28ee4200b3902165a3fb84d1a6ce5 Mon Sep 17 00:00:00 2001 From: dsyer Date: Mon, 24 Nov 2008 08:24:51 +0000 Subject: [PATCH] RESOLVED BATCH-939: Make step scope work with aop-scoped-proxy --- spring-batch-core/.springBeans | 26 ++++++ .../batch/core/scope/StepContext.java | 2 +- .../batch/core/scope/StepScope.java | 1 + .../util/PlaceholderProxyFactoryBean.java | 2 +- .../scope/util/PlaceholderTargetSource.java | 19 +--- ...peDestructionCallbackIntegrationTests.java | 72 +++++++++++++++ .../StepScopeNestedIntegrationTests.java | 33 +++++++ .../batch/core/scope/TestAdvice.java | 20 ++++ .../batch/core/scope/TestCollaborator.java | 3 - .../scope/TestDisposableCollaborator.java | 14 +++ .../batch/core/scope/TestStep.java | 3 +- ...onent-context-with-resolver-nonscanned.xml | 12 +-- .../batch-component-context-with-resolver.xml | 17 ++-- .../annotation/batch-component-context.xml | 6 +- .../annotation/component-scan-context.xml | 91 ++++++++++--------- ...uctionCallbackIntegrationTests-context.xml | 49 ++++++++++ ...tepScopeNestedIntegrationTests-context.xml | 33 +++++++ 17 files changed, 319 insertions(+), 84 deletions(-) create mode 100644 spring-batch-core/src/test/java/org/springframework/batch/core/scope/StepScopeDestructionCallbackIntegrationTests.java create mode 100644 spring-batch-core/src/test/java/org/springframework/batch/core/scope/StepScopeNestedIntegrationTests.java create mode 100644 spring-batch-core/src/test/java/org/springframework/batch/core/scope/TestAdvice.java create mode 100644 spring-batch-core/src/test/java/org/springframework/batch/core/scope/TestDisposableCollaborator.java create mode 100644 spring-batch-core/src/test/resources/org/springframework/batch/core/scope/StepScopeDestructionCallbackIntegrationTests-context.xml create mode 100644 spring-batch-core/src/test/resources/org/springframework/batch/core/scope/StepScopeNestedIntegrationTests-context.xml diff --git a/spring-batch-core/.springBeans b/spring-batch-core/.springBeans index 5388934d3..88b2c2284 100644 --- a/spring-batch-core/.springBeans +++ b/spring-batch-core/.springBeans @@ -22,6 +22,32 @@ src/test/resources/org/springframework/batch/core/configuration/xml/OneStepJobParserTests-context.xml src/test/resources/org/springframework/batch/core/configuration/xml/TwoStepJobParserTests-context.xml src/test/resources/org/springframework/batch/core/annotation/batch-component-context.xml + src/test/resources/org/springframework/batch/core/scope/util/AsyncPlaceholderTargetSourceTests-context.xml + src/test/resources/org/springframework/batch/core/scope/AsyncStepScopeIntegrationTests-context.xml + src/test/resources/org/springframework/batch/core/annotation/batch-component-context-scoped.xml + src/test/resources/org/springframework/batch/core/annotation/batch-component-context-with-resolver-nonscanned.xml + src/test/resources/org/springframework/batch/core/annotation/batch-component-context-with-resolver.xml + src/test/resources/org/springframework/batch/core/annotation/component-scan-context.xml + src/test/resources/org/springframework/batch/core/configuration/xml/DecisionJobParserTests-context.xml + src/test/resources/org/springframework/batch/core/configuration/xml/JobExecutionListenerParserTests-context.xml + src/test/resources/org/springframework/batch/core/configuration/xml/JobRepositoryParserTests-context.xml + src/test/resources/org/springframework/batch/core/partition/launch-context.xml + src/test/resources/org/springframework/batch/core/scope/util/MultipleContextPlaceholderTargetSourceTests-context.xml + src/test/resources/org/springframework/batch/core/configuration/xml/NextAttributeFailureJobParserTests-context.xml + src/test/resources/org/springframework/batch/core/configuration/xml/NextAttributeJobParserTests-context.xml + src/test/resources/org/springframework/batch/core/scope/util/PlaceholderTargetSourceTests-context.xml + src/test/resources/org/springframework/batch/core/configuration/xml/RepositoryJobParserTests-context.xml + src/test/resources/org/springframework/batch/core/scope/util/SimplePlaceholderTargetSourceTests-context.xml + src/test/resources/org/springframework/batch/core/configuration/xml/SplitJobParserTests-context.xml + src/test/resources/org/springframework/batch/core/scope/StepScopeDestructionCallbackIntegrationTests-context.xml + src/test/resources/org/springframework/batch/core/scope/StepScopeIntegrationTests-context.xml + src/test/resources/org/springframework/batch/core/scope/StepScopeNestedIntegrationTests-context.xml + src/test/resources/org/springframework/batch/core/scope/StepScopePlaceholderIntegrationTests-context.xml + src/test/resources/org/springframework/batch/core/scope/StepScopeStartupIntegrationTests-context.xml + src/test/resources/org/springframework/batch/core/configuration/xml/StepWithBasicProcessTaskJobParserTests-context.xml + src/test/resources/org/springframework/batch/core/configuration/xml/StepWithFaultTolerantProcessTaskJobParserTests-context.xml + src/test/resources/org/springframework/batch/core/configuration/xml/StepWithSimpleTaskJobParserTests-context.xml + src/test/resources/org/springframework/batch/core/configuration/xml/StopJobParserTests-context.xml diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/scope/StepContext.java b/spring-batch-core/src/main/java/org/springframework/batch/core/scope/StepContext.java index 9209627a3..539c24102 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/scope/StepContext.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/scope/StepContext.java @@ -164,7 +164,7 @@ public class StepContext extends SynchronizedAttributeAccessor { } else { throw new UnexpectedJobExecutionException("Could not close step context, rethrowing first of " - + errors.size() + " execptions.", error); + + errors.size() + " exceptions.", error); } } diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/scope/StepScope.java b/spring-batch-core/src/main/java/org/springframework/batch/core/scope/StepScope.java index 383e50b71..1cb6a36a5 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/scope/StepScope.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/scope/StepScope.java @@ -285,6 +285,7 @@ public class StepScope implements Scope, BeanFactoryPostProcessor, Ordered { return createScopedProxy(holder.getBeanName(), definition, registry, proxyTargetClass); } } + value = super.resolveValue(value); return value; } } diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/scope/util/PlaceholderProxyFactoryBean.java b/spring-batch-core/src/main/java/org/springframework/batch/core/scope/util/PlaceholderProxyFactoryBean.java index a5f393903..ceecb1540 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/scope/util/PlaceholderProxyFactoryBean.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/scope/util/PlaceholderProxyFactoryBean.java @@ -47,7 +47,7 @@ public class PlaceholderProxyFactoryBean extends ProxyConfig implements FactoryB */ public PlaceholderProxyFactoryBean(ContextFactory contextFactory) { this.contextFactory = contextFactory; - setProxyTargetClass(true); + // setProxyTargetClass(false); } /** diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/scope/util/PlaceholderTargetSource.java b/spring-batch-core/src/main/java/org/springframework/batch/core/scope/util/PlaceholderTargetSource.java index 6b56879a2..2ba1277d2 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/scope/util/PlaceholderTargetSource.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/scope/util/PlaceholderTargetSource.java @@ -17,7 +17,6 @@ package org.springframework.batch.core.scope.util; import org.springframework.aop.TargetSource; import org.springframework.aop.target.SimpleBeanTargetSource; -import org.springframework.batch.core.scope.StepScope; import org.springframework.beans.BeanWrapper; import org.springframework.beans.BeanWrapperImpl; import org.springframework.beans.BeansException; @@ -25,7 +24,6 @@ import org.springframework.beans.TypeConverter; import org.springframework.beans.TypeMismatchException; import org.springframework.beans.factory.InitializingBean; import org.springframework.beans.factory.config.BeanDefinitionVisitor; -import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.beans.factory.config.TypedStringValue; import org.springframework.beans.factory.support.DefaultListableBeanFactory; import org.springframework.beans.factory.support.GenericBeanDefinition; @@ -60,8 +58,6 @@ public class PlaceholderTargetSource extends SimpleBeanTargetSource implements I private ContextFactory contextFactory; - private StepScope scope = new StepScope(); - /** * Public setter for the context factory. Used to construct the context root * whenever placeholders are replaced in a bean definition. @@ -95,6 +91,8 @@ public class PlaceholderTargetSource extends SimpleBeanTargetSource implements I final TypeConverter typeConverter = listableBeanFactory.getTypeConverter(); DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory(listableBeanFactory); + beanFactory.copyConfigurationFrom(listableBeanFactory); + beanFactory.setTypeConverter(new TypeConverter() { @SuppressWarnings("unchecked") public Object convertIfNecessary(Object value, Class requiredType, MethodParameter methodParam) @@ -155,17 +153,6 @@ public class PlaceholderTargetSource extends SimpleBeanTargetSource implements I }; - String beanScope = beanDefinition.getScope(); - /* - * The scope for the bean really ought to be "step" if we get this - * far, but we'll be cautious anyway and avoid a potential issue - * with trying to replace the default scopes (which is not allowed) - */ - if (!beanScope.equals(ConfigurableListableBeanFactory.SCOPE_PROTOTYPE) - && !beanScope.equals(ConfigurableListableBeanFactory.SCOPE_SINGLETON)) { - // Need this otherwise there will be no step scope available. - beanFactory.registerScope(beanScope, scope); - } beanFactory.registerBeanDefinition(beanName, beanDefinition); // Make the replacements before the target is hydrated visitor.visitBeanDefinition(beanDefinition); @@ -173,7 +160,7 @@ public class PlaceholderTargetSource extends SimpleBeanTargetSource implements I } finally { - beanFactory.destroySingletons(); + beanFactory.removeBeanDefinition(beanName); beanFactory = null; // Anything else we can do to clean it up? } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/scope/StepScopeDestructionCallbackIntegrationTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/scope/StepScopeDestructionCallbackIntegrationTests.java new file mode 100644 index 000000000..445222f11 --- /dev/null +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/scope/StepScopeDestructionCallbackIntegrationTests.java @@ -0,0 +1,72 @@ +package org.springframework.batch.core.scope; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.batch.core.JobExecution; +import org.springframework.batch.core.Step; +import org.springframework.batch.core.StepExecution; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +@ContextConfiguration +@RunWith(SpringJUnit4ClassRunner.class) +public class StepScopeDestructionCallbackIntegrationTests { + + @Autowired + @Qualifier("proxied") + private Step proxied; + + @Autowired + @Qualifier("nested") + private Step nested; + + @Autowired + @Qualifier("foo") + private Collaborator foo; + + @Before + @After + public void resetMessage() throws Exception { + TestDisposableCollaborator.message = "none"; + TestAdvice.names.clear(); + } + + @Test + public void testDisposableScopedProxy() throws Exception { + assertNotNull(proxied); + proxied.execute(new StepExecution("step", new JobExecution(0L), 1L)); + assertEquals("destroyed", TestDisposableCollaborator.message); + } + + @Test + public void testDisposableInnerScopedProxy() throws Exception { + assertNotNull(nested); + nested.execute(new StepExecution("step", new JobExecution(0L), 1L)); + assertEquals("destroyed", TestDisposableCollaborator.message); + } + + @Test + public void testProxiedScopedProxy() throws Exception { + assertNotNull(nested); + nested.execute(new StepExecution("step", new JobExecution(0L), 1L)); + assertEquals(2, TestAdvice.names.size()); + assertEquals("bar", TestAdvice.names.get(0)); + assertEquals("destroyed", TestDisposableCollaborator.message); + } + + @Test + public void testProxiedNormalBean() throws Exception { + assertNotNull(nested); + String name = foo.getName(); + assertEquals(1, TestAdvice.names.size()); + assertEquals(name, TestAdvice.names.get(0)); + } + +} diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/scope/StepScopeNestedIntegrationTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/scope/StepScopeNestedIntegrationTests.java new file mode 100644 index 000000000..8aa13fea7 --- /dev/null +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/scope/StepScopeNestedIntegrationTests.java @@ -0,0 +1,33 @@ +package org.springframework.batch.core.scope; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.batch.core.Step; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + + +@ContextConfiguration +@RunWith(SpringJUnit4ClassRunner.class) +public class StepScopeNestedIntegrationTests { + + @Autowired + @Qualifier("proxied") + private Step proxied; + + @Autowired + @Qualifier("parent") + private Collaborator parent; + + @Test + public void testNestedScopedProxy() throws Exception { + assertNotNull(proxied); + assertEquals("foo", parent.getName()); + } + +} diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/scope/TestAdvice.java b/spring-batch-core/src/test/java/org/springframework/batch/core/scope/TestAdvice.java new file mode 100644 index 000000000..3e9547881 --- /dev/null +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/scope/TestAdvice.java @@ -0,0 +1,20 @@ +package org.springframework.batch.core.scope; + +import java.util.ArrayList; +import java.util.List; + +import org.aspectj.lang.annotation.AfterReturning; +import org.aspectj.lang.annotation.Aspect; + +@Aspect +public class TestAdvice { + + public static List names = new ArrayList(); + + @AfterReturning(pointcut="execution(String org.springframework.batch.core.scope.Collaborator+.getName(..))", returning="name") + public void registerCollaborator(String name) { + names.add(name); + } + + +} diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/scope/TestCollaborator.java b/spring-batch-core/src/test/java/org/springframework/batch/core/scope/TestCollaborator.java index 74c177017..aa525ec99 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/scope/TestCollaborator.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/scope/TestCollaborator.java @@ -17,9 +17,6 @@ public class TestCollaborator implements Collaborator, Serializable { this.parent = parent; } - /* (non-Javadoc) - * @see org.springframework.batch.core.scope.Collaborator#getName() - */ public String getName() { return name; } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/scope/TestDisposableCollaborator.java b/spring-batch-core/src/test/java/org/springframework/batch/core/scope/TestDisposableCollaborator.java new file mode 100644 index 000000000..70596dfd3 --- /dev/null +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/scope/TestDisposableCollaborator.java @@ -0,0 +1,14 @@ +package org.springframework.batch.core.scope; + +import org.springframework.beans.factory.DisposableBean; + + +public class TestDisposableCollaborator extends TestCollaborator implements DisposableBean { + + public static volatile String message = "none"; + + public void destroy() throws Exception { + message = "destroyed"; + } + +} diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/scope/TestStep.java b/spring-batch-core/src/test/java/org/springframework/batch/core/scope/TestStep.java index e8d7d8fb6..51f8ae046 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/scope/TestStep.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/scope/TestStep.java @@ -31,7 +31,8 @@ public class TestStep implements Step { private void setContextFromCollaborator() { if (context != null) { - context.setAttribute("collaborator", collaborator.getName()); + String name = collaborator.getName(); + context.setAttribute("collaborator", name); } } diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/annotation/batch-component-context-with-resolver-nonscanned.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/annotation/batch-component-context-with-resolver-nonscanned.xml index 1d6622885..213d3811a 100644 --- a/spring-batch-core/src/test/resources/org/springframework/batch/core/annotation/batch-component-context-with-resolver-nonscanned.xml +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/annotation/batch-component-context-with-resolver-nonscanned.xml @@ -1,13 +1,13 @@ - + - - + + - + - + diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/annotation/batch-component-context-with-resolver.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/annotation/batch-component-context-with-resolver.xml index e603b6c97..9b7c02cfc 100644 --- a/spring-batch-core/src/test/resources/org/springframework/batch/core/annotation/batch-component-context-with-resolver.xml +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/annotation/batch-component-context-with-resolver.xml @@ -1,14 +1,13 @@ - + - - - + + + - + diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/annotation/batch-component-context.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/annotation/batch-component-context.xml index dc8798759..087e365bb 100644 --- a/spring-batch-core/src/test/resources/org/springframework/batch/core/annotation/batch-component-context.xml +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/annotation/batch-component-context.xml @@ -1,12 +1,12 @@ - + diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/annotation/component-scan-context.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/annotation/component-scan-context.xml index c05f3e68f..113248dda 100644 --- a/spring-batch-core/src/test/resources/org/springframework/batch/core/annotation/component-scan-context.xml +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/annotation/component-scan-context.xml @@ -1,49 +1,52 @@ - + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + - + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/scope/StepScopeDestructionCallbackIntegrationTests-context.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/scope/StepScopeDestructionCallbackIntegrationTests-context.xml new file mode 100644 index 000000000..1dee023f0 --- /dev/null +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/scope/StepScopeDestructionCallbackIntegrationTests-context.xml @@ -0,0 +1,49 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/scope/StepScopeNestedIntegrationTests-context.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/scope/StepScopeNestedIntegrationTests-context.xml new file mode 100644 index 000000000..b168b1a07 --- /dev/null +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/scope/StepScopeNestedIntegrationTests-context.xml @@ -0,0 +1,33 @@ + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file