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

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;
 	}
 
 }

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<String, Object> 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));
 		}
 	}
 

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.
  *
- * <p>This class does <i>not</i> load the bean <code>Class</code> early.
+ * <p>This class does <i>not</i> 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 <em>scanned</em> 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;

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"; }
+	}
+}