From 52bef0b7b024e794186437dee78945fbb5bd209a Mon Sep 17 00:00:00 2001 From: Chris Beams Date: Tue, 10 May 2011 11:55:41 +0000 Subject: [PATCH] Allow static modifier on @Bean methods Declaring @Bean methods as 'static' is now permitted, whereas previously it raised an exception at @Configuration class validation time. A static @Bean method can be called by the container without requiring the instantiation of its declaring @Configuration class. This is particularly useful when dealing with BeanFactoryPostProcessor beans, as they can interfere with the standard post-processing lifecycle necessary to handle @Autowired, @Inject, @Value, @PostConstruct and other annotations. static @Bean methods cannot recieve CGLIB enhancement for scoping and AOP concerns. This is acceptable in BFPP cases as they rarely if ever need it, and should not in typical cases ever be called by another @Bean method. Once invoked by the container, the resulting bean will be cached as usual, but multiple invocations of the static @Bean method will result in creation of multiple instances of the bean. static @Bean methods may not, for obvious reasons, refer to normal instance @Bean methods, but again this is not likely a concern for BFPP types. In the rare case that they do need a bean reference, parameter injection into the static @Bean method is technically an option, but should be avoided as it will potentially cause premature instantiation of more beans that the user may have intended. Note particularly that a WARN-level log message is now issued for any non-static @Bean method with a return type assignable to BFPP. This serves as a strong recommendation to users that they always mark BFPP @Bean methods as static. See @Bean Javadoc for complete details. Issue: SPR-8257, SPR-8269 --- .../context/annotation/Bean.java | 25 +++- .../context/annotation/BeanMethod.java | 30 ++--- ...onfigurationClassBeanDefinitionReader.java | 12 +- .../ConfigurationClassEnhancer.java | 10 ++ .../ConfigurationClassAndBFPPTests.java | 122 ++++++++++++++++++ 5 files changed, 173 insertions(+), 26 deletions(-) create mode 100644 org.springframework.context/src/test/java/org/springframework/context/annotation/ConfigurationClassAndBFPPTests.java 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 5d65222a1c..818ce70526 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 4b5d762d1d..939cb6b300 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 fd4b13e7d9..d9e687b59e 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 2e4bc37de9..23a6baff8e 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 0000000000..060c9f6bf2 --- /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"); + } + } + + +} -- GitLab