From 6dc14af92dbfa7b5281072d9fdde241ce0da3679 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson <awilkinson@pivotal.io> 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 957eb948c34f..799b039b0cf1 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<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(); } } 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 336ff31d736b..e54f3ef2ce2d 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 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) { + + } + + } + }