Injection support for Collection/Map beans and self references

Issue: SPR-13585
Issue: SPR-12180
Issue: SPR-7915
Issue: SPR-8450

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)),
+			if (!(autowiredValue instanceof ObjectFactory || dependencyType.isInstance(autowiredValue))) {
-					"Value [" + autowiredValue + "] does not implement specified type [" + dependencyType.getName() + "]");
+				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<String, Object> 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<String, Object> entry = matchingBeans.entrySet().iterator().next();
+		if (autowiredBeanNames != null) {
+			autowiredBeanNames.add(entry.getKey());
+		}
+		return entry.getValue();
+	}
+
+	private Object resolveMultipleBeans(DependencyDescriptor descriptor, String beanName,
+			Set<String> autowiredBeanNames, TypeConverter typeConverter) {
+
+		Class<?> type = descriptor.getDependencyType();
 		if (type.isArray()) {
 			Class<?> componentType = type.getComponentType();
 			DependencyDescriptor targetDesc = new DependencyDescriptor(descriptor);
 			targetDesc.increaseNestingLevel();
 			Map<String, Object> 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<String, Object> 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<String, Object> 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<String, Object> matchingBeans = findAutowireCandidates(beanName, type, descriptor);
+			return NOT_MULTIPLE_BEANS;
-			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<String, Object> entry = matchingBeans.entrySet().iterator().next();
-			if (autowiredBeanNames != null) {
-				autowiredBeanNames.add(entry.getKey());
-			}
-			return entry.getValue();
 		}
 	}
 
@@ -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;
 	}

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 tb1 = new TestBean("tb1");
-		TestBean tb2 = new TestBean();
+		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 tb1 = new TestBean("tb1");
-		TestBean tb2 = new TestBean();
+		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<String, TestBean> tbm = new LinkedHashMap<String, TestBean>();
+		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<Object>());
+
+		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<TestBean> tbs = new LinkedHashSet<TestBean>();
+		tbs.add(new TestBean("tb1"));
+		tbs.add(new TestBean("tb2"));
+		bf.registerSingleton("testBeanSet", tbs);
+		bf.registerSingleton("otherSet", new HashSet<Object>());
+
+		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<String, TestBean> {
+	}
+
+
+	public static class MyTestBeanSet extends LinkedHashSet<TestBean> {
+	}
+
+
 	public static class MapConstructorInjectionBean {
 
 		private Map<String, TestBean> testBeanMap;
@@ -2259,6 +2374,28 @@ public class AutowiredAnnotationBeanPostProcessorTests {
 	}
 
 
+	public static class SetConstructorInjectionBean {
+
+		private Set<TestBean> testBeanSet;
+
+		@Autowired
+		public SetConstructorInjectionBean(Set<TestBean> testBeanSet) {
+			this.testBeanSet = testBeanSet;
+		}
+
+		public Set<TestBean> getTestBeanSet() {
+			return this.testBeanSet;
+		}
+	}
+
+
+	public static class SelfInjectionBean {
+
+		@Autowired
+		public SelfInjectionBean selfReference;
+	}
+
+
 	public static class MapFieldInjectionBean {
 
 		@Autowired

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();
 	}