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

Author: philwebb

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<String> roles;
 
+			@ConstructorBinding
 			public Security(String username, String password,
 					@DefaultValue("USER") List<String> 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`)
 
 
 

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;

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationProperties.java

@@ -29,11 +29,18 @@
  * {@code @Bean} method in a {@code @Configuration} class if you want to bind and validate
  * some external Properties (e.g. from a .properties file).
  * <p>
+ * 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.
+ * <p>
  * 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
  */

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.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 Object getInstance() {
 		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 @@ private static Map<String, ConfigurationPropertiesBean> getAll(ConfigurableAppli
 	 */
 	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 @@ private static Method findFactoryMethod(ConfigurableApplicationContext applicati
 		return null;
 	}
 
-	private static <A extends Annotation> A getAnnotation(ApplicationContext applicationContext, Object bean,
-			String beanName, Method factoryMethod, Class<A> 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<Object> 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 extends Annotation> A findAnnotation(Object instance, Class<?> type, Method factory,
+			Class<A> annotationType) {
+		MergedAnnotation<A> 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;
+
 	}
 
 }

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,23 +88,12 @@ private void registerBeanDefinition(String beanName, Class<?> type,
 	}
 
 	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();
 		definition.setBeanClass(type);
 		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;
-	}
-
 }

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindException.java

@@ -29,41 +29,34 @@
  */
 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;
 	}
 
 	/**
 	 * Return the bean type that was being bound.
 	 * @return the bean type
 	 */
 	public Class<?> getBeanType() {
-		return this.beanType;
+		return this.bean.getType();
 	}
 
 	/**
 	 * Return the configuration properties annotation that triggered the binding.
 	 * @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());