Skip to content

Commit

Permalink
Fixed detection of generic types and qualifier annotations on scoped-…
Browse files Browse the repository at this point in the history 
…proxy factory methods

Issue: SPR-11116
  • Loading branch information
@jhoeller
jhoeller committed Dec 8, 2013
1 parent 1f9b833 commit 8580d2d
Show file tree
Hide file tree
Showing 5 changed files with 216 additions and 36 deletions.
4 changes: 2 additions & 2 deletions spring-aop/src/main/java/org/springframework/aop/scope/ScopedProxyUtils.java
View file
Expand Up @@ -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.
Expand Down
30 changes: 20 additions & 10 deletions ...pringframework/beans/factory/annotation/QualifierAnnotationAutowireCandidateResolver.java
View file
Expand Up @@ -216,18 +216,22 @@ protected boolean checkQualifier(

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) {
Expand All @@ -242,30 +246,31 @@ protected boolean checkQualifier(
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) {
Expand All @@ -278,6 +283,11 @@ protected boolean checkQualifier(
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.
Expand Down
60 changes: 44 additions & 16 deletions .../org/springframework/beans/factory/support/GenericTypeAwareAutowireCandidateResolver.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
}
Expand Down Expand Up @@ -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
Expand Down
103 changes: 99 additions & 4 deletions ...est/java/org/springframework/context/annotation/ConfigurationClassPostProcessorTests.java
View file
Expand Up @@ -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<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
Expand Down Expand Up @@ -295,12 +330,72 @@ public static class RepositoryConfiguration {

@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>";
}
};
}
}

Expand Down
55 changes: 51 additions & 4 deletions ...va/org/springframework/context/annotation/configuration/BeanMethodQualificationTests.java
View file
Expand Up @@ -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.*;
Expand All @@ -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 =
Expand All @@ -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");
}
Expand All @@ -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 {

Expand Down

0 comments on commit 8580d2d

Please sign in to comment.