From 471ca01ccf9038a5ab430c04d09358ceea6fe643 Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Tue, 5 Nov 2019 21:54:16 -0800 Subject: [PATCH] Do not validate value object bean definion when singleton present Prior to this commit constructor bound configuration properties could not be mocked because it would fail validation from ConfigurationPropertiesBeanDefinitionValidator. The MockitoPostProcessor registers the mocked bean as a singleton and validation can be skipped if a singleton for the type is found in the bean factory. Fixes gh-18652 --- ...tionPropertiesBeanDefinitionValidator.java | 8 +++- .../ConfigurationPropertiesTests.java | 18 +++++++++ .../KotlinConfigurationPropertiesTests.kt | 40 +++++++++++++++++++ 3 files changed, 64 insertions(+), 2 deletions(-) create mode 100644 spring-boot-project/spring-boot/src/test/kotlin/org/springframework/boot/context/properties/KotlinConfigurationPropertiesTests.kt diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanDefinitionValidator.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanDefinitionValidator.java index c5c4653c2c3..413fedb99a0 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanDefinitionValidator.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanDefinitionValidator.java @@ -40,13 +40,17 @@ class ConfigurationPropertiesBeanDefinitionValidator implements BeanFactoryPostP @Override public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) throws BeansException { for (String beanName : beanFactory.getBeanDefinitionNames()) { - BeanDefinition definition = beanFactory.getBeanDefinition(beanName); - if (!(definition instanceof ConfigurationPropertiesValueObjectBeanDefinition)) { + if (!(beanFactory.containsSingleton(beanName) || isValueObjectBeanDefinition(beanFactory, beanName))) { validate(beanFactory, beanName); } } } + private boolean isValueObjectBeanDefinition(ConfigurableListableBeanFactory beanFactory, String beanName) { + BeanDefinition definition = beanFactory.getBeanDefinition(beanName); + return (definition instanceof ConfigurationPropertiesValueObjectBeanDefinition); + } + @Override public int getOrder() { return Ordered.LOWEST_PRECEDENCE; diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesTests.java index 9c3ee77a5a3..729b70b2163 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesTests.java @@ -44,8 +44,11 @@ import org.springframework.beans.factory.InitializingBean; import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; +import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.beans.factory.support.AbstractBeanDefinition; +import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.beans.factory.support.GenericBeanDefinition; +import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.boot.context.properties.bind.BindException; import org.springframework.boot.context.properties.bind.DefaultValue; import org.springframework.boot.context.properties.bind.validation.BindValidationException; @@ -895,6 +898,15 @@ class ConfigurationPropertiesTests { assertThat(bean.getNested().getAge()).isEqualTo(10); } + @Test // gh-18652 + void loadWhenBeanFactoryContainsSingletonForConstructorBindingTypeShouldNotFail() { + ConfigurableListableBeanFactory beanFactory = this.context.getBeanFactory(); + ((BeanDefinitionRegistry) beanFactory).registerBeanDefinition("test", + new RootBeanDefinition(ConstructorParameterProperties.class)); + beanFactory.registerSingleton("test", new ConstructorParameterProperties("bar", 5)); + load(TestConfiguration.class); + } + private AnnotationConfigApplicationContext load(Class configuration, String... inlinedProperties) { return load(new Class[] { configuration }, inlinedProperties); } @@ -921,6 +933,12 @@ class ConfigurationPropertiesTests { this.context = new AnnotationConfigApplicationContext(); } + @Configuration(proxyBeanMethods = false) + @EnableConfigurationProperties + static class TestConfiguration { + + } + @Configuration(proxyBeanMethods = false) @EnableConfigurationProperties(BasicProperties.class) static class BasicConfiguration { diff --git a/spring-boot-project/spring-boot/src/test/kotlin/org/springframework/boot/context/properties/KotlinConfigurationPropertiesTests.kt b/spring-boot-project/spring-boot/src/test/kotlin/org/springframework/boot/context/properties/KotlinConfigurationPropertiesTests.kt new file mode 100644 index 00000000000..db95e2346ca --- /dev/null +++ b/spring-boot-project/spring-boot/src/test/kotlin/org/springframework/boot/context/properties/KotlinConfigurationPropertiesTests.kt @@ -0,0 +1,40 @@ +package org.springframework.boot.context.properties + +import org.junit.jupiter.api.Test +import org.springframework.beans.factory.support.BeanDefinitionRegistry +import org.springframework.beans.factory.support.RootBeanDefinition +import org.springframework.context.annotation.AnnotationConfigApplicationContext +import org.springframework.context.annotation.Configuration + +/** + * Tests for {@link ConfigurationProperties @ConfigurationProperties}-annotated beans. + * + * @author Madhura Bhave + */ +class KotlinConfigurationPropertiesTests { + + private var context = AnnotationConfigApplicationContext() + + @Test //gh-18652 + fun `type with constructor binding and existing singleton should not fail`() { + val beanFactory = this.context.beanFactory + (beanFactory as BeanDefinitionRegistry).registerBeanDefinition("foo", + RootBeanDefinition(BingProperties::class.java)) + beanFactory.registerSingleton("foo", BingProperties("")) + this.context.register(TestConfig::class.java) + this.context.refresh(); + } + + @ConfigurationProperties(prefix = "foo") + @ConstructorBinding + class BingProperties(@Suppress("UNUSED_PARAMETER") bar: String) { + + } + + @Configuration(proxyBeanMethods = false) + @EnableConfigurationProperties + internal open class TestConfig { + + } + +} \ No newline at end of file