Update view of bean types when an override is detected

Previously, when a bean was overridden and its type changes,
BeanTypeRegistry could be left with a stale view of the bean's type.
This would lead to incorrect bean condition evaluation as conditions
would match or not match based on the bean's old type.

This commit updates the type registry to refresh its view of a bean's
type when its definition changes.

Closes gh-13588

Author: wilkinsona

spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/BeanTypeRegistry.java

@@ -76,7 +76,7 @@ final class BeanTypeRegistry implements SmartInitializingSingleton {
 
 	private final Map<String, Class<?>> beanTypes = new HashMap<String, Class<?>>();
 
-	private int lastBeanDefinitionCount = 0;
+	private final Map<String, RootBeanDefinition> beanDefinitions = new HashMap<String, RootBeanDefinition>();
 
 	private BeanTypeRegistry(DefaultListableBeanFactory beanFactory) {
 		this.beanFactory = beanFactory;
@@ -146,7 +146,7 @@ Set<String> getNamesForAnnotation(Class<? extends Annotation> annotation) {
 	public void afterSingletonsInstantiated() {
 		// We're done at this point, free up some memory
 		this.beanTypes.clear();
-		this.lastBeanDefinitionCount = 0;
+		this.beanDefinitions.clear();
 	}
 
 	private void addBeanType(String name) {
@@ -159,10 +159,23 @@ else if (!this.beanFactory.isAlias(name)) {
 	}
 
 	private void addBeanTypeForNonAliasDefinition(String name) {
+		addBeanTypeForNonAliasDefinition(name, getBeanDefinition(name));
+	}
+
+	private RootBeanDefinition getBeanDefinition(String name) {
+		try {
+			return (RootBeanDefinition) this.beanFactory.getMergedBeanDefinition(name);
+		}
+		catch (BeanDefinitionStoreException ex) {
+			logIgnoredError("unresolvable metadata in bean definition", name, ex);
+			return null;
+		}
+	}
+
+	private void addBeanTypeForNonAliasDefinition(String name,
+			RootBeanDefinition beanDefinition) {
 		try {
 			String factoryName = BeanFactory.FACTORY_BEAN_PREFIX + name;
-			RootBeanDefinition beanDefinition = (RootBeanDefinition) this.beanFactory
-					.getMergedBeanDefinition(name);
 			if (!beanDefinition.isAbstract()
 					&& !requiresEagerInit(beanDefinition.getFactoryBeanName())) {
 				if (this.beanFactory.isFactoryBean(factoryName)) {
@@ -176,15 +189,12 @@ private void addBeanTypeForNonAliasDefinition(String name) {
 					this.beanTypes.put(name, this.beanFactory.getType(name));
 				}
 			}
+			this.beanDefinitions.put(name, beanDefinition);
 		}
 		catch (CannotLoadBeanClassException ex) {
 			// Probably contains a placeholder
 			logIgnoredError("bean class loading failure for bean", name, ex);
 		}
-		catch (BeanDefinitionStoreException ex) {
-			// Probably contains a placeholder
-			logIgnoredError("unresolvable metadata in bean definition", name, ex);
-		}
 	}
 
 	private void logIgnoredError(String message, String name, Exception ex) {
@@ -199,15 +209,24 @@ private boolean requiresEagerInit(String factoryBeanName) {
 	}
 
 	private void updateTypesIfNecessary() {
-		if (this.lastBeanDefinitionCount != this.beanFactory.getBeanDefinitionCount()) {
-			Iterator<String> names = this.beanFactory.getBeanNamesIterator();
-			while (names.hasNext()) {
-				String name = names.next();
-				if (!this.beanTypes.containsKey(name)) {
-					addBeanType(name);
+		Iterator<String> names = this.beanFactory.getBeanNamesIterator();
+		while (names.hasNext()) {
+			String name = names.next();
+			if (!this.beanTypes.containsKey(name)) {
+				addBeanType(name);
+			}
+			else {
+				if (!this.beanFactory.isAlias(name)
+						&& !this.beanFactory.containsSingleton(name)) {
+					RootBeanDefinition beanDefinition = getBeanDefinition(name);
+					RootBeanDefinition existingDefinition = this.beanDefinitions.put(name,
+							beanDefinition);
+					if (existingDefinition != null
+							&& !beanDefinition.equals(existingDefinition)) {
+						addBeanTypeForNonAliasDefinition(name, beanDefinition);
+					}
 				}
 			}
-			this.lastBeanDefinitionCount = this.beanFactory.getBeanDefinitionCount();
 		}
 	}
 

spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBeanTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2017 the original author or authors.
+ * Copyright 2012-2018 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.
@@ -139,6 +139,16 @@ public void beanProducedByFactoryBeanIsConsideredWhenMatchingOnAnnotation() {
 		assertThat(this.context.getBeansOfType(ExampleBean.class)).hasSize(1);
 	}
 
+	@Test
+	public void conditionEvaluationConsidersChangeInTypeWhenBeanIsOverridden() {
+		this.context.register(OriginalDefinition.class, OverridingDefinition.class,
+				ConsumingConfiguration.class);
+		this.context.refresh();
+		assertThat(this.context.containsBean("testBean")).isTrue();
+		assertThat(this.context.getBean(Integer.class)).isEqualTo(1);
+		assertThat(this.context.getBeansOfType(ConsumingConfiguration.class)).isEmpty();
+	}
+
 	@Configuration
 	@ConditionalOnBean(name = "foo")
 	protected static class OnBeanNameConfiguration {
@@ -311,4 +321,35 @@ public String toString() {
 
 	}
 
+	@Configuration
+	public static class OriginalDefinition {
+
+		@Bean
+		public String testBean() {
+			return "test";
+		}
+
+	}
+
+	@Configuration
+	@ConditionalOnBean(String.class)
+	public static class OverridingDefinition {
+
+		@Bean
+		public Integer testBean() {
+			return 1;
+		}
+
+	}
+
+	@Configuration
+	@ConditionalOnBean(String.class)
+	public static class ConsumingConfiguration {
+
+		ConsumingConfiguration(String testBean) {
+
+		}
+
+	}
+
 }