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

Author: cbeams

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

@@ -37,8 +37,8 @@
  *
  * <p>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.
  *
  * <p>The <code>@Bean</code> annotation may be used on any methods in an <code>@Component</code>
@@ -56,6 +56,27 @@
  * 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.
  *
+ * <h3>A note on {@code BeanFactoryPostProcessor}-returning {@code @Bean} methods</h3>
+ * <p>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:
+ * <pre class="code">
+ *     &#064;Bean
+ *     public static PropertyPlaceholderConfigurer ppc() {
+ *         // instantiate, configure and return ppc ...
+ *     }
+ * </pre>
+ * 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

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

@@ -39,39 +39,25 @@ public BeanMethod(MethodMetadata metadata, ConfigurationClass configurationClass
 
 	@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());
 		}
 	}
-
 }

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

@@ -162,8 +162,16 @@ private void loadBeanDefinitionsForBeanMethod(BeanMethod beanMethod) {
 		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);
 

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

@@ -33,6 +33,7 @@
 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 @@ public Object intercept(Object enhancedConfigInstance, Method beanMethod, Object
 				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);
 		}
 

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