Spring/spring-batch
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

BATCH-282: fix issue with bean factory initialization and concurrent modification

Browse Source
This commit is contained in:
dsyer
2008-11-20 08:34:53 +00:00
parent 41e22f08b9
commit 892e037f1d
9 changed files with 173 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files

45
spring-batch-core/src/main/java/org/springframework/batch/core/scope/StepScope.java
View File

@@ -17,9 +17,7 @@ package org.springframework.batch.core.scope;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.aop.framework.autoproxy.AutoProxyUtils;
import org.springframework.batch.core.scope.util.PlaceholderProxyFactoryBean;
import org.springframework.batch.core.scope.util.StepContextFactory;
import org.springframework.beans.BeanWrapper;
import org.springframework.beans.BeansException;
import org.springframework.beans.factory.ObjectFactory;
@@ -31,7 +29,6 @@ import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
import org.springframework.beans.factory.config.Scope;
import org.springframework.beans.factory.support.BeanDefinitionReaderUtils;
import org.springframework.beans.factory.support.BeanDefinitionRegistry;
import org.springframework.beans.factory.support.RootBeanDefinition;
import org.springframework.core.Ordered;
import org.springframework.util.Assert;
import org.springframework.util.StringValueResolver;
@@ -214,7 +211,7 @@ public class StepScope implements Scope, BeanFactoryPostProcessor, Ordered {
public void setName(String name) {
this.name = name;
}
/**
* Wrap a target bean definition in a proxy that defers initialization until
* after the {@link StepContext} is available. Amounts to adding
@@ -237,7 +234,7 @@ public class StepScope implements Scope, BeanFactoryPostProcessor, Ordered {
// TODO: detect presence of Spring 3.0 and use ScopedPoxyUtils instead
// Create the scoped proxy...
BeanDefinitionHolder proxyHolder = createScopedProxy(new BeanDefinitionHolder(definition, beanName), registry,
BeanDefinitionHolder proxyHolder = PlaceholderProxyFactoryBean.createScopedProxy(new BeanDefinitionHolder(definition, beanName), registry,
proxyTargetClass);
// ...and register it under the original target name
registry.registerBeanDefinition(beanName, proxyHolder.getBeanDefinition());
@@ -246,44 +243,6 @@ public class StepScope implements Scope, BeanFactoryPostProcessor, Ordered {
}
private static BeanDefinitionHolder createScopedProxy(BeanDefinitionHolder definition,
BeanDefinitionRegistry registry, boolean proxyTargetClass) {
String originalBeanName = definition.getBeanName();
BeanDefinition targetDefinition = definition.getBeanDefinition();
// Create a proxy definition for the original bean name,
// "hiding" the target bean in an internal target definition.
RootBeanDefinition proxyDefinition = new RootBeanDefinition(PlaceholderProxyFactoryBean.class);
proxyDefinition.getConstructorArgumentValues().addGenericArgumentValue(new StepContextFactory());
proxyDefinition.setOriginatingBeanDefinition(definition.getBeanDefinition());
proxyDefinition.setSource(definition.getSource());
proxyDefinition.setRole(BeanDefinition.ROLE_INFRASTRUCTURE);
String targetBeanName = "lazyBindingProxy." + originalBeanName;
proxyDefinition.getPropertyValues().addPropertyValue("targetBeanName", targetBeanName);
if (proxyTargetClass) {
targetDefinition.setAttribute(AutoProxyUtils.PRESERVE_TARGET_CLASS_ATTRIBUTE, Boolean.TRUE);
// ProxyFactoryBean's "proxyTargetClass" default is TRUE, so we
// don't need to set it explicitly here.
}
else {
proxyDefinition.getPropertyValues().addPropertyValue("proxyTargetClass", Boolean.FALSE);
}
proxyDefinition.setAutowireCandidate(targetDefinition.isAutowireCandidate());
// The target bean should be ignored in favor of the proxy.
targetDefinition.setAutowireCandidate(false);
// Register the target bean as separate bean in the factory.
registry.registerBeanDefinition(targetBeanName, targetDefinition);
// Return the scoped proxy definition as primary bean definition
// (potentially an inner bean).
return new BeanDefinitionHolder(proxyDefinition, originalBeanName, definition.getAliases());
}
/**
* Helper class to scan a bean definition hierarchy and force the use of
* auto-proxy for step scoped beans.

60
spring-batch-core/src/main/java/org/springframework/batch/core/scope/util/PlaceholderProxyFactoryBean.java
View File

@@ -5,6 +5,7 @@ import java.lang.reflect.Modifier;
import org.springframework.aop.framework.AopInfrastructureBean;
import org.springframework.aop.framework.ProxyConfig;
import org.springframework.aop.framework.ProxyFactory;
import org.springframework.aop.framework.autoproxy.AutoProxyUtils;
import org.springframework.aop.scope.DefaultScopedObject;
import org.springframework.aop.scope.ScopedObject;
import org.springframework.aop.scope.ScopedProxyFactoryBean;
@@ -13,7 +14,11 @@ import org.springframework.beans.factory.BeanFactory;
import org.springframework.beans.factory.BeanFactoryAware;
import org.springframework.beans.factory.FactoryBean;
import org.springframework.beans.factory.FactoryBeanNotInitializedException;
import org.springframework.beans.factory.config.BeanDefinition;
import org.springframework.beans.factory.config.BeanDefinitionHolder;
import org.springframework.beans.factory.config.ConfigurableBeanFactory;
import org.springframework.beans.factory.support.BeanDefinitionRegistry;
import org.springframework.beans.factory.support.RootBeanDefinition;
import org.springframework.util.ClassUtils;
/**
@@ -80,8 +85,8 @@ public class PlaceholderProxyFactoryBean extends ProxyConfig implements FactoryB
pf.addAdvice(new DelegatingIntroductionInterceptor(scopedObject));
// Add the AopInfrastructureBean marker to indicate that the scoped
// proxy
// itself is not subject to auto-proxying! Only its target bean is.
// proxy itself is not subject to auto-proxying! Only its target bean
// is.
pf.addInterface(AopInfrastructureBean.class);
this.proxy = pf.getProxy(cbf.getBeanClassLoader());
@@ -107,4 +112,55 @@ public class PlaceholderProxyFactoryBean extends ProxyConfig implements FactoryB
public boolean isSingleton() {
return true;
}
/**
* Convenience method to create a {@link BeanDefinition} for a target
* wrapped in a placeholder tarrget source, able to defer binding of
* placeholders until the bean is used.
*
* @param definition a target bean definition
* @param registry a {@link BeanDefinitionRegistry}
* @param proxyTargetClass true if we need to use CGlib to create the
* proxies
* @return a {@link BeanDefinitionHolder} for a
* {@link PlaceholderProxyFactoryBean}
*/
public static BeanDefinitionHolder createScopedProxy(BeanDefinitionHolder definition,
BeanDefinitionRegistry registry, boolean proxyTargetClass) {
String originalBeanName = definition.getBeanName();
BeanDefinition targetDefinition = definition.getBeanDefinition();
// Create a proxy definition for the original bean name,
// "hiding" the target bean in an internal target definition.
RootBeanDefinition proxyDefinition = new RootBeanDefinition(PlaceholderProxyFactoryBean.class);
proxyDefinition.getConstructorArgumentValues().addGenericArgumentValue(new StepContextFactory());
proxyDefinition.setOriginatingBeanDefinition(definition.getBeanDefinition());
proxyDefinition.setSource(definition.getSource());
proxyDefinition.setRole(BeanDefinition.ROLE_INFRASTRUCTURE);
String targetBeanName = "lazyBindingProxy." + originalBeanName;
proxyDefinition.getPropertyValues().addPropertyValue("targetBeanName", targetBeanName);
if (proxyTargetClass) {
targetDefinition.setAttribute(AutoProxyUtils.PRESERVE_TARGET_CLASS_ATTRIBUTE, Boolean.TRUE);
// ProxyFactoryBean's "proxyTargetClass" default is TRUE, so we
// don't need to set it explicitly here.
}
else {
proxyDefinition.getPropertyValues().addPropertyValue("proxyTargetClass", Boolean.FALSE);
}
proxyDefinition.setAutowireCandidate(targetDefinition.isAutowireCandidate());
// The target bean should be ignored in favor of the proxy.
targetDefinition.setAutowireCandidate(false);
// Register the target bean as separate bean in the factory.
registry.registerBeanDefinition(targetBeanName, targetDefinition);
// Return the scoped proxy definition as primary bean definition
// (potentially an inner bean).
return new BeanDefinitionHolder(proxyDefinition, originalBeanName, definition.getAliases());
}
}

52
spring-batch-core/src/main/java/org/springframework/batch/core/scope/util/PlaceholderTargetSource.java
View File

@@ -17,14 +17,15 @@ 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;
import org.springframework.beans.TypeConverter;
import org.springframework.beans.TypeMismatchException;
import org.springframework.beans.factory.InitializingBean;
import org.springframework.beans.factory.config.BeanDefinition;
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;
@@ -57,10 +58,10 @@ public class PlaceholderTargetSource extends SimpleBeanTargetSource implements I
private static final String PLACEHOLDER_SUFFIX = "}";
private volatile boolean active = false;
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.
@@ -93,10 +94,8 @@ public class PlaceholderTargetSource extends SimpleBeanTargetSource implements I
final TypeConverter typeConverter = listableBeanFactory.getTypeConverter();
// Try to prevent other threads from using this TypeConverter
active = true;
listableBeanFactory.setTypeConverter(new TypeConverter() {
DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory(listableBeanFactory);
beanFactory.setTypeConverter(new TypeConverter() {
@SuppressWarnings("unchecked")
public Object convertIfNecessary(Object value, Class requiredType, MethodParameter methodParam)
throws TypeMismatchException {
@@ -127,8 +126,8 @@ public class PlaceholderTargetSource extends SimpleBeanTargetSource implements I
* come back when getBean() is called later on
*/
String targetBeanName = getTargetBeanName();
BeanDefinition originalDefinition = listableBeanFactory.getMergedBeanDefinition(getTargetBeanName());
GenericBeanDefinition beanDefinition = new GenericBeanDefinition(originalDefinition);
GenericBeanDefinition beanDefinition = new GenericBeanDefinition(listableBeanFactory
.getMergedBeanDefinition(targetBeanName));
logger.debug("Rehydrating scoped target: [" + targetBeanName + "]");
BeanDefinitionVisitor visitor = new BeanDefinitionVisitor(new StringValueResolver() {
@@ -156,18 +155,27 @@ public class PlaceholderTargetSource extends SimpleBeanTargetSource implements I
};
listableBeanFactory.registerBeanDefinition(beanName, beanDefinition);
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);
return listableBeanFactory.getBean(beanName);
return beanFactory.getBean(beanName);
}
finally {
listableBeanFactory.removeBeanDefinition(beanName);
// Replace the original type converter. TODO: does this cause
// problems if in fact it was null to start with?
listableBeanFactory.setTypeConverter(typeConverter);
active = false;
beanFactory.destroySingletons();
beanFactory = null;
// Anything else we can do to clean it up?
}
}
@@ -179,13 +187,11 @@ public class PlaceholderTargetSource extends SimpleBeanTargetSource implements I
*/
private Object convertFromContext(String key, Class<?> requiredType) {
Object result = null;
if (active) {
BeanWrapper wrapper = new BeanWrapperImpl(contextFactory.getContext());
if (wrapper.isReadableProperty(key)) {
Object property = wrapper.getPropertyValue(key);
if (requiredType.isAssignableFrom(property.getClass())) {
result = property;
}
BeanWrapper wrapper = new BeanWrapperImpl(contextFactory.getContext());
if (wrapper.isReadableProperty(key)) {
Object property = wrapper.getPropertyValue(key);
if (requiredType.isAssignableFrom(property.getClass())) {
result = property;
}
}
return result;

24
spring-batch-core/src/test/java/org/springframework/batch/core/scope/JobStartupRunner.java Normal file
View File

@@ -0,0 +1,24 @@
package org.springframework.batch.core.scope;
import javax.annotation.PostConstruct;
import org.springframework.batch.core.JobExecution;
import org.springframework.batch.core.Step;
import org.springframework.batch.core.StepExecution;
public class JobStartupRunner {
private Step step;
public void setStep(Step step) {
this.step = step;
}
@PostConstruct
public void launch() throws Exception {
StepExecution stepExecution = new StepExecution("step", new JobExecution(1L), 0L);
step.execute(stepExecution);
// expect no errors
}
}

4
spring-batch-core/src/test/java/org/springframework/batch/core/scope/StepScopePlaceholderIntegrationTests.java
View File

@@ -79,6 +79,10 @@ public class StepScopePlaceholderIntegrationTests implements BeanFactoryAware {
@Test
public void testSimpleProperty() throws Exception {
assertEquals("bar", simple.getName());
// Once the step context is set up it should be baked into the proxies
// so changing it now should have no effect
stepExecution.getExecutionContext().put("foo", "wrong!");
assertEquals("bar", simple.getName());
}
@Test

17
spring-batch-core/src/test/java/org/springframework/batch/core/scope/StepScopeStartupIntegrationTests.java Normal file
View File

@@ -0,0 +1,17 @@
package org.springframework.batch.core.scope;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
@ContextConfiguration
@RunWith(SpringJUnit4ClassRunner.class)
public class StepScopeStartupIntegrationTests {
@Test
public void testScopedProxyDuringStartup() throws Exception {
}
}

6
spring-batch-core/src/test/java/org/springframework/batch/core/scope/TestStep.java
View File

@@ -24,6 +24,12 @@ public class TestStep implements Step {
public void execute(StepExecution stepExecution) throws JobInterruptedException {
context = StepSynchronizationManager.getContext();
setContextFromCollaborator();
stepExecution.getExecutionContext().put("foo", "changed but it shouldn't affect the collaborator");
setContextFromCollaborator();
}
private void setContextFromCollaborator() {
if (context != null) {
context.setAttribute("collaborator", collaborator.getName());
}

3
spring-batch-core/src/test/resources/org/springframework/batch/core/scope/StepScopeIntegrationTests-context.xml
View File

@@ -5,7 +5,7 @@
xsi:schemaLocation="
http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd
http://www.springframework.org/schema/aop http://www.springframework.org/schema/aop/spring-aop.xsd
http://www.springframework.org/schema/tx http://www.springframework.org/schema/tx/spring-tx.xsd">
http://www.springframework.org/schema/tx http://www.springframework.org/schema/tx/spring-tx.xsd">
<bean id="vanilla" class="org.springframework.batch.core.scope.TestStep">
<property name="collaborator">
@@ -40,4 +40,5 @@
<aop:aspectj-autoproxy />
<bean class="org.springframework.batch.core.scope.StepScopeManager" />
<bean class="org.springframework.batch.core.scope.StepScope" />
</beans>

31
spring-batch-core/src/test/resources/org/springframework/batch/core/scope/StepScopeStartupIntegrationTests-context.xml Normal file
View File

@@ -0,0 +1,31 @@
<?xml version="1.0" encoding="UTF-8"?>
<beans xmlns="http://www.springframework.org/schema/beans"
xmlns:context="http://www.springframework.org/schema/context"
xmlns:aop="http://www.springframework.org/schema/aop" xmlns:tx="http://www.springframework.org/schema/tx"
xmlns:p="http://www.springframework.org/schema/p" 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.xsd
http://www.springframework.org/schema/context http://www.springframework.org/schema/context/spring-context.xsd
http://www.springframework.org/schema/aop http://www.springframework.org/schema/aop/spring-aop.xsd
http://www.springframework.org/schema/tx http://www.springframework.org/schema/tx/spring-tx.xsd">
<context:annotation-config />
<bean class="org.springframework.batch.core.scope.JobStartupRunner">
<property name="step" ref="proxied" />
</bean>
<bean id="proxied" class="org.springframework.batch.core.scope.TestStep">
<property name="collaborator">
<bean class="org.springframework.batch.core.scope.TestCollaborator"
scope="step">
<property name="name" value="bar" />
</bean>
</property>
</bean>
<aop:aspectj-autoproxy />
<bean class="org.springframework.batch.core.scope.StepScopeManager" />
<bean class="org.springframework.batch.core.scope.StepScope" />
</beans>