diff --git a/org.springframework.context/src/main/java/org/springframework/context/annotation/Bean.java b/org.springframework.context/src/main/java/org/springframework/context/annotation/Bean.java index 5d65222a1c7..818ce705261 100644 --- a/org.springframework.context/src/main/java/org/springframework/context/annotation/Bean.java +++ b/org.springframework.context/src/main/java/org/springframework/context/annotation/Bean.java @@ -37,8 +37,8 @@ import org.springframework.beans.factory.annotation.Autowire; * *

While a {@link #name()} attribute is available, the default strategy for determining * the name of a bean is to use the name of the Bean method. This is convenient and - * intuitive, but if explicit naming is desired, the {@link #name()} attribute may be used. - * Also note that {@link #name()} accepts an array of Strings. This is in order to allow + * intuitive, but if explicit naming is desired, the {@code name()} attribute may be used. + * Also note that {@code name()} accepts an array of Strings. This is in order to allow * for specifying multiple names (i.e., aliases) for a single bean. * *

The @Bean annotation may be used on any methods in an @Component @@ -56,6 +56,27 @@ import org.springframework.beans.factory.annotation.Autowire; * subclassing of each such configuration class at runtime. As a consequence, configuration * classes and their factory methods must not be marked as final or private in this mode. * + *

A note on {@code BeanFactoryPostProcessor}-returning {@code @Bean} methods

+ *

Special consideration must be taken for {@code @Bean} methods that return Spring + * {@link org.springframework.beans.factory.config.BeanFactoryPostProcessor BeanFactoryPostProcessor} + * ({@code BFPP}) types. Because {@code BFPP} objects must be instantiated very early in the + * container lifecycle, they can interfere with processing of annotations such as {@code @Autowired}, + * {@code @Value}, and {@code @PostConstruct} within {@code @Configuration} classes. To avoid these + * lifecycle issues, mark {@code BFPP}-returning {@code @Bean} methods as {@code static}. For example: + *

+ *     @Bean
+ *     public static PropertyPlaceholderConfigurer ppc() {
+ *         // instantiate, configure and return ppc ...
+ *     }
+ * 
+ * By marking this method as {@code static}, it can be invoked without causing instantiation of its + * declaring {@code @Configuration} class, thus avoiding the above-mentioned lifecycle conflicts. + * Note however that {@code static} {@code @Bean} methods will not be enhanced for scoping and AOP + * semantics as mentioned above. This works out in {@code BFPP} cases, as they are not typically + * referenced by other {@code @Bean} methods. As a reminder, a WARN-level log message will be + * issued for any non-static {@code @Bean} methods having a return type assignable to + * {@code BeanFactoryPostProcessor}. + * * @author Rod Johnson * @author Costin Leau * @author Chris Beams diff --git a/org.springframework.context/src/main/java/org/springframework/context/annotation/BeanMethod.java b/org.springframework.context/src/main/java/org/springframework/context/annotation/BeanMethod.java index 4b5d762d1d5..939cb6b300e 100644 --- a/org.springframework.context/src/main/java/org/springframework/context/annotation/BeanMethod.java +++ b/org.springframework.context/src/main/java/org/springframework/context/annotation/BeanMethod.java @@ -39,39 +39,25 @@ final class BeanMethod extends ConfigurationMethod { @Override public void validate(ProblemReporter problemReporter) { + if (getMetadata().isStatic()) { + // static @Bean methods have no constraints to validate -> return immediately + return; + } + if (this.configurationClass.getMetadata().isAnnotated(Configuration.class.getName())) { if (!getMetadata().isOverridable()) { + // instance @Bean methods within @Configuration classes must be overridable to accommodate CGLIB problemReporter.error(new NonOverridableMethodError()); } } - else { - if (getMetadata().isStatic()) { - problemReporter.error(new StaticMethodError()); - } - } } - /** - * {@link Bean} methods must be overridable in order to accommodate CGLIB. - */ + private class NonOverridableMethodError extends Problem { public NonOverridableMethodError() { - super(String.format("Method '%s' must not be private, final or static; change the method's modifiers to continue", - getMetadata().getMethodName()), getResourceLocation()); - } - } - - - /** - * {@link Bean} methods must at least not be static in the non-CGLIB case. - */ - private class StaticMethodError extends Problem { - - public StaticMethodError() { - super(String.format("Method '%s' must not be static; remove the method's static modifier to continue", + super(String.format("@Bean method '%s' must not be private or final; change the method's modifiers to continue", getMetadata().getMethodName()), getResourceLocation()); } } - } 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 fd4b13e7d97..d9e687b59e6 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 @@ -162,8 +162,16 @@ public class ConfigurationClassBeanDefinitionReader { RootBeanDefinition beanDef = new ConfigurationClassBeanDefinition(configClass); beanDef.setResource(configClass.getResource()); beanDef.setSource(this.sourceExtractor.extractSource(metadata, configClass.getResource())); - beanDef.setFactoryBeanName(configClass.getBeanName()); - beanDef.setUniqueFactoryMethodName(metadata.getMethodName()); + if (metadata.isStatic()) { + // static @Bean method + beanDef.setBeanClassName(configClass.getMetadata().getClassName()); + beanDef.setFactoryMethodName(metadata.getMethodName()); + } + else { + // instance @Bean method + beanDef.setFactoryBeanName(configClass.getBeanName()); + beanDef.setUniqueFactoryMethodName(metadata.getMethodName()); + } beanDef.setAutowireMode(RootBeanDefinition.AUTOWIRE_CONSTRUCTOR); beanDef.setAttribute(RequiredAnnotationBeanPostProcessor.SKIP_REQUIRED_CHECK_ATTRIBUTE, Boolean.TRUE); 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 2e4bc37de9b..23a6baff8ee 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 @@ -33,6 +33,7 @@ import org.springframework.aop.scope.ScopedProxyFactoryBean; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.DisposableBean; import org.springframework.beans.factory.FactoryBean; +import org.springframework.beans.factory.config.BeanFactoryPostProcessor; import org.springframework.beans.factory.config.ConfigurableBeanFactory; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.util.Assert; @@ -239,6 +240,15 @@ class ConfigurationClassEnhancer { return this.beanFactory.getBean(beanName); } + if (BeanFactoryPostProcessor.class.isAssignableFrom(beanMethod.getReturnType())) { + logger.warn(String.format("@Bean method %s.%s is non-static and returns an object " + + "assignable to Spring's BeanFactoryPostProcessor interface. This will " + + "result in a failure to process annotations such as @Autowired, " + + "@Resource and @PostConstruct within the method's declaring " + + "@Configuration class. Add the 'static' modifier to this method to avoid" + + "these container lifecycle issues; see @Bean Javadoc for complete details", + beanMethod.getDeclaringClass().getSimpleName(), beanMethod.getName())); + } return cglibMethodProxy.invokeSuper(enhancedConfigInstance, beanMethodArgs); } diff --git a/org.springframework.context/src/test/java/org/springframework/context/annotation/ConfigurationClassAndBFPPTests.java b/org.springframework.context/src/test/java/org/springframework/context/annotation/ConfigurationClassAndBFPPTests.java new file mode 100644 index 00000000000..060c9f6bf2c --- /dev/null +++ b/org.springframework.context/src/test/java/org/springframework/context/annotation/ConfigurationClassAndBFPPTests.java @@ -0,0 +1,122 @@ +/* + * Copyright 2002-2011 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; + +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.CoreMatchers.notNullValue; +import static org.hamcrest.CoreMatchers.nullValue; +import static org.hamcrest.CoreMatchers.sameInstance; +import static org.junit.Assert.assertThat; + +import org.junit.Test; +import org.springframework.beans.BeansException; +import org.springframework.beans.TestBean; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.config.BeanFactoryPostProcessor; +import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; + +/** + * Tests semantics of declaring {@link BeanFactoryPostProcessor}-returning @Bean + * methods, specifically as regards static @Bean methods and the avoidance of + * container lifecycle issues when BFPPs are in the mix. + * + * @author Chris Beams + * @since 3.1 + */ +public class ConfigurationClassAndBFPPTests { + + @Test + public void autowiringFailsWithBFPPAsInstanceMethod() { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); + ctx.register(TestBeanConfig.class, AutowiredConfigWithBFPPAsInstanceMethod.class); + ctx.refresh(); + // instance method BFPP interferes with lifecycle -> autowiring fails! + // WARN-level logging should have been issued about returning BFPP from non-static @Bean method + assertThat(ctx.getBean(AutowiredConfigWithBFPPAsInstanceMethod.class).autowiredTestBean, nullValue()); + } + + @Test + public void autowiringSucceedsWithBFPPAsStaticMethod() { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); + ctx.register(TestBeanConfig.class, AutowiredConfigWithBFPPAsStaticMethod.class); + ctx.refresh(); + // static method BFPP does not interfere with lifecycle -> autowiring succeeds + assertThat(ctx.getBean(AutowiredConfigWithBFPPAsStaticMethod.class).autowiredTestBean, notNullValue()); + } + + + @Configuration + static class TestBeanConfig { + @Bean + public TestBean testBean() { + return new TestBean(); + } + } + + + @Configuration + static class AutowiredConfigWithBFPPAsInstanceMethod { + @Autowired TestBean autowiredTestBean; + + @Bean + public BeanFactoryPostProcessor bfpp() { + return new BeanFactoryPostProcessor() { + public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) throws BeansException { + // no-op + } + }; + } + } + + + @Configuration + static class AutowiredConfigWithBFPPAsStaticMethod { + @Autowired TestBean autowiredTestBean; + + @Bean + public static final BeanFactoryPostProcessor bfpp() { + return new BeanFactoryPostProcessor() { + public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) throws BeansException { + // no-op + } + }; + } + } + + + @Test + @SuppressWarnings("static-access") + public void staticBeanMethodsDoNotRespectScoping() { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); + ctx.register(ConfigWithStaticBeanMethod.class); + ctx.refresh(); + + ConfigWithStaticBeanMethod config = ctx.getBean(ConfigWithStaticBeanMethod.class); + assertThat(config.testBean(), not(sameInstance(config.testBean()))); + } + + + @Configuration + static class ConfigWithStaticBeanMethod { + @Bean + public static TestBean testBean() { + return new TestBean("foo"); + } + } + + +}