From 8dcf1e4033eb1b453447fe48d2cbc9aa8c674d97 Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Tue, 10 Dec 2019 13:55:23 -0800 Subject: [PATCH] Provide end-to-end traceability for config properties Closes gh-14880 Co-authored-by: Andy Wilkinson --- ...rtiesReportEndpointDocumentationTests.java | 2 + ...ConfigurationPropertiesReportEndpoint.java | 83 +++++++++++- ...gurationPropertiesReportEndpointTests.java | 124 +++++++++++++++--- .../ConfigurationPropertiesBinder.java | 18 ++- ...rationPropertiesBoundPropertiesHolder.java | 66 ++++++++++ ...nableConfigurationPropertiesRegistrar.java | 1 + .../boot/context/properties/bind/Binder.java | 3 +- .../bind/BoundPropertiesHolder.java | 35 +++++ .../BoundPropertiesTrackingBindHandler.java | 46 +++++++ .../bind/IndexedElementsBinder.java | 1 + .../properties/bind/JavaBeanBinder.java | 6 +- ...eTopLevelConverterNotFoundBindHandler.java | 1 + .../source/ConfigurationPropertyName.java | 2 +- .../ConfigurationPropertiesTests.java | 11 ++ .../context/properties/bind/BinderTests.java | 16 --- ...undPropertiesTrackingBindHandlerTests.java | 79 +++++++++++ 16 files changed, 452 insertions(+), 42 deletions(-) create mode 100644 spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBoundPropertiesHolder.java create mode 100644 spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/BoundPropertiesHolder.java create mode 100644 spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/BoundPropertiesTrackingBindHandler.java create mode 100644 spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BoundPropertiesTrackingBindHandlerTests.java diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/endpoint/web/documentation/ConfigurationPropertiesReportEndpointDocumentationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/endpoint/web/documentation/ConfigurationPropertiesReportEndpointDocumentationTests.java index 7ba8209566..a7059faca8 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/endpoint/web/documentation/ConfigurationPropertiesReportEndpointDocumentationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/endpoint/web/documentation/ConfigurationPropertiesReportEndpointDocumentationTests.java @@ -51,6 +51,8 @@ class ConfigurationPropertiesReportEndpointDocumentationTests extends MockMvcEnd .description("Prefix applied to the names of the bean's properties."), subsectionWithPath("contexts.*.beans.*.properties") .description("Properties of the bean as name-value pairs."), + subsectionWithPath("contexts.*.beans.*.inputs").description( + "Origin and value of the configuration property used when binding to this bean."), parentIdField()))); } diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/context/properties/ConfigurationPropertiesReportEndpoint.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/context/properties/ConfigurationPropertiesReportEndpoint.java index 88a51d12c8..fd5bca8b66 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/context/properties/ConfigurationPropertiesReportEndpoint.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/context/properties/ConfigurationPropertiesReportEndpoint.java @@ -22,6 +22,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.stream.Collectors; @@ -55,7 +56,10 @@ import org.springframework.boot.actuate.endpoint.annotation.Endpoint; import org.springframework.boot.actuate.endpoint.annotation.ReadOperation; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.boot.context.properties.ConfigurationPropertiesBean; +import org.springframework.boot.context.properties.ConfigurationPropertiesBoundPropertiesHolder; import org.springframework.boot.context.properties.ConstructorBinding; +import org.springframework.boot.context.properties.source.ConfigurationProperty; +import org.springframework.boot.context.properties.source.ConfigurationPropertyName; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; import org.springframework.core.KotlinDetector; @@ -77,6 +81,8 @@ import org.springframework.util.StringUtils; * @author Christian Dupuis * @author Dave Syer * @author Stephane Nicoll + * @author Madhura Bhave + * @author Andy Wilkinson * @since 2.0.0 */ @Endpoint(id = "configprops") @@ -120,8 +126,10 @@ public class ConfigurationPropertiesReportEndpoint implements ApplicationContext Map descriptors = new HashMap<>(); beans.forEach((beanName, bean) -> { String prefix = bean.getAnnotation().prefix(); - descriptors.put(beanName, new ConfigurationPropertiesBeanDescriptor(prefix, - sanitize(prefix, safeSerialize(mapper, bean.getInstance(), prefix)))); + descriptors.put(beanName, + new ConfigurationPropertiesBeanDescriptor(prefix, + sanitize(prefix, safeSerialize(mapper, bean.getInstance(), prefix)), + getInputs(prefix, safeSerialize(mapper, bean.getInstance(), prefix)))); }); return new ContextConfigurationProperties(descriptors, (context.getParent() != null) ? context.getParent().getId() : null); @@ -229,6 +237,67 @@ public class ConfigurationPropertiesReportEndpoint implements ApplicationContext return sanitized; } + @SuppressWarnings("unchecked") + private Map getInputs(String prefix, Map map) { + map.forEach((key, value) -> { + String qualifiedKey = (prefix.isEmpty() ? prefix : prefix + ".") + key; + if (value instanceof Map) { + map.put(key, getInputs(qualifiedKey, (Map) value)); + } + else if (value instanceof List) { + map.put(key, getInputs(qualifiedKey, (List) value)); + } + else { + map.put(key, applyInput(qualifiedKey)); + } + }); + return map; + } + + @SuppressWarnings("unchecked") + private List getInputs(String prefix, List list) { + List augmented = new ArrayList<>(); + int index = 0; + for (Object item : list) { + String name = prefix + "[" + index++ + "]"; + if (item instanceof Map) { + augmented.add(getInputs(name, (Map) item)); + } + else if (item instanceof List) { + augmented.add(getInputs(name, (List) item)); + } + else { + augmented.add(applyInput(name)); + } + } + return augmented; + } + + private Map applyInput(String qualifiedKey) { + if (!this.context.containsBean(ConfigurationPropertiesBoundPropertiesHolder.BEAN_NAME)) { + return Collections.emptyMap(); + } + ConfigurationPropertiesBoundPropertiesHolder bean = this.context.getBean( + ConfigurationPropertiesBoundPropertiesHolder.BEAN_NAME, + ConfigurationPropertiesBoundPropertiesHolder.class); + Map boundProperties = bean.getProperties(); + ConfigurationPropertyName currentName = ConfigurationPropertyName.adapt(qualifiedKey, '.'); + ConfigurationProperty candidate = boundProperties.get(currentName); + if (candidate == null && currentName.isLastElementIndexed()) { + candidate = boundProperties.get(currentName.chop(currentName.getNumberOfElements() - 1)); + } + return (candidate != null) ? getInput(currentName.toString(), candidate) : Collections.emptyMap(); + } + + private Map getInput(String property, ConfigurationProperty candidate) { + Map input = new LinkedHashMap<>(); + String origin = (candidate.getOrigin() != null) ? candidate.getOrigin().toString() : "none"; + Object value = candidate.getValue(); + input.put("origin", origin); + input.put("value", this.sanitizer.sanitize(property, value)); + return input; + } + /** * Extension to {@link JacksonAnnotationIntrospector} to suppress CGLIB generated bean * properties. @@ -457,9 +526,13 @@ public class ConfigurationPropertiesReportEndpoint implements ApplicationContext private final Map properties; - private ConfigurationPropertiesBeanDescriptor(String prefix, Map properties) { + private final Map inputs; + + private ConfigurationPropertiesBeanDescriptor(String prefix, Map properties, + Map inputs) { this.prefix = prefix; this.properties = properties; + this.inputs = inputs; } public String getPrefix() { @@ -470,6 +543,10 @@ public class ConfigurationPropertiesReportEndpoint implements ApplicationContext return this.properties; } + public Map getInputs() { + return this.inputs; + } + } } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/context/properties/ConfigurationPropertiesReportEndpointTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/context/properties/ConfigurationPropertiesReportEndpointTests.java index db9e661e0d..ae48d1f7b3 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/context/properties/ConfigurationPropertiesReportEndpointTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/context/properties/ConfigurationPropertiesReportEndpointTests.java @@ -60,20 +60,26 @@ class ConfigurationPropertiesReportEndpointTests { @Test void descriptorWithJavaBeanBindMethodDetectsRelevantProperties() { this.contextRunner.withUserConfiguration(TestPropertiesConfiguration.class).run(assertProperties("test", - (properties) -> assertThat(properties).containsOnlyKeys("dbPassword", "myTestProperty", "duration"))); + (properties) -> assertThat(properties).containsOnlyKeys("dbPassword", "myTestProperty", "duration"), + (inputs) -> { + })); } @Test void descriptorWithValueObjectBindMethodDetectsRelevantProperties() { this.contextRunner.withUserConfiguration(ImmutablePropertiesConfiguration.class).run(assertProperties( "immutable", - (properties) -> assertThat(properties).containsOnlyKeys("dbPassword", "myTestProperty", "duration"))); + (properties) -> assertThat(properties).containsOnlyKeys("dbPassword", "myTestProperty", "duration"), + (inputs) -> { + })); } @Test void descriptorWithValueObjectBindMethodUseDedicatedConstructor() { - this.contextRunner.withUserConfiguration(MultiConstructorPropertiesConfiguration.class).run(assertProperties( - "multiconstructor", (properties) -> assertThat(properties).containsOnly(entry("name", "test")))); + this.contextRunner.withUserConfiguration(MultiConstructorPropertiesConfiguration.class) + .run(assertProperties("multiconstructor", + (properties) -> assertThat(properties).containsOnly(entry("name", "test")), (inputs) -> { + })); } @Test @@ -84,49 +90,89 @@ class ConfigurationPropertiesReportEndpointTests { assertThat(properties).containsOnlyKeys("name", "nested"); Map nested = (Map) properties.get("nested"); assertThat(nested).containsOnly(entry("name", "nested"), entry("counter", 42)); + }, (inputs) -> { + Map nested = (Map) inputs.get("nested"); + Map name = (Map) nested.get("name"); + Map counter = (Map) nested.get("counter"); + assertThat(name.get("value")).isEqualTo("nested"); + assertThat(name.get("origin")) + .isEqualTo("\"immutablenested.nested.name\" from property source \"test\""); + assertThat(counter.get("origin")) + .isEqualTo("\"immutablenested.nested.counter\" from property source \"test\""); + assertThat(counter.get("value")).isEqualTo("42"); + })); + } + + @Test + void descriptorWithSimpleList() { + this.contextRunner.withUserConfiguration(SensiblePropertiesConfiguration.class) + .withPropertyValues("sensible.simpleList=a,b").run(assertProperties("sensible", (properties) -> { + assertThat(properties.get("simpleList")).isInstanceOf(List.class); + List list = (List) properties.get("simpleList"); + assertThat(list).hasSize(2); + assertThat(list.get(0)).isEqualTo("a"); + assertThat(list.get(1)).isEqualTo("b"); + }, (inputs) -> { + List list = (List) inputs.get("simpleList"); + assertThat(list).hasSize(2); + Map item = (Map) list.get(0); + String origin = item.get("origin"); + String value = item.get("value"); + assertThat(value).isEqualTo("a,b"); + assertThat(origin).isEqualTo("\"sensible.simpleList\" from property source \"test\""); })); } @Test void descriptorDoesNotIncludePropertyWithNullValue() { - this.contextRunner.withUserConfiguration(TestPropertiesConfiguration.class) - .run(assertProperties("test", (properties) -> assertThat(properties).doesNotContainKey("nullValue"))); + this.contextRunner.withUserConfiguration(TestPropertiesConfiguration.class).run(assertProperties("test", + (properties) -> assertThat(properties).doesNotContainKey("nullValue"), (inputs) -> { + })); } @Test void descriptorWithDurationProperty() { this.contextRunner.withUserConfiguration(TestPropertiesConfiguration.class).run(assertProperties("test", - (properties) -> assertThat(properties.get("duration")).isEqualTo(Duration.ofSeconds(10).toString()))); + (properties) -> assertThat(properties.get("duration")).isEqualTo(Duration.ofSeconds(10).toString()), + (inputs) -> { + })); } @Test void descriptorWithNonCamelCaseProperty() { - this.contextRunner.withUserConfiguration(MixedCasePropertiesConfiguration.class).run(assertProperties( - "mixedcase", (properties) -> assertThat(properties.get("myURL")).isEqualTo("https://example.com"))); + this.contextRunner.withUserConfiguration(MixedCasePropertiesConfiguration.class) + .run(assertProperties("mixedcase", + (properties) -> assertThat(properties.get("myURL")).isEqualTo("https://example.com"), + (inputs) -> { + })); } @Test void descriptorWithMixedCaseProperty() { this.contextRunner.withUserConfiguration(MixedCasePropertiesConfiguration.class).run(assertProperties( - "mixedcase", (properties) -> assertThat(properties.get("mIxedCase")).isEqualTo("mixed"))); + "mixedcase", (properties) -> assertThat(properties.get("mIxedCase")).isEqualTo("mixed"), (inputs) -> { + })); } @Test void descriptorWithSingleLetterProperty() { - this.contextRunner.withUserConfiguration(MixedCasePropertiesConfiguration.class) - .run(assertProperties("mixedcase", (properties) -> assertThat(properties.get("z")).isEqualTo("zzz"))); + this.contextRunner.withUserConfiguration(MixedCasePropertiesConfiguration.class).run(assertProperties( + "mixedcase", (properties) -> assertThat(properties.get("z")).isEqualTo("zzz"), (inputs) -> { + })); } @Test void descriptorWithSimpleBooleanProperty() { this.contextRunner.withUserConfiguration(BooleanPropertiesConfiguration.class).run(assertProperties("boolean", - (properties) -> assertThat(properties.get("simpleBoolean")).isEqualTo(true))); + (properties) -> assertThat(properties.get("simpleBoolean")).isEqualTo(true), (inputs) -> { + })); } @Test void descriptorWithMixedBooleanProperty() { this.contextRunner.withUserConfiguration(BooleanPropertiesConfiguration.class).run(assertProperties("boolean", - (properties) -> assertThat(properties.get("mixedBoolean")).isEqualTo(true))); + (properties) -> assertThat(properties.get("mixedBoolean")).isEqualTo(true), (inputs) -> { + })); } @Test @@ -135,6 +181,7 @@ class ConfigurationPropertiesReportEndpointTests { .run(assertProperties("test", (properties) -> { assertThat(properties.get("dbPassword")).isEqualTo("******"); assertThat(properties.get("myTestProperty")).isEqualTo("654321"); + }, (inputs) -> { })); } @@ -144,6 +191,7 @@ class ConfigurationPropertiesReportEndpointTests { .withPropertyValues("test.keys-to-sanitize=property").run(assertProperties("test", (properties) -> { assertThat(properties.get("dbPassword")).isEqualTo("123456"); assertThat(properties.get("myTestProperty")).isEqualTo("******"); + }, (inputs) -> { })); } @@ -153,6 +201,7 @@ class ConfigurationPropertiesReportEndpointTests { .withPropertyValues("test.keys-to-sanitize=.*pass.*").run(assertProperties("test", (properties) -> { assertThat(properties.get("dbPassword")).isEqualTo("******"); assertThat(properties.get("myTestProperty")).isEqualTo("654321"); + }, (inputs) -> { })); } @@ -166,38 +215,61 @@ class ConfigurationPropertiesReportEndpointTests { assertThat(secrets.get("mine")).isEqualTo("******"); assertThat(secrets.get("yours")).isEqualTo("******"); assertThat(hidden.get("mine")).isEqualTo("******"); + }, (inputs) -> { })); } @Test void sanitizedUriWithSensitiveInfo() { this.contextRunner.withUserConfiguration(SensiblePropertiesConfiguration.class) + .withPropertyValues("sensible.sensitiveUri=http://user:password@localhost:8080") .run(assertProperties("sensible", (properties) -> assertThat(properties.get("sensitiveUri")) - .isEqualTo("http://user:******@localhost:8080"))); + .isEqualTo("http://user:******@localhost:8080"), (inputs) -> { + Map sensitiveUri = (Map) inputs.get("sensitiveUri"); + assertThat(sensitiveUri.get("value")).isEqualTo("http://user:******@localhost:8080"); + assertThat(sensitiveUri.get("origin")) + .isEqualTo("\"sensible.sensitiveUri\" from property source \"test\""); + })); } @Test void sanitizedUriWithNoPassword() { this.contextRunner.withUserConfiguration(SensiblePropertiesConfiguration.class) + .withPropertyValues("sensible.noPasswordUri=http://user:@localhost:8080") .run(assertProperties("sensible", (properties) -> assertThat(properties.get("noPasswordUri")) - .isEqualTo("http://user:******@localhost:8080"))); + .isEqualTo("http://user:******@localhost:8080"), (inputs) -> { + Map noPasswordUri = (Map) inputs.get("noPasswordUri"); + assertThat(noPasswordUri.get("value")).isEqualTo("http://user:******@localhost:8080"); + assertThat(noPasswordUri.get("origin")) + .isEqualTo("\"sensible.noPasswordUri\" from property source \"test\""); + })); } @Test void sanitizeLists() { this.contextRunner.withUserConfiguration(SensiblePropertiesConfiguration.class) + .withPropertyValues("sensible.listItems[0].some-password=password") .run(assertProperties("sensible", (properties) -> { assertThat(properties.get("listItems")).isInstanceOf(List.class); List list = (List) properties.get("listItems"); assertThat(list).hasSize(1); Map item = (Map) list.get(0); assertThat(item.get("somePassword")).isEqualTo("******"); + }, (inputs) -> { + List list = (List) inputs.get("listItems"); + assertThat(list).hasSize(1); + Map item = (Map) list.get(0); + Map somePassword = (Map) item.get("somePassword"); + assertThat(somePassword.get("value")).isEqualTo("******"); + assertThat(somePassword.get("origin")) + .isEqualTo("\"sensible.listItems[0].some-password\" from property source \"test\""); })); } @Test void listsOfListsAreSanitized() { this.contextRunner.withUserConfiguration(SensiblePropertiesConfiguration.class) + .withPropertyValues("sensible.listOfListItems[0][0].some-password=password") .run(assertProperties("sensible", (properties) -> { assertThat(properties.get("listOfListItems")).isInstanceOf(List.class); List> listOfLists = (List>) properties.get("listOfListItems"); @@ -206,11 +278,22 @@ class ConfigurationPropertiesReportEndpointTests { assertThat(list).hasSize(1); Map item = (Map) list.get(0); assertThat(item.get("somePassword")).isEqualTo("******"); + }, (inputs) -> { + assertThat(inputs.get("listOfListItems")).isInstanceOf(List.class); + List> listOfLists = (List>) inputs.get("listOfListItems"); + assertThat(listOfLists).hasSize(1); + List list = listOfLists.get(0); + assertThat(list).hasSize(1); + Map item = (Map) list.get(0); + Map somePassword = (Map) item.get("somePassword"); + assertThat(somePassword.get("value")).isEqualTo("******"); + assertThat(somePassword.get("origin")).isEqualTo( + "\"sensible.listOfListItems[0][0].some-password\" from property source \"test\""); })); } private ContextConsumer assertProperties(String prefix, - Consumer> properties) { + Consumer> properties, Consumer> inputs) { return (context) -> { ConfigurationPropertiesReportEndpoint endpoint = context .getBean(ConfigurationPropertiesReportEndpoint.class); @@ -222,6 +305,7 @@ class ConfigurationPropertiesReportEndpointTests { ConfigurationPropertiesBeanDescriptor descriptor = allProperties.getBeans().get(key.get()); assertThat(descriptor.getPrefix()).isEqualTo(prefix); properties.accept(descriptor.getProperties()); + inputs.accept(descriptor.getInputs()); }; } @@ -574,6 +658,8 @@ class ConfigurationPropertiesReportEndpointTests { private URI noPasswordUri = URI.create("http://user:@localhost:8080"); + private List simpleList = new ArrayList<>(); + private List listItems = new ArrayList<>(); private List> listOfListItems = new ArrayList<>(); @@ -615,6 +701,10 @@ class ConfigurationPropertiesReportEndpointTests { this.listOfListItems = listOfListItems; } + public List getSimpleList() { + return this.simpleList; + } + public static class ListItem { private String somePassword = "secret"; 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 18e7fdceb6..b21afa2ab5 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 @@ -32,6 +32,7 @@ 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.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; @@ -103,9 +104,18 @@ class ConfigurationPropertiesBinder { return null; } + private ConfigurationPropertiesBoundPropertiesHolder getBoundPropertiesHolder() { + if (this.applicationContext.containsBean(ConfigurationPropertiesBoundPropertiesHolder.BEAN_NAME)) { + return this.applicationContext.getBean(ConfigurationPropertiesBoundPropertiesHolder.BEAN_NAME, + ConfigurationPropertiesBoundPropertiesHolder.class); + } + return null; + } + private BindHandler getBindHandler(Bindable target, ConfigurationProperties annotation) { List validators = getValidators(target); - BindHandler handler = new IgnoreTopLevelConverterNotFoundBindHandler(); + ConfigurationPropertiesBoundPropertiesHolder holder = getBoundPropertiesHolder(); + BindHandler handler = getHandler(holder); if (annotation.ignoreInvalidFields()) { handler = new IgnoreErrorsBindHandler(handler); } @@ -122,6 +132,12 @@ class ConfigurationPropertiesBinder { return handler; } + private IgnoreTopLevelConverterNotFoundBindHandler getHandler(ConfigurationPropertiesBoundPropertiesHolder holder) { + return (holder != null) + ? new IgnoreTopLevelConverterNotFoundBindHandler(new BoundPropertiesTrackingBindHandler(holder)) + : new IgnoreTopLevelConverterNotFoundBindHandler(); + } + private List getValidators(Bindable target) { List validators = new ArrayList<>(3); if (this.configurationPropertiesValidator != null) { diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBoundPropertiesHolder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBoundPropertiesHolder.java new file mode 100644 index 0000000000..99cf14468f --- /dev/null +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBoundPropertiesHolder.java @@ -0,0 +1,66 @@ +/* + * 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.util.LinkedHashMap; +import java.util.Map; + +import org.springframework.beans.factory.config.BeanDefinition; +import org.springframework.beans.factory.support.BeanDefinitionRegistry; +import org.springframework.beans.factory.support.GenericBeanDefinition; +import org.springframework.boot.context.properties.bind.BoundPropertiesHolder; +import org.springframework.boot.context.properties.source.ConfigurationProperty; +import org.springframework.boot.context.properties.source.ConfigurationPropertyName; +import org.springframework.util.Assert; + +/** + * {@link BoundPropertiesHolder} for + * {@link ConfigurationProperties @ConfigurationProperties}. + * + * @author Madhura Bhave + * @since 2.3.0 + */ +public class ConfigurationPropertiesBoundPropertiesHolder implements BoundPropertiesHolder { + + private Map properties = new LinkedHashMap<>(); + + /** + * The bean name that this class is registered with. + */ + public static final String BEAN_NAME = ConfigurationPropertiesBoundPropertiesHolder.class.getName(); + + @Override + public void recordBinding(ConfigurationProperty configurationProperty) { + Assert.notNull(configurationProperty, "ConfigurationProperty should not be null"); + this.properties.put(configurationProperty.getName(), configurationProperty); + } + + public Map getProperties() { + return this.properties; + } + + static void register(BeanDefinitionRegistry registry) { + Assert.notNull(registry, "Registry must not be null"); + if (!registry.containsBeanDefinition(BEAN_NAME)) { + GenericBeanDefinition definition = new GenericBeanDefinition(); + definition.setBeanClass(ConfigurationPropertiesBoundPropertiesHolder.class); + definition.setRole(BeanDefinition.ROLE_INFRASTRUCTURE); + registry.registerBeanDefinition(BEAN_NAME, definition); + } + } + +} diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/EnableConfigurationPropertiesRegistrar.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/EnableConfigurationPropertiesRegistrar.java index f5f14cea14..8c0c4a4e0b 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/EnableConfigurationPropertiesRegistrar.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/EnableConfigurationPropertiesRegistrar.java @@ -49,6 +49,7 @@ class EnableConfigurationPropertiesRegistrar implements ImportBeanDefinitionRegi @SuppressWarnings("deprecation") static void registerInfrastructureBeans(BeanDefinitionRegistry registry) { ConfigurationPropertiesBindingPostProcessor.register(registry); + ConfigurationPropertiesBoundPropertiesHolder.register(registry); ConfigurationPropertiesBeanDefinitionValidator.register(registry); ConfigurationBeanFactoryMetadata.register(registry); } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Binder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Binder.java index 78f7b74bca..4f3b0d65ff 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Binder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Binder.java @@ -310,7 +310,6 @@ public class Binder { private T bind(ConfigurationPropertyName name, Bindable target, BindHandler handler, Context context, boolean allowRecursiveBinding, boolean create) { - context.clearConfigurationProperty(); try { Bindable replacementTarget = handler.onStart(name, target, context); if (replacementTarget == null) { @@ -576,7 +575,7 @@ public class Binder { } } - private void setConfigurationProperty(ConfigurationProperty configurationProperty) { + void setConfigurationProperty(ConfigurationProperty configurationProperty) { this.configurationProperty = configurationProperty; } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/BoundPropertiesHolder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/BoundPropertiesHolder.java new file mode 100644 index 0000000000..4f412d3dc0 --- /dev/null +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/BoundPropertiesHolder.java @@ -0,0 +1,35 @@ +/* + * Copyright 2012-2019 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.context.properties.bind; + +import org.springframework.boot.context.properties.source.ConfigurationProperty; + +/** + * Record bound configuration properties. + * + * @author Madhura Bhave + * @since 2.3.0 + */ +public interface BoundPropertiesHolder { + + /** + * Record the bound configuration property. + * @param configurationProperty the bound property + */ + void recordBinding(ConfigurationProperty configurationProperty); + +} diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/BoundPropertiesTrackingBindHandler.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/BoundPropertiesTrackingBindHandler.java new file mode 100644 index 0000000000..5cc98bc7e2 --- /dev/null +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/BoundPropertiesTrackingBindHandler.java @@ -0,0 +1,46 @@ +/* + * Copyright 2012-2019 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.context.properties.bind; + +import org.springframework.boot.context.properties.source.ConfigurationPropertyName; +import org.springframework.util.Assert; + +/** + * {@link BindHandler} that can be used to track bound configuration properties. + * + * @author Madhura Bhave + * @since 2.3.0 + */ +public class BoundPropertiesTrackingBindHandler extends AbstractBindHandler { + + private final BoundPropertiesHolder holder; + + public BoundPropertiesTrackingBindHandler(BoundPropertiesHolder holder) { + super(); + Assert.notNull(holder, "Bound properties holder should not be null."); + this.holder = holder; + } + + @Override + public Object onSuccess(ConfigurationPropertyName name, Bindable target, BindContext context, Object result) { + if (context.getConfigurationProperty() != null && name.equals(context.getConfigurationProperty().getName())) { + this.holder.recordBinding(context.getConfigurationProperty()); + } + return super.onSuccess(name, target, context, result); + } + +} diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/IndexedElementsBinder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/IndexedElementsBinder.java index 7ceed825d8..6ccdd3a3ae 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/IndexedElementsBinder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/IndexedElementsBinder.java @@ -80,6 +80,7 @@ abstract class IndexedElementsBinder extends AggregateBinder { ResolvableType elementType) { ConfigurationProperty property = source.getConfigurationProperty(root); if (property != null) { + getContext().setConfigurationProperty(property); bindValue(target, collection.get(), aggregateType, elementType, property.getValue()); } else { diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/JavaBeanBinder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/JavaBeanBinder.java index 35ca1be480..07f8535368 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/JavaBeanBinder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/JavaBeanBinder.java @@ -53,7 +53,7 @@ class JavaBeanBinder implements DataObjectBinder { return null; } BeanSupplier beanSupplier = bean.getSupplier(target); - boolean bound = bind(propertyBinder, bean, beanSupplier); + boolean bound = bind(propertyBinder, bean, beanSupplier, context); return (bound ? beanSupplier.get() : null); } @@ -73,10 +73,12 @@ class JavaBeanBinder implements DataObjectBinder { return false; } - private boolean bind(DataObjectPropertyBinder propertyBinder, Bean bean, BeanSupplier beanSupplier) { + private boolean bind(DataObjectPropertyBinder propertyBinder, Bean bean, BeanSupplier beanSupplier, + Context context) { boolean bound = false; for (BeanProperty beanProperty : bean.getProperties().values()) { bound |= bind(beanSupplier, propertyBinder, beanProperty); + context.clearConfigurationProperty(); } return bound; } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/handler/IgnoreTopLevelConverterNotFoundBindHandler.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/handler/IgnoreTopLevelConverterNotFoundBindHandler.java index 7d2f9eb264..cc90e3fc08 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/handler/IgnoreTopLevelConverterNotFoundBindHandler.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/handler/IgnoreTopLevelConverterNotFoundBindHandler.java @@ -44,6 +44,7 @@ public class IgnoreTopLevelConverterNotFoundBindHandler extends AbstractBindHand * @param parent the parent handler */ public IgnoreTopLevelConverterNotFoundBindHandler(BindHandler parent) { + super(parent); } @Override diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyName.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyName.java index cb63a5b8fd..1f8ba7b2f3 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyName.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyName.java @@ -543,7 +543,7 @@ public final class ConfigurationPropertyName implements Comparable configuration, String... inlinedProperties) { return load(new Class[] { configuration }, inlinedProperties); } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BinderTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BinderTests.java index 8a7ffb8b62..0a6bccf48a 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BinderTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BinderTests.java @@ -241,22 +241,6 @@ class BinderTests { assertThat(result.toString()).isEqualTo("2014-04-01"); } - @Test - void bindExceptionWhenBeanBindingFailsShouldHaveNullConfigurationProperty() { - MockConfigurationPropertySource source = new MockConfigurationPropertySource(); - source.put("foo.value", "hello"); - source.put("foo.items", "bar,baz"); - this.sources.add(source); - Bindable target = Bindable.of(JavaBean.class); - assertThatExceptionOfType(BindException.class).isThrownBy(() -> this.binder.bind("foo", target)) - .satisfies(this::noItemsSetterRequirements); - } - - private void noItemsSetterRequirements(BindException ex) { - assertThat(ex.getCause().getMessage()).isEqualTo("No setter found for property: items"); - assertThat(ex.getProperty()).isNull(); - } - @Test void bindToValidatedBeanWithResourceAndNonEnumerablePropertySource() { ConfigurationPropertySources.from(new PropertySource("test") { diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BoundPropertiesTrackingBindHandlerTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BoundPropertiesTrackingBindHandlerTests.java new file mode 100644 index 0000000000..bc3d8a909d --- /dev/null +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BoundPropertiesTrackingBindHandlerTests.java @@ -0,0 +1,79 @@ +/* + * Copyright 2012-2019 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.context.properties.bind; + +import java.util.ArrayList; +import java.util.List; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import org.springframework.boot.context.properties.source.ConfigurationProperty; +import org.springframework.boot.context.properties.source.ConfigurationPropertySource; +import org.springframework.boot.context.properties.source.MockConfigurationPropertySource; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +/** + * Tests for {@link BoundPropertiesTrackingBindHandler}. + * + * @author Madhura Bhave + */ +public class BoundPropertiesTrackingBindHandlerTests { + + private List sources = new ArrayList<>(); + + private BoundPropertiesTrackingBindHandler handler; + + private Binder binder; + + private BoundPropertiesHolder recorder; + + @BeforeEach + void setup() { + this.binder = new Binder(this.sources); + this.recorder = mock(BoundPropertiesHolder.class); + this.handler = new BoundPropertiesTrackingBindHandler(this.recorder); + } + + @Test + void handlerShouldCallRecordBindingIfConfigurationPropertyIsNotNull() { + this.sources.add(new MockConfigurationPropertySource("foo.age", 4)); + this.binder.bind("foo", Bindable.of(ExampleBean.class), this.handler); + verify(this.recorder, times(1)).recordBinding(any(ConfigurationProperty.class)); + verify(this.recorder, never()).recordBinding(null); + } + + static class ExampleBean { + + private int age; + + int getAge() { + return this.age; + } + + void setAge(int age) { + this.age = age; + } + + } + +}