From 5f10c82284dfcebd2eb81a43e4c49e14595ab338 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Thu, 21 Dec 2017 13:01:50 -0800 Subject: [PATCH] Change property mapper to use array returns Update the `PropertyMapper` interface to return arrays rather than Lists. Since implementations are package-private it's possible for us to control how they are used and it helps to save a little memory. Fixes gh-11411 --- .../source/DefaultPropertyMapper.java | 29 ++++++--------- .../properties/source/PropertyMapper.java | 8 ++-- .../SpringConfigurationPropertySource.java | 26 +++++++------ ...ngIterableConfigurationPropertySource.java | 33 +++++++++-------- .../SystemEnvironmentPropertyMapper.java | 37 +++++++------------ .../source/AbstractPropertyMapperTests.java | 5 ++- .../SystemEnvironmentPropertyMapperTests.java | 6 +-- .../properties/source/TestPropertyMapper.java | 20 ++++------ 8 files changed, 73 insertions(+), 91 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/DefaultPropertyMapper.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/DefaultPropertyMapper.java index 47857a321f1..74aff653491 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/DefaultPropertyMapper.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/DefaultPropertyMapper.java @@ -16,9 +16,6 @@ package org.springframework.boot.context.properties.source; -import java.util.Collections; -import java.util.List; - import org.springframework.util.ObjectUtils; /** @@ -43,55 +40,53 @@ final class DefaultPropertyMapper implements PropertyMapper { } @Override - public List map( - ConfigurationPropertyName configurationPropertyName) { + public PropertyMapping[] map(ConfigurationPropertyName configurationPropertyName) { // Use a local copy in case another thread changes things LastMapping last = this.lastMappedConfigurationPropertyName; if (last != null && last.isFrom(configurationPropertyName)) { return last.getMapping(); } String convertedName = configurationPropertyName.toString(); - List mapping = Collections.singletonList( - new PropertyMapping(convertedName, configurationPropertyName)); + PropertyMapping[] mapping = { + new PropertyMapping(convertedName, configurationPropertyName) }; this.lastMappedConfigurationPropertyName = new LastMapping<>( configurationPropertyName, mapping); return mapping; } @Override - public List map(String propertySourceName) { + public PropertyMapping[] map(String propertySourceName) { // Use a local copy in case another thread changes things LastMapping last = this.lastMappedPropertyName; if (last != null && last.isFrom(propertySourceName)) { return last.getMapping(); } - List mapping = tryMap(propertySourceName); + PropertyMapping[] mapping = tryMap(propertySourceName); this.lastMappedPropertyName = new LastMapping<>(propertySourceName, mapping); return mapping; } - private List tryMap(String propertySourceName) { + private PropertyMapping[] tryMap(String propertySourceName) { try { ConfigurationPropertyName convertedName = ConfigurationPropertyName .adapt(propertySourceName, '.'); if (!convertedName.isEmpty()) { - PropertyMapping o = new PropertyMapping(propertySourceName, - convertedName); - return Collections.singletonList(o); + return new PropertyMapping[] { + new PropertyMapping(propertySourceName, convertedName) }; } } catch (Exception ex) { } - return Collections.emptyList(); + return NO_MAPPINGS; } private static class LastMapping { private final T from; - private final List mapping; + private final PropertyMapping[] mapping; - LastMapping(T from, List mapping) { + LastMapping(T from, PropertyMapping[] mapping) { this.from = from; this.mapping = mapping; } @@ -100,7 +95,7 @@ final class DefaultPropertyMapper implements PropertyMapper { return ObjectUtils.nullSafeEquals(from, this.from); } - public List getMapping() { + public PropertyMapping[] getMapping() { return this.mapping; } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/PropertyMapper.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/PropertyMapper.java index 5bd37c757bc..9118860fc97 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/PropertyMapper.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/PropertyMapper.java @@ -16,8 +16,6 @@ package org.springframework.boot.context.properties.source; -import java.util.List; - import org.springframework.core.env.EnumerablePropertySource; import org.springframework.core.env.PropertySource; @@ -40,19 +38,21 @@ import org.springframework.core.env.PropertySource; */ interface PropertyMapper { + PropertyMapping[] NO_MAPPINGS = {}; + /** * Provide mappings from a {@link ConfigurationPropertySource} * {@link ConfigurationPropertyName}. * @param configurationPropertyName the name to map * @return a stream of mappings or {@code Stream#empty()} */ - List map(ConfigurationPropertyName configurationPropertyName); + PropertyMapping[] map(ConfigurationPropertyName configurationPropertyName); /** * Provide mappings from a {@link PropertySource} property name. * @param propertySourceName the name to map * @return a stream of mappings or {@code Stream#empty()} */ - List map(String propertySourceName); + PropertyMapping[] map(String propertySourceName); } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringConfigurationPropertySource.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringConfigurationPropertySource.java index b75bc3080b8..be02b035959 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringConfigurationPropertySource.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringConfigurationPropertySource.java @@ -16,10 +16,7 @@ package org.springframework.boot.context.properties.source; -import java.util.Collections; -import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Random; import java.util.function.Function; @@ -85,7 +82,7 @@ class SpringConfigurationPropertySource implements ConfigurationPropertySource { @Override public ConfigurationProperty getConfigurationProperty( ConfigurationPropertyName name) { - List mappings = getMapper().map(name); + PropertyMapping[] mappings = getMapper().map(name); return find(mappings, name); } @@ -100,10 +97,17 @@ class SpringConfigurationPropertySource implements ConfigurationPropertySource { return this.propertySource; } - protected final ConfigurationProperty find(List mappings, + protected final ConfigurationProperty find(PropertyMapping[] mappings, ConfigurationPropertyName name) { - return mappings.stream().filter((m) -> m.isApplicable(name)).map(this::find) - .filter(Objects::nonNull).findFirst().orElse(null); + for (PropertyMapping candidate : mappings) { + if (candidate.isApplicable(name)) { + ConfigurationProperty result = find(candidate); + if (result != null) { + return result; + } + } + } + return null; } private ConfigurationProperty find(PropertyMapping mapping) { @@ -214,23 +218,23 @@ class SpringConfigurationPropertySource implements ConfigurationPropertySource { } @Override - public List map( + public PropertyMapping[] map( ConfigurationPropertyName configurationPropertyName) { try { return this.mapper.map(configurationPropertyName); } catch (Exception ex) { - return Collections.emptyList(); + return NO_MAPPINGS; } } @Override - public List map(String propertySourceName) { + public PropertyMapping[] map(String propertySourceName) { try { return this.mapper.map(propertySourceName); } catch (Exception ex) { - return Collections.emptyList(); + return NO_MAPPINGS; } } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringIterableConfigurationPropertySource.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringIterableConfigurationPropertySource.java index 21cfad0848d..2faaf570bff 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringIterableConfigurationPropertySource.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SpringIterableConfigurationPropertySource.java @@ -71,7 +71,7 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope ConfigurationProperty configurationProperty = super.getConfigurationProperty( name); if (configurationProperty == null) { - configurationProperty = find(getPropertyMappings(), name); + configurationProperty = find(getPropertyMappings(getCache()), name); } return configurationProperty; } @@ -98,8 +98,8 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope if (names != null) { return names; } - List mappings = getPropertyMappings(); - names = new ArrayList<>(mappings.size()); + PropertyMapping[] mappings = getPropertyMappings(cache); + names = new ArrayList<>(mappings.length); for (PropertyMapping mapping : mappings) { names.add(mapping.getConfigurationPropertyName()); } @@ -110,22 +110,23 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope return names; } - private List getPropertyMappings() { - Cache cache = getCache(); - List mappings = (cache != null ? cache.getMappings() : null); - if (mappings != null) { - return mappings; + private PropertyMapping[] getPropertyMappings(Cache cache) { + PropertyMapping[] result = (cache != null ? cache.getMappings() : null); + if (result != null) { + return result; } String[] names = getPropertySource().getPropertyNames(); - mappings = new ArrayList<>(names.length); + List mappings = new ArrayList<>(names.length * 2); for (String name : names) { - mappings.addAll(getMapper().map(name)); + for (PropertyMapping mapping : getMapper().map(name)) { + mappings.add(mapping); + } } - mappings = Collections.unmodifiableList(mappings); + result = mappings.toArray(new PropertyMapping[mappings.size()]); if (cache != null) { - cache.setMappings(mappings); + cache.setMappings(result); } - return mappings; + return result; } private Cache getCache() { @@ -157,7 +158,7 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope private List names; - private List mappings; + private PropertyMapping[] mappings; public List getNames() { return this.names; @@ -167,11 +168,11 @@ class SpringIterableConfigurationPropertySource extends SpringConfigurationPrope this.names = names; } - public List getMappings() { + public PropertyMapping[] getMappings() { return this.mappings; } - public void setMappings(List mappings) { + public void setMappings(PropertyMapping[] mappings) { this.mappings = mappings; } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SystemEnvironmentPropertyMapper.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SystemEnvironmentPropertyMapper.java index d2069fbe317..74a13a4803e 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SystemEnvironmentPropertyMapper.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SystemEnvironmentPropertyMapper.java @@ -16,11 +16,6 @@ package org.springframework.boot.context.properties.source; -import java.util.ArrayList; -import java.util.Collections; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Set; import java.util.stream.IntStream; import org.springframework.boot.context.properties.source.ConfigurationPropertyName.Form; @@ -31,10 +26,6 @@ import org.springframework.boot.context.properties.source.ConfigurationPropertyN * "{@code .}". For example, "{@code SERVER_PORT}" is mapped to "{@code server.port}". In * addition, numeric elements are mapped to indexes (e.g. "{@code HOST_0}" is mapped to * "{@code host[0]}"). - *

- * List shortcuts (names that end with double underscore) are also supported by this - * mapper. For example, "{@code MY_LIST__=a,b,c}" is mapped to "{@code my.list[0]=a}", - * "{@code my.list[1]=b}", "{@code my.list[2]=c}". * * @author Phillip Webb * @author Madhura Bhave @@ -45,28 +36,26 @@ final class SystemEnvironmentPropertyMapper implements PropertyMapper { public static final PropertyMapper INSTANCE = new SystemEnvironmentPropertyMapper(); - private SystemEnvironmentPropertyMapper() { - } - @Override - public List map( - ConfigurationPropertyName configurationPropertyName) { - Set names = new LinkedHashSet<>(); - names.add(convertName(configurationPropertyName)); - names.add(convertLegacyName(configurationPropertyName)); - List result = new ArrayList<>(); - names.forEach((name) -> result - .add(new PropertyMapping(name, configurationPropertyName))); - return result; + public PropertyMapping[] map(ConfigurationPropertyName configurationPropertyName) { + String name = convertName(configurationPropertyName); + String legacyName = convertLegacyName(configurationPropertyName); + if (name.equals(legacyName)) { + return new PropertyMapping[] { + new PropertyMapping(name, configurationPropertyName) }; + } + return new PropertyMapping[] { + new PropertyMapping(name, configurationPropertyName), + new PropertyMapping(legacyName, configurationPropertyName), }; } @Override - public List map(String propertySourceName) { + public PropertyMapping[] map(String propertySourceName) { ConfigurationPropertyName name = convertName(propertySourceName); if (name == null || name.isEmpty()) { - return Collections.emptyList(); + return NO_MAPPINGS; } - return Collections.singletonList(new PropertyMapping(propertySourceName, name)); + return new PropertyMapping[] { new PropertyMapping(propertySourceName, name) }; } private ConfigurationPropertyName convertName(String propertySourceName) { diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/AbstractPropertyMapperTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/AbstractPropertyMapperTests.java index aab3ec1e4da..bacbcc8e098 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/AbstractPropertyMapperTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/AbstractPropertyMapperTests.java @@ -16,6 +16,7 @@ package org.springframework.boot.context.properties.source; +import java.util.Arrays; import java.util.Iterator; /** @@ -34,7 +35,7 @@ public abstract class AbstractPropertyMapperTests { } protected final Iterator namesFromString(String name, Object value) { - return getMapper().map(name).stream() + return Arrays.stream(getMapper().map(name)) .map((mapping) -> mapping.getConfigurationPropertyName().toString()) .iterator(); } @@ -44,7 +45,7 @@ public abstract class AbstractPropertyMapperTests { } protected final Iterator namesFromConfiguration(String name, String value) { - return getMapper().map(ConfigurationPropertyName.of(name)).stream() + return Arrays.stream(getMapper().map(ConfigurationPropertyName.of(name))) .map(PropertyMapping::getPropertySourceName).iterator(); } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/SystemEnvironmentPropertyMapperTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/SystemEnvironmentPropertyMapperTests.java index 5e52530473a..ed90af79f0c 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/SystemEnvironmentPropertyMapperTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/SystemEnvironmentPropertyMapperTests.java @@ -16,8 +16,6 @@ package org.springframework.boot.context.properties.source; -import java.util.List; - import org.junit.Test; import static org.assertj.core.api.Assertions.assertThat; @@ -61,7 +59,7 @@ public class SystemEnvironmentPropertyMapperTests extends AbstractPropertyMapper @Test public void underscoreShouldNotMapToEmptyString() { - List mappings = getMapper().map("_"); + PropertyMapping[] mappings = getMapper().map("_"); boolean applicable = false; for (PropertyMapping mapping : mappings) { applicable = mapping.isApplicable(ConfigurationPropertyName.of("")); @@ -71,7 +69,7 @@ public class SystemEnvironmentPropertyMapperTests extends AbstractPropertyMapper @Test public void underscoreWithWhitespaceShouldNotMapToEmptyString() { - List mappings = getMapper().map(" _"); + PropertyMapping[] mappings = getMapper().map(" _"); boolean applicable = false; for (PropertyMapping mapping : mappings) { applicable = mapping.isApplicable(ConfigurationPropertyName.of("")); diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/TestPropertyMapper.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/TestPropertyMapper.java index 9033347bcf9..1f9db4427b3 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/TestPropertyMapper.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/source/TestPropertyMapper.java @@ -17,8 +17,6 @@ package org.springframework.boot.context.properties.source; import java.util.Collections; -import java.util.List; -import java.util.function.Function; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; @@ -46,21 +44,17 @@ class TestPropertyMapper implements PropertyMapper { } } - public void addFromConfigurationProperty(ConfigurationPropertyName from, String to, - Function extractor) { - this.fromConfig.add(from, new PropertyMapping(to, from, extractor)); - } - @Override - public List map(String propertySourceName) { - return this.fromSource.getOrDefault(propertySourceName, Collections.emptyList()); + public PropertyMapping[] map(String propertySourceName) { + return this.fromSource.getOrDefault(propertySourceName, Collections.emptyList()) + .toArray(new PropertyMapping[0]); } @Override - public List map( - ConfigurationPropertyName configurationPropertyName) { - return this.fromConfig.getOrDefault(configurationPropertyName, - Collections.emptyList()); + public PropertyMapping[] map(ConfigurationPropertyName configurationPropertyName) { + return this.fromConfig + .getOrDefault(configurationPropertyName, Collections.emptyList()) + .toArray(new PropertyMapping[0]); } }