Browse Source

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 <mbhave@pivotal.io>
pull/24980/head
Phillip Webb 5 years ago
parent
commit
c268f5d418
  1. 30
      spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBinder.java
  2. 59
      spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Bindable.java
  3. 10
      spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Binder.java
  4. 20
      spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesTests.java
  5. 45
      spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/WithPublicStringConstructorProperties.java
  6. 19
      spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BindableTests.java
  7. 27
      spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BinderTests.java
  8. 43
      spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/JavaBeanWithPublicConstructor.java

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

@ -1,5 +1,5 @@ @@ -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,9 +29,12 @@ import org.springframework.beans.factory.config.BeanDefinition; @@ -29,9 +29,12 @@ import org.springframework.beans.factory.config.BeanDefinition;
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;
@ -39,12 +42,14 @@ import org.springframework.boot.context.properties.bind.handler.IgnoreErrorsBind @@ -39,12 +42,14 @@ import org.springframework.boot.context.properties.bind.handler.IgnoreErrorsBind
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 @@ class ConfigurationPropertiesBinder { @@ -108,6 +113,7 @@ class ConfigurationPropertiesBinder {
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 @@ class ConfigurationPropertiesBinder { @@ -230,4 +236,26 @@ class ConfigurationPropertiesBinder {
}
/**
* {@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);
}
}
}

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

@ -1,5 +1,5 @@ @@ -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 @@ package org.springframework.boot.context.properties.bind; @@ -18,11 +18,14 @@ package org.springframework.boot.context.properties.bind;
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> { @@ -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> { @@ -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 final class Bindable<T> { @@ -105,6 +114,16 @@ public final class Bindable<T> {
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 final class Bindable<T> { @@ -117,6 +136,7 @@ public final class Bindable<T> {
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 final class Bindable<T> { @@ -126,6 +146,7 @@ public final class Bindable<T> {
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 @@ public final class Bindable<T> { @@ -149,7 +170,7 @@ public final class Bindable<T> {
*/
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 final class Bindable<T> { @@ -162,7 +183,7 @@ public final class Bindable<T> {
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 final class Bindable<T> { @@ -171,7 +192,19 @@ public final class Bindable<T> {
* @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 final class Bindable<T> { @@ -244,7 +277,7 @@ public final class Bindable<T> {
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 @@ public final class Bindable<T> { @@ -260,4 +293,18 @@ public final class Bindable<T> {
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
}
}

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

@ -1,5 +1,5 @@ @@ -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 java.util.function.Supplier; @@ -30,6 +30,7 @@ import java.util.function.Supplier;
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 @@ public class Binder { @@ -366,7 +367,7 @@ public class Binder {
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 @@ public class Binder { @@ -414,8 +415,9 @@ public class Binder {
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()) {

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

@ -1,5 +1,5 @@ @@ -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 @@ class ConfigurationPropertiesTests { @@ -1010,6 +1010,18 @@ class ConfigurationPropertiesTests {
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 @@ class ConfigurationPropertiesTests { @@ -2584,4 +2596,10 @@ class ConfigurationPropertiesTests {
}
@Configuration
@EnableConfigurationProperties(WithPublicStringConstructorProperties.class)
static class WithPublicStringConstructorPropertiesConfiguration {
}
}

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

@ -0,0 +1,45 @@ @@ -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;
}
}

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

@ -1,5 +1,5 @@ @@ -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 java.lang.annotation.RetentionPolicy; @@ -22,6 +22,7 @@ import java.lang.annotation.RetentionPolicy;
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 @@ class BindableTests { @@ -173,6 +174,22 @@ class BindableTests {
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 {

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

@ -1,5 +1,5 @@ @@ -1,5 +1,5 @@
/*
* Copyright 2012-2019 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.junit.jupiter.api.Test; @@ -30,6 +30,7 @@ import org.junit.jupiter.api.Test;
import org.mockito.Answers;
import org.mockito.InOrder;
import org.springframework.boot.context.properties.bind.Bindable.BindRestriction;
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;
@ -316,6 +317,30 @@ class BinderTests { @@ -316,6 +317,30 @@ class BinderTests {
assertThat(result.isBound()).isFalse();
}
@Test
void bindToJavaBeanWithPublicConstructor() {
Bindable<JavaBeanWithPublicConstructor> bindable = Bindable.of(JavaBeanWithPublicConstructor.class);
JavaBeanWithPublicConstructor result = bindToJavaBeanWithPublicConstructor(bindable);
assertThat(result.getValue()).isEqualTo("constructor");
}
@Test
void bindToJavaBeanWithPublicConstructorWhenHasBindRestriction() {
Bindable<JavaBeanWithPublicConstructor> bindable = Bindable.of(JavaBeanWithPublicConstructor.class)
.withBindRestrictions(BindRestriction.NO_DIRECT_PROPERTY);
JavaBeanWithPublicConstructor result = bindToJavaBeanWithPublicConstructor(bindable);
assertThat(result.getValue()).isEqualTo("setter");
}
private JavaBeanWithPublicConstructor bindToJavaBeanWithPublicConstructor(
Bindable<JavaBeanWithPublicConstructor> bindable) {
MockConfigurationPropertySource source = new MockConfigurationPropertySource();
source.put("foo", "constructor");
source.put("foo.value", "setter");
this.sources.add(source);
return this.binder.bindOrCreate("foo", bindable);
}
static class JavaBean {
private String value;

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

@ -0,0 +1,43 @@ @@ -0,0 +1,43 @@
/*
* 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.bind;
/**
* Java bean with an additional public single-arg constructor.
*
* @author Phillip Webb
*/
public class JavaBeanWithPublicConstructor {
private String value;
public JavaBeanWithPublicConstructor() {
}
public JavaBeanWithPublicConstructor(String value) {
setValue(value);
}
public String getValue() {
return this.value;
}
public void setValue(String value) {
this.value = value;
}
}
Loading…
Cancel
Save