From 00430ac2161e3d9e7ee9546e87d74e2bd85f2f85 Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Wed, 28 Jun 2017 14:45:46 -0700 Subject: [PATCH] Ignore unbound env variables and system props Closes gh-3832 --- ...urationPropertiesBindingPostProcessor.java | 4 +- .../handler/NoUnboundElementsBindHandler.java | 12 +++- .../AliasedConfigurationPropertySource.java | 5 ++ .../source/ConfigurationPropertySource.java | 6 ++ ...FilteredConfigurationPropertiesSource.java | 5 ++ .../MapConfigurationPropertySource.java | 5 ++ .../SpringConfigurationPropertySource.java | 5 ++ .../source/UnboundElementsSourceFilter.java | 55 +++++++++++++++ ...onPropertiesBindingPostProcessorTests.java | 15 +++++ .../NoUnboundElementsBindHandlerTests.java | 15 +++++ .../MockConfigurationPropertySource.java | 10 +++ .../UnboundElementsSourceFilterTests.java | 67 +++++++++++++++++++ 12 files changed, 201 insertions(+), 3 deletions(-) create mode 100644 spring-boot/src/main/java/org/springframework/boot/context/properties/source/UnboundElementsSourceFilter.java create mode 100644 spring-boot/src/test/java/org/springframework/boot/context/properties/source/UnboundElementsSourceFilterTests.java diff --git a/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindingPostProcessor.java b/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindingPostProcessor.java index ab1586789c..7a1356b9ad 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindingPostProcessor.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindingPostProcessor.java @@ -43,6 +43,7 @@ import org.springframework.boot.context.properties.bind.handler.NoUnboundElement import org.springframework.boot.context.properties.bind.validation.ValidationBindHandler; 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.boot.validation.MessageInterpolatorFactory; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; @@ -420,7 +421,8 @@ public class ConfigurationPropertiesBindingPostProcessor implements BeanPostProc handler = new IgnoreErrorsBindHandler(handler); } if (!annotation.ignoreUnknownFields()) { - handler = new NoUnboundElementsBindHandler(handler); + UnboundElementsSourceFilter filter = new UnboundElementsSourceFilter(); + handler = new NoUnboundElementsBindHandler(handler, filter); } if (validator != null) { handler = new ValidationBindHandler(handler, validator); diff --git a/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/handler/NoUnboundElementsBindHandler.java b/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/handler/NoUnboundElementsBindHandler.java index 162464a51a..b0f0ae5fd3 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/handler/NoUnboundElementsBindHandler.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/handler/NoUnboundElementsBindHandler.java @@ -19,6 +19,7 @@ package org.springframework.boot.context.properties.bind.handler; import java.util.HashSet; import java.util.Set; import java.util.TreeSet; +import java.util.function.Function; import org.springframework.boot.context.properties.bind.AbstractBindHandler; import org.springframework.boot.context.properties.bind.BindContext; @@ -42,12 +43,19 @@ public class NoUnboundElementsBindHandler extends AbstractBindHandler { private final Set boundNames = new HashSet<>(); + private final Function filter; + NoUnboundElementsBindHandler() { - super(); + this(BindHandler.DEFAULT, (configurationPropertySource) -> true); } public NoUnboundElementsBindHandler(BindHandler parent) { + this(parent, (configurationPropertySource) -> true); + } + + public NoUnboundElementsBindHandler(BindHandler parent, Function filter) { super(parent); + this.filter = filter; } @Override @@ -69,7 +77,7 @@ public class NoUnboundElementsBindHandler extends AbstractBindHandler { BindContext context) { Set unbound = new TreeSet<>(); for (ConfigurationPropertySource source : context.getSources()) { - if (source instanceof IterableConfigurationPropertySource) { + if (this.filter.apply(source)) { collectUnbound(name, unbound, (IterableConfigurationPropertySource) source); } diff --git a/spring-boot/src/main/java/org/springframework/boot/context/properties/source/AliasedConfigurationPropertySource.java b/spring-boot/src/main/java/org/springframework/boot/context/properties/source/AliasedConfigurationPropertySource.java index 135ecbc37a..ea9496881b 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/properties/source/AliasedConfigurationPropertySource.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/properties/source/AliasedConfigurationPropertySource.java @@ -68,6 +68,11 @@ class AliasedConfigurationPropertySource implements ConfigurationPropertySource return ConfigurationPropertyState.ABSENT; } + @Override + public Object getUnderlyingSource() { + return this.source.getUnderlyingSource(); + } + protected ConfigurationPropertySource getSource() { return this.source; } diff --git a/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertySource.java b/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertySource.java index a3c33ebda4..cb89c98bf9 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertySource.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertySource.java @@ -72,4 +72,10 @@ public interface ConfigurationPropertySource { return new AliasedConfigurationPropertySource(this, aliases); } + /** + * Return the underlying {@PropertySource}. + * @return the underlying property source. + */ + Object getUnderlyingSource(); + } diff --git a/spring-boot/src/main/java/org/springframework/boot/context/properties/source/FilteredConfigurationPropertiesSource.java b/spring-boot/src/main/java/org/springframework/boot/context/properties/source/FilteredConfigurationPropertiesSource.java index 91584108cb..9f0d83d02b 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/properties/source/FilteredConfigurationPropertiesSource.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/properties/source/FilteredConfigurationPropertiesSource.java @@ -58,6 +58,11 @@ class FilteredConfigurationPropertiesSource implements ConfigurationPropertySour return result; } + @Override + public Object getUnderlyingSource() { + return this.source.getUnderlyingSource(); + } + protected ConfigurationPropertySource getSource() { return this.source; } diff --git a/spring-boot/src/main/java/org/springframework/boot/context/properties/source/MapConfigurationPropertySource.java b/spring-boot/src/main/java/org/springframework/boot/context/properties/source/MapConfigurationPropertySource.java index 1ac954c737..41c5ea1255 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/properties/source/MapConfigurationPropertySource.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/properties/source/MapConfigurationPropertySource.java @@ -78,6 +78,11 @@ public class MapConfigurationPropertySource this.source.put((name == null ? null : name.toString()), value); } + @Override + public Object getUnderlyingSource() { + return this.source; + } + @Override public ConfigurationProperty getConfigurationProperty( ConfigurationPropertyName name) { diff --git a/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringConfigurationPropertySource.java b/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringConfigurationPropertySource.java index ab9eeffee1..0a0592a530 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringConfigurationPropertySource.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringConfigurationPropertySource.java @@ -94,6 +94,11 @@ class SpringConfigurationPropertySource implements ConfigurationPropertySource { return this.containsDescendantOfMethod.apply(name); } + @Override + public Object getUnderlyingSource() { + return this.propertySource; + } + protected final ConfigurationProperty find(List mappings, ConfigurationPropertyName name) { return mappings.stream().filter((m) -> m.isApplicable(name)).map(this::find) diff --git a/spring-boot/src/main/java/org/springframework/boot/context/properties/source/UnboundElementsSourceFilter.java b/spring-boot/src/main/java/org/springframework/boot/context/properties/source/UnboundElementsSourceFilter.java new file mode 100644 index 0000000000..2b857c60e1 --- /dev/null +++ b/spring-boot/src/main/java/org/springframework/boot/context/properties/source/UnboundElementsSourceFilter.java @@ -0,0 +1,55 @@ +/* + * Copyright 2012-2017 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 + * + * http://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.source; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; +import java.util.function.Function; + +import org.springframework.core.env.PropertySource; +import org.springframework.core.env.StandardEnvironment; + +/** + * Function used to determine if a {@link ConfigurationPropertySource} should be + * included when determining unbound elements. If the underlying {@link PropertySource} + * is a systemEnvironment or systemProperties property source , it will not be considered + * for unbound element failures. + * + * @author Madhura Bhave + * @since 2.0.0 + */ +public class UnboundElementsSourceFilter implements Function { + + private static final Set BENIGN_PROPERTY_SOURCE_NAMES = Collections.unmodifiableSet(new HashSet<>(Arrays.asList( + StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME, + StandardEnvironment.SYSTEM_PROPERTIES_PROPERTY_SOURCE_NAME))); + + @Override + public Boolean apply(ConfigurationPropertySource configurationPropertySource) { + Object underlyingSource = configurationPropertySource.getUnderlyingSource(); + if (underlyingSource instanceof PropertySource) { + String name = ((PropertySource) underlyingSource).getName(); + return !BENIGN_PROPERTY_SOURCE_NAMES.contains(name); + + } + return true; + } + +} + diff --git a/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindingPostProcessorTests.java b/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindingPostProcessorTests.java index 7e764ad4dc..58c915b1b4 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindingPostProcessorTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindingPostProcessorTests.java @@ -419,6 +419,21 @@ public class ConfigurationPropertiesBindingPostProcessorTests { assertThat(foo).isEqualTo(10); } + @Test + public void unboundElementsFromSystemEnvironmentShouldNotThrowException() throws Exception { + this.context = new AnnotationConfigApplicationContext(); + ConfigurableEnvironment env = this.context.getEnvironment(); + MutablePropertySources propertySources = env.getPropertySources(); + propertySources.addFirst(new MapPropertySource("test", + Collections.singletonMap("com.example.foo", 5))); + propertySources.addLast(new SystemEnvironmentPropertySource("system", + Collections.singletonMap("COM_EXAMPLE_OTHER", "10"))); + this.context.register(TestConfiguration.class); + this.context.refresh(); + int foo = this.context.getBean(TestConfiguration.class).getFoo(); + assertThat(foo).isEqualTo(5); + } + @Test public void rebindableConfigurationProperties() throws Exception { // gh-9160 diff --git a/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/handler/NoUnboundElementsBindHandlerTests.java b/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/handler/NoUnboundElementsBindHandlerTests.java index 0733005839..9281fe4d1e 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/handler/NoUnboundElementsBindHandlerTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/handler/NoUnboundElementsBindHandlerTests.java @@ -24,6 +24,7 @@ import org.junit.Test; import org.junit.rules.ExpectedException; import org.springframework.boot.context.properties.bind.BindException; +import org.springframework.boot.context.properties.bind.BindHandler; import org.springframework.boot.context.properties.bind.Bindable; import org.springframework.boot.context.properties.bind.Binder; import org.springframework.boot.context.properties.source.ConfigurationPropertyName; @@ -103,6 +104,20 @@ public class NoUnboundElementsBindHandlerTests { assertThat(bound.getFoo()).isEqualTo("bar"); } + @Test + public void bindWhenUsingNoUnboundElementsHandlerShouldBindIfUnboundSystemProperties() + throws Exception { + MockConfigurationPropertySource source = new MockConfigurationPropertySource(); + source.put("example.foo", "bar"); + source.put("example.other", "baz"); + this.sources.add(source); + this.binder = new Binder(this.sources); + NoUnboundElementsBindHandler handler = new NoUnboundElementsBindHandler(BindHandler.DEFAULT, (configurationPropertySource -> false)); + Example bound = this.binder.bind("example", Bindable.of(Example.class), + handler).get(); + assertThat(bound.getFoo()).isEqualTo("bar"); + } + public static class Example { private String foo; diff --git a/spring-boot/src/test/java/org/springframework/boot/context/properties/source/MockConfigurationPropertySource.java b/spring-boot/src/test/java/org/springframework/boot/context/properties/source/MockConfigurationPropertySource.java index b99596bd66..d44119fb8c 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/properties/source/MockConfigurationPropertySource.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/properties/source/MockConfigurationPropertySource.java @@ -75,6 +75,11 @@ public class MockConfigurationPropertySource return this.map.keySet().stream(); } + @Override + public Object getUnderlyingSource() { + return this.map; + } + @Override public ConfigurationProperty getConfigurationProperty( ConfigurationPropertyName name) { @@ -91,6 +96,11 @@ public class MockConfigurationPropertySource private class NonIterable implements ConfigurationPropertySource { + @Override + public Object getUnderlyingSource() { + return MockConfigurationPropertySource.this.map; + } + @Override public ConfigurationProperty getConfigurationProperty( ConfigurationPropertyName name) { diff --git a/spring-boot/src/test/java/org/springframework/boot/context/properties/source/UnboundElementsSourceFilterTests.java b/spring-boot/src/test/java/org/springframework/boot/context/properties/source/UnboundElementsSourceFilterTests.java new file mode 100644 index 0000000000..ee4af02fcd --- /dev/null +++ b/spring-boot/src/test/java/org/springframework/boot/context/properties/source/UnboundElementsSourceFilterTests.java @@ -0,0 +1,67 @@ +/* + * Copyright 2012-2017 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 + * + * http://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.source; + +import org.junit.Before; +import org.junit.Test; + +import org.springframework.core.env.StandardEnvironment; +import org.springframework.mock.env.MockPropertySource; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; + +/** + * Tests for {@link UnboundElementsSourceFilter}. + * + * @author Madhura Bhave + */ +public class UnboundElementsSourceFilterTests { + + private UnboundElementsSourceFilter filter; + + private ConfigurationPropertySource source; + + @Before + public void setUp() throws Exception { + this.filter = new UnboundElementsSourceFilter(); + this.source = mock(ConfigurationPropertySource.class); + } + + @Test + public void filterWhenSourceIsSystemEnvironmentPropertySourceShouldReturnFalse() throws Exception { + MockPropertySource propertySource = new MockPropertySource(StandardEnvironment.SYSTEM_PROPERTIES_PROPERTY_SOURCE_NAME); + given(this.source.getUnderlyingSource()).willReturn(propertySource); + assertThat(this.filter.apply(this.source)).isFalse(); + } + + @Test + public void filterWhenSourceIsSystemPropertiesPropertySourceShouldReturnTrue() throws Exception { + MockPropertySource propertySource = new MockPropertySource(StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME); + given(this.source.getUnderlyingSource()).willReturn(propertySource); + assertThat(this.filter.apply(this.source)).isFalse(); + } + + @Test + public void filterWhenSourceIsNotSystemShouldReturnTrue() throws Exception { + MockPropertySource propertySource = new MockPropertySource("test"); + given(this.source.getUnderlyingSource()).willReturn(propertySource); + assertThat(this.filter.apply(this.source)).isTrue(); + } + +}