From 6dc14af92dbfa7b5281072d9fdde241ce0da3679 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Mon, 9 Jul 2018 16:22:06 +0100 Subject: [PATCH] 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 --- .../condition/BeanTypeRegistry.java | 49 +++++++++++++------ .../condition/ConditionalOnBeanTests.java | 43 +++++++++++++++- 2 files changed, 76 insertions(+), 16 deletions(-) diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/BeanTypeRegistry.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/BeanTypeRegistry.java index 957eb948c3..799b039b0c 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/BeanTypeRegistry.java +++ b/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> beanTypes = new HashMap>(); - private int lastBeanDefinitionCount = 0; + private final Map beanDefinitions = new HashMap(); private BeanTypeRegistry(DefaultListableBeanFactory beanFactory) { this.beanFactory = beanFactory; @@ -146,7 +146,7 @@ final class BeanTypeRegistry implements SmartInitializingSingleton { 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 @@ final class BeanTypeRegistry implements SmartInitializingSingleton { } 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 @@ final class BeanTypeRegistry implements SmartInitializingSingleton { 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 @@ final class BeanTypeRegistry implements SmartInitializingSingleton { } private void updateTypesIfNecessary() { - if (this.lastBeanDefinitionCount != this.beanFactory.getBeanDefinitionCount()) { - Iterator names = this.beanFactory.getBeanNamesIterator(); - while (names.hasNext()) { - String name = names.next(); - if (!this.beanTypes.containsKey(name)) { - addBeanType(name); + Iterator 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(); } } diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBeanTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBeanTests.java index 336ff31d73..e54f3ef2ce 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBeanTests.java +++ b/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 class ConditionalOnBeanTests { 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 class ConditionalOnBeanTests { } + @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) { + + } + + } + }