Refine constructor detection logic when binding to existing values
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
This commit is contained in:
parent
a2ac38e203
commit
84b13f0748
|
@ -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;
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -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;
|
||||
}
|
||||
return new Constructors(hasAutowiredConstructor, bind);
|
||||
if (bind != null || isNestedConstructorBinding) {
|
||||
Assert.state(!hasAutowiredConstructor,
|
||||
() -> type.getName() + " declares @ConstructorBinding and @Autowired constructor");
|
||||
}
|
||||
return new Constructors(hasAutowiredConstructor, bind, deducedBindConstructor);
|
||||
}
|
||||
|
||||
private static boolean isAutowiredPresent(Class<?> type) {
|
||||
|
|
|
@ -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");
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
static class ConstructorUsedInBeanMethod {
|
||||
@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 ConstructorUsedDirectly {
|
||||
|
||||
private String one;
|
||||
|
||||
private String two;
|
||||
|
||||
ConstructorUsedInBeanMethod(String one, String two) {
|
||||
ConstructorUsedDirectly(String one, String two) {
|
||||
this.one = one;
|
||||
this.two = two;
|
||||
}
|
||||
|
|
|
@ -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
|
||||
|
|
Loading…
Reference in New Issue