From e81df2ef3e6b2359de1970f2b0b5df0075abc554 Mon Sep 17 00:00:00 2001 From: Chris Beams Date: Wed, 15 Feb 2012 10:39:16 +0100 Subject: [PATCH] Improve @Configuration bean name discovery Prior to this commit, and based on earlier changes supporting SPR-9023, ConfigurationClassBeanDefinitionReader employed a simplistic strategy for extracting the 'value' attribute (if any) from @Configuration in order to determine the bean name for imported and nested configuration classes. An example case follows: @Configuration("myConfig") public class AppConfig { ... } This approach is too simplistic however, given that it is possible in 'configuration lite' mode to specify a @Component-annotated class with @Bean methods, e.g. @Component("myConfig") public class AppConfig { @Bean public Foo foo() { ... } } In this case, it's the 'value' attribute of @Component, not @Configuration, that should be consulted for the bean name. Or indeed if it were any other stereotype annotation meta-annotated with @Component, the value attribute should respected. This kind of sophisticated discovery is exactly what AnnotationBeanNameGenerator was designed to do, and ConfigurationClassBeanDefinitionReader now uses it in favor of the custom approach described above. To enable this refactoring, nested and imported configuration classes are no longer registered as GenericBeanDefinition, but rather as AnnotatedGenericBeanDefinition given that AnnotationBeanNameGenerator falls back to a generic strategy unless the bean definition in question is assignable to AnnotatedBeanDefinition. A new constructor accepting AnnotationMetadata has been added to AnnotatedGenericBeanDefinition in order to support the ASM-based approach in use by configuration class processing. Javadoc has been updated for both AnnotatedGenericBeanDefinition and its now very similar cousin ScannedGenericBeanDefinition to make clear the semantics and intention of these two variants. Issue: SPR-9023 --- .../AnnotatedGenericBeanDefinition.java | 30 ++++++-- ...onfigurationClassBeanDefinitionReader.java | 39 +++------- .../ScannedGenericBeanDefinition.java | 13 +++- .../ConfigurationBeanNameTests.java | 76 +++++++++++++++++++ 4 files changed, 121 insertions(+), 37 deletions(-) create mode 100644 org.springframework.context/src/test/java/org/springframework/context/annotation/configuration/ConfigurationBeanNameTests.java diff --git a/org.springframework.beans/src/main/java/org/springframework/beans/factory/annotation/AnnotatedGenericBeanDefinition.java b/org.springframework.beans/src/main/java/org/springframework/beans/factory/annotation/AnnotatedGenericBeanDefinition.java index 5f4ee7f555..f0fd4c5cac 100644 --- a/org.springframework.beans/src/main/java/org/springframework/beans/factory/annotation/AnnotatedGenericBeanDefinition.java +++ b/org.springframework.beans/src/main/java/org/springframework/beans/factory/annotation/AnnotatedGenericBeanDefinition.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2008 the original author or authors. + * Copyright 2002-2012 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. @@ -19,6 +19,7 @@ package org.springframework.beans.factory.annotation; import org.springframework.beans.factory.support.GenericBeanDefinition; import org.springframework.core.type.AnnotationMetadata; import org.springframework.core.type.StandardAnnotationMetadata; +import org.springframework.util.Assert; /** * Extension of the {@link org.springframework.beans.factory.support.GenericBeanDefinition} @@ -32,27 +33,46 @@ import org.springframework.core.type.StandardAnnotationMetadata; * which also implements the AnnotatedBeanDefinition interface). * * @author Juergen Hoeller + * @author Chris Beams * @since 2.5 * @see AnnotatedBeanDefinition#getMetadata() * @see org.springframework.core.type.StandardAnnotationMetadata */ +@SuppressWarnings("serial") public class AnnotatedGenericBeanDefinition extends GenericBeanDefinition implements AnnotatedBeanDefinition { - private final AnnotationMetadata annotationMetadata; + private final AnnotationMetadata metadata; /** * Create a new AnnotatedGenericBeanDefinition for the given bean class. * @param beanClass the loaded bean class */ - public AnnotatedGenericBeanDefinition(Class beanClass) { + public AnnotatedGenericBeanDefinition(Class beanClass) { setBeanClass(beanClass); - this.annotationMetadata = new StandardAnnotationMetadata(beanClass, true); + this.metadata = new StandardAnnotationMetadata(beanClass, true); + } + + /** + * Create a new AnnotatedGenericBeanDefinition for the given annotation metadata, + * allowing for ASM-based processing and avoidance of early loading of the bean class. + * Note that this constructor is functionally equivalent to + * {@link org.springframework.context.annotation.ScannedGenericBeanDefinition + * ScannedGenericBeanDefinition}, however the semantics of the latter indicate that + * a bean was discovered specifically via component-scanning as opposed to other + * means. + * @param metadata the annotation metadata for the bean class in question + * @since 3.1.1 + */ + public AnnotatedGenericBeanDefinition(AnnotationMetadata metadata) { + Assert.notNull(metadata, "AnnotationMetadata must not be null"); + setBeanClassName(metadata.getClassName()); + this.metadata = metadata; } public final AnnotationMetadata getMetadata() { - return this.annotationMetadata; + return this.metadata; } } diff --git a/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java b/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java index ffa0e08d82..23dcbe6ef0 100644 --- a/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java +++ b/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java @@ -16,7 +16,6 @@ package org.springframework.context.annotation; -import java.io.IOException; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Arrays; @@ -29,6 +28,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.annotation.AnnotatedBeanDefinition; +import org.springframework.beans.factory.annotation.AnnotatedGenericBeanDefinition; import org.springframework.beans.factory.annotation.Autowire; import org.springframework.beans.factory.annotation.RequiredAnnotationBeanPostProcessor; import org.springframework.beans.factory.config.BeanDefinition; @@ -37,12 +37,10 @@ import org.springframework.beans.factory.parsing.Location; import org.springframework.beans.factory.parsing.Problem; import org.springframework.beans.factory.parsing.ProblemReporter; import org.springframework.beans.factory.parsing.SourceExtractor; -import org.springframework.beans.factory.support.AbstractBeanDefinition; import org.springframework.beans.factory.support.AbstractBeanDefinitionReader; import org.springframework.beans.factory.support.BeanDefinitionReader; -import org.springframework.beans.factory.support.BeanDefinitionReaderUtils; import org.springframework.beans.factory.support.BeanDefinitionRegistry; -import org.springframework.beans.factory.support.GenericBeanDefinition; +import org.springframework.beans.factory.support.BeanNameGenerator; import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.core.annotation.AnnotationAttributes; import org.springframework.core.env.Environment; @@ -50,7 +48,6 @@ import org.springframework.core.io.Resource; import org.springframework.core.io.ResourceLoader; import org.springframework.core.type.AnnotationMetadata; import org.springframework.core.type.MethodMetadata; -import org.springframework.core.type.classreading.MetadataReader; import org.springframework.core.type.classreading.MetadataReaderFactory; import org.springframework.util.StringUtils; @@ -85,6 +82,8 @@ class ConfigurationClassBeanDefinitionReader { private final Environment environment; + private BeanNameGenerator beanNameGenerator = new AnnotationBeanNameGenerator(); + /** * Create a new {@link ConfigurationClassBeanDefinitionReader} instance that will be used @@ -135,39 +134,21 @@ class ConfigurationClassBeanDefinitionReader { return; } - BeanDefinition configBeanDef = new GenericBeanDefinition(); - String className = configClass.getMetadata().getClassName(); + AnnotationMetadata metadata = configClass.getMetadata(); + BeanDefinition configBeanDef = new AnnotatedGenericBeanDefinition(metadata); + String className = metadata.getClassName(); configBeanDef.setBeanClassName(className); - MetadataReader reader; - try { - reader = this.metadataReaderFactory.getMetadataReader(className); - } - catch (IOException ex) { - throw new IllegalStateException("Could not create MetadataReader for class " + className); - } if (ConfigurationClassUtils.checkConfigurationClassCandidate(configBeanDef, this.metadataReaderFactory)) { - Map configAttributes = - reader.getAnnotationMetadata().getAnnotationAttributes(Configuration.class.getName()); - - // has the 'value' attribute of @Configuration been set? - String configBeanName = (String) configAttributes.get("value"); - if (StringUtils.hasText(configBeanName)) { - // yes -> register the configuration class bean with this name - this.registry.registerBeanDefinition(configBeanName, configBeanDef); - } - else { - // no -> register the configuration class bean with a generated name - configBeanName = BeanDefinitionReaderUtils.registerWithGeneratedName((AbstractBeanDefinition)configBeanDef, this.registry); - } + String configBeanName = this.beanNameGenerator.generateBeanName(configBeanDef, this.registry); + this.registry.registerBeanDefinition(configBeanName, configBeanDef); configClass.setBeanName(configBeanName); if (logger.isDebugEnabled()) { logger.debug(String.format("Registered bean definition for imported @Configuration class %s", configBeanName)); } } else { - AnnotationMetadata metadata = reader.getAnnotationMetadata(); this.problemReporter.error( - new InvalidConfigurationImportProblem(className, reader.getResource(), metadata)); + new InvalidConfigurationImportProblem(className, configClass.getResource(), metadata)); } } diff --git a/org.springframework.context/src/main/java/org/springframework/context/annotation/ScannedGenericBeanDefinition.java b/org.springframework.context/src/main/java/org/springframework/context/annotation/ScannedGenericBeanDefinition.java index 22d918c887..b5afe9f6de 100644 --- a/org.springframework.context/src/main/java/org/springframework/context/annotation/ScannedGenericBeanDefinition.java +++ b/org.springframework.context/src/main/java/org/springframework/context/annotation/ScannedGenericBeanDefinition.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2007 the original author or authors. + * Copyright 2002-2012 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. @@ -17,6 +17,7 @@ package org.springframework.context.annotation; import org.springframework.beans.factory.annotation.AnnotatedBeanDefinition; +import org.springframework.beans.factory.annotation.AnnotatedGenericBeanDefinition; import org.springframework.beans.factory.support.GenericBeanDefinition; import org.springframework.core.type.AnnotationMetadata; import org.springframework.core.type.classreading.MetadataReader; @@ -27,16 +28,22 @@ import org.springframework.util.Assert; * class, based on an ASM ClassReader, with support for annotation metadata exposed * through the {@link AnnotatedBeanDefinition} interface. * - *

This class does not load the bean Class early. + *

This class does not load the bean {@code Class} early. * It rather retrieves all relevant metadata from the ".class" file itself, - * parsed with the ASM ClassReader. + * parsed with the ASM ClassReader. It is functionally equivalent to + * {@link AnnotatedGenericBeanDefinition#AnnotatedGenericBeanDefinition(AnnotationMetadata)} + * but distinguishes by type beans that have been scanned vs those that have + * been otherwise registered or detected by other means. * * @author Juergen Hoeller + * @author Chris Beams * @since 2.5 * @see #getMetadata() * @see #getBeanClassName() * @see org.springframework.core.type.classreading.MetadataReaderFactory + * @see AnnotatedGenericBeanDefinition */ +@SuppressWarnings("serial") public class ScannedGenericBeanDefinition extends GenericBeanDefinition implements AnnotatedBeanDefinition { private final AnnotationMetadata metadata; diff --git a/org.springframework.context/src/test/java/org/springframework/context/annotation/configuration/ConfigurationBeanNameTests.java b/org.springframework.context/src/test/java/org/springframework/context/annotation/configuration/ConfigurationBeanNameTests.java new file mode 100644 index 0000000000..799e55a3b6 --- /dev/null +++ b/org.springframework.context/src/test/java/org/springframework/context/annotation/configuration/ConfigurationBeanNameTests.java @@ -0,0 +1,76 @@ +/* + * Copyright 2002-2012 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.context.annotation.configuration; + +import org.junit.Test; +import org.springframework.beans.factory.config.BeanDefinition; +import org.springframework.beans.factory.support.BeanDefinitionRegistry; +import org.springframework.context.annotation.AnnotationBeanNameGenerator; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; +import org.springframework.stereotype.Component; + +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; + +/** + * Unit tests ensuring that configuration class bean names as expressed via @Configuration + * or @Component 'value' attributes are indeed respected + * + * @author Chris Beams + * @since 3.1.1 + */ +public class ConfigurationBeanNameTests { + + @Test + public void registerOuterConfig() { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); + ctx.register(A.class); + ctx.refresh(); + assertThat(ctx.containsBean("outer"), is(true)); + assertThat(ctx.containsBean("imported"), is(true)); + assertThat(ctx.containsBean("nested"), is(true)); + assertThat(ctx.containsBean("nestedBean"), is(true)); + } + + @Test + public void registerNestedConfig() { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); + ctx.register(A.B.class); + ctx.refresh(); + assertThat(ctx.containsBean("outer"), is(false)); + assertThat(ctx.containsBean("imported"), is(false)); + assertThat(ctx.containsBean("nested"), is(true)); + assertThat(ctx.containsBean("nestedBean"), is(true)); + } + + @Configuration("outer") + @Import(C.class) + static class A { + @Component("nested") + static class B { + @Bean public String nestedBean() { return ""; } + } + } + + @Configuration("imported") + static class C { + @Bean public String s() { return "s"; } + } +}