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

Author: cbeams

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 @@
  * @author Rob Harrop
  * @author Mark Fisher
  * @author Costin Leau
+ * @author Chris Beams
  * @since 13.02.2004
  * @see RootBeanDefinition
  * @see DefaultListableBeanFactory
@@ -663,9 +664,10 @@ protected Class getTypeForFactoryMethod(String beanName, RootBeanDefinition mbd,
 	 */
 	@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 @@ protected Class<?> getTypeForFactoryBean(String beanName, RootBeanDefinition mbd
 					// 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 @@ protected Class<?> getTypeForFactoryBean(String beanName, RootBeanDefinition mbd
 
 		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;
 			}
 		}
 

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<Foo> 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<Foo> {
+
+	public Foo getObject() {
+		return new Foo();
+	}
+
+	public Class<Foo> getObjectType() {
+		return Foo.class;
+	}
+
+	public boolean isSingleton() {
+		return true;
+	}
+}

org.springframework.core/src/main/java/org/springframework/core/GenericTypeResolver.java

@@ -112,6 +112,7 @@ public static Class<?> resolveReturnType(Method method, Class clazz) {
 	 * 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)) {