From 4a0fa69ce469cae2e8c8a1a45f0b43f74a74481d Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Mon, 25 Jan 2016 21:25:48 +0100 Subject: [PATCH] Injection support for Collection/Map beans and self references Issue: SPR-13585 Issue: SPR-12180 Issue: SPR-7915 Issue: SPR-8450 --- .../support/DefaultListableBeanFactory.java | 110 ++++++------ ...wiredAnnotationBeanPostProcessorTests.java | 163 ++++++++++++++++-- .../ClassWithComplexConstructor.java | 6 +- 3 files changed, 217 insertions(+), 62 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java index 7a32060e28f..ed5e0b0de6f 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.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. @@ -45,7 +45,6 @@ import java.util.concurrent.ConcurrentHashMap; import javax.inject.Provider; import org.springframework.beans.BeansException; -import org.springframework.beans.FatalBeanException; import org.springframework.beans.TypeConverter; import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.BeanCurrentlyInCreationException; @@ -113,6 +112,9 @@ import org.springframework.util.StringUtils; public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFactory implements ConfigurableListableBeanFactory, BeanDefinitionRegistry, Serializable { + private static final Object NOT_MULTIPLE_BEANS = new Object(); + + private static Class javaUtilOptionalClass = null; private static Class javaxInjectProviderClass = null; @@ -615,10 +617,12 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto @Override public void registerResolvableDependency(Class dependencyType, Object autowiredValue) { - Assert.notNull(dependencyType, "Type must not be null"); + Assert.notNull(dependencyType, "Dependency type must not be null"); if (autowiredValue != null) { - Assert.isTrue((autowiredValue instanceof ObjectFactory || dependencyType.isInstance(autowiredValue)), - "Value [" + autowiredValue + "] does not implement specified type [" + dependencyType.getName() + "]"); + if (!(autowiredValue instanceof ObjectFactory || dependencyType.isInstance(autowiredValue))) { + throw new IllegalArgumentException("Value [" + autowiredValue + + "] does not implement specified dependency type [" + dependencyType.getName() + "]"); + } this.resolvableDependencies.put(dependencyType, autowiredValue); } } @@ -1034,15 +1038,54 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto converter.convertIfNecessary(value, type, descriptor.getMethodParameter())); } + Object multipleBeans = resolveMultipleBeans(descriptor, beanName, autowiredBeanNames, typeConverter); + if (multipleBeans != null && multipleBeans != NOT_MULTIPLE_BEANS) { + return multipleBeans; + } + + Map matchingBeans = findAutowireCandidates(beanName, type, descriptor); + if (matchingBeans.isEmpty()) { + if (descriptor.isRequired()) { + raiseNoSuchBeanDefinitionException(type, descriptor.getResolvableType().toString(), descriptor); + } + return null; + } + if (matchingBeans.size() > 1) { + String primaryBeanName = determineAutowireCandidate(matchingBeans, descriptor); + if (primaryBeanName == null) { + if (multipleBeans == NOT_MULTIPLE_BEANS || descriptor.isRequired()) { + throw new NoUniqueBeanDefinitionException(type, matchingBeans.keySet()); + } + else { + // In case of an optional Collection/Map, silently ignore a non-unique case: + // possibly it was meant to be an empty collection of multiple regular beans + // (before 4.3 in particular when we didn't even look for collection beans). + return null; + } + } + if (autowiredBeanNames != null) { + autowiredBeanNames.add(primaryBeanName); + } + return matchingBeans.get(primaryBeanName); + } + // We have exactly one match. + Map.Entry entry = matchingBeans.entrySet().iterator().next(); + if (autowiredBeanNames != null) { + autowiredBeanNames.add(entry.getKey()); + } + return entry.getValue(); + } + + private Object resolveMultipleBeans(DependencyDescriptor descriptor, String beanName, + Set autowiredBeanNames, TypeConverter typeConverter) { + + Class type = descriptor.getDependencyType(); if (type.isArray()) { Class componentType = type.getComponentType(); DependencyDescriptor targetDesc = new DependencyDescriptor(descriptor); targetDesc.increaseNestingLevel(); Map matchingBeans = findAutowireCandidates(beanName, componentType, targetDesc); if (matchingBeans.isEmpty()) { - if (descriptor.isRequired()) { - raiseNoSuchBeanDefinitionException(componentType, "array of " + componentType.getName(), descriptor); - } return null; } if (autowiredBeanNames != null) { @@ -1058,18 +1101,12 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto else if (Collection.class.isAssignableFrom(type) && type.isInterface()) { Class elementType = descriptor.getCollectionType(); if (elementType == null) { - if (descriptor.isRequired()) { - throw new FatalBeanException("No element type declared for collection [" + type.getName() + "]"); - } return null; } DependencyDescriptor targetDesc = new DependencyDescriptor(descriptor); targetDesc.increaseNestingLevel(); Map matchingBeans = findAutowireCandidates(beanName, elementType, targetDesc); if (matchingBeans.isEmpty()) { - if (descriptor.isRequired()) { - raiseNoSuchBeanDefinitionException(elementType, "collection of " + elementType.getName(), descriptor); - } return null; } if (autowiredBeanNames != null) { @@ -1085,26 +1122,16 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto else if (Map.class.isAssignableFrom(type) && type.isInterface()) { Class keyType = descriptor.getMapKeyType(); if (String.class != keyType) { - if (descriptor.isRequired()) { - throw new FatalBeanException("Key type [" + keyType + "] of map [" + type.getName() + - "] must be [java.lang.String]"); - } return null; } Class valueType = descriptor.getMapValueType(); if (valueType == null) { - if (descriptor.isRequired()) { - throw new FatalBeanException("No value type declared for map [" + type.getName() + "]"); - } return null; } DependencyDescriptor targetDesc = new DependencyDescriptor(descriptor); targetDesc.increaseNestingLevel(); Map matchingBeans = findAutowireCandidates(beanName, valueType, targetDesc); if (matchingBeans.isEmpty()) { - if (descriptor.isRequired()) { - raiseNoSuchBeanDefinitionException(valueType, "map with value type " + valueType.getName(), descriptor); - } return null; } if (autowiredBeanNames != null) { @@ -1113,29 +1140,7 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto return matchingBeans; } else { - Map matchingBeans = findAutowireCandidates(beanName, type, descriptor); - if (matchingBeans.isEmpty()) { - if (descriptor.isRequired()) { - raiseNoSuchBeanDefinitionException(type, "", descriptor); - } - return null; - } - if (matchingBeans.size() > 1) { - String primaryBeanName = determineAutowireCandidate(matchingBeans, descriptor); - if (primaryBeanName == null) { - throw new NoUniqueBeanDefinitionException(type, matchingBeans.keySet()); - } - if (autowiredBeanNames != null) { - autowiredBeanNames.add(primaryBeanName); - } - return matchingBeans.get(primaryBeanName); - } - // We have exactly one match. - Map.Entry entry = matchingBeans.entrySet().iterator().next(); - if (autowiredBeanNames != null) { - autowiredBeanNames.add(entry.getKey()); - } - return entry.getValue(); + return NOT_MULTIPLE_BEANS; } } @@ -1193,12 +1198,21 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto } } if (result.isEmpty()) { + // Consider fallback matches if the first pass failed to find anything... DependencyDescriptor fallbackDescriptor = descriptor.forFallbackMatch(); for (String candidateName : candidateNames) { - if (!candidateName.equals(beanName) && isAutowireCandidate(candidateName, fallbackDescriptor)) { + if (!isSelfReference(beanName, candidateName) && isAutowireCandidate(candidateName, fallbackDescriptor)) { result.put(candidateName, getBean(candidateName)); } } + if (result.isEmpty()) { + // Consider self references before as a final pass + for (String candidateName : candidateNames) { + if (isSelfReference(beanName, candidateName) && isAutowireCandidate(candidateName, fallbackDescriptor)) { + result.put(candidateName, getBean(candidateName)); + } + } + } } return result; } 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 index d04003b007f..58ab5f9122f 100644 --- 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 @@ -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. @@ -25,8 +25,13 @@ import java.lang.reflect.InvocationHandler; import java.lang.reflect.Method; import java.lang.reflect.Proxy; import java.util.Collections; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Properties; +import java.util.Set; import java.util.concurrent.Callable; import org.junit.Ignore; @@ -741,10 +746,10 @@ public class AutowiredAnnotationBeanPostProcessorTests { RootBeanDefinition bd = new RootBeanDefinition(MapConstructorInjectionBean.class); bd.setScope(RootBeanDefinition.SCOPE_PROTOTYPE); bf.registerBeanDefinition("annotatedBean", bd); - TestBean tb1 = new TestBean(); - TestBean tb2 = new TestBean(); + TestBean tb1 = new TestBean("tb1"); + TestBean tb2 = new TestBean("tb2"); bf.registerSingleton("testBean1", tb1); - bf.registerSingleton("testBean2", tb1); + bf.registerSingleton("testBean2", tb2); MapConstructorInjectionBean bean = (MapConstructorInjectionBean) bf.getBean("annotatedBean"); assertEquals(2, bean.getTestBeanMap().size()); @@ -770,10 +775,10 @@ public class AutowiredAnnotationBeanPostProcessorTests { RootBeanDefinition bd = new RootBeanDefinition(MapFieldInjectionBean.class); bd.setScope(RootBeanDefinition.SCOPE_PROTOTYPE); bf.registerBeanDefinition("annotatedBean", bd); - TestBean tb1 = new TestBean(); - TestBean tb2 = new TestBean(); + TestBean tb1 = new TestBean("tb1"); + TestBean tb2 = new TestBean("tb2"); bf.registerSingleton("testBean1", tb1); - bf.registerSingleton("testBean2", tb1); + bf.registerSingleton("testBean2", tb2); MapFieldInjectionBean bean = (MapFieldInjectionBean) bf.getBean("annotatedBean"); assertEquals(2, bean.getTestBeanMap().size()); @@ -829,7 +834,7 @@ public class AutowiredAnnotationBeanPostProcessorTests { bf.getBean("annotatedBean"); fail("should have failed, more than one bean of type"); } - catch (BeanCreationException e) { + catch (BeanCreationException ex) { // expected } bf.destroySingletons(); @@ -870,6 +875,108 @@ public class AutowiredAnnotationBeanPostProcessorTests { bf.destroySingletons(); } + @Test + public void testConstructorInjectionWithTypedMapAsBean() { + DefaultListableBeanFactory bf = new DefaultListableBeanFactory(); + bf.setAutowireCandidateResolver(new QualifierAnnotationAutowireCandidateResolver()); + AutowiredAnnotationBeanPostProcessor bpp = new AutowiredAnnotationBeanPostProcessor(); + bpp.setBeanFactory(bf); + bf.addBeanPostProcessor(bpp); + RootBeanDefinition bd = new RootBeanDefinition(MapConstructorInjectionBean.class); + bd.setScope(RootBeanDefinition.SCOPE_PROTOTYPE); + bf.registerBeanDefinition("annotatedBean", bd); + MyTestBeanMap tbm = new MyTestBeanMap(); + tbm.put("testBean1", new TestBean("tb1")); + tbm.put("testBean2", new TestBean("tb2")); + bf.registerSingleton("testBeans", tbm); + bf.registerSingleton("otherMap", new Properties()); + + MapConstructorInjectionBean bean = (MapConstructorInjectionBean) bf.getBean("annotatedBean"); + assertSame(tbm, bean.getTestBeanMap()); + bean = (MapConstructorInjectionBean) bf.getBean("annotatedBean"); + assertSame(tbm, bean.getTestBeanMap()); + } + + @Test + public void testConstructorInjectionWithPlainMapAsBean() { + DefaultListableBeanFactory bf = new DefaultListableBeanFactory(); + bf.setAutowireCandidateResolver(new QualifierAnnotationAutowireCandidateResolver()); + AutowiredAnnotationBeanPostProcessor bpp = new AutowiredAnnotationBeanPostProcessor(); + bpp.setBeanFactory(bf); + bf.addBeanPostProcessor(bpp); + RootBeanDefinition bd = new RootBeanDefinition(MapConstructorInjectionBean.class); + bd.setScope(RootBeanDefinition.SCOPE_PROTOTYPE); + bf.registerBeanDefinition("annotatedBean", bd); + Map tbm = new LinkedHashMap(); + tbm.put("testBean1", new TestBean("tb1")); + tbm.put("testBean2", new TestBean("tb2")); + bf.registerSingleton("testBeans", tbm); + bf.registerSingleton("otherMap", new Properties()); + + MapConstructorInjectionBean bean = (MapConstructorInjectionBean) bf.getBean("annotatedBean"); + assertSame(tbm, bean.getTestBeanMap()); + bean = (MapConstructorInjectionBean) bf.getBean("annotatedBean"); + assertSame(tbm, bean.getTestBeanMap()); + } + + @Test + public void testConstructorInjectionWithTypedSetAsBean() { + DefaultListableBeanFactory bf = new DefaultListableBeanFactory(); + bf.setAutowireCandidateResolver(new QualifierAnnotationAutowireCandidateResolver()); + AutowiredAnnotationBeanPostProcessor bpp = new AutowiredAnnotationBeanPostProcessor(); + bpp.setBeanFactory(bf); + bf.addBeanPostProcessor(bpp); + RootBeanDefinition bd = new RootBeanDefinition(SetConstructorInjectionBean.class); + bd.setScope(RootBeanDefinition.SCOPE_PROTOTYPE); + bf.registerBeanDefinition("annotatedBean", bd); + MyTestBeanSet tbs = new MyTestBeanSet(); + tbs.add(new TestBean("tb1")); + tbs.add(new TestBean("tb2")); + bf.registerSingleton("testBeans", tbs); + bf.registerSingleton("otherSet", new HashSet()); + + SetConstructorInjectionBean bean = (SetConstructorInjectionBean) bf.getBean("annotatedBean"); + assertSame(tbs, bean.getTestBeanSet()); + bean = (SetConstructorInjectionBean) bf.getBean("annotatedBean"); + assertSame(tbs, bean.getTestBeanSet()); + } + + @Test + public void testConstructorInjectionWithPlainSetAsBean() { + DefaultListableBeanFactory bf = new DefaultListableBeanFactory(); + bf.setAutowireCandidateResolver(new QualifierAnnotationAutowireCandidateResolver()); + AutowiredAnnotationBeanPostProcessor bpp = new AutowiredAnnotationBeanPostProcessor(); + bpp.setBeanFactory(bf); + bf.addBeanPostProcessor(bpp); + RootBeanDefinition bd = new RootBeanDefinition(SetConstructorInjectionBean.class); + bd.setScope(RootBeanDefinition.SCOPE_PROTOTYPE); + bf.registerBeanDefinition("annotatedBean", bd); + Set tbs = new LinkedHashSet(); + tbs.add(new TestBean("tb1")); + tbs.add(new TestBean("tb2")); + bf.registerSingleton("testBeanSet", tbs); + bf.registerSingleton("otherSet", new HashSet()); + + SetConstructorInjectionBean bean = (SetConstructorInjectionBean) bf.getBean("annotatedBean"); + assertSame(tbs, bean.getTestBeanSet()); + bean = (SetConstructorInjectionBean) bf.getBean("annotatedBean"); + assertSame(tbs, bean.getTestBeanSet()); + } + + @Test + public void testSelfReference() { + DefaultListableBeanFactory bf = new DefaultListableBeanFactory(); + bf.setAutowireCandidateResolver(new QualifierAnnotationAutowireCandidateResolver()); + AutowiredAnnotationBeanPostProcessor bpp = new AutowiredAnnotationBeanPostProcessor(); + bpp.setBeanFactory(bf); + bf.addBeanPostProcessor(bpp); + RootBeanDefinition bd = new RootBeanDefinition(SelfInjectionBean.class); + bf.registerBeanDefinition("annotatedBean", bd); + + SelfInjectionBean bean = (SelfInjectionBean) bf.getBean("annotatedBean"); + assertSame(bean, bean.selfReference); + } + @Test public void testObjectFactoryInjection() { DefaultListableBeanFactory bf = new DefaultListableBeanFactory(); @@ -972,7 +1079,7 @@ public class AutowiredAnnotationBeanPostProcessorTests { bf.getBean("customBean"); fail("expected BeanCreationException; no dependency available for required field"); } - catch (BeanCreationException e) { + catch (BeanCreationException ex) { // expected } bf.destroySingletons(); @@ -998,7 +1105,7 @@ public class AutowiredAnnotationBeanPostProcessorTests { bf.getBean("customBean"); fail("expected BeanCreationException; multiple beans of dependency type available"); } - catch (BeanCreationException e) { + catch (BeanCreationException ex) { // expected } bf.destroySingletons(); @@ -1065,7 +1172,7 @@ public class AutowiredAnnotationBeanPostProcessorTests { bf.getBean("customBean"); fail("expected BeanCreationException; multiple beans of dependency type available"); } - catch (BeanCreationException e) { + catch (BeanCreationException ex) { // expected } bf.destroySingletons(); @@ -1131,7 +1238,7 @@ public class AutowiredAnnotationBeanPostProcessorTests { bf.getBean("customBean"); fail("expected BeanCreationException; multiple beans of dependency type available"); } - catch (BeanCreationException e) { + catch (BeanCreationException ex) { // expected } bf.destroySingletons(); @@ -1197,7 +1304,7 @@ public class AutowiredAnnotationBeanPostProcessorTests { bf.getBean("customBean"); fail("expected BeanCreationException; multiple beans of dependency type available"); } - catch (BeanCreationException e) { + catch (BeanCreationException ex) { // expected } bf.destroySingletons(); @@ -2244,6 +2351,14 @@ public class AutowiredAnnotationBeanPostProcessorTests { } + public static class MyTestBeanMap extends LinkedHashMap { + } + + + public static class MyTestBeanSet extends LinkedHashSet { + } + + public static class MapConstructorInjectionBean { private Map testBeanMap; @@ -2259,6 +2374,28 @@ public class AutowiredAnnotationBeanPostProcessorTests { } + public static class SetConstructorInjectionBean { + + private Set testBeanSet; + + @Autowired + public SetConstructorInjectionBean(Set testBeanSet) { + this.testBeanSet = testBeanSet; + } + + public Set getTestBeanSet() { + return this.testBeanSet; + } + } + + + public static class SelfInjectionBean { + + @Autowired + public SelfInjectionBean selfReference; + } + + public static class MapFieldInjectionBean { @Autowired diff --git a/spring-context/src/test/java/org/springframework/aop/framework/ClassWithComplexConstructor.java b/spring-context/src/test/java/org/springframework/aop/framework/ClassWithComplexConstructor.java index 92118c0c5cb..19bb244bff0 100644 --- a/spring-context/src/test/java/org/springframework/aop/framework/ClassWithComplexConstructor.java +++ b/spring-context/src/test/java/org/springframework/aop/framework/ClassWithComplexConstructor.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. @@ -16,6 +16,7 @@ package org.springframework.aop.framework; +import org.springframework.aop.support.AopUtils; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; import org.springframework.util.Assert; @@ -28,6 +29,8 @@ public class ClassWithComplexConstructor { private final Dependency dependency; + @Autowired ClassWithComplexConstructor selfReference; + @Autowired public ClassWithComplexConstructor(Dependency dependency) { Assert.notNull(dependency); @@ -39,6 +42,7 @@ public class ClassWithComplexConstructor { } public void method() { + Assert.isTrue(this.selfReference != this && AopUtils.isCglibProxy(this.selfReference)); this.dependency.method(); }