Revised naming for prototype inner beans to avoid excessive unique name calculation

Restored original singleton-only adaptInnerBeanName behavior, avoiding endless unique name calculation for every single prototype instance. Non-named inner BeanDefinition objects now suffixed with an identity hex code analogous to non-named XML bean definitions, largely avoiding naming collisions to begin with. After SPR-11246, post-processors can deal with unstable classes per bean name, so occasional collisions aren't a hard problem anymore.

Issue: SPR-11545
(cherry picked from commit 5308b3e)

Author: jhoeller

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2014 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.
@@ -81,6 +81,7 @@ public BeanDefinitionValueResolver(
 		this.typeConverter = typeConverter;
 	}
 
+
 	/**
 	 * Given a PropertyValue, return a value, resolving any references to other
 	 * beans in the factory if necessary. The value could be:
@@ -123,7 +124,9 @@ else if (value instanceof BeanDefinitionHolder) {
 		else if (value instanceof BeanDefinition) {
 			// Resolve plain BeanDefinition, without contained name: use dummy name.
 			BeanDefinition bd = (BeanDefinition) value;
-			return resolveInnerBean(argName, "(inner bean)", bd);
+			String innerBeanName = "(inner bean)" + BeanFactoryUtils.GENERATED_BEAN_NAME_SEPARATOR +
+					ObjectUtils.getIdentityHexString(bd);
+			return resolveInnerBean(argName, innerBeanName, bd);
 		}
 		else if (value instanceof ManagedArray) {
 			// May need to resolve contained runtime references.
@@ -164,7 +167,7 @@ else if (value instanceof ManagedMap) {
 		else if (value instanceof ManagedProperties) {
 			Properties original = (Properties) value;
 			Properties copy = new Properties();
-			for (Map.Entry propEntry : original.entrySet()) {
+			for (Map.Entry<Object, Object> propEntry : original.entrySet()) {
 				Object propKey = propEntry.getKey();
 				Object propValue = propEntry.getValue();
 				if (propKey instanceof TypedStringValue) {
@@ -256,20 +259,25 @@ private Object resolveInnerBean(Object argName, String innerBeanName, BeanDefini
 			mbd = this.beanFactory.getMergedBeanDefinition(innerBeanName, innerBd, this.beanDefinition);
 			// Check given bean name whether it is unique. If not already unique,
 			// add counter - increasing the counter until the name is unique.
-			String actualInnerBeanName = adaptInnerBeanName(innerBeanName);
+			String actualInnerBeanName = innerBeanName;
+			if (mbd.isSingleton()) {
+				actualInnerBeanName = adaptInnerBeanName(innerBeanName);
+			}
 			this.beanFactory.registerContainedBean(actualInnerBeanName, this.beanName);
 			// Guarantee initialization of beans that the inner bean depends on.
 			String[] dependsOn = mbd.getDependsOn();
 			if (dependsOn != null) {
 				for (String dependsOnBean : dependsOn) {
-					this.beanFactory.getBean(dependsOnBean);
 					this.beanFactory.registerDependentBean(dependsOnBean, actualInnerBeanName);
+					this.beanFactory.getBean(dependsOnBean);
 				}
 			}
+			// Actually create the inner bean instance now...
 			Object innerBean = this.beanFactory.createBean(actualInnerBeanName, mbd, null);
 			if (innerBean instanceof FactoryBean) {
 				boolean synthetic = mbd.isSynthetic();
-				return this.beanFactory.getObjectFromFactoryBean((FactoryBean) innerBean, actualInnerBeanName, !synthetic);
+				return this.beanFactory.getObjectFromFactoryBean(
+						(FactoryBean<?>) innerBean, actualInnerBeanName, !synthetic);
 			}
 			else {
 				return innerBean;
@@ -344,7 +352,7 @@ private Object resolveManagedArray(Object argName, List<?> ml, Class<?> elementT
 	/**
 	 * For each element in the managed list, resolve reference if necessary.
 	 */
-	private List resolveManagedList(Object argName, List<?> ml) {
+	private List<?> resolveManagedList(Object argName, List<?> ml) {
 		List<Object> resolved = new ArrayList<Object>(ml.size());
 		for (int i = 0; i < ml.size(); i++) {
 			resolved.add(
@@ -356,7 +364,7 @@ private List resolveManagedList(Object argName, List<?> ml) {
 	/**
 	 * For each element in the managed set, resolve reference if necessary.
 	 */
-	private Set resolveManagedSet(Object argName, Set<?> ms) {
+	private Set<?> resolveManagedSet(Object argName, Set<?> ms) {
 		Set<Object> resolved = new LinkedHashSet<Object>(ms.size());
 		int i = 0;
 		for (Object m : ms) {
@@ -369,9 +377,9 @@ private Set resolveManagedSet(Object argName, Set<?> ms) {
 	/**
 	 * For each element in the managed map, resolve reference if necessary.
 	 */
-	private Map resolveManagedMap(Object argName, Map<?, ?> mm) {
+	private Map<?, ?> resolveManagedMap(Object argName, Map<?, ?> mm) {
 		Map<Object, Object> resolved = new LinkedHashMap<Object, Object>(mm.size());
-		for (Map.Entry entry : mm.entrySet()) {
+		for (Map.Entry<?, ?> entry : mm.entrySet()) {
 			Object resolvedKey = resolveValueIfNecessary(argName, entry.getKey());
 			Object resolvedValue = resolveValueIfNecessary(
 					new KeyedArgName(argName, entry.getKey()), entry.getValue());

spring-context/src/test/java/org/springframework/beans/factory/xml/XmlBeanFactoryTests-reftypes.xml

@@ -129,6 +129,15 @@
 		</constructor-arg>
 	</bean>
 
+	<bean id="hasInnerBeansAsPrototype" class="org.springframework.tests.sample.beans.TestBean" scope="prototype">
+		<constructor-arg>
+			<bean id="innerBean" class="org.springframework.tests.sample.beans.TestBean" destroy-method="destroy">
+				<constructor-arg><value>inner1</value></constructor-arg>
+				<constructor-arg type="int"><value>6</value></constructor-arg>
+			</bean>
+		</constructor-arg>
+	</bean>
+
 	<bean id="hasInnerBeansWithoutDestroy" class="org.springframework.tests.sample.beans.TestBean">
 		<constructor-arg><value>hasInner</value></constructor-arg>
 		<constructor-arg index="1" type="int"><value>5</value></constructor-arg>

spring-context/src/test/java/org/springframework/beans/factory/xml/XmlBeanFactoryTests.java

@@ -27,8 +27,8 @@
 import java.util.Map;
 
 import org.apache.commons.logging.LogFactory;
-
 import org.junit.Test;
+import org.xml.sax.InputSource;
 
 import org.springframework.aop.framework.ProxyFactory;
 import org.springframework.aop.support.AopUtils;
@@ -66,14 +66,12 @@
 import org.springframework.util.SerializationTestUtils;
 import org.springframework.util.StopWatch;
 
-import org.xml.sax.InputSource;
-
 import static org.hamcrest.CoreMatchers.*;
 import static org.junit.Assert.*;
 
 /**
  * Miscellaneous tests for XML bean definitions.
- * 
+ *
  * @author Juergen Hoeller
  * @author Rod Johnson
  * @author Rick Evans
@@ -230,6 +228,20 @@ public void testInnerBeans() throws IOException {
 		assertEquals("inner1", innerForConstructor.getName());
 		assertEquals(6, innerForConstructor.getAge());
 
+		hasInnerBeansForConstructor = (TestBean) xbf.getBean("hasInnerBeansAsPrototype");
+		innerForConstructor = (TestBean) hasInnerBeansForConstructor.getSpouse();
+		assertNotNull(innerForConstructor);
+		assertEquals("innerBean", innerForConstructor.getBeanName());
+		assertEquals("inner1", innerForConstructor.getName());
+		assertEquals(6, innerForConstructor.getAge());
+
+		hasInnerBeansForConstructor = (TestBean) xbf.getBean("hasInnerBeansAsPrototype");
+		innerForConstructor = (TestBean) hasInnerBeansForConstructor.getSpouse();
+		assertNotNull(innerForConstructor);
+		assertEquals("innerBean", innerForConstructor.getBeanName());
+		assertEquals("inner1", innerForConstructor.getName());
+		assertEquals(6, innerForConstructor.getAge());
+
 		xbf.destroySingletons();
 		assertTrue(inner1.wasDestroyed());
 		assertTrue(inner2.wasDestroyed());
@@ -390,11 +402,11 @@ public void testAutowireModeNotInherited() {
 		XmlBeanDefinitionReader reader = new XmlBeanDefinitionReader(xbf);
 		reader.loadBeanDefinitions(OVERRIDES_CONTEXT);
 
-		TestBean david = (TestBean)xbf.getBean("magicDavid");
+		TestBean david = (TestBean) xbf.getBean("magicDavid");
 		// the parent bean is autowiring
 		assertNotNull(david.getSpouse());
 
-		TestBean derivedDavid = (TestBean)xbf.getBean("magicDavidDerived");
+		TestBean derivedDavid = (TestBean) xbf.getBean("magicDavidDerived");
 		// this fails while it inherits from the child bean
 		assertNull("autowiring not propagated along child relationships", derivedDavid.getSpouse());
 	}
@@ -492,7 +504,7 @@ public void testSingletonInheritsFromParentFactoryPrototype() throws Exception {
 		DefaultListableBeanFactory child = new DefaultListableBeanFactory(parent);
 		new XmlBeanDefinitionReader(child).loadBeanDefinitions(CHILD_CONTEXT);
 		TestBean inherits = (TestBean) child.getBean("singletonInheritsFromParentFactoryPrototype");
-		// Name property value is overriden
+		// Name property value is overridden
 		assertTrue(inherits.getName().equals("prototype-override"));
 		// Age property is inherited from bean in parent factory
 		assertTrue(inherits.getAge() == 2);