From 090802b11b25a16694cc50f39431b16c1e54fc72 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Mon, 10 Jul 2023 18:25:44 +0100 Subject: [PATCH] Use existing value rather than deducing bind method When there is an existing value, deducing the bind method may incorrectly result in the use of constructor binding. This results in a failure in the configuration properties post-processor as it refuses to bind properties to a bean whose attributes indicate that constructor binding should have been used. This commit updates ConfigurationPropertiesBean to avoid tryin to deduce the bind method and instead use the presence or absence of an existing value to determine the type of binding that should be used. Only when there is no existing value is constructor binding appropriate. Fixes gh-36175 --- .../ConfigurationPropertiesBean.java | 3 -- .../ConfigurationPropertiesTests.java | 39 +++++++++++++++++++ .../KotlinConfigurationPropertiesTests.kt | 20 ++++++++++ 3 files changed, 59 insertions(+), 3 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBean.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBean.java index 3e42c133fb..186184b7c8 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBean.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBean.java @@ -216,9 +216,6 @@ public final class ConfigurationPropertiesBean { if (bindTarget.getBindMethod() == null && factoryMethod != null) { bindTarget = bindTarget.withBindMethod(JAVA_BEAN_BIND_METHOD); } - if (bindTarget.getBindMethod() == null) { - bindTarget = bindTarget.withBindMethod(deduceBindMethod(bindTarget)); - } if (bindTarget.getBindMethod() != VALUE_OBJECT_BIND_METHOD) { bindTarget = bindTarget.withExistingValue(bean); } 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 a0683316bb..98f78b06be 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 @@ -72,6 +72,7 @@ import org.springframework.boot.testsupport.system.OutputCaptureExtension; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; import org.springframework.context.annotation.ImportResource; import org.springframework.context.annotation.Scope; import org.springframework.context.support.PropertySourcesPlaceholderConfigurer; @@ -1152,6 +1153,14 @@ class ConfigurationPropertiesTests { assertThat(bean.getNested().name()).isEqualTo("spring"); } + @Test + void loadWhenPotentiallyConstructorBoundPropertiesAreImportedUsesJavaBeanBinding() { + load(PotentiallyConstructorBoundPropertiesImporter.class, "test.prop=alpha"); + PotentiallyConstructorBoundProperties properties = this.context + .getBean(PotentiallyConstructorBoundProperties.class); + assertThat(properties.getProp()).isEqualTo("alpha"); + } + private AnnotationConfigApplicationContext load(Class configuration, String... inlinedProperties) { return load(new Class[] { configuration }, inlinedProperties); } @@ -3004,4 +3013,34 @@ class ConfigurationPropertiesTests { static record NestedRecord(String name) { } + @EnableConfigurationProperties + @Import(PotentiallyConstructorBoundProperties.class) + static class PotentiallyConstructorBoundPropertiesImporter { + + @Bean + String notAProperty() { + return "notAProperty"; + } + + } + + @ConfigurationProperties("test") + static class PotentiallyConstructorBoundProperties { + + private String prop; + + PotentiallyConstructorBoundProperties(String notAProperty) { + + } + + String getProp() { + return this.prop; + } + + void setProp(String prop) { + this.prop = prop; + } + + } + } 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 index 6420fc8dbb..2aa81e9c2f 100644 --- 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 @@ -22,6 +22,7 @@ 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 +import org.springframework.context.annotation.Import import org.springframework.test.context.support.TestPropertySourceUtils import org.assertj.core.api.Assertions.assertThat @@ -68,6 +69,14 @@ class KotlinConfigurationPropertiesTests { assertThat(properties.inner.bravo).isEqualTo("two") } + @Test + fun `mutable data class properties can be imported`() { + this.context.register(MutableDataClassPropertiesImporter::class.java) + TestPropertySourceUtils.addInlinedPropertiesToEnvironment(this.context, "mutable.prop=alpha"); + this.context.refresh(); + assertThat(this.context.getBean(MutableDataClassProperties::class.java).prop).isEqualTo("alpha") + } + @ConfigurationProperties(prefix = "foo") class BingProperties(@Suppress("UNUSED_PARAMETER") bar: String) { @@ -106,4 +115,15 @@ class KotlinConfigurationPropertiesTests { } + @EnableConfigurationProperties + @Configuration(proxyBeanMethods = false) + @Import(MutableDataClassProperties::class) + class MutableDataClassPropertiesImporter { + } + + @ConfigurationProperties(prefix = "mutable") + data class MutableDataClassProperties( + var prop: String = "" + ) + } \ No newline at end of file