Allow for specific instance-based match to override factory method signature match

Issue: SPR-11046

Author: jhoeller

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

@@ -87,7 +87,12 @@ protected boolean checkGenericTypeMatch(BeanDefinitionHolder bdHolder, Dependenc
 			else {
 				Method resolvedFactoryMethod = rbd.getResolvedFactoryMethod();
 				if (resolvedFactoryMethod != null) {
-					targetType = ResolvableType.forMethodReturnType(resolvedFactoryMethod);
+					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.
+						targetType = ResolvableType.forMethodReturnType(resolvedFactoryMethod);
+					}
 				}
 			}
 		}

spring-context/src/test/java/org/springframework/context/annotation/ConfigurationClassPostProcessorTests.java

@@ -16,8 +16,10 @@
 
 package org.springframework.context.annotation;
 
+import org.junit.Before;
 import org.junit.Test;
 
+import org.springframework.beans.factory.FactoryBean;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.AutowiredAnnotationBeanPostProcessor;
 import org.springframework.beans.factory.annotation.QualifierAnnotationAutowireCandidateResolver;
@@ -31,9 +33,21 @@
 
 /**
  * @author Chris Beams
+ * @author Juergen Hoeller
  */
 public class ConfigurationClassPostProcessorTests {
 
+	private DefaultListableBeanFactory beanFactory;
+
+	@Before
+	public void setUp() {
+		DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
+		QualifierAnnotationAutowireCandidateResolver acr = new QualifierAnnotationAutowireCandidateResolver();
+		acr.setBeanFactory(bf);
+		bf.setAutowireCandidateResolver(acr);
+		this.beanFactory = bf;
+	}
+
 	/**
 	 * Enhanced {@link Configuration} classes are only necessary for respecting
 	 * certain bean semantics, like singleton-scoping, scoped proxies, etc.
@@ -45,7 +59,6 @@ public class ConfigurationClassPostProcessorTests {
 	 */
 	@Test
 	public void testEnhancementIsPresentBecauseSingletonSemanticsAreRespected() {
-		DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory();
 		beanFactory.registerBeanDefinition("config", new RootBeanDefinition(SingletonBeanConfig.class));
 		ConfigurationClassPostProcessor pp = new ConfigurationClassPostProcessor();
 		pp.postProcessBeanFactory(beanFactory);
@@ -60,7 +73,6 @@ public void testEnhancementIsPresentBecauseSingletonSemanticsAreRespected() {
 	 */
 	@Test
 	public void testAlreadyLoadedConfigurationClasses() {
-		DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory();
 		beanFactory.registerBeanDefinition("unloadedConfig",
 				new RootBeanDefinition(UnloadedConfig.class.getName(), null, null));
 		beanFactory.registerBeanDefinition("loadedConfig", new RootBeanDefinition(LoadedConfig.class));
@@ -76,7 +88,6 @@ public void testAlreadyLoadedConfigurationClasses() {
 	 */
 	@Test
 	public void testPostProcessorIntrospectsInheritedDefinitionsCorrectly() {
-		DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory();
 		beanFactory.registerBeanDefinition("config", new RootBeanDefinition(SingletonBeanConfig.class));
 		beanFactory.registerBeanDefinition("parent", new RootBeanDefinition(TestBean.class));
 		beanFactory.registerBeanDefinition("child", new ChildBeanDefinition("parent"));
@@ -89,7 +100,6 @@ public void testPostProcessorIntrospectsInheritedDefinitionsCorrectly() {
 
 	@Test
 	public void testPostProcessorOverridesNonApplicationBeanDefinitions() {
-		DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory();
 		RootBeanDefinition rbd = new RootBeanDefinition(TestBean.class);
 		rbd.setRole(RootBeanDefinition.ROLE_SUPPORT);
 		beanFactory.registerBeanDefinition("bar", rbd);
@@ -103,7 +113,6 @@ public void testPostProcessorOverridesNonApplicationBeanDefinitions() {
 
 	@Test
 	public void testPostProcessorDoesNotOverrideRegularBeanDefinitions() {
-		DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory();
 		RootBeanDefinition rbd = new RootBeanDefinition(TestBean.class);
 		rbd.setResource(new DescriptiveResource("XML or something"));
 		beanFactory.registerBeanDefinition("bar", rbd);
@@ -137,32 +146,62 @@ public void testProcessingAllowedOnlyOncePerProcessorRegistryPair() {
 
 	@Test
 	public void testGenericsBasedInjection() {
-		DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
-		bf.setAutowireCandidateResolver(new QualifierAnnotationAutowireCandidateResolver());
 		AutowiredAnnotationBeanPostProcessor bpp = new AutowiredAnnotationBeanPostProcessor();
-		bpp.setBeanFactory(bf);
-		bf.addBeanPostProcessor(bpp);
+		bpp.setBeanFactory(beanFactory);
+		beanFactory.addBeanPostProcessor(bpp);
 		RootBeanDefinition bd = new RootBeanDefinition(RepositoryInjectionBean.class);
 		bd.setScope(RootBeanDefinition.SCOPE_PROTOTYPE);
-		bf.registerBeanDefinition("annotatedBean", bd);
-		bf.registerBeanDefinition("configClass", new RootBeanDefinition(RepositoryConfiguration.class));
+		beanFactory.registerBeanDefinition("annotatedBean", bd);
+		beanFactory.registerBeanDefinition("configClass", new RootBeanDefinition(RepositoryConfiguration.class));
+		ConfigurationClassPostProcessor pp = new ConfigurationClassPostProcessor();
+		pp.postProcessBeanFactory(beanFactory);
+
+		RepositoryInjectionBean bean = (RepositoryInjectionBean) beanFactory.getBean("annotatedBean");
+		assertSame(beanFactory.getBean("stringRepo"), bean.stringRepository);
+		assertSame(beanFactory.getBean("integerRepo"), bean.integerRepository);
+	}
+
+	@Test
+	public void testGenericsBasedInjectionWithImplTypeAtInjectionPoint() {
+		AutowiredAnnotationBeanPostProcessor bpp = new AutowiredAnnotationBeanPostProcessor();
+		bpp.setBeanFactory(beanFactory);
+		beanFactory.addBeanPostProcessor(bpp);
+		RootBeanDefinition bd = new RootBeanDefinition(SpecificRepositoryInjectionBean.class);
+		bd.setScope(RootBeanDefinition.SCOPE_PROTOTYPE);
+		beanFactory.registerBeanDefinition("annotatedBean", bd);
+		beanFactory.registerBeanDefinition("configClass", new RootBeanDefinition(SpecificRepositoryConfiguration.class));
+		ConfigurationClassPostProcessor pp = new ConfigurationClassPostProcessor();
+		pp.postProcessBeanFactory(beanFactory);
+		beanFactory.preInstantiateSingletons();
+
+		SpecificRepositoryInjectionBean bean = (SpecificRepositoryInjectionBean) beanFactory.getBean("annotatedBean");
+		assertSame(beanFactory.getBean("genericRepo"), bean.genericRepository);
+	}
+
+	@Test
+	public void testGenericsBasedInjectionWithFactoryBean() {
+		AutowiredAnnotationBeanPostProcessor bpp = new AutowiredAnnotationBeanPostProcessor();
+		bpp.setBeanFactory(beanFactory);
+		beanFactory.addBeanPostProcessor(bpp);
+		RootBeanDefinition bd = new RootBeanDefinition(RepositoryFactoryBeanInjectionBean.class);
+		bd.setScope(RootBeanDefinition.SCOPE_PROTOTYPE);
+		beanFactory.registerBeanDefinition("annotatedBean", bd);
+		beanFactory.registerBeanDefinition("configClass", new RootBeanDefinition(RepositoryFactoryBeanConfiguration.class));
 		ConfigurationClassPostProcessor pp = new ConfigurationClassPostProcessor();
-		pp.postProcessBeanFactory(bf);
+		pp.postProcessBeanFactory(beanFactory);
+		beanFactory.preInstantiateSingletons();
 
-		RepositoryInjectionBean bean = (RepositoryInjectionBean) bf.getBean("annotatedBean");
-		assertSame(bf.getBean("stringRepo"), bean.stringRepository);
-		assertSame(bf.getBean("integerRepo"), bean.integerRepository);
+		RepositoryFactoryBeanInjectionBean bean = (RepositoryFactoryBeanInjectionBean) beanFactory.getBean("annotatedBean");
+		assertSame(beanFactory.getBean("&repoFactoryBean"), bean.repositoryFactoryBean);
 	}
 
 	@Test
 	public void testGenericsBasedInjectionWithRawMatch() {
-		DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
-		bf.setAutowireCandidateResolver(new QualifierAnnotationAutowireCandidateResolver());
-		bf.registerBeanDefinition("configClass", new RootBeanDefinition(RawMatchingConfiguration.class));
+		beanFactory.registerBeanDefinition("configClass", new RootBeanDefinition(RawMatchingConfiguration.class));
 		ConfigurationClassPostProcessor pp = new ConfigurationClassPostProcessor();
-		pp.postProcessBeanFactory(bf);
+		pp.postProcessBeanFactory(beanFactory);
 
-		assertSame(bf.getBean("repo"), bf.getBean("repoConsumer"));
+		assertSame(beanFactory.getBean("repo"), beanFactory.getBean("repoConsumer"));
 	}
 
 
@@ -215,6 +254,29 @@ public static class Repository<T> {
 	}
 
 
+	public static class GenericRepository<T> extends Repository<T> {
+	}
+
+
+	public static class RepositoryFactoryBean<T> implements FactoryBean<T> {
+
+		@Override
+		public T getObject() {
+			throw new IllegalStateException();
+		}
+
+		@Override
+		public Class<?> getObjectType() {
+			return Object.class;
+		}
+
+		@Override
+		public boolean isSingleton() {
+			return false;
+		}
+	}
+
+
 	public static class RepositoryInjectionBean {
 
 		@Autowired
@@ -240,6 +302,40 @@ public Repository<Integer> integerRepo() {
 	}
 
 
+	public static class SpecificRepositoryInjectionBean {
+
+		@Autowired
+		public GenericRepository<?> genericRepository;
+	}
+
+
+	@Configuration
+	public static class SpecificRepositoryConfiguration {
+
+		@Bean
+		public Repository<Object> genericRepo() {
+			return new GenericRepository<Object>();
+		}
+	}
+
+
+	public static class RepositoryFactoryBeanInjectionBean {
+
+		@Autowired
+		public RepositoryFactoryBean<?> repositoryFactoryBean;
+	}
+
+
+	@Configuration
+	public static class RepositoryFactoryBeanConfiguration {
+
+		@Bean
+		public RepositoryFactoryBean<Object> repoFactoryBean() {
+			return new RepositoryFactoryBean<>();
+		}
+	}
+
+
 	@Configuration
 	public static class RawMatchingConfiguration {