Skip direct @ConfugurationProperties binding

Add a `BindRestriction` option to `Bindable` which allows direct
property binding to be bypassed. The option is automatically applied
by the `ConfigurationPropertiesBinder`.

Prior to this commit, `@ConfugurationProperties` binding could silently
fail if a direct property existed that could be converted to the
properties class. This can be the case if a single-argument constructor
is available as the `ObjectToObject` converter would kick in.

Closes gh-16038

Co-authored-by: Madhura Bhave <[email protected]>

Author: philwebb

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2020 the original author or authors.
+ * Copyright 2012-2021 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.
@@ -29,22 +29,27 @@
 import org.springframework.beans.factory.support.AbstractBeanDefinition;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
 import org.springframework.beans.factory.support.BeanDefinitionRegistry;
+import org.springframework.boot.context.properties.bind.AbstractBindHandler;
+import org.springframework.boot.context.properties.bind.BindContext;
 import org.springframework.boot.context.properties.bind.BindHandler;
 import org.springframework.boot.context.properties.bind.BindResult;
 import org.springframework.boot.context.properties.bind.Bindable;
+import org.springframework.boot.context.properties.bind.Bindable.BindRestriction;
 import org.springframework.boot.context.properties.bind.Binder;
 import org.springframework.boot.context.properties.bind.BoundPropertiesTrackingBindHandler;
 import org.springframework.boot.context.properties.bind.PropertySourcesPlaceholdersResolver;
 import org.springframework.boot.context.properties.bind.handler.IgnoreErrorsBindHandler;
 import org.springframework.boot.context.properties.bind.handler.IgnoreTopLevelConverterNotFoundBindHandler;
 import org.springframework.boot.context.properties.bind.handler.NoUnboundElementsBindHandler;
 import org.springframework.boot.context.properties.bind.validation.ValidationBindHandler;
+import org.springframework.boot.context.properties.source.ConfigurationPropertyName;
 import org.springframework.boot.context.properties.source.ConfigurationPropertySource;
 import org.springframework.boot.context.properties.source.ConfigurationPropertySources;
 import org.springframework.boot.context.properties.source.UnboundElementsSourceFilter;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.ApplicationContextAware;
 import org.springframework.context.ConfigurableApplicationContext;
+import org.springframework.core.annotation.MergedAnnotations;
 import org.springframework.core.convert.ConversionService;
 import org.springframework.core.env.PropertySources;
 import org.springframework.validation.Validator;
@@ -108,6 +113,7 @@ private Validator getConfigurationPropertiesValidator(ApplicationContext applica
 	private <T> BindHandler getBindHandler(Bindable<T> target, ConfigurationProperties annotation) {
 		List<Validator> validators = getValidators(target);
 		BindHandler handler = getHandler();
+		handler = new ConfigurationPropertiesBindHander(handler);
 		if (annotation.ignoreInvalidFields()) {
 			handler = new IgnoreErrorsBindHandler(handler);
 		}
@@ -230,4 +236,26 @@ ConfigurationPropertiesBinder create() {
 
 	}
 
+	/**
+	 * {@link BindHandler} to deal with
+	 * {@link ConfigurationProperties @ConfigurationProperties} concerns.
+	 */
+	private static class ConfigurationPropertiesBindHander extends AbstractBindHandler {
+
+		ConfigurationPropertiesBindHander(BindHandler handler) {
+			super(handler);
+		}
+
+		@Override
+		public <T> Bindable<T> onStart(ConfigurationPropertyName name, Bindable<T> target, BindContext context) {
+			return isConfigurationProperties(target.getType().resolve())
+					? target.withBindRestrictions(BindRestriction.NO_DIRECT_PROPERTY) : target;
+		}
+
+		private boolean isConfigurationProperties(Class<?> target) {
+			return target != null && MergedAnnotations.from(target).isPresent(ConfigurationProperties.class);
+		}
+
+	}
+
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2020 the original author or authors.
+ * Copyright 2012-2021 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.
@@ -18,11 +18,14 @@
 
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Array;
+import java.util.Arrays;
+import java.util.EnumSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.function.Supplier;
 
+import org.springframework.boot.context.properties.source.ConfigurationProperty;
 import org.springframework.core.ResolvableType;
 import org.springframework.core.style.ToStringCreator;
 import org.springframework.util.Assert;
@@ -42,6 +45,8 @@ public final class Bindable<T> {
 
 	private static final Annotation[] NO_ANNOTATIONS = {};
 
+	private static final EnumSet<BindRestriction> NO_BIND_RESTRICTIONS = EnumSet.noneOf(BindRestriction.class);
+
 	private final ResolvableType type;
 
 	private final ResolvableType boxedType;
@@ -50,11 +55,15 @@ public final class Bindable<T> {
 
 	private final Annotation[] annotations;
 
-	private Bindable(ResolvableType type, ResolvableType boxedType, Supplier<T> value, Annotation[] annotations) {
+	private final EnumSet<BindRestriction> bindRestrictions;
+
+	private Bindable(ResolvableType type, ResolvableType boxedType, Supplier<T> value, Annotation[] annotations,
+			EnumSet<BindRestriction> bindRestrictions) {
 		this.type = type;
 		this.boxedType = boxedType;
 		this.value = value;
 		this.annotations = annotations;
+		this.bindRestrictions = bindRestrictions;
 	}
 
 	/**
@@ -105,6 +114,16 @@ public <A extends Annotation> A getAnnotation(Class<A> type) {
 		return null;
 	}
 
+	/**
+	 * Returns {@code true} if the specified bind restriction has been added.
+	 * @param bindRestriction the bind restriction to check
+	 * @return if the bind restriction has been added
+	 * @since 2.5.0
+	 */
+	public boolean hasBindRestriction(BindRestriction bindRestriction) {
+		return this.bindRestrictions.contains(bindRestriction);
+	}
+
 	@Override
 	public boolean equals(Object obj) {
 		if (this == obj) {
@@ -117,6 +136,7 @@ public boolean equals(Object obj) {
 		boolean result = true;
 		result = result && nullSafeEquals(this.type.resolve(), other.type.resolve());
 		result = result && nullSafeEquals(this.annotations, other.annotations);
+		result = result && nullSafeEquals(this.bindRestrictions, other.bindRestrictions);
 		return result;
 	}
 
@@ -126,6 +146,7 @@ public int hashCode() {
 		int result = 1;
 		result = prime * result + ObjectUtils.nullSafeHashCode(this.type);
 		result = prime * result + ObjectUtils.nullSafeHashCode(this.annotations);
+		result = prime * result + ObjectUtils.nullSafeHashCode(this.bindRestrictions);
 		return result;
 	}
 
@@ -149,7 +170,7 @@ private boolean nullSafeEquals(Object o1, Object o2) {
 	 */
 	public Bindable<T> withAnnotations(Annotation... annotations) {
 		return new Bindable<>(this.type, this.boxedType, this.value,
-				(annotations != null) ? annotations : NO_ANNOTATIONS);
+				(annotations != null) ? annotations : NO_ANNOTATIONS, NO_BIND_RESTRICTIONS);
 	}
 
 	/**
@@ -162,7 +183,7 @@ public Bindable<T> withExistingValue(T existingValue) {
 				existingValue == null || this.type.isArray() || this.boxedType.resolve().isInstance(existingValue),
 				() -> "ExistingValue must be an instance of " + this.type);
 		Supplier<T> 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.bindRestrictions);
 	}
 
 	/**
@@ -171,7 +192,19 @@ public Bindable<T> withExistingValue(T existingValue) {
 	 * @return an updated {@link Bindable}
 	 */
 	public Bindable<T> withSuppliedValue(Supplier<T> suppliedValue) {
-		return new Bindable<>(this.type, this.boxedType, suppliedValue, this.annotations);
+		return new Bindable<>(this.type, this.boxedType, suppliedValue, this.annotations, this.bindRestrictions);
+	}
+
+	/**
+	 * Create an updated {@link Bindable} instance with additional bind restrictions.
+	 * @param additionalRestrictions any additional restrictions to apply
+	 * @return an updated {@link Bindable}
+	 * @since 2.5.0
+	 */
+	public Bindable<T> withBindRestrictions(BindRestriction... additionalRestrictions) {
+		EnumSet<BindRestriction> bindRestrictions = EnumSet.copyOf(this.bindRestrictions);
+		bindRestrictions.addAll(Arrays.asList(additionalRestrictions));
+		return new Bindable<>(this.type, this.boxedType, this.value, this.annotations, bindRestrictions);
 	}
 
 	/**
@@ -244,7 +277,7 @@ public static <K, V> Bindable<Map<K, V>> mapOf(Class<K> keyType, Class<V> valueT
 	public static <T> Bindable<T> 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, NO_BIND_RESTRICTIONS);
 	}
 
 	private static ResolvableType box(ResolvableType type) {
@@ -260,4 +293,18 @@ private static ResolvableType box(ResolvableType type) {
 		return type;
 	}
 
+	/**
+	 * Restrictions that can be applied when binding values.
+	 *
+	 * @since 2.5.0
+	 */
+	public enum BindRestriction {
+
+		/**
+		 * Do not bind direct {@link ConfigurationProperty} matches.
+		 */
+		NO_DIRECT_PROPERTY
+
+	}
+
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2020 the original author or authors.
+ * Copyright 2012-2021 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.
@@ -30,6 +30,7 @@
 
 import org.springframework.beans.PropertyEditorRegistry;
 import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
+import org.springframework.boot.context.properties.bind.Bindable.BindRestriction;
 import org.springframework.boot.context.properties.source.ConfigurationProperty;
 import org.springframework.boot.context.properties.source.ConfigurationPropertyName;
 import org.springframework.boot.context.properties.source.ConfigurationPropertySource;
@@ -366,7 +367,7 @@ private <T> T handleBindError(ConfigurationPropertyName name, Bindable<T> target
 
 	private <T> Object bindObject(ConfigurationPropertyName name, Bindable<T> target, BindHandler handler,
 			Context context, boolean allowRecursiveBinding) {
-		ConfigurationProperty property = findProperty(name, context);
+		ConfigurationProperty property = findProperty(name, target, context);
 		if (property == null && context.depth != 0 && containsNoDescendantOf(context.getSources(), name)) {
 			return null;
 		}
@@ -414,8 +415,9 @@ private <T> Object bindAggregate(ConfigurationPropertyName name, Bindable<T> tar
 		return context.withIncreasedDepth(() -> aggregateBinder.bind(name, target, elementBinder));
 	}
 
-	private ConfigurationProperty findProperty(ConfigurationPropertyName name, Context context) {
-		if (name.isEmpty()) {
+	private <T> ConfigurationProperty findProperty(ConfigurationPropertyName name, Bindable<T> target,
+			Context context) {
+		if (name.isEmpty() || target.hasBindRestriction(BindRestriction.NO_DIRECT_PROPERTY)) {
 			return null;
 		}
 		for (ConfigurationPropertySource source : context.getSources()) {

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2020 the original author or authors.
+ * Copyright 2012-2021 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.
@@ -1010,6 +1010,18 @@ void loadWhenConstructorBindingWithOuterClassDeducedConstructorBound() {
 		assertThat(bean.getNested().getOuter().getAge()).isEqualTo(5);
 	}
 
+	@Test
+	void loadWhenConfigurationPropertiesPrefixMatchesPropertyInEnvironment() {
+		MutablePropertySources sources = this.context.getEnvironment().getPropertySources();
+		Map<String, Object> source = new HashMap<>();
+		source.put("test", "bar");
+		source.put("test.a", "baz");
+		sources.addLast(new MapPropertySource("test", source));
+		load(WithPublicStringConstructorPropertiesConfiguration.class);
+		WithPublicStringConstructorProperties bean = this.context.getBean(WithPublicStringConstructorProperties.class);
+		assertThat(bean.getA()).isEqualTo("baz");
+	}
+
 	@Test
 	void boundPropertiesShouldBeRecorded() {
 		load(NestedConfiguration.class, "name=foo", "nested.name=bar");
@@ -2584,4 +2596,10 @@ int getAge() {
 
 	}
 
+	@Configuration
+	@EnableConfigurationProperties(WithPublicStringConstructorProperties.class)
+	static class WithPublicStringConstructorPropertiesConfiguration {
+
+	}
+
 }

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/WithPublicStringConstructorProperties.java

@@ -0,0 +1,45 @@
+/*
+ * Copyright 2012-2021 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;
+
+/**
+ * A {@link ConfigurationProperties @ConfigurationProperties} with an additional
+ * single-arg public constructor. Used in {@link ConfigurationPropertiesTests}.
+ *
+ * @author Madhura Bhave
+ */
+@ConfigurationProperties(prefix = "test")
+public class WithPublicStringConstructorProperties {
+
+	private String a;
+
+	public WithPublicStringConstructorProperties() {
+	}
+
+	public WithPublicStringConstructorProperties(String a) {
+		this.a = a;
+	}
+
+	public String getA() {
+		return this.a;
+	}
+
+	public void setA(String a) {
+		this.a = a;
+	}
+
+}

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BindableTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2020 the original author or authors.
+ * Copyright 2012-2021 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.
@@ -22,6 +22,7 @@
 
 import org.junit.jupiter.api.Test;
 
+import org.springframework.boot.context.properties.bind.Bindable.BindRestriction;
 import org.springframework.context.annotation.Bean;
 import org.springframework.core.ResolvableType;
 import org.springframework.core.annotation.AnnotationUtils;
@@ -173,6 +174,22 @@ void withSuppliedValueDoesNotForgetAnnotations() {
 		assertThat(bindable.getAnnotations()).containsExactly(annotation);
 	}
 
+	@Test
+	void hasBindRestrictionWhenDefaultReturnsFalse() {
+		Bindable<String> bindable = Bindable.of(String.class);
+		for (BindRestriction bindRestriction : BindRestriction.values()) {
+			assertThat(bindable.hasBindRestriction(bindRestriction)).isFalse();
+		}
+	}
+
+	@Test
+	void withBindRestrictionAddsBindRestriction() {
+		Bindable<String> bindable = Bindable.of(String.class);
+		Bindable<String> restricted = bindable.withBindRestrictions(BindRestriction.NO_DIRECT_PROPERTY);
+		assertThat(bindable.hasBindRestriction(BindRestriction.NO_DIRECT_PROPERTY)).isFalse();
+		assertThat(restricted.hasBindRestriction(BindRestriction.NO_DIRECT_PROPERTY)).isTrue();
+	}
+
 	@Retention(RetentionPolicy.RUNTIME)
 	@interface TestAnnotation {