From e41c5a43273cab9dc2ce0c1c1e86ca7c633634c8 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 1 Oct 2019 17:09:02 -0700 Subject: [PATCH] Require annotation signal for constructor binding Update `@ConfigurationProperties` constructor binding support to only apply when a `@ConstructorBinding` annotation is present on either the type or the specific constructor to use. Prior to this commit we didn't have a good way to tell when constructor binding should be used vs regular autowiring. For convenience, an `@ImmutableConfigurationProperties` meta-annotation has also been added which is composed of `@ConfigurationProperties` and `@ConstructorBinding`. Closes gh-18469 --- .../main/asciidoc/spring-boot-features.adoc | 14 +- .../web/client/ExampleProperties.java | 8 +- .../properties/ConfigurationProperties.java | 7 + .../ConfigurationPropertiesBean.java | 148 ++++++++++++++--- .../ConfigurationPropertiesBeanRegistrar.java | 18 +-- .../ConfigurationPropertiesBindException.java | 23 +-- .../ConfigurationPropertiesBinder.java | 17 +- ...urationPropertiesBindingPostProcessor.java | 26 +-- .../ConfigurationPropertiesScan.java | 2 +- ...onPropertiesValueObjectBeanDefinition.java | 19 +-- .../properties/ConstructorBinding.java | 40 +++++ .../ImmutableConfigurationProperties.java | 60 +++++++ .../context/properties/bind/Bindable.java | 40 ++++- .../properties/bind/ValueObjectBinder.java | 23 ++- ...igurationPropertiesBeanRegistrarTests.java | 2 +- .../ConfigurationPropertiesBeanTests.java | 153 +++++++++++++++++- ...igurationPropertiesBindExceptionTests.java | 16 -- .../ConfigurationPropertiesTests.java | 63 +++++++- ...ConfigurationPropertiesRegistrarTests.java | 4 +- .../properties/bind/BindableTests.java | 15 ++ .../bind/ValueObjectBinderTests.java | 21 ++- ...figurationPropertiesScanConfiguration.java | 3 +- .../scan/valid/b/BScanConfiguration.java | 3 +- ...nfigurationPropertiesBeanRegistrarTests.kt | 2 +- 24 files changed, 584 insertions(+), 143 deletions(-) create mode 100644 spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConstructorBinding.java create mode 100644 spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ImmutableConfigurationProperties.java diff --git a/spring-boot-project/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc b/spring-boot-project/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc index f593f115ee..8990892f75 100644 --- a/spring-boot-project/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc +++ b/spring-boot-project/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc @@ -852,7 +852,7 @@ The example in the previous section can be rewritten in an immutable fashion as import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.boot.context.properties.DefaultValue; - @ConfigurationProperties("acme") + @ImmutableConfigurationProperties("acme") public class AcmeProperties { private final boolean enabled; @@ -881,6 +881,7 @@ The example in the previous section can be rewritten in an immutable fashion as private final List roles; + @ConstructorBinding public Security(String username, String password, @DefaultValue("USER") List roles) { this.username = username; @@ -899,11 +900,18 @@ The example in the previous section can be rewritten in an immutable fashion as } ---- -In this setup one, and only one constructor must be defined with the list of properties that you wish to bind and not other properties than the ones in the constructor are bound. +In this setup, the `@ImmutableConfigurationProperties` annotation is used to indicate that constructor binding should be used. +This means that the binder will expect to find a constructor with the parameters that you wish to have bound bind. + +Nested classes that also require constructor binding (such as `Security` in the example above) should use the `@ConstructorBinding` annotation. Default values can be specified using `@DefaultValue` and the same conversion service will be applied to coerce the `String` value to the target type of a missing property. -NOTE: To use constructor binding the class must not be annotated with `@Component` and must be enabled using `@EnableConfigurationProperties` or configuration property scanning instead. +TIP: You can also use `@ConstructorBinding` on the actual constructor that should be bound. +This is required if you have more than one constructor for your class. + +NOTE: To use constructor binding the class must be enabled using `@EnableConfigurationProperties` or configuration property scanning. +You cannot use constructor binding with beans that are created by the regular Spring mechanisms (e.g. `@Component` beans, beans created via `@Bean` methods or beans loaded using `@Import`) diff --git a/spring-boot-project/spring-boot-test-autoconfigure/src/test/java/org/springframework/boot/test/autoconfigure/web/client/ExampleProperties.java b/spring-boot-project/spring-boot-test-autoconfigure/src/test/java/org/springframework/boot/test/autoconfigure/web/client/ExampleProperties.java index 7b25bcba79..8fd908ae60 100644 --- a/spring-boot-project/spring-boot-test-autoconfigure/src/test/java/org/springframework/boot/test/autoconfigure/web/client/ExampleProperties.java +++ b/spring-boot-project/spring-boot-test-autoconfigure/src/test/java/org/springframework/boot/test/autoconfigure/web/client/ExampleProperties.java @@ -16,16 +16,16 @@ package org.springframework.boot.test.autoconfigure.web.client; -import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.ImmutableConfigurationProperties; import org.springframework.boot.context.properties.bind.DefaultValue; /** - * Example {@link ConfigurationProperties} used to test the use of configuration - * properties scan with sliced test. + * Example {@link ImmutableConfigurationProperties @ImmutableConfigurationProperties} used + * to test the use of configuration properties scan with sliced test. * * @author Stephane Nicoll */ -@ConfigurationProperties("example") +@ImmutableConfigurationProperties("example") public class ExampleProperties { private final String name; diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationProperties.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationProperties.java index d295f45ee8..0aaaff3503 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationProperties.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationProperties.java @@ -29,11 +29,18 @@ import org.springframework.core.annotation.AliasFor; * {@code @Bean} method in a {@code @Configuration} class if you want to bind and validate * some external Properties (e.g. from a .properties file). *

+ * Binding can is either performed by calling setters on the annotated class or, if + * {@link ConstructorBinding @ConstructorBinding} is in use, by binding to the constructor + * parameters. + *

* Note that contrary to {@code @Value}, SpEL expressions are not evaluated since property * values are externalized. * * @author Dave Syer * @since 1.0.0 + * @see ConfigurationPropertiesScan + * @see ConstructorBinding + * @see ImmutableConfigurationProperties * @see ConfigurationPropertiesBindingPostProcessor * @see EnableConfigurationProperties */ 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 cf0cca0570..fdc2c6076c 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 @@ -17,12 +17,14 @@ package org.springframework.boot.context.properties; import java.lang.annotation.Annotation; +import java.lang.reflect.Constructor; import java.lang.reflect.Method; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.Map; 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.ConfigurableListableBeanFactory; @@ -32,8 +34,11 @@ 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.AnnotationUtils; +import org.springframework.core.annotation.MergedAnnotation; +import org.springframework.core.annotation.MergedAnnotations; +import org.springframework.core.annotation.MergedAnnotations.SearchStrategy; import org.springframework.util.Assert; import org.springframework.validation.annotation.Validated; @@ -56,12 +61,15 @@ public final class ConfigurationPropertiesBean { private final Bindable bindTarget; + private final BindMethod bindMethod; + private ConfigurationPropertiesBean(String name, Object instance, ConfigurationProperties annotation, - Bindable bindTarget) { + Bindable bindTarget, BindMethod bindMethod) { this.name = name; this.instance = instance; this.annotation = annotation; this.bindTarget = bindTarget; + this.bindMethod = bindMethod; } /** @@ -80,6 +88,22 @@ public final class ConfigurationPropertiesBean { return this.instance; } + /** + * Return the bean type. + * @return the bean type + */ + Class getType() { + return this.bindTarget.getType().resolve(); + } + + /** + * Return the property binding method that was used for the bean. + * @return the bind type + */ + public BindMethod getBindMethod() { + return this.bindMethod; + } + /** * Return the {@link ConfigurationProperties} annotation for the bean. The annotation * may be defined on the bean itself or from the factory method that create the bean @@ -152,18 +176,7 @@ public final class ConfigurationPropertiesBean { */ public static ConfigurationPropertiesBean get(ApplicationContext applicationContext, Object bean, String beanName) { Method factoryMethod = findFactoryMethod(applicationContext, beanName); - ConfigurationProperties annotation = getAnnotation(applicationContext, bean, beanName, factoryMethod, - ConfigurationProperties.class); - if (annotation == null) { - return null; - } - ResolvableType type = (factoryMethod != null) ? ResolvableType.forMethodReturnType(factoryMethod) - : ResolvableType.forClass(bean.getClass()); - Validated validated = getAnnotation(applicationContext, bean, beanName, factoryMethod, Validated.class); - Annotation[] annotations = (validated != null) ? new Annotation[] { annotation, validated } - : new Annotation[] { annotation }; - Bindable bindTarget = Bindable.of(type).withAnnotations(annotations).withExistingValue(bean); - return new ConfigurationPropertiesBean(beanName, bean, annotation, bindTarget); + return create(beanName, bean, bean.getClass(), factoryMethod); } private static Method findFactoryMethod(ApplicationContext applicationContext, String beanName) { @@ -184,22 +197,105 @@ public final class ConfigurationPropertiesBean { return null; } - private static A getAnnotation(ApplicationContext applicationContext, Object bean, - String beanName, Method factoryMethod, Class annotationType) { - if (factoryMethod != null) { - A annotation = AnnotationUtils.findAnnotation(factoryMethod, annotationType); - if (annotation != null) { - return annotation; + static ConfigurationPropertiesBean forValueObject(Class beanClass, String beanName) { + ConfigurationPropertiesBean propertiesBean = create(beanName, null, beanClass, null); + Assert.state(propertiesBean != null && propertiesBean.getBindMethod() == BindMethod.VALUE_OBJECT, + "Bean '" + beanName + "' is not a @ConfigurationProperties value object"); + return propertiesBean; + } + + private static ConfigurationPropertiesBean create(String name, Object instance, Class type, Method factory) { + ConfigurationProperties annotation = findAnnotation(instance, type, factory, ConfigurationProperties.class); + if (annotation == null) { + return null; + } + 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); + if (instance != null) { + bindTarget = bindTarget.withExistingValue(instance); + } + return new ConfigurationPropertiesBean(name, instance, annotation, bindTarget, bindMethod); + } + + private static A findAnnotation(Object instance, Class type, Method factory, + Class annotationType) { + MergedAnnotation annotation = MergedAnnotation.missing(); + if (factory != null) { + annotation = MergedAnnotations.from(factory, SearchStrategy.TYPE_HIERARCHY).get(annotationType); + } + if (!annotation.isPresent()) { + annotation = MergedAnnotations.from(type, SearchStrategy.TYPE_HIERARCHY).get(annotationType); + } + if (!annotation.isPresent() && AopUtils.isAopProxy(instance)) { + annotation = MergedAnnotations.from(AopUtils.getTargetClass(instance), SearchStrategy.TYPE_HIERARCHY) + .get(annotationType); + } + return annotation.isPresent() ? annotation.synthesize() : null; + } + + 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 use used for the bean. + */ + public enum BindMethod { + + /** + * Java Bean using getter/setter binding. + */ + JAVA_BEAN, + + /** + * Value object using constructor binding. + */ + VALUE_OBJECT; + + static BindMethod forClass(Class type) { + if (MergedAnnotations.from(type, SearchStrategy.TYPE_HIERARCHY).isPresent(ConstructorBinding.class) + || findBindConstructor(type) != null) { + return VALUE_OBJECT; } + return JAVA_BEAN; } - A annotation = AnnotationUtils.findAnnotation(bean.getClass(), annotationType); - if (annotation != null) { - return annotation; + + 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()); } - if (AopUtils.isAopProxy(bean)) { - return AnnotationUtils.findAnnotation(AopUtils.getTargetClass(bean), annotationType); + + 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; } - return null; + } } 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 f85c9dea7c..657fb3deab 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 @@ -15,16 +15,13 @@ */ package org.springframework.boot.context.properties; -import java.lang.reflect.Constructor; - -import org.springframework.beans.BeanUtils; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.HierarchicalBeanFactory; import org.springframework.beans.factory.ListableBeanFactory; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.beans.factory.support.GenericBeanDefinition; -import org.springframework.core.KotlinDetector; +import org.springframework.boot.context.properties.ConfigurationPropertiesBean.BindMethod; import org.springframework.core.annotation.MergedAnnotation; import org.springframework.core.annotation.MergedAnnotations; import org.springframework.core.annotation.MergedAnnotations.SearchStrategy; @@ -91,7 +88,7 @@ final class ConfigurationPropertiesBeanRegistrar { } private BeanDefinition createBeanDefinition(String beanName, Class type) { - if (isValueObject(type)) { + if (BindMethod.forClass(type) == BindMethod.VALUE_OBJECT) { return new ConfigurationPropertiesValueObjectBeanDefinition(this.beanFactory, beanName, type); } GenericBeanDefinition definition = new GenericBeanDefinition(); @@ -99,15 +96,4 @@ final class ConfigurationPropertiesBeanRegistrar { return definition; } - private boolean isValueObject(Class type) { - if (KotlinDetector.isKotlinPresent() && KotlinDetector.isKotlinType(type)) { - Constructor primaryConstructor = BeanUtils.findPrimaryConstructor(type); - if (primaryConstructor != null) { - return primaryConstructor.getParameterCount() > 0; - } - } - Constructor[] constructors = type.getDeclaredConstructors(); - return constructors.length == 1 && constructors[0].getParameterCount() > 0; - } - } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindException.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindException.java index 90eac42cc8..9a8b072a56 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindException.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindException.java @@ -29,19 +29,11 @@ import org.springframework.util.ClassUtils; */ public class ConfigurationPropertiesBindException extends BeanCreationException { - private final Class beanType; - - private final ConfigurationProperties annotation; + private final ConfigurationPropertiesBean bean; ConfigurationPropertiesBindException(ConfigurationPropertiesBean bean, Exception cause) { - this(bean.getName(), bean.getInstance().getClass(), bean.getAnnotation(), cause); - } - - ConfigurationPropertiesBindException(String beanName, Class beanType, ConfigurationProperties annotation, - Exception cause) { - super(beanName, getMessage(beanType, annotation), cause); - this.beanType = beanType; - this.annotation = annotation; + super(bean.getName(), getMessage(bean), cause); + this.bean = bean; } /** @@ -49,7 +41,7 @@ public class ConfigurationPropertiesBindException extends BeanCreationException * @return the bean type */ public Class getBeanType() { - return this.beanType; + return this.bean.getType(); } /** @@ -57,13 +49,14 @@ public class ConfigurationPropertiesBindException extends BeanCreationException * @return the configuration properties annotation */ public ConfigurationProperties getAnnotation() { - return this.annotation; + return this.bean.getAnnotation(); } - private static String getMessage(Class beanType, ConfigurationProperties annotation) { + private static String getMessage(ConfigurationPropertiesBean bean) { + ConfigurationProperties annotation = bean.getAnnotation(); StringBuilder message = new StringBuilder(); message.append("Could not bind properties to '"); - message.append(ClassUtils.getShortName(beanType)).append("' : "); + message.append(ClassUtils.getShortName(bean.getType())).append("' : "); message.append("prefix=").append(annotation.prefix()); message.append(", ignoreInvalidFields=").append(annotation.ignoreInvalidFields()); message.append(", ignoreUnknownFields=").append(annotation.ignoreUnknownFields()); 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 fb5a1c7348..97fae2230e 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 @@ -45,7 +45,6 @@ import org.springframework.context.ApplicationContextAware; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.core.convert.ConversionService; import org.springframework.core.env.PropertySources; -import org.springframework.util.Assert; import org.springframework.validation.Validator; import org.springframework.validation.annotation.Validated; @@ -83,24 +82,20 @@ class ConfigurationPropertiesBinder { this.jsr303Present = ConfigurationPropertiesJsr303Validator.isJsr303Present(applicationContext); } - BindResult bind(Bindable target) { - ConfigurationProperties annotation = getAnnotation(target); + BindResult bind(ConfigurationPropertiesBean propertiesBean) { + Bindable target = propertiesBean.asBindTarget(); + ConfigurationProperties annotation = propertiesBean.getAnnotation(); BindHandler bindHandler = getBindHandler(target, annotation); return getBinder().bind(annotation.prefix(), target, bindHandler); } - T bindOrCreate(Bindable target) { - ConfigurationProperties annotation = getAnnotation(target); + Object bindOrCreate(ConfigurationPropertiesBean propertiesBean) { + Bindable target = propertiesBean.asBindTarget(); + ConfigurationProperties annotation = propertiesBean.getAnnotation(); BindHandler bindHandler = getBindHandler(target, annotation); return getBinder().bindOrCreate(annotation.prefix(), target, bindHandler); } - private ConfigurationProperties getAnnotation(Bindable target) { - ConfigurationProperties annotation = target.getAnnotation(ConfigurationProperties.class); - Assert.state(annotation != null, () -> "Missing @ConfigurationProperties on " + target); - return annotation; - } - private Validator getConfigurationPropertiesValidator(ApplicationContext applicationContext) { if (applicationContext.containsBean(VALIDATOR_BEAN_NAME)) { return applicationContext.getBean(VALIDATOR_BEAN_NAME, Validator.class); diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindingPostProcessor.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindingPostProcessor.java index 01a4ca37b9..102c677560 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindingPostProcessor.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindingPostProcessor.java @@ -22,6 +22,7 @@ import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.BeanPostProcessor; import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.beans.factory.support.GenericBeanDefinition; +import org.springframework.boot.context.properties.ConfigurationPropertiesBean.BindMethod; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; import org.springframework.core.Ordered; @@ -92,19 +93,24 @@ public class ConfigurationPropertiesBindingPostProcessor @Override public Object postProcessBeforeInitialization(Object bean, String beanName) throws BeansException { - ConfigurationPropertiesBean configurationPropertiesBean = ConfigurationPropertiesBean - .get(this.applicationContext, bean, beanName); - if (configurationPropertiesBean != null && !hasBoundValueObject(beanName)) { - try { - this.binder.bind(configurationPropertiesBean.asBindTarget()); - } - catch (Exception ex) { - throw new ConfigurationPropertiesBindException(configurationPropertiesBean, ex); - } - } + bind(ConfigurationPropertiesBean.get(this.applicationContext, bean, beanName)); return bean; } + private void bind(ConfigurationPropertiesBean bean) { + if (bean == null || hasBoundValueObject(bean.getName())) { + return; + } + Assert.state(bean.getBindMethod() == BindMethod.JAVA_BEAN, "Cannot bind @ConfigurationProperties for bean '" + + bean.getName() + "'. Ensure that @ConstructorBinding has not been applied to regular bean"); + try { + this.binder.bind(bean); + } + catch (Exception ex) { + throw new ConfigurationPropertiesBindException(bean, ex); + } + } + private boolean hasBoundValueObject(String beanName) { return this.registry.containsBeanDefinition(beanName) && this.registry .getBeanDefinition(beanName) instanceof ConfigurationPropertiesValueObjectBeanDefinition; diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesScan.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesScan.java index 41fe0bbd9c..c2c801ba1e 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesScan.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesScan.java @@ -41,7 +41,7 @@ import org.springframework.stereotype.Component; @Target(ElementType.TYPE) @Retention(RetentionPolicy.RUNTIME) @Documented -@Import({ ConfigurationPropertiesScanRegistrar.class }) +@Import(ConfigurationPropertiesScanRegistrar.class) @EnableConfigurationProperties public @interface ConfigurationPropertiesScan { diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesValueObjectBeanDefinition.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesValueObjectBeanDefinition.java index e0374fb670..08af79b96d 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesValueObjectBeanDefinition.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesValueObjectBeanDefinition.java @@ -16,15 +16,9 @@ package org.springframework.boot.context.properties; -import java.lang.annotation.Annotation; - import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.support.GenericBeanDefinition; -import org.springframework.boot.context.properties.bind.Bindable; -import org.springframework.core.ResolvableType; -import org.springframework.core.annotation.AnnotationUtils; -import org.springframework.validation.annotation.Validated; /** * {@link BeanDefinition} that is used for registering @@ -33,6 +27,7 @@ import org.springframework.validation.annotation.Validated; * * @author Stephane Nicoll * @author Madhura Bhave + * @author Phillip Webb */ final class ConfigurationPropertiesValueObjectBeanDefinition extends GenericBeanDefinition { @@ -48,19 +43,13 @@ final class ConfigurationPropertiesValueObjectBeanDefinition extends GenericBean } private Object createBean() { + ConfigurationPropertiesBean bean = ConfigurationPropertiesBean.forValueObject(getBeanClass(), this.beanName); ConfigurationPropertiesBinder binder = ConfigurationPropertiesBinder.get(this.beanFactory); - ResolvableType type = ResolvableType.forClass(getBeanClass()); - ConfigurationProperties annotation = AnnotationUtils.findAnnotation(getBeanClass(), - ConfigurationProperties.class); - Validated validated = AnnotationUtils.findAnnotation(getBeanClass(), Validated.class); - Annotation[] annotations = (validated != null) ? new Annotation[] { annotation, validated } - : new Annotation[] { annotation }; - Bindable bindTarget = Bindable.of(type).withAnnotations(annotations); try { - return binder.bindOrCreate(bindTarget); + return binder.bindOrCreate(bean); } catch (Exception ex) { - throw new ConfigurationPropertiesBindException(this.beanName, getBeanClass(), annotation, ex); + throw new ConfigurationPropertiesBindException(bean, ex); } } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConstructorBinding.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConstructorBinding.java new file mode 100644 index 0000000000..54dac121a4 --- /dev/null +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConstructorBinding.java @@ -0,0 +1,40 @@ +/* + * 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.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Annotation that can be used to indicate that configuration properties should be bound + * using constructor arguments rather than by calling setters. Can be added at the type + * level (if there is an unambiguous constructor) or on the actual constructor to use. + * + * @author Phillip Webb + * @since 2.2.0 + * @see ConfigurationProperties + * @see ImmutableConfigurationProperties + */ +@Target({ ElementType.TYPE, ElementType.CONSTRUCTOR }) +@Retention(RetentionPolicy.RUNTIME) +@Documented +public @interface ConstructorBinding { + +} diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ImmutableConfigurationProperties.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ImmutableConfigurationProperties.java new file mode 100644 index 0000000000..6d37e908bd --- /dev/null +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ImmutableConfigurationProperties.java @@ -0,0 +1,60 @@ +/* + * 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.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import org.springframework.core.annotation.AliasFor; + +/** + * A convenience annotation that can be used for immutable + * {@link ConfigurationProperties @ConfigurationProperties} that use + * {@link ConstructorBinding @ConstructorBinding}. + * + * @author Phillip Webb + * @since 2.2.0 + */ +@Target(ElementType.TYPE) +@Retention(RetentionPolicy.RUNTIME) +@Documented +@ConfigurationProperties +@ConstructorBinding +public @interface ImmutableConfigurationProperties { + + /** + * The name prefix of the properties that are valid to bind to this object. Synonym + * for {@link #prefix()}. A valid prefix is defined by one or more words separated + * with dots (e.g. {@code "acme.system.feature"}). + * @return the name prefix of the properties to bind + */ + @AliasFor(annotation = ConfigurationProperties.class) + String value() default ""; + + /** + * The name prefix of the properties that are valid to bind to this object. Synonym + * for {@link #value()}. A valid prefix is defined by one or more words separated with + * dots (e.g. {@code "acme.system.feature"}). + * @return the name prefix of the properties to bind + */ + @AliasFor(annotation = ConfigurationProperties.class) + String prefix() default ""; + +} 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 a7f2bdbaaf..16f3f6e602 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,9 +18,11 @@ 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; @@ -42,6 +44,8 @@ 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; @@ -50,11 +54,15 @@ public final class Bindable { private final Annotation[] annotations; - private Bindable(ResolvableType type, ResolvableType boxedType, Supplier value, Annotation[] annotations) { + private final Predicate> constructorFilter; + + private Bindable(ResolvableType type, ResolvableType boxedType, Supplier value, Annotation[] annotations, + Predicate> constructorFilter) { this.type = type; this.boxedType = boxedType; this.value = value; this.annotations = annotations; + this.constructorFilter = constructorFilter; } /** @@ -105,6 +113,16 @@ public final class Bindable { return null; } + /** + * Return the constructor filter that can be used to limit the constructor 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) { @@ -149,7 +167,7 @@ public final class Bindable { */ public Bindable withAnnotations(Annotation... annotations) { return new Bindable<>(this.type, this.boxedType, this.value, - (annotations != null) ? annotations : NO_ANNOTATIONS); + (annotations != null) ? annotations : NO_ANNOTATIONS, this.constructorFilter); } /** @@ -162,7 +180,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); + return new Bindable<>(this.type, this.boxedType, value, this.annotations, this.constructorFilter); } /** @@ -171,7 +189,19 @@ public final class Bindable { * @return an updated {@link Bindable} */ public Bindable withSuppliedValue(Supplier suppliedValue) { - return new Bindable<>(this.type, this.boxedType, suppliedValue, this.annotations); + 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); } /** @@ -244,7 +274,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); + return new Bindable<>(type, boxedType, null, NO_ANNOTATIONS, ANY_CONSTRUCTOR); } private static ResolvableType box(ResolvableType type) { 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 c46589dd9f..a849cf301f 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,6 +22,7 @@ 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; @@ -106,9 +107,9 @@ class ValueObjectBinder implements DataObjectBinder { return null; } if (KotlinDetector.isKotlinType(type)) { - return KotlinValueObject.get(type); + return KotlinValueObject.get(type, bindable.getConstructorFilter()); } - return DefaultValueObject.get(type); + return DefaultValueObject.get(type, bindable.getConstructorFilter()); } } @@ -142,9 +143,10 @@ class ValueObjectBinder implements DataObjectBinder { return this.constructorParameters; } - static ValueObject get(Class type) { + static ValueObject get(Class type, Predicate> constructorFilter) { Constructor primaryConstructor = BeanUtils.findPrimaryConstructor(type); - if (primaryConstructor == null || primaryConstructor.getParameterCount() == 0) { + if (primaryConstructor == null || primaryConstructor.getParameterCount() == 0 + || !constructorFilter.test(primaryConstructor)) { return null; } KFunction kotlinConstructor = ReflectJvmMapping.getKotlinFunction(primaryConstructor); @@ -191,18 +193,23 @@ class ValueObjectBinder implements DataObjectBinder { } @SuppressWarnings("unchecked") - static ValueObject get(Class type) { + static ValueObject get(Class type, Predicate> constructorFilter) { Constructor constructor = null; for (Constructor candidate : type.getDeclaredConstructors()) { - int modifiers = candidate.getModifiers(); - if (!Modifier.isPrivate(modifiers) && !Modifier.isProtected(modifiers)) { + if (isCandidateConstructor(candidate, constructorFilter)) { if (constructor != null) { return null; } constructor = candidate; } } - return get((Constructor) constructor); + return (constructor != null) ? new DefaultValueObject<>((Constructor) constructor) : null; + } + + private static boolean isCandidateConstructor(Constructor candidate, Predicate> filter) { + int modifiers = candidate.getModifiers(); + return !Modifier.isPrivate(modifiers) && !Modifier.isProtected(modifiers) + && candidate.getParameterCount() > 0 && filter.test(candidate); } static DefaultValueObject get(Constructor constructor) { diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanRegistrarTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanRegistrarTests.java index 53e1d6e947..5e34617966 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanRegistrarTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanRegistrarTests.java @@ -90,7 +90,7 @@ class ConfigurationPropertiesBeanRegistrarTests { } - @ConfigurationProperties(prefix = "valuecp") + @ImmutableConfigurationProperties("valuecp") static class ValueObjectConfigurationProperties { ValueObjectConfigurationProperties(String name) { 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 dfea006eac..b1dd46968d 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,11 +16,14 @@ package org.springframework.boot.context.properties; +import java.util.Arrays; import java.util.Map; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.function.ThrowingConsumer; +import org.springframework.boot.context.properties.ConfigurationPropertiesBean.BindMethod; +import org.springframework.boot.context.properties.bind.Bindable; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -29,6 +32,7 @@ import org.springframework.stereotype.Component; import org.springframework.validation.annotation.Validated; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalStateException; /** * Tests for {@link ConfigurationPropertiesBean}. @@ -40,17 +44,28 @@ class ConfigurationPropertiesBeanTests { @Test void getAllReturnsAll() { try (AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( - NonAnnotatedComponent.class, AnnotatedComponent.class, AnnotatedBeanConfiguration.class)) { + NonAnnotatedComponent.class, AnnotatedComponent.class, AnnotatedBeanConfiguration.class, + ValueObjectConfiguration.class)) { Map all = ConfigurationPropertiesBean.getAll(context); - assertThat(all).containsOnlyKeys("annotatedComponent", "annotatedBean"); + assertThat(all).containsOnlyKeys("annotatedComponent", "annotatedBean", ValueObject.class.getName()); ConfigurationPropertiesBean component = all.get("annotatedComponent"); assertThat(component.getName()).isEqualTo("annotatedComponent"); assertThat(component.getInstance()).isInstanceOf(AnnotatedComponent.class); assertThat(component.getAnnotation()).isNotNull(); + assertThat(component.getType()).isEqualTo(AnnotatedComponent.class); + assertThat(component.getBindMethod()).isEqualTo(BindMethod.JAVA_BEAN); ConfigurationPropertiesBean bean = all.get("annotatedBean"); assertThat(bean.getName()).isEqualTo("annotatedBean"); assertThat(bean.getInstance()).isInstanceOf(AnnotatedBean.class); + assertThat(bean.getType()).isEqualTo(AnnotatedBean.class); assertThat(bean.getAnnotation()).isNotNull(); + assertThat(bean.getBindMethod()).isEqualTo(BindMethod.JAVA_BEAN); + ConfigurationPropertiesBean valueObject = all.get(ValueObject.class.getName()); + assertThat(valueObject.getName()).isEqualTo(ValueObject.class.getName()); + assertThat(valueObject.getInstance()).isInstanceOf(ValueObject.class); + assertThat(valueObject.getType()).isEqualTo(ValueObject.class); + assertThat(valueObject.getAnnotation()).isNotNull(); + assertThat(valueObject.getBindMethod()).isEqualTo(BindMethod.VALUE_OBJECT); } } @@ -66,7 +81,9 @@ class ConfigurationPropertiesBeanTests { assertThat(propertiesBean).isNotNull(); assertThat(propertiesBean.getName()).isEqualTo("annotatedComponent"); assertThat(propertiesBean.getInstance()).isInstanceOf(AnnotatedComponent.class); + assertThat(propertiesBean.getType()).isEqualTo(AnnotatedComponent.class); assertThat(propertiesBean.getAnnotation().prefix()).isEqualTo("prefix"); + assertThat(propertiesBean.getBindMethod()).isEqualTo(BindMethod.JAVA_BEAN); }); } @@ -76,13 +93,17 @@ class ConfigurationPropertiesBeanTests { assertThat(propertiesBean).isNotNull(); assertThat(propertiesBean.getName()).isEqualTo("nonAnnotatedBean"); assertThat(propertiesBean.getInstance()).isInstanceOf(NonAnnotatedBean.class); + assertThat(propertiesBean.getType()).isEqualTo(NonAnnotatedBean.class); assertThat(propertiesBean.getAnnotation().prefix()).isEqualTo("prefix"); + assertThat(propertiesBean.getBindMethod()).isEqualTo(BindMethod.JAVA_BEAN); }); } @Test void getWhenHasFactoryMethodBindsUsingMethodReturnType() throws Throwable { get(NonAnnotatedGenericBeanConfiguration.class, "nonAnnotatedGenericBean", (propertiesBean) -> { + assertThat(propertiesBean.getType()).isEqualTo(NonAnnotatedGenericBean.class); + assertThat(propertiesBean.getBindMethod()).isEqualTo(BindMethod.JAVA_BEAN); ResolvableType type = propertiesBean.asBindTarget().getType(); assertThat(type.resolve()).isEqualTo(NonAnnotatedGenericBean.class); assertThat(type.resolveGeneric(0)).isEqualTo(String.class); @@ -92,6 +113,8 @@ class ConfigurationPropertiesBeanTests { @Test void getWhenHasFactoryMethodWithoutAnnotationBindsUsingMethodType() throws Throwable { get(AnnotatedGenericBeanConfiguration.class, "annotatedGenericBean", (propertiesBean) -> { + assertThat(propertiesBean.getType()).isEqualTo(AnnotatedGenericBean.class); + assertThat(propertiesBean.getBindMethod()).isEqualTo(BindMethod.JAVA_BEAN); ResolvableType type = propertiesBean.asBindTarget().getType(); assertThat(type.resolve()).isEqualTo(AnnotatedGenericBean.class); assertThat(type.resolveGeneric(0)).isEqualTo(String.class); @@ -101,6 +124,8 @@ class ConfigurationPropertiesBeanTests { @Test void getWhenHasNoFactoryMethodBindsUsingObjectType() throws Throwable { get(AnnotatedGenericComponent.class, "annotatedGenericComponent", (propertiesBean) -> { + assertThat(propertiesBean.getType()).isEqualTo(AnnotatedGenericComponent.class); + assertThat(propertiesBean.getBindMethod()).isEqualTo(BindMethod.JAVA_BEAN); ResolvableType type = propertiesBean.asBindTarget().getType(); assertThat(type.resolve()).isEqualTo(AnnotatedGenericComponent.class); assertThat(type.getGeneric(0).resolve()).isNull(); @@ -137,6 +162,66 @@ class ConfigurationPropertiesBeanTests { }); } + @Test + void forValueObjectReturnsBean() { + ConfigurationPropertiesBean propertiesBean = ConfigurationPropertiesBean + .forValueObject(ConstructorBindingOnConstructor.class, "valueObjectBean"); + assertThat(propertiesBean.getName()).isEqualTo("valueObjectBean"); + assertThat(propertiesBean.getInstance()).isNull(); + assertThat(propertiesBean.getType()).isEqualTo(ConstructorBindingOnConstructor.class); + assertThat(propertiesBean.getBindMethod()).isEqualTo(BindMethod.VALUE_OBJECT); + assertThat(propertiesBean.getAnnotation()).isNotNull(); + 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); + } + + @Test + void forValueObjectWhenJavaBeanBindTypeThrowsException() { + assertThatIllegalStateException() + .isThrownBy(() -> ConfigurationPropertiesBean.forValueObject(AnnotatedBean.class, "annotatedBean")) + .withMessage("Bean 'annotatedBean' is not a @ConfigurationProperties value object"); + assertThatIllegalStateException() + .isThrownBy( + () -> ConfigurationPropertiesBean.forValueObject(NonAnnotatedBean.class, "nonAnnotatedBean")) + .withMessage("Bean 'nonAnnotatedBean' is not a @ConfigurationProperties value object"); + + } + + @Test + void bindTypeForClassWhenNoConstructorBindingReturnsJavaBean() { + BindMethod bindType = BindMethod.forClass(NoConstructorBinding.class); + assertThat(bindType).isEqualTo(BindMethod.JAVA_BEAN); + } + + @Test + void bindTypeForClassWhenNoConstructorBindingOnTypeReturnsValueObject() { + BindMethod bindType = BindMethod.forClass(ConstructorBindingOnType.class); + assertThat(bindType).isEqualTo(BindMethod.VALUE_OBJECT); + } + + @Test + void bindTypeForClassWhenNoMetaConstructorBindingOnTypeReturnsValueObject() { + BindMethod bindType = BindMethod.forClass(MetaConstructorBindingOnType.class); + assertThat(bindType).isEqualTo(BindMethod.VALUE_OBJECT); + } + + @Test + void bindTypeForClassWhenNoConstructorBindingOnConstructorReturnsValueObject() { + BindMethod bindType = BindMethod.forClass(ConstructorBindingOnConstructor.class); + assertThat(bindType).isEqualTo(BindMethod.VALUE_OBJECT); + } + + @Test + void bindTypeForClassWhenConstructorBindingOnMultipleConstructorsThrowsException() { + assertThatIllegalStateException() + .isThrownBy(() -> BindMethod.forClass(ConstructorBindingOnMultipleConstructors.class)) + .withMessage(ConstructorBindingOnMultipleConstructors.class.getName() + + " has more than one @ConstructorBinding constructor"); + } + private void get(Class configuration, String beanName, ThrowingConsumer consumer) throws Throwable { try (AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(configuration)) { @@ -261,6 +346,21 @@ class ConfigurationPropertiesBeanTests { } + @Configuration(proxyBeanMethods = false) + @EnableConfigurationProperties(ValueObject.class) + static class ValueObjectConfiguration { + + } + + @ConfigurationProperties + @ConstructorBinding + static class ValueObject { + + ValueObject(String name) { + } + + } + static class BeanGroup { } @@ -269,4 +369,53 @@ class ConfigurationPropertiesBeanTests { } + @ConfigurationProperties + static class NoConstructorBinding { + + } + + @ConfigurationProperties + @ConstructorBinding + static class ConstructorBindingOnType { + + ConstructorBindingOnType(String name) { + } + + } + + @ImmutableConfigurationProperties + static class MetaConstructorBindingOnType { + + MetaConstructorBindingOnType(String name) { + } + + } + + @ConfigurationProperties + static class ConstructorBindingOnConstructor { + + ConstructorBindingOnConstructor(String name) { + this(name, -1); + } + + @ConstructorBinding + ConstructorBindingOnConstructor(String name, int age) { + } + + } + + @ConfigurationProperties + static class ConstructorBindingOnMultipleConstructors { + + @ConstructorBinding + ConstructorBindingOnMultipleConstructors(String name) { + this(name, -1); + } + + @ConstructorBinding + ConstructorBindingOnMultipleConstructors(String name, int age) { + } + + } + } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindExceptionTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindExceptionTests.java index 7a4be60c6d..23290e122f 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindExceptionTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindExceptionTests.java @@ -48,22 +48,6 @@ class ConfigurationPropertiesBindExceptionTests { assertThat(exception.getCause()).isInstanceOf(IllegalStateException.class); } - @Test - void createFromItemsHasDetails() { - Example example = new Example(); - ConfigurationProperties annotation = example.getClass().getDeclaredAnnotation(ConfigurationProperties.class); - ConfigurationPropertiesBindException exception = new ConfigurationPropertiesBindException("example", - Example.class, annotation, new IllegalStateException()); - assertThat(exception.getMessage()).isEqualTo("Error creating bean with name 'example': " - + "Could not bind properties to 'ConfigurationPropertiesBindExceptionTests.Example' : " - + "prefix=, ignoreInvalidFields=false, ignoreUnknownFields=true; " - + "nested exception is java.lang.IllegalStateException"); - assertThat(exception.getBeanType()).isEqualTo(Example.class); - assertThat(exception.getBeanName()).isEqualTo("example"); - assertThat(exception.getAnnotation()).isInstanceOf(ConfigurationProperties.class); - assertThat(exception.getCause()).isInstanceOf(IllegalStateException.class); - } - @Component("example") @ConfigurationProperties static class Example { 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 5bfcca6909..4ba63cdf17 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 @@ -776,7 +776,6 @@ class ConfigurationPropertiesTests { } @Test - @SuppressWarnings("deprecation") void loadWhenBindingOnBeanWithoutBeanDefinitionShouldBind() { load(BasicConfiguration.class, "name=test"); BasicProperties bean = this.context.getBean(BasicProperties.class); @@ -787,6 +786,19 @@ class ConfigurationPropertiesTests { assertThat(bean.name).isEqualTo("test"); } + @Test + void loadWhenBindingToNestedConstructorPropertiesShouldBind() { + MutablePropertySources sources = this.context.getEnvironment().getPropertySources(); + Map source = new HashMap<>(); + source.put("test.name", "spring"); + source.put("test.nested.age", "5"); + sources.addLast(new MapPropertySource("test", source)); + load(NestedConstructorPropertiesConfiguration.class); + NestedConstructorProperties bean = this.context.getBean(NestedConstructorProperties.class); + assertThat(bean.getName()).isEqualTo("spring"); + assertThat(bean.getNested().getAge()).isEqualTo(5); + } + private AnnotationConfigApplicationContext load(Class configuration, String... inlinedProperties) { return load(new Class[] { configuration }, inlinedProperties); } @@ -1744,7 +1756,7 @@ class ConfigurationPropertiesTests { } - @ConfigurationProperties(prefix = "test") + @ImmutableConfigurationProperties(prefix = "test") static class OtherInjectedProperties { final DataSizeProperties dataSizeProperties; @@ -1761,7 +1773,7 @@ class ConfigurationPropertiesTests { } - @ConfigurationProperties(prefix = "test") + @ImmutableConfigurationProperties(prefix = "test") @Validated static class ConstructorParameterProperties { @@ -1785,7 +1797,7 @@ class ConfigurationPropertiesTests { } - @ConfigurationProperties(prefix = "test") + @ImmutableConfigurationProperties(prefix = "test") @Validated static class ConstructorParameterValidatedProperties { @@ -1898,4 +1910,47 @@ class ConfigurationPropertiesTests { } + @Configuration(proxyBeanMethods = false) + @EnableConfigurationProperties(NestedConstructorProperties.class) + static class NestedConstructorPropertiesConfiguration { + + } + + @ImmutableConfigurationProperties("test") + static class NestedConstructorProperties { + + private final String name; + + private final Nested nested; + + NestedConstructorProperties(String name, Nested nested) { + this.name = name; + this.nested = nested; + } + + String getName() { + return this.name; + } + + Nested getNested() { + return this.nested; + } + + static class Nested { + + private final int age; + + @ConstructorBinding + Nested(int age) { + this.age = age; + } + + int getAge() { + return this.age; + } + + } + + } + } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/EnableConfigurationPropertiesRegistrarTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/EnableConfigurationPropertiesRegistrarTests.java index 931b1c0e5d..4cc95a8888 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/EnableConfigurationPropertiesRegistrarTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/EnableConfigurationPropertiesRegistrarTests.java @@ -60,7 +60,7 @@ class EnableConfigurationPropertiesRegistrarTests { } @Test - void typeWithOneConstructorWithParametersShouldRegisterConfigurationPropertiesBeanDefinition() throws Exception { + void typeWithConstructorBindingShouldRegisterConfigurationPropertiesBeanDefinition() throws Exception { register(TestConfiguration.class); BeanDefinition beanDefinition = this.beanFactory .getBeanDefinition("bar-" + getClass().getName() + "$BarProperties"); @@ -128,7 +128,7 @@ class EnableConfigurationPropertiesRegistrarTests { } - @ConfigurationProperties(prefix = "bar") + @ImmutableConfigurationProperties(prefix = "bar") static class BarProperties { BarProperties(String foo) { 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 6883dc1eb2..ccc0a16e06 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,6 +19,8 @@ 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; @@ -174,6 +176,19 @@ 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/ValueObjectBinderTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/ValueObjectBinderTests.java index 7a2bff9088..48aa5dfabe 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 @@ -15,6 +15,7 @@ */ package org.springframework.boot.context.properties.bind; +import java.lang.reflect.Constructor; import java.time.LocalDate; import java.util.ArrayList; import java.util.List; @@ -93,6 +94,18 @@ class ValueObjectBinderTests { assertThat(bound).isFalse(); } + @Test + void bindToClassWithMultipleConstructorsAndFilterShouldBind() { + MockConfigurationPropertySource source = new MockConfigurationPropertySource(); + source.put("foo.int-value", "12"); + 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(); + assertThat(bound.getIntValue()).isEqualTo(12); + } + @Test void bindToClassWithOnlyDefaultConstructorShouldNotBind() { MockConfigurationPropertySource source = new MockConfigurationPropertySource(); @@ -258,14 +271,20 @@ class ValueObjectBinderTests { } - @SuppressWarnings("unused") static class MultipleConstructorsBean { + private final int intValue; + MultipleConstructorsBean(int intValue) { this(intValue, 23L, "hello"); } MultipleConstructorsBean(int intValue, long longValue, String stringValue) { + this.intValue = intValue; + } + + int getIntValue() { + return this.intValue; } } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/scan/valid/ConfigurationPropertiesScanConfiguration.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/scan/valid/ConfigurationPropertiesScanConfiguration.java index 70affd8e22..d316e6031f 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/scan/valid/ConfigurationPropertiesScanConfiguration.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/scan/valid/ConfigurationPropertiesScanConfiguration.java @@ -18,6 +18,7 @@ package org.springframework.boot.context.properties.scan.valid; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.boot.context.properties.ConfigurationPropertiesScan; import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.boot.context.properties.ImmutableConfigurationProperties; import org.springframework.boot.context.properties.scan.valid.b.BScanConfiguration; /** @@ -45,7 +46,7 @@ public class ConfigurationPropertiesScanConfiguration { } - @ConfigurationProperties(prefix = "bar") + @ImmutableConfigurationProperties(prefix = "bar") static class BarProperties { BarProperties(String foo) { diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/scan/valid/b/BScanConfiguration.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/scan/valid/b/BScanConfiguration.java index 7ffedd0539..900e7a30cc 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/scan/valid/b/BScanConfiguration.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/scan/valid/b/BScanConfiguration.java @@ -16,6 +16,7 @@ package org.springframework.boot.context.properties.scan.valid.b; import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.ImmutableConfigurationProperties; /** * @author Madhura Bhave @@ -27,7 +28,7 @@ public class BScanConfiguration { } - @ConfigurationProperties(prefix = "b.first") + @ImmutableConfigurationProperties(prefix = "b.first") public static class BFirstProperties implements BProperties { private final String name; diff --git a/spring-boot-project/spring-boot/src/test/kotlin/org/springframework/boot/context/properties/KotlinConfigurationPropertiesBeanRegistrarTests.kt b/spring-boot-project/spring-boot/src/test/kotlin/org/springframework/boot/context/properties/KotlinConfigurationPropertiesBeanRegistrarTests.kt index acf4449ecf..5c64255583 100644 --- a/spring-boot-project/spring-boot/src/test/kotlin/org/springframework/boot/context/properties/KotlinConfigurationPropertiesBeanRegistrarTests.kt +++ b/spring-boot-project/spring-boot/src/test/kotlin/org/springframework/boot/context/properties/KotlinConfigurationPropertiesBeanRegistrarTests.kt @@ -47,7 +47,7 @@ class KotlinConfigurationPropertiesBeanRegistrarTests { @ConfigurationProperties(prefix = "foo") class FooProperties - @ConfigurationProperties(prefix = "bar") + @ImmutableConfigurationProperties(prefix = "bar") class BarProperties(val name: String?, val counter: Int = 42) @ConfigurationProperties(prefix = "bing")