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 70669481ad..ccd66d6534 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2009 the original author or authors. + * Copyright 2002-2010 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. @@ -16,6 +16,7 @@ package org.springframework.context.annotation; +import java.io.IOException; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Arrays; @@ -26,22 +27,31 @@ import java.util.Set; 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.Autowire; import org.springframework.beans.factory.annotation.RequiredAnnotationBeanPostProcessor; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.BeanDefinitionHolder; +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.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.RootBeanDefinition; +import org.springframework.core.Conventions; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.core.io.Resource; import org.springframework.core.type.AnnotationMetadata; import org.springframework.core.type.MethodMetadata; +import org.springframework.core.type.StandardAnnotationMetadata; +import org.springframework.core.type.classreading.CachingMetadataReaderFactory; +import org.springframework.core.type.classreading.MetadataReader; +import org.springframework.core.type.classreading.MetadataReaderFactory; +import org.springframework.stereotype.Component; import org.springframework.util.StringUtils; /** @@ -58,20 +68,37 @@ import org.springframework.util.StringUtils; */ class ConfigurationClassBeanDefinitionReader { - private static final Log logger = LogFactory.getLog(ConfigurationClassBeanDefinitionReader.class); + static final String CONFIGURATION_CLASS_FULL = "full"; + + static final String CONFIGURATION_CLASS_LITE = "lite"; + static final String CONFIGURATION_CLASS_ATTRIBUTE = + Conventions.getQualifiedAttributeName(ConfigurationClassPostProcessor.class, "configurationClass"); + + private static final Log logger = LogFactory.getLog(ConfigurationClassBeanDefinitionReader.class); + private final BeanDefinitionRegistry registry; private final SourceExtractor sourceExtractor; + private final ProblemReporter problemReporter; + + private final MetadataReaderFactory metadataReaderFactory; + + /** * Create a new {@link ConfigurationClassBeanDefinitionReader} instance that will be used * to populate the given {@link BeanDefinitionRegistry}. + * @param problemReporter + * @param metadataReaderFactory */ - public ConfigurationClassBeanDefinitionReader(BeanDefinitionRegistry registry, SourceExtractor sourceExtractor) { + public ConfigurationClassBeanDefinitionReader(BeanDefinitionRegistry registry, SourceExtractor sourceExtractor, + ProblemReporter problemReporter, MetadataReaderFactory metadataReaderFactory) { this.registry = registry; this.sourceExtractor = sourceExtractor; + this.problemReporter = problemReporter; + this.metadataReaderFactory = metadataReaderFactory; } @@ -90,7 +117,7 @@ class ConfigurationClassBeanDefinitionReader { * class itself, all its {@link Bean} methods */ private void loadBeanDefinitionsForConfigurationClass(ConfigurationClass configClass) { - doLoadBeanDefinitionForConfigurationClass(configClass); + doLoadBeanDefinitionForConfigurationClassIfNecessary(configClass); for (ConfigurationClassMethod method : configClass.getMethods()) { loadBeanDefinitionsForModelMethod(method); @@ -102,16 +129,31 @@ class ConfigurationClassBeanDefinitionReader { /** * Registers the {@link Configuration} class itself as a bean definition. */ - private void doLoadBeanDefinitionForConfigurationClass(ConfigurationClass configClass) { - if (configClass.getBeanName() == null) { - GenericBeanDefinition configBeanDef = new GenericBeanDefinition(); - configBeanDef.setBeanClassName(configClass.getMetadata().getClassName()); - new ConfigurationClassPostProcessor().checkConfigurationClassCandidate(configBeanDef); + private void doLoadBeanDefinitionForConfigurationClassIfNecessary(ConfigurationClass configClass) { + if (configClass.getBeanName() != null) { + // 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). + GenericBeanDefinition configBeanDef = new GenericBeanDefinition(); + String className = configClass.getMetadata().getClassName(); + configBeanDef.setBeanClassName(className); + if(checkConfigurationClassCandidate(configBeanDef, this.metadataReaderFactory)) { String configBeanName = BeanDefinitionReaderUtils.registerWithGeneratedName(configBeanDef, this.registry); configClass.setBeanName(configBeanName); if (logger.isDebugEnabled()) { logger.debug(String.format("Registered bean definition for imported @Configuration class %s", configBeanName)); } + } else { + try { + MetadataReader reader = this.metadataReaderFactory.getMetadataReader(className); + AnnotationMetadata metadata = reader.getAnnotationMetadata(); + this.problemReporter.error( + new InvalidConfigurationImportProblem(className, reader.getResource(), metadata)); + } catch (IOException ex) { + throw new IllegalStateException("Could not create MetadataReader for class " + className); + } } } @@ -235,6 +277,51 @@ class ConfigurationClassBeanDefinitionReader { } } + /** + * Check whether the given bean definition is a candidate for a configuration class, + * and mark it accordingly. + * @param beanDef the bean definition to check + * @param metadataReaderFactory the current factory in use by the caller + * @return whether the candidate qualifies as (any kind of) configuration class + */ + static boolean checkConfigurationClassCandidate(BeanDefinition beanDef, MetadataReaderFactory metadataReaderFactory) { + AnnotationMetadata metadata = null; + + // Check already loaded Class if present... + // since we possibly can't even load the class file for this Class. + if (beanDef instanceof AbstractBeanDefinition && ((AbstractBeanDefinition) beanDef).hasBeanClass()) { + metadata = new StandardAnnotationMetadata(((AbstractBeanDefinition) beanDef).getBeanClass()); + } + else { + String className = beanDef.getBeanClassName(); + if (className != null) { + try { + MetadataReader metadataReader = metadataReaderFactory.getMetadataReader(className); + metadata = metadataReader.getAnnotationMetadata(); + } + catch (IOException ex) { + if (logger.isDebugEnabled()) { + logger.debug("Could not find class file for introspecting factory methods: " + className, ex); + } + return false; + } + } + } + + if (metadata != null) { + if (metadata.isAnnotated(Configuration.class.getName())) { + beanDef.setAttribute(CONFIGURATION_CLASS_ATTRIBUTE, CONFIGURATION_CLASS_FULL); + return true; + } + else if (metadata.isAnnotated(Component.class.getName()) || + metadata.hasAnnotatedMethods(Bean.class.getName())) { + beanDef.setAttribute(CONFIGURATION_CLASS_ATTRIBUTE, CONFIGURATION_CLASS_LITE); + return true; + } + } + return false; + } + /** * {@link RootBeanDefinition} marker subclass used to signify that a bean definition * created from a configuration class as opposed to any other configuration source. @@ -270,4 +357,20 @@ class ConfigurationClassBeanDefinitionReader { } } + + /** + * Configuration classes must be annotated with {@link Configuration @Configuration} or + * declare at least one {@link Bean @Bean} method. + */ + private static class InvalidConfigurationImportProblem extends Problem { + + public InvalidConfigurationImportProblem(String className, Resource resource, AnnotationMetadata metadata) { + super(String.format("%s was imported as a Configuration class but is not annotated " + + "with @Configuration nor does it declare any @Bean methods. Update the class to " + + "meet either of these requirements or do not attempt to import it.", className), + new Location(resource, metadata)); + } + + } + } diff --git a/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassEnhancer.java b/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassEnhancer.java index db62168951..e3f0ae2938 100644 --- a/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassEnhancer.java +++ b/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassEnhancer.java @@ -86,9 +86,6 @@ class ConfigurationClassEnhancer { * @return fully-qualified name of the enhanced subclass */ public Class enhance(Class configClass) { - if (logger.isDebugEnabled()) { - logger.debug("Enhancing " + configClass.getName()); - } Class enhancedClass = createClass(newEnhancer(configClass)); if (logger.isDebugEnabled()) { logger.debug(String.format("Successfully enhanced %s; enhanced class name is: %s", diff --git a/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java b/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java index 6b3771d59e..22473758f3 100644 --- a/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java +++ b/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2009 the original author or authors. + * Copyright 2002-2010 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. @@ -24,11 +24,9 @@ import java.util.LinkedHashSet; import java.util.Set; import java.util.Stack; -import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.parsing.Location; import org.springframework.beans.factory.parsing.Problem; import org.springframework.beans.factory.parsing.ProblemReporter; -import org.springframework.core.io.Resource; import org.springframework.core.type.AnnotationMetadata; import org.springframework.core.type.MethodMetadata; import org.springframework.core.type.StandardAnnotationMetadata; @@ -43,7 +41,7 @@ import org.springframework.util.StringUtils; * another using the {@link Import} annotation). * *

This class helps separate the concern of parsing the structure of a Configuration - * class from the concern of registering {@link BeanDefinition} objects based on the + * class from the concern of registering BeanDefinition objects based on the * content of that model. * *

This ASM-based implementation avoids reflection and eager class loading in order to @@ -153,24 +151,13 @@ class ConfigurationClassParser { else { this.importStack.push(configClass); for (String classToImport : classesToImport) { - processClassToImport(classToImport); + MetadataReader reader = this.metadataReaderFactory.getMetadataReader(classToImport); + processConfigurationClass(new ConfigurationClass(reader, null)); } this.importStack.pop(); } } - private void processClassToImport(String classToImport) throws IOException { - MetadataReader reader = this.metadataReaderFactory.getMetadataReader(classToImport); - AnnotationMetadata metadata = reader.getAnnotationMetadata(); - if (!metadata.isAnnotated(Configuration.class.getName())) { - this.problemReporter.error( - new NonAnnotatedConfigurationProblem(metadata.getClassName(), reader.getResource(), metadata)); - } - else { - processConfigurationClass(new ConfigurationClass(reader, null)); - } - } - /** * Validate each {@link ConfigurationClass} object. * @see ConfigurationClass#validate @@ -229,20 +216,6 @@ class ConfigurationClassParser { } - /** - * Configuration classes must be annotated with {@link Configuration @Configuration}. - */ - private static class NonAnnotatedConfigurationProblem extends Problem { - - public NonAnnotatedConfigurationProblem(String className, Resource resource, AnnotationMetadata metadata) { - super(String.format("%s was imported as a @Configuration class but was not actually annotated " + - "with @Configuration. Annotate the class or do not attempt to process it.", className), - new Location(resource, metadata)); - } - - } - - /** * {@link Problem} registered upon detection of a circular {@link Import}. */ diff --git a/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassPostProcessor.java b/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassPostProcessor.java index eee004a859..17f1da85ad 100644 --- a/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassPostProcessor.java +++ b/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassPostProcessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2009 the original author or authors. + * Copyright 2002-2010 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. @@ -16,6 +16,9 @@ package org.springframework.context.annotation; +import static org.springframework.context.annotation.ConfigurationClassBeanDefinitionReader.CONFIGURATION_CLASS_ATTRIBUTE; +import static org.springframework.context.annotation.ConfigurationClassBeanDefinitionReader.CONFIGURATION_CLASS_FULL; + import java.io.IOException; import java.util.LinkedHashMap; import java.util.LinkedHashSet; @@ -37,14 +40,9 @@ 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.BeanDefinitionRegistry; -import org.springframework.core.Conventions; import org.springframework.core.Ordered; -import org.springframework.core.type.AnnotationMetadata; -import org.springframework.core.type.StandardAnnotationMetadata; import org.springframework.core.type.classreading.CachingMetadataReaderFactory; -import org.springframework.core.type.classreading.MetadataReader; import org.springframework.core.type.classreading.MetadataReaderFactory; -import org.springframework.stereotype.Component; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -67,14 +65,6 @@ import org.springframework.util.ClassUtils; */ public class ConfigurationClassPostProcessor implements BeanFactoryPostProcessor, BeanClassLoaderAware { - private static final String CONFIGURATION_CLASS_ATTRIBUTE = - Conventions.getQualifiedAttributeName(ConfigurationClassPostProcessor.class, "configurationClass"); - - private static final String CONFIGURATION_CLASS_FULL = "full"; - - private static final String CONFIGURATION_CLASS_LITE = "lite"; - - /** Whether the CGLIB2 library is present on the classpath */ private static final boolean cglibAvailable = ClassUtils.isPresent( "net.sf.cglib.proxy.Enhancer", ConfigurationClassPostProcessor.class.getClassLoader()); @@ -155,7 +145,7 @@ public class ConfigurationClassPostProcessor implements BeanFactoryPostProcessor Set configCandidates = new LinkedHashSet(); for (String beanName : registry.getBeanDefinitionNames()) { BeanDefinition beanDef = registry.getBeanDefinition(beanName); - if (checkConfigurationClassCandidate(beanDef)) { + if (ConfigurationClassBeanDefinitionReader.checkConfigurationClassCandidate(beanDef, this.metadataReaderFactory)) { configCandidates.add(new BeanDefinitionHolder(beanDef, beanName)); } } @@ -184,51 +174,9 @@ public class ConfigurationClassPostProcessor implements BeanFactoryPostProcessor parser.validate(); // Read the model and create bean definitions based on its content - new ConfigurationClassBeanDefinitionReader(registry, this.sourceExtractor).loadBeanDefinitions(parser.getConfigurationClasses()); - } - - /** - * Check whether the given bean definition is a candidate for a configuration class, - * and mark it accordingly. - * @param beanDef the bean definition to check - * @return whether the candidate qualifies as (any kind of) configuration class - */ - protected boolean checkConfigurationClassCandidate(BeanDefinition beanDef) { - AnnotationMetadata metadata = null; - - // Check already loaded Class if present... - // since we possibly can't even load the class file for this Class. - if (beanDef instanceof AbstractBeanDefinition && ((AbstractBeanDefinition) beanDef).hasBeanClass()) { - metadata = new StandardAnnotationMetadata(((AbstractBeanDefinition) beanDef).getBeanClass()); - } - else { - String className = beanDef.getBeanClassName(); - if (className != null) { - try { - MetadataReader metadataReader = this.metadataReaderFactory.getMetadataReader(className); - metadata = metadataReader.getAnnotationMetadata(); - } - catch (IOException ex) { - if (logger.isDebugEnabled()) { - logger.debug("Could not find class file for introspecting factory methods: " + className, ex); - } - return false; - } - } - } - - if (metadata != null) { - if (metadata.isAnnotated(Configuration.class.getName())) { - beanDef.setAttribute(CONFIGURATION_CLASS_ATTRIBUTE, CONFIGURATION_CLASS_FULL); - return true; - } - else if (metadata.isAnnotated(Component.class.getName()) || - metadata.hasAnnotatedMethods(Bean.class.getName())) { - beanDef.setAttribute(CONFIGURATION_CLASS_ATTRIBUTE, CONFIGURATION_CLASS_LITE); - return true; - } - } - return false; + ConfigurationClassBeanDefinitionReader reader = + new ConfigurationClassBeanDefinitionReader(registry, this.sourceExtractor, this.problemReporter, this.metadataReaderFactory); + reader.loadBeanDefinitions(parser.getConfigurationClasses()); } /** @@ -262,8 +210,8 @@ public class ConfigurationClassPostProcessor implements BeanFactoryPostProcessor for (Map.Entry entry : configBeanDefs.entrySet()) { AbstractBeanDefinition beanDef = entry.getValue(); try { - Class configClass = beanDef.resolveBeanClass(this.beanClassLoader); - Class enhancedClass = enhancer.enhance(configClass); + Class configClass = beanDef.resolveBeanClass(this.beanClassLoader); + Class enhancedClass = enhancer.enhance(configClass); if (logger.isDebugEnabled()) { logger.debug(String.format("Replacing bean definition '%s' existing class name '%s' " + "with enhanced class name '%s'", entry.getKey(), configClass.getName(), enhancedClass.getName())); diff --git a/org.springframework.context/src/test/java/org/springframework/context/annotation/configuration/ImportedConfigurationClassEnhancementTests.java b/org.springframework.context/src/test/java/org/springframework/context/annotation/configuration/ImportedConfigurationClassEnhancementTests.java index 8ed1c7ee15..f3ea386dcf 100644 --- a/org.springframework.context/src/test/java/org/springframework/context/annotation/configuration/ImportedConfigurationClassEnhancementTests.java +++ b/org.springframework.context/src/test/java/org/springframework/context/annotation/configuration/ImportedConfigurationClassEnhancementTests.java @@ -1,3 +1,19 @@ +/* + * Copyright 2002-2010 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 static org.hamcrest.CoreMatchers.sameInstance; @@ -7,6 +23,7 @@ import static org.junit.Assert.assertTrue; import org.junit.Test; import org.springframework.aop.support.AopUtils; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.parsing.BeanDefinitionParsingException; import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; @@ -59,7 +76,12 @@ public class ImportedConfigurationClassEnhancementTests { assertThat("got two distinct instances of testBean when singleton scoping was expected", testBean1, sameInstance(testBean2)); } - + + + @Test(expected=BeanDefinitionParsingException.class) + public void importingAnNonConfigurationClassCausesIllegalArgumentException() { + new AnnotationConfigApplicationContext(ConfigThatImportsNonConfigClass.class); + } } @Configuration @@ -79,3 +101,7 @@ class ConfigThatDoesImport extends Config { } @Configuration class ConfigThatDoesNotImport extends Config { } + +@Configuration +@Import(Void.class) +class ConfigThatImportsNonConfigClass { } \ No newline at end of file diff --git a/org.springframework.context/src/test/resources/log4j.xml b/org.springframework.context/src/test/resources/log4j.xml index 65718d1ae1..69c2659f9b 100644 --- a/org.springframework.context/src/test/resources/log4j.xml +++ b/org.springframework.context/src/test/resources/log4j.xml @@ -11,9 +11,9 @@ - - - + + +