From 8580d2d19edcd10d0e0f4f45e482cc366cef4517 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Sun, 8 Dec 2013 23:24:16 +0100 Subject: [PATCH] Fixed detection of generic types and qualifier annotations on scoped-proxy factory methods Issue: SPR-11116 --- .../aop/scope/ScopedProxyUtils.java | 4 +- ...erAnnotationAutowireCandidateResolver.java | 30 +++-- ...ricTypeAwareAutowireCandidateResolver.java | 60 +++++++--- .../ConfigurationClassPostProcessorTests.java | 103 +++++++++++++++++- .../BeanMethodQualificationTests.java | 55 +++++++++- 5 files changed, 216 insertions(+), 36 deletions(-) diff --git a/spring-aop/src/main/java/org/springframework/aop/scope/ScopedProxyUtils.java b/spring-aop/src/main/java/org/springframework/aop/scope/ScopedProxyUtils.java index ce4f4e926b0f..2f2a89887c0f 100644 --- a/spring-aop/src/main/java/org/springframework/aop/scope/ScopedProxyUtils.java +++ b/spring-aop/src/main/java/org/springframework/aop/scope/ScopedProxyUtils.java @@ -50,17 +50,17 @@ public static BeanDefinitionHolder createScopedProxy(BeanDefinitionHolder defini 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. diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/annotation/QualifierAnnotationAutowireCandidateResolver.java b/spring-beans/src/main/java/org/springframework/beans/factory/annotation/QualifierAnnotationAutowireCandidateResolver.java index 35c36ce295ff..89df2d9ebe0e 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/annotation/QualifierAnnotationAutowireCandidateResolver.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/annotation/QualifierAnnotationAutowireCandidateResolver.java @@ -216,18 +216,22 @@ protected boolean checkQualifier( Class 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 @@ protected boolean checkQualifier( return true; } } + Map 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 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 @@ protected boolean checkQualifier( return true; } + protected Annotation getFactoryMethodAnnotation(RootBeanDefinition bd, Class type) { + Method resolvedFactoryMethod = bd.getResolvedFactoryMethod(); + return (resolvedFactoryMethod != null ? AnnotationUtils.getAnnotation(resolvedFactoryMethod, type) : null); + } + /** * Determine whether the given dependency carries a value annotation. 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 index 3aaf1b15e0fe..8b62873e1dc2 100644 --- 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 @@ -21,7 +21,9 @@ 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 @@ protected boolean checkGenericTypeMatch(BeanDefinitionHolder bdHolder, Dependenc 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 @@ protected boolean checkGenericTypeMatch(BeanDefinitionHolder bdHolder, Dependenc 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 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 index cdcc1ecc29da..19d87946fe19 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/ConfigurationClassPostProcessorTests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/ConfigurationClassPostProcessorTests.java @@ -158,8 +158,43 @@ public void testGenericsBasedInjection() { pp.postProcessBeanFactory(beanFactory); RepositoryInjectionBean bean = (RepositoryInjectionBean) beanFactory.getBean("annotatedBean"); - assertSame(beanFactory.getBean("stringRepo"), bean.stringRepository); - assertSame(beanFactory.getBean("integerRepo"), bean.integerRepository); + assertEquals("Repository", bean.stringRepository.toString()); + assertEquals("Repository", 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", bean.stringRepository.toString()); + assertEquals("Repository", 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", bean.stringRepository.toString()); + assertEquals("Repository", bean.integerRepository.toString()); } @Test @@ -295,12 +330,72 @@ public static class RepositoryConfiguration { @Bean public Repository stringRepo() { - return new Repository(); + return new Repository() { + @Override + public String toString() { + return "Repository"; + } + }; } @Bean public Repository integerRepo() { - return new Repository(); + return new Repository() { + @Override + public String toString() { + return "Repository"; + } + }; + } + } + + + @Configuration + public static class ScopedRepositoryConfiguration { + + @Bean @Scope("prototype") + public Repository stringRepo() { + return new Repository() { + @Override + public String toString() { + return "Repository"; + } + }; + } + + @Bean @Scope("prototype") + public Repository integerRepo() { + return new Repository() { + @Override + public String toString() { + return "Repository"; + } + }; + } + } + + + @Configuration + public static class ScopedProxyRepositoryConfiguration { + + @Bean @Scope(value="prototype", proxyMode=ScopedProxyMode.TARGET_CLASS) + public Repository stringRepo() { + return new Repository() { + @Override + public String toString() { + return "Repository"; + } + }; + } + + @Bean @Scope(value="prototype", proxyMode=ScopedProxyMode.TARGET_CLASS) + public Repository integerRepo() { + return new Repository() { + @Override + public String toString() { + return "Repository"; + } + }; } } diff --git a/spring-context/src/test/java/org/springframework/context/annotation/configuration/BeanMethodQualificationTests.java b/spring-context/src/test/java/org/springframework/context/annotation/configuration/BeanMethodQualificationTests.java index 7840a222fceb..65591d848584 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/configuration/BeanMethodQualificationTests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/configuration/BeanMethodQualificationTests.java @@ -21,16 +21,17 @@ 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 void testStandard() { 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 void testCustomWithAttributeOverride() { @Configuration static class StandardConfig { - @Bean @Lazy @Qualifier("interesting") + @Bean @Qualifier("interesting") @Lazy public TestBean testBean1() { return new TestBean("interesting"); } @@ -86,6 +105,34 @@ public TestBean testBean2() { } } + @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 {