Fix APC registration for @EnableTransactionManagement

Prior to this change, @EnableTransactionManagement (via the
ProxyTransactionManagementConfiguration class) did not properly
register its auto-proxy creator through the usual AopConfigUtils
methods.  It was trying to register the APC as a normal @Bean method,
but this causes issues (SPR-8494) with the logic in
AopConfigUtils#registerOrEscalateApcAsRequired, which expects the APC
bean definition to have a beanClassName property.  When the APC is
registered via a @Bean definition, it is actually a
factoryBean/factoryMethod situation with no directly resolvable
beanClass/beanClassName.

To solve this problem, ImportSelector#selectImports has been refactored
to accept an ImportSelector.Context instance. This object contains the
AnnotationMetadata of the importing class as well as the enclosing
BeanDefinitionRegistry to allow for the kind of conditional bean
registration necessary here. In this case, the bean definition that
must be registered conditionally is that of the auto-proxy creator.
It should only be registered if AdviceMode == PROXY, and thus the
ImportSelector is an appropriate place to make this happen.  It must
happen as a BeanDefinition (rather than a @Bean method) for
compatibility with AopConfigUtils, and working with the
BeanDefinitionRegistry API allows for that. This change does mean that
in certain cases like this one, #selectImports has container modifying
side effects. Documentation has been updated to reflect.

Issue: SPR-8411, SPR-8494
This commit is contained in:
Chris Beams
2011-07-11 01:17:19 +00:00
parent d8d9e3fc27
commit 431e935011
7 changed files with 121 additions and 26 deletions

View File

@@ -84,6 +84,8 @@ class ConfigurationClassParser {
private final ResourceLoader resourceLoader;
private final BeanDefinitionRegistry registry;
private final ComponentScanAnnotationParser componentScanParser;
@@ -98,8 +100,9 @@ class ConfigurationClassParser {
this.problemReporter = problemReporter;
this.environment = environment;
this.resourceLoader = resourceLoader;
this.componentScanParser = new ComponentScanAnnotationParser(this.resourceLoader, this.environment, registry);
this.registry = registry;
this.componentScanParser =
new ComponentScanAnnotationParser(this.resourceLoader, this.environment, this.registry);
}
@@ -245,7 +248,8 @@ class ConfigurationClassParser {
// the candidate class is an ImportSelector -> delegate to it to determine imports
try {
ImportSelector selector = BeanUtils.instantiateClass(Class.forName(candidate), ImportSelector.class);
processImport(configClass, selector.selectImports(importingClassMetadata), false);
ImportSelector.Context context = new ImportSelector.Context(importingClassMetadata, this.registry);
processImport(configClass, selector.selectImports(context), false);
} catch (ClassNotFoundException ex) {
throw new IllegalStateException(ex);
}

View File

@@ -16,6 +16,7 @@
package org.springframework.context.annotation;
import org.springframework.beans.factory.support.BeanDefinitionRegistry;
import org.springframework.core.type.AnnotationMetadata;
/**
@@ -23,6 +24,10 @@ import org.springframework.core.type.AnnotationMetadata;
* class(es) should be imported based on a given selection criteria, usually one or more
* annotation attributes.
*
* <p>In certain cases, an {@code ImportSelector} may register additional bean definitions
* through the {@link BeanDefinitionRegistry} available in the
* {@code Context} provided to the {@link #selectImports} method.
*
* @author Chris Beams
* @since 3.1
* @see Import
@@ -31,10 +36,41 @@ import org.springframework.core.type.AnnotationMetadata;
public interface ImportSelector {
/**
* Select and return the names of which class(es) should be imported.
* @param importingClassMetadata the AnnotationMetodata of the
* importing @{@link Configuration} class.
* Select and return the names of which class(es) should be imported based on
* the {@code AnnotationMetadata} of the importing {@code @Configuration} class and
* optionally register any {@code BeanDefinition}s necessary to support the selected
* classes.
* @param context containing the AnnotationMetadata of the importing @{@link
* Configuration} class and the enclosing {@link BeanDefinitionRegistry}.
*/
String[] selectImports(AnnotationMetadata importingClassMetadata);
String[] selectImports(Context context);
/**
* Context object holding the {@link AnnotationMetadata} of the {@code @Configuration}
* class that imported this {@link ImportSelector} as well as the enclosing
* {@link BeanDefinitionRegistry} to allow for conditional bean definition
* registration when necessary.
*
* @author Chris Beams
* @since 3.1
*/
static class Context {
private final AnnotationMetadata importingClassMetadata;
private final BeanDefinitionRegistry registry;
public Context(AnnotationMetadata importingClassMetadata, BeanDefinitionRegistry registry) {
this.importingClassMetadata = importingClassMetadata;
this.registry = registry;
}
public AnnotationMetadata getImportingClassMetadata() {
return this.importingClassMetadata;
}
public BeanDefinitionRegistry getBeanDefinitionRegistry() {
return registry;
}
}
}

View File

@@ -39,12 +39,15 @@ import org.springframework.util.Assert;
public class AsyncConfigurationSelector implements ImportSelector {
/**
* Import {@link ProxyAsyncConfiguration} if {@link EnableAsync#mode()} equals
* {@code PROXY}, otherwise import
* {@inheritDoc}
* <p>This implementation selects {@link ProxyAsyncConfiguration} if
* {@link EnableAsync#mode()} equals {@code PROXY}, and otherwise selects
* {@link org.springframework.scheduling.aspectj.AspectJAsyncConfiguration
* AspectJAsyncConfiguration}.
* AspectJAsyncConfiguration}. No additional {@code BeanDefinition}s are registered
* in either case.
*/
public String[] selectImports(AnnotationMetadata importingClassMetadata) {
public String[] selectImports(ImportSelector.Context context) {
AnnotationMetadata importingClassMetadata = context.getImportingClassMetadata();
Map<String, Object> enableAsync =
importingClassMetadata.getAnnotationAttributes(EnableAsync.class.getName());
Assert.notNull(enableAsync,

View File

@@ -22,6 +22,7 @@ import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
@@ -32,9 +33,14 @@ import org.junit.Test;
import org.springframework.aop.Advisor;
import org.springframework.aop.framework.Advised;
import org.springframework.aop.support.AopUtils;
import org.springframework.cache.Cache;
import org.springframework.cache.CacheManager;
import org.springframework.cache.concurrent.ConcurrentMapCache;
import org.springframework.cache.support.SimpleCacheManager;
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.ImportResource;
import org.springframework.context.config.AdviceMode;
import org.springframework.jdbc.datasource.DataSourceTransactionManager;
import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder;
@@ -152,6 +158,38 @@ public class EnableTransactionManagementIntegrationTests {
assertThat(txManager2.rollbacks, equalTo(0));
}
@Test
public void apcEscalation() {
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
ctx.register(EnableTxAndCachingConfig.class);
ctx.refresh();
}
@Configuration
@EnableTransactionManagement
@ImportResource("org/springframework/transaction/annotation/enable-caching.xml")
static class EnableTxAndCachingConfig {
@Bean
public PlatformTransactionManager txManager() {
return new CallCountingTransactionManager();
}
@Bean
public FooRepository fooRepository() {
return new DummyFooRepository();
}
@Bean
public CacheManager cacheManager() {
SimpleCacheManager mgr = new SimpleCacheManager();
ArrayList<Cache> caches = new ArrayList<Cache>();
caches.add(new ConcurrentMapCache());
mgr.setCaches(caches);
return mgr;
}
}
@Configuration
@EnableTransactionManagement

View File

@@ -0,0 +1,10 @@
<?xml version="1.0" encoding="UTF-8"?>
<beans xmlns="http://www.springframework.org/schema/beans"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:cache="http://www.springframework.org/schema/cache"
xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd
http://www.springframework.org/schema/cache http://www.springframework.org/schema/cache/spring-cache.xsd">
<cache:annotation-driven/>
</beans>

View File

@@ -16,8 +16,6 @@
package org.springframework.transaction.annotation;
import org.springframework.aop.config.AopConfigUtils;
import org.springframework.aop.framework.autoproxy.InfrastructureAdvisorAutoProxyCreator;
import org.springframework.beans.factory.config.BeanDefinition;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
@@ -67,12 +65,4 @@ public class ProxyTransactionManagementConfiguration extends AbstractTransaction
return interceptor;
}
// TODO: deal with escalation of APCs
@Bean(name=AopConfigUtils.AUTO_PROXY_CREATOR_BEAN_NAME)
@Role(BeanDefinition.ROLE_INFRASTRUCTURE)
public InfrastructureAdvisorAutoProxyCreator apc() {
InfrastructureAdvisorAutoProxyCreator apc = new InfrastructureAdvisorAutoProxyCreator();
apc.setProxyTargetClass((Boolean) this.enableTx.get("proxyTargetClass"));
return apc;
}
}

View File

@@ -18,6 +18,9 @@ package org.springframework.transaction.annotation;
import java.util.Map;
import org.springframework.aop.config.AopConfigUtils;
import org.springframework.beans.factory.support.BeanDefinitionRegistry;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.ImportSelector;
import org.springframework.context.config.AdviceMode;
import org.springframework.core.type.AnnotationMetadata;
@@ -38,12 +41,19 @@ import org.springframework.util.Assert;
public class TransactionManagementConfigurationSelector implements ImportSelector {
/**
* Import {@link ProxyTransactionManagementConfiguration} if {@link
* EnableTransactionManagement#mode()} equals {@code PROXY}, otherwise import {@link
* org.springframework.transaction.aspectj.AspectJTransactionManagementConfiguration
* {@inheritDoc}
* <p>This implementation selects {@link ProxyTransactionManagementConfiguration} if
* {@link EnableTransactionManagement#mode()} equals {@code PROXY}, and otherwise selects
* {@link org.springframework.transaction.aspectj.AspectJTransactionManagementConfiguration
* AspectJTransactionManagementConfiguration}.
* <p>If {@code #mode()} equals {@code PROXY}, an auto-proxy creator bean definition
* will also be added to the enclosing {@link BeanDefinitionRegistry} and escalated
* if necessary through the usual {@link AopConfigUtils} family of methods.
*/
public String[] selectImports(AnnotationMetadata importingClassMetadata) {
public String[] selectImports(ImportSelector.Context context) {
AnnotationMetadata importingClassMetadata = context.getImportingClassMetadata();
BeanDefinitionRegistry registry = context.getBeanDefinitionRegistry();
Map<String, Object> enableTx =
importingClassMetadata.getAnnotationAttributes(EnableTransactionManagement.class.getName());
Assert.notNull(enableTx,
@@ -52,7 +62,11 @@ public class TransactionManagementConfigurationSelector implements ImportSelecto
switch ((AdviceMode) enableTx.get("mode")) {
case PROXY:
return new String[] {ProxyTransactionManagementConfiguration.class.getName()};
AopConfigUtils.registerAutoProxyCreatorIfNecessary(registry);
if ((Boolean)enableTx.get("proxyTargetClass")) {
AopConfigUtils.forceAutoProxyCreatorToUseClassProxying(registry);
}
return new String[] { ProxyTransactionManagementConfiguration.class.getName() };
case ASPECTJ:
return new String[] {"org.springframework.transaction.aspectj.AspectJTransactionManagementConfiguration"};
default: