Respect @Configuration(value) for @Imported classes

Prior to this commit, @Configuration classes included via @Import (or
via automatic registration of nested configuration classes) would
always be registered with a generated bean name, regardless of whether
the user had specified a 'value' indicating a customized bean name, e.g.

    @Configuration("myConfig")
    public class AppConfig { ... }

Now this bean name is propagated as intended in all cases, meaning that
in the example above, the resulting bean definition of type AppConfig
will be named "myConfig" regardless how it was registered with the
container -- directly against the application context, via component
scanning, via @Import, or via automatic registration of nested
configuration classes.

Issue: SPR-9023

org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClass.java

@@ -30,6 +30,7 @@ import org.springframework.core.io.Resource;
 import org.springframework.core.type.AnnotationMetadata;
 import org.springframework.core.type.StandardAnnotationMetadata;
 import org.springframework.core.type.classreading.MetadataReader;
+import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
 
 /**
@@ -55,19 +56,66 @@ final class ConfigurationClass {
 
 	private String beanName;
 
+	private final boolean imported;
 
+
+	/**
+	 * Create a new {@link ConfigurationClass} with the given name.
+	 * @param metadataReader reader used to parse the underlying {@link Class}
+	 * @param beanName must not be {@code null}
+	 * @throws IllegalArgumentException if beanName is null (as of Spring 3.1.1)
+	 * @see ConfigurationClass#ConfigurationClass(Class, boolean)
+	 */
 	public ConfigurationClass(MetadataReader metadataReader, String beanName) {
+		Assert.hasText(beanName, "bean name must not be null");
 		this.metadata = metadataReader.getAnnotationMetadata();
 		this.resource = metadataReader.getResource();
 		this.beanName = beanName;
+		this.imported = false;
 	}
 
+	/**
+	 * Create a new {@link ConfigurationClass} representing a class that was imported
+	 * using the {@link Import} annotation or automatically processed as a nested
+	 * configuration class (if imported is {@code true}).
+	 * @param metadataReader reader used to parse the underlying {@link Class}
+	 * @param beanName name of the {@code @Configuration} class bean
+	 * @since 3.1.1
+	 */
+	public ConfigurationClass(MetadataReader metadataReader, boolean imported) {
+		this.metadata = metadataReader.getAnnotationMetadata();
+		this.resource = metadataReader.getResource();
+		this.imported = imported;
+	}
+
+	/**
+	 * Create a new {@link ConfigurationClass} with the given name.
+	 * @param clazz the underlying {@link Class} to represent
+	 * @param beanName name of the {@code @Configuration} class bean
+	 * @throws IllegalArgumentException if beanName is null (as of Spring 3.1.1)
+	 * @see ConfigurationClass#ConfigurationClass(Class, boolean)
+	 */
 	public ConfigurationClass(Class<?> clazz, String beanName) {
+		Assert.hasText(beanName, "bean name must not be null");
 		this.metadata = new StandardAnnotationMetadata(clazz);
 		this.resource = new DescriptiveResource(clazz.toString());
 		this.beanName = beanName;
+		this.imported = false;
 	}
 
+	/**
+	 * Create a new {@link ConfigurationClass} representing a class that was imported
+	 * using the {@link Import} annotation or automatically processed as a nested
+	 * configuration class (if imported is {@code true}).
+	 * @param clazz the underlying {@link Class} to represent
+	 * @param beanName name of the {@code @Configuration} class bean
+	 * @since 3.1.1
+	 */
+	public ConfigurationClass(Class<?> clazz, boolean imported) {
+		this.metadata = new StandardAnnotationMetadata(clazz);
+		this.resource = new DescriptiveResource(clazz.toString());
+		this.imported = imported;
+	}
 
 	public AnnotationMetadata getMetadata() {
 		return this.metadata;
@@ -81,6 +129,15 @@ final class ConfigurationClass {
 		return ClassUtils.getShortName(getMetadata().getClassName());
 	}
 
+	/**
+	 * Return whether this configuration class was registered via @{@link Import} or
+	 * automatically registered due to being nested within another configuration class.
+	 * @since 3.1.1
+	 */
+	public boolean isImported() {
+		return this.imported;
+	}
+
 	public void setBeanName(String beanName) {
 		this.beanName = beanName;
 	}
@@ -181,5 +238,4 @@ final class ConfigurationClass {
 		}
 	}
 
-
 }

org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java

@@ -122,32 +122,43 @@ class ConfigurationClassBeanDefinitionReader {
 	 * Register the {@link Configuration} class itself as a bean definition.
 	 */
 	private void doLoadBeanDefinitionForConfigurationClassIfNecessary(ConfigurationClass configClass) {
-		if (configClass.getBeanName() != null) {
+		if (!configClass.isImported()) {
-			// a bean definition already exists for this configuration class -> nothing to do
 			return;
 		}
 
-		// no bean definition exists yet -> this must be an imported configuration class (@Import).
 		BeanDefinition configBeanDef = new GenericBeanDefinition();
 		String className = configClass.getMetadata().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)) {
-			String configBeanName = BeanDefinitionReaderUtils.registerWithGeneratedName((AbstractBeanDefinition)configBeanDef, this.registry);
+			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);
+			}
 			configClass.setBeanName(configBeanName);
 			if (logger.isDebugEnabled()) {
 				logger.debug(String.format("Registered bean definition for imported @Configuration class %s", configBeanName));
 			}
 		}
 		else {
-			try {
+			AnnotationMetadata metadata = reader.getAnnotationMetadata();
-				MetadataReader reader = this.metadataReaderFactory.getMetadataReader(className);
+			this.problemReporter.error(
-				AnnotationMetadata metadata = reader.getAnnotationMetadata();
+					new InvalidConfigurationImportProblem(className, reader.getResource(), metadata));
-				this.problemReporter.error(
-						new InvalidConfigurationImportProblem(className, reader.getResource(), metadata));
-			}
-			catch (IOException ex) {
-				throw new IllegalStateException("Could not create MetadataReader for class " + className);
-			}
 		}
 	}
 

org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java

@@ -119,8 +119,7 @@ class ConfigurationClassParser {
 	/**
 	 * Parse the specified {@link Configuration @Configuration} class.
 	 * @param clazz the Class to parse
-	 * @param beanName may be null, but if populated represents the bean id
+	 * @param beanName must not be null (as of Spring 3.1.1)
-	 * (assumes that this configuration class was configured via XML)
 	 */
 	public void parse(Class<?> clazz, String beanName) throws IOException {
 		processConfigurationClass(new ConfigurationClass(clazz, beanName));
@@ -167,7 +166,7 @@ class ConfigurationClassParser {
 			MetadataReader reader = this.metadataReaderFactory.getMetadataReader(memberClassName);
 			AnnotationMetadata memberClassMetadata = reader.getAnnotationMetadata();
 			if (ConfigurationClassUtils.isConfigurationCandidate(memberClassMetadata)) {
-				processConfigurationClass(new ConfigurationClass(reader, null));
+				processConfigurationClass(new ConfigurationClass(reader, true));
 			}
 		}
 
@@ -300,7 +299,7 @@ class ConfigurationClassParser {
 				else {
 					// the candidate class not an ImportSelector or ImportBeanDefinitionRegistrar -> process it as a @Configuration class
 					this.importStack.registerImport(importingClassMetadata.getClassName(), candidate);
-					processConfigurationClass(new ConfigurationClass(reader, null));
+					processConfigurationClass(new ConfigurationClass(reader, true));
 				}
 			}
 			this.importStack.pop();

org.springframework.context/src/test/java/org/springframework/context/annotation/AbstractCircularImportDetectionTests.java

@@ -42,7 +42,7 @@ public abstract class AbstractCircularImportDetectionTests {
 	public void simpleCircularImportIsDetected() throws Exception {
 		boolean threw = false;
 		try {
-			newParser().parse(loadAsConfigurationSource(A.class), null);
+			newParser().parse(loadAsConfigurationSource(A.class), "A");
 		} catch (BeanDefinitionParsingException ex) {
 			assertTrue("Wrong message. Got: " + ex.getMessage(),
 					ex.getMessage().contains(
@@ -59,7 +59,7 @@ public abstract class AbstractCircularImportDetectionTests {
 	public void complexCircularImportIsDetected() throws Exception {
 		boolean threw = false;
 		try {
-			newParser().parse(loadAsConfigurationSource(X.class), null);
+			newParser().parse(loadAsConfigurationSource(X.class), "X");
 		}
 		catch (BeanDefinitionParsingException ex) {
 			assertTrue("Wrong message. Got: " + ex.getMessage(),

org.springframework.context/src/test/java/org/springframework/context/annotation/configuration/ImportTests.java

@@ -25,6 +25,7 @@ import test.beans.TestBean;
 import org.springframework.beans.factory.parsing.BeanDefinitionParsingException;
 import org.springframework.beans.factory.support.DefaultListableBeanFactory;
 import org.springframework.beans.factory.support.RootBeanDefinition;
+import org.springframework.context.annotation.AnnotationConfigApplicationContext;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.ConfigurationClassPostProcessor;
@@ -53,13 +54,6 @@ public class ImportTests {
 		assertThat(beanFactory.getBeanDefinitionCount(), equalTo(expectedCount));
 	}
 
-	@Test
-	public void testProcessImports() {
-		int configClasses = 2;
-		int beansInClasses = 2;
-		assertBeanDefinitionCount((configClasses + beansInClasses), ConfigurationWithImportAnnotation.class);
-	}
-
 	@Test
 	public void testProcessImportsWithAsm() {
 		int configClasses = 2;
@@ -315,4 +309,36 @@ public class ImportTests {
 	static class ConfigAnnotated { }
 
 	static class NonConfigAnnotated { }
+
+	// ------------------------------------------------------------------------
+
+	/**
+	 * Test that values supplied to @Configuration(value="...") are propagated as the
+	 * bean name for the configuration class even in the case of inclusion via @Import
+	 * or in the case of automatic registration via nesting
+	 */
+	@Test
+	public void reproSpr9023() {
+		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
+		ctx.register(B.class);
+		ctx.refresh();
+		System.out.println(ctx.getBeanFactory());
+		assertThat(ctx.getBeanNamesForType(B.class)[0], is("config-b"));
+		assertThat(ctx.getBeanNamesForType(A.class)[0], is("config-a"));
+	}
+
+	@Configuration("config-a")
+	static class A { }
+
+	@Configuration("config-b")
+	@Import(A.class)
+	static class B { }
+
+	@Test
+	public void testProcessImports() {
+		int configClasses = 2;
+		int beansInClasses = 2;
+		assertBeanDefinitionCount((configClasses + beansInClasses), ConfigurationWithImportAnnotation.class);
+	}
+
 }