From 71c6eb2bb594f29803b6ec4a50de875def98be86 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 12 Aug 2014 17:23:21 +0200 Subject: [PATCH] Additional configuration classes get detected when imported through XML or registrars Issue: SPR-11430 Issue: SPR-11723 --- .../annotation/ConfigurationClassParser.java | 8 +- .../ConfigurationClassPostProcessor.java | 76 ++++++--- .../context/annotation/ImportAwareTests.java | 48 ++++-- .../configuration/ImportResourceTests.java | 154 ++++++++++-------- ...mportXmlWithConfigurationClass-context.xml | 8 + 5 files changed, 182 insertions(+), 112 deletions(-) create mode 100644 spring-context/src/test/java/org/springframework/context/annotation/configuration/ImportXmlWithConfigurationClass-context.xml 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 069de4f1b55..d299c1d6b56 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 @@ -113,6 +113,8 @@ class ConfigurationClassParser { private final ComponentScanAnnotationParser componentScanParser; + private final ConditionEvaluator conditionEvaluator; + private final Map configurationClasses = new LinkedHashMap(); @@ -125,8 +127,6 @@ class ConfigurationClassParser { private final List deferredImportSelectors = new LinkedList(); - private final ConditionEvaluator conditionEvaluator; - /** * Create a new {@link ConfigurationClassParser} instance that will be used @@ -488,6 +488,10 @@ class ConfigurationClassParser { return this.configurationClasses.keySet(); } + public int getPropertySourceCount() { + return this.propertySources.size(); + } + public List> getPropertySources() { List> propertySources = new LinkedList>(); for (Map.Entry> entry : this.propertySources.entrySet()) { diff --git a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassPostProcessor.java b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassPostProcessor.java index bb488888317..1cfe9931504 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassPostProcessor.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassPostProcessor.java @@ -17,6 +17,7 @@ package org.springframework.context.annotation; import java.beans.PropertyDescriptor; +import java.util.Arrays; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.LinkedHashSet; @@ -269,7 +270,9 @@ public class ConfigurationClassPostProcessor implements BeanDefinitionRegistryPo */ public void processConfigBeanDefinitions(BeanDefinitionRegistry registry) { Set configCandidates = new LinkedHashSet(); - for (String beanName : registry.getBeanDefinitionNames()) { + String[] candidateNames = registry.getBeanDefinitionNames(); + + for (String beanName : candidateNames) { BeanDefinition beanDef = registry.getBeanDefinition(beanName); if (ConfigurationClassUtils.isFullConfigurationClass(beanDef) || ConfigurationClassUtils.isLiteConfigurationClass(beanDef)) { @@ -302,32 +305,59 @@ public class ConfigurationClassPostProcessor implements BeanDefinitionRegistryPo ConfigurationClassParser parser = new ConfigurationClassParser( this.metadataReaderFactory, this.problemReporter, this.environment, this.resourceLoader, this.componentScanBeanNameGenerator, registry); - parser.parse(configCandidates); - parser.validate(); - - // Handle any @PropertySource annotations - List> parsedPropertySources = parser.getPropertySources(); - if (!parsedPropertySources.isEmpty()) { - if (!(this.environment instanceof ConfigurableEnvironment)) { - logger.warn("Ignoring @PropertySource annotations. " + - "Reason: Environment must implement ConfigurableEnvironment"); - } - else { - MutablePropertySources envPropertySources = ((ConfigurableEnvironment) this.environment).getPropertySources(); - for (PropertySource propertySource : parsedPropertySources) { - envPropertySources.addLast(propertySource); + + Set alreadyParsed = new HashSet(configCandidates.size()); + int propertySourceCount = 0; + do { + parser.parse(configCandidates); + parser.validate(); + + // Handle any @PropertySource annotations + if (parser.getPropertySourceCount() > propertySourceCount) { + List> parsedPropertySources = parser.getPropertySources(); + if (!parsedPropertySources.isEmpty()) { + if (!(this.environment instanceof ConfigurableEnvironment)) { + logger.warn("Ignoring @PropertySource annotations. " + + "Reason: Environment must implement ConfigurableEnvironment"); + } + else { + MutablePropertySources envPropertySources = ((ConfigurableEnvironment) this.environment).getPropertySources(); + for (PropertySource propertySource : parsedPropertySources) { + envPropertySources.addLast(propertySource); + } + } } + propertySourceCount = parser.getPropertySourceCount(); } - } - // Read the model and create bean definitions based on its content - if (this.reader == null) { - this.reader = new ConfigurationClassBeanDefinitionReader(registry, this.sourceExtractor, - this.problemReporter, this.metadataReaderFactory, this.resourceLoader, this.environment, - this.importBeanNameGenerator); - } + Set configClasses = new LinkedHashSet(parser.getConfigurationClasses()); + configClasses.removeAll(alreadyParsed); - this.reader.loadBeanDefinitions(parser.getConfigurationClasses()); + // Read the model and create bean definitions based on its content + if (this.reader == null) { + this.reader = new ConfigurationClassBeanDefinitionReader(registry, this.sourceExtractor, + this.problemReporter, this.metadataReaderFactory, this.resourceLoader, this.environment, + this.importBeanNameGenerator); + } + this.reader.loadBeanDefinitions(configClasses); + alreadyParsed.addAll(configClasses); + + configCandidates.clear(); + if (registry.getBeanDefinitionCount() > candidateNames.length) { + String[] newCandidateNames = registry.getBeanDefinitionNames(); + Set oldCandidateNames = new HashSet(Arrays.asList(candidateNames)); + for (String candidateName : newCandidateNames) { + if (!oldCandidateNames.contains(candidateName)) { + BeanDefinition beanDef = registry.getBeanDefinition(candidateName); + if (ConfigurationClassUtils.checkConfigurationClassCandidate(beanDef, this.metadataReaderFactory)) { + configCandidates.add(new BeanDefinitionHolder(beanDef, candidateName)); + } + } + } + candidateNames = newCandidateNames; + } + } + while (!configCandidates.isEmpty()); // Register the ImportRegistry as a bean in order to support ImportAware @Configuration classes if (singletonRegistry != null) { diff --git a/spring-context/src/test/java/org/springframework/context/annotation/ImportAwareTests.java b/spring-context/src/test/java/org/springframework/context/annotation/ImportAwareTests.java index 6f353c04934..77e2e883704 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/ImportAwareTests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/ImportAwareTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2014 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. @@ -22,10 +22,9 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; import org.junit.Test; -import org.springframework.beans.BeansException; + import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; -import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.BeanPostProcessor; import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.beans.factory.support.GenericBeanDefinition; @@ -42,6 +41,7 @@ import static org.junit.Assert.*; * annotation metadata of the @Configuration class that imported it. * * @author Chris Beams + * @author Juergen Hoeller * @since 3.1 */ public class ImportAwareTests { @@ -51,8 +51,7 @@ public class ImportAwareTests { AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); ctx.register(ImportingConfig.class); ctx.refresh(); - - ctx.getBean("importedConfigBean"); + assertNotNull(ctx.getBean("importedConfigBean")); ImportedConfig importAwareConfig = ctx.getBean(ImportedConfig.class); AnnotationMetadata importMetadata = importAwareConfig.importMetadata; @@ -68,8 +67,7 @@ public class ImportAwareTests { AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); ctx.register(IndirectlyImportingConfig.class); ctx.refresh(); - - ctx.getBean("importedConfigBean"); + assertNotNull(ctx.getBean("importedConfigBean")); ImportedConfig importAwareConfig = ctx.getBean(ImportedConfig.class); AnnotationMetadata importMetadata = importAwareConfig.importMetadata; @@ -87,6 +85,7 @@ public class ImportAwareTests { ctx.register(ImportingRegistrarConfig.class); ctx.refresh(); assertNotNull(ctx.getBean("registrarImportedBean")); + assertNotNull(ctx.getBean("otherImportedConfigBean")); } @Test @@ -96,9 +95,12 @@ public class ImportAwareTests { ctx.register(ImportingRegistrarConfigWithImport.class); ctx.refresh(); assertNotNull(ctx.getBean("registrarImportedBean")); + assertNotNull(ctx.getBean("otherImportedConfigBean")); + assertNotNull(ctx.getBean("importedConfigBean")); assertNotNull(ctx.getBean(ImportedConfig.class)); } + @Configuration @Import(ImportedConfig.class) static class ImportingConfig { @@ -140,28 +142,40 @@ public class ImportAwareTests { } + @Configuration + static class OtherImportedConfig { + + @Bean + public String otherImportedConfigBean() { + return ""; + } + } + + static class BPP implements BeanFactoryAware, BeanPostProcessor { @Override - public Object postProcessBeforeInitialization(Object bean, String beanName) throws BeansException { + public Object postProcessBeforeInitialization(Object bean, String beanName) { return bean; } @Override - public Object postProcessAfterInitialization(Object bean, String beanName) throws BeansException { + public Object postProcessAfterInitialization(Object bean, String beanName) { return bean; } @Override - public void setBeanFactory(BeanFactory beanFactory) throws BeansException { + public void setBeanFactory(BeanFactory beanFactory) { } } + @Configuration @EnableImportRegistrar static class ImportingRegistrarConfig { } + @Configuration @EnableImportRegistrar @Import(ImportedConfig.class) @@ -174,18 +188,22 @@ public class ImportAwareTests { public @interface EnableImportRegistrar { } + static class ImportedRegistrar implements ImportBeanDefinitionRegistrar { static boolean called; @Override - public void registerBeanDefinitions(AnnotationMetadata importingClassMetadata, - BeanDefinitionRegistry registry) { - BeanDefinition beanDefinition = new GenericBeanDefinition(); + public void registerBeanDefinitions(AnnotationMetadata importingClassMetadata, BeanDefinitionRegistry registry) { + GenericBeanDefinition beanDefinition = new GenericBeanDefinition(); beanDefinition.setBeanClassName(String.class.getName()); - registry.registerBeanDefinition("registrarImportedBean", beanDefinition ); - Assert.state(called == false, "ImportedRegistrar called twice"); + registry.registerBeanDefinition("registrarImportedBean", beanDefinition); + GenericBeanDefinition beanDefinition2 = new GenericBeanDefinition(); + beanDefinition2.setBeanClass(OtherImportedConfig.class); + registry.registerBeanDefinition("registrarImportedConfig", beanDefinition2); + Assert.state(!called, "ImportedRegistrar called twice"); called = true; } } + } diff --git a/spring-context/src/test/java/org/springframework/context/annotation/configuration/ImportResourceTests.java b/spring-context/src/test/java/org/springframework/context/annotation/configuration/ImportResourceTests.java index ae055bc0cf8..bc0f4966f95 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/configuration/ImportResourceTests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/configuration/ImportResourceTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2014 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. @@ -20,13 +20,9 @@ import java.util.Collections; import org.aspectj.lang.annotation.Aspect; import org.aspectj.lang.annotation.Before; - -import static org.hamcrest.CoreMatchers.*; -import static org.junit.Assert.*; - import org.junit.Ignore; import org.junit.Test; -import org.springframework.tests.sample.beans.TestBean; + import org.springframework.aop.support.AopUtils; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; @@ -39,6 +35,10 @@ import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.ImportResource; import org.springframework.core.env.MapPropertySource; import org.springframework.core.env.PropertySource; +import org.springframework.tests.sample.beans.TestBean; + +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; /** * Integration tests for {@link ImportResource} support. @@ -47,6 +47,7 @@ import org.springframework.core.env.PropertySource; * @author Juergen Hoeller */ public class ImportResourceTests { + @Test public void testImportXml() { AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(ImportXmlConfig.class); @@ -56,16 +57,6 @@ public class ImportResourceTests { assertEquals("myName", tb.getName()); } - @Configuration - @ImportResource("classpath:org/springframework/context/annotation/configuration/ImportXmlConfig-context.xml") - static class ImportXmlConfig { - @Value("${name}") - private String name; - public @Bean TestBean javaDeclaredBean() { - return new TestBean(this.name); - } - } - @Ignore // TODO: SPR-6310 @Test public void testImportXmlWithRelativePath() { @@ -76,14 +67,6 @@ public class ImportResourceTests { assertEquals("myName", tb.getName()); } - @Configuration - @ImportResource("ImportXmlConfig-context.xml") - static class ImportXmlWithRelativePathConfig { - public @Bean TestBean javaDeclaredBean() { - return new TestBean("java.declared"); - } - } - @Ignore // TODO: SPR-6310 @Test public void testImportXmlByConvention() { @@ -91,11 +74,6 @@ public class ImportResourceTests { assertTrue("context does not contain xml-declared bean", ctx.containsBean("xmlDeclaredBean")); } - @Configuration - //@ImportXml - static class ImportXmlByConventionConfig { - } - @Test public void testImportXmlIsInheritedFromSuperclassDeclarations() { AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(FirstLevelSubConfig.class); @@ -109,6 +87,78 @@ public class ImportResourceTests { assertTrue("failed to pick up parent-declared XML bean", ctx.containsBean("xmlDeclaredBean")); } + @Test + public void testImportXmlWithNamespaceConfig() { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(ImportXmlWithAopNamespaceConfig.class); + Object bean = ctx.getBean("proxiedXmlBean"); + assertTrue(AopUtils.isAopProxy(bean)); + } + + @Test + public void testImportXmlWithOtherConfigurationClass() { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(ImportXmlWithConfigurationClass.class); + assertTrue("did not contain java-declared bean", ctx.containsBean("javaDeclaredBean")); + assertTrue("did not contain xml-declared bean", ctx.containsBean("xmlDeclaredBean")); + TestBean tb = ctx.getBean("javaDeclaredBean", TestBean.class); + assertEquals("myName", tb.getName()); + } + + @Ignore // TODO: SPR-6327 + @Test + public void testImportDifferentResourceTypes() { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(SubResourceConfig.class); + assertTrue(ctx.containsBean("propertiesDeclaredBean")); + assertTrue(ctx.containsBean("xmlDeclaredBean")); + } + + @Test + public void importWithPlaceholder() throws Exception { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); + PropertySource propertySource = new MapPropertySource("test", + Collections. singletonMap("test", "springframework")); + ctx.getEnvironment().getPropertySources().addFirst(propertySource); + ctx.register(ImportXmlConfig.class); + ctx.refresh(); + assertTrue("did not contain xml-declared bean", ctx.containsBean("xmlDeclaredBean")); + } + + @Test + public void testImportXmlWithAutowiredConfig() { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(ImportXmlAutowiredConfig.class); + String name = ctx.getBean("xmlBeanName", String.class); + assertThat(name, equalTo("xml.declared")); + } + + @Test + public void testImportNonXmlResource() { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(ImportNonXmlResourceConfig.class); + assertTrue(ctx.containsBean("propertiesDeclaredBean")); + } + + + @Configuration + @ImportResource("classpath:org/springframework/context/annotation/configuration/ImportXmlConfig-context.xml") + static class ImportXmlConfig { + @Value("${name}") + private String name; + public @Bean TestBean javaDeclaredBean() { + return new TestBean(this.name); + } + } + + @Configuration + @ImportResource("ImportXmlConfig-context.xml") + static class ImportXmlWithRelativePathConfig { + public @Bean TestBean javaDeclaredBean() { + return new TestBean("java.declared"); + } + } + + @Configuration + //@ImportXml + static class ImportXmlByConventionConfig { + } + @Configuration @ImportResource("classpath:org/springframework/context/annotation/configuration/ImportXmlConfig-context.xml") static class BaseConfig { @@ -123,13 +173,6 @@ public class ImportResourceTests { static class SecondLevelSubConfig extends BaseConfig { } - @Test - public void testImportXmlWithNamespaceConfig() { - AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(ImportXmlWithAopNamespaceConfig.class); - Object bean = ctx.getBean("proxiedXmlBean"); - assertTrue(AopUtils.isAopProxy(bean)); - } - @Configuration @ImportResource("classpath:org/springframework/context/annotation/configuration/ImportXmlWithAopNamespace-context.xml") static class ImportXmlWithAopNamespaceConfig { @@ -141,11 +184,9 @@ public class ImportResourceTests { public void advice() { } } - @Test - public void testImportXmlWithAutowiredConfig() { - AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(ImportXmlAutowiredConfig.class); - String name = ctx.getBean("xmlBeanName", String.class); - assertThat(name, equalTo("xml.declared")); + @Configuration + @ImportResource("classpath:org/springframework/context/annotation/configuration/ImportXmlWithConfigurationClass-context.xml") + static class ImportXmlWithConfigurationClass { } @Configuration @@ -158,47 +199,16 @@ public class ImportResourceTests { } } - @Test - public void testImportNonXmlResource() { - AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(ImportNonXmlResourceConfig.class); - assertTrue(ctx.containsBean("propertiesDeclaredBean")); - } - @Configuration @ImportResource(value="classpath:org/springframework/context/annotation/configuration/ImportNonXmlResourceConfig-context.properties", reader=PropertiesBeanDefinitionReader.class) static class ImportNonXmlResourceConfig { } - @Ignore // TODO: SPR-6327 - @Test - public void testImportDifferentResourceTypes() { - AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(SubResourceConfig.class); - assertTrue(ctx.containsBean("propertiesDeclaredBean")); - assertTrue(ctx.containsBean("xmlDeclaredBean")); - } - @Configuration @ImportResource(value="classpath:org/springframework/context/annotation/configuration/ImportXmlConfig-context.xml", reader=XmlBeanDefinitionReader.class) static class SubResourceConfig extends ImportNonXmlResourceConfig { } - @Test - public void importWithPlaceHolder() throws Exception { - AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); - PropertySource propertySource = new MapPropertySource("test", - Collections. singletonMap("test", "springframework")); - ctx.getEnvironment().getPropertySources().addFirst(propertySource); - ctx.register(ImportXmlConfig.class); - ctx.refresh(); - assertTrue("did not contain xml-declared bean", ctx.containsBean("xmlDeclaredBean")); - } - - @Configuration - @ImportResource("classpath:org/${test}/context/annotation/configuration/ImportXmlConfig-context.xml") - static class ImportWithPlaceHolder { - } - - } diff --git a/spring-context/src/test/java/org/springframework/context/annotation/configuration/ImportXmlWithConfigurationClass-context.xml b/spring-context/src/test/java/org/springframework/context/annotation/configuration/ImportXmlWithConfigurationClass-context.xml new file mode 100644 index 00000000000..284425983a1 --- /dev/null +++ b/spring-context/src/test/java/org/springframework/context/annotation/configuration/ImportXmlWithConfigurationClass-context.xml @@ -0,0 +1,8 @@ + + + + + +