Skip to content

Commit

Permalink
Use existing value rather than deducing bind method
Browse files Browse the repository at this point in the history
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
  • Loading branch information
wilkinsona committed Jul 12, 2023
1 parent 2a5a5d1 commit 090802b
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -216,9 +216,6 @@ public static ConfigurationPropertiesBean get(ApplicationContext applicationCont
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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
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;
Expand Down Expand Up @@ -1152,6 +1153,14 @@ void loadWhenNestedRecordWithExistingInstance() {
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);
}
Expand Down Expand Up @@ -3004,4 +3013,34 @@ void setNested(NestedRecord nestedRecord) {
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;
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {

Expand Down Expand Up @@ -106,4 +115,15 @@ class KotlinConfigurationPropertiesTests {

}

@EnableConfigurationProperties
@Configuration(proxyBeanMethods = false)
@Import(MutableDataClassProperties::class)
class MutableDataClassPropertiesImporter {
}

@ConfigurationProperties(prefix = "mutable")
data class MutableDataClassProperties(
var prop: String = ""
)

}

0 comments on commit 090802b

Please sign in to comment.