root
/
spring-boot
1
0
Fork 0
Code Issues Pull Requests 1 Packages Projects Releases Wiki Activity

Refine constructor detection logic when binding to existing values

Browse Source 
Update `DefaultBindConstructorProvider` so that deduced constructors
are not used if there is an existing value.

Prior to this commit, constructor detection logic was not compatible
with earlier versions of Spring Boot. With Spring Boot 3.0.1, given
a class of the following form:

	@ConfigurationProperties(prefix = "example")
	public class ExampleProperties {

	    @NestedConfigurationProperty
	    private final NestedProperty nested = new NestedProperty(
	    		"Default", "default");

	    public NestedProperty getNested() {
	        return nested;
	    }

	}

If `NestedProperty` has a single constructor with arguments, constructor
binding would be used. In Spring Boot 2.x, setter injection would have
been used.

The updated code now only uses constructor injection if an explicit
`@ConstructorBinding` annotation is present, or if there is no existing
value.

Fixes gh-33409
See gh-33710
pull/33751/head
Phillip Webb 2 years ago
parent a2ac38e203
commit 84b13f0748
4 changed files with 83 additions and 53 deletions
Show Stats Download Patch File Download Diff File

30
spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Bindable.java
Unescape Escape View File

@ -90,18 +90,6 @@ public final class Bindable<T> {
return this.value;
}
/**
* Return if the bindable is for an existing non-null value. If this method return
* true then {@link #getValue()} will return a {@link Supplier} that never returns
* {@code null}.
* @return if the {@link Bindable} is for an existing value
* @since 3.0.2
* @see #withExistingValue(Object)
*/
public boolean hasExistingValue() {
return this.value instanceof ExistingValueSupplier;
}
/**
* Return any associated annotations that could affect binding.
* @return the associated annotations
@ -194,7 +182,7 @@ public final class Bindable<T> {
Assert.isTrue(
existingValue == null || this.type.isArray() || this.boxedType.resolve().isInstance(existingValue),
() -> "ExistingValue must be an instance of " + this.type);
Supplier<T> value = (existingValue != null) ? new ExistingValueSupplier(existingValue) : null;
Supplier<T> value = (existingValue != null) ? () -> existingValue : null;
return new Bindable<>(this.type, this.boxedType, value, this.annotations, this.bindRestrictions);
}
@ -319,20 +307,4 @@ public final class Bindable<T> {
}
private class ExistingValueSupplier implements Supplier<T> {
private final T value;
ExistingValueSupplier(T value) {
Assert.notNull(value, "Value must not be null");
this.value = value;
}
@Override
public T get() {
return this.value;
}
}
}

47
spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/DefaultBindConstructorProvider.java
Unescape Escape View File

@ -39,25 +39,19 @@ class DefaultBindConstructorProvider implements BindConstructorProvider {
@Override
public Constructor<?> getBindConstructor(Bindable<?> bindable, boolean isNestedConstructorBinding) {
return getBindConstructor(bindable.getType().resolve(), bindable.hasExistingValue(),
Constructors constructors = Constructors.getConstructors(bindable.getType().resolve(),
isNestedConstructorBinding);
if (constructors.getBind() != null && constructors.isDeducedBindConstructor()) {
if (bindable.getValue() != null && bindable.getValue().get() != null) {
return null;
}
}
return constructors.getBind();
}
@Override
public Constructor<?> getBindConstructor(Class<?> type, boolean isNestedConstructorBinding) {
return getBindConstructor(type, false, isNestedConstructorBinding);
}
private Constructor<?> getBindConstructor(Class<?> type, boolean hasExistingValue,
boolean isNestedConstructorBinding) {
if (type == null || hasExistingValue) {
return null;
}
Constructors constructors = Constructors.getConstructors(type);
if (constructors.getBind() != null || isNestedConstructorBinding) {
Assert.state(!constructors.hasAutowired(),
() -> type.getName() + " declares @ConstructorBinding and @Autowired constructor");
}
Constructors constructors = Constructors.getConstructors(type, isNestedConstructorBinding);
return constructors.getBind();
}
@ -66,13 +60,18 @@ class DefaultBindConstructorProvider implements BindConstructorProvider {
*/
static final class Constructors {
private static final Constructors NONE = new Constructors(false, null, false);
private final boolean hasAutowired;
private final Constructor<?> bind;
private Constructors(boolean hasAutowired, Constructor<?> bind) {
private final boolean deducedBindConstructor;
private Constructors(boolean hasAutowired, Constructor<?> bind, boolean deducedBindConstructor) {
this.hasAutowired = hasAutowired;
this.bind = bind;
this.deducedBindConstructor = deducedBindConstructor;
}
boolean hasAutowired() {
@ -83,18 +82,32 @@ class DefaultBindConstructorProvider implements BindConstructorProvider {
return this.bind;
}
static Constructors getConstructors(Class<?> type) {
boolean isDeducedBindConstructor() {
return this.deducedBindConstructor;
}
static Constructors getConstructors(Class<?> type, boolean isNestedConstructorBinding) {
if (type == null) {
return NONE;
}
boolean hasAutowiredConstructor = isAutowiredPresent(type);
Constructor<?>[] candidates = getCandidateConstructors(type);
MergedAnnotations[] candidateAnnotations = getAnnotations(candidates);
boolean deducedBindConstructor = false;
Constructor<?> bind = getConstructorBindingAnnotated(type, candidates, candidateAnnotations);
if (bind == null && !hasAutowiredConstructor) {
bind = deduceBindConstructor(type, candidates);
deducedBindConstructor = bind != null;
}
if (bind == null && !hasAutowiredConstructor && isKotlinType(type)) {
bind = deduceKotlinBindConstructor(type);
deducedBindConstructor = bind != null;
}
if (bind != null || isNestedConstructorBinding) {
Assert.state(!hasAutowiredConstructor,
() -> type.getName() + " declares @ConstructorBinding and @Autowired constructor");
}
return new Constructors(hasAutowiredConstructor, bind);
return new Constructors(hasAutowiredConstructor, bind, deducedBindConstructor);
}
private static boolean isAutowiredPresent(Class<?> type) {

40
spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesTests.java
Unescape Escape View File

@ -1116,11 +1116,19 @@ class ConfigurationPropertiesTests {
@Test // gh-33710
void loadWhenConstructorUsedInBeanMethodAndNotAsConstructorBinding() {
load(ConstructorUsedInBeanMethodConfiguration.class, "test.two=bound-2");
ConstructorUsedInBeanMethod bean = this.context.getBean(ConstructorUsedInBeanMethod.class);
ConstructorUsedDirectly bean = this.context.getBean(ConstructorUsedDirectly.class);
assertThat(bean.getOne()).isEqualTo("bean-method-1");
assertThat(bean.getTwo()).isEqualTo("bound-2");
}
@Test // gh-33409
void loadWhenConstructorUsedInNestedPropertyAndNotAsConstructorBinding() {
load(ConstructorUsedInNestedPropertyConfiguration.class, "test.nested.two=bound-2");
ConstructorUsedInNestedProperty bean = this.context.getBean(ConstructorUsedInNestedProperty.class);
assertThat(bean.getNested().getOne()).isEqualTo("nested-1");
assertThat(bean.getNested().getTwo()).isEqualTo("bound-2");
}
private AnnotationConfigApplicationContext load(Class<?> configuration, String... inlinedProperties) {
return load(new Class<?>[] { configuration }, inlinedProperties);
}
@ -2861,19 +2869,41 @@ class ConfigurationPropertiesTests {
@Bean
@ConfigurationProperties("test")
ConstructorUsedInBeanMethod constructorUsedInBeanMethod() {
return new ConstructorUsedInBeanMethod("bean-method-1", "bean-method-2");
ConstructorUsedDirectly constructorUsedInBeanMethod() {
return new ConstructorUsedDirectly("bean-method-1", "bean-method-2");
}
}
@Configuration(proxyBeanMethods = false)
@EnableConfigurationProperties(ConstructorUsedInNestedProperty.class)
static class ConstructorUsedInNestedPropertyConfiguration {
}
@ConfigurationProperties("test")
static class ConstructorUsedInNestedProperty {
@NestedConfigurationProperty
private ConstructorUsedDirectly nested = new ConstructorUsedDirectly("nested-1", "nested-2");
ConstructorUsedDirectly getNested() {
return this.nested;
}
void setNested(ConstructorUsedDirectly nested) {
this.nested = nested;
}
}
static class ConstructorUsedInBeanMethod {
static class ConstructorUsedDirectly {
private String one;
private String two;
ConstructorUsedInBeanMethod(String one, String two) {
ConstructorUsedDirectly(String one, String two) {
this.one = one;
this.two = two;
}

19
spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/DefaultBindConstructorProviderTests.java
Unescape Escape View File

@ -111,11 +111,19 @@ class DefaultBindConstructorProviderTests {
}
@Test
void getBindConstructorWhenHasExistingValueReturnsNull() {
void getBindConstructorWhenHasExistingValueAndOneConstructorWithoutAnnotationsReturnsNull() {
OneConstructorWithoutAnnotations existingValue = new OneConstructorWithoutAnnotations("name", 123);
Bindable<?> bindable = Bindable.of(OneConstructorWithoutAnnotations.class).withExistingValue(existingValue);
Constructor<?> bindConstructor = this.provider.getBindConstructor(bindable, false);
assertThat(bindConstructor).isNull();
}
@Test
void getBindConstructorWhenHasExistingValueAndOneConstructorWithConstructorBindingReturnsConstructor() {
OneConstructorWithConstructorBinding existingValue = new OneConstructorWithConstructorBinding("name", 123);
Bindable<?> bindable = Bindable.of(OneConstructorWithConstructorBinding.class).withExistingValue(existingValue);
Constructor<?> bindConstructor = this.provider.getBindConstructor(bindable, false);
assertThat(bindConstructor).isNull();
assertThat(bindConstructor).isNotNull();
}
static class OnlyDefaultConstructor {
@ -185,6 +193,13 @@ class DefaultBindConstructorProviderTests {
}
static class OneConstructorWithoutAnnotations {
OneConstructorWithoutAnnotations(String name, int age) {
}
}
static class TwoConstructorsWithBothConstructorBinding {
@ConstructorBinding

Write Preview
Loading…
Cancel
Save