diff --git a/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyName.java b/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyName.java index f174b08f3b..c0f19d972c 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyName.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyName.java @@ -23,6 +23,7 @@ import java.util.regex.Pattern; import java.util.stream.Stream; import org.springframework.boot.context.properties.source.ConfigurationPropertyName.Element; +import org.springframework.boot.context.properties.source.ConfigurationPropertyNameBuilder.ElementValueProcessor; import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; @@ -62,7 +63,7 @@ public final class ConfigurationPropertyName null, new Element()); private static final ConfigurationPropertyNameBuilder BUILDER = new ConfigurationPropertyNameBuilder( - Pattern.compile("[a-z]([a-z0-9\\-])*")); + ElementValueProcessor.identity().withValidName()); private final ConfigurationPropertyName parent; @@ -72,7 +73,7 @@ public final class ConfigurationPropertyName ConfigurationPropertyName(ConfigurationPropertyName parent, Element element) { Assert.notNull(element, "Element must not be null"); - this.parent = parent; + this.parent = (parent == EMPTY ? null : parent); this.element = element; } @@ -207,7 +208,7 @@ public final class ConfigurationPropertyName * @return a new {@link ConfigurationPropertyName} */ public ConfigurationPropertyName appendIndex(int index) { - return append("[" + index + "]"); + return BUILDER.from(this, index); } /** @@ -217,7 +218,7 @@ public final class ConfigurationPropertyName */ public ConfigurationPropertyName append(String element) { if (StringUtils.hasLength(element)) { - return BUILDER.from(this).append(element).build(); + return BUILDER.from(this, element); } return this; } @@ -236,7 +237,7 @@ public final class ConfigurationPropertyName if (StringUtils.isEmpty(name)) { return EMPTY; } - return BUILDER.from(name, '.').build(); + return BUILDER.from(name, '.'); } /** @@ -257,7 +258,7 @@ public final class ConfigurationPropertyName Element(String value) { Assert.notNull(value, "Value must not be null"); - this.indexed = (value.startsWith("[") && value.endsWith("]")); + this.indexed = isIndexed(value); value = (this.indexed ? value.substring(1, value.length() - 1) : value); if (!this.indexed) { validate(value); @@ -331,6 +332,10 @@ public final class ConfigurationPropertyName return this.value[form.ordinal()]; } + public static boolean isIndexed(String value) { + return value.startsWith("[") && value.endsWith("]"); + } + } /** diff --git a/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameBuilder.java b/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameBuilder.java index 2738fc840d..4cbad8d8e4 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameBuilder.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameBuilder.java @@ -17,16 +17,15 @@ package org.springframework.boot.context.properties.source; import java.util.ArrayList; -import java.util.Collections; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Objects; -import java.util.regex.Pattern; import org.springframework.boot.context.properties.source.ConfigurationPropertyName.Element; import org.springframework.boot.context.properties.source.ConfigurationPropertyName.Form; import org.springframework.util.Assert; -import org.springframework.util.CollectionUtils; +import org.springframework.util.ConcurrentReferenceHashMap; /** * Builder class that can be used to create {@link ConfigurationPropertyName @@ -39,64 +38,40 @@ import org.springframework.util.CollectionUtils; * * @author Phillip Webb * @author Madhura Bhave - * @since 2.0.0 * @see ConfigurationPropertyName */ -public class ConfigurationPropertyNameBuilder { +class ConfigurationPropertyNameBuilder { + + private static final Element INDEX_ZERO_ELEMENT = new Element("[0]"); private final ElementValueProcessor processor; - private final List elements; + private final Map nameCache = new ConcurrentReferenceHashMap<>(); - /** - * Create a new {@link ConfigurationPropertyNameBuilder} instance. - */ - public ConfigurationPropertyNameBuilder() { - this(ElementValueProcessor.empty()); - } - - /** - * Create a new {@link ConfigurationPropertyNameBuilder} instance that enforces a - * {@link Pattern} on all element values. - * @param elementValuePattern the element value pattern to enforce - */ - public ConfigurationPropertyNameBuilder(Pattern elementValuePattern) { - this(ElementValueProcessor.empty().withPatternCheck(elementValuePattern)); + ConfigurationPropertyNameBuilder() { + this(ElementValueProcessor.identity()); } - /** - * Create a new {@link ConfigurationPropertyNameBuilder} with the specified - * {@link ElementValueProcessor}. - * @param processor the element value processor. - */ - public ConfigurationPropertyNameBuilder(ElementValueProcessor processor) { + ConfigurationPropertyNameBuilder(ElementValueProcessor processor) { Assert.notNull(processor, "Processor must not be null"); this.processor = processor; - this.elements = Collections.emptyList(); - } - - /** - * Internal constructor used to create new builders. - * @param processor the element value processor. - * @param elements the elements built so far - */ - private ConfigurationPropertyNameBuilder(ElementValueProcessor processor, - List elements) { - this.processor = processor; - this.elements = elements; } /** - * Start building using the specified name split up into elements using a known - * separator. For example {@code from("foo.bar", '.')} will return a new builder - * containing the elements "{@code foo}" and "{@code bar}". Any element in square - * brackets will be considered "indexed" and will not be considered for splitting. + * Build using the specified name split up into elements using a known separator. For + * example {@code from("foo.bar", '.')} will return a new builder containing the + * elements "{@code foo}" and "{@code bar}". Any element in square brackets will be + * considered "indexed" and will not be considered for splitting. * @param name the name build from * @param separator the separator * @return a builder with elements populated from the name */ - public ConfigurationPropertyNameBuilder from(String name, char separator) { + public ConfigurationPropertyName from(String name, char separator) { Assert.notNull(name, "Name must not be null"); + ConfigurationPropertyName result = this.nameCache.get(name); + if (result != null) { + return result; + } List elements = new ArrayList<>(); StringBuilder value = new StringBuilder(name.length()); boolean indexed = false; @@ -124,8 +99,10 @@ public class ConfigurationPropertyNameBuilder { } } addElement(elements, value); - return from(elements.stream().filter(Objects::nonNull) + result = from(elements.stream().filter(Objects::nonNull) .filter((e) -> !e.getValue(Form.UNIFORM).isEmpty()).iterator()); + this.nameCache.put(name, result); + return result; } private void addElement(List elements, StringBuilder value) { @@ -135,74 +112,32 @@ public class ConfigurationPropertyNameBuilder { } } - /** - * Return a new {@link ConfigurationPropertyNameBuilder} starting with the specified - * elements. - * @param elements the elements that the new builder should contain - * @return a new initialized builder instance - */ - public ConfigurationPropertyNameBuilder from(Iterable elements) { - return from(elements.iterator()); - } - - /** - * Return a new {@link ConfigurationPropertyNameBuilder} starting with the specified - * elements. - * @param elements the elements that the new builder should contain - * @return a new initialized builder instance - */ - public ConfigurationPropertyNameBuilder from(Iterator elements) { - Assert.state(CollectionUtils.isEmpty(this.elements), - "Existing elements must not be present"); - return new ConfigurationPropertyNameBuilder(this.processor, toList(elements)); + private ConfigurationPropertyName from(Iterator elements) { + ConfigurationPropertyName name = null; + while (elements.hasNext()) { + name = new ConfigurationPropertyName(name, elements.next()); + } + Assert.state(name != null, "At least one element must be defined"); + return name; } - private List toList(Iterator iterator) { - List list = new ArrayList<>(); - while (iterator.hasNext()) { - list.add(iterator.next()); - } - if (isRoot(list)) { - return Collections.emptyList(); + public ConfigurationPropertyName from(ConfigurationPropertyName parent, int index) { + if (index == 0) { + return new ConfigurationPropertyName(parent, INDEX_ZERO_ELEMENT); } - return list; - } + return from(parent, "[" + index + "]"); - private boolean isRoot(List list) { - return (list.size() == 1 && list.get(0).getValue(Form.ORIGINAL).isEmpty()); } - /** - * Return a new builder containing the elements built so far appended with the - * specified element value. - * @param elementValue the element value to append - * @return a new builder instance - */ - public ConfigurationPropertyNameBuilder append(String elementValue) { - Assert.notNull(elementValue, "ElementValue must not be null"); - List elements = new ArrayList<>(this.elements); - elements.add(buildElement(elementValue)); - return new ConfigurationPropertyNameBuilder(this.processor, elements); + public ConfigurationPropertyName from(ConfigurationPropertyName parent, + String elementValue) { + return new ConfigurationPropertyName(parent, buildElement(elementValue)); } private Element buildElement(String value) { return new Element(this.processor.apply(value)); } - /** - * Build a new {@link ConfigurationPropertyName} from the elements contained in this - * builder. - * @return a new {@link ConfigurationPropertyName}. - */ - public ConfigurationPropertyName build() { - ConfigurationPropertyName name = null; - for (Element element : this.elements) { - name = new ConfigurationPropertyName(name, element); - } - Assert.state(name != null, "At least one element must be defined"); - return name; - } - /** * An processor that will be applied to element values. Can be used to manipulate or * restrict the values that are used. @@ -218,32 +153,37 @@ public class ConfigurationPropertyNameBuilder { */ String apply(String value) throws RuntimeException; - /** - * Extend this processor with a {@link Pattern} regular expression check. - * @param pattern the patter to check - * @return an element processor that additionally checks against the pattern - */ - default ElementValueProcessor withPatternCheck(Pattern pattern) { - Assert.notNull(pattern, "Pattern must not be null"); - return (value) -> { - value = apply(value); - Element element = new Element(value); - Assert.isTrue(element.isIndexed() || pattern.matcher(value).matches(), - "Element value '" + value + "' is not valid (" + pattern - + " does not match)"); - return value; - }; - } - /** * Return an empty {@link ElementValueProcessor} that simply returns the original * value unchanged. * @return an empty {@link ElementValueProcessor}. */ - static ElementValueProcessor empty() { + static ElementValueProcessor identity() { return (value) -> value; } + /** + * Extend this processor with a to enforce standard element name rules. + * @return an element processor that additionally enforces a valid name + */ + default ElementValueProcessor withValidName() { + return (value) -> { + value = apply(value); + if (!Element.isIndexed(value)) { + for (int i = 0; i < value.length(); i++) { + char ch = value.charAt(i); + boolean isAlpha = ch >= 'a' && ch <= 'z'; + boolean isNumeric = ch >= '0' && ch <= '9'; + if (i == 0 && !isAlpha || !(isAlpha || isNumeric || ch == '-')) { + throw new IllegalArgumentException( + "Element value '" + value + "' is not valid"); + } + } + } + return value; + }; + } + } } diff --git a/spring-boot/src/main/java/org/springframework/boot/context/properties/source/DefaultPropertyMapper.java b/spring-boot/src/main/java/org/springframework/boot/context/properties/source/DefaultPropertyMapper.java index 00360a81cd..bb60f04a9b 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/properties/source/DefaultPropertyMapper.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/properties/source/DefaultPropertyMapper.java @@ -65,7 +65,7 @@ class DefaultPropertyMapper implements PropertyMapper { private List tryMap(String propertySourceName) { try { ConfigurationPropertyName convertedName = this.nameBuilder - .from(propertySourceName, '.').build(); + .from(propertySourceName, '.'); PropertyMapping o = new PropertyMapping(propertySourceName, convertedName); return Collections.singletonList(o); } diff --git a/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SystemEnvironmentPropertyMapper.java b/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SystemEnvironmentPropertyMapper.java index 73b51ed7e4..5389e2ff9f 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SystemEnvironmentPropertyMapper.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/properties/source/SystemEnvironmentPropertyMapper.java @@ -66,7 +66,7 @@ class SystemEnvironmentPropertyMapper implements PropertyMapper { private ConfigurationPropertyName convertName(String propertySourceName) { try { - return this.nameBuilder.from(propertySourceName, '_').build(); + return this.nameBuilder.from(propertySourceName, '_'); } catch (Exception ex) { return null; diff --git a/spring-boot/src/test/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameBuilderTests.java b/spring-boot/src/test/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameBuilderTests.java index 9ccba0c0d9..cbbf99cff8 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameBuilderTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameBuilderTests.java @@ -16,9 +16,9 @@ package org.springframework.boot.context.properties.source; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import java.util.regex.Pattern; import java.util.stream.Collectors; import org.junit.Rule; @@ -27,7 +27,6 @@ import org.junit.rules.ExpectedException; import org.springframework.boot.context.properties.source.ConfigurationPropertyName.Element; import org.springframework.boot.context.properties.source.ConfigurationPropertyNameBuilder.ElementValueProcessor; -import org.springframework.test.util.ReflectionTestUtils; import static org.assertj.core.api.Assertions.assertThat; @@ -44,13 +43,6 @@ public class ConfigurationPropertyNameBuilderTests { private ConfigurationPropertyNameBuilder builder; - @Test - public void createWhenPatternIsNullShouldThrowException() throws Exception { - this.thrown.expect(IllegalArgumentException.class); - this.thrown.expectMessage("Pattern must not be null"); - this.builder = new ConfigurationPropertyNameBuilder((Pattern) null); - } - @Test public void createWhenElementProcessorIsNullShouldThrowException() throws Exception { this.thrown.expect(IllegalArgumentException.class); @@ -62,39 +54,31 @@ public class ConfigurationPropertyNameBuilderTests { public void buildShouldCreateName() throws Exception { this.builder = new ConfigurationPropertyNameBuilder(); ConfigurationPropertyName expected = ConfigurationPropertyName.of("foo.bar.baz"); - ConfigurationPropertyName name = this.builder.from("foo.bar.baz", '.').build(); + ConfigurationPropertyName name = this.builder.from("foo.bar.baz", '.'); assertThat(name.toString()).isEqualTo(expected.toString()); } @Test - public void buildShouldValidateUsingPattern() { - Pattern pattern = Pattern.compile("[a-z]([a-z0-9\\-])*"); - this.builder = new ConfigurationPropertyNameBuilder(pattern); + public void buildShouldValidateProcessor() { + this.builder = new ConfigurationPropertyNameBuilder( + ElementValueProcessor.identity().withValidName()); this.thrown.expect(IllegalArgumentException.class); this.thrown.expectMessage("Element value 'foo@!' is not valid"); - this.builder.from("foo@!.bar", '.').build(); - } - - @Test - public void buildWhenHasNoElementsShouldThrowException() throws Exception { - this.builder = new ConfigurationPropertyNameBuilder(); - this.thrown.expect(IllegalStateException.class); - this.thrown.expectMessage("At least one element must be defined"); - this.builder.build(); + this.builder.from("foo@!.bar", '.'); } @Test public void buildShouldUseElementProcessor() throws Exception { this.builder = new ConfigurationPropertyNameBuilder( value -> value.replace("-", "")); - ConfigurationPropertyName name = this.builder.from("FOO_THE-BAR", '_').build(); + ConfigurationPropertyName name = this.builder.from("FOO_THE-BAR", '_'); assertThat(name.toString()).isEqualTo("foo.thebar"); } @Test public void fromNameShouldSetElements() throws Exception { this.builder = new ConfigurationPropertyNameBuilder(); - ConfigurationPropertyName name = this.builder.from("foo.bar", '.').build(); + ConfigurationPropertyName name = this.builder.from("foo.bar", '.'); assertThat(name.toString()).isEqualTo("foo.bar"); } @@ -109,19 +93,11 @@ public class ConfigurationPropertyNameBuilderTests { .isEqualTo(elements("foo", "[bar]", "baz")); } - @Test - public void fromNameWhenHasExistingShouldSetNewElements() throws Exception { - this.thrown.expect(IllegalStateException.class); - this.thrown.expectMessage("Existing elements must not be present"); - new ConfigurationPropertyNameBuilder().from("foo.bar", '.').from("baz", '.') - .build(); - } - @Test public void appendShouldAppendElement() throws Exception { this.builder = new ConfigurationPropertyNameBuilder(); - ConfigurationPropertyName name = this.builder.from("foo.bar", '.').append("baz") - .build(); + ConfigurationPropertyName parent = this.builder.from("foo.bar", '.'); + ConfigurationPropertyName name = this.builder.from(parent, "baz"); assertThat(name.toString()).isEqualTo("foo.bar.baz"); } @@ -129,10 +105,12 @@ public class ConfigurationPropertyNameBuilderTests { return Arrays.stream(elements).map(Element::new).collect(Collectors.toList()); } - @SuppressWarnings("unchecked") private List getElements(String name) { - ConfigurationPropertyNameBuilder builder = this.builder.from(name, '.'); - return (List) ReflectionTestUtils.getField(builder, "elements"); + List elements = new ArrayList<>(); + for (Element element : this.builder.from(name, '.')) { + elements.add(element); + } + return elements; } }