diff --git a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassEnhancer.java b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassEnhancer.java
index 5a1a1186f3f..df2ae5b6a7b 100644
--- a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassEnhancer.java
+++ b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassEnhancer.java
@@ -1,5 +1,5 @@
/*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2017 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.
@@ -17,7 +17,10 @@
package org.springframework.context.annotation;
import java.lang.reflect.Field;
+import java.lang.reflect.InvocationHandler;
import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.lang.reflect.Proxy;
import java.util.Arrays;
import org.apache.commons.logging.Log;
@@ -330,12 +333,11 @@ class ConfigurationClassEnhancer {
factoryContainsBean(beanFactory, beanName)) {
Object factoryBean = beanFactory.getBean(BeanFactory.FACTORY_BEAN_PREFIX + beanName);
if (factoryBean instanceof ScopedProxyFactoryBean) {
- // Pass through - scoped proxy factory beans are a special case and should not
- // be further proxied
+ // Scoped proxy factory beans are a special case and should not be further proxied
}
else {
// It is a candidate FactoryBean - go ahead with enhancement
- return enhanceFactoryBean(factoryBean, beanFactory, beanName);
+ return enhanceFactoryBean(factoryBean, beanMethod.getReturnType(), beanFactory, beanName);
}
}
@@ -346,68 +348,88 @@ class ConfigurationClassEnhancer {
if (logger.isWarnEnabled() &&
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.",
+ "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);
}
- else {
- // The user (i.e. not the factory) is requesting this bean through a call to
- // the bean method, direct or indirect. The bean may have already been marked
- // as 'in creation' in certain autowiring scenarios; if so, temporarily set
- // the in-creation status to false in order to avoid an exception.
- boolean alreadyInCreation = beanFactory.isCurrentlyInCreation(beanName);
- try {
- if (alreadyInCreation) {
- beanFactory.setCurrentlyInCreation(beanName, false);
- }
- boolean useArgs = !ObjectUtils.isEmpty(beanMethodArgs);
- if (useArgs && beanFactory.isSingleton(beanName)) {
- // Stubbed null arguments just for reference purposes,
- // expecting them to be autowired for regular singleton references?
- // A safe assumption since @Bean singleton arguments cannot be optional...
- for (Object arg : beanMethodArgs) {
- if (arg == null) {
- useArgs = false;
- break;
- }
+
+ return obtainBeanInstanceFromFactory(beanMethod, beanMethodArgs, beanFactory, beanName);
+ }
+
+ private Object obtainBeanInstanceFromFactory(Method beanMethod, Object[] beanMethodArgs,
+ ConfigurableBeanFactory beanFactory, String beanName) {
+
+ // The user (i.e. not the factory) is requesting this bean through a call to
+ // the bean method, direct or indirect. The bean may have already been marked
+ // as 'in creation' in certain autowiring scenarios; if so, temporarily set
+ // the in-creation status to false in order to avoid an exception.
+ boolean alreadyInCreation = beanFactory.isCurrentlyInCreation(beanName);
+ try {
+ if (alreadyInCreation) {
+ beanFactory.setCurrentlyInCreation(beanName, false);
+ }
+ boolean useArgs = !ObjectUtils.isEmpty(beanMethodArgs);
+ if (useArgs && beanFactory.isSingleton(beanName)) {
+ // Stubbed null arguments just for reference purposes,
+ // expecting them to be autowired for regular singleton references?
+ // A safe assumption since @Bean singleton arguments cannot be optional...
+ for (Object arg : beanMethodArgs) {
+ if (arg == null) {
+ useArgs = false;
+ break;
}
}
- Object beanInstance = (useArgs ? beanFactory.getBean(beanName, beanMethodArgs) :
- beanFactory.getBean(beanName));
- if (beanInstance != null && !ClassUtils.isAssignableValue(beanMethod.getReturnType(), beanInstance)) {
- String msg = String.format("@Bean method %s.%s called as a bean reference " +
- "for type [%s] but overridden by non-compatible bean instance of type [%s].",
- beanMethod.getDeclaringClass().getSimpleName(), beanMethod.getName(),
- beanMethod.getReturnType().getName(), beanInstance.getClass().getName());
- try {
- BeanDefinition beanDefinition = beanFactory.getMergedBeanDefinition(beanName);
- msg += " Overriding bean of same name declared in: " + beanDefinition.getResourceDescription();
- }
- catch (NoSuchBeanDefinitionException ex) {
- // Ignore - simply no detailed message then.
- }
- throw new IllegalStateException(msg);
+ }
+ Object beanInstance = (useArgs ? beanFactory.getBean(beanName, beanMethodArgs) :
+ beanFactory.getBean(beanName));
+ if (beanInstance != null && !ClassUtils.isAssignableValue(beanMethod.getReturnType(), beanInstance)) {
+ String msg = String.format("@Bean method %s.%s called as a bean reference " +
+ "for type [%s] but overridden by non-compatible bean instance of type [%s].",
+ beanMethod.getDeclaringClass().getSimpleName(), beanMethod.getName(),
+ beanMethod.getReturnType().getName(), beanInstance.getClass().getName());
+ try {
+ BeanDefinition beanDefinition = beanFactory.getMergedBeanDefinition(beanName);
+ msg += " Overriding bean of same name declared in: " + beanDefinition.getResourceDescription();
}
- Method currentlyInvoked = SimpleInstantiationStrategy.getCurrentlyInvokedFactoryMethod();
- if (currentlyInvoked != null) {
- String outerBeanName = BeanAnnotationHelper.determineBeanNameFor(currentlyInvoked);
- beanFactory.registerDependentBean(beanName, outerBeanName);
+ catch (NoSuchBeanDefinitionException ex) {
+ // Ignore - simply no detailed message then.
}
- return beanInstance;
+ throw new IllegalStateException(msg);
}
- finally {
- if (alreadyInCreation) {
- beanFactory.setCurrentlyInCreation(beanName, true);
- }
+ Method currentlyInvoked = SimpleInstantiationStrategy.getCurrentlyInvokedFactoryMethod();
+ if (currentlyInvoked != null) {
+ String outerBeanName = BeanAnnotationHelper.determineBeanNameFor(currentlyInvoked);
+ beanFactory.registerDependentBean(beanName, outerBeanName);
+ }
+ return beanInstance;
+ }
+ finally {
+ if (alreadyInCreation) {
+ beanFactory.setCurrentlyInCreation(beanName, true);
}
}
}
+ @Override
+ public boolean isMatch(Method candidateMethod) {
+ return BeanAnnotationHelper.isBeanAnnotated(candidateMethod);
+ }
+
+ private ConfigurableBeanFactory getBeanFactory(Object enhancedConfigInstance) {
+ Field field = ReflectionUtils.findField(enhancedConfigInstance.getClass(), BEAN_FACTORY_FIELD);
+ Assert.state(field != null, "Unable to find generated bean factory field");
+ Object beanFactory = ReflectionUtils.getField(field, enhancedConfigInstance);
+ Assert.state(beanFactory != null, "BeanFactory has not been injected into @Configuration class");
+ Assert.state(beanFactory instanceof ConfigurableBeanFactory,
+ "Injected BeanFactory is not a ConfigurableBeanFactory");
+ return (ConfigurableBeanFactory) beanFactory;
+ }
+
/**
* Check the BeanFactory to see whether the bean named beanName already
* exists. Accounts for the fact that the requested bean may be "in creation", i.e.:
@@ -444,8 +466,60 @@ class ConfigurationClassEnhancer {
* instance directly. If a FactoryBean instance is fetched through the container via &-dereferencing,
* it will not be proxied. This too is aligned with the way XML configuration works.
*/
- private Object enhanceFactoryBean(final Object factoryBean, final ConfigurableBeanFactory beanFactory,
- final String beanName) {
+ private Object enhanceFactoryBean(final Object factoryBean, Class exposedType,
+ final ConfigurableBeanFactory beanFactory, final String beanName) {
+
+ try {
+ Class clazz = factoryBean.getClass();
+ boolean finalClass = Modifier.isFinal(clazz.getModifiers());
+ boolean finalMethod = Modifier.isFinal(clazz.getMethod("getObject").getModifiers());
+ if (finalClass || finalMethod) {
+ if (exposedType.isInterface()) {
+ if (logger.isDebugEnabled()) {
+ logger.debug("Creating interface proxy for FactoryBean '" + beanName + "' of type [" +
+ clazz.getName() + "] for use within another @Bean method because its " +
+ (finalClass ? "implementation class" : "getObject() method") +
+ " is final: Otherwise a getObject() call would not be routed to the factory.");
+ }
+ return createInterfaceProxyForFactoryBean(factoryBean, exposedType, beanFactory, beanName);
+ }
+ else {
+ if (logger.isInfoEnabled()) {
+ logger.info("Unable to proxy FactoryBean '" + beanName + "' of type [" +
+ clazz.getName() + "] for use within another @Bean method because its " +
+ (finalClass ? "implementation class" : "getObject() method") +
+ " is final: A getObject() call will NOT be routed to the factory. " +
+ "Consider declaring the return type as a FactoryBean interface.");
+ }
+ return factoryBean;
+ }
+ }
+ }
+ catch (NoSuchMethodException ex) {
+ // No getObject() method -> shouldn't happen, but as long as nobody is trying to call it...
+ }
+
+ return createCglibProxyForFactoryBean(factoryBean, beanFactory, beanName);
+ }
+
+ private Object createInterfaceProxyForFactoryBean(final Object factoryBean, Class interfaceType,
+ final ConfigurableBeanFactory beanFactory, final String beanName) {
+
+ return Proxy.newProxyInstance(
+ factoryBean.getClass().getClassLoader(), new Class[] {interfaceType},
+ new InvocationHandler() {
+ @Override
+ public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
+ if (method.getName().equals("getObject") && args == null) {
+ return beanFactory.getBean(beanName);
+ }
+ return ReflectionUtils.invokeMethod(method, factoryBean, args);
+ }
+ });
+ }
+
+ private Object createCglibProxyForFactoryBean(final Object factoryBean,
+ final ConfigurableBeanFactory beanFactory, final String beanName) {
Enhancer enhancer = new Enhancer();
enhancer.setSuperclass(factoryBean.getClass());
@@ -489,21 +563,6 @@ class ConfigurationClassEnhancer {
return fbProxy;
}
-
- private ConfigurableBeanFactory getBeanFactory(Object enhancedConfigInstance) {
- Field field = ReflectionUtils.findField(enhancedConfigInstance.getClass(), BEAN_FACTORY_FIELD);
- Assert.state(field != null, "Unable to find generated bean factory field");
- Object beanFactory = ReflectionUtils.getField(field, enhancedConfigInstance);
- Assert.state(beanFactory != null, "BeanFactory has not been injected into @Configuration class");
- Assert.state(beanFactory instanceof ConfigurableBeanFactory,
- "Injected BeanFactory is not a ConfigurableBeanFactory");
- return (ConfigurableBeanFactory) beanFactory;
- }
-
- @Override
- public boolean isMatch(Method candidateMethod) {
- return BeanAnnotationHelper.isBeanAnnotated(candidateMethod);
- }
}
}
diff --git a/spring-context/src/test/java/org/springframework/context/annotation/Spr15275Tests.java b/spring-context/src/test/java/org/springframework/context/annotation/Spr15275Tests.java
new file mode 100644
index 00000000000..a15c79df00f
--- /dev/null
+++ b/spring-context/src/test/java/org/springframework/context/annotation/Spr15275Tests.java
@@ -0,0 +1,250 @@
+/*
+ * Copyright 2002-2017 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 org.junit.Test;
+
+import org.springframework.beans.factory.FactoryBean;
+import org.springframework.beans.factory.config.AbstractFactoryBean;
+import org.springframework.context.ApplicationContext;
+
+import static org.junit.Assert.*;
+
+/**
+ * @author Juergen Hoeller
+ */
+public class Spr15275Tests {
+
+ @Test
+ public void testWithFactoryBean() {
+ ApplicationContext context = new AnnotationConfigApplicationContext(ConfigWithFactoryBean.class);
+ assertEquals("x", context.getBean(Bar.class).foo.toString());
+ assertSame(context.getBean(FooInterface.class), context.getBean(Bar.class).foo);
+ }
+
+ @Test
+ public void testWithAbstractFactoryBean() {
+ ApplicationContext context = new AnnotationConfigApplicationContext(ConfigWithAbstractFactoryBean.class);
+ assertEquals("x", context.getBean(Bar.class).foo.toString());
+ assertSame(context.getBean(FooInterface.class), context.getBean(Bar.class).foo);
+ }
+
+ @Test
+ public void testWithAbstractFactoryBeanForInterface() {
+ ApplicationContext context = new AnnotationConfigApplicationContext(ConfigWithAbstractFactoryBeanForInterface.class);
+ assertEquals("x", context.getBean(Bar.class).foo.toString());
+ assertSame(context.getBean(FooInterface.class), context.getBean(Bar.class).foo);
+ }
+
+ @Test
+ public void testWithAbstractFactoryBeanAsReturnType() {
+ ApplicationContext context = new AnnotationConfigApplicationContext(ConfigWithAbstractFactoryBeanAsReturnType.class);
+ assertEquals("x", context.getBean(Bar.class).foo.toString());
+ assertSame(context.getBean(FooInterface.class), context.getBean(Bar.class).foo);
+ }
+
+ @Test
+ public void testWithFinalFactoryBean() {
+ ApplicationContext context = new AnnotationConfigApplicationContext(ConfigWithFinalFactoryBean.class);
+ assertEquals("x", context.getBean(Bar.class).foo.toString());
+ assertSame(context.getBean(FooInterface.class), context.getBean(Bar.class).foo);
+ }
+
+ @Test
+ public void testWithFinalFactoryBeanAsReturnType() {
+ ApplicationContext context = new AnnotationConfigApplicationContext(ConfigWithFinalFactoryBeanAsReturnType.class);
+ assertEquals("x", context.getBean(Bar.class).foo.toString());
+ // not same due to fallback to raw FinalFactoryBean instance with repeated getObject() invocations
+ assertNotSame(context.getBean(FooInterface.class), context.getBean(Bar.class).foo);
+ }
+
+
+ @Configuration
+ protected static class ConfigWithFactoryBean {
+
+ @Bean
+ public FactoryBean foo() {
+ return new FactoryBean() {
+ @Override
+ public Foo getObject() {
+ return new Foo("x");
+ }
+ @Override
+ public Class getObjectType() {
+ return Foo.class;
+ }
+ };
+ }
+
+ @Bean
+ public Bar bar() throws Exception {
+ assertTrue(foo().isSingleton());
+ return new Bar(foo().getObject());
+ }
+ }
+
+
+ @Configuration
+ protected static class ConfigWithAbstractFactoryBean {
+
+ @Bean
+ public FactoryBean foo() {
+ return new AbstractFactoryBean() {
+ @Override
+ public Foo createInstance() {
+ return new Foo("x");
+ }
+ @Override
+ public Class getObjectType() {
+ return Foo.class;
+ }
+ };
+ }
+
+ @Bean
+ public Bar bar() throws Exception {
+ assertTrue(foo().isSingleton());
+ return new Bar(foo().getObject());
+ }
+ }
+
+
+ @Configuration
+ protected static class ConfigWithAbstractFactoryBeanForInterface {
+
+ @Bean
+ public FactoryBean foo() {
+ return new AbstractFactoryBean() {
+ @Override
+ public FooInterface createInstance() {
+ return new Foo("x");
+ }
+ @Override
+ public Class getObjectType() {
+ return FooInterface.class;
+ }
+ };
+ }
+
+ @Bean
+ public Bar bar() throws Exception {
+ assertTrue(foo().isSingleton());
+ return new Bar(foo().getObject());
+ }
+ }
+
+
+ @Configuration
+ protected static class ConfigWithAbstractFactoryBeanAsReturnType {
+
+ @Bean
+ public AbstractFactoryBean foo() {
+ return new AbstractFactoryBean() {
+ @Override
+ public FooInterface createInstance() {
+ return new Foo("x");
+ }
+ @Override
+ public Class getObjectType() {
+ return Foo.class;
+ }
+ };
+ }
+
+ @Bean
+ public Bar bar() throws Exception {
+ assertTrue(foo().isSingleton());
+ return new Bar(foo().getObject());
+ }
+ }
+
+
+ @Configuration
+ protected static class ConfigWithFinalFactoryBean {
+
+ @Bean
+ public FactoryBean foo() {
+ return new FinalFactoryBean();
+ }
+
+ @Bean
+ public Bar bar() throws Exception {
+ assertTrue(foo().isSingleton());
+ return new Bar(foo().getObject());
+ }
+ }
+
+
+ @Configuration
+ protected static class ConfigWithFinalFactoryBeanAsReturnType {
+
+ @Bean
+ public FinalFactoryBean foo() {
+ return new FinalFactoryBean();
+ }
+
+ @Bean
+ public Bar bar() throws Exception {
+ assertTrue(foo().isSingleton());
+ return new Bar(foo().getObject());
+ }
+ }
+
+
+ private static final class FinalFactoryBean implements FactoryBean {
+
+ @Override
+ public Foo getObject() {
+ return new Foo("x");
+ }
+
+ @Override
+ public Class getObjectType() {
+ return FooInterface.class;
+ }
+ };
+
+
+ protected interface FooInterface {
+ }
+
+
+ protected static class Foo implements FooInterface {
+
+ private final String value;
+
+ public Foo(String value) {
+ this.value = value;
+ }
+
+ @Override
+ public String toString() {
+ return this.value;
+ }
+ }
+
+
+ protected static class Bar {
+
+ public final FooInterface foo;
+
+ public Bar(FooInterface foo) {
+ this.foo = foo;
+ }
+ }
+
+}