From a7ec6dc0afb5ad83371ed073a26a31cb9e1dd4f1 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 9 Dec 2016 15:01:21 +0100 Subject: [PATCH] ImportRegistry properly tracks excluded superclasses Issue: SPR-14972 --- ...onfigurationClassBeanDefinitionReader.java | 2 +- .../annotation/ConfigurationClassParser.java | 17 +++-- .../annotation/ConfigurationCondition.java | 4 +- .../context/annotation/ImportRegistry.java | 4 +- .../EnableTransactionManagementTests.java | 75 +++++++++++++++++-- 5 files changed, 81 insertions(+), 21 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java index ea437e85fa..683745db15 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java @@ -129,7 +129,7 @@ class ConfigurationClassBeanDefinitionReader { if (StringUtils.hasLength(beanName) && this.registry.containsBeanDefinition(beanName)) { this.registry.removeBeanDefinition(beanName); } - this.importRegistry.removeImportingClassFor(configClass.getMetadata().getClassName()); + this.importRegistry.removeImportingClass(configClass.getMetadata().getClassName()); return; } diff --git a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java index 2cb2fd4c49..51242ad237 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java @@ -644,22 +644,23 @@ class ConfigurationClassParser { } @Override - public void removeImportingClassFor(String importedClass) { + public AnnotationMetadata getImportingClassFor(String importedClass) { + List list = this.imports.get(importedClass); + return (!CollectionUtils.isEmpty(list) ? list.get(list.size() - 1) : null); + } + + @Override + public void removeImportingClass(String importingClass) { for (List list : this.imports.values()) { for (Iterator iterator = list.iterator(); iterator.hasNext();) { - if (iterator.next().getClassName().equals(importedClass)) { + if (iterator.next().getClassName().equals(importingClass)) { iterator.remove(); + break; } } } } - @Override - public AnnotationMetadata getImportingClassFor(String importedClass) { - List list = this.imports.get(importedClass); - return (!CollectionUtils.isEmpty(list) ? list.get(list.size() - 1) : null); - } - /** * Given a stack containing (in order) *
    diff --git a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationCondition.java b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationCondition.java index 8bd3d044b3..c46f8a030b 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationCondition.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationCondition.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2016 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. @@ -38,7 +38,7 @@ public interface ConfigurationCondition extends Condition { /** * The various configuration phases where the condition could be evaluated. */ - public static enum ConfigurationPhase { + enum ConfigurationPhase { /** * The {@link Condition} should be evaluated as a {@code @Configuration} diff --git a/spring-context/src/main/java/org/springframework/context/annotation/ImportRegistry.java b/spring-context/src/main/java/org/springframework/context/annotation/ImportRegistry.java index 2d20ab17d7..c614d55d8d 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/ImportRegistry.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/ImportRegistry.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2016 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. @@ -26,6 +26,6 @@ interface ImportRegistry { AnnotationMetadata getImportingClassFor(String importedClass); - void removeImportingClassFor(String importedClass); + void removeImportingClass(String importingClass); } diff --git a/spring-tx/src/test/java/org/springframework/transaction/annotation/EnableTransactionManagementTests.java b/spring-tx/src/test/java/org/springframework/transaction/annotation/EnableTransactionManagementTests.java index 5d47d8533c..ed4e127dba 100644 --- a/spring-tx/src/test/java/org/springframework/transaction/annotation/EnableTransactionManagementTests.java +++ b/spring-tx/src/test/java/org/springframework/transaction/annotation/EnableTransactionManagementTests.java @@ -26,7 +26,11 @@ import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.AdviceMode; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.ConditionContext; +import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.ConfigurationCondition; +import org.springframework.core.type.AnnotatedTypeMetadata; import org.springframework.stereotype.Service; import org.springframework.tests.transaction.CallCountingTransactionManager; import org.springframework.transaction.PlatformTransactionManager; @@ -49,7 +53,8 @@ public class EnableTransactionManagementTests { @Test public void transactionProxyIsCreated() { - AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(EnableTxConfig.class, TxManagerConfig.class); + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext( + EnableTxConfig.class, TxManagerConfig.class); TransactionalTestBean bean = ctx.getBean(TransactionalTestBean.class); assertTrue("testBean is not a proxy", AopUtils.isAopProxy(bean)); Map services = ctx.getBeansWithAnnotation(Service.class); @@ -59,7 +64,19 @@ public class EnableTransactionManagementTests { @Test public void transactionProxyIsCreatedWithEnableOnSuperclass() { - AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(InheritedEnableTxConfig.class, TxManagerConfig.class); + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext( + InheritedEnableTxConfig.class, TxManagerConfig.class); + TransactionalTestBean bean = ctx.getBean(TransactionalTestBean.class); + assertTrue("testBean is not a proxy", AopUtils.isAopProxy(bean)); + Map services = ctx.getBeansWithAnnotation(Service.class); + assertTrue("Stereotype annotation not visible", services.containsKey("testBean")); + ctx.close(); + } + + @Test + public void transactionProxyIsCreatedWithEnableOnExcludedSuperclass() { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext( + ParentEnableTxConfig.class, ChildEnableTxConfig.class, TxManagerConfig.class); TransactionalTestBean bean = ctx.getBean(TransactionalTestBean.class); assertTrue("testBean is not a proxy", AopUtils.isAopProxy(bean)); Map services = ctx.getBeansWithAnnotation(Service.class); @@ -69,7 +86,8 @@ public class EnableTransactionManagementTests { @Test public void txManagerIsResolvedOnInvocationOfTransactionalMethod() { - AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(EnableTxConfig.class, TxManagerConfig.class); + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext( + EnableTxConfig.class, TxManagerConfig.class); TransactionalTestBean bean = ctx.getBean(TransactionalTestBean.class); // invoke a transactional method, causing the PlatformTransactionManager bean to be resolved. @@ -79,7 +97,8 @@ public class EnableTransactionManagementTests { @Test public void txManagerIsResolvedCorrectlyWhenMultipleManagersArePresent() { - AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(EnableTxConfig.class, MultiTxManagerConfig.class); + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext( + EnableTxConfig.class, MultiTxManagerConfig.class); TransactionalTestBean bean = ctx.getBean(TransactionalTestBean.class); // invoke a transactional method, causing the PlatformTransactionManager bean to be resolved. @@ -95,7 +114,7 @@ public class EnableTransactionManagementTests { @SuppressWarnings("resource") public void proxyTypeAspectJCausesRegistrationOfAnnotationTransactionAspect() { try { - new AnnotationConfigApplicationContext(EnableAspectJTxConfig.class, TxManagerConfig.class); + new AnnotationConfigApplicationContext(EnableAspectjTxConfig.class, TxManagerConfig.class); fail("should have thrown CNFE when trying to load AnnotationTransactionAspect. " + "Do you actually have org.springframework.aspects on the classpath?"); } @@ -137,15 +156,54 @@ public class EnableTransactionManagementTests { static class EnableTxConfig { } + @Configuration static class InheritedEnableTxConfig extends EnableTxConfig { } + + @Configuration + @EnableTransactionManagement + @Conditional(NeverCondition.class) + static class ParentEnableTxConfig { + + @Bean + Object someBean() { + return new Object(); + } + } + + + @Configuration + static class ChildEnableTxConfig extends ParentEnableTxConfig { + + @Override + Object someBean() { + return "X"; + } + } + + + private static class NeverCondition implements ConfigurationCondition { + + @Override + public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) { + return false; + } + + @Override + public ConfigurationPhase getConfigurationPhase() { + return ConfigurationPhase.REGISTER_BEAN; + } + } + + @Configuration - @EnableTransactionManagement(mode=AdviceMode.ASPECTJ) - static class EnableAspectJTxConfig { + @EnableTransactionManagement(mode = AdviceMode.ASPECTJ) + static class EnableAspectjTxConfig { } + @Configuration @EnableTransactionManagement static class Spr11915Config { @@ -162,6 +220,7 @@ public class EnableTransactionManagementTests { } } + @Configuration static class TxManagerConfig { @@ -174,9 +233,9 @@ public class EnableTransactionManagementTests { public PlatformTransactionManager txManager() { return new CallCountingTransactionManager(); } - } + @Configuration static class MultiTxManagerConfig extends TxManagerConfig implements TransactionManagementConfigurer {