From af6d5387818aa6230d95775b5c71724e425eac2a Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Mon, 13 Apr 2020 15:45:21 -0700 Subject: [PATCH] Add support for initializing nested object when nothing bound When using constructor binding, if no properties are bound to a nested property, the top-level instance will be created with a null value for the nested property. This commit introduces support for an empty `@DefaultValue` which indicates that an instance of the nested property must be created even if nothing is bound to it. It honors any `@DefaultValue` annotations that the nested property might have in its constructor. Closes gh-18917 --- .../main/asciidoc/spring-boot-features.adoc | 31 +++ .../context/properties/bind/DefaultValue.java | 5 +- .../properties/bind/ValueObjectBinder.java | 61 +++++- .../bind/ValueObjectBinderTests.java | 203 ++++++++++++++++++ 4 files changed, 287 insertions(+), 13 deletions(-) diff --git a/spring-boot-project/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc b/spring-boot-project/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc index fa9c9a657464..1d1aa8aa1098 100644 --- a/spring-boot-project/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc +++ b/spring-boot-project/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc @@ -912,6 +912,37 @@ This means that the binder will expect to find a constructor with the parameters Nested members of a `@ConstructorBinding` class (such as `Security` in the example above) will also be bound via their constructor. Default values can be specified using `@DefaultValue` and the same conversion service will be applied to coerce the `String` value to the target type of a missing property. +By default, if no properties are bound to `Security`, the `AcmeProperties` instance will contain a `null` value for `security`. +If you wish you return a non-null instance of `Security` even when no properties are bound to it, you can use an empty `@DefaultValue` annotation to do so: + +[source,java,indent=0] +---- + package com.example; + import java.net.InetAddress; + import java.util.List; + + import org.springframework.boot.context.properties.ConfigurationProperties; + import org.springframework.boot.context.properties.ConstructorBinding; + import org.springframework.boot.context.properties.bind.DefaultValue; + + @ConstructorBinding + @ConfigurationProperties("acme") + public class AcmeProperties { + + private final boolean enabled; + + private final InetAddress remoteAddress; + + private final Security security; + + public AcmeProperties(boolean enabled, InetAddress remoteAddress, @DefaultValue Security security) { + this.enabled = enabled; + this.remoteAddress = remoteAddress; + this.security = security; + } + } +---- + NOTE: To use constructor binding the class must be enabled using `@EnableConfigurationProperties` or configuration property scanning. You cannot use constructor binding with beans that are created by the regular Spring mechanisms (e.g. `@Component` beans, beans created via `@Bean` methods or beans loaded using `@Import`) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/DefaultValue.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/DefaultValue.java index 28bfac847ff0..d58a977ffcaa 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/DefaultValue.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/DefaultValue.java @@ -23,7 +23,8 @@ /** * Annotation that can be used to specify the default value when binding to an immutable - * property. + * property. This annotation can also be used with nested properties to indicate that a + * value should always be bound (rather than binding {@code null}). * * @author Madhura Bhave * @since 2.2.0 @@ -38,6 +39,6 @@ * array-based properties. * @return the default value of the property. */ - String[] value(); + String[] value() default {}; } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/ValueObjectBinder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/ValueObjectBinder.java index e88473631218..d765d5fa501a 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/ValueObjectBinder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/ValueObjectBinder.java @@ -20,8 +20,10 @@ import java.lang.reflect.Modifier; import java.lang.reflect.Parameter; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Map; import kotlin.reflect.KFunction; import kotlin.reflect.KParameter; @@ -65,7 +67,7 @@ public T bind(ConfigurationPropertyName name, Bindable target, Binder.Con for (ConstructorParameter parameter : parameters) { Object arg = parameter.bind(propertyBinder); bound = bound || arg != null; - arg = (arg != null) ? arg : parameter.getDefaultValue(context.getConverter()); + arg = (arg != null) ? arg : getDefaultValue(context, parameter); args.add(arg); } context.clearConfigurationProperty(); @@ -82,11 +84,49 @@ public T create(Bindable target, Binder.Context context) { List parameters = valueObject.getConstructorParameters(); List args = new ArrayList<>(parameters.size()); for (ConstructorParameter parameter : parameters) { - args.add(parameter.getDefaultValue(context.getConverter())); + args.add(getDefaultValue(context, parameter)); } return valueObject.instantiate(args); } + private T getDefaultValue(Binder.Context context, ConstructorParameter parameter) { + ResolvableType type = parameter.getType(); + Annotation[] annotations = parameter.getAnnotations(); + for (Annotation annotation : annotations) { + if (annotation instanceof DefaultValue) { + DefaultValue defaultValue = (DefaultValue) annotation; + if (defaultValue.value().length == 0) { + return getNewInstanceIfPossible(context, type); + } + return context.getConverter().convert(defaultValue.value(), type, annotations); + } + } + return null; + } + + @SuppressWarnings("unchecked") + private T getNewInstanceIfPossible(Binder.Context context, ResolvableType type) { + Class resolved = (Class) type.resolve(); + Assert.state(resolved == null || isEmptyDefaultValueAllowed(resolved), + () -> "Parameter of type " + type + " must have a non-empty default value."); + T instance = create(Bindable.of(type), context); + if (instance != null) { + return instance; + } + return (resolved != null) ? BeanUtils.instantiateClass(resolved) : null; + } + + private boolean isEmptyDefaultValueAllowed(Class type) { + if (type.isPrimitive() || type.isEnum() || isAggregate(type) || type.getName().startsWith("java.lang")) { + return false; + } + return true; + } + + private boolean isAggregate(Class type) { + return type.isArray() || Map.class.isAssignableFrom(type) || Collection.class.isAssignableFrom(type); + } + /** * The value object being bound. * @@ -228,19 +268,18 @@ private static class ConstructorParameter { this.annotations = annotations; } - Object getDefaultValue(BindConverter converter) { - for (Annotation annotation : this.annotations) { - if (annotation instanceof DefaultValue) { - return converter.convert(((DefaultValue) annotation).value(), this.type, this.annotations); - } - } - return null; - } - Object bind(DataObjectPropertyBinder propertyBinder) { return propertyBinder.bindProperty(this.name, Bindable.of(this.type).withAnnotations(this.annotations)); } + Annotation[] getAnnotations() { + return this.annotations; + } + + ResolvableType getType() { + return this.type; + } + } } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/ValueObjectBinderTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/ValueObjectBinderTests.java index f75b089fcc24..e69d42fe7f94 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/ValueObjectBinderTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/ValueObjectBinderTests.java @@ -248,6 +248,65 @@ void bindToClassShouldBindWithGenerics() { assertThat(bean.getValue().get("bar")).isEqualTo("baz"); } + @Test + void bindWhenParametersWithDefaultValueShouldReturnNonNullValues() { + NestedConstructorBeanWithDefaultValue bound = this.binder.bindOrCreate("foo", + Bindable.of(NestedConstructorBeanWithDefaultValue.class)); + assertThat(bound.getNestedImmutable().getFoo()).isEqualTo("hello"); + assertThat(bound.getNestedJavaBean()).isNotNull(); + } + + @Test + void bindWhenJavaLangParameterWithEmptyDefaultValueShouldThrowException() { + assertThatExceptionOfType(BindException.class) + .isThrownBy(() -> this.binder.bindOrCreate("foo", + Bindable.of(NestedConstructorBeanWithEmptyDefaultValueForJavaLangTypes.class))) + .withStackTraceContaining("Parameter of type java.lang.String must have a non-empty default value."); + } + + @Test + void bindWhenCollectionParameterWithEmptyDefaultValueShouldThrowException() { + assertThatExceptionOfType(BindException.class) + .isThrownBy(() -> this.binder.bindOrCreate("foo", + Bindable.of(NestedConstructorBeanWithEmptyDefaultValueForCollectionTypes.class))) + .withStackTraceContaining( + "Parameter of type java.util.List must have a non-empty default value."); + } + + @Test + void bindWhenMapParametersWithEmptyDefaultValueShouldThrowException() { + assertThatExceptionOfType(BindException.class) + .isThrownBy(() -> this.binder.bindOrCreate("foo", + Bindable.of(NestedConstructorBeanWithEmptyDefaultValueForMapTypes.class))) + .withStackTraceContaining( + "Parameter of type java.util.Map must have a non-empty default value."); + } + + @Test + void bindWhenArrayParameterWithEmptyDefaultValueShouldThrowException() { + assertThatExceptionOfType(BindException.class) + .isThrownBy(() -> this.binder.bindOrCreate("foo", + Bindable.of(NestedConstructorBeanWithEmptyDefaultValueForArrayTypes.class))) + .withStackTraceContaining("Parameter of type java.lang.String[] must have a non-empty default value."); + } + + @Test + void bindWhenEnumParameterWithEmptyDefaultValueShouldThrowException() { + assertThatExceptionOfType(BindException.class) + .isThrownBy(() -> this.binder.bindOrCreate("foo", + Bindable.of(NestedConstructorBeanWithEmptyDefaultValueForEnumTypes.class))) + .withStackTraceContaining( + "Parameter of type org.springframework.boot.context.properties.bind.ValueObjectBinderTests$NestedConstructorBeanWithEmptyDefaultValueForEnumTypes$Foo must have a non-empty default value."); + } + + @Test + void bindWhenPrimitiveParameterWithEmptyDefaultValueShouldThrowException() { + assertThatExceptionOfType(BindException.class) + .isThrownBy(() -> this.binder.bindOrCreate("foo", + Bindable.of(NestedConstructorBeanWithEmptyDefaultValueForPrimitiveTypes.class))) + .withStackTraceContaining("Parameter of type int must have a non-empty default value."); + } + private void noConfigurationProperty(BindException ex) { assertThat(ex.getProperty()).isNull(); } @@ -481,4 +540,148 @@ T getValue() { } + static class NestedConstructorBeanWithDefaultValue { + + private final NestedImmutable nestedImmutable; + + private final NestedJavaBean nestedJavaBean; + + NestedConstructorBeanWithDefaultValue(@DefaultValue NestedImmutable nestedImmutable, + @DefaultValue NestedJavaBean nestedJavaBean) { + this.nestedImmutable = nestedImmutable; + this.nestedJavaBean = nestedJavaBean; + } + + NestedImmutable getNestedImmutable() { + return this.nestedImmutable; + } + + NestedJavaBean getNestedJavaBean() { + return this.nestedJavaBean; + } + + } + + static class NestedImmutable { + + private final String foo; + + private final String bar; + + NestedImmutable(@DefaultValue("hello") String foo, String bar) { + this.foo = foo; + this.bar = bar; + } + + String getFoo() { + return this.foo; + } + + String getBar() { + return this.bar; + } + + } + + static class NestedJavaBean { + + private String value; + + String getValue() { + return this.value; + } + + } + + static class NestedConstructorBeanWithEmptyDefaultValueForJavaLangTypes { + + private final String stringValue; + + NestedConstructorBeanWithEmptyDefaultValueForJavaLangTypes(@DefaultValue String stringValue) { + this.stringValue = stringValue; + } + + String getStringValue() { + return this.stringValue; + } + + } + + static class NestedConstructorBeanWithEmptyDefaultValueForCollectionTypes { + + private final List listValue; + + NestedConstructorBeanWithEmptyDefaultValueForCollectionTypes(@DefaultValue List listValue) { + this.listValue = listValue; + } + + List getListValue() { + return this.listValue; + } + + } + + static class NestedConstructorBeanWithEmptyDefaultValueForMapTypes { + + private final Map mapValue; + + NestedConstructorBeanWithEmptyDefaultValueForMapTypes(@DefaultValue Map mapValue) { + this.mapValue = mapValue; + } + + Map getMapValue() { + return this.mapValue; + } + + } + + static class NestedConstructorBeanWithEmptyDefaultValueForArrayTypes { + + private final String[] arrayValue; + + NestedConstructorBeanWithEmptyDefaultValueForArrayTypes(@DefaultValue String[] arrayValue, + @DefaultValue Integer intValue) { + this.arrayValue = arrayValue; + } + + String[] getArrayValue() { + return this.arrayValue; + } + + } + + static class NestedConstructorBeanWithEmptyDefaultValueForEnumTypes { + + private Foo foo; + + NestedConstructorBeanWithEmptyDefaultValueForEnumTypes(@DefaultValue Foo foo) { + this.foo = foo; + } + + Foo getFoo() { + return this.foo; + } + + enum Foo { + + BAR, BAZ + + } + + } + + static class NestedConstructorBeanWithEmptyDefaultValueForPrimitiveTypes { + + private int intValue; + + NestedConstructorBeanWithEmptyDefaultValueForPrimitiveTypes(@DefaultValue int intValue) { + this.intValue = intValue; + } + + int getIntValue() { + return this.intValue; + } + + } + }