Commit 12b876fb authored by Phillip Webb's avatar Phillip Webb

Reduce PropertySource access when binding

Update `PropertiesConfigurationFactory` so that when possible fewer
calls are made to the underlying `PropertySource`. The
`PropertySourcesPropertyValues` class now accepts a matcher which is
used to limit the properties that should be adapted. The factory will
create a matcher based on the standard relaxed binding rules.

Fixes gh-3402
See gh-3515
parent 1d31d23e
...@@ -21,22 +21,34 @@ import java.util.HashSet; ...@@ -21,22 +21,34 @@ import java.util.HashSet;
import java.util.Set; import java.util.Set;
/** /**
* Default {@link PropertyNamePatternsMatcher} that matches when a property name exactly * {@link PropertyNamePatternsMatcher} that matches when a property name exactly matches
* matches one of the given names, or starts with one of the given names followed by '.' * one of the given names, or starts with one of the given names followed by a delimiter.
* or '_'. This implementation is optimized for frequent calls. * This implementation is optimized for frequent calls.
* *
* @author Phillip Webb * @author Phillip Webb
* @since 1.2.0 * @since 1.2.0
*/ */
class DefaultPropertyNamePatternsMatcher implements PropertyNamePatternsMatcher { class DefaultPropertyNamePatternsMatcher implements PropertyNamePatternsMatcher {
private final char[] delimiters;
private final boolean ignoreCase;
private final String[] names; private final String[] names;
public DefaultPropertyNamePatternsMatcher(String... names) { protected DefaultPropertyNamePatternsMatcher(char[] delimiters, String... names) {
this(new HashSet<String>(Arrays.asList(names))); this(delimiters, false, names);
}
protected DefaultPropertyNamePatternsMatcher(char[] delimiters, boolean ignoreCase,
String... names) {
this(delimiters, ignoreCase, new HashSet<String>(Arrays.asList(names)));
} }
public DefaultPropertyNamePatternsMatcher(Set<String> names) { public DefaultPropertyNamePatternsMatcher(char[] delimiters, boolean ignoreCase,
Set<String> names) {
this.delimiters = delimiters;
this.ignoreCase = ignoreCase;
this.names = names.toArray(new String[names.size()]); this.names = names.toArray(new String[names.size()]);
} }
...@@ -55,18 +67,19 @@ class DefaultPropertyNamePatternsMatcher implements PropertyNamePatternsMatcher ...@@ -55,18 +67,19 @@ class DefaultPropertyNamePatternsMatcher implements PropertyNamePatternsMatcher
return false; return false;
} }
for (int charIndex = 0; charIndex < propertyNameChars.length; charIndex++) { for (int charIndex = 0; charIndex < propertyNameChars.length; charIndex++) {
noneMatched = true;
for (int nameIndex = 0; nameIndex < this.names.length; nameIndex++) { for (int nameIndex = 0; nameIndex < this.names.length; nameIndex++) {
if (match[nameIndex]) { if (match[nameIndex]) {
match[nameIndex] = false;
if (charIndex < this.names[nameIndex].length()) { if (charIndex < this.names[nameIndex].length()) {
if (this.names[nameIndex].charAt(charIndex) == propertyNameChars[charIndex]) { if (isCharMatch(this.names[nameIndex].charAt(charIndex),
propertyNameChars[charIndex])) {
match[nameIndex] = true; match[nameIndex] = true;
noneMatched = false; noneMatched = false;
} }
} }
else { else {
char charAfter = propertyNameChars[this.names[nameIndex].length()]; char charAfter = propertyNameChars[this.names[nameIndex].length()];
if (charAfter == '.' || charAfter == '_') { if (isDelimeter(charAfter)) {
match[nameIndex] = true; match[nameIndex] = true;
noneMatched = false; noneMatched = false;
} }
...@@ -85,4 +98,20 @@ class DefaultPropertyNamePatternsMatcher implements PropertyNamePatternsMatcher ...@@ -85,4 +98,20 @@ class DefaultPropertyNamePatternsMatcher implements PropertyNamePatternsMatcher
return false; return false;
} }
private boolean isCharMatch(char c1, char c2) {
if (this.ignoreCase) {
return Character.toLowerCase(c1) == Character.toLowerCase(c2);
}
return c1 == c2;
}
private boolean isDelimeter(char c) {
for (char delimiter : this.delimiters) {
if (c == delimiter) {
return true;
}
}
return false;
}
} }
...@@ -27,11 +27,11 @@ import org.springframework.util.PatternMatchUtils; ...@@ -27,11 +27,11 @@ import org.springframework.util.PatternMatchUtils;
* @author Phillip Webb * @author Phillip Webb
* @since 1.2.0 * @since 1.2.0
*/ */
class SimplePropertyNamePatternsMatcher implements PropertyNamePatternsMatcher { class PatternPropertyNamePatternsMatcher implements PropertyNamePatternsMatcher {
private final String[] patterns; private final String[] patterns;
public SimplePropertyNamePatternsMatcher(Collection<String> patterns) { public PatternPropertyNamePatternsMatcher(Collection<String> patterns) {
this.patterns = (patterns == null ? new String[] {} : patterns this.patterns = (patterns == null ? new String[] {} : patterns
.toArray(new String[patterns.size()])); .toArray(new String[patterns.size()]));
} }
......
...@@ -17,8 +17,9 @@ ...@@ -17,8 +17,9 @@
package org.springframework.boot.bind; package org.springframework.boot.bind;
import java.beans.PropertyDescriptor; import java.beans.PropertyDescriptor;
import java.util.HashSet; import java.util.LinkedHashSet;
import java.util.Locale; import java.util.Locale;
import java.util.Map;
import java.util.Properties; import java.util.Properties;
import java.util.Set; import java.util.Set;
...@@ -34,6 +35,7 @@ import org.springframework.context.MessageSourceAware; ...@@ -34,6 +35,7 @@ import org.springframework.context.MessageSourceAware;
import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.ConversionService;
import org.springframework.core.env.PropertySources; import org.springframework.core.env.PropertySources;
import org.springframework.util.Assert; import org.springframework.util.Assert;
import org.springframework.util.StringUtils;
import org.springframework.validation.BindException; import org.springframework.validation.BindException;
import org.springframework.validation.BindingResult; import org.springframework.validation.BindingResult;
import org.springframework.validation.DataBinder; import org.springframework.validation.DataBinder;
...@@ -51,6 +53,10 @@ import org.springframework.validation.Validator; ...@@ -51,6 +53,10 @@ import org.springframework.validation.Validator;
public class PropertiesConfigurationFactory<T> implements FactoryBean<T>, public class PropertiesConfigurationFactory<T> implements FactoryBean<T>,
MessageSourceAware, InitializingBean { MessageSourceAware, InitializingBean {
private static final char[] EXACT_DELIMETERS = { '_', '.', '[' };
private static final char[] TARGET_NAME_DELIMETERS = { '_', '.' };
private final Log logger = LogFactory.getLog(getClass()); private final Log logger = LogFactory.getLog(getClass());
private boolean ignoreUnknownFields = true; private boolean ignoreUnknownFields = true;
...@@ -257,16 +263,28 @@ public class PropertiesConfigurationFactory<T> implements FactoryBean<T>, ...@@ -257,16 +263,28 @@ public class PropertiesConfigurationFactory<T> implements FactoryBean<T>,
} }
private Set<String> getNames() { private Set<String> getNames() {
Set<String> names = new HashSet<String>(); Set<String> names = new LinkedHashSet<String>();
if (this.target != null) { if (this.target != null) {
Iterable<String> prefixes = (StringUtils.hasLength(this.targetName) ? new RelaxedNames(
this.targetName) : null);
PropertyDescriptor[] descriptors = BeanUtils PropertyDescriptor[] descriptors = BeanUtils
.getPropertyDescriptors(this.target.getClass()); .getPropertyDescriptors(this.target.getClass());
String prefix = (this.targetName != null ? this.targetName + "." : "");
for (PropertyDescriptor descriptor : descriptors) { for (PropertyDescriptor descriptor : descriptors) {
String name = descriptor.getName(); String name = descriptor.getName();
if (!name.equals("class")) { if (!name.equals("class")) {
for (String relaxedName : new RelaxedNames(prefix + name)) { RelaxedNames relaxedNames = RelaxedNames.forCamelCase(name);
names.add(relaxedName); if (prefixes == null) {
for (String relaxedName : relaxedNames) {
names.add(relaxedName);
}
}
else {
for (String prefix : prefixes) {
for (String relaxedName : relaxedNames) {
names.add(prefix + "." + relaxedName);
names.add(prefix + "_" + relaxedName);
}
}
} }
} }
} }
...@@ -278,8 +296,33 @@ public class PropertiesConfigurationFactory<T> implements FactoryBean<T>, ...@@ -278,8 +296,33 @@ public class PropertiesConfigurationFactory<T> implements FactoryBean<T>,
if (this.properties != null) { if (this.properties != null) {
return new MutablePropertyValues(this.properties); return new MutablePropertyValues(this.properties);
} }
return new PropertySourcesPropertyValues(this.propertySources, return getPropertySourcesPropertyValues(names);
new DefaultPropertyNamePatternsMatcher(names), names); }
private PropertyValues getPropertySourcesPropertyValues(Set<String> names) {
PropertyNamePatternsMatcher includes = getPropertyNamePatternsMatcher(names);
return new PropertySourcesPropertyValues(this.propertySources, names, includes);
}
private PropertyNamePatternsMatcher getPropertyNamePatternsMatcher(Set<String> names) {
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_DELIMETERS, true, names);
}
if (this.targetName != 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
return new DefaultPropertyNamePatternsMatcher(TARGET_NAME_DELIMETERS,
this.targetName);
}
// Not ideal, we basically can't filter anything
return PropertyNamePatternsMatcher.ALL;
}
private boolean isMapTarget() {
return this.target != null && Map.class.isAssignableFrom(this.target.getClass());
} }
private void validate(RelaxedDataBinder dataBinder) throws BindException { private void validate(RelaxedDataBinder dataBinder) throws BindException {
......
...@@ -24,6 +24,15 @@ package org.springframework.boot.bind; ...@@ -24,6 +24,15 @@ package org.springframework.boot.bind;
*/ */
interface PropertyNamePatternsMatcher { interface PropertyNamePatternsMatcher {
PropertyNamePatternsMatcher ALL = new PropertyNamePatternsMatcher() {
@Override
public boolean matches(String propertyName) {
return true;
}
};
PropertyNamePatternsMatcher NONE = new PropertyNamePatternsMatcher() { PropertyNamePatternsMatcher NONE = new PropertyNamePatternsMatcher() {
@Override @Override
......
...@@ -16,7 +16,6 @@ ...@@ -16,7 +16,6 @@
package org.springframework.boot.bind; package org.springframework.boot.bind;
import java.util.Arrays;
import java.util.Collection; import java.util.Collection;
import java.util.Collections; import java.util.Collections;
import java.util.LinkedHashMap; import java.util.LinkedHashMap;
...@@ -30,7 +29,7 @@ import org.springframework.core.env.EnumerablePropertySource; ...@@ -30,7 +29,7 @@ import org.springframework.core.env.EnumerablePropertySource;
import org.springframework.core.env.PropertySource; import org.springframework.core.env.PropertySource;
import org.springframework.core.env.PropertySources; import org.springframework.core.env.PropertySources;
import org.springframework.core.env.PropertySourcesPropertyResolver; import org.springframework.core.env.PropertySourcesPropertyResolver;
import org.springframework.core.env.StandardEnvironment; import org.springframework.util.Assert;
import org.springframework.validation.DataBinder; import org.springframework.validation.DataBinder;
/** /**
...@@ -46,17 +45,16 @@ public class PropertySourcesPropertyValues implements PropertyValues { ...@@ -46,17 +45,16 @@ public class PropertySourcesPropertyValues implements PropertyValues {
private final PropertySources propertySources; private final PropertySources propertySources;
private static final Collection<String> PATTERN_MATCHED_PROPERTY_SOURCES = Arrays private final Collection<String> propertyNames;
.asList(StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME,
StandardEnvironment.SYSTEM_PROPERTIES_PROPERTY_SOURCE_NAME); private final PropertyNamePatternsMatcher includes;
/** /**
* Create a new PropertyValues from the given PropertySources * Create a new PropertyValues from the given PropertySources
* @param propertySources a PropertySources instance * @param propertySources a PropertySources instance
*/ */
public PropertySourcesPropertyValues(PropertySources propertySources) { public PropertySourcesPropertyValues(PropertySources propertySources) {
this(propertySources, (PropertyNamePatternsMatcher) null, this(propertySources, (Collection<String>) null, PropertyNamePatternsMatcher.ALL);
(Collection<String>) null);
} }
/** /**
...@@ -64,52 +62,55 @@ public class PropertySourcesPropertyValues implements PropertyValues { ...@@ -64,52 +62,55 @@ public class PropertySourcesPropertyValues implements PropertyValues {
* @param propertySources a PropertySources instance * @param propertySources a PropertySources instance
* @param includePatterns property name patterns to include from system properties and * @param includePatterns property name patterns to include from system properties and
* environment variables * environment variables
* @param names exact property names to include * @param propertyNames the property names to used in lieu of an
* {@link EnumerablePropertySource}.
*/ */
public PropertySourcesPropertyValues(PropertySources propertySources, public PropertySourcesPropertyValues(PropertySources propertySources,
Collection<String> includePatterns, Collection<String> names) { Collection<String> includePatterns, Collection<String> propertyNames) {
this(propertySources, new SimplePropertyNamePatternsMatcher(includePatterns), this(propertySources, propertyNames, new PatternPropertyNamePatternsMatcher(
names); includePatterns));
} }
/** /**
* Create a new PropertyValues from the given PropertySources * Create a new PropertyValues from the given PropertySources
* @param propertySources a PropertySources instance * @param propertySources a PropertySources instance
* @param includes property name patterns to include from system properties and * @param propertyNames the property names to used in lieu of an
* environment variables * {@link EnumerablePropertySource}.
* @param names exact property names to include * @param includes the property name patterns to include
*/ */
PropertySourcesPropertyValues(PropertySources propertySources, PropertySourcesPropertyValues(PropertySources propertySources,
PropertyNamePatternsMatcher includes, Collection<String> names) { Collection<String> propertyNames, PropertyNamePatternsMatcher includes) {
Assert.notNull(propertySources, "PropertySources must not be null");
Assert.notNull(includes, "Includes must not be null");
this.propertySources = propertySources; this.propertySources = propertySources;
if (includes == null) { this.propertyNames = (propertyNames == null ? Collections.<String> emptySet()
includes = PropertyNamePatternsMatcher.NONE; : propertyNames);
} this.includes = includes;
if (names == null) {
names = Collections.emptySet();
}
PropertySourcesPropertyResolver resolver = new PropertySourcesPropertyResolver( PropertySourcesPropertyResolver resolver = new PropertySourcesPropertyResolver(
propertySources); propertySources);
for (PropertySource<?> source : propertySources) { for (PropertySource<?> source : propertySources) {
processPropertySource(source, resolver, includes, names); processPropertySource(source, resolver);
} }
} }
private void processPropertySource(PropertySource<?> source, private void processPropertySource(PropertySource<?> source,
PropertySourcesPropertyResolver resolver, PropertySourcesPropertyResolver resolver) {
PropertyNamePatternsMatcher includes, Collection<String> exacts) {
if (source instanceof CompositePropertySource) { if (source instanceof CompositePropertySource) {
processCompositePropertySource((CompositePropertySource) source, resolver, processCompositePropertySource((CompositePropertySource) source, resolver);
includes, exacts);
} }
else if (source instanceof EnumerablePropertySource) { else if (source instanceof EnumerablePropertySource) {
processEnumerablePropertySource((EnumerablePropertySource<?>) source, processEnumerablePropertySource((EnumerablePropertySource<?>) source,
resolver, includes); resolver, this.includes);
} }
else { else {
// We can only do exact matches for non-enumerable property names, but processNonEnumerablePropertySource(source, resolver);
// that's better than nothing... }
processDefaultPropertySource(source, resolver, includes, exacts); }
private void processCompositePropertySource(CompositePropertySource source,
PropertySourcesPropertyResolver resolver) {
for (PropertySource<?> nested : source.getPropertySources()) {
processPropertySource(nested, resolver);
} }
} }
...@@ -117,14 +118,9 @@ public class PropertySourcesPropertyValues implements PropertyValues { ...@@ -117,14 +118,9 @@ public class PropertySourcesPropertyValues implements PropertyValues {
PropertySourcesPropertyResolver resolver, PropertyNamePatternsMatcher includes) { PropertySourcesPropertyResolver resolver, PropertyNamePatternsMatcher includes) {
if (source.getPropertyNames().length > 0) { if (source.getPropertyNames().length > 0) {
for (String propertyName : source.getPropertyNames()) { for (String propertyName : source.getPropertyNames()) {
if (PropertySourcesPropertyValues.PATTERN_MATCHED_PROPERTY_SOURCES if (includes.matches(propertyName)) {
.contains(source.getName()) && !includes.matches(propertyName)) { Object value = getEnumerableProperty(source, resolver, propertyName);
continue; putIfAbsent(propertyName, value);
}
Object value = getEnumerableProperty(source, resolver, propertyName);
if (!this.propertyValues.containsKey(propertyName)) {
this.propertyValues.put(propertyName, new PropertyValue(propertyName,
value));
} }
} }
} }
...@@ -141,18 +137,11 @@ public class PropertySourcesPropertyValues implements PropertyValues { ...@@ -141,18 +137,11 @@ public class PropertySourcesPropertyValues implements PropertyValues {
} }
} }
private void processCompositePropertySource(CompositePropertySource source, private void processNonEnumerablePropertySource(PropertySource<?> source,
PropertySourcesPropertyResolver resolver, PropertySourcesPropertyResolver resolver) {
PropertyNamePatternsMatcher includes, Collection<String> exacts) { // We can only do exact matches for non-enumerable property names, but
for (PropertySource<?> nested : source.getPropertySources()) { // that's better than nothing...
processPropertySource(nested, resolver, includes, exacts); for (String propertyName : this.propertyNames) {
}
}
private void processDefaultPropertySource(PropertySource<?> source,
PropertySourcesPropertyResolver resolver,
PropertyNamePatternsMatcher includes, Collection<String> exacts) {
for (String propertyName : exacts) {
if (!source.containsProperty(propertyName)) { if (!source.containsProperty(propertyName)) {
continue; continue;
} }
...@@ -166,11 +155,13 @@ public class PropertySourcesPropertyValues implements PropertyValues { ...@@ -166,11 +155,13 @@ public class PropertySourcesPropertyValues implements PropertyValues {
if (value == null) { if (value == null) {
value = source.getProperty(propertyName.toUpperCase()); value = source.getProperty(propertyName.toUpperCase());
} }
if (value != null && !this.propertyValues.containsKey(propertyName)) { putIfAbsent(propertyName, value);
this.propertyValues.put(propertyName, new PropertyValue(propertyName, }
value)); }
continue;
} private void putIfAbsent(String propertyName, Object value) {
if (value != null && !this.propertyValues.containsKey(propertyName)) {
this.propertyValues.put(propertyName, new PropertyValue(propertyName, value));
} }
} }
......
...@@ -188,4 +188,13 @@ public final class RelaxedNames implements Iterable<String> { ...@@ -188,4 +188,13 @@ public final class RelaxedNames implements Iterable<String> {
} }
} }
/**
* Return a {@link RelaxedNames} for the given source camelCase source name
* @param name the source name in camelCase
* @return the relaxed names
*/
public static RelaxedNames forCamelCase(String name) {
return new RelaxedNames(Manipulation.CAMELCASE_TO_HYPHEN.apply(name));
}
} }
...@@ -28,42 +28,53 @@ import static org.junit.Assert.assertTrue; ...@@ -28,42 +28,53 @@ import static org.junit.Assert.assertTrue;
*/ */
public class DefaultPropertyNamePatternsMatcherTests { public class DefaultPropertyNamePatternsMatcherTests {
private static final char[] DELIMITERS = { '.', '_' };
@Test @Test
public void namesShorter() { public void namesShorter() {
assertFalse(new DefaultPropertyNamePatternsMatcher("aaaa", "bbbb") assertFalse(new DefaultPropertyNamePatternsMatcher(DELIMITERS, "aaaa", "bbbb")
.matches("zzzzz")); .matches("zzzzz"));
} }
@Test @Test
public void namesExactMatch() { public void namesExactMatch() {
assertTrue(new DefaultPropertyNamePatternsMatcher("aaaa", "bbbb", "cccc") assertTrue(new DefaultPropertyNamePatternsMatcher(DELIMITERS, "aaaa", "bbbb",
.matches("bbbb")); "cccc").matches("bbbb"));
} }
@Test @Test
public void namesLonger() { public void namesLonger() {
assertFalse(new DefaultPropertyNamePatternsMatcher("aaaaa", "bbbbb", "ccccc") assertFalse(new DefaultPropertyNamePatternsMatcher(DELIMITERS, "aaaaa", "bbbbb",
.matches("bbbb")); "ccccc").matches("bbbb"));
} }
@Test @Test
public void nameWithDot() throws Exception { public void nameWithDot() throws Exception {
assertTrue(new DefaultPropertyNamePatternsMatcher("aaaa", "bbbb", "cccc") assertTrue(new DefaultPropertyNamePatternsMatcher(DELIMITERS, "aaaa", "bbbb",
.matches("bbbb.anything")); "cccc").matches("bbbb.anything"));
} }
@Test @Test
public void nameWithUnderscore() throws Exception { public void nameWithUnderscore() throws Exception {
assertTrue(new DefaultPropertyNamePatternsMatcher("aaaa", "bbbb", "cccc") assertTrue(new DefaultPropertyNamePatternsMatcher(DELIMITERS, "aaaa", "bbbb",
.matches("bbbb_anything")); "cccc").matches("bbbb_anything"));
} }
@Test @Test
public void namesMatchWithDifferentLengths() throws Exception { public void namesMatchWithDifferentLengths() throws Exception {
assertTrue(new DefaultPropertyNamePatternsMatcher("aaa", "bbbb", "ccccc") assertTrue(new DefaultPropertyNamePatternsMatcher(DELIMITERS, "aaa", "bbbb",
.matches("bbbb")); "ccccc").matches("bbbb"));
}
@Test
public void withSquareBrackets() throws Exception {
char[] delimeters = "._[".toCharArray();
PropertyNamePatternsMatcher matcher = new DefaultPropertyNamePatternsMatcher(
delimeters, "aaa", "bbbb", "ccccc");
assertTrue(matcher.matches("bbbb"));
assertTrue(matcher.matches("bbbb[4]"));
assertFalse(matcher.matches("bbb[4]"));
} }
} }
...@@ -102,6 +102,20 @@ public class RelaxedDataBinderTests { ...@@ -102,6 +102,20 @@ public class RelaxedDataBinderTests {
assertEquals("bar", target.getFoo()); assertEquals("bar", target.getFoo());
} }
@Test
public void testBindToCamelCaseFromEnvironmentStyleWithPrefix() throws Exception {
VanillaTarget target = new VanillaTarget();
bind(target, "TEST_FOO_BAZ: bar", "test");
assertEquals("bar", target.getFooBaz());
}
@Test
public void testBindToCamelCaseFromEnvironmentStyle() throws Exception {
VanillaTarget target = new VanillaTarget();
bind(target, "test.FOO_BAZ: bar", "test");
assertEquals("bar", target.getFooBaz());
}
@Test @Test
public void testBindFromEnvironmentStyleWithNestedPrefix() throws Exception { public void testBindFromEnvironmentStyleWithNestedPrefix() throws Exception {
VanillaTarget target = new VanillaTarget(); VanillaTarget target = new VanillaTarget();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment