From ad9f28110c7d2c2e73e8a98ae5f68d8f11c64660 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Wed, 26 Jul 2017 15:38:58 -0700 Subject: [PATCH] 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 --- ...o4jRepositoriesAutoConfigurationTests.java | 2 +- .../jdbc/DataSourceJmxConfigurationTests.java | 2 +- .../HikariDataSourceConfigurationTests.java | 4 +- .../AbstractJpaAutoConfigurationTests.java | 2 +- .../AbstractApplicationContextRunner.java | 37 +---- .../boot/test/util/TestPropertyValues.java | 155 ++++++++++-------- ...AbstractApplicationContextRunnerTests.java | 2 +- .../test/util/TestPropertyValuesTests.java | 6 +- 8 files changed, 103 insertions(+), 107 deletions(-) diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/neo4j/MixedNeo4jRepositoriesAutoConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/neo4j/MixedNeo4jRepositoriesAutoConfigurationTests.java index 3c66694189..a6da511cc8 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/neo4j/MixedNeo4jRepositoriesAutoConfigurationTests.java +++ b/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, diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/DataSourceJmxConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/DataSourceJmxConfigurationTests.java index 1bdac7960f..20d13f518b 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/DataSourceJmxConfigurationTests.java +++ b/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); diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/HikariDataSourceConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/HikariDataSourceConfigurationTests.java index 6c45a83e5a..087507e7ad 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/HikariDataSourceConfigurationTests.java +++ b/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") - .and("spring.datasource.type", HikariDataSource.class.getName()) + TestPropertyValues.of(environment).and("spring.datasource.initialize=false") + .and("spring.datasource.type=" + HikariDataSource.class.getName()) .applyTo(ctx); ctx.register(DataSourceAutoConfiguration.class); ctx.refresh(); diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/AbstractJpaAutoConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/AbstractJpaAutoConfigurationTests.java index 9fa650b48b..f57abf69ad 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/AbstractJpaAutoConfigurationTests.java +++ b/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); diff --git a/spring-boot-test/src/main/java/org/springframework/boot/test/context/runner/AbstractApplicationContextRunner.java b/spring-boot-test/src/main/java/org/springframework/boot/test/context/runner/AbstractApplicationContextRunner.java index 270beff416..f22f6f7a81 100644 --- a/spring-boot-test/src/main/java/org/springframework/boot/test/context/runner/AbstractApplicationContextRunner.java +++ b/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 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 properties = new LinkedHashMap<>(); + private static final TestPropertyValues EMPTY = new TestPropertyValues( + Collections.emptyMap()); - private TestPropertyValues(String[] pairs) { - addProperties(pairs); - } + private final Map properties; - private void addProperties(String[] pairs) { - for (String pair : pairs) { - and(pair); - } + private TestPropertyValues(Map properties) { + this.properties = Collections.unmodifiableMap(properties); } /** - * Builder method to append another property pair the underlying map of properties. - * @param pair The property pair to add - * @return the existing instance of {@link TestPropertyValues} + * Builder method to add more properties. + * @param pairs The property pairs to add + * @return a new {@link TestPropertyValues} instance */ - public TestPropertyValues and(String pair) { - int index = getSeparatorIndex(pair); - 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; + public TestPropertyValues and(String... pairs) { + return and(Arrays.stream(pairs).map(Pair::parse)); } - private 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); - } - - /** - * 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; + private TestPropertyValues and(Stream pairs) { + Map properties = new LinkedHashMap<>(this.properties); + pairs.filter(Objects::nonNull).forEach((pair) -> pair.addTo(properties)); + return new TestPropertyValues(properties); } /** @@ -170,30 +146,21 @@ public final class TestPropertyValues { return; } } - MapPropertySource source = (type.equals(Type.MAP) - ? new MapPropertySource(name, this.properties) - : new SystemEnvironmentPropertySource(name, this.properties)); - sources.addFirst(source); - } - - /** - * Return a new empty {@link TestPropertyValues} instance. - * @return an empty instance - */ - public static TestPropertyValues empty() { - return of(); + Map source = new LinkedHashMap<>(this.properties); + sources.addFirst((type.equals(Type.MAP) ? new MapPropertySource(name, source) + : new SystemEnvironmentPropertySource(name, source))); } /** * Return a new {@link TestPropertyValues} with the underlying map populated with the - * given property pair. - * @param name the property name - * @param value the property value + * given property pairs. Name-value pairs can be specified with colon (":") or equals + * ("=") separators. + * @param pairs The key value pairs for properties that need to be added to the + * environment * @return the new instance */ - - public static TestPropertyValues ofPair(String name, String value) { - return of().and(name, value); + public static TestPropertyValues of(String... pairs) { + return of(Stream.of(pairs)); } /** @@ -206,9 +173,9 @@ public final class TestPropertyValues { */ public static TestPropertyValues of(Iterable 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) { - return new TestPropertyValues(pairs); + public static TestPropertyValues of(Stream 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 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); diff --git a/spring-boot-test/src/test/java/org/springframework/boot/test/context/runner/AbstractApplicationContextRunnerTests.java b/spring-boot-test/src/test/java/org/springframework/boot/test/context/runner/AbstractApplicationContextRunnerTests.java index 835ca1576a..ce0d95e36f 100644 --- a/spring-boot-test/src/test/java/org/springframework/boot/test/context/runner/AbstractApplicationContextRunnerTests.java +++ b/spring-boot-test/src/test/java/org/springframework/boot/test/context/runner/AbstractApplicationContextRunnerTests.java @@ -95,7 +95,7 @@ public abstract class AbstractApplicationContextRunnerTests { + get().withSystemProperties(key + "=").run((loaded) -> { assertThat(System.getProperties()).doesNotContainKey(key); }); assertThat(System.getProperties().getProperty(key)).isEqualTo("value"); diff --git a/spring-boot-test/src/test/java/org/springframework/boot/test/util/TestPropertyValuesTests.java b/spring-boot-test/src/test/java/org/springframework/boot/test/util/TestPropertyValuesTests.java index e5f7a238fe..3d97c5ec86 100644 --- a/spring-boot-test/src/test/java/org/springframework/boot/test/util/TestPropertyValuesTests.java +++ b/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") - .and("bling.blah", "bing").applyTo(this.environment, Type.MAP); + TestPropertyValues.of("foo.bar=baz").and("hello.world=hi").and("bling.blah=bing") + .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; });