From 48836e2ebb5990d9dd2f7656c0dfb66650b49e0d Mon Sep 17 00:00:00 2001 From: Chris Beams Date: Sat, 10 Dec 2011 19:32:02 +0000 Subject: [PATCH] Allow parameters in FactoryBean-returning @Bean methods Prior to this change, an assumption was made in AbstractAutowireCapableBeanFactory that any factory-method would have zero parameters. This may not be the case in @Bean methods. We now look for the factory-method by name in a more flexible fashion that accomodates the possibility of method parameters. There remains at least one edge cases here where things could still fail, for example a @Configuration class could have two FactoryBean-returning methods of the same name, but each with different generic FactoryBean types and different parameter lists. In this case, the implementation may infer and return the wrong object type, as it currently returns the first match for the given factory-method name. The complexity cost of ensuring that this never happens is not likely worth the trouble given the very low likelihood of such an arrangement. Issue: SPR-8762 --- .../AbstractAutowireCapableBeanFactory.java | 31 +++++--- ...tionWithFactoryBeanAndParametersTests.java | 77 +++++++++++++++++++ .../core/GenericTypeResolver.java | 1 + 3 files changed, 99 insertions(+), 10 deletions(-) create mode 100644 org.springframework.context/src/test/java/org/springframework/context/annotation/ConfigurationWithFactoryBeanAndParametersTests.java diff --git a/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java b/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java index 289c3ff146..67ec455140 100644 --- a/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java +++ b/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2010 the original author or authors. + * 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. @@ -105,6 +105,7 @@ import org.springframework.util.StringUtils; * @author Rob Harrop * @author Mark Fisher * @author Costin Leau + * @author Chris Beams * @since 13.02.2004 * @see RootBeanDefinition * @see DefaultListableBeanFactory @@ -663,9 +664,10 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac */ @Override protected Class getTypeForFactoryBean(String beanName, RootBeanDefinition mbd) { - Class objectType = null; + class Holder { Class value = null; } + final Holder objectType = new Holder(); String factoryBeanName = mbd.getFactoryBeanName(); - String factoryMethodName = mbd.getFactoryMethodName(); + final String factoryMethodName = mbd.getFactoryMethodName(); if (factoryBeanName != null && factoryMethodName != null) { // Try to obtain the FactoryBean's object type without instantiating it at all. BeanDefinition fbDef = getBeanDefinition(factoryBeanName); @@ -675,10 +677,19 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac // CGLIB subclass methods hide generic parameters. look at the superclass. fbClass = fbClass.getSuperclass(); } - Method m = ReflectionUtils.findMethod(fbClass, factoryMethodName); - objectType = GenericTypeResolver.resolveReturnTypeArgument(m, FactoryBean.class); - if (objectType != null) { - return objectType; + // find the given factory method, taking into account that in the case of + // @Bean methods, there may be parameters present. + ReflectionUtils.doWithMethods(fbClass, + new ReflectionUtils.MethodCallback() { + public void doWith(Method method) throws IllegalArgumentException, IllegalAccessException { + if (method.getName().equals(factoryMethodName) && + FactoryBean.class.isAssignableFrom(method.getReturnType())) { + objectType.value = GenericTypeResolver.resolveReturnTypeArgument(method, FactoryBean.class); + } + } + }); + if (objectType.value != null) { + return objectType.value; } } } @@ -689,9 +700,9 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac if (fb != null) { // Try to obtain the FactoryBean's object type from this early stage of the instance. - objectType = getTypeForFactoryBean(fb); - if (objectType != null) { - return objectType; + objectType.value = getTypeForFactoryBean(fb); + if (objectType.value != null) { + return objectType.value; } } diff --git a/org.springframework.context/src/test/java/org/springframework/context/annotation/ConfigurationWithFactoryBeanAndParametersTests.java b/org.springframework.context/src/test/java/org/springframework/context/annotation/ConfigurationWithFactoryBeanAndParametersTests.java new file mode 100644 index 0000000000..b2c1db67d9 --- /dev/null +++ b/org.springframework.context/src/test/java/org/springframework/context/annotation/ConfigurationWithFactoryBeanAndParametersTests.java @@ -0,0 +1,77 @@ +/* + * 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.junit.Assert.assertNotNull; + +import org.junit.Test; +import org.springframework.beans.factory.FactoryBean; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.context.ApplicationContext; + + +/** + * Test case cornering the bug initially raised with SPR-8762, in which a + * NullPointerException would be raised if a FactoryBean-returning @Bean method also + * accepts parameters + * + * @author Chris Beams + * @since 3.1 + */ +public class ConfigurationWithFactoryBeanAndParametersTests { + @Test + public void test() { + ApplicationContext ctx = new AnnotationConfigApplicationContext(Config.class, Bar.class); + assertNotNull(ctx.getBean(Bar.class).foo); + } +} + +@Configuration +class Config { + @Bean + public FactoryBean fb(@Value("42") String answer) { + return new FooFactoryBean(); + } +} + +class Foo { +} + +class Bar { + Foo foo; + + @Autowired + public Bar(Foo foo) { + this.foo = foo; + } +} + +class FooFactoryBean implements FactoryBean { + + public Foo getObject() { + return new Foo(); + } + + public Class getObjectType() { + return Foo.class; + } + + public boolean isSingleton() { + return true; + } +} diff --git a/org.springframework.core/src/main/java/org/springframework/core/GenericTypeResolver.java b/org.springframework.core/src/main/java/org/springframework/core/GenericTypeResolver.java index e00b6011f8..1111a1268c 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/GenericTypeResolver.java +++ b/org.springframework.core/src/main/java/org/springframework/core/GenericTypeResolver.java @@ -112,6 +112,7 @@ public abstract class GenericTypeResolver { * if not resolvable or if the single argument is of type {@link WildcardType}. */ public static Class resolveReturnTypeArgument(Method method, Class genericIfc) { + Assert.notNull(method, "method must not be null"); Type returnType = method.getReturnType(); Type genericReturnType = method.getGenericReturnType(); if (returnType.equals(genericIfc)) {