From efc431bdc4c1f91ba3802a25454229a7f4f68f7b Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Fri, 30 Sep 2022 17:59:28 -0700 Subject: [PATCH] Support empty @DefaultValue annotations on aggregates and optional Update `ValueObjectBinder` to allow an empty `@DefaultValue` to be used on map, collection, arrays and optional types. Closes gh-32559 --- .../properties/bind/ValueObjectBinder.java | 32 ++++++++--- .../ConfigurationPropertiesTests.java | 56 +++++++++++++++++++ .../bind/ValueObjectBinderTests.java | 54 +++++++++++------- 3 files changed, 115 insertions(+), 27 deletions(-) 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 ab446865b8a6..e6736155bc2f 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 @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,6 +17,7 @@ package org.springframework.boot.context.properties.bind; import java.lang.annotation.Annotation; +import java.lang.reflect.Array; import java.lang.reflect.Constructor; import java.lang.reflect.Modifier; import java.lang.reflect.Parameter; @@ -25,6 +26,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Optional; import kotlin.reflect.KFunction; import kotlin.reflect.KParameter; @@ -32,6 +34,7 @@ import org.springframework.beans.BeanUtils; import org.springframework.boot.context.properties.source.ConfigurationPropertyName; +import org.springframework.core.CollectionFactory; import org.springframework.core.DefaultParameterNameDiscoverer; import org.springframework.core.KotlinDetector; import org.springframework.core.MethodParameter; @@ -101,7 +104,7 @@ private T getDefaultValue(Binder.Context context, ConstructorParameter param if (annotation instanceof DefaultValue) { String[] defaultValue = ((DefaultValue) annotation).value(); if (defaultValue.length == 0) { - return getNewInstanceIfPossible(context, type); + return getNewDefaultValueInstanceIfPossible(context, type); } return convertDefaultValue(context.getConverter(), defaultValue, type, annotations); } @@ -124,7 +127,7 @@ private T convertDefaultValue(BindConverter converter, String[] defaultValue } @SuppressWarnings("unchecked") - private T getNewInstanceIfPossible(Binder.Context context, ResolvableType type) { + private T getNewDefaultValueInstanceIfPossible(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."); @@ -132,14 +135,27 @@ private T getNewInstanceIfPossible(Binder.Context context, ResolvableType ty if (instance != null) { return instance; } - return (resolved != null) ? BeanUtils.instantiateClass(resolved) : null; + if (resolved != null) { + if (Optional.class == resolved) { + return (T) Optional.empty(); + } + if (Collection.class.isAssignableFrom(resolved)) { + return (T) CollectionFactory.createCollection(resolved, 0); + } + if (Map.class.isAssignableFrom(resolved)) { + return (T) CollectionFactory.createMap(resolved, 0); + } + if (resolved.isArray()) { + return (T) Array.newInstance(resolved.getComponentType(), 0); + } + return BeanUtils.instantiateClass(resolved); + } + return null; } private boolean isEmptyDefaultValueAllowed(Class type) { - if (type.isPrimitive() || type.isEnum() || isAggregate(type) || type.getName().startsWith("java.lang")) { - return false; - } - return true; + return (Optional.class == type || isAggregate(type)) + || !(type.isPrimitive() || type.isEnum() || type.getName().startsWith("java.lang")); } private boolean isAggregate(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 eda312947acf..b23119dc0d5c 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 @@ -29,6 +29,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Optional; import java.util.Properties; import java.util.Set; @@ -853,6 +854,17 @@ void loadWhenBindingToConstructorParametersWithDefaultValuesShouldBind() { assertThat(bean.getBar()).isEqualTo(0); } + @Test + void loadWhenBindingToConstructorParametersWithEmptyDefaultValueShouldBind() { + load(ConstructorParameterEmptyDefaultValueConfiguration.class); + ConstructorParameterEmptyDefaultValueProperties bean = this.context + .getBean(ConstructorParameterEmptyDefaultValueProperties.class); + assertThat(bean.getSet()).isEmpty(); + assertThat(bean.getMap()).isEmpty(); + assertThat(bean.getArray()).isEmpty(); + assertThat(bean.getOptional()).isEmpty(); + } + @Test void loadWhenBindingToConstructorParametersWithDefaultDataUnitShouldBind() { load(ConstructorParameterWithUnitConfiguration.class); @@ -2145,6 +2157,45 @@ int getBar() { } + @ConstructorBinding + @ConfigurationProperties(prefix = "test") + static class ConstructorParameterEmptyDefaultValueProperties { + + private final Set set; + + private final Map map; + + private final int[] array; + + private final Optional optional; + + ConstructorParameterEmptyDefaultValueProperties(@DefaultValue Set set, + @DefaultValue Map map, @DefaultValue int[] array, + @DefaultValue Optional optional) { + this.set = set; + this.map = map; + this.array = array; + this.optional = optional; + } + + Set getSet() { + return this.set; + } + + Map getMap() { + return this.map; + } + + int[] getArray() { + return this.array; + } + + Optional getOptional() { + return this.optional; + } + + } + @ConstructorBinding @ConfigurationProperties(prefix = "test") static class ConstructorParameterWithUnitProperties { @@ -2225,6 +2276,11 @@ static class ConstructorParameterConfiguration { } + @EnableConfigurationProperties(ConstructorParameterEmptyDefaultValueProperties.class) + static class ConstructorParameterEmptyDefaultValueConfiguration { + + } + @EnableConfigurationProperties(ConstructorParameterWithUnitProperties.class) static class ConstructorParameterWithUnitConfiguration { 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 73711f854634..529e2aeeb7d7 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 @@ -31,6 +31,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.EnabledForJreRange; @@ -293,29 +294,31 @@ void bindWhenJavaLangParameterWithEmptyDefaultValueShouldThrowException() { } @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."); + void bindWhenCollectionParameterWithEmptyDefaultValueShouldReturnEmptyInstance() { + NestedConstructorBeanWithEmptyDefaultValueForCollectionTypes bound = this.binder.bindOrCreate("foo", + Bindable.of(NestedConstructorBeanWithEmptyDefaultValueForCollectionTypes.class)); + assertThat(bound.getListValue()).isEmpty(); } @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."); + void bindWhenMapParametersWithEmptyDefaultValueShouldReturnEmptyInstance() { + NestedConstructorBeanWithEmptyDefaultValueForMapTypes bound = this.binder.bindOrCreate("foo", + Bindable.of(NestedConstructorBeanWithEmptyDefaultValueForMapTypes.class)); + assertThat(bound.getMapValue()).isEmpty(); } @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."); + void bindWhenArrayParameterWithEmptyDefaultValueShouldReturnEmptyInstance() { + NestedConstructorBeanWithEmptyDefaultValueForArrayTypes bound = this.binder.bindOrCreate("foo", + Bindable.of(NestedConstructorBeanWithEmptyDefaultValueForArrayTypes.class)); + assertThat(bound.getArrayValue()).isEmpty(); + } + + @Test + void bindWhenOptionalParameterWithEmptyDefaultValueShouldReturnEmptyInstance() { + NestedConstructorBeanWithEmptyDefaultValueForOptionalTypes bound = this.binder.bindOrCreate("foo", + Bindable.of(NestedConstructorBeanWithEmptyDefaultValueForOptionalTypes.class)); + assertThat(bound.getOptionalValue()).isEmpty(); } @Test @@ -753,8 +756,7 @@ static class NestedConstructorBeanWithEmptyDefaultValueForArrayTypes { private final String[] arrayValue; - NestedConstructorBeanWithEmptyDefaultValueForArrayTypes(@DefaultValue String[] arrayValue, - @DefaultValue Integer intValue) { + NestedConstructorBeanWithEmptyDefaultValueForArrayTypes(@DefaultValue String[] arrayValue) { this.arrayValue = arrayValue; } @@ -764,6 +766,20 @@ String[] getArrayValue() { } + static class NestedConstructorBeanWithEmptyDefaultValueForOptionalTypes { + + private final Optional optionalValue; + + NestedConstructorBeanWithEmptyDefaultValueForOptionalTypes(@DefaultValue Optional optionalValue) { + this.optionalValue = optionalValue; + } + + Optional getOptionalValue() { + return this.optionalValue; + } + + } + static class NestedConstructorBeanWithEmptyDefaultValueForEnumTypes { private Foo foo;