Reduce ConfigurationPropertyName garbage

Update the `ConfigurationPropertyNameBuilder` to reduce the number of
objects created.

Fixes gh-9022

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("]");
+		}
+
 	}
 
 	/**

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<Element> elements;
+	private final Map<String, ConfigurationPropertyName> 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<Element> 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<Element> 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<Element> 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<Element> 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<Element> elements) {
-		Assert.state(CollectionUtils.isEmpty(this.elements),
-				"Existing elements must not be present");
-		return new ConfigurationPropertyNameBuilder(this.processor, toList(elements));
+	private ConfigurationPropertyName from(Iterator<Element> 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<Element> toList(Iterator<Element> iterator) {
-		List<Element> 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<Element> 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<Element> 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;
+			};
+		}
+
 	}
 
 }

spring-boot/src/main/java/org/springframework/boot/context/properties/source/DefaultPropertyMapper.java

@@ -65,7 +65,7 @@ class DefaultPropertyMapper implements PropertyMapper {
 	private List<PropertyMapping> tryMap(String propertySourceName) {
 		try {
 			ConfigurationPropertyName convertedName = this.nameBuilder
-					.from(propertySourceName, '.').build();
+					.from(propertySourceName, '.');
 			PropertyMapping o = new PropertyMapping(propertySourceName, convertedName);
 			return Collections.singletonList(o);
 		}

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;

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<Element> getElements(String name) {
-		ConfigurationPropertyNameBuilder builder = this.builder.from(name, '.');
-		return (List<Element>) ReflectionTestUtils.getField(builder, "elements");
+		List<Element> elements = new ArrayList<>();
+		for (Element element : this.builder.from(name, '.')) {
+			elements.add(element);
+		}
+		return elements;
 	}
 
 }