ImportRegistry properly tracks excluded superclasses

Issue: SPR-14972 (cherry picked from commit a7ec6dc)
spring-projects · Dec 18, 2016 · dd3c370 · dd3c370
1 parent 4e41f74
commit dd3c370
Show file tree

Hide file tree

Showing 5 changed files with 81 additions and 21 deletions.
diff --git a/...n/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java b/...n/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java
@@ -129,7 +129,7 @@ private void loadBeanDefinitionsForConfigurationClass(ConfigurationClass configC
 			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/...ontext/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java b/...ontext/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java
@@ -644,22 +644,23 @@ public void registerImport(AnnotationMetadata importingClass, String importedCla
 		}
 
 		@Override
-		public void removeImportingClassFor(String importedClass) {
+		public AnnotationMetadata getImportingClassFor(String importedClass) {
+			List<AnnotationMetadata> list = this.imports.get(importedClass);
+			return (!CollectionUtils.isEmpty(list) ? list.get(list.size() - 1) : null);
+		}
+
+		@Override
+		public void removeImportingClass(String importingClass) {
 			for (List<AnnotationMetadata> list : this.imports.values()) {
 				for (Iterator<AnnotationMetadata> 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<AnnotationMetadata> list = this.imports.get(importedClass);
-			return (!CollectionUtils.isEmpty(list) ? list.get(list.size() - 1) : null);
-		}
-
 		/**
 		 * Given a stack containing (in order)
 		 * <ul>

diff --git a/...-context/src/main/java/org/springframework/context/annotation/ConfigurationCondition.java b/...-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
@@ -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/...est/java/org/springframework/transaction/annotation/EnableTransactionManagementTests.java b/...est/java/org/springframework/transaction/annotation/EnableTransactionManagementTests.java
@@ -28,7 +28,11 @@
 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;
@@ -51,7 +55,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);
@@ -61,7 +66,19 @@ public void transactionProxyIsCreated() {
 
 	@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);
@@ -71,7 +88,8 @@ public void transactionProxyIsCreatedWithEnableOnSuperclass() {
 
 	@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.
@@ -81,7 +99,8 @@ public void txManagerIsResolvedOnInvocationOfTransactionalMethod() {
 
 	@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.
@@ -97,7 +116,7 @@ public void txManagerIsResolvedCorrectlyWhenMultipleManagersArePresent() {
 	@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?");
 		}
@@ -139,15 +158,54 @@ public void spr11915() {
 	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 {
@@ -167,6 +225,7 @@ public TransactionalTestBean testBean() {
 		}
 	}
 
+
 	@Configuration
 	static class TxManagerConfig {
 
@@ -179,9 +238,9 @@ public TransactionalTestBean testBean() {
 		public PlatformTransactionManager txManager() {
 			return new CallCountingTransactionManager();
 		}
-
 	}
 
+
 	@Configuration
 	static class MultiTxManagerConfig extends TxManagerConfig implements TransactionManagementConfigurer {