Commit f9785d2b authored by Madhura Bhave's avatar Madhura Bhave

Fix constructor binding issues

This commit fixes a few bugs related to constructor binding.
The ContructorFilter on the Bindable has been replaced with a
Binder level BinderConstructorProvider so that it can be used to
determine the constructor to use for nested properties as well.

Fixes gh-18810
Fixes gh-18670
Closes gh-18685
Closes gh-18894
Co-authored-by: 's avatarPhillip Webb <pwebb@pivotal.io>
parent 90e1046d
...@@ -18,7 +18,6 @@ package org.springframework.boot.context.properties; ...@@ -18,7 +18,6 @@ package org.springframework.boot.context.properties;
import java.lang.annotation.Annotation; import java.lang.annotation.Annotation;
import java.lang.reflect.AnnotatedElement; import java.lang.reflect.AnnotatedElement;
import java.lang.reflect.Constructor;
import java.lang.reflect.Method; import java.lang.reflect.Method;
import java.util.Iterator; import java.util.Iterator;
import java.util.LinkedHashMap; import java.util.LinkedHashMap;
...@@ -26,7 +25,6 @@ import java.util.Map; ...@@ -26,7 +25,6 @@ import java.util.Map;
import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.atomic.AtomicReference;
import org.springframework.aop.support.AopUtils; import org.springframework.aop.support.AopUtils;
import org.springframework.beans.BeanUtils;
import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.beans.factory.NoSuchBeanDefinitionException;
import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.BeanDefinition;
import org.springframework.beans.factory.config.BeanPostProcessor; import org.springframework.beans.factory.config.BeanPostProcessor;
...@@ -37,7 +35,6 @@ import org.springframework.boot.context.properties.bind.Binder; ...@@ -37,7 +35,6 @@ import org.springframework.boot.context.properties.bind.Binder;
import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContext;
import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.ConfigurableApplicationContext;
import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Bean;
import org.springframework.core.KotlinDetector;
import org.springframework.core.ResolvableType; import org.springframework.core.ResolvableType;
import org.springframework.core.annotation.MergedAnnotation; import org.springframework.core.annotation.MergedAnnotation;
import org.springframework.core.annotation.MergedAnnotations; import org.springframework.core.annotation.MergedAnnotations;
...@@ -73,12 +70,12 @@ public final class ConfigurationPropertiesBean { ...@@ -73,12 +70,12 @@ public final class ConfigurationPropertiesBean {
private final BindMethod bindMethod; private final BindMethod bindMethod;
private ConfigurationPropertiesBean(String name, Object instance, ConfigurationProperties annotation, private ConfigurationPropertiesBean(String name, Object instance, ConfigurationProperties annotation,
Bindable<?> bindTarget, BindMethod bindMethod) { Bindable<?> bindTarget) {
this.name = name; this.name = name;
this.instance = instance; this.instance = instance;
this.annotation = annotation; this.annotation = annotation;
this.bindTarget = bindTarget; this.bindTarget = bindTarget;
this.bindMethod = bindMethod; this.bindMethod = BindMethod.forType(bindTarget.getType().resolve());
} }
/** /**
...@@ -264,16 +261,13 @@ public final class ConfigurationPropertiesBean { ...@@ -264,16 +261,13 @@ public final class ConfigurationPropertiesBean {
Validated validated = findAnnotation(instance, type, factory, Validated.class); Validated validated = findAnnotation(instance, type, factory, Validated.class);
Annotation[] annotations = (validated != null) ? new Annotation[] { annotation, validated } Annotation[] annotations = (validated != null) ? new Annotation[] { annotation, validated }
: new Annotation[] { annotation }; : 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 bindType = (factory != null) ? ResolvableType.forMethodReturnType(factory)
: ResolvableType.forClass(type); : ResolvableType.forClass(type);
Bindable<Object> bindTarget = Bindable.of(bindType).withAnnotations(annotations) Bindable<Object> bindTarget = Bindable.of(bindType).withAnnotations(annotations);
.withConstructorFilter(ConfigurationPropertiesBean::isBindableConstructor);
if (instance != null) { if (instance != null) {
bindTarget = bindTarget.withExistingValue(instance); bindTarget = bindTarget.withExistingValue(instance);
} }
return new ConfigurationPropertiesBean(name, instance, annotation, bindTarget, bindMethod); return new ConfigurationPropertiesBean(name, instance, annotation, bindTarget);
} }
private static <A extends Annotation> A findAnnotation(Object instance, Class<?> type, Method factory, private static <A extends Annotation> A findAnnotation(Object instance, Class<?> type, Method factory,
...@@ -298,15 +292,6 @@ public final class ConfigurationPropertiesBean { ...@@ -298,15 +292,6 @@ public final class ConfigurationPropertiesBean {
: MergedAnnotation.missing(); : 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. * The binding method that is used for the bean.
*/ */
...@@ -322,40 +307,9 @@ public final class ConfigurationPropertiesBean { ...@@ -322,40 +307,9 @@ public final class ConfigurationPropertiesBean {
*/ */
VALUE_OBJECT; VALUE_OBJECT;
static BindMethod forClass(Class<?> type) { static BindMethod forType(Class<?> type) {
if (isConstructorBindingType(type) || findBindConstructor(type) != null) { return (ConfigurationPropertiesBindConstructorProvider.INSTANCE.getBindConstructor(type) != null)
return VALUE_OBJECT; ? VALUE_OBJECT : JAVA_BEAN;
}
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;
} }
} }
......
...@@ -54,7 +54,7 @@ class ConfigurationPropertiesBeanDefinitionValidator implements BeanFactoryPostP ...@@ -54,7 +54,7 @@ class ConfigurationPropertiesBeanDefinitionValidator implements BeanFactoryPostP
private void validate(ConfigurableListableBeanFactory beanFactory, String beanName) { private void validate(ConfigurableListableBeanFactory beanFactory, String beanName) {
Class<?> beanClass = beanFactory.getType(beanName, false); 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, throw new BeanCreationException(beanName,
"@EnableConfigurationProperties or @ConfigurationPropertiesScan must be used to add " "@EnableConfigurationProperties or @ConfigurationPropertiesScan must be used to add "
+ "@ConstructorBinding type " + beanClass.getName()); + "@ConstructorBinding type " + beanClass.getName());
......
...@@ -88,7 +88,7 @@ final class ConfigurationPropertiesBeanRegistrar { ...@@ -88,7 +88,7 @@ final class ConfigurationPropertiesBeanRegistrar {
} }
private BeanDefinition createBeanDefinition(String beanName, Class<?> type) { 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); return new ConfigurationPropertiesValueObjectBeanDefinition(this.beanFactory, beanName, type);
} }
GenericBeanDefinition definition = new GenericBeanDefinition(); GenericBeanDefinition definition = new GenericBeanDefinition();
......
/*
* 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);
}
}
...@@ -151,7 +151,8 @@ class ConfigurationPropertiesBinder { ...@@ -151,7 +151,8 @@ class ConfigurationPropertiesBinder {
private Binder getBinder() { private Binder getBinder() {
if (this.binder == null) { if (this.binder == null) {
this.binder = new Binder(getConfigurationPropertySources(), getPropertySourcesPlaceholdersResolver(), this.binder = new Binder(getConfigurationPropertySources(), getPropertySourcesPlaceholdersResolver(),
getConversionService(), getPropertyEditorInitializer()); getConversionService(), getPropertyEditorInitializer(), null,
ConfigurationPropertiesBindConstructorProvider.INSTANCE);
} }
return this.binder; return this.binder;
} }
......
/*
* 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);
}
...@@ -18,11 +18,9 @@ package org.springframework.boot.context.properties.bind; ...@@ -18,11 +18,9 @@ package org.springframework.boot.context.properties.bind;
import java.lang.annotation.Annotation; import java.lang.annotation.Annotation;
import java.lang.reflect.Array; import java.lang.reflect.Array;
import java.lang.reflect.Constructor;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Set; import java.util.Set;
import java.util.function.Predicate;
import java.util.function.Supplier; import java.util.function.Supplier;
import org.springframework.core.ResolvableType; import org.springframework.core.ResolvableType;
...@@ -44,8 +42,6 @@ public final class Bindable<T> { ...@@ -44,8 +42,6 @@ public final class Bindable<T> {
private static final Annotation[] NO_ANNOTATIONS = {}; private static final Annotation[] NO_ANNOTATIONS = {};
private static final Predicate<Constructor<?>> ANY_CONSTRUCTOR = (constructor) -> true;
private final ResolvableType type; private final ResolvableType type;
private final ResolvableType boxedType; private final ResolvableType boxedType;
...@@ -54,15 +50,11 @@ public final class Bindable<T> { ...@@ -54,15 +50,11 @@ public final class Bindable<T> {
private final Annotation[] annotations; private final Annotation[] annotations;
private final Predicate<Constructor<?>> constructorFilter; private Bindable(ResolvableType type, ResolvableType boxedType, Supplier<T> value, Annotation[] annotations) {
private Bindable(ResolvableType type, ResolvableType boxedType, Supplier<T> value, Annotation[] annotations,
Predicate<Constructor<?>> constructorFilter) {
this.type = type; this.type = type;
this.boxedType = boxedType; this.boxedType = boxedType;
this.value = value; this.value = value;
this.annotations = annotations; this.annotations = annotations;
this.constructorFilter = constructorFilter;
} }
/** /**
...@@ -113,16 +105,6 @@ public final class Bindable<T> { ...@@ -113,16 +105,6 @@ public final class Bindable<T> {
return null; 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<Constructor<?>> getConstructorFilter() {
return this.constructorFilter;
}
@Override @Override
public boolean equals(Object obj) { public boolean equals(Object obj) {
if (this == obj) { if (this == obj) {
...@@ -167,7 +149,7 @@ public final class Bindable<T> { ...@@ -167,7 +149,7 @@ public final class Bindable<T> {
*/ */
public Bindable<T> withAnnotations(Annotation... annotations) { public Bindable<T> withAnnotations(Annotation... annotations) {
return new Bindable<>(this.type, this.boxedType, this.value, 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<T> { ...@@ -180,7 +162,7 @@ public final class Bindable<T> {
existingValue == null || this.type.isArray() || this.boxedType.resolve().isInstance(existingValue), existingValue == null || this.type.isArray() || this.boxedType.resolve().isInstance(existingValue),
() -> "ExistingValue must be an instance of " + this.type); () -> "ExistingValue must be an instance of " + this.type);
Supplier<T> value = (existingValue != null) ? () -> existingValue : null; Supplier<T> 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<T> { ...@@ -189,19 +171,7 @@ public final class Bindable<T> {
* @return an updated {@link Bindable} * @return an updated {@link Bindable}
*/ */
public Bindable<T> withSuppliedValue(Supplier<T> suppliedValue) { public Bindable<T> withSuppliedValue(Supplier<T> suppliedValue) {
return new Bindable<>(this.type, this.boxedType, suppliedValue, this.annotations, this.constructorFilter); return new Bindable<>(this.type, this.boxedType, suppliedValue, this.annotations);
}
/**
* 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<T> withConstructorFilter(Predicate<Constructor<?>> constructorFilter) {
return new Bindable<>(this.type, this.boxedType, this.value, this.annotations,
(constructorFilter != null) ? constructorFilter : ANY_CONSTRUCTOR);
} }
/** /**
...@@ -274,7 +244,7 @@ public final class Bindable<T> { ...@@ -274,7 +244,7 @@ public final class Bindable<T> {
public static <T> Bindable<T> of(ResolvableType type) { public static <T> Bindable<T> of(ResolvableType type) {
Assert.notNull(type, "Type must not be null"); Assert.notNull(type, "Type must not be null");
ResolvableType boxedType = box(type); 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) { private static ResolvableType box(ResolvableType type) {
......
...@@ -55,8 +55,6 @@ public class Binder { ...@@ -55,8 +55,6 @@ public class Binder {
private static final Set<Class<?>> NON_BEAN_CLASSES = Collections private static final Set<Class<?>> NON_BEAN_CLASSES = Collections
.unmodifiableSet(new HashSet<>(Arrays.asList(Object.class, Class.class))); .unmodifiableSet(new HashSet<>(Arrays.asList(Object.class, Class.class)));
private static final DataObjectBinder[] DATA_OBJECT_BINDERS = { new ValueObjectBinder(), new JavaBeanBinder() };
private final Iterable<ConfigurationPropertySource> sources; private final Iterable<ConfigurationPropertySource> sources;
private final PlaceholdersResolver placeholdersResolver; private final PlaceholdersResolver placeholdersResolver;
...@@ -67,6 +65,8 @@ public class Binder { ...@@ -67,6 +65,8 @@ public class Binder {
private final BindHandler defaultBindHandler; private final BindHandler defaultBindHandler;
private final List<DataObjectBinder> dataObjectBinders;
/** /**
* Create a new {@link Binder} instance for the specified sources. A * Create a new {@link Binder} instance for the specified sources. A
* {@link DefaultFormattingConversionService} will be used for all conversion. * {@link DefaultFormattingConversionService} will be used for all conversion.
...@@ -137,6 +137,27 @@ public class Binder { ...@@ -137,6 +137,27 @@ public class Binder {
public Binder(Iterable<ConfigurationPropertySource> sources, PlaceholdersResolver placeholdersResolver, public Binder(Iterable<ConfigurationPropertySource> sources, PlaceholdersResolver placeholdersResolver,
ConversionService conversionService, Consumer<PropertyEditorRegistry> propertyEditorInitializer, ConversionService conversionService, Consumer<PropertyEditorRegistry> propertyEditorInitializer,
BindHandler defaultBindHandler) { 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<ConfigurationPropertySource> sources, PlaceholdersResolver placeholdersResolver,
ConversionService conversionService, Consumer<PropertyEditorRegistry> propertyEditorInitializer,
BindHandler defaultBindHandler, BindConstructorProvider constructorProvider) {
Assert.notNull(sources, "Sources must not be null"); Assert.notNull(sources, "Sources must not be null");
this.sources = sources; this.sources = sources;
this.placeholdersResolver = (placeholdersResolver != null) ? placeholdersResolver : PlaceholdersResolver.NONE; this.placeholdersResolver = (placeholdersResolver != null) ? placeholdersResolver : PlaceholdersResolver.NONE;
...@@ -144,6 +165,12 @@ public class Binder { ...@@ -144,6 +165,12 @@ public class Binder {
: ApplicationConversionService.getSharedInstance(); : ApplicationConversionService.getSharedInstance();
this.propertyEditorInitializer = propertyEditorInitializer; this.propertyEditorInitializer = propertyEditorInitializer;
this.defaultBindHandler = (defaultBindHandler != null) ? defaultBindHandler : BindHandler.DEFAULT; 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 { ...@@ -315,7 +342,7 @@ public class Binder {
} }
private Object create(Bindable<?> target, Context context) { private Object create(Bindable<?> target, Context context) {
for (DataObjectBinder dataObjectBinder : DATA_OBJECT_BINDERS) { for (DataObjectBinder dataObjectBinder : this.dataObjectBinders) {
Object instance = dataObjectBinder.create(target, context); Object instance = dataObjectBinder.create(target, context);
if (instance != null) { if (instance != null) {
return instance; return instance;
...@@ -421,7 +448,7 @@ public class Binder { ...@@ -421,7 +448,7 @@ public class Binder {
DataObjectPropertyBinder propertyBinder = (propertyName, propertyTarget) -> bind(name.append(propertyName), DataObjectPropertyBinder propertyBinder = (propertyName, propertyTarget) -> bind(name.append(propertyName),
propertyTarget, handler, context, false, false); propertyTarget, handler, context, false, false);
return context.withDataObject(type, () -> { return context.withDataObject(type, () -> {
for (DataObjectBinder dataObjectBinder : DATA_OBJECT_BINDERS) { for (DataObjectBinder dataObjectBinder : this.dataObjectBinders) {
Object instance = dataObjectBinder.bind(name, target, context, propertyBinder); Object instance = dataObjectBinder.bind(name, target, context, propertyBinder);
if (instance != null) { if (instance != null) {
return instance; return instance;
......
/*
* 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;
}
}
...@@ -42,6 +42,8 @@ import org.springframework.core.ResolvableType; ...@@ -42,6 +42,8 @@ import org.springframework.core.ResolvableType;
*/ */
class JavaBeanBinder implements DataObjectBinder { class JavaBeanBinder implements DataObjectBinder {
static final JavaBeanBinder INSTANCE = new JavaBeanBinder();
@Override @Override
public <T> T bind(ConfigurationPropertyName name, Bindable<T> target, Context context, public <T> T bind(ConfigurationPropertyName name, Bindable<T> target, Context context,
DataObjectPropertyBinder propertyBinder) { DataObjectPropertyBinder propertyBinder) {
......
...@@ -22,7 +22,6 @@ import java.lang.reflect.Parameter; ...@@ -22,7 +22,6 @@ import java.lang.reflect.Parameter;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
import java.util.List; import java.util.List;
import java.util.function.Predicate;
import kotlin.reflect.KFunction; import kotlin.reflect.KFunction;
import kotlin.reflect.KParameter; import kotlin.reflect.KParameter;
...@@ -45,10 +44,16 @@ import org.springframework.util.Assert; ...@@ -45,10 +44,16 @@ import org.springframework.util.Assert;
*/ */
class ValueObjectBinder implements DataObjectBinder { class ValueObjectBinder implements DataObjectBinder {
private final BindConstructorProvider constructorProvider;
ValueObjectBinder(BindConstructorProvider constructorProvider) {
this.constructorProvider = constructorProvider;
}
@Override @Override
public <T> T bind(ConfigurationPropertyName name, Bindable<T> target, Binder.Context context, public <T> T bind(ConfigurationPropertyName name, Bindable<T> target, Binder.Context context,
DataObjectPropertyBinder propertyBinder) { DataObjectPropertyBinder propertyBinder) {
ValueObject<T> valueObject = ValueObject.get(target); ValueObject<T> valueObject = ValueObject.get(target, this.constructorProvider);
if (valueObject == null) { if (valueObject == null) {
return null; return null;
} }
...@@ -67,7 +72,7 @@ class ValueObjectBinder implements DataObjectBinder { ...@@ -67,7 +72,7 @@ class ValueObjectBinder implements DataObjectBinder {
@Override @Override
public <T> T create(Bindable<T> target, Binder.Context context) { public <T> T create(Bindable<T> target, Binder.Context context) {
ValueObject<T> valueObject = ValueObject.get(target); ValueObject<T> valueObject = ValueObject.get(target, this.constructorProvider);
if (valueObject == null) { if (valueObject == null) {
return null; return null;
} }
...@@ -99,18 +104,19 @@ class ValueObjectBinder implements DataObjectBinder { ...@@ -99,18 +104,19 @@ class ValueObjectBinder implements DataObjectBinder {
abstract List<ConstructorParameter> getConstructorParameters(); abstract List<ConstructorParameter> getConstructorParameters();
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
static <T> ValueObject<T> get(Bindable<T> bindable) { static <T> ValueObject<T> get(Bindable<T> bindable, BindConstructorProvider constructorProvider) {
if (bindable.getValue() != null) {
return null;
}
Class<T> type = (Class<T>) bindable.getType().resolve(); Class<T> type = (Class<T>) bindable.getType().resolve();
if (type == null || type.isEnum() || Modifier.isAbstract(type.getModifiers())) { if (type == null || type.isEnum() || Modifier.isAbstract(type.getModifiers())) {
return null; return null;
} }
Constructor<?> bindConstructor = constructorProvider.getBindConstructor(bindable);
if (bindConstructor == null) {
return null;
}
if (KotlinDetector.isKotlinType(type)) { if (KotlinDetector.isKotlinType(type)) {
return KotlinValueObject.get(type, bindable.getConstructorFilter()); return KotlinValueObject.get((Constructor<T>) bindConstructor);
} }
return DefaultValueObject.get(type, bindable.getConstructorFilter()); return DefaultValueObject.get(bindConstructor);
} }
} }
...@@ -144,17 +150,12 @@ class ValueObjectBinder implements DataObjectBinder { ...@@ -144,17 +150,12 @@ class ValueObjectBinder implements DataObjectBinder {
return this.constructorParameters; return this.constructorParameters;
} }
static <T> ValueObject<T> get(Class<T> type, Predicate<Constructor<?>> constructorFilter) { static <T> ValueObject<T> get(Constructor<T> bindConstructor) {
Constructor<T> primaryConstructor = BeanUtils.findPrimaryConstructor(type); KFunction<T> kotlinConstructor = ReflectJvmMapping.getKotlinFunction(bindConstructor);
if (primaryConstructor == null || primaryConstructor.getParameterCount() == 0
|| !constructorFilter.test(primaryConstructor)) {
return null;
}
KFunction<T> kotlinConstructor = ReflectJvmMapping.getKotlinFunction(primaryConstructor);
if (kotlinConstructor != null) { 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 { ...@@ -194,29 +195,8 @@ class ValueObjectBinder implements DataObjectBinder {
} }
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
static <T> ValueObject<T> get(Class<T> type, Predicate<Constructor<?>> constructorFilter) { static <T> ValueObject<T> get(Constructor<?> bindConstructor) {
Constructor<?> constructor = null; return new DefaultValueObject<>((Constructor<T>) bindConstructor);
for (Constructor<?> candidate : type.getDeclaredConstructors()) {
if (isCandidateConstructor(candidate, constructorFilter)) {
if (constructor != null) {
return null;
}
constructor = candidate;
}
}
return get((Constructor<T>) constructor);
}
private static boolean isCandidateConstructor(Constructor<?> candidate, Predicate<Constructor<?>> filter) {
int modifiers = candidate.getModifiers();
return !Modifier.isPrivate(modifiers) && !Modifier.isProtected(modifiers) && filter.test(candidate);
}
static <T> DefaultValueObject<T> get(Constructor<T> constructor) {
if (constructor == null || constructor.getParameterCount() == 0) {
return null;
}
return new DefaultValueObject<>(constructor);
} }
} }
......
...@@ -16,7 +16,6 @@ ...@@ -16,7 +16,6 @@
package org.springframework.boot.context.properties; package org.springframework.boot.context.properties;
import java.util.Arrays;
import java.util.Map; import java.util.Map;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
...@@ -213,8 +212,8 @@ class ConfigurationPropertiesBeanTests { ...@@ -213,8 +212,8 @@ class ConfigurationPropertiesBeanTests {
Bindable<?> target = propertiesBean.asBindTarget(); Bindable<?> target = propertiesBean.asBindTarget();
assertThat(target.getType()).isEqualTo(ResolvableType.forClass(ConstructorBindingOnConstructor.class)); assertThat(target.getType()).isEqualTo(ResolvableType.forClass(ConstructorBindingOnConstructor.class));
assertThat(target.getValue()).isNull(); assertThat(target.getValue()).isNull();
assertThat(Arrays.stream(ConstructorBindingOnConstructor.class.getDeclaredConstructors()) assertThat(ConfigurationPropertiesBindConstructorProvider.INSTANCE
.filter(target.getConstructorFilter())).hasSize(1); .getBindConstructor(ConstructorBindingOnConstructor.class)).isNotNull();
} }
@Test @Test
...@@ -230,27 +229,27 @@ class ConfigurationPropertiesBeanTests { ...@@ -230,27 +229,27 @@ class ConfigurationPropertiesBeanTests {
} }
@Test @Test
void bindTypeForClassWhenNoConstructorBindingReturnsJavaBean() { void bindTypeForTypeWhenNoConstructorBindingReturnsJavaBean() {
BindMethod bindType = BindMethod.forClass(NoConstructorBinding.class); BindMethod bindType = BindMethod.forType(NoConstructorBinding.class);
assertThat(bindType).isEqualTo(BindMethod.JAVA_BEAN); assertThat(bindType).isEqualTo(BindMethod.JAVA_BEAN);
} }
@Test @Test
void bindTypeForClassWhenNoConstructorBindingOnTypeReturnsValueObject() { void bindTypeForTypeWhenNoConstructorBindingOnTypeReturnsValueObject() {
BindMethod bindType = BindMethod.forClass(ConstructorBindingOnType.class); BindMethod bindType = BindMethod.forType(ConstructorBindingOnType.class);
assertThat(bindType).isEqualTo(BindMethod.VALUE_OBJECT); assertThat(bindType).isEqualTo(BindMethod.VALUE_OBJECT);
} }
@Test @Test
void bindTypeForClassWhenNoConstructorBindingOnConstructorReturnsValueObject() { void bindTypeForTypeWhenNoConstructorBindingOnConstructorReturnsValueObject() {
BindMethod bindType = BindMethod.forClass(ConstructorBindingOnConstructor.class); BindMethod bindType = BindMethod.forType(ConstructorBindingOnConstructor.class);
assertThat(bindType).isEqualTo(BindMethod.VALUE_OBJECT); assertThat(bindType).isEqualTo(BindMethod.VALUE_OBJECT);
} }
@Test @Test
void bindTypeForClassWhenConstructorBindingOnMultipleConstructorsThrowsException() { void bindTypeForTypeWhenConstructorBindingOnMultipleConstructorsThrowsException() {
assertThatIllegalStateException() assertThatIllegalStateException()
.isThrownBy(() -> BindMethod.forClass(ConstructorBindingOnMultipleConstructors.class)) .isThrownBy(() -> BindMethod.forType(ConstructorBindingOnMultipleConstructors.class))
.withMessage(ConstructorBindingOnMultipleConstructors.class.getName() .withMessage(ConstructorBindingOnMultipleConstructors.class.getName()
+ " has more than one @ConstructorBinding constructor"); + " has more than one @ConstructorBinding constructor");
} }
......
...@@ -849,6 +849,52 @@ class ConfigurationPropertiesTests { ...@@ -849,6 +849,52 @@ class ConfigurationPropertiesTests {
assertThat(bean.getNested().getAge()).isEqualTo(5); assertThat(bean.getNested().getAge()).isEqualTo(5);
} }
@Test
void loadWhenBindingToNestedPropertiesWithSyntheticConstructorShouldBind() {
MutablePropertySources sources = this.context.getEnvironment().getPropertySources();
Map<String, Object> 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<String, Object> 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<String, Object> 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<String, Object> 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) { private AnnotationConfigApplicationContext load(Class<?> configuration, String... inlinedProperties) {
return load(new Class<?>[] { configuration }, inlinedProperties); return load(new Class<?>[] { configuration }, inlinedProperties);
} }
...@@ -2014,6 +2060,54 @@ class ConfigurationPropertiesTests { ...@@ -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") @ConfigurationProperties("test")
static class MultiConstructorConfigurationListProperties { static class MultiConstructorConfigurationListProperties {
...@@ -2031,6 +2125,139 @@ class ConfigurationPropertiesTests { ...@@ -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) @Configuration(proxyBeanMethods = false)
@EnableConfigurationProperties(DeducedNestedConstructorProperties.class) @EnableConfigurationProperties(DeducedNestedConstructorProperties.class)
static class DeducedNestedConstructorPropertiesConfiguration { static class DeducedNestedConstructorPropertiesConfiguration {
......
...@@ -19,8 +19,6 @@ package org.springframework.boot.context.properties.bind; ...@@ -19,8 +19,6 @@ package org.springframework.boot.context.properties.bind;
import java.lang.annotation.Annotation; import java.lang.annotation.Annotation;
import java.lang.annotation.Retention; import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy; import java.lang.annotation.RetentionPolicy;
import java.lang.reflect.Constructor;
import java.util.function.Predicate;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
...@@ -176,19 +174,6 @@ class BindableTests { ...@@ -176,19 +174,6 @@ class BindableTests {
assertThat(bindable.getAnnotations()).containsExactly(annotation); assertThat(bindable.getAnnotations()).containsExactly(annotation);
} }
@Test
void withConstructorFilterSetsConstructorFilter() {
Predicate<Constructor<?>> 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) @Retention(RetentionPolicy.RUNTIME)
@interface TestAnnotation { @interface TestAnnotation {
......
...@@ -339,11 +339,12 @@ class JavaBeanBinderTests { ...@@ -339,11 +339,12 @@ class JavaBeanBinderTests {
@Test @Test
void bindToInstanceWhenNoDefaultConstructorShouldBind() { void bindToInstanceWhenNoDefaultConstructorShouldBind() {
Binder binder = new Binder(this.sources, null, null, null, null, (type) -> null);
MockConfigurationPropertySource source = new MockConfigurationPropertySource(); MockConfigurationPropertySource source = new MockConfigurationPropertySource();
source.put("foo.value", "bar"); source.put("foo.value", "bar");
this.sources.add(source); this.sources.add(source);
ExampleWithNonDefaultConstructor bean = new ExampleWithNonDefaultConstructor("faf"); ExampleWithNonDefaultConstructor bean = new ExampleWithNonDefaultConstructor("faf");
ExampleWithNonDefaultConstructor boundBean = this.binder ExampleWithNonDefaultConstructor boundBean = binder
.bind("foo", Bindable.of(ExampleWithNonDefaultConstructor.class).withExistingValue(bean)).get(); .bind("foo", Bindable.of(ExampleWithNonDefaultConstructor.class).withExistingValue(bean)).get();
assertThat(boundBean).isSameAs(bean); assertThat(boundBean).isSameAs(bean);
assertThat(bean.getValue()).isEqualTo("bar"); assertThat(bean.getValue()).isEqualTo("bar");
......
...@@ -103,8 +103,8 @@ class ValueObjectBinderTests { ...@@ -103,8 +103,8 @@ class ValueObjectBinderTests {
this.sources.add(source); this.sources.add(source);
Constructor<?>[] constructors = MultipleConstructorsBean.class.getDeclaredConstructors(); Constructor<?>[] constructors = MultipleConstructorsBean.class.getDeclaredConstructors();
Constructor<?> constructor = (constructors[0].getParameterCount() == 1) ? constructors[0] : constructors[1]; Constructor<?> constructor = (constructors[0].getParameterCount() == 1) ? constructors[0] : constructors[1];
MultipleConstructorsBean bound = this.binder.bind("foo", Bindable.of(MultipleConstructorsBean.class) Binder binder = new Binder(this.sources, null, null, null, null, (type) -> constructor);
.withConstructorFilter((candidate) -> candidate.equals(constructor))).get(); MultipleConstructorsBean bound = binder.bind("foo", Bindable.of(MultipleConstructorsBean.class)).get();
assertThat(bound.getIntValue()).isEqualTo(12); assertThat(bound.getIntValue()).isEqualTo(12);
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment