Fixed detection of generic types and qualifier annotations on scoped-proxy factory methods

Issue: SPR-11116

spring-aop/src/main/java/org/springframework/aop/scope/ScopedProxyUtils.java

@@ -50,17 +50,17 @@ public abstract class ScopedProxyUtils {
 
 		String originalBeanName = definition.getBeanName();
 		BeanDefinition targetDefinition = definition.getBeanDefinition();
+		String targetBeanName = getTargetBeanName(originalBeanName);
 
 		// Create a scoped proxy definition for the original bean name,
 		// "hiding" the target bean in an internal target definition.
 		RootBeanDefinition proxyDefinition = new RootBeanDefinition(ScopedProxyFactoryBean.class);
+		proxyDefinition.setDecoratedDefinition(new BeanDefinitionHolder(targetDefinition, targetBeanName));
 		proxyDefinition.setOriginatingBeanDefinition(targetDefinition);
 		proxyDefinition.setSource(definition.getSource());
 		proxyDefinition.setRole(BeanDefinition.ROLE_INFRASTRUCTURE);
 
-		String targetBeanName = getTargetBeanName(originalBeanName);
 		proxyDefinition.getPropertyValues().add("targetBeanName", targetBeanName);
-
 		if (proxyTargetClass) {
 			targetDefinition.setAttribute(AutoProxyUtils.PRESERVE_TARGET_CLASS_ATTRIBUTE, Boolean.TRUE);
 			// ScopedProxyFactoryBean's "proxyTargetClass" default is TRUE, so we don't need to set it explicitly here.

spring-beans/src/main/java/org/springframework/beans/factory/annotation/QualifierAnnotationAutowireCandidateResolver.java

@@ -216,18 +216,22 @@ public class QualifierAnnotationAutowireCandidateResolver extends GenericTypeAwa
 
 		Class<? extends Annotation> type = annotation.annotationType();
 		RootBeanDefinition bd = (RootBeanDefinition) bdHolder.getBeanDefinition();
+
 		AutowireCandidateQualifier qualifier = bd.getQualifier(type.getName());
 		if (qualifier == null) {
 			qualifier = bd.getQualifier(ClassUtils.getShortName(type));
 		}
 		if (qualifier == null) {
-			Annotation targetAnnotation = null;
-			Method resolvedFactoryMethod = bd.getResolvedFactoryMethod();
-			if (resolvedFactoryMethod != null) {
-				targetAnnotation = AnnotationUtils.getAnnotation(resolvedFactoryMethod, type);
+			// First, check annotation on factory method, if applicable
+			Annotation targetAnnotation = getFactoryMethodAnnotation(bd, type);
+			if (targetAnnotation == null) {
+				RootBeanDefinition dbd = getResolvedDecoratedDefinition(bd);
+				if (dbd != null) {
+					targetAnnotation = getFactoryMethodAnnotation(dbd, type);
+				}
 			}
 			if (targetAnnotation == null) {
-				// look for matching annotation on the target class
+				// Look for matching annotation on the target class
 				if (getBeanFactory() != null) {
 					Class<?> beanType = getBeanFactory().getType(bdHolder.getBeanName());
 					if (beanType != null) {
@@ -242,30 +246,31 @@ public class QualifierAnnotationAutowireCandidateResolver extends GenericTypeAwa
 				return true;
 			}
 		}
+
 		Map<String, Object> attributes = AnnotationUtils.getAnnotationAttributes(annotation);
 		if (attributes.isEmpty() && qualifier == null) {
-			// if no attributes, the qualifier must be present
+			// If no attributes, the qualifier must be present
 			return false;
 		}
 		for (Map.Entry<String, Object> entry : attributes.entrySet()) {
 			String attributeName = entry.getKey();
 			Object expectedValue = entry.getValue();
 			Object actualValue = null;
-			// check qualifier first
+			// Check qualifier first
 			if (qualifier != null) {
 				actualValue = qualifier.getAttribute(attributeName);
 			}
 			if (actualValue == null) {
-				// fall back on bean definition attribute
+				// Fall back on bean definition attribute
 				actualValue = bd.getAttribute(attributeName);
 			}
 			if (actualValue == null && attributeName.equals(AutowireCandidateQualifier.VALUE_KEY) &&
 					expectedValue instanceof String && bdHolder.matchesName((String) expectedValue)) {
-				// fall back on bean name (or alias) match
+				// Fall back on bean name (or alias) match
 				continue;
 			}
 			if (actualValue == null && qualifier != null) {
-				// fall back on default, but only if the qualifier is present
+				// Fall back on default, but only if the qualifier is present
 				actualValue = AnnotationUtils.getDefaultValue(annotation, attributeName);
 			}
 			if (actualValue != null) {
@@ -278,6 +283,11 @@ public class QualifierAnnotationAutowireCandidateResolver extends GenericTypeAwa
 		return true;
 	}
 
+	protected Annotation getFactoryMethodAnnotation(RootBeanDefinition bd, Class<? extends Annotation> type) {
+		Method resolvedFactoryMethod = bd.getResolvedFactoryMethod();
+		return (resolvedFactoryMethod != null ? AnnotationUtils.getAnnotation(resolvedFactoryMethod, type) : null);
+	}
+
 
 	/**
 	 * Determine whether the given dependency carries a value annotation.

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

@@ -21,7 +21,9 @@ import java.lang.reflect.Method;
 import org.springframework.beans.factory.BeanFactory;
 import org.springframework.beans.factory.BeanFactoryAware;
 import org.springframework.beans.factory.FactoryBean;
+import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.beans.factory.config.BeanDefinitionHolder;
+import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
 import org.springframework.beans.factory.config.DependencyDescriptor;
 import org.springframework.core.ResolvableType;
 import org.springframework.util.ClassUtils;
@@ -77,22 +79,13 @@ public class GenericTypeAwareAutowireCandidateResolver implements AutowireCandid
 		if (bdHolder.getBeanDefinition() instanceof RootBeanDefinition) {
 			rbd = (RootBeanDefinition) bdHolder.getBeanDefinition();
 		}
-		if (rbd != null && rbd.getFactoryMethodName() != null) {
-			// 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) {
-				targetType = ResolvableType.forClass(preResolved);
-			}
-			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.
-						targetType = ResolvableType.forMethodReturnType(resolvedFactoryMethod);
-					}
+		if (rbd != null) {
+			// First, check factory method return type, if applicable
+			targetType = getReturnTypeForFactoryMethod(rbd, descriptor);
+			if (targetType == null) {
+				RootBeanDefinition dbd = getResolvedDecoratedDefinition(rbd);
+				if (dbd != null) {
+					targetType = getReturnTypeForFactoryMethod(dbd, descriptor);
 				}
 			}
 		}
@@ -122,6 +115,41 @@ public class GenericTypeAwareAutowireCandidateResolver implements AutowireCandid
 		return dependencyType.isAssignableFrom(targetType);
 	}
 
+	protected RootBeanDefinition getResolvedDecoratedDefinition(RootBeanDefinition rbd) {
+		BeanDefinitionHolder decDef = rbd.getDecoratedDefinition();
+		if (decDef != null && this.beanFactory instanceof ConfigurableListableBeanFactory) {
+			ConfigurableListableBeanFactory clbf = (ConfigurableListableBeanFactory) this.beanFactory;
+			if (clbf.containsBeanDefinition(decDef.getBeanName())) {
+				BeanDefinition dbd = clbf.getMergedBeanDefinition(decDef.getBeanName());
+				if (dbd instanceof RootBeanDefinition) {
+					return (RootBeanDefinition) dbd;
+				}
+			}
+		}
+		return null;
+	}
+
+	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);
+		}
+		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);
+				}
+			}
+			return null;
+		}
+	}
+
 
 	/**
 	 * This implementation always returns {@code null}, leaving suggested value support up

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

@@ -158,8 +158,43 @@ public class ConfigurationClassPostProcessorTests {
 		pp.postProcessBeanFactory(beanFactory);
 
 		RepositoryInjectionBean bean = (RepositoryInjectionBean) beanFactory.getBean("annotatedBean");
-		assertSame(beanFactory.getBean("stringRepo"), bean.stringRepository);
-		assertSame(beanFactory.getBean("integerRepo"), bean.integerRepository);
+		assertEquals("Repository<String>", bean.stringRepository.toString());
+		assertEquals("Repository<Integer>", bean.integerRepository.toString());
+	}
+
+	@Test
+	public void testGenericsBasedInjectionWithScoped() {
+		AutowiredAnnotationBeanPostProcessor bpp = new AutowiredAnnotationBeanPostProcessor();
+		bpp.setBeanFactory(beanFactory);
+		beanFactory.addBeanPostProcessor(bpp);
+		RootBeanDefinition bd = new RootBeanDefinition(RepositoryInjectionBean.class);
+		bd.setScope(RootBeanDefinition.SCOPE_PROTOTYPE);
+		beanFactory.registerBeanDefinition("annotatedBean", bd);
+		beanFactory.registerBeanDefinition("configClass", new RootBeanDefinition(ScopedRepositoryConfiguration.class));
+		ConfigurationClassPostProcessor pp = new ConfigurationClassPostProcessor();
+		pp.postProcessBeanFactory(beanFactory);
+
+		RepositoryInjectionBean bean = (RepositoryInjectionBean) beanFactory.getBean("annotatedBean");
+		assertEquals("Repository<String>", bean.stringRepository.toString());
+		assertEquals("Repository<Integer>", bean.integerRepository.toString());
+	}
+
+	@Test
+	public void testGenericsBasedInjectionWithScopedProxy() {
+		AutowiredAnnotationBeanPostProcessor bpp = new AutowiredAnnotationBeanPostProcessor();
+		bpp.setBeanFactory(beanFactory);
+		beanFactory.addBeanPostProcessor(bpp);
+		RootBeanDefinition bd = new RootBeanDefinition(RepositoryInjectionBean.class);
+		bd.setScope(RootBeanDefinition.SCOPE_PROTOTYPE);
+		beanFactory.registerBeanDefinition("annotatedBean", bd);
+		beanFactory.registerBeanDefinition("configClass", new RootBeanDefinition(ScopedProxyRepositoryConfiguration.class));
+		ConfigurationClassPostProcessor pp = new ConfigurationClassPostProcessor();
+		pp.postProcessBeanFactory(beanFactory);
+		beanFactory.freezeConfiguration();
+
+		RepositoryInjectionBean bean = (RepositoryInjectionBean) beanFactory.getBean("annotatedBean");
+		assertEquals("Repository<String>", bean.stringRepository.toString());
+		assertEquals("Repository<Integer>", bean.integerRepository.toString());
 	}
 
 	@Test
@@ -295,12 +330,72 @@ public class ConfigurationClassPostProcessorTests {
 
 		@Bean
 		public Repository<String> stringRepo() {
-			return new Repository<String>();
+			return new Repository<String>() {
+				@Override
+				public String toString() {
+					return "Repository<String>";
+				}
+			};
 		}
 
 		@Bean
 		public Repository<Integer> integerRepo() {
-			return new Repository<Integer>();
+			return new Repository<Integer>() {
+				@Override
+				public String toString() {
+					return "Repository<Integer>";
+				}
+			};
+		}
+	}
+
+
+	@Configuration
+	public static class ScopedRepositoryConfiguration {
+
+		@Bean @Scope("prototype")
+		public Repository<String> stringRepo() {
+			return new Repository<String>() {
+				@Override
+				public String toString() {
+					return "Repository<String>";
+				}
+			};
+		}
+
+		@Bean @Scope("prototype")
+		public Repository<Integer> integerRepo() {
+			return new Repository<Integer>() {
+				@Override
+				public String toString() {
+					return "Repository<Integer>";
+				}
+			};
+		}
+	}
+
+
+	@Configuration
+	public static class ScopedProxyRepositoryConfiguration {
+
+		@Bean @Scope(value="prototype", proxyMode=ScopedProxyMode.TARGET_CLASS)
+		public Repository<String> stringRepo() {
+			return new Repository<String>() {
+				@Override
+				public String toString() {
+					return "Repository<String>";
+				}
+			};
+		}
+
+		@Bean @Scope(value="prototype", proxyMode=ScopedProxyMode.TARGET_CLASS)
+		public Repository<Integer> integerRepo() {
+			return new Repository<Integer>() {
+				@Override
+				public String toString() {
+					return "Repository<Integer>";
+				}
+			};
 		}
 	}
 

spring-context/src/test/java/org/springframework/context/annotation/configuration/BeanMethodQualificationTests.java

@@ -21,16 +21,17 @@ import java.lang.annotation.RetentionPolicy;
 
 import org.junit.Test;
 
-import org.springframework.tests.sample.beans.NestedTestBean;
-import org.springframework.tests.sample.beans.TestBean;
-
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Qualifier;
 import org.springframework.context.annotation.AnnotationConfigApplicationContext;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.Lazy;
+import org.springframework.context.annotation.Scope;
+import org.springframework.context.annotation.ScopedProxyMode;
 import org.springframework.stereotype.Component;
+import org.springframework.tests.sample.beans.NestedTestBean;
+import org.springframework.tests.sample.beans.TestBean;
 
 import static org.hamcrest.CoreMatchers.*;
 import static org.junit.Assert.*;
@@ -53,6 +54,24 @@ public class BeanMethodQualificationTests {
 		assertThat(pojo.testBean.getName(), equalTo("interesting"));
 	}
 
+	@Test
+	public void testScoped() {
+		AnnotationConfigApplicationContext ctx =
+				new AnnotationConfigApplicationContext(ScopedConfig.class, StandardPojo.class);
+		assertFalse(ctx.getBeanFactory().containsSingleton("testBean1"));
+		StandardPojo pojo = ctx.getBean(StandardPojo.class);
+		assertThat(pojo.testBean.getName(), equalTo("interesting"));
+	}
+
+	@Test
+	public void testScopedProxy() {
+		AnnotationConfigApplicationContext ctx =
+				new AnnotationConfigApplicationContext(ScopedProxyConfig.class, StandardPojo.class);
+		assertTrue(ctx.getBeanFactory().containsSingleton("testBean1"));  // a shared scoped proxy
+		StandardPojo pojo = ctx.getBean(StandardPojo.class);
+		assertThat(pojo.testBean.getName(), equalTo("interesting"));
+	}
+
 	@Test
 	public void testCustom() {
 		AnnotationConfigApplicationContext ctx =
@@ -75,7 +94,7 @@ public class BeanMethodQualificationTests {
 	@Configuration
 	static class StandardConfig {
 
-		@Bean @Lazy @Qualifier("interesting")
+		@Bean @Qualifier("interesting") @Lazy
 		public TestBean testBean1() {
 			return new TestBean("interesting");
 		}
@@ -86,6 +105,34 @@ public class BeanMethodQualificationTests {
 		}
 	}
 
+	@Configuration
+	static class ScopedConfig {
+
+		@Bean @Qualifier("interesting") @Scope("prototype")
+		public TestBean testBean1() {
+			return new TestBean("interesting");
+		}
+
+		@Bean @Qualifier("boring") @Scope("prototype")
+		public TestBean testBean2() {
+			return new TestBean("boring");
+		}
+	}
+
+	@Configuration
+	static class ScopedProxyConfig {
+
+		@Bean @Qualifier("interesting") @Scope(value="prototype", proxyMode=ScopedProxyMode.TARGET_CLASS)
+		public TestBean testBean1() {
+			return new TestBean("interesting");
+		}
+
+		@Bean @Qualifier("boring") @Scope(value="prototype", proxyMode=ScopedProxyMode.TARGET_CLASS)
+		public TestBean testBean2() {
+			return new TestBean("boring");
+		}
+	}
+
 	@Component @Lazy
 	static class StandardPojo {