From e203a689bf0c25a75efcbd13042a44523942c67a Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Tue, 15 Dec 2015 12:52:58 +0000 Subject: [PATCH] Consider relaxed variants of target name when filtering property names Previously, when ignoreUnknownFields was false and property names were being filtered based on whether or not they begin with the target name, relaxed variants of the target name were not considered. This resulted in different delimiters resulting in a non-match. For example, the property ENV_FOO_NAME would be filtered out when the target name was env.foo. This commit updates PropertiesConfigurationFactory to pass all of the relaxed variants for the target name to the matcher. For the example above one of those variants will be env_foo which matches ENV_FOO_NAME due to the matching delimiter. PropertiesConfigurationFactory was already creating a RelaxedNames instance for the target name. The code has been reworked a little to allow these relaxed names to be reused, thereby avoiding the cost of computing all of the relaxed variants of the target name a second time. Closes gh-4775 --- .../bind/PropertiesConfigurationFactory.java | 38 ++++++++++++------- .../PropertiesConfigurationFactoryTests.java | 30 +++++++++++++++ 2 files changed, 55 insertions(+), 13 deletions(-) diff --git a/spring-boot/src/main/java/org/springframework/boot/bind/PropertiesConfigurationFactory.java b/spring-boot/src/main/java/org/springframework/boot/bind/PropertiesConfigurationFactory.java index d9718ab393d..37c17f8b214 100644 --- a/spring-boot/src/main/java/org/springframework/boot/bind/PropertiesConfigurationFactory.java +++ b/spring-boot/src/main/java/org/springframework/boot/bind/PropertiesConfigurationFactory.java @@ -17,6 +17,7 @@ package org.springframework.boot.bind; import java.beans.PropertyDescriptor; +import java.util.HashSet; import java.util.LinkedHashSet; import java.util.Locale; import java.util.Map; @@ -262,19 +263,23 @@ public class PropertiesConfigurationFactory dataBinder.setIgnoreInvalidFields(this.ignoreInvalidFields); dataBinder.setIgnoreUnknownFields(this.ignoreUnknownFields); customizeBinder(dataBinder); - Set names = getNames(); - PropertyValues propertyValues = getPropertyValues(names); + Iterable relaxedTargetNames = getRelaxedTargetNames(); + Set names = getNames(relaxedTargetNames); + PropertyValues propertyValues = getPropertyValues(names, relaxedTargetNames); dataBinder.bind(propertyValues); if (this.validator != null) { validate(dataBinder); } } - private Set getNames() { + private Iterable getRelaxedTargetNames() { + return (this.target != null && StringUtils.hasLength(this.targetName) + ? new RelaxedNames(this.targetName) : null); + } + + private Set getNames(Iterable prefixes) { Set names = new LinkedHashSet(); if (this.target != null) { - Iterable prefixes = (StringUtils.hasLength(this.targetName) - ? new RelaxedNames(this.targetName) : null); PropertyDescriptor[] descriptors = BeanUtils .getPropertyDescriptors(this.target.getClass()); for (PropertyDescriptor descriptor : descriptors) { @@ -300,31 +305,38 @@ public class PropertiesConfigurationFactory return names; } - private PropertyValues getPropertyValues(Set names) { + private PropertyValues getPropertyValues(Set names, + Iterable relaxedTargetNames) { if (this.properties != null) { return new MutablePropertyValues(this.properties); } - return getPropertySourcesPropertyValues(names); + return getPropertySourcesPropertyValues(names, relaxedTargetNames); } - private PropertyValues getPropertySourcesPropertyValues(Set names) { - PropertyNamePatternsMatcher includes = getPropertyNamePatternsMatcher(names); + private PropertyValues getPropertySourcesPropertyValues(Set names, + Iterable relaxedTargetNames) { + PropertyNamePatternsMatcher includes = getPropertyNamePatternsMatcher(names, + relaxedTargetNames); return new PropertySourcesPropertyValues(this.propertySources, names, includes); } - private PropertyNamePatternsMatcher getPropertyNamePatternsMatcher( - Set names) { + private PropertyNamePatternsMatcher getPropertyNamePatternsMatcher(Set names, + Iterable relaxedTargetNames) { if (this.ignoreUnknownFields && !isMapTarget()) { // Since unknown fields are ignored we can filter them out early to save // unnecessary calls to the PropertySource. return new DefaultPropertyNamePatternsMatcher(EXACT_DELIMITERS, true, names); } - if (this.targetName != null) { + if (relaxedTargetNames != null) { // We can filter properties to those starting with the target name, but // we can't do a complete filter since we need to trigger the // unknown fields check + Set relaxedNames = new HashSet(); + for (String relaxedTargetName : relaxedTargetNames) { + relaxedNames.add(relaxedTargetName); + } return new DefaultPropertyNamePatternsMatcher(TARGET_NAME_DELIMITERS, true, - this.targetName); + relaxedNames); } // Not ideal, we basically can't filter anything return PropertyNamePatternsMatcher.ALL; diff --git a/spring-boot/src/test/java/org/springframework/boot/bind/PropertiesConfigurationFactoryTests.java b/spring-boot/src/test/java/org/springframework/boot/bind/PropertiesConfigurationFactoryTests.java index b22e00c81a5..dbe8e70d698 100644 --- a/spring-boot/src/test/java/org/springframework/boot/bind/PropertiesConfigurationFactoryTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/bind/PropertiesConfigurationFactoryTests.java @@ -134,6 +134,36 @@ public class PropertiesConfigurationFactoryTests { assertEquals("blah", foo.name); } + @Test + public void testBindWithDelimitedPrefixUsingMatchingDelimiter() throws Exception { + this.targetName = "env_foo"; + this.ignoreUnknownFields = false; + MutablePropertySources propertySources = new MutablePropertySources(); + propertySources.addLast(new SystemEnvironmentPropertySource("systemEnvironment", + Collections.singletonMap("ENV_FOO_NAME", "blah"))); + propertySources.addLast(new RandomValuePropertySource("random")); + setupFactory(); + this.factory.setPropertySources(propertySources); + this.factory.afterPropertiesSet(); + Foo foo = this.factory.getObject(); + assertEquals("blah", foo.name); + } + + @Test + public void testBindWithDelimitedPrefixUsingDifferentDelimiter() throws Exception { + this.targetName = "env.foo"; + MutablePropertySources propertySources = new MutablePropertySources(); + propertySources.addLast(new SystemEnvironmentPropertySource("systemEnvironment", + Collections.singletonMap("ENV_FOO_NAME", "blah"))); + propertySources.addLast(new RandomValuePropertySource("random")); + this.ignoreUnknownFields = false; + setupFactory(); + this.factory.setPropertySources(propertySources); + this.factory.afterPropertiesSet(); + Foo foo = this.factory.getObject(); + assertEquals("blah", foo.name); + } + private Foo createFoo(final String values) throws Exception { setupFactory(); return bindFoo(values);