Drop environment variable "__" list support

Remove support for the `__` environment variable list shortcut in order
to reduce complexity. The shortcut was developed before the new `Binder`
code was fully formed and isn't really necessary now since comma-lists
can be converted automatically.

Fixes gh-11410
pull/11365/merge
Phillip Webb 7 years ago
parent 1ea921bfdd
commit c024313141

@ -19,7 +19,6 @@ package org.springframework.boot.context.properties.source;
import java.util.Collections; import java.util.Collections;
import java.util.List; import java.util.List;
import org.springframework.core.env.PropertySource;
import org.springframework.util.ObjectUtils; import org.springframework.util.ObjectUtils;
/** /**
@ -44,7 +43,7 @@ final class DefaultPropertyMapper implements PropertyMapper {
} }
@Override @Override
public List<PropertyMapping> map(PropertySource<?> propertySource, public List<PropertyMapping> map(
ConfigurationPropertyName configurationPropertyName) { ConfigurationPropertyName configurationPropertyName) {
// Use a local copy in case another thread changes things // Use a local copy in case another thread changes things
LastMapping<ConfigurationPropertyName> last = this.lastMappedConfigurationPropertyName; LastMapping<ConfigurationPropertyName> last = this.lastMappedConfigurationPropertyName;
@ -60,8 +59,7 @@ final class DefaultPropertyMapper implements PropertyMapper {
} }
@Override @Override
public List<PropertyMapping> map(PropertySource<?> propertySource, public List<PropertyMapping> map(String propertySourceName) {
String propertySourceName) {
// Use a local copy in case another thread changes things // Use a local copy in case another thread changes things
LastMapping<String> last = this.lastMappedPropertyName; LastMapping<String> last = this.lastMappedPropertyName;
if (last != null && last.isFrom(propertySourceName)) { if (last != null && last.isFrom(propertySourceName)) {

@ -43,20 +43,16 @@ interface PropertyMapper {
/** /**
* Provide mappings from a {@link ConfigurationPropertySource} * Provide mappings from a {@link ConfigurationPropertySource}
* {@link ConfigurationPropertyName}. * {@link ConfigurationPropertyName}.
* @param propertySource the property source
* @param configurationPropertyName the name to map * @param configurationPropertyName the name to map
* @return a stream of mappings or {@code Stream#empty()} * @return a stream of mappings or {@code Stream#empty()}
*/ */
List<PropertyMapping> map(PropertySource<?> propertySource, List<PropertyMapping> map(ConfigurationPropertyName configurationPropertyName);
ConfigurationPropertyName configurationPropertyName);
/** /**
* Provide mappings from a {@link PropertySource} property name. * Provide mappings from a {@link PropertySource} property name.
* @param propertySource the property source
* @param propertySourceName the name to map * @param propertySourceName the name to map
* @return a stream of mappings or {@code Stream#empty()} * @return a stream of mappings or {@code Stream#empty()}
*/ */
List<PropertyMapping> map(PropertySource<?> propertySource, List<PropertyMapping> map(String propertySourceName);
String propertySourceName);
} }

@ -16,8 +16,6 @@
package org.springframework.boot.context.properties.source; package org.springframework.boot.context.properties.source;
import java.util.function.Function;
import org.springframework.core.env.PropertySource; import org.springframework.core.env.PropertySource;
/** /**
@ -34,8 +32,6 @@ class PropertyMapping {
private final ConfigurationPropertyName configurationPropertyName; private final ConfigurationPropertyName configurationPropertyName;
private final Function<Object, Object> valueExtractor;
/** /**
* Create a new {@link PropertyMapper} instance. * Create a new {@link PropertyMapper} instance.
* @param propertySourceName the {@link PropertySource} name * @param propertySourceName the {@link PropertySource} name
@ -44,22 +40,8 @@ class PropertyMapping {
*/ */
PropertyMapping(String propertySourceName, PropertyMapping(String propertySourceName,
ConfigurationPropertyName configurationPropertyName) { ConfigurationPropertyName configurationPropertyName) {
this(propertySourceName, configurationPropertyName, Function.identity());
}
/**
* Create a new {@link PropertyMapper} instance.
* @param propertySourceName the {@link PropertySource} name
* @param configurationPropertyName the {@link ConfigurationPropertySource}
* {@link ConfigurationPropertyName}
* @param valueExtractor the extractor used to obtain the value
*/
PropertyMapping(String propertySourceName,
ConfigurationPropertyName configurationPropertyName,
Function<Object, Object> valueExtractor) {
this.propertySourceName = propertySourceName; this.propertySourceName = propertySourceName;
this.configurationPropertyName = configurationPropertyName; this.configurationPropertyName = configurationPropertyName;
this.valueExtractor = valueExtractor;
} }
/** /**
@ -81,14 +63,6 @@ class PropertyMapping {
} }
/**
* Return a function that can be used to extract the {@link PropertySource} value.
* @return the value extractor (never {@code null})
*/
public Function<Object, Object> getValueExtractor() {
return this.valueExtractor;
}
/** /**
* Return if this mapping is applicable for the given * Return if this mapping is applicable for the given
* {@link ConfigurationPropertyName}. * {@link ConfigurationPropertyName}.

@ -40,7 +40,7 @@ import org.springframework.util.Assert;
* <p> * <p>
* Each {@link ConfigurationPropertySource#getConfigurationProperty * Each {@link ConfigurationPropertySource#getConfigurationProperty
* getConfigurationProperty} call attempts to * getConfigurationProperty} call attempts to
* {@link PropertyMapper#map(PropertySource, ConfigurationPropertyName) map} the * {@link PropertyMapper#map(ConfigurationPropertyName) map} the
* {@link ConfigurationPropertyName} to one or more {@code String} based names. This * {@link ConfigurationPropertyName} to one or more {@code String} based names. This
* allows fast property resolution for well formed property sources. * allows fast property resolution for well formed property sources.
* <p> * <p>
@ -85,7 +85,7 @@ class SpringConfigurationPropertySource implements ConfigurationPropertySource {
@Override @Override
public ConfigurationProperty getConfigurationProperty( public ConfigurationProperty getConfigurationProperty(
ConfigurationPropertyName name) { ConfigurationPropertyName name) {
List<PropertyMapping> mappings = getMapper().map(getPropertySource(), name); List<PropertyMapping> mappings = getMapper().map(name);
return find(mappings, name); return find(mappings, name);
} }
@ -112,7 +112,6 @@ class SpringConfigurationPropertySource implements ConfigurationPropertySource {
if (value == null) { if (value == null) {
return null; return null;
} }
value = mapping.getValueExtractor().apply(value);
ConfigurationPropertyName configurationPropertyName = mapping ConfigurationPropertyName configurationPropertyName = mapping
.getConfigurationPropertyName(); .getConfigurationPropertyName();
Origin origin = PropertySourceOrigin.get(this.propertySource, propertySourceName); Origin origin = PropertySourceOrigin.get(this.propertySource, propertySourceName);
@ -215,10 +214,10 @@ class SpringConfigurationPropertySource implements ConfigurationPropertySource {
} }
@Override @Override
public List<PropertyMapping> map(PropertySource<?> propertySource, public List<PropertyMapping> map(
ConfigurationPropertyName configurationPropertyName) { ConfigurationPropertyName configurationPropertyName) {
try { try {
return this.mapper.map(propertySource, configurationPropertyName); return this.mapper.map(configurationPropertyName);
} }
catch (Exception ex) { catch (Exception ex) {
return Collections.emptyList(); return Collections.emptyList();
@ -226,10 +225,9 @@ class SpringConfigurationPropertySource implements ConfigurationPropertySource {
} }
@Override @Override
public List<PropertyMapping> map(PropertySource<?> propertySource, public List<PropertyMapping> map(String propertySourceName) {
String propertySourceName) {
try { try {
return this.mapper.map(propertySource, propertySourceName); return this.mapper.map(propertySourceName);
} }
catch (Exception ex) { catch (Exception ex) {
return Collections.emptyList(); return Collections.emptyList();

@ -119,7 +119,7 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope
String[] names = getPropertySource().getPropertyNames(); String[] names = getPropertySource().getPropertyNames();
mappings = new ArrayList<>(names.length); mappings = new ArrayList<>(names.length);
for (String name : names) { for (String name : names) {
mappings.addAll(getMapper().map(getPropertySource(), name)); mappings.addAll(getMapper().map(name));
} }
mappings = Collections.unmodifiableList(mappings); mappings = Collections.unmodifiableList(mappings);
if (cache != null) { if (cache != null) {

@ -21,12 +21,9 @@ import java.util.Collections;
import java.util.LinkedHashSet; import java.util.LinkedHashSet;
import java.util.List; import java.util.List;
import java.util.Set; import java.util.Set;
import java.util.function.Function;
import java.util.stream.IntStream; import java.util.stream.IntStream;
import org.springframework.boot.context.properties.source.ConfigurationPropertyName.Form; import org.springframework.boot.context.properties.source.ConfigurationPropertyName.Form;
import org.springframework.core.env.PropertySource;
import org.springframework.util.StringUtils;
/** /**
* {@link PropertyMapper} for system environment variables. Names are mapped by removing * {@link PropertyMapper} for system environment variables. Names are mapped by removing
@ -52,7 +49,7 @@ final class SystemEnvironmentPropertyMapper implements PropertyMapper {
} }
@Override @Override
public List<PropertyMapping> map(PropertySource<?> propertySource, public List<PropertyMapping> map(
ConfigurationPropertyName configurationPropertyName) { ConfigurationPropertyName configurationPropertyName) {
Set<String> names = new LinkedHashSet<>(); Set<String> names = new LinkedHashSet<>();
names.add(convertName(configurationPropertyName)); names.add(convertName(configurationPropertyName));
@ -60,39 +57,15 @@ final class SystemEnvironmentPropertyMapper implements PropertyMapper {
List<PropertyMapping> result = new ArrayList<>(); List<PropertyMapping> result = new ArrayList<>();
names.forEach((name) -> result names.forEach((name) -> result
.add(new PropertyMapping(name, configurationPropertyName))); .add(new PropertyMapping(name, configurationPropertyName)));
if (isListShortcutPossible(configurationPropertyName)) {
result.addAll(mapListShortcut(propertySource, configurationPropertyName));
}
return result; return result;
} }
private boolean isListShortcutPossible(ConfigurationPropertyName name) {
return (name.isLastElementIndexed() && isNumber(name.getLastElement(Form.UNIFORM))
&& name.getNumberOfElements() >= 1);
}
private List<PropertyMapping> mapListShortcut(PropertySource<?> propertySource,
ConfigurationPropertyName name) {
String result = convertName(name, name.getNumberOfElements() - 1) + "__";
if (propertySource.containsProperty(result)) {
int index = Integer.parseInt(name.getLastElement(Form.UNIFORM));
return Collections.singletonList(
new PropertyMapping(result, name, new ElementExtractor(index)));
}
return Collections.emptyList();
}
@Override @Override
public List<PropertyMapping> map(PropertySource<?> propertySource, public List<PropertyMapping> map(String propertySourceName) {
String propertySourceName) {
ConfigurationPropertyName name = convertName(propertySourceName); ConfigurationPropertyName name = convertName(propertySourceName);
if (name == null || name.isEmpty()) { if (name == null || name.isEmpty()) {
return Collections.emptyList(); return Collections.emptyList();
} }
if (propertySourceName.endsWith("__")) {
return expandListShortcut(propertySourceName, name,
propertySource.getProperty(propertySourceName));
}
return Collections.singletonList(new PropertyMapping(propertySourceName, name)); return Collections.singletonList(new PropertyMapping(propertySourceName, name));
} }
@ -106,22 +79,6 @@ final class SystemEnvironmentPropertyMapper implements PropertyMapper {
} }
} }
private List<PropertyMapping> expandListShortcut(String propertySourceName,
ConfigurationPropertyName rootName, Object value) {
if (value == null) {
return Collections.emptyList();
}
List<PropertyMapping> mappings = new ArrayList<>();
String[] elements = StringUtils
.commaDelimitedListToStringArray(String.valueOf(value));
for (int i = 0; i < elements.length; i++) {
ConfigurationPropertyName name = rootName.append("[" + i + "]");
mappings.add(new PropertyMapping(propertySourceName, name,
new ElementExtractor(i)));
}
return mappings;
}
private String convertName(ConfigurationPropertyName name) { private String convertName(ConfigurationPropertyName name) {
return convertName(name, name.getNumberOfElements()); return convertName(name, name.getNumberOfElements());
} }
@ -159,26 +116,4 @@ final class SystemEnvironmentPropertyMapper implements PropertyMapper {
return !hasNonDigit; return !hasNonDigit;
} }
/**
* Function used to extract an element from a comma list.
*/
private static class ElementExtractor implements Function<Object, Object> {
private final int index;
ElementExtractor(int index) {
this.index = index;
}
@Override
public Object apply(Object value) {
if (value == null) {
return null;
}
return StringUtils
.commaDelimitedListToStringArray(value.toString())[this.index];
}
}
} }

@ -16,12 +16,8 @@
package org.springframework.boot.context.properties.source; package org.springframework.boot.context.properties.source;
import java.util.Collections;
import java.util.Iterator; import java.util.Iterator;
import org.springframework.core.env.MapPropertySource;
import org.springframework.core.env.PropertySource;
/** /**
* Abstract base class for {@link PropertyMapper} tests. * Abstract base class for {@link PropertyMapper} tests.
* *
@ -38,9 +34,7 @@ public abstract class AbstractPropertyMapperTests {
} }
protected final Iterator<String> namesFromString(String name, Object value) { protected final Iterator<String> namesFromString(String name, Object value) {
PropertySource<?> propertySource = new MapPropertySource("test", return getMapper().map(name).stream()
Collections.singletonMap(name, value));
return getMapper().map(propertySource, name).stream()
.map((mapping) -> mapping.getConfigurationPropertyName().toString()) .map((mapping) -> mapping.getConfigurationPropertyName().toString())
.iterator(); .iterator();
} }
@ -50,10 +44,8 @@ public abstract class AbstractPropertyMapperTests {
} }
protected final Iterator<String> namesFromConfiguration(String name, String value) { protected final Iterator<String> namesFromConfiguration(String name, String value) {
PropertySource<?> propertySource = new MapPropertySource("test", return getMapper().map(ConfigurationPropertyName.of(name)).stream()
Collections.singletonMap(name, value)); .map(PropertyMapping::getPropertySourceName).iterator();
return getMapper().map(propertySource, ConfigurationPropertyName.of(name))
.stream().map(PropertyMapping::getPropertySourceName).iterator();
} }
} }

@ -71,20 +71,6 @@ public class SpringConfigurationPropertySourceTests {
assertThat(adapter.getConfigurationProperty(name).getValue()).isEqualTo("value2"); assertThat(adapter.getConfigurationProperty(name).getValue()).isEqualTo("value2");
} }
@Test
public void getValueShouldUseExtractor() {
Map<String, Object> source = new LinkedHashMap<>();
source.put("key", "value");
PropertySource<?> propertySource = new MapPropertySource("test", source);
TestPropertyMapper mapper = new TestPropertyMapper();
ConfigurationPropertyName name = ConfigurationPropertyName.of("my.key");
mapper.addFromConfigurationProperty(name, "key",
(value) -> value.toString().replace("ue", "let"));
SpringConfigurationPropertySource adapter = new SpringConfigurationPropertySource(
propertySource, mapper, null);
assertThat(adapter.getConfigurationProperty(name).getValue()).isEqualTo("vallet");
}
@Test @Test
public void getValueOrigin() { public void getValueOrigin() {
Map<String, Object> source = new LinkedHashMap<>(); Map<String, Object> source = new LinkedHashMap<>();

@ -110,21 +110,6 @@ public class SpringIterableConfigurationPropertySourceTests {
assertThat(adapter.getConfigurationProperty(name).getValue()).isEqualTo("value2"); assertThat(adapter.getConfigurationProperty(name).getValue()).isEqualTo("value2");
} }
@Test
public void getValueShouldUseExtractor() {
Map<String, Object> source = new LinkedHashMap<>();
source.put("key", "value");
EnumerablePropertySource<?> propertySource = new MapPropertySource("test",
source);
TestPropertyMapper mapper = new TestPropertyMapper();
ConfigurationPropertyName name = ConfigurationPropertyName.of("my.key");
mapper.addFromConfigurationProperty(name, "key",
(value) -> value.toString().replace("ue", "let"));
SpringIterableConfigurationPropertySource adapter = new SpringIterableConfigurationPropertySource(
propertySource, mapper);
assertThat(adapter.getConfigurationProperty(name).getValue()).isEqualTo("vallet");
}
@Test @Test
public void getValueOrigin() { public void getValueOrigin() {
Map<String, Object> source = new LinkedHashMap<>(); Map<String, Object> source = new LinkedHashMap<>();

@ -16,16 +16,10 @@
package org.springframework.boot.context.properties.source; package org.springframework.boot.context.properties.source;
import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List; import java.util.List;
import java.util.Map;
import org.junit.Test; import org.junit.Test;
import org.springframework.core.env.MapPropertySource;
import org.springframework.core.env.PropertySource;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
/** /**
@ -50,10 +44,6 @@ public class SystemEnvironmentPropertyMapperTests extends AbstractPropertyMapper
assertThat(namesFromString("HOST_0_NAME")).containsExactly("host[0].name"); assertThat(namesFromString("HOST_0_NAME")).containsExactly("host[0].name");
assertThat(namesFromString("HOST_F00_NAME")).containsExactly("host.f00.name"); assertThat(namesFromString("HOST_F00_NAME")).containsExactly("host.f00.name");
assertThat(namesFromString("S-ERVER")).containsExactly("s-erver"); assertThat(namesFromString("S-ERVER")).containsExactly("s-erver");
assertThat(namesFromString("SERVERS__", "1,2,3")).containsExactly("servers[0]",
"servers[1]", "servers[2]");
assertThat(namesFromString("SERVERS_0__", "1,2,3"))
.containsExactly("servers[0][0]", "servers[0][1]", "servers[0][2]");
} }
@Test @Test
@ -69,44 +59,9 @@ public class SystemEnvironmentPropertyMapperTests extends AbstractPropertyMapper
"FOO_THE_BAR"); "FOO_THE_BAR");
} }
@Test
public void mapFromStringWhenListShortcutShouldExtractValues() {
Map<String, Object> source = new LinkedHashMap<>();
source.put("SERVER__", "foo,bar,baz");
PropertySource<?> propertySource = new MapPropertySource("test", source);
List<PropertyMapping> mappings = getMapper().map(propertySource, "SERVER__");
List<Object> result = new ArrayList<>();
for (PropertyMapping mapping : mappings) {
Object value = propertySource.getProperty(mapping.getPropertySourceName());
value = mapping.getValueExtractor().apply(value);
result.add(value);
}
assertThat(result).containsExactly("foo", "bar", "baz");
}
@Test
public void mapFromConfigurationShouldIncludeShortcutAndExtractValues() {
Map<String, Object> source = new LinkedHashMap<>();
source.put("SERVER__", "foo,bar,baz");
PropertySource<?> propertySource = new MapPropertySource("test", source);
List<PropertyMapping> mappings = getMapper().map(propertySource,
ConfigurationPropertyName.of("server[1]"));
List<Object> result = new ArrayList<>();
for (PropertyMapping mapping : mappings) {
Object value = propertySource.getProperty(mapping.getPropertySourceName());
value = mapping.getValueExtractor().apply(value);
if (value != null) {
result.add(value);
}
}
assertThat(result).containsExactly("bar");
}
@Test @Test
public void underscoreShouldNotMapToEmptyString() { public void underscoreShouldNotMapToEmptyString() {
Map<String, Object> source = new LinkedHashMap<>(); List<PropertyMapping> mappings = getMapper().map("_");
PropertySource<?> propertySource = new MapPropertySource("test", source);
List<PropertyMapping> mappings = getMapper().map(propertySource, "_");
boolean applicable = false; boolean applicable = false;
for (PropertyMapping mapping : mappings) { for (PropertyMapping mapping : mappings) {
applicable = mapping.isApplicable(ConfigurationPropertyName.of("")); applicable = mapping.isApplicable(ConfigurationPropertyName.of(""));
@ -116,9 +71,7 @@ public class SystemEnvironmentPropertyMapperTests extends AbstractPropertyMapper
@Test @Test
public void underscoreWithWhitespaceShouldNotMapToEmptyString() { public void underscoreWithWhitespaceShouldNotMapToEmptyString() {
Map<String, Object> source = new LinkedHashMap<>(); List<PropertyMapping> mappings = getMapper().map(" _");
PropertySource<?> propertySource = new MapPropertySource("test", source);
List<PropertyMapping> mappings = getMapper().map(propertySource, " _");
boolean applicable = false; boolean applicable = false;
for (PropertyMapping mapping : mappings) { for (PropertyMapping mapping : mappings) {
applicable = mapping.isApplicable(ConfigurationPropertyName.of("")); applicable = mapping.isApplicable(ConfigurationPropertyName.of(""));

@ -20,7 +20,6 @@ import java.util.Collections;
import java.util.List; import java.util.List;
import java.util.function.Function; import java.util.function.Function;
import org.springframework.core.env.PropertySource;
import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap; import org.springframework.util.MultiValueMap;
@ -53,13 +52,12 @@ class TestPropertyMapper implements PropertyMapper {
} }
@Override @Override
public List<PropertyMapping> map(PropertySource<?> propertySource, public List<PropertyMapping> map(String propertySourceName) {
String propertySourceName) {
return this.fromSource.getOrDefault(propertySourceName, Collections.emptyList()); return this.fromSource.getOrDefault(propertySourceName, Collections.emptyList());
} }
@Override @Override
public List<PropertyMapping> map(PropertySource<?> propertySource, public List<PropertyMapping> map(
ConfigurationPropertyName configurationPropertyName) { ConfigurationPropertyName configurationPropertyName) {
return this.fromConfig.getOrDefault(configurationPropertyName, return this.fromConfig.getOrDefault(configurationPropertyName,
Collections.emptyList()); Collections.emptyList());

Loading…
Cancel
Save