Make TestPropertySourceUtils more robust

 - Added assertions for pre-conditions on method arguments for all
   public utility methods.

 - Introduced additional tests in TestPropertySourceUtilsTests to verify
   the new pre-conditions.

 - Introduced INLINED_PROPERTIES_PROPERTY_SOURCE_NAME constant for the
   name of the MapPropertySource created from inlined properties; the
   name therefore no longer contains the inlined properties, but the
   original values of the inlined properties can now be logged at debug
   level.

 - Simplified tests in InlinedPropertiesTestPropertySourceTests.

Issue: SPR-12721
(cherry picked from commit 42af330)

Author: sbrannen

spring-test/src/main/java/org/springframework/test/context/support/TestPropertySourceUtils.java

@@ -60,6 +60,13 @@ public abstract class TestPropertySourceUtils {
 
 	private static final Log logger = LogFactory.getLog(TestPropertySourceUtils.class);
 
+	/**
+	 * The name of the {@link MapPropertySource} created from <em>inlined properties</em>.
+	 * @since 4.1.5
+	 * @see {@link #addInlinedPropertiesToEnvironment(ConfigurableEnvironment, String[])}
+	 */
+	public static final String INLINED_PROPERTIES_PROPERTY_SOURCE_NAME = "Inlined Test Properties";
+
 
 	private TestPropertySourceUtils() {
 		/* no-op */
@@ -151,19 +158,25 @@ private static String[] mergeProperties(List<TestPropertySourceAttributes> attri
 	/**
 	 * Add the {@link Properties} files from the given resource {@code locations}
 	 * to the {@link Environment} of the supplied {@code context}.
-	 * <p>Each properties file will be converted to a {@code ResourcePropertySource}
+	 * <p>Property placeholders in resource locations (i.e., <code>${...}</code>)
+	 * will be {@linkplain Environment#resolveRequiredPlaceholders(String) resolved}
+	 * against the {@code Environment}.
+	 * <p>Each properties file will be converted to a {@link ResourcePropertySource}
 	 * that will be added to the {@link PropertySources} of the environment with
 	 * highest precedence.
-	 * @param context the application context whose environment should be updated
-	 * @param locations the resource locations of {@link Properties} files to add
-	 * to the environment
+	 * @param context the application context whose environment should be updated;
+	 * never {@code null}
+	 * @param locations the resource locations of {@code Properties} files to add
+	 * to the environment; potentially empty but never {@code null}
 	 * @since 4.1.5
 	 * @see ResourcePropertySource
 	 * @see TestPropertySource#locations
 	 * @throws IllegalStateException if an error occurs while processing a properties file
 	 */
 	public static void addPropertiesFilesToEnvironment(ConfigurableApplicationContext context,
 			String[] locations) {
+		Assert.notNull(context, "context must not be null");
+		Assert.notNull(locations, "locations must not be null");
 		try {
 			ConfigurableEnvironment environment = context.getEnvironment();
 			for (String location : locations) {
@@ -178,18 +191,22 @@ public static void addPropertiesFilesToEnvironment(ConfigurableApplicationContex
 	}
 
 	/**
-	 * Add the given <em>inlined properties</em> (in the form of <em>key-value</em>
-	 * pairs) to the {@link Environment} of the supplied {@code context}.
+	 * Add the given <em>inlined properties</em> to the {@link Environment} of the
+	 * supplied {@code context}.
 	 * <p>This method simply delegates to
 	 * {@link #addInlinedPropertiesToEnvironment(ConfigurableEnvironment, String[])}.
-	 * @param context the application context whose environment should be updated
-	 * @param inlinedProperties the inlined properties to add to the environment
+	 * @param context the application context whose environment should be updated;
+	 * never {@code null}
+	 * @param inlinedProperties the inlined properties to add to the environment;
+	 * potentially empty but never {@code null}
 	 * @since 4.1.5
 	 * @see TestPropertySource#properties
 	 * @see #addInlinedPropertiesToEnvironment(ConfigurableEnvironment, String[])
 	 */
 	public static void addInlinedPropertiesToEnvironment(ConfigurableApplicationContext context,
 			String[] inlinedProperties) {
+		Assert.notNull(context, "context must not be null");
+		Assert.notNull(inlinedProperties, "inlinedProperties must not be null");
 		addInlinedPropertiesToEnvironment(context.getEnvironment(), inlinedProperties);
 	}
 
@@ -200,17 +217,25 @@ public static void addInlinedPropertiesToEnvironment(ConfigurableApplicationCont
 	 * single {@link MapPropertySource} with the highest precedence.
 	 * <p>For details on the parsing of <em>inlined properties</em>, consult the
 	 * Javadoc for {@link #convertInlinedPropertiesToMap}.
-	 * @param environment the environment to update
-	 * @param inlinedProperties the inlined properties to add to the environment
+	 * @param environment the environment to update; never {@code null}
+	 * @param inlinedProperties the inlined properties to add to the environment;
+	 * potentially empty but never {@code null}
 	 * @since 4.1.5
 	 * @see MapPropertySource
+	 * @see #INLINED_PROPERTIES_PROPERTY_SOURCE_NAME
 	 * @see TestPropertySource#properties
 	 * @see #convertInlinedPropertiesToMap
 	 */
 	public static void addInlinedPropertiesToEnvironment(ConfigurableEnvironment environment, String[] inlinedProperties) {
+		Assert.notNull(environment, "environment must not be null");
+		Assert.notNull(inlinedProperties, "inlinedProperties must not be null");
 		if (!ObjectUtils.isEmpty(inlinedProperties)) {
-			String name = "test properties " + ObjectUtils.nullSafeToString(inlinedProperties);
-			MapPropertySource ps = new MapPropertySource(name, convertInlinedPropertiesToMap(inlinedProperties));
+			if (logger.isDebugEnabled()) {
+				logger.debug("Adding inlined properties to environment: "
+						+ ObjectUtils.nullSafeToString(inlinedProperties));
+			}
+			MapPropertySource ps = new MapPropertySource(INLINED_PROPERTIES_PROPERTY_SOURCE_NAME,
+				convertInlinedPropertiesToMap(inlinedProperties));
 			environment.getPropertySources().addFirst(ps);
 		}
 	}
@@ -224,11 +249,16 @@ public static void addInlinedPropertiesToEnvironment(ConfigurableEnvironment env
 	 * {@link Properties#load(java.io.Reader)} to parse each virtual file.
 	 * <p>For a full discussion of <em>inlined properties</em>, consult the Javadoc
 	 * for {@link TestPropertySource#properties}.
+	 * @param inlinedProperties the inlined properties to convert; potentially empty
+	 * but never {@code null}
+	 * @return a new, ordered map containing the converted properties
 	 * @since 4.1.5
 	 * @throws IllegalStateException if a given key-value pair cannot be parsed, or if
 	 * a given inlined property contains multiple key-value pairs
+	 * @see #addInlinedPropertiesToEnvironment(ConfigurableEnvironment, String[])
 	 */
 	public static Map<String, Object> convertInlinedPropertiesToMap(String[] inlinedProperties) {
+		Assert.notNull(inlinedProperties, "inlinedProperties must not be null");
 		Map<String, Object> map = new LinkedHashMap<String, Object>();
 
 		Properties props = new Properties();

spring-test/src/test/java/org/springframework/test/context/env/InlinedPropertiesTestPropertySourceTests.java

@@ -23,13 +23,13 @@
 import org.springframework.context.annotation.Configuration;
 import org.springframework.core.env.ConfigurableEnvironment;
 import org.springframework.core.env.EnumerablePropertySource;
-import org.springframework.core.env.Environment;
-import org.springframework.core.env.PropertySource;
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.TestPropertySource;
 import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
 
+import static org.hamcrest.CoreMatchers.*;
 import static org.junit.Assert.*;
+import static org.springframework.test.context.support.TestPropertySourceUtils.*;
 
 /**
  * Integration tests for {@link TestPropertySource @TestPropertySource} support with
@@ -45,45 +45,36 @@
 public class InlinedPropertiesTestPropertySourceTests {
 
 	@Autowired
-	private Environment env;
+	private ConfigurableEnvironment env;
 
 
+	private String property(String key) {
+		return env.getProperty(key);
+	}
+
 	@Test
 	public void propertiesAreAvailableInEnvironment() {
-
 		// Simple key/value pairs
-		assertEquals("bar", env.getProperty("foo"));
-		assertEquals("quux", env.getProperty("baz"));
-		assertEquals(42, env.getProperty("enigma", Integer.class).intValue());
+		assertThat(property("foo"), is("bar"));
+		assertThat(property("baz"), is("quux"));
+		assertThat(property("enigma"), is("42"));
 
 		// Values containing key/value delimiters (":", "=", " ")
-		assertEquals("a=b=c", env.getProperty("x.y.z"));
-		assertEquals("http://example.com", env.getProperty("server.url"));
-		assertEquals("key=value", env.getProperty("key.value.1"));
-		assertEquals("key=value", env.getProperty("key.value.2"));
-		assertEquals("key:value", env.getProperty("key.value.3"));
+		assertThat(property("x.y.z"), is("a=b=c"));
+		assertThat(property("server.url"), is("http://example.com"));
+		assertThat(property("key.value.1"), is("key=value"));
+		assertThat(property("key.value.2"), is("key=value"));
+		assertThat(property("key.value.3"), is("key:value"));
 	}
 
 	@Test
 	@SuppressWarnings("rawtypes")
 	public void propertyNameOrderingIsPreservedInEnvironment() {
-		String[] propertyNames = null;
-
-		ConfigurableEnvironment configurableEnvironment = (ConfigurableEnvironment) env;
-		for (PropertySource<?> propertySource : configurableEnvironment.getPropertySources()) {
-			if (propertySource instanceof EnumerablePropertySource) {
-				EnumerablePropertySource eps = (EnumerablePropertySource) propertySource;
-				if (eps.getName().startsWith("test properties")) {
-					propertyNames = eps.getPropertyNames();
-					break;
-				}
-			}
-		}
-
 		final String[] expectedPropertyNames = new String[] { "foo", "baz", "enigma", "x.y.z", "server.url",
 			"key.value.1", "key.value.2", "key.value.3" };
-
-		assertArrayEquals(expectedPropertyNames, propertyNames);
+		EnumerablePropertySource eps = (EnumerablePropertySource) env.getPropertySources().get(
+			INLINED_PROPERTIES_PROPERTY_SOURCE_NAME);
+		assertArrayEquals(expectedPropertyNames, eps.getPropertyNames());
 	}
 
 

spring-test/src/test/java/org/springframework/test/context/support/TestPropertySourceUtilsTests.java

@@ -22,6 +22,7 @@
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 
+import org.springframework.context.ConfigurableApplicationContext;
 import org.springframework.core.env.ConfigurableEnvironment;
 import org.springframework.core.env.MutablePropertySources;
 import org.springframework.mock.env.MockEnvironment;
@@ -30,6 +31,7 @@
 
 import static org.hamcrest.CoreMatchers.*;
 import static org.junit.Assert.*;
+import static org.mockito.Mockito.mock;
 import static org.springframework.test.context.support.TestPropertySourceUtils.*;
 
 /**
@@ -41,6 +43,7 @@
 public class TestPropertySourceUtilsTests {
 
 	private static final String[] EMPTY_STRING_ARRAY = new String[0];
+	private static final String[] KEY_VALUE_PAIR = new String[] { "key = value" };
 
 	@Rule
 	public ExpectedException expectedException = ExpectedException.none();
@@ -110,35 +113,60 @@ public void overriddenLocations() {
 	@Test
 	public void overriddenProperties() {
 		assertMergedTestPropertySources(OverriddenPropertiesPropertySources.class, new String[] {
-			"classpath:/foo1.xml", "classpath:/foo2.xml", "classpath:/baz.properties" }, new String[] { "key = value" });
+			"classpath:/foo1.xml", "classpath:/foo2.xml", "classpath:/baz.properties" }, KEY_VALUE_PAIR);
 	}
 
 	@Test
 	public void overriddenLocationsAndProperties() {
 		assertMergedTestPropertySources(OverriddenLocationsAndPropertiesPropertySources.class,
-			new String[] { "classpath:/baz.properties" }, new String[] { "key = value" });
+			new String[] { "classpath:/baz.properties" }, KEY_VALUE_PAIR);
 	}
 
 	/**
 	 * @since 4.1.5
 	 */
 	@Test
-	@SuppressWarnings("rawtypes")
-	public void emptyInlinedProperty() {
-		ConfigurableEnvironment environment = new MockEnvironment();
-		MutablePropertySources propertySources = environment.getPropertySources();
-		propertySources.remove(MockPropertySource.MOCK_PROPERTIES_PROPERTY_SOURCE_NAME);
-		assertEquals(0, propertySources.size());
-		addInlinedPropertiesToEnvironment(environment, new String[] { "  " });
-		assertEquals(1, propertySources.size());
-		assertEquals(0, ((Map) propertySources.iterator().next().getSource()).size());
+	public void addInlinedPropertiesToEnvironmentWithNullContext() {
+		expectedException.expect(IllegalArgumentException.class);
+		expectedException.expectMessage("context");
+		addInlinedPropertiesToEnvironment((ConfigurableApplicationContext) null, KEY_VALUE_PAIR);
+	}
+
+	/**
+	 * @since 4.1.5
+	 */
+	@Test
+	public void addInlinedPropertiesToEnvironmentWithContextAndNullInlinedProperties() {
+		expectedException.expect(IllegalArgumentException.class);
+		expectedException.expectMessage("inlined");
+		addInlinedPropertiesToEnvironment(mock(ConfigurableApplicationContext.class), null);
+	}
+
+	/**
+	 * @since 4.1.5
+	 */
+	@Test
+	public void addInlinedPropertiesToEnvironmentWithNullEnvironment() {
+		expectedException.expect(IllegalArgumentException.class);
+		expectedException.expectMessage("environment");
+		addInlinedPropertiesToEnvironment((ConfigurableEnvironment) null, KEY_VALUE_PAIR);
+	}
+
+	/**
+	 * @since 4.1.5
+	 */
+	@Test
+	public void addInlinedPropertiesToEnvironmentWithEnvironmentAndNullInlinedProperties() {
+		expectedException.expect(IllegalArgumentException.class);
+		expectedException.expectMessage("inlined");
+		addInlinedPropertiesToEnvironment(new MockEnvironment(), null);
 	}
 
 	/**
 	 * @since 4.1.5
 	 */
 	@Test
-	public void inlinedPropertyWithMalformedUnicodeInValue() {
+	public void addInlinedPropertiesToEnvironmentWithMalformedUnicodeInValue() {
 		expectedException.expect(IllegalStateException.class);
 		expectedException.expectMessage("Failed to load test environment property");
 		addInlinedPropertiesToEnvironment(new MockEnvironment(), new String[] { "key = \\uZZZZ" });
@@ -148,12 +176,33 @@ public void inlinedPropertyWithMalformedUnicodeInValue() {
 	 * @since 4.1.5
 	 */
 	@Test
-	public void inlinedPropertyWithMultipleKeyValuePairs() {
+	public void addInlinedPropertiesToEnvironmentWithMultipleKeyValuePairsInSingleInlinedProperty() {
 		expectedException.expect(IllegalStateException.class);
 		expectedException.expectMessage("Failed to load exactly one test environment property");
 		addInlinedPropertiesToEnvironment(new MockEnvironment(), new String[] { "a=b\nx=y" });
 	}
 
+	/**
+	 * @since 4.1.5
+	 */
+	@Test
+	@SuppressWarnings("rawtypes")
+	public void addInlinedPropertiesToEnvironmentWithEmptyProperty() {
+		ConfigurableEnvironment environment = new MockEnvironment();
+		MutablePropertySources propertySources = environment.getPropertySources();
+		propertySources.remove(MockPropertySource.MOCK_PROPERTIES_PROPERTY_SOURCE_NAME);
+		assertEquals(0, propertySources.size());
+		addInlinedPropertiesToEnvironment(environment, new String[] { "  " });
+		assertEquals(1, propertySources.size());
+		assertEquals(0, ((Map) propertySources.iterator().next().getSource()).size());
+	}
+
+	@Test
+	public void convertInlinedPropertiesToMapWithNullInlinedProperties() {
+		expectedException.expect(IllegalArgumentException.class);
+		expectedException.expectMessage("inlined");
+		convertInlinedPropertiesToMap(null);
+	}
 
 	// -------------------------------------------------------------------