diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Bindable.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Bindable.java index 795179bf1a..67bff4f621 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Bindable.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Bindable.java @@ -90,18 +90,6 @@ public final class Bindable { 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 { Assert.isTrue( existingValue == null || this.type.isArray() || this.boxedType.resolve().isInstance(existingValue), () -> "ExistingValue must be an instance of " + this.type); - Supplier value = (existingValue != null) ? new ExistingValueSupplier(existingValue) : null; + Supplier 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 { } - private class ExistingValueSupplier implements Supplier { - - 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; - } - - } - } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/DefaultBindConstructorProvider.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/DefaultBindConstructorProvider.java index 8f2c287355..d5bf68a783 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/DefaultBindConstructorProvider.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/DefaultBindConstructorProvider.java @@ -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) { 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 dce89e318a..de44144c60 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 @@ -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; } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/DefaultBindConstructorProviderTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/DefaultBindConstructorProviderTests.java index dfe420aca4..6d09fe98a5 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/DefaultBindConstructorProviderTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/DefaultBindConstructorProviderTests.java @@ -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