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 3f111223e38..a1a93cf7fe3 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 @@ -18,7 +18,6 @@ package org.springframework.boot.context.properties; import java.lang.annotation.Annotation; import java.lang.reflect.AnnotatedElement; -import java.lang.reflect.Constructor; import java.lang.reflect.Method; import java.util.Iterator; import java.util.LinkedHashMap; @@ -26,7 +25,6 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicReference; import org.springframework.aop.support.AopUtils; -import org.springframework.beans.BeanUtils; import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.BeanPostProcessor; @@ -37,7 +35,6 @@ import org.springframework.boot.context.properties.bind.Binder; import org.springframework.context.ApplicationContext; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.Bean; -import org.springframework.core.KotlinDetector; import org.springframework.core.ResolvableType; import org.springframework.core.annotation.MergedAnnotation; import org.springframework.core.annotation.MergedAnnotations; @@ -73,12 +70,12 @@ public final class ConfigurationPropertiesBean { private final BindMethod bindMethod; private ConfigurationPropertiesBean(String name, Object instance, ConfigurationProperties annotation, - Bindable bindTarget, BindMethod bindMethod) { + Bindable bindTarget) { this.name = name; this.instance = instance; this.annotation = annotation; this.bindTarget = bindTarget; - this.bindMethod = bindMethod; + this.bindMethod = BindMethod.forType(bindTarget.getType().resolve()); } /** @@ -264,16 +261,13 @@ public final class ConfigurationPropertiesBean { Validated validated = findAnnotation(instance, type, factory, Validated.class); Annotation[] annotations = (validated != null) ? new Annotation[] { annotation, validated } : new Annotation[] { annotation }; - Constructor bindConstructor = BindMethod.findBindConstructor(type); - BindMethod bindMethod = (bindConstructor != null) ? BindMethod.VALUE_OBJECT : BindMethod.forClass(type); ResolvableType bindType = (factory != null) ? ResolvableType.forMethodReturnType(factory) : ResolvableType.forClass(type); - Bindable bindTarget = Bindable.of(bindType).withAnnotations(annotations) - .withConstructorFilter(ConfigurationPropertiesBean::isBindableConstructor); + Bindable bindTarget = Bindable.of(bindType).withAnnotations(annotations); if (instance != null) { bindTarget = bindTarget.withExistingValue(instance); } - return new ConfigurationPropertiesBean(name, instance, annotation, bindTarget, bindMethod); + return new ConfigurationPropertiesBean(name, instance, annotation, bindTarget); } private static A findAnnotation(Object instance, Class type, Method factory, @@ -298,15 +292,6 @@ public final class ConfigurationPropertiesBean { : MergedAnnotation.missing(); } - private static boolean isBindableConstructor(Constructor constructor) { - Class declaringClass = constructor.getDeclaringClass(); - Constructor bindConstructor = BindMethod.findBindConstructor(declaringClass); - if (bindConstructor != null) { - return bindConstructor.equals(constructor); - } - return BindMethod.forClass(declaringClass) == BindMethod.VALUE_OBJECT; - } - /** * The binding method that is used for the bean. */ @@ -322,40 +307,9 @@ public final class ConfigurationPropertiesBean { */ VALUE_OBJECT; - static BindMethod forClass(Class type) { - if (isConstructorBindingType(type) || findBindConstructor(type) != null) { - return VALUE_OBJECT; - } - return JAVA_BEAN; - } - - private static boolean isConstructorBindingType(Class type) { - return MergedAnnotations.from(type, SearchStrategy.TYPE_HIERARCHY_AND_ENCLOSING_CLASSES) - .isPresent(ConstructorBinding.class); - } - - static Constructor findBindConstructor(Class type) { - if (KotlinDetector.isKotlinPresent() && KotlinDetector.isKotlinType(type)) { - Constructor constructor = BeanUtils.findPrimaryConstructor(type); - if (constructor != null) { - return findBindConstructor(type, constructor); - } - } - return findBindConstructor(type, type.getDeclaredConstructors()); - } - - private static Constructor findBindConstructor(Class type, Constructor... candidates) { - Constructor constructor = null; - for (Constructor candidate : candidates) { - if (MergedAnnotations.from(candidate).isPresent(ConstructorBinding.class)) { - Assert.state(candidate.getParameterCount() > 0, - type.getName() + " declares @ConstructorBinding on a no-args constructor"); - Assert.state(constructor == null, - type.getName() + " has more than one @ConstructorBinding constructor"); - constructor = candidate; - } - } - return constructor; + static BindMethod forType(Class type) { + return (ConfigurationPropertiesBindConstructorProvider.INSTANCE.getBindConstructor(type) != null) + ? VALUE_OBJECT : JAVA_BEAN; } } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanDefinitionValidator.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanDefinitionValidator.java index aa60114bf47..c5c4653c2c3 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanDefinitionValidator.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanDefinitionValidator.java @@ -54,7 +54,7 @@ class ConfigurationPropertiesBeanDefinitionValidator implements BeanFactoryPostP private void validate(ConfigurableListableBeanFactory beanFactory, String beanName) { Class beanClass = beanFactory.getType(beanName, false); - if (beanClass != null && BindMethod.forClass(beanClass) == BindMethod.VALUE_OBJECT) { + if (beanClass != null && BindMethod.forType(beanClass) == BindMethod.VALUE_OBJECT) { throw new BeanCreationException(beanName, "@EnableConfigurationProperties or @ConfigurationPropertiesScan must be used to add " + "@ConstructorBinding type " + beanClass.getName()); diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanRegistrar.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanRegistrar.java index 657fb3deabf..791e31e32ef 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanRegistrar.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanRegistrar.java @@ -88,7 +88,7 @@ final class ConfigurationPropertiesBeanRegistrar { } private BeanDefinition createBeanDefinition(String beanName, Class type) { - if (BindMethod.forClass(type) == BindMethod.VALUE_OBJECT) { + if (BindMethod.forType(type) == BindMethod.VALUE_OBJECT) { return new ConfigurationPropertiesValueObjectBeanDefinition(this.beanFactory, beanName, type); } GenericBeanDefinition definition = new GenericBeanDefinition(); 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 new file mode 100644 index 00000000000..d506881a7d7 --- /dev/null +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindConstructorProvider.java @@ -0,0 +1,107 @@ +/* + * Copyright 2012-2019 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.context.properties; + +import java.lang.reflect.Constructor; + +import org.springframework.beans.BeanUtils; +import org.springframework.boot.context.properties.bind.BindConstructorProvider; +import org.springframework.boot.context.properties.bind.Bindable; +import org.springframework.core.KotlinDetector; +import org.springframework.core.annotation.MergedAnnotations; +import org.springframework.util.Assert; + +/** + * {@link BindConstructorProvider} used when binding + * {@link ConfigurationProperties @ConfigurationProperties}. + * + * @author Madhura Bhave + * @author Phillip Webb + */ +class ConfigurationPropertiesBindConstructorProvider implements BindConstructorProvider { + + static final ConfigurationPropertiesBindConstructorProvider INSTANCE = new ConfigurationPropertiesBindConstructorProvider(); + + @Override + public Constructor getBindConstructor(Bindable bindable) { + return getBindConstructor(bindable.getType().resolve()); + } + + Constructor getBindConstructor(Class type) { + if (type == null) { + return null; + } + Constructor constructor = findConstructorBindingAnnotatedConstructor(type); + if (constructor == null && isConstructorBindingAnnotatedType(type)) { + constructor = deduceBindConstructor(type); + } + return constructor; + } + + private Constructor findConstructorBindingAnnotatedConstructor(Class type) { + if (isKotlinType(type)) { + Constructor constructor = BeanUtils.findPrimaryConstructor(type); + if (constructor != null) { + return findAnnotatedConstructor(type, constructor); + } + } + return findAnnotatedConstructor(type, type.getDeclaredConstructors()); + } + + private Constructor findAnnotatedConstructor(Class type, Constructor... candidates) { + Constructor constructor = null; + for (Constructor candidate : candidates) { + if (MergedAnnotations.from(candidate).isPresent(ConstructorBinding.class)) { + Assert.state(candidate.getParameterCount() > 0, + type.getName() + " declares @ConstructorBinding on a no-args constructor"); + Assert.state(constructor == null, + type.getName() + " has more than one @ConstructorBinding constructor"); + constructor = candidate; + } + } + return constructor; + } + + private boolean isConstructorBindingAnnotatedType(Class type) { + return MergedAnnotations.from(type, MergedAnnotations.SearchStrategy.TYPE_HIERARCHY_AND_ENCLOSING_CLASSES) + .isPresent(ConstructorBinding.class); + } + + private Constructor deduceBindConstructor(Class type) { + if (isKotlinType(type)) { + return deducedKotlinBindConstructor(type); + } + Constructor[] constructors = type.getDeclaredConstructors(); + if (constructors.length == 1 && constructors[0].getParameterCount() > 0) { + return constructors[0]; + } + return null; + } + + private Constructor deducedKotlinBindConstructor(Class type) { + Constructor primaryConstructor = BeanUtils.findPrimaryConstructor(type); + if (primaryConstructor != null && primaryConstructor.getParameterCount() > 0) { + return primaryConstructor; + } + return null; + } + + private boolean isKotlinType(Class type) { + return KotlinDetector.isKotlinPresent() && KotlinDetector.isKotlinType(type); + } + +} diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBinder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBinder.java index 97fae2230ea..18e7fdceb6e 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBinder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBinder.java @@ -151,7 +151,8 @@ class ConfigurationPropertiesBinder { private Binder getBinder() { if (this.binder == null) { this.binder = new Binder(getConfigurationPropertySources(), getPropertySourcesPlaceholdersResolver(), - getConversionService(), getPropertyEditorInitializer()); + getConversionService(), getPropertyEditorInitializer(), null, + ConfigurationPropertiesBindConstructorProvider.INSTANCE); } return this.binder; } 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 new file mode 100644 index 00000000000..1d26ed36146 --- /dev/null +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/BindConstructorProvider.java @@ -0,0 +1,44 @@ +/* + * Copyright 2012-2019 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.context.properties.bind; + +import java.lang.reflect.Constructor; + +/** + * Strategy interface used to determine a specific constructor to use when binding. + * + * @author Madhura Bhave + * @since 2.2.1 + */ +@FunctionalInterface +public interface BindConstructorProvider { + + /** + * Default {@link BindConstructorProvider} implementation that only returns a value + * when there's a single constructor and when the bindable has no existing value. + */ + BindConstructorProvider DEFAULT = new DefaultBindConstructorProvider(); + + /** + * 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 + * @return the bind constructor or {@code null} + */ + Constructor getBindConstructor(Bindable bindable); + +} 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 f5f66f8f63c..a7f2bdbaaff 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 @@ -18,11 +18,9 @@ package org.springframework.boot.context.properties.bind; import java.lang.annotation.Annotation; import java.lang.reflect.Array; -import java.lang.reflect.Constructor; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.function.Predicate; import java.util.function.Supplier; import org.springframework.core.ResolvableType; @@ -44,8 +42,6 @@ public final class Bindable { private static final Annotation[] NO_ANNOTATIONS = {}; - private static final Predicate> ANY_CONSTRUCTOR = (constructor) -> true; - private final ResolvableType type; private final ResolvableType boxedType; @@ -54,15 +50,11 @@ public final class Bindable { private final Annotation[] annotations; - private final Predicate> constructorFilter; - - private Bindable(ResolvableType type, ResolvableType boxedType, Supplier value, Annotation[] annotations, - Predicate> constructorFilter) { + private Bindable(ResolvableType type, ResolvableType boxedType, Supplier value, Annotation[] annotations) { this.type = type; this.boxedType = boxedType; this.value = value; this.annotations = annotations; - this.constructorFilter = constructorFilter; } /** @@ -113,16 +105,6 @@ public final class Bindable { return null; } - /** - * Return the constructor filter that can be used to limit the constructors that are - * considered when binding. - * @return the constructor filter - * @since 2.2.0 - */ - public Predicate> getConstructorFilter() { - return this.constructorFilter; - } - @Override public boolean equals(Object obj) { if (this == obj) { @@ -167,7 +149,7 @@ public final class Bindable { */ public Bindable withAnnotations(Annotation... annotations) { return new Bindable<>(this.type, this.boxedType, this.value, - (annotations != null) ? annotations : NO_ANNOTATIONS, this.constructorFilter); + (annotations != null) ? annotations : NO_ANNOTATIONS); } /** @@ -180,7 +162,7 @@ public final class Bindable { existingValue == null || this.type.isArray() || this.boxedType.resolve().isInstance(existingValue), () -> "ExistingValue must be an instance of " + this.type); Supplier value = (existingValue != null) ? () -> existingValue : null; - return new Bindable<>(this.type, this.boxedType, value, this.annotations, this.constructorFilter); + return new Bindable<>(this.type, this.boxedType, value, this.annotations); } /** @@ -189,19 +171,7 @@ public final class Bindable { * @return an updated {@link Bindable} */ public Bindable withSuppliedValue(Supplier suppliedValue) { - return new Bindable<>(this.type, this.boxedType, suppliedValue, this.annotations, this.constructorFilter); - } - - /** - * Create an updated {@link Bindable} instance with a constructor filter that can be - * used to limit the constructors considered when binding. - * @param constructorFilter the constructor filter to use - * @return an updated {@link Bindable} - * @since 2.2.0 - */ - public Bindable withConstructorFilter(Predicate> constructorFilter) { - return new Bindable<>(this.type, this.boxedType, this.value, this.annotations, - (constructorFilter != null) ? constructorFilter : ANY_CONSTRUCTOR); + return new Bindable<>(this.type, this.boxedType, suppliedValue, this.annotations); } /** @@ -274,7 +244,7 @@ public final class Bindable { public static Bindable of(ResolvableType type) { Assert.notNull(type, "Type must not be null"); ResolvableType boxedType = box(type); - return new Bindable<>(type, boxedType, null, NO_ANNOTATIONS, ANY_CONSTRUCTOR); + return new Bindable<>(type, boxedType, null, NO_ANNOTATIONS); } private static ResolvableType box(ResolvableType type) { 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 61c68169646..bb042298ae0 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 @@ -55,8 +55,6 @@ public class Binder { private static final Set> NON_BEAN_CLASSES = Collections .unmodifiableSet(new HashSet<>(Arrays.asList(Object.class, Class.class))); - private static final DataObjectBinder[] DATA_OBJECT_BINDERS = { new ValueObjectBinder(), new JavaBeanBinder() }; - private final Iterable sources; private final PlaceholdersResolver placeholdersResolver; @@ -67,6 +65,8 @@ public class Binder { private final BindHandler defaultBindHandler; + private final List dataObjectBinders; + /** * Create a new {@link Binder} instance for the specified sources. A * {@link DefaultFormattingConversionService} will be used for all conversion. @@ -137,6 +137,27 @@ public class Binder { public Binder(Iterable sources, PlaceholdersResolver placeholdersResolver, ConversionService conversionService, Consumer propertyEditorInitializer, BindHandler defaultBindHandler) { + this(sources, placeholdersResolver, conversionService, propertyEditorInitializer, defaultBindHandler, null); + } + + /** + * Create a new {@link Binder} instance for the specified sources. + * @param sources the sources used for binding + * @param placeholdersResolver strategy to resolve any property placeholders + * @param conversionService the conversion service to convert values (or {@code null} + * to use {@link ApplicationConversionService}) + * @param propertyEditorInitializer initializer used to configure the property editors + * that can convert values (or {@code null} if no initialization is required). Often + * used to call {@link ConfigurableListableBeanFactory#copyRegisteredEditorsTo}. + * @param defaultBindHandler the default bind handler to use if none is specified when + * binding + * @param constructorProvider the constructor provider which provides the bind + * constructor to use when binding + * @since 2.2.1 + */ + public Binder(Iterable sources, PlaceholdersResolver placeholdersResolver, + ConversionService conversionService, Consumer propertyEditorInitializer, + BindHandler defaultBindHandler, BindConstructorProvider constructorProvider) { Assert.notNull(sources, "Sources must not be null"); this.sources = sources; this.placeholdersResolver = (placeholdersResolver != null) ? placeholdersResolver : PlaceholdersResolver.NONE; @@ -144,6 +165,12 @@ public class Binder { : ApplicationConversionService.getSharedInstance(); this.propertyEditorInitializer = propertyEditorInitializer; this.defaultBindHandler = (defaultBindHandler != null) ? defaultBindHandler : BindHandler.DEFAULT; + if (constructorProvider == null) { + constructorProvider = BindConstructorProvider.DEFAULT; + } + ValueObjectBinder valueObjectBinder = new ValueObjectBinder(constructorProvider); + JavaBeanBinder javaBeanBinder = JavaBeanBinder.INSTANCE; + this.dataObjectBinders = Collections.unmodifiableList(Arrays.asList(valueObjectBinder, javaBeanBinder)); } /** @@ -315,7 +342,7 @@ public class Binder { } private Object create(Bindable target, Context context) { - for (DataObjectBinder dataObjectBinder : DATA_OBJECT_BINDERS) { + for (DataObjectBinder dataObjectBinder : this.dataObjectBinders) { Object instance = dataObjectBinder.create(target, context); if (instance != null) { return instance; @@ -421,7 +448,7 @@ public class Binder { DataObjectPropertyBinder propertyBinder = (propertyName, propertyTarget) -> bind(name.append(propertyName), propertyTarget, handler, context, false, false); return context.withDataObject(type, () -> { - for (DataObjectBinder dataObjectBinder : DATA_OBJECT_BINDERS) { + for (DataObjectBinder dataObjectBinder : this.dataObjectBinders) { Object instance = dataObjectBinder.bind(name, target, context, propertyBinder); if (instance != null) { return instance; 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 new file mode 100644 index 00000000000..c9e00110f1b --- /dev/null +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/DefaultBindConstructorProvider.java @@ -0,0 +1,56 @@ +/* + * Copyright 2012-2019 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.context.properties.bind; + +import java.lang.reflect.Constructor; + +import org.springframework.beans.BeanUtils; +import org.springframework.core.KotlinDetector; + +/** + * Default {@link BindConstructorProvider} implementation. + * + * @author Madhura Bhave + * @author Phillip Webb + */ +class DefaultBindConstructorProvider implements BindConstructorProvider { + + @Override + public Constructor getBindConstructor(Bindable bindable) { + Class type = bindable.getType().resolve(); + if (bindable.getValue() != null || type == null) { + return null; + } + if (KotlinDetector.isKotlinPresent() && KotlinDetector.isKotlinType(type)) { + return getDeducedKotlinConstructor(type); + } + Constructor[] constructors = type.getDeclaredConstructors(); + if (constructors.length == 1 && constructors[0].getParameterCount() > 0) { + return constructors[0]; + } + return null; + } + + private Constructor getDeducedKotlinConstructor(Class type) { + Constructor primaryConstructor = BeanUtils.findPrimaryConstructor(type); + if (primaryConstructor != null && primaryConstructor.getParameterCount() > 0) { + return primaryConstructor; + } + return null; + } + +} diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/JavaBeanBinder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/JavaBeanBinder.java index 8af5adedf0e..35ca1be4808 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/JavaBeanBinder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/JavaBeanBinder.java @@ -42,6 +42,8 @@ import org.springframework.core.ResolvableType; */ class JavaBeanBinder implements DataObjectBinder { + static final JavaBeanBinder INSTANCE = new JavaBeanBinder(); + @Override public T bind(ConfigurationPropertyName name, Bindable target, Context context, DataObjectPropertyBinder propertyBinder) { 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 833afdfa1f8..c8efd8ac75d 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 @@ -22,7 +22,6 @@ import java.lang.reflect.Parameter; import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.function.Predicate; import kotlin.reflect.KFunction; import kotlin.reflect.KParameter; @@ -45,10 +44,16 @@ import org.springframework.util.Assert; */ class ValueObjectBinder implements DataObjectBinder { + private final BindConstructorProvider constructorProvider; + + ValueObjectBinder(BindConstructorProvider constructorProvider) { + this.constructorProvider = constructorProvider; + } + @Override public T bind(ConfigurationPropertyName name, Bindable target, Binder.Context context, DataObjectPropertyBinder propertyBinder) { - ValueObject valueObject = ValueObject.get(target); + ValueObject valueObject = ValueObject.get(target, this.constructorProvider); if (valueObject == null) { return null; } @@ -67,7 +72,7 @@ class ValueObjectBinder implements DataObjectBinder { @Override public T create(Bindable target, Binder.Context context) { - ValueObject valueObject = ValueObject.get(target); + ValueObject valueObject = ValueObject.get(target, this.constructorProvider); if (valueObject == null) { return null; } @@ -99,18 +104,19 @@ class ValueObjectBinder implements DataObjectBinder { abstract List getConstructorParameters(); @SuppressWarnings("unchecked") - static ValueObject get(Bindable bindable) { - if (bindable.getValue() != null) { - return null; - } + static ValueObject get(Bindable bindable, BindConstructorProvider constructorProvider) { Class type = (Class) bindable.getType().resolve(); if (type == null || type.isEnum() || Modifier.isAbstract(type.getModifiers())) { return null; } + Constructor bindConstructor = constructorProvider.getBindConstructor(bindable); + if (bindConstructor == null) { + return null; + } if (KotlinDetector.isKotlinType(type)) { - return KotlinValueObject.get(type, bindable.getConstructorFilter()); + return KotlinValueObject.get((Constructor) bindConstructor); } - return DefaultValueObject.get(type, bindable.getConstructorFilter()); + return DefaultValueObject.get(bindConstructor); } } @@ -144,17 +150,12 @@ class ValueObjectBinder implements DataObjectBinder { return this.constructorParameters; } - static ValueObject get(Class type, Predicate> constructorFilter) { - Constructor primaryConstructor = BeanUtils.findPrimaryConstructor(type); - if (primaryConstructor == null || primaryConstructor.getParameterCount() == 0 - || !constructorFilter.test(primaryConstructor)) { - return null; - } - KFunction kotlinConstructor = ReflectJvmMapping.getKotlinFunction(primaryConstructor); + static ValueObject get(Constructor bindConstructor) { + KFunction kotlinConstructor = ReflectJvmMapping.getKotlinFunction(bindConstructor); if (kotlinConstructor != null) { - return new KotlinValueObject<>(primaryConstructor, kotlinConstructor); + return new KotlinValueObject<>(bindConstructor, kotlinConstructor); } - return DefaultValueObject.get(primaryConstructor); + return DefaultValueObject.get(bindConstructor); } } @@ -194,29 +195,8 @@ class ValueObjectBinder implements DataObjectBinder { } @SuppressWarnings("unchecked") - static ValueObject get(Class type, Predicate> constructorFilter) { - Constructor constructor = null; - for (Constructor candidate : type.getDeclaredConstructors()) { - if (isCandidateConstructor(candidate, constructorFilter)) { - if (constructor != null) { - return null; - } - constructor = candidate; - } - } - return get((Constructor) constructor); - } - - private static boolean isCandidateConstructor(Constructor candidate, Predicate> filter) { - int modifiers = candidate.getModifiers(); - return !Modifier.isPrivate(modifiers) && !Modifier.isProtected(modifiers) && filter.test(candidate); - } - - static DefaultValueObject get(Constructor constructor) { - if (constructor == null || constructor.getParameterCount() == 0) { - return null; - } - return new DefaultValueObject<>(constructor); + static ValueObject get(Constructor bindConstructor) { + return new DefaultValueObject<>((Constructor) bindConstructor); } } 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 9fffe208f07..3df78ee425a 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 @@ -16,7 +16,6 @@ package org.springframework.boot.context.properties; -import java.util.Arrays; import java.util.Map; import org.junit.jupiter.api.Test; @@ -213,8 +212,8 @@ class ConfigurationPropertiesBeanTests { Bindable target = propertiesBean.asBindTarget(); assertThat(target.getType()).isEqualTo(ResolvableType.forClass(ConstructorBindingOnConstructor.class)); assertThat(target.getValue()).isNull(); - assertThat(Arrays.stream(ConstructorBindingOnConstructor.class.getDeclaredConstructors()) - .filter(target.getConstructorFilter())).hasSize(1); + assertThat(ConfigurationPropertiesBindConstructorProvider.INSTANCE + .getBindConstructor(ConstructorBindingOnConstructor.class)).isNotNull(); } @Test @@ -230,27 +229,27 @@ class ConfigurationPropertiesBeanTests { } @Test - void bindTypeForClassWhenNoConstructorBindingReturnsJavaBean() { - BindMethod bindType = BindMethod.forClass(NoConstructorBinding.class); + void bindTypeForTypeWhenNoConstructorBindingReturnsJavaBean() { + BindMethod bindType = BindMethod.forType(NoConstructorBinding.class); assertThat(bindType).isEqualTo(BindMethod.JAVA_BEAN); } @Test - void bindTypeForClassWhenNoConstructorBindingOnTypeReturnsValueObject() { - BindMethod bindType = BindMethod.forClass(ConstructorBindingOnType.class); + void bindTypeForTypeWhenNoConstructorBindingOnTypeReturnsValueObject() { + BindMethod bindType = BindMethod.forType(ConstructorBindingOnType.class); assertThat(bindType).isEqualTo(BindMethod.VALUE_OBJECT); } @Test - void bindTypeForClassWhenNoConstructorBindingOnConstructorReturnsValueObject() { - BindMethod bindType = BindMethod.forClass(ConstructorBindingOnConstructor.class); + void bindTypeForTypeWhenNoConstructorBindingOnConstructorReturnsValueObject() { + BindMethod bindType = BindMethod.forType(ConstructorBindingOnConstructor.class); assertThat(bindType).isEqualTo(BindMethod.VALUE_OBJECT); } @Test - void bindTypeForClassWhenConstructorBindingOnMultipleConstructorsThrowsException() { + void bindTypeForTypeWhenConstructorBindingOnMultipleConstructorsThrowsException() { assertThatIllegalStateException() - .isThrownBy(() -> BindMethod.forClass(ConstructorBindingOnMultipleConstructors.class)) + .isThrownBy(() -> BindMethod.forType(ConstructorBindingOnMultipleConstructors.class)) .withMessage(ConstructorBindingOnMultipleConstructors.class.getName() + " has more than one @ConstructorBinding constructor"); } 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 4243ba3afde..9c3ee77a5a3 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 @@ -849,6 +849,52 @@ class ConfigurationPropertiesTests { assertThat(bean.getNested().getAge()).isEqualTo(5); } + @Test + void loadWhenBindingToNestedPropertiesWithSyntheticConstructorShouldBind() { + MutablePropertySources sources = this.context.getEnvironment().getPropertySources(); + Map source = new HashMap<>(); + source.put("test.nested.age", "5"); + sources.addLast(new MapPropertySource("test", source)); + load(SyntheticConstructorPropertiesConfiguration.class); + SyntheticNestedConstructorProperties bean = this.context.getBean(SyntheticNestedConstructorProperties.class); + assertThat(bean.getNested().getAge()).isEqualTo(5); + } + + @Test + void loadWhenBindingToJavaBeanWithNestedConstructorBindingShouldBind() { + MutablePropertySources sources = this.context.getEnvironment().getPropertySources(); + Map source = new HashMap<>(); + source.put("test.nested.age", "5"); + sources.addLast(new MapPropertySource("test", source)); + load(JavaBeanNestedConstructorBindingPropertiesConfiguration.class); + JavaBeanNestedConstructorBindingProperties bean = this.context + .getBean(JavaBeanNestedConstructorBindingProperties.class); + assertThat(bean.getNested().getAge()).isEqualTo(5); + } + + @Test + void loadWhenBindingToNestedWithMultipleConstructorsShouldBind() { + MutablePropertySources sources = this.context.getEnvironment().getPropertySources(); + Map source = new HashMap<>(); + source.put("test.nested.age", "5"); + sources.addLast(new MapPropertySource("test", source)); + load(NestedMultipleConstructorsConfiguration.class); + NestedMultipleConstructorProperties bean = this.context.getBean(NestedMultipleConstructorProperties.class); + assertThat(bean.getNested().getAge()).isEqualTo(5); + } + + @Test + void loadWhenBindingToJavaBeanWithoutExplicitConstructorBindingOnNestedShouldUseSetterBasedBinding() { + MutablePropertySources sources = this.context.getEnvironment().getPropertySources(); + Map source = new HashMap<>(); + source.put("test.nested.age", "5"); + sources.addLast(new MapPropertySource("test", source)); + load(JavaBeanNonDefaultConstructorPropertiesConfiguration.class); + JavaBeanNonDefaultConstructorProperties bean = this.context + .getBean(JavaBeanNonDefaultConstructorProperties.class); + assertThat(bean.getNested().getAge()).isEqualTo(10); + } + private AnnotationConfigApplicationContext load(Class configuration, String... inlinedProperties) { return load(new Class[] { configuration }, inlinedProperties); } @@ -2014,6 +2060,54 @@ class ConfigurationPropertiesTests { } + @ConstructorBinding + @ConfigurationProperties("test") + static class NestedMultipleConstructorProperties { + + private final String name; + + private final Nested nested; + + NestedMultipleConstructorProperties(String name, Nested nested) { + this.name = name; + this.nested = nested; + } + + String getName() { + return this.name; + } + + Nested getNested() { + return this.nested; + } + + static class Nested { + + private int age; + + Nested(String property) { + + } + + @ConstructorBinding + Nested(int age) { + this.age = age; + } + + int getAge() { + return this.age; + } + + } + + } + + @Configuration(proxyBeanMethods = false) + @EnableConfigurationProperties(NestedMultipleConstructorProperties.class) + static class NestedMultipleConstructorsConfiguration { + + } + @ConfigurationProperties("test") static class MultiConstructorConfigurationListProperties { @@ -2031,6 +2125,139 @@ class ConfigurationPropertiesTests { } + @Configuration(proxyBeanMethods = false) + @EnableConfigurationProperties(JavaBeanNestedConstructorBindingProperties.class) + static class JavaBeanNestedConstructorBindingPropertiesConfiguration { + + } + + @ConfigurationProperties("test") + static class JavaBeanNestedConstructorBindingProperties { + + private Nested nested; + + Nested getNested() { + return this.nested; + } + + void setNested(Nested nested) { + this.nested = nested; + } + + static final class Nested { + + private final int age; + + @ConstructorBinding + private Nested(int age) { + this.age = age; + } + + int getAge() { + return this.age; + } + + } + + } + + @Configuration(proxyBeanMethods = false) + @EnableConfigurationProperties(JavaBeanNonDefaultConstructorProperties.class) + static class JavaBeanNonDefaultConstructorPropertiesConfiguration { + + } + + @ConfigurationProperties("test") + static class JavaBeanNonDefaultConstructorProperties { + + private Nested nested; + + Nested getNested() { + return this.nested; + } + + void setNested(Nested nested) { + this.nested = nested; + } + + static final class Nested { + + private int age; + + private Nested() { + + } + + private Nested(int age) { + this.age = age; + } + + int getAge() { + return this.age; + } + + void setAge(int age) { + this.age = age + 5; + } + + } + + } + + @Configuration(proxyBeanMethods = false) + @EnableConfigurationProperties(SyntheticNestedConstructorProperties.class) + static class SyntheticConstructorPropertiesConfiguration { + + } + + @ConstructorBinding + @ConfigurationProperties("test") + static class SyntheticNestedConstructorProperties { + + private final Nested nested; + + SyntheticNestedConstructorProperties(Nested nested) { + this.nested = nested; + } + + Nested getNested() { + return this.nested; + } + + static final class Nested { + + private int age; + + private Nested() { + + } + + int getAge() { + return this.age; + } + + void setAge(int age) { + this.age = age; + } + + static class AnotherNested { + + private final Nested nested; + + AnotherNested(String name) { + this.nested = new Nested(); + } + + Nested getNested() { + return this.nested; + } + + } + + } + + } + @Configuration(proxyBeanMethods = false) @EnableConfigurationProperties(DeducedNestedConstructorProperties.class) static class DeducedNestedConstructorPropertiesConfiguration { diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BindableTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BindableTests.java index 6cd1fc3d968..ea05728fec0 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BindableTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BindableTests.java @@ -19,8 +19,6 @@ package org.springframework.boot.context.properties.bind; import java.lang.annotation.Annotation; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; -import java.lang.reflect.Constructor; -import java.util.function.Predicate; import org.junit.jupiter.api.Test; @@ -176,19 +174,6 @@ class BindableTests { assertThat(bindable.getAnnotations()).containsExactly(annotation); } - @Test - void withConstructorFilterSetsConstructorFilter() { - Predicate> constructorFilter = (constructor) -> false; - Bindable bindable = Bindable.of(TestNewInstance.class).withConstructorFilter(constructorFilter); - assertThat(bindable.getConstructorFilter()).isSameAs(constructorFilter); - } - - @Test - void withConstructorFilterWhenFilterIsNullMatchesAll() { - Bindable bindable = Bindable.of(TestNewInstance.class).withConstructorFilter(null); - assertThat(bindable.getConstructorFilter()).isSameAs(Bindable.of(TestNewInstance.class).getConstructorFilter()); - } - @Retention(RetentionPolicy.RUNTIME) @interface TestAnnotation { 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 7b42c8b7b5c..80c8748f4dc 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,11 +339,12 @@ class JavaBeanBinderTests { @Test void bindToInstanceWhenNoDefaultConstructorShouldBind() { + Binder binder = new Binder(this.sources, null, null, null, null, (type) -> null); MockConfigurationPropertySource source = new MockConfigurationPropertySource(); source.put("foo.value", "bar"); this.sources.add(source); ExampleWithNonDefaultConstructor bean = new ExampleWithNonDefaultConstructor("faf"); - ExampleWithNonDefaultConstructor boundBean = this.binder + ExampleWithNonDefaultConstructor boundBean = binder .bind("foo", Bindable.of(ExampleWithNonDefaultConstructor.class).withExistingValue(bean)).get(); assertThat(boundBean).isSameAs(bean); assertThat(bean.getValue()).isEqualTo("bar"); 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 ef4c67b42e1..ad2e3d16bdb 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,8 +103,8 @@ class ValueObjectBinderTests { this.sources.add(source); Constructor[] constructors = MultipleConstructorsBean.class.getDeclaredConstructors(); Constructor constructor = (constructors[0].getParameterCount() == 1) ? constructors[0] : constructors[1]; - MultipleConstructorsBean bound = this.binder.bind("foo", Bindable.of(MultipleConstructorsBean.class) - .withConstructorFilter((candidate) -> candidate.equals(constructor))).get(); + Binder binder = new Binder(this.sources, null, null, null, null, (type) -> constructor); + MultipleConstructorsBean bound = binder.bind("foo", Bindable.of(MultipleConstructorsBean.class)).get(); assertThat(bound.getIntValue()).isEqualTo(12); }