Make TestPropertyValues immutable

Update `TestPropertyValues` so that it is totally immutable. Methods
now return a new instance rather than changing existing state.

See gh-9875

spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/neo4j/MixedNeo4jRepositoriesAutoConfigurationTests.java

@@ -93,7 +93,7 @@ public class MixedNeo4jRepositoriesAutoConfigurationTests {
 
 	private void load(Class<?> config, String... environment) {
 		AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext();
-		TestPropertyValues.of(environment).and("spring.datasource.initialize", "false")
+		TestPropertyValues.of(environment).and("spring.datasource.initialize=false")
 				.applyTo(context);
 		context.register(config);
 		context.register(DataSourceAutoConfiguration.class,

spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/DataSourceJmxConfigurationTests.java

@@ -130,7 +130,7 @@ public class DataSourceJmxConfigurationTests {
 	private void load(Class<?> config, String... environment) {
 		AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext();
 		String jdbcUrl = "jdbc:hsqldb:mem:test-" + UUID.randomUUID().toString();
-		TestPropertyValues.of(environment).and("spring.datasource.url", jdbcUrl)
+		TestPropertyValues.of(environment).and("spring.datasource.url=" + jdbcUrl)
 				.applyTo(context);
 		if (config != null) {
 			context.register(config);

spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/HikariDataSourceConfigurationTests.java

@@ -104,8 +104,8 @@ public class HikariDataSourceConfigurationTests {
 
 	private void load(String... environment) {
 		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
-		TestPropertyValues.of(environment).and("spring.datasource.initialize", "false")
+		TestPropertyValues.of(environment).and("spring.datasource.initialize=false")
-				.and("spring.datasource.type", HikariDataSource.class.getName())
+				.and("spring.datasource.type=" + HikariDataSource.class.getName())
 				.applyTo(ctx);
 		ctx.register(DataSourceAutoConfiguration.class);
 		ctx.refresh();

spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/AbstractJpaAutoConfigurationTests.java

@@ -213,7 +213,7 @@ public abstract class AbstractJpaAutoConfigurationTests {
 			ctx.setClassLoader(classLoader);
 		}
 		TestPropertyValues.of(environment)
-				.and("spring.datasource.generate-unique-name", "true").applyTo(ctx);
+				.and("spring.datasource.generate-unique-name=true").applyTo(ctx);
 		ctx.register(TestConfiguration.class);
 		if (!ObjectUtils.isEmpty(configs)) {
 			ctx.register(configs);

spring-boot-test/src/main/java/org/springframework/boot/test/context/runner/AbstractApplicationContextRunner.java

@@ -17,7 +17,6 @@
 package org.springframework.boot.test.context.runner;
 
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.List;
 import java.util.function.Supplier;
 
@@ -99,9 +98,9 @@ abstract class AbstractApplicationContextRunner<SELF extends AbstractApplication
 
 	private final Supplier<C> contextFactory;
 
-	private final TestPropertyValues environmentProperties;
+	private TestPropertyValues environmentProperties;
 
-	private final TestPropertyValues systemProperties;
+	private TestPropertyValues systemProperties;
 
 	private ClassLoader classLoader;
 
@@ -131,20 +130,7 @@ abstract class AbstractApplicationContextRunner<SELF extends AbstractApplication
 	 * @see #withSystemProperties(String...)
 	 */
 	public SELF withPropertyValues(String... pairs) {
-		Arrays.stream(pairs).forEach(this.environmentProperties::and);
+		this.environmentProperties = this.environmentProperties.and(pairs);
-		return self();
-	}
-
-	/**
-	 * Add the specified {@link Environment} property.
-	 * @param name the name of the property
-	 * @param value the value of the property
-	 * @return this instance
-	 * @see TestPropertyValues
-	 * @see #withSystemProperties(String...)
-	 */
-	public SELF withPropertyValue(String name, String value) {
-		this.environmentProperties.and(name, value);
 		return self();
 	}
 
@@ -159,22 +145,7 @@ abstract class AbstractApplicationContextRunner<SELF extends AbstractApplication
 	 * @see #withSystemProperties(String...)
 	 */
 	public SELF withSystemProperties(String... pairs) {
-		Arrays.stream(pairs).forEach(this.systemProperties::and);
+		this.systemProperties = this.systemProperties.and(pairs);
-		return self();
-	}
-
-	/**
-	 * Add the specified {@link System} property. System properties are added before the
-	 * context is {@link #run(ContextConsumer) run} and restored when the context is
-	 * closed.
-	 * @param name the property name
-	 * @param value the property value
-	 * @return this instance
-	 * @see TestPropertyValues
-	 * @see #withSystemProperties(String...)
-	 */
-	public SELF withSystemProperty(String name, String value) {
-		this.systemProperties.and(name, value);
 		return self();
 	}
 

spring-boot-test/src/main/java/org/springframework/boot/test/util/TestPropertyValues.java

@@ -17,9 +17,13 @@
 package org.springframework.boot.test.util;
 
 import java.io.Closeable;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.Map;
+import java.util.Objects;
 import java.util.concurrent.Callable;
+import java.util.stream.Stream;
 
 import com.google.common.collect.Streams;
 
@@ -45,56 +49,28 @@ import org.springframework.util.StringUtils;
  */
 public final class TestPropertyValues {
 
-	private final Map<String, Object> properties = new LinkedHashMap<>();
+	private static final TestPropertyValues EMPTY = new TestPropertyValues(
+			Collections.emptyMap());
 
-	private TestPropertyValues(String[] pairs) {
+	private final Map<String, Object> properties;
-		addProperties(pairs);
-	}
 
-	private void addProperties(String[] pairs) {
+	private TestPropertyValues(Map<String, Object> properties) {
-		for (String pair : pairs) {
+		this.properties = Collections.unmodifiableMap(properties);
-			and(pair);
-		}
 	}
 
 	/**
-	 * Builder method to append another property pair the underlying map of properties.
+	 * Builder method to add more properties.
-	 * @param pair The property pair to add
+	 * @param pairs The property pairs to add
-	 * @return the existing instance of {@link TestPropertyValues}
+	 * @return a new {@link TestPropertyValues} instance
 	 */
-	public TestPropertyValues and(String pair) {
+	public TestPropertyValues and(String... pairs) {
-		int index = getSeparatorIndex(pair);
+		return and(Arrays.stream(pairs).map(Pair::parse));
-		String key = pair.substring(0, index > 0 ? index : pair.length());
-		String value = index > 0 ? pair.substring(index + 1) : "";
-		and(key.trim(), value.trim());
-		return this;
 	}
 
-	private int getSeparatorIndex(String pair) {
+	private TestPropertyValues and(Stream<Pair> pairs) {
-		int colonIndex = pair.indexOf(":");
+		Map<String, Object> properties = new LinkedHashMap<>(this.properties);
-		int equalIndex = pair.indexOf("=");
+		pairs.filter(Objects::nonNull).forEach((pair) -> pair.addTo(properties));
-		if (colonIndex == -1) {
+		return new TestPropertyValues(properties);
-			return equalIndex;
-		}
-		if (equalIndex == -1) {
-			return colonIndex;
-		}
-		return Math.min(colonIndex, equalIndex);
-	}
-
-	/**
-	 * Builder method to append another property to the underlying map of properties.
-	 * @param name The property name
-	 * @param value The property value
-	 * @return the existing instance of {@link TestPropertyValues}
-	 */
-	public TestPropertyValues and(String name, String value) {
-		if (StringUtils.isEmpty(name) && StringUtils.isEmpty(value)) {
-			return this;
-		}
-		Assert.hasLength(name, "Name must not be empty");
-		this.properties.put(name, value);
-		return this;
 	}
 
 	/**
@@ -170,30 +146,21 @@ public final class TestPropertyValues {
 				return;
 			}
 		}
-		MapPropertySource source = (type.equals(Type.MAP)
+		Map<String, Object> source = new LinkedHashMap<>(this.properties);
-				? new MapPropertySource(name, this.properties)
+		sources.addFirst((type.equals(Type.MAP) ? new MapPropertySource(name, source)
-				: new SystemEnvironmentPropertySource(name, this.properties));
+				: new SystemEnvironmentPropertySource(name, source)));
-		sources.addFirst(source);
-	}
-
-	/**
-	 * Return a new empty {@link TestPropertyValues} instance.
-	 * @return an empty instance
-	 */
-	public static TestPropertyValues empty() {
-		return of();
 	}
 
 	/**
 	 * Return a new {@link TestPropertyValues} with the underlying map populated with the
-	 * given property pair.
+	 * given property pairs. Name-value pairs can be specified with colon (":") or equals
-	 * @param name the property name
+	 * ("=") separators.
-	 * @param value the property value
+	 * @param pairs The key value pairs for properties that need to be added to the
+	 * environment
 	 * @return the new instance
 	 */
-
+	public static TestPropertyValues of(String... pairs) {
-	public static TestPropertyValues ofPair(String name, String value) {
+		return of(Stream.of(pairs));
-		return of().and(name, value);
 	}
 
 	/**
@@ -206,9 +173,9 @@ public final class TestPropertyValues {
 	 */
 	public static TestPropertyValues of(Iterable<String> pairs) {
 		if (pairs == null) {
-			return of();
+			return empty();
 		}
-		return of(Streams.stream(pairs).toArray(String[]::new));
+		return of(Streams.stream(pairs));
 	}
 
 	/**
@@ -219,8 +186,19 @@ public final class TestPropertyValues {
 	 * environment
 	 * @return the new instance
 	 */
-	public static TestPropertyValues of(String... pairs) {
+	public static TestPropertyValues of(Stream<String> pairs) {
-		return new TestPropertyValues(pairs);
+		if (pairs == null) {
+			return empty();
+		}
+		return empty().and(pairs.map(Pair::parse));
+	}
+
+	/**
+	 * Return a new empty {@link TestPropertyValues} instance.
+	 * @return an empty instance
+	 */
+	public static TestPropertyValues empty() {
+		return EMPTY;
 	}
 
 	/**
@@ -250,6 +228,53 @@ public final class TestPropertyValues {
 
 	}
 
+	/**
+	 * A single name value pair.
+	 */
+	public static class Pair {
+
+		private String name;
+
+		private String value;
+
+		public Pair(String name, String value) {
+			Assert.hasLength(name, "Name must not be empty");
+			this.name = name;
+			this.value = value;
+		}
+
+		public void addTo(Map<String, Object> properties) {
+			properties.put(this.name, this.value);
+		}
+
+		public static Pair parse(String pair) {
+			int index = getSeparatorIndex(pair);
+			String key = pair.substring(0, index > 0 ? index : pair.length());
+			String value = index > 0 ? pair.substring(index + 1) : "";
+			return of(key.trim(), value.trim());
+		}
+
+		private static int getSeparatorIndex(String pair) {
+			int colonIndex = pair.indexOf(":");
+			int equalIndex = pair.indexOf("=");
+			if (colonIndex == -1) {
+				return equalIndex;
+			}
+			if (equalIndex == -1) {
+				return colonIndex;
+			}
+			return Math.min(colonIndex, equalIndex);
+		}
+
+		private static Pair of(String name, String value) {
+			if (StringUtils.isEmpty(name) && StringUtils.isEmpty(value)) {
+				return null;
+			}
+			return new Pair(name, value);
+		}
+
+	}
+
 	/**
 	 * Handler to apply and restore system properties.
 	 */
@@ -278,7 +303,7 @@ public final class TestPropertyValues {
 
 		private String setOrClear(String name, String value) {
 			Assert.notNull(name, "Name must not be null");
-			if (value == null) {
+			if (StringUtils.isEmpty(value)) {
 				return (String) System.getProperties().remove(name);
 			}
 			return (String) System.getProperties().setProperty(name, value);

spring-boot-test/src/test/java/org/springframework/boot/test/context/runner/AbstractApplicationContextRunnerTests.java

@@ -95,7 +95,7 @@ public abstract class AbstractApplicationContextRunnerTests<T extends AbstractAp
 		System.setProperty(key, "value");
 		try {
 			assertThat(System.getProperties().getProperty(key)).isEqualTo("value");
-			get().withSystemProperty(key, null).run((loaded) -> {
+			get().withSystemProperties(key + "=").run((loaded) -> {
 				assertThat(System.getProperties()).doesNotContainKey(key);
 			});
 			assertThat(System.getProperties().getProperty(key)).isEqualTo("value");

spring-boot-test/src/test/java/org/springframework/boot/test/util/TestPropertyValuesTests.java

@@ -89,8 +89,8 @@ public class TestPropertyValuesTests {
 
 	@Test
 	public void andShouldChainAndAddSingleKeyValue() throws Exception {
-		TestPropertyValues.of("foo.bar=baz").and("hello.world", "hi")
+		TestPropertyValues.of("foo.bar=baz").and("hello.world=hi").and("bling.blah=bing")
-				.and("bling.blah", "bing").applyTo(this.environment, Type.MAP);
+				.applyTo(this.environment, Type.MAP);
 		assertThat(this.environment.getProperty("foo.bar")).isEqualTo("baz");
 		assertThat(this.environment.getProperty("hello.world")).isEqualTo("hi");
 		assertThat(this.environment.getProperty("bling.blah")).isEqualTo("bing");
@@ -127,7 +127,7 @@ public class TestPropertyValuesTests {
 			throws Exception {
 		System.setProperty("foo", "bar1");
 		try {
-			TestPropertyValues.ofPair("foo", null).applyToSystemProperties(() -> {
+			TestPropertyValues.of("foo").applyToSystemProperties(() -> {
 				assertThat(System.getProperties()).doesNotContainKey("foo");
 				return null;
 			});