instantiateUsingFactoryMethod avoids NPE and reports argument types in case of explicitArgs and resolved generic arguments as well

Issue: SPR-11517

Author: jhoeller

spring-beans/src/main/java/org/springframework/beans/factory/support/ConstructorResolver.java

@@ -523,24 +523,30 @@ else if (factoryMethodToUse != null && typeDiffWeight == minTypeDiffWeight &&
 			}
 
 			if (factoryMethodToUse == null) {
-				boolean hasArgs = (resolvedValues.getArgumentCount() > 0);
-				String argDesc = "";
-				if (hasArgs) {
-					List<String> argTypes = new ArrayList<String>();
-					for (ValueHolder value : resolvedValues.getIndexedArgumentValues().values()) {
-						String argType = (value.getType() != null ?
-								ClassUtils.getShortName(value.getType()) : value.getValue().getClass().getSimpleName());
+				List<String> argTypes = new ArrayList<String>(minNrOfArgs);
+				if (explicitArgs != null) {
+					for (Object arg : explicitArgs) {
+						argTypes.add(arg != null ? arg.getClass().getSimpleName() : "null");
+					}
+				}
+				else {
+					Set<ValueHolder> valueHolders = new LinkedHashSet<ValueHolder>(resolvedValues.getArgumentCount());
+					valueHolders.addAll(resolvedValues.getIndexedArgumentValues().values());
+					valueHolders.addAll(resolvedValues.getGenericArgumentValues());
+					for (ValueHolder value : valueHolders) {
+						String argType = (value.getType() != null ? ClassUtils.getShortName(value.getType()) :
+								(value.getValue() != null ? value.getValue().getClass().getSimpleName() : "null"));
 						argTypes.add(argType);
 					}
-					argDesc = StringUtils.collectionToCommaDelimitedString(argTypes);
 				}
+				String argDesc = StringUtils.collectionToCommaDelimitedString(argTypes);
 				throw new BeanCreationException(mbd.getResourceDescription(), beanName,
 						"No matching factory method found: " +
 						(mbd.getFactoryBeanName() != null ?
 							"factory bean '" + mbd.getFactoryBeanName() + "'; " : "") +
 						"factory method '" + mbd.getFactoryMethodName() + "(" + argDesc + ")'. " +
 						"Check that a method with the specified name " +
-						(hasArgs ? "and arguments " : "") +
+						(minNrOfArgs > 0 ? "and arguments " : "") +
 						"exists and that it is " +
 						(isStatic ? "static" : "non-static") + ".");
 			}

spring-beans/src/test/java/org/springframework/beans/factory/xml/FactoryMethodTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2014 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.
@@ -21,7 +21,6 @@
 import java.util.List;
 import java.util.Properties;
 
-import static org.junit.Assert.*;
 import org.junit.Test;
 
 import org.springframework.beans.factory.BeanCreationException;
@@ -30,6 +29,8 @@
 import org.springframework.core.io.ClassPathResource;
 import org.springframework.tests.sample.beans.TestBean;
 
+import static org.junit.Assert.*;
+
 /**
  * @author Juergen Hoeller
  * @author Chris Beams
@@ -259,6 +260,34 @@ public void testFactoryMethodNoMatchingStaticMethod() {
 		}
 	}
 
+	@Test
+	public void testNonExistingFactoryMethod() {
+		DefaultListableBeanFactory xbf = new DefaultListableBeanFactory();
+		XmlBeanDefinitionReader reader = new XmlBeanDefinitionReader(xbf);
+		reader.loadBeanDefinitions(new ClassPathResource("factory-methods.xml", getClass()));
+		try {
+			xbf.getBean("invalidPrototype");
+			fail("Should have thrown BeanCreationException");
+		}
+		catch (BeanCreationException ex) {
+			assertTrue(ex.getMessage().contains("nonExisting(TestBean)"));
+		}
+	}
+
+	@Test
+	public void testFactoryMethodArgumentsForNonExistingMethod() {
+		DefaultListableBeanFactory xbf = new DefaultListableBeanFactory();
+		XmlBeanDefinitionReader reader = new XmlBeanDefinitionReader(xbf);
+		reader.loadBeanDefinitions(new ClassPathResource("factory-methods.xml", getClass()));
+		try {
+			xbf.getBean("invalidPrototype", new TestBean());
+			fail("Should have thrown BeanCreationException");
+		}
+		catch (BeanCreationException ex) {
+			assertTrue(ex.getMessage().contains("nonExisting(TestBean)"));
+		}
+	}
+
 	@Test
 	public void testCanSpecifyFactoryMethodArgumentsOnFactoryMethodPrototype() {
 		DefaultListableBeanFactory xbf = new DefaultListableBeanFactory();
@@ -360,7 +389,9 @@ public void testFactoryMethodForJavaMailSession() {
 
 }
 
+
 class MailSession {
+
 	private Properties props;
 
 	private MailSession() {
@@ -377,6 +408,6 @@ public static MailSession getDefaultInstance(Properties props) {
 	}
 
 	public Object getProperty(String key) {
-		return props.get(key);
+		return this.props.get(key);
 	}
 }

spring-beans/src/test/resources/org/springframework/beans/factory/xml/factory-methods.xml

@@ -84,6 +84,11 @@
 		<property name="stringValue"><value>testBeanOnlyPrototypeDISetterString</value></property>
 	</bean>
 
+	<bean id="invalidPrototype" class="org.springframework.beans.factory.xml.FactoryMethods"
+		  factory-method="nonExisting" scope="prototype">
+		<constructor-arg><ref local="juergen"/></constructor-arg>
+	</bean>
+
 	<bean id="fullPrototype" class="org.springframework.beans.factory.xml.FactoryMethods"
 			factory-method="newInstance" scope="prototype">
 		<constructor-arg type="int"><value>27</value></constructor-arg>
@@ -120,9 +125,7 @@
 		<constructor-arg><value type="java.lang.Integer">33</value></constructor-arg>
 	</bean>
 
-	<bean id="instanceFactoryMethodWithoutArgs"
-			factory-bean="instanceFactory"
-			factory-method="defaultInstance"/>
+	<bean id="instanceFactoryMethodWithoutArgs" factory-bean="instanceFactory" factory-method="defaultInstance"/>
 
 	<!-- Unnamed bean with factory-bean declaration -->
 	<bean factory-bean="instanceFactory" factory-method="defaultInstance"/>