Preserve ordering of inlined props in @TestPropertySource

The initial implementation for adding inlined properties configured via
@TestPropertySource to the context's environment did not preserve the
order in which the properties were physically declared. This makes
@TestPropertySource a poor testing facility for mimicking the
production environment's configuration if the property source mechanism
used in production preserves ordering of property names -- which is the
case for YAML-based property sources used in Spring Boot, Spring Yarn,
etc.

This commit addresses this issue by ensuring that the ordering of
inlined properties declared via @TestPropertySource is preserved.
Specifically, the original functionality has been refactored. extracted
from AbstractContextLoader, and moved to TestPropertySourceUtils where
it may later be made public for general purpose use in other frameworks.

Issue: SPR-12710
(cherry picked from commit d6a799a)

Author: sbrannen

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,13 +16,8 @@
 
 package org.springframework.test.context.support;
 
-import java.io.IOException;
-import java.io.StringReader;
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
-import java.util.Properties;
 import java.util.Set;
 
 import org.apache.commons.logging.Log;
@@ -34,12 +29,8 @@
 import org.springframework.context.ConfigurableApplicationContext;
 import org.springframework.core.GenericTypeResolver;
 import org.springframework.core.annotation.AnnotationAwareOrderComparator;
-import org.springframework.core.env.ConfigurableEnvironment;
-import org.springframework.core.env.MapPropertySource;
 import org.springframework.core.env.PropertySource;
 import org.springframework.core.io.ClassPathResource;
-import org.springframework.core.io.Resource;
-import org.springframework.core.io.support.ResourcePropertySource;
 import org.springframework.test.context.ContextConfigurationAttributes;
 import org.springframework.test.context.ContextLoader;
 import org.springframework.test.context.MergedContextConfiguration;
@@ -64,7 +55,6 @@
  *
  * @author Sam Brannen
  * @author Juergen Hoeller
- * @author Dave Syer
  * @since 2.5
  * @see #generateDefaultLocations
  * @see #getResourceSuffixes
@@ -74,8 +64,6 @@ public abstract class AbstractContextLoader implements SmartContextLoader {
 
 	private static final String[] EMPTY_STRING_ARRAY = new String[0];
 
-	private static final String LINE_SEPARATOR = System.getProperty("line.separator");
-
 	private static final Log logger = LogFactory.getLog(AbstractContextLoader.class);
 
 
@@ -137,74 +125,11 @@ public void processContextConfiguration(ContextConfigurationAttributes configAtt
 	 */
 	protected void prepareContext(ConfigurableApplicationContext context, MergedContextConfiguration mergedConfig) {
 		context.getEnvironment().setActiveProfiles(mergedConfig.getActiveProfiles());
-		addResourcePropertySourcesToEnvironment(context, mergedConfig);
-		addInlinedPropertiesToEnvironment(context, mergedConfig);
+		TestPropertySourceUtils.addResourcePropertySourcesToEnvironment(context, mergedConfig.getPropertySourceLocations());
+		TestPropertySourceUtils.addInlinedPropertiesToEnvironment(context, mergedConfig.getPropertySourceProperties());
 		invokeApplicationContextInitializers(context, mergedConfig);
 	}
 
-	/**
-	 * @since 4.1
-	 */
-	private void addResourcePropertySourcesToEnvironment(ConfigurableApplicationContext context,
-			MergedContextConfiguration mergedConfig) {
-		try {
-			ConfigurableEnvironment environment = context.getEnvironment();
-			String[] locations = mergedConfig.getPropertySourceLocations();
-			for (String location : locations) {
-				String resolvedLocation = environment.resolveRequiredPlaceholders(location);
-				Resource resource = context.getResource(resolvedLocation);
-				ResourcePropertySource ps = new ResourcePropertySource(resource);
-				environment.getPropertySources().addFirst(ps);
-			}
-		}
-		catch (IOException e) {
-			throw new IllegalStateException("Failed to add PropertySource to Environment", e);
-		}
-	}
-
-	/**
-	 * @since 4.1
-	 */
-	private void addInlinedPropertiesToEnvironment(ConfigurableApplicationContext context,
-			MergedContextConfiguration mergedConfig) {
-		String[] keyValuePairs = mergedConfig.getPropertySourceProperties();
-		if (!ObjectUtils.isEmpty(keyValuePairs)) {
-			String name = "test properties " + ObjectUtils.nullSafeToString(keyValuePairs);
-			MapPropertySource ps = new MapPropertySource(name, extractEnvironmentProperties(keyValuePairs));
-			context.getEnvironment().getPropertySources().addFirst(ps);
-		}
-	}
-
-	/**
-	 * Extract environment properties from the supplied key/value pairs.
-	 * <p>Parsing of the key/value pairs is achieved by converting all pairs
-	 * into a single <em>virtual</em> properties file in memory and delegating
-	 * to {@link Properties#load(java.io.Reader)} to parse that virtual file.
-	 * <p>This code has been adapted from Spring Boot's
-	 * {@link org.springframework.boot.test.SpringApplicationContextLoader SpringApplicationContextLoader}.
-	 * @since 4.1
-	 */
-	private Map<String, Object> extractEnvironmentProperties(String[] keyValuePairs) {
-		StringBuilder sb = new StringBuilder();
-		for (String keyValuePair : keyValuePairs) {
-			sb.append(keyValuePair).append(LINE_SEPARATOR);
-		}
-		String content = sb.toString();
-		Properties props = new Properties();
-		try {
-			props.load(new StringReader(content));
-		}
-		catch (IOException e) {
-			throw new IllegalStateException("Failed to load test environment properties from: " + content, e);
-		}
-
-		Map<String, Object> properties = new HashMap<String, Object>();
-		for (String name : props.stringPropertyNames()) {
-			properties.put(name, props.getProperty(name));
-		}
-		return properties;
-	}
-
 	@SuppressWarnings("unchecked")
 	private void invokeApplicationContextInitializers(ConfigurableApplicationContext context,
 			MergedContextConfiguration mergedConfig) {

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,24 +16,39 @@
 
 package org.springframework.test.context.support;
 
+import java.io.IOException;
+import java.io.StringReader;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.LinkedHashMap;
 import java.util.List;
+import java.util.Map;
+import java.util.Properties;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
+import org.springframework.context.ConfigurableApplicationContext;
 import org.springframework.core.annotation.AnnotationAttributes;
+import org.springframework.core.env.ConfigurableEnvironment;
+import org.springframework.core.env.MapPropertySource;
+import org.springframework.core.env.PropertySource;
+import org.springframework.core.io.Resource;
+import org.springframework.core.io.support.ResourcePropertySource;
 import org.springframework.test.context.TestPropertySource;
 import org.springframework.test.context.util.TestContextResourceUtils;
-import org.springframework.test.util.MetaAnnotationUtils.*;
+import org.springframework.test.util.MetaAnnotationUtils.AnnotationDescriptor;
 import org.springframework.util.Assert;
+import org.springframework.util.ObjectUtils;
 import org.springframework.util.StringUtils;
 
 import static org.springframework.test.util.MetaAnnotationUtils.*;
 
 /**
- * Utility methods for working with {@link TestPropertySource @TestPropertySource}.
+ * Utility methods for working with {@link TestPropertySource @TestPropertySource}
+ * and adding test {@link PropertySource PropertySources} to the {@code Environment}.
+ *
+ * <p>Primarily intended for use within the framework.
  *
  * @author Sam Brannen
  * @since 4.1
@@ -131,4 +146,76 @@ private static String[] mergeProperties(List<TestPropertySourceAttributes> attri
 		return StringUtils.toStringArray(properties);
 	}
 
+	/**
+	 * @since 4.1.5
+	 */
+	static void addResourcePropertySourcesToEnvironment(ConfigurableApplicationContext context,
+			String[] propertySourceLocations) {
+		try {
+			ConfigurableEnvironment environment = context.getEnvironment();
+			String[] locations = propertySourceLocations;
+			for (String location : locations) {
+				String resolvedLocation = environment.resolveRequiredPlaceholders(location);
+				Resource resource = context.getResource(resolvedLocation);
+				ResourcePropertySource ps = new ResourcePropertySource(resource);
+				environment.getPropertySources().addFirst(ps);
+			}
+		}
+		catch (IOException e) {
+			throw new IllegalStateException("Failed to add PropertySource to Environment", e);
+		}
+	}
+
+	/**
+	 * @since 4.1.5
+	 */
+	static void addInlinedPropertiesToEnvironment(ConfigurableApplicationContext context,
+			String[] propertySourceProperties) {
+		addInlinedPropertiesToEnvironment(context.getEnvironment(), propertySourceProperties);
+	}
+
+	/**
+	 * @since 4.1.5
+	 */
+	static void addInlinedPropertiesToEnvironment(ConfigurableEnvironment environment, String[] propertySourceProperties) {
+		if (!ObjectUtils.isEmpty(propertySourceProperties)) {
+			String name = "test properties " + ObjectUtils.nullSafeToString(propertySourceProperties);
+			MapPropertySource ps = new MapPropertySource(name, extractEnvironmentProperties(propertySourceProperties));
+			environment.getPropertySources().addFirst(ps);
+		}
+	}
+
+	/**
+	 * Extract environment properties from the supplied key/value pairs,
+	 * preserving the ordering of property names in the returned map.
+	 * <p>Parsing of the key/value pairs is achieved by converting all pairs
+	 * into <em>virtual</em> properties files in memory and delegating to
+	 * {@link Properties#load(java.io.Reader)} to parse each virtual file.
+	 */
+	private static Map<String, Object> extractEnvironmentProperties(String[] keyValuePairs) {
+		Map<String, Object> map = new LinkedHashMap<String, Object>();
+
+		Properties props = new Properties();
+		for (String pair : keyValuePairs) {
+			if (!StringUtils.hasText(pair)) {
+				continue;
+			}
+
+			try {
+				props.load(new StringReader(pair));
+			}
+			catch (Exception e) {
+				throw new IllegalStateException("Failed to load test environment property from [" + pair + "].", e);
+			}
+			Assert.state(props.size() == 1, "Failed to load exactly one test environment property from [" + pair + "].");
+
+			for (String name : props.stringPropertyNames()) {
+				map.put(name, props.getProperty(name));
+			}
+			props.clear();
+		}
+
+		return map;
+	}
+
 }

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

@@ -21,43 +21,71 @@
 
 import org.springframework.beans.factory.annotation.Autowired;
 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.junit.Assert.*;
 
 /**
- * Integration tests for {@link TestPropertySource @TestPropertySource}
- * support with an inlined properties.
+ * Integration tests for {@link TestPropertySource @TestPropertySource} support with
+ * inlined properties.
  *
  * @author Sam Brannen
  * @since 4.1
  */
 @RunWith(SpringJUnit4ClassRunner.class)
 @ContextConfiguration
-@TestPropertySource(properties = { "foo = bar", "baz quux", "enigma: 42",
-	"x.y.z = a=b=c", "server.url = http://example.com", "key.value.1: key=value",
-	"key.value.2 key=value", "key.value.3 key:value" })
+@TestPropertySource(properties = { "", "foo = bar", "baz quux", "enigma: 42", "x.y.z = a=b=c",
+	"server.url = http://example.com", "key.value.1: key=value", "key.value.2 key=value", "key.value.3 key:value" })
 public class InlinedPropertiesTestPropertySourceTests {
 
 	@Autowired
-	protected Environment env;
+	private Environment env;
 
 
 	@Test
-	public void verifyPropertiesAreAvailableInEnvironment() {
+	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());
+
+		// 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"));
 	}
 
+	@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);
+	}
+
 
 	// -------------------------------------------------------------------
 

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,10 +16,16 @@
 
 package org.springframework.test.context.support;
 
+import java.util.Map;
+
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 
+import org.springframework.core.env.ConfigurableEnvironment;
+import org.springframework.core.env.MutablePropertySources;
+import org.springframework.mock.env.MockEnvironment;
+import org.springframework.mock.env.MockPropertySource;
 import org.springframework.test.context.TestPropertySource;
 
 import static org.hamcrest.CoreMatchers.*;
@@ -113,6 +119,41 @@ public void overriddenLocationsAndProperties() {
 			new String[] { "classpath:/baz.properties" }, new String[] { "key = value" });
 	}
 
+	/**
+	 * @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());
+	}
+
+	/**
+	 * @since 4.1.5
+	 */
+	@Test
+	public void inlinedPropertyWithMalformedUnicodeInValue() {
+		expectedException.expect(IllegalStateException.class);
+		expectedException.expectMessage("Failed to load test environment property");
+		addInlinedPropertiesToEnvironment(new MockEnvironment(), new String[] { "key = \\uZZZZ" });
+	}
+
+	/**
+	 * @since 4.1.5
+	 */
+	@Test
+	public void inlinedPropertyWithMultipleKeyValuePairs() {
+		expectedException.expect(IllegalStateException.class);
+		expectedException.expectMessage("Failed to load exactly one test environment property");
+		addInlinedPropertiesToEnvironment(new MockEnvironment(), new String[] { "a=b\nx=y" });
+	}
+
 
 	// -------------------------------------------------------------------