From b6ff0b7c5fcef28f9ba8fd715509b9a631f0f918 Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Wed, 13 Nov 2019 12:25:07 -0800 Subject: [PATCH] Support constructor binding on 3rd party classes Closes gh-18935 --- .../ConfigurationPropertiesBean.java | 2 +- ...tionPropertiesBindConstructorProvider.java | 8 +-- .../bind/BindConstructorProvider.java | 4 +- .../boot/context/properties/bind/Binder.java | 14 +++++ .../bind/DefaultBindConstructorProvider.java | 2 +- .../properties/bind/ValueObjectBinder.java | 12 ++-- .../ConfigurationPropertiesBeanTests.java | 2 +- .../ConfigurationPropertiesTests.java | 61 +++++++++++++++++++ .../properties/bind/JavaBeanBinderTests.java | 3 +- .../bind/ValueObjectBinderTests.java | 3 +- 10 files changed, 97 insertions(+), 14 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBean.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBean.java index a1a93cf7fe..48b5061212 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBean.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBean.java @@ -308,7 +308,7 @@ public final class ConfigurationPropertiesBean { VALUE_OBJECT; static BindMethod forType(Class type) { - return (ConfigurationPropertiesBindConstructorProvider.INSTANCE.getBindConstructor(type) != null) + return (ConfigurationPropertiesBindConstructorProvider.INSTANCE.getBindConstructor(type, false) != null) ? VALUE_OBJECT : JAVA_BEAN; } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindConstructorProvider.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindConstructorProvider.java index d506881a7d..28c3b0412b 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindConstructorProvider.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindConstructorProvider.java @@ -37,16 +37,16 @@ class ConfigurationPropertiesBindConstructorProvider implements BindConstructorP static final ConfigurationPropertiesBindConstructorProvider INSTANCE = new ConfigurationPropertiesBindConstructorProvider(); @Override - public Constructor getBindConstructor(Bindable bindable) { - return getBindConstructor(bindable.getType().resolve()); + public Constructor getBindConstructor(Bindable bindable, boolean isNestedConstructorBinding) { + return getBindConstructor(bindable.getType().resolve(), isNestedConstructorBinding); } - Constructor getBindConstructor(Class type) { + Constructor getBindConstructor(Class type, boolean isNestedConstructorBinding) { if (type == null) { return null; } Constructor constructor = findConstructorBindingAnnotatedConstructor(type); - if (constructor == null && isConstructorBindingAnnotatedType(type)) { + if (constructor == null && (isConstructorBindingAnnotatedType(type) || isNestedConstructorBinding)) { constructor = deduceBindConstructor(type); } return constructor; diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/BindConstructorProvider.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/BindConstructorProvider.java index 1d26ed3614..8446323150 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/BindConstructorProvider.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/BindConstructorProvider.java @@ -37,8 +37,10 @@ public interface BindConstructorProvider { * Return the bind constructor to use for the given bindable, or {@code null} if * constructor binding is not supported. * @param bindable the bindable to check + * @param isNestedConstructorBinding if this binding is nested within a constructor + * binding * @return the bind constructor or {@code null} */ - Constructor getBindConstructor(Bindable bindable); + Constructor getBindConstructor(Bindable bindable, boolean isNestedConstructorBinding); } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Binder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Binder.java index bb042298ae..78f7b74bca 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Binder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Binder.java @@ -522,6 +522,8 @@ public class Binder { private final Deque> dataObjectBindings = new ArrayDeque<>(); + private final Deque> constructorBindings = new ArrayDeque<>(); + private ConfigurationProperty configurationProperty; Context() { @@ -582,6 +584,18 @@ public class Binder { this.configurationProperty = null; } + void pushConstructorBoundTypes(Class value) { + this.constructorBindings.push(value); + } + + boolean isNestedConstructorBinding() { + return !this.constructorBindings.isEmpty(); + } + + void popConstructorBoundTypes() { + this.constructorBindings.pop(); + } + PlaceholdersResolver getPlaceholdersResolver() { return Binder.this.placeholdersResolver; } 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 c9e00110f1..b36b12b2cc 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 @@ -30,7 +30,7 @@ import org.springframework.core.KotlinDetector; class DefaultBindConstructorProvider implements BindConstructorProvider { @Override - public Constructor getBindConstructor(Bindable bindable) { + public Constructor getBindConstructor(Bindable bindable, boolean isNestedConstructorBinding) { Class type = bindable.getType().resolve(); if (bindable.getValue() != null || type == null) { return null; 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 c8efd8ac75..73ac82f1e6 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 @@ -53,10 +53,11 @@ class ValueObjectBinder implements DataObjectBinder { @Override public T bind(ConfigurationPropertyName name, Bindable target, Binder.Context context, DataObjectPropertyBinder propertyBinder) { - ValueObject valueObject = ValueObject.get(target, this.constructorProvider); + ValueObject valueObject = ValueObject.get(target, this.constructorProvider, context); if (valueObject == null) { return null; } + context.pushConstructorBoundTypes(target.getType().resolve()); List parameters = valueObject.getConstructorParameters(); List args = new ArrayList<>(parameters.size()); boolean bound = false; @@ -67,12 +68,13 @@ class ValueObjectBinder implements DataObjectBinder { args.add(arg); } context.clearConfigurationProperty(); + context.popConstructorBoundTypes(); return bound ? valueObject.instantiate(args) : null; } @Override public T create(Bindable target, Binder.Context context) { - ValueObject valueObject = ValueObject.get(target, this.constructorProvider); + ValueObject valueObject = ValueObject.get(target, this.constructorProvider, context); if (valueObject == null) { return null; } @@ -104,12 +106,14 @@ class ValueObjectBinder implements DataObjectBinder { abstract List getConstructorParameters(); @SuppressWarnings("unchecked") - static ValueObject get(Bindable bindable, BindConstructorProvider constructorProvider) { + static ValueObject get(Bindable bindable, BindConstructorProvider constructorProvider, + Binder.Context context) { Class type = (Class) bindable.getType().resolve(); if (type == null || type.isEnum() || Modifier.isAbstract(type.getModifiers())) { return null; } - Constructor bindConstructor = constructorProvider.getBindConstructor(bindable); + Constructor bindConstructor = constructorProvider.getBindConstructor(bindable, + context.isNestedConstructorBinding()); if (bindConstructor == null) { return null; } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanTests.java index 3df78ee425..867b68150f 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanTests.java @@ -213,7 +213,7 @@ class ConfigurationPropertiesBeanTests { assertThat(target.getType()).isEqualTo(ResolvableType.forClass(ConstructorBindingOnConstructor.class)); assertThat(target.getValue()).isNull(); assertThat(ConfigurationPropertiesBindConstructorProvider.INSTANCE - .getBindConstructor(ConstructorBindingOnConstructor.class)).isNotNull(); + .getBindConstructor(ConstructorBindingOnConstructor.class, false)).isNotNull(); } @Test 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 729b70b216..fdb675d57c 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 @@ -907,6 +907,18 @@ class ConfigurationPropertiesTests { load(TestConfiguration.class); } + @Test + void loadWhenConstructorBindingWithOuterClassDeducedConstructorBound() { + MutablePropertySources sources = this.context.getEnvironment().getPropertySources(); + Map source = new HashMap<>(); + source.put("test.nested.outer.age", "5"); + sources.addLast(new MapPropertySource("test", source)); + load(ConstructorBindingWithOuterClassConstructorBoundConfiguration.class); + ConstructorBindingWithOuterClassConstructorBoundProperties bean = this.context + .getBean(ConstructorBindingWithOuterClassConstructorBoundProperties.class); + assertThat(bean.getNested().getOuter().getAge()).isEqualTo(5); + } + private AnnotationConfigApplicationContext load(Class configuration, String... inlinedProperties) { return load(new Class[] { configuration }, inlinedProperties); } @@ -2126,6 +2138,55 @@ class ConfigurationPropertiesTests { } + @ConfigurationProperties("test") + @ConstructorBinding + static class ConstructorBindingWithOuterClassConstructorBoundProperties { + + private final Nested nested; + + ConstructorBindingWithOuterClassConstructorBoundProperties(Nested nested) { + this.nested = nested; + } + + Nested getNested() { + return this.nested; + } + + static class Nested { + + private Outer outer; + + Outer getOuter() { + return this.outer; + } + + void setOuter(Outer nested) { + this.outer = nested; + } + + } + + } + + static class Outer { + + private int age; + + Outer(int age) { + this.age = age; + } + + int getAge() { + return this.age; + } + + } + + @EnableConfigurationProperties(ConstructorBindingWithOuterClassConstructorBoundProperties.class) + static class ConstructorBindingWithOuterClassConstructorBoundConfiguration { + + } + @ConfigurationProperties("test") static class MultiConstructorConfigurationListProperties { diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/JavaBeanBinderTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/JavaBeanBinderTests.java index 80c8748f4d..9bd77b27bb 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/JavaBeanBinderTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/JavaBeanBinderTests.java @@ -339,7 +339,8 @@ class JavaBeanBinderTests { @Test void bindToInstanceWhenNoDefaultConstructorShouldBind() { - Binder binder = new Binder(this.sources, null, null, null, null, (type) -> null); + Binder binder = new Binder(this.sources, null, null, null, null, + (bindable, isNestedConstructorBinding) -> null); MockConfigurationPropertySource source = new MockConfigurationPropertySource(); source.put("foo.value", "bar"); this.sources.add(source); 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 ad2e3d16bd..14cf117522 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 @@ -103,7 +103,8 @@ class ValueObjectBinderTests { this.sources.add(source); Constructor[] constructors = MultipleConstructorsBean.class.getDeclaredConstructors(); Constructor constructor = (constructors[0].getParameterCount() == 1) ? constructors[0] : constructors[1]; - Binder binder = new Binder(this.sources, null, null, null, null, (type) -> constructor); + Binder binder = new Binder(this.sources, null, null, null, null, + (bindable, isNestedConstructorBinding) -> constructor); MultipleConstructorsBean bound = binder.bind("foo", Bindable.of(MultipleConstructorsBean.class)).get(); assertThat(bound.getIntValue()).isEqualTo(12); }