ResolvableType-based matching respects generic factory method return type

jhoeller · jhoeller · commit 4c005e6336e1 · 2016-12-17T23:10:48.000+01:00
Includes consistent use of ResolvableType.resolve() wherever applicable.

Issue: SPR-15011
diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/NoSuchBeanDefinitionException.java b/spring-beans/src/main/java/org/springframework/beans/factory/NoSuchBeanDefinitionException.java
@@ -109,7 +109,7 @@ public String getBeanName() {
 	 * that failed.
 	 */
 	public Class<?> getBeanType() {
-		return (this.resolvableType != null ? this.resolvableType.getRawClass() : null);
+		return (this.resolvableType != null ? this.resolvableType.resolve() : null);
 	}
 
 	/**
diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java
@@ -74,6 +74,7 @@
 import org.springframework.core.MethodParameter;
 import org.springframework.core.ParameterNameDiscoverer;
 import org.springframework.core.PriorityOrdered;
+import org.springframework.core.ResolvableType;
 import org.springframework.util.ClassUtils;
 import org.springframework.util.ObjectUtils;
 import org.springframework.util.ReflectionUtils;
@@ -672,9 +673,9 @@ protected Class<?> determineTargetType(String beanName, RootBeanDefinition mbd,
 	 * @see #createBean
 	 */
 	protected Class<?> getTypeForFactoryMethod(String beanName, RootBeanDefinition mbd, Class<?>... typesToMatch) {
-		Class<?> preResolved = mbd.resolvedFactoryMethodReturnType;
-		if (preResolved != null) {
-			return preResolved;
+		ResolvableType cachedReturnType = mbd.factoryMethodReturnType;
+		if (cachedReturnType != null) {
+			return cachedReturnType.resolve();
 		}
 
 		Class<?> factoryClass;
@@ -698,11 +699,12 @@ protected Class<?> getTypeForFactoryMethod(String beanName, RootBeanDefinition m
 		if (factoryClass == null) {
 			return null;
 		}
+		factoryClass = ClassUtils.getUserClass(factoryClass);
 
 		// If all factory methods have the same return type, return that type.
 		// Can't clearly figure out exact method due to type converting / autowiring!
 		Class<?> commonType = null;
-		boolean cache = false;
+		Method uniqueCandidate = null;
 		int minNrOfArgs = mbd.getConstructorArgumentValues().getArgumentCount();
 		Method[] candidates = ReflectionUtils.getUniqueDeclaredMethods(factoryClass);
 		for (Method factoryMethod : candidates) {
@@ -736,8 +738,12 @@ protected Class<?> getTypeForFactoryMethod(String beanName, RootBeanDefinition m
 						Class<?> returnType = AutowireUtils.resolveReturnTypeForFactoryMethod(
 								factoryMethod, args, getBeanClassLoader());
 						if (returnType != null) {
-							cache = true;
+							uniqueCandidate = (commonType == null ? factoryMethod : null);
 							commonType = ClassUtils.determineCommonAncestor(returnType, commonType);
+							if (commonType == null) {
+								// Ambiguous return types found: return null to indicate "not determinable".
+								return null;
+							}
 						}
 					}
 					catch (Throwable ex) {
@@ -747,22 +753,22 @@ protected Class<?> getTypeForFactoryMethod(String beanName, RootBeanDefinition m
 					}
 				}
 				else {
+					uniqueCandidate = (commonType == null ? factoryMethod : null);
 					commonType = ClassUtils.determineCommonAncestor(factoryMethod.getReturnType(), commonType);
+					if (commonType == null) {
+						// Ambiguous return types found: return null to indicate "not determinable".
+						return null;
+					}
 				}
 			}
 		}
 
 		if (commonType != null) {
 			// Clear return type found: all factory methods return same type.
-			if (cache) {
-				mbd.resolvedFactoryMethodReturnType = commonType;
-			}
-			return commonType;
-		}
-		else {
-			// Ambiguous return types found: return null to indicate "not determinable".
-			return null;
+			mbd.factoryMethodReturnType = (uniqueCandidate != null ?
+					ResolvableType.forMethodReturnType(uniqueCandidate) : ResolvableType.forClass(commonType));
 		}
+		return commonType;
 	}
 
 	/**
diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java
@@ -513,7 +513,10 @@ else if (containsSingleton(beanName) && !containsBeanDefinition(beanName)) {
 			// Retrieve corresponding bean definition.
 			RootBeanDefinition mbd = getMergedLocalBeanDefinition(beanName);
 
-			Class<?> classToMatch = typeToMatch.getRawClass();
+			Class<?> classToMatch = typeToMatch.resolve();
+			if (classToMatch == null) {
+				classToMatch = FactoryBean.class;
+			}
 			Class<?>[] typesToMatch = (FactoryBean.class == classToMatch ?
 					new Class<?>[] {classToMatch} : new Class<?>[] {FactoryBean.class, classToMatch});
 
@@ -553,6 +556,13 @@ else if (BeanFactoryUtils.isFactoryDereference(name)) {
 				}
 			}
 
+			ResolvableType resolvableType = mbd.targetType;
+			if (resolvableType == null) {
+				resolvableType = mbd.factoryMethodReturnType;
+			}
+			if (resolvableType != null && resolvableType.resolve() == beanType) {
+				return typeToMatch.isAssignableFrom(resolvableType);
+			}
 			return typeToMatch.isAssignableFrom(beanType);
 		}
 	}
@@ -1443,6 +1453,10 @@ protected Object evaluateBeanDefinitionString(String value, BeanDefinition beanD
 	 * @return the type of the bean, or {@code null} if not predictable
 	 */
 	protected Class<?> predictBeanType(String beanName, RootBeanDefinition mbd, Class<?>... typesToMatch) {
+		Class<?> targetType = mbd.getTargetType();
+		if (targetType != null) {
+			return targetType;
+		}
 		if (mbd.getFactoryMethodName() != null) {
 			return null;
 		}
diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/AutowireUtils.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/AutowireUtils.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2016 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.
@@ -162,8 +162,8 @@ public static Object resolveAutowiringValue(Object autowiringValue, Class<?> req
 	 * Determine the target type for the generic return type of the given
 	 * <em>generic factory method</em>, where formal type variables are declared
 	 * on the given method itself.
-	 * <p>For example, given a factory method with the following signature,
-	 * if {@code resolveReturnTypeForFactoryMethod()} is invoked with the reflected
+	 * <p>For example, given a factory method with the following signature, if
+	 * {@code resolveReturnTypeForFactoryMethod()} is invoked with the reflected
 	 * method for {@code creatProxy()} and an {@code Object[]} array containing
 	 * {@code MyService.class}, {@code resolveReturnTypeForFactoryMethod()} will
 	 * infer that the target return type is {@code MyService}.
@@ -184,9 +184,9 @@ public static Object resolveAutowiringValue(Object autowiringValue, Class<?> req
 	 * @param method the method to introspect (never {@code null})
 	 * @param args the arguments that will be supplied to the method when it is
 	 * invoked (never {@code null})
-	 * @param classLoader the ClassLoader to resolve class names against, if necessary
-	 * (never {@code null})
-	 * @return the resolved target return type, the standard return type, or {@code null}
+	 * @param classLoader the ClassLoader to resolve class names against,
+	 * if necessary (never {@code null})
+	 * @return the resolved target return type or the standard method return type
 	 * @since 3.2.5
 	 */
 	public static Class<?> resolveReturnTypeForFactoryMethod(Method method, Object[] args, ClassLoader classLoader) {
diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/GenericTypeAwareAutowireCandidateResolver.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/GenericTypeAwareAutowireCandidateResolver.java
@@ -147,22 +147,23 @@ protected RootBeanDefinition getResolvedDecoratedDefinition(RootBeanDefinition r
 	protected ResolvableType getReturnTypeForFactoryMethod(RootBeanDefinition rbd, DependencyDescriptor descriptor) {
 		// Should typically be set for any kind of factory method, since the BeanFactory
 		// pre-resolves them before reaching out to the AutowireCandidateResolver...
-		Class<?> preResolved = rbd.resolvedFactoryMethodReturnType;
-		if (preResolved != null) {
-			return ResolvableType.forClass(preResolved);
+		ResolvableType returnType = rbd.factoryMethodReturnType;
+		if (returnType == null) {
+			Method factoryMethod = rbd.getResolvedFactoryMethod();
+			if (factoryMethod != null) {
+				returnType = ResolvableType.forMethodReturnType(factoryMethod);
+			}
 		}
-		else {
-			Method resolvedFactoryMethod = rbd.getResolvedFactoryMethod();
-			if (resolvedFactoryMethod != null) {
-				if (descriptor.getDependencyType().isAssignableFrom(resolvedFactoryMethod.getReturnType())) {
-					// Only use factory method metadata if the return type is actually expressive enough
-					// for our dependency. Otherwise, the returned instance type may have matched instead
-					// in case of a singleton instance having been registered with the container already.
-					return ResolvableType.forMethodReturnType(resolvedFactoryMethod);
-				}
+		if (returnType != null) {
+			Class<?> resolvedClass = returnType.resolve();
+			if (resolvedClass != null && descriptor.getDependencyType().isAssignableFrom(resolvedClass)) {
+				// Only use factory method metadata if the return type is actually expressive enough
+				// for our dependency. Otherwise, the returned instance type may have matched instead
+				// in case of a singleton instance having been registered with the container already.
+				return returnType;
 			}
-			return null;
 		}
+		return null;
 	}
 
 
diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java
@@ -65,7 +65,7 @@ public class RootBeanDefinition extends AbstractBeanDefinition {
 	volatile Class<?> resolvedTargetType;
 
 	/** Package-visible field for caching the return type of a generically typed factory method */
-	volatile Class<?> resolvedFactoryMethodReturnType;
+	volatile ResolvableType factoryMethodReturnType;
 
 	/** Common lock for the four constructor fields below */
 	final Object constructorArgumentLock = new Object();
diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/StaticListableBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/StaticListableBeanFactory.java
@@ -248,7 +248,13 @@ public String[] getBeanDefinitionNames() {
 
 	@Override
 	public String[] getBeanNamesForType(ResolvableType type) {
-		boolean isFactoryType = (type != null && FactoryBean.class.isAssignableFrom(type.getRawClass()));
+		boolean isFactoryType = false;
+		if (type != null) {
+			Class<?> resolved = type.resolve();
+			if (resolved != null && FactoryBean.class.isAssignableFrom(resolved)) {
+				isFactoryType = true;
+			}
+		}
 		List<String> matches = new ArrayList<>();
 		for (Map.Entry<String, Object> entry : this.beans.entrySet()) {
 			String name = entry.getKey();
diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessorTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessorTests.java
@@ -1295,7 +1295,6 @@ public void testCustomAnnotationRequiredFieldResourceInjectionFailsWhenMultipleD
 		}
 		catch (UnsatisfiedDependencyException ex) {
 			// expected
-			ex.printStackTrace();
 			assertSame(CustomAnnotationRequiredFieldResourceInjectionBean.class,
 					ex.getInjectionPoint().getField().getDeclaringClass());
 		}
@@ -1681,9 +1680,6 @@ public void testGenericsBasedFieldInjectionWithMocks() {
 		assertSame(ir, bean.integerRepositoryMap.get("integerRepository"));
 	}
 
-	@Qualifier("integerRepo")
-	private Repository<?> integerRepositoryQualifierProvider;
-
 	@Test
 	public void testGenericsBasedFieldInjectionWithSimpleMatch() {
 		DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
@@ -2194,6 +2190,10 @@ public void testAnnotatedDefaultConstructor() {
 	}
 
 
+	@Qualifier("integerRepo")
+	private Repository<?> integerRepositoryQualifierProvider;
+
+
 	public static class ResourceInjectionBean {
 
 		@Autowired(required = false)
diff --git a/spring-context/src/main/java/org/springframework/context/event/ApplicationListenerMethodAdapter.java b/spring-context/src/main/java/org/springframework/context/event/ApplicationListenerMethodAdapter.java
@@ -343,17 +343,15 @@ private ResolvableType getResolvableType(ApplicationEvent event) {
 		ResolvableType payloadType = null;
 		if (event instanceof PayloadApplicationEvent) {
 			PayloadApplicationEvent<?> payloadEvent = (PayloadApplicationEvent<?>) event;
-			payloadType = payloadEvent.getResolvableType().as(
-					PayloadApplicationEvent.class).getGeneric(0);
+			payloadType = payloadEvent.getResolvableType().as(PayloadApplicationEvent.class).getGeneric();
 		}
 		for (ResolvableType declaredEventType : this.declaredEventTypes) {
-			if (!ApplicationEvent.class.isAssignableFrom(declaredEventType.getRawClass())
-					&& payloadType != null) {
+			if (!ApplicationEvent.class.isAssignableFrom(declaredEventType.getRawClass()) && payloadType != null) {
 				if (declaredEventType.isAssignableFrom(payloadType)) {
 					return declaredEventType;
 				}
 			}
-			if (declaredEventType.getRawClass().isAssignableFrom(event.getClass())) {
+			if (declaredEventType.getRawClass().isInstance(event)) {
 				return declaredEventType;
 			}
 		}
diff --git a/spring-context/src/main/java/org/springframework/context/event/GenericApplicationListenerAdapter.java b/spring-context/src/main/java/org/springframework/context/event/GenericApplicationListenerAdapter.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2016 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.
@@ -60,8 +60,8 @@ public void onApplicationEvent(ApplicationEvent event) {
 	@SuppressWarnings("unchecked")
 	public boolean supportsEventType(ResolvableType eventType) {
 		if (this.delegate instanceof SmartApplicationListener) {
-			Class<? extends ApplicationEvent> eventClass = (Class<? extends ApplicationEvent>) eventType.getRawClass();
-			return ((SmartApplicationListener) this.delegate).supportsEventType(eventClass);
+			Class<? extends ApplicationEvent> eventClass = (Class<? extends ApplicationEvent>) eventType.resolve();
+			return (eventClass != null && ((SmartApplicationListener) this.delegate).supportsEventType(eventClass));
 		}
 		else {
 			return (this.declaredEventType == null || this.declaredEventType.isAssignableFrom(eventType));
@@ -70,7 +70,7 @@ public boolean supportsEventType(ResolvableType eventType) {
 
 	@Override
 	public boolean supportsEventType(Class<? extends ApplicationEvent> eventType) {
-		return supportsEventType(ResolvableType.forType(eventType));
+		return supportsEventType(ResolvableType.forClass(eventType));
 	}
 
 	@Override
diff --git a/spring-context/src/test/java/org/springframework/context/annotation/ConfigurationClassPostProcessorTests.java b/spring-context/src/test/java/org/springframework/context/annotation/ConfigurationClassPostProcessorTests.java
@@ -43,13 +43,15 @@
 import org.springframework.beans.factory.support.RootBeanDefinition;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.annotation.componentscan.simple.SimpleComponent;
+import org.springframework.core.ResolvableType;
 import org.springframework.core.annotation.Order;
 import org.springframework.core.env.StandardEnvironment;
 import org.springframework.core.io.DescriptiveResource;
 import org.springframework.stereotype.Component;
 import org.springframework.tests.sample.beans.ITestBean;
 import org.springframework.tests.sample.beans.TestBean;
 import org.springframework.util.Assert;
+import org.springframework.util.ObjectUtils;
 
 import static org.junit.Assert.*;
 
@@ -522,6 +524,33 @@ public void genericsBasedInjectionWithWildcardWithGenericExtendsMatch() {
 		assertSame(beanFactory.getBean("genericRepo"), beanFactory.getBean("repoConsumer"));
 	}
 
+	@Test
+	public void genericsBasedInjectionWithLateGenericsMatching() {
+		beanFactory.registerBeanDefinition("configClass", new RootBeanDefinition(RepositoryConfiguration.class));
+		new ConfigurationClassPostProcessor().postProcessBeanFactory(beanFactory);
+		beanFactory.preInstantiateSingletons();
+
+		String[] beanNames = beanFactory.getBeanNamesForType(Repository.class);
+		assertTrue(ObjectUtils.containsElement(beanNames, "stringRepo"));
+
+		beanNames = beanFactory.getBeanNamesForType(ResolvableType.forClassWithGenerics(Repository.class, String.class));
+		assertEquals(1, beanNames.length);
+		assertEquals("stringRepo", beanNames[0]);
+	}
+
+	@Test
+	public void genericsBasedInjectionWithEarlyGenericsMatching() {
+		beanFactory.registerBeanDefinition("configClass", new RootBeanDefinition(RepositoryConfiguration.class));
+		new ConfigurationClassPostProcessor().postProcessBeanFactory(beanFactory);
+
+		String[] beanNames = beanFactory.getBeanNamesForType(Repository.class);
+		assertTrue(ObjectUtils.containsElement(beanNames, "stringRepo"));
+
+		beanNames = beanFactory.getBeanNamesForType(ResolvableType.forClassWithGenerics(Repository.class, String.class));
+		assertEquals(1, beanNames.length);
+		assertEquals("stringRepo", beanNames[0]);
+	}
+
 	@Test
 	public void testSelfReferenceExclusionForFactoryMethodOnSameBean() {
 		AutowiredAnnotationBeanPostProcessor bpp = new AutowiredAnnotationBeanPostProcessor();
diff --git a/spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/InvocableHandlerMethod.java b/spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/InvocableHandlerMethod.java
@@ -283,7 +283,7 @@ public Class<?> getParameterType() {
 				return this.returnValue.getClass();
 			}
 			if (!ResolvableType.NONE.equals(this.returnType)) {
-				return this.returnType.getRawClass();
+				return this.returnType.resolve();
 			}
 			return super.getParameterType();
 		}
diff --git a/spring-messaging/src/main/java/org/springframework/messaging/simp/stomp/DefaultStompSession.java b/spring-messaging/src/main/java/org/springframework/messaging/simp/stomp/DefaultStompSession.java
@@ -436,7 +436,7 @@ private void invokeHandler(StompFrameHandler handler, Message<byte[]> message, S
 			return;
 		}
 		Type type = handler.getPayloadType(stompHeaders);
-		Class<?> payloadType = ResolvableType.forType(type).getRawClass();
+		Class<?> payloadType = ResolvableType.forType(type).resolve();
 		Object object = getMessageConverter().fromMessage(message, payloadType);
 		if (object == null) {
 			throw new MessageConversionException("No suitable converter, payloadType=" + payloadType +
diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethod.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethod.java
@@ -285,7 +285,7 @@ public Class<?> getParameterType() {
 				return this.returnValue.getClass();
 			}
 			if (!ResolvableType.NONE.equals(this.returnType)) {
-				return this.returnType.getRawClass();
+				return this.returnType.resolve();
 			}
 			return super.getParameterType();
 		}

Original file line number	Diff line number	Diff line change
`@@ -109,7 +109,7 @@ public String getBeanName() {`
`109`	`109`	`* that failed.`
`110`	`110`	`*/`
`111`	`111`	`public Class<?> getBeanType() {`
`112`		`- return (this.resolvableType != null ? this.resolvableType.getRawClass() : null);`
	`112`	`+ return (this.resolvableType != null ? this.resolvableType.resolve() : null);`
`113`	`113`	`}`
`114`	`114`
`115`	`115`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -1295,7 +1295,6 @@ public void testCustomAnnotationRequiredFieldResourceInjectionFailsWhenMultipleD`
`1295`	`1295`	`}`
`1296`	`1296`	`catch (UnsatisfiedDependencyException ex) {`
`1297`	`1297`	`// expected`
`1298`		`- ex.printStackTrace();`
`1299`	`1298`	`assertSame(CustomAnnotationRequiredFieldResourceInjectionBean.class,`
`1300`	`1299`	`ex.getInjectionPoint().getField().getDeclaringClass());`
`1301`	`1300`	`}`
`@@ -1681,9 +1680,6 @@ public void testGenericsBasedFieldInjectionWithMocks() {`
`1681`	`1680`	`assertSame(ir, bean.integerRepositoryMap.get("integerRepository"));`
`1682`	`1681`	`}`
`1683`	`1682`
`1684`		`- @Qualifier("integerRepo")`
`1685`		`- private Repository<?> integerRepositoryQualifierProvider;`
`1686`		`-`
`1687`	`1683`	`@Test`
`1688`	`1684`	`public void testGenericsBasedFieldInjectionWithSimpleMatch() {`
`1689`	`1685`	`DefaultListableBeanFactory bf = new DefaultListableBeanFactory();`
`@@ -2194,6 +2190,10 @@ public void testAnnotatedDefaultConstructor() {`
`2194`	`2190`	`}`
`2195`	`2191`
`2196`	`2192`
	`2193`	`+ @Qualifier("integerRepo")`
	`2194`	`+ private Repository<?> integerRepositoryQualifierProvider;`
	`2195`	`+`
	`2196`	`+`
`2197`	`2197`	`public static class ResourceInjectionBean {`
`2198`	`2198`
`2199`	`2199`	`@Autowired(required = false)`
Original file line number	Diff line number	Diff line change
`@@ -343,17 +343,15 @@ private ResolvableType getResolvableType(ApplicationEvent event) {`
`343`	`343`	`ResolvableType payloadType = null;`
`344`	`344`	`if (event instanceof PayloadApplicationEvent) {`
`345`	`345`	`PayloadApplicationEvent<?> payloadEvent = (PayloadApplicationEvent<?>) event;`
`346`		`- payloadType = payloadEvent.getResolvableType().as(`
`347`		`- PayloadApplicationEvent.class).getGeneric(0);`
	`346`	`+ payloadType = payloadEvent.getResolvableType().as(PayloadApplicationEvent.class).getGeneric();`
`348`	`347`	`}`
`349`	`348`	`for (ResolvableType declaredEventType : this.declaredEventTypes) {`
`350`		`- if (!ApplicationEvent.class.isAssignableFrom(declaredEventType.getRawClass())`
`351`		`- && payloadType != null) {`
	`349`	`+ if (!ApplicationEvent.class.isAssignableFrom(declaredEventType.getRawClass()) && payloadType != null) {`
`352`	`350`	`if (declaredEventType.isAssignableFrom(payloadType)) {`
`353`	`351`	`return declaredEventType;`
`354`	`352`	`}`
`355`	`353`	`}`
`356`		`- if (declaredEventType.getRawClass().isAssignableFrom(event.getClass())) {`
	`354`	`+ if (declaredEventType.getRawClass().isInstance(event)) {`
`357`	`355`	`return declaredEventType;`
`358`	`356`	`}`
`359`	`357`	`}`