Retain merged bean definition caches when possible

Update the logic in `AbstractBeanFactory` so that caches from merged
bean definitions remain whenever possible.

Prior to this commit, all merged bean definitions would be completely
removed after bean post processing in case a processor changed the bean
type. It's fairly unlikely these days that the bean type will actually
change, so instead we now compare a subset of the old cached properties
against the newly created definition. Only if key properties have
changed do we now discard the older cached values.

Closes gh-23336

Author: philwebb

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

@@ -114,6 +114,7 @@
  * @author Costin Leau
  * @author Chris Beams
  * @author Sam Brannen
+ * @author Phillip Webb
  * @since 13.02.2004
  * @see RootBeanDefinition
  * @see DefaultListableBeanFactory
@@ -646,16 +647,16 @@ else if (!this.allowRawInjectionDespiteWrapping && hasDependentBean(beanName)) {
 	@Nullable
 	protected Class<?> predictBeanType(String beanName, RootBeanDefinition mbd, Class<?>... typesToMatch) {
 		Class<?> targetType = determineTargetType(beanName, mbd, typesToMatch);
-
 		// Apply SmartInstantiationAwareBeanPostProcessors to predict the
 		// eventual type after a before-instantiation shortcut.
 		if (targetType != null && !mbd.isSynthetic() && hasInstantiationAwareBeanPostProcessors()) {
+			boolean matchingOnlyFactoryBean = typesToMatch.length == 1 && typesToMatch[0] == FactoryBean.class;
 			for (BeanPostProcessor bp : getBeanPostProcessors()) {
 				if (bp instanceof SmartInstantiationAwareBeanPostProcessor) {
 					SmartInstantiationAwareBeanPostProcessor ibp = (SmartInstantiationAwareBeanPostProcessor) bp;
 					Class<?> predicted = ibp.predictBeanType(targetType, beanName);
-					if (predicted != null && (typesToMatch.length != 1 || FactoryBean.class != typesToMatch[0] ||
-							FactoryBean.class.isAssignableFrom(predicted))) {
+					if (predicted != null &&
+							(!matchingOnlyFactoryBean || FactoryBean.class.isAssignableFrom(predicted))) {
 						return predicted;
 					}
 				}

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

@@ -103,6 +103,7 @@
  * @author Juergen Hoeller
  * @author Costin Leau
  * @author Chris Beams
+ * @author Phillip Webb
  * @since 15 April 2001
  * @see #getBeanDefinition
  * @see #createBean
@@ -1215,7 +1216,7 @@ protected void registerCustomEditors(PropertyEditorRegistry registry) {
 	protected RootBeanDefinition getMergedLocalBeanDefinition(String beanName) throws BeansException {
 		// Quick check on the concurrent map first, with minimal locking.
 		RootBeanDefinition mbd = this.mergedBeanDefinitions.get(beanName);
-		if (mbd != null) {
+		if (mbd != null && !mbd.stale) {
 			return mbd;
 		}
 		return getMergedBeanDefinition(beanName, getBeanDefinition(beanName));
@@ -1251,13 +1252,16 @@ protected RootBeanDefinition getMergedBeanDefinition(
 
 		synchronized (this.mergedBeanDefinitions) {
 			RootBeanDefinition mbd = null;
+			RootBeanDefinition previous = null;
 
 			// Check with full lock now in order to enforce the same merged instance.
 			if (containingBd == null) {
 				mbd = this.mergedBeanDefinitions.get(beanName);
 			}
 
-			if (mbd == null) {
+			if (mbd == null || mbd.stale) {
+				previous = mbd;
+				mbd = null;
 				if (bd.getParentName() == null) {
 					// Use copy of given root bean definition.
 					if (bd instanceof RootBeanDefinition) {
@@ -1315,11 +1319,26 @@ protected RootBeanDefinition getMergedBeanDefinition(
 					this.mergedBeanDefinitions.put(beanName, mbd);
 				}
 			}
-
+			if (previous != null) {
+				copyRelevantMergedBeanDefinitionCaches(previous, mbd);
+			}
 			return mbd;
 		}
 	}
 
+	private void copyRelevantMergedBeanDefinitionCaches(RootBeanDefinition previous,
+			RootBeanDefinition mbd) {
+		if (ObjectUtils.nullSafeEquals(mbd.getBeanClassName(), previous.getBeanClassName()) &&
+				ObjectUtils.nullSafeEquals(mbd.getFactoryBeanName(), previous.getFactoryBeanName()) &&
+				ObjectUtils.nullSafeEquals(mbd.getFactoryMethodName(), previous.getFactoryMethodName()) &&
+				(mbd.targetType == null || mbd.targetType.equals(previous.targetType))) {
+			mbd.targetType = previous.targetType;
+			mbd.resolvedTargetType = previous.resolvedTargetType;
+			mbd.factoryMethodReturnType = previous.factoryMethodReturnType;
+			mbd.factoryMethodToIntrospect = previous.factoryMethodToIntrospect;
+		}
+	}
+
 	/**
 	 * Check the given merged bean definition,
 	 * potentially throwing validation exceptions.
@@ -1342,7 +1361,10 @@ protected void checkMergedBeanDefinition(RootBeanDefinition mbd, String beanName
 	 * @param beanName the bean name to clear the merged definition for
 	 */
 	protected void clearMergedBeanDefinition(String beanName) {
-		this.mergedBeanDefinitions.remove(beanName);
+		RootBeanDefinition bd = this.mergedBeanDefinitions.get(beanName);
+		if (bd != null) {
+			bd.stale = true;
+		}
 	}
 
 	/**
@@ -1354,7 +1376,11 @@ protected void clearMergedBeanDefinition(String beanName) {
 	 * @since 4.2
 	 */
 	public void clearMetadataCache() {
-		this.mergedBeanDefinitions.keySet().removeIf(bean -> !isBeanEligibleForMetadataCaching(bean));
+		this.mergedBeanDefinitions.forEach((beanName, bd) -> {
+			if (!isBeanEligibleForMetadataCaching(beanName)) {
+				bd.stale = true;
+			}
+		});
 	}
 
 	/**

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

@@ -60,6 +60,9 @@ public class RootBeanDefinition extends AbstractBeanDefinition {
 	@Nullable
 	private AnnotatedElement qualifiedElement;
 
+	/** Determines if the definition needs to be re-merged. */
+	volatile boolean stale;
+
 	boolean allowCaching = true;
 
 	boolean isFactoryMethodUnique = false;