Retain order of active profiles in the TCF

Ever since @ActiveProfiles was introduced, the declared active profiles
for integration tests have been sorted in order to support unique cache
key generation; however, there are use cases for which the original
ordering should be retained.

For example, Spring Boot's ConfigFileApplicationListener loads
configuration files for active profiles in the order returned by
Environment.getActiveProfiles(), with the assumption that the ordering
matches the order in which the developer declared the active profiles.

This commit maintains the uniqueness of active profiles declared via
@ActiveProfiles but no longer sorts them.

Issue: SPR-12492

spring-test/src/main/java/org/springframework/test/context/MergedContextConfiguration.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.
@@ -19,9 +19,8 @@ package org.springframework.test.context;
 import java.io.Serializable;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.LinkedHashSet;
 import java.util.Set;
-import java.util.SortedSet;
-import java.util.TreeSet;
 
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.ApplicationContextInitializer;
@@ -49,8 +48,9 @@ import org.springframework.util.StringUtils;
  * <p>A {@link SmartContextLoader} uses {@code MergedContextConfiguration}
  * to load an {@link org.springframework.context.ApplicationContext ApplicationContext}.
  *
- * <p>{@code MergedContextConfiguration} is also used by the {@link TestContext}
- * as the context cache key for caching an
+ * <p>{@code MergedContextConfiguration} is also used by the
+ * {@link org.springframework.test.context.cache.ContextCache ContextCache}
+ * as the key for caching an
  * {@link org.springframework.context.ApplicationContext ApplicationContext}
  * that was loaded using properties of this {@code MergedContextConfiguration}.
  *
@@ -116,11 +116,9 @@ public class MergedContextConfiguration implements Serializable {
 			return EMPTY_STRING_ARRAY;
 		}
 
-		// Active profiles must be unique and sorted in order to support proper
-		// cache key generation. Specifically, profile sets {foo,bar} and
-		// {bar,foo} must both result in the same array (e.g., [bar,foo]).
-		SortedSet<String> sortedProfilesSet = new TreeSet<String>(Arrays.asList(activeProfiles));
-		return StringUtils.toStringArray(sortedProfilesSet);
+		// Active profiles must be unique
+		Set<String> profilesSet = new LinkedHashSet<String>(Arrays.asList(activeProfiles));
+		return StringUtils.toStringArray(profilesSet);
 	}
 
 	/**

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

@@ -16,7 +16,10 @@
 
 package org.springframework.test.context.support;
 
-import java.util.HashSet;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.LinkedHashSet;
+import java.util.List;
 import java.util.Set;
 
 import org.apache.commons.logging.Log;
@@ -72,7 +75,7 @@ abstract class ActiveProfilesUtils {
 	static String[] resolveActiveProfiles(Class<?> testClass) {
 		Assert.notNull(testClass, "Class must not be null");
 
-		final Set<String> activeProfiles = new HashSet<String>();
+		final List<String[]> profileArrays = new ArrayList<String[]>();
 
 		Class<ActiveProfiles> annotationType = ActiveProfiles.class;
 		AnnotationDescriptor<ActiveProfiles> descriptor = MetaAnnotationUtils.findAnnotationDescriptor(testClass,
@@ -118,14 +121,22 @@ abstract class ActiveProfilesUtils {
 				throw new IllegalStateException(msg);
 			}
 
+			profileArrays.add(profiles);
+
+			descriptor = (annotation.inheritProfiles() ? MetaAnnotationUtils.findAnnotationDescriptor(
+				rootDeclaringClass.getSuperclass(), annotationType) : null);
+		}
+
+		// Reverse the list so that we can traverse "down" the hierarchy.
+		Collections.reverse(profileArrays);
+
+		final Set<String> activeProfiles = new LinkedHashSet<String>();
+		for (String[] profiles : profileArrays) {
 			for (String profile : profiles) {
 				if (StringUtils.hasText(profile)) {
 					activeProfiles.add(profile.trim());
 				}
 			}
-
-			descriptor = (annotation.inheritProfiles() ? MetaAnnotationUtils.findAnnotationDescriptor(
-				rootDeclaringClass.getSuperclass(), annotationType) : null);
 		}
 
 		return StringUtils.toStringArray(activeProfiles);

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

@@ -16,7 +16,7 @@
 
 package org.springframework.test.context.support;
 
-import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.Set;
 
 import org.apache.commons.logging.Log;
@@ -59,7 +59,7 @@ public class DefaultActiveProfilesResolver implements ActiveProfilesResolver {
 	public String[] resolve(Class<?> testClass) {
 		Assert.notNull(testClass, "Class must not be null");
 
-		final Set<String> activeProfiles = new HashSet<String>();
+		final Set<String> activeProfiles = new LinkedHashSet<String>();
 
 		Class<ActiveProfiles> annotationType = ActiveProfiles.class;
 		AnnotationDescriptor<ActiveProfiles> descriptor = findAnnotationDescriptor(testClass, annotationType);

spring-test/src/test/java/org/springframework/test/context/MergedContextConfigurationTests.java

@@ -141,7 +141,7 @@ public class MergedContextConfigurationTests {
 				EMPTY_STRING_ARRAY, EMPTY_CLASS_ARRAY, activeProfiles1, loader);
 		MergedContextConfiguration mergedConfig2 = new MergedContextConfiguration(getClass(),
 				EMPTY_STRING_ARRAY, EMPTY_CLASS_ARRAY, activeProfiles2, loader);
-		assertEquals(mergedConfig1.hashCode(), mergedConfig2.hashCode());
+		assertNotEquals(mergedConfig1.hashCode(), mergedConfig2.hashCode());
 	}
 
 	@Test
@@ -337,7 +337,7 @@ public class MergedContextConfigurationTests {
 				EMPTY_STRING_ARRAY, EMPTY_CLASS_ARRAY, activeProfiles1, loader);
 		MergedContextConfiguration mergedConfig2 = new MergedContextConfiguration(getClass(),
 				EMPTY_STRING_ARRAY, EMPTY_CLASS_ARRAY, activeProfiles2, loader);
-		assertEquals(mergedConfig1, mergedConfig2);
+		assertNotEquals(mergedConfig1, mergedConfig2);
 	}
 
 	@Test

spring-test/src/test/java/org/springframework/test/context/cache/ContextCacheTests.java

@@ -86,14 +86,15 @@ public class ContextCacheTests {
 
 	@Test
 	public void verifyCacheKeyIsBasedOnActiveProfiles() {
-		loadCtxAndAssertStats(FooBarProfilesTestCase.class, 1, 0, 1);
-		loadCtxAndAssertStats(FooBarProfilesTestCase.class, 1, 1, 1);
-		// Profiles {foo, bar} should hash to the same as {bar,foo}
-		loadCtxAndAssertStats(BarFooProfilesTestCase.class, 1, 2, 1);
-		loadCtxAndAssertStats(FooBarProfilesTestCase.class, 1, 3, 1);
-		loadCtxAndAssertStats(FooBarProfilesTestCase.class, 1, 4, 1);
-		loadCtxAndAssertStats(BarFooProfilesTestCase.class, 1, 5, 1);
-		loadCtxAndAssertStats(FooBarActiveProfilesResolverTestCase.class, 1, 6, 1);
+		int size = 0, hit = 0, miss = 0;
+		loadCtxAndAssertStats(FooBarProfilesTestCase.class, ++size, hit, ++miss);
+		loadCtxAndAssertStats(FooBarProfilesTestCase.class, size, ++hit, miss);
+		// Profiles {foo, bar} should not hash to the same as {bar,foo}
+		loadCtxAndAssertStats(BarFooProfilesTestCase.class, ++size, hit, ++miss);
+		loadCtxAndAssertStats(FooBarProfilesTestCase.class, size, ++hit, miss);
+		loadCtxAndAssertStats(FooBarProfilesTestCase.class, size, ++hit, miss);
+		loadCtxAndAssertStats(BarFooProfilesTestCase.class, size, ++hit, miss);
+		loadCtxAndAssertStats(FooBarActiveProfilesResolverTestCase.class, size, ++hit, miss);
 	}
 
 	@Test

spring-test/src/test/java/org/springframework/test/context/support/AbstractContextConfigurationUtilsTests.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.
@@ -164,12 +164,12 @@ abstract class AbstractContextConfigurationUtilsTests {
 	}
 
 	@ContextConfiguration(locations = "/foo.xml", inheritLocations = false)
-	@ActiveProfiles(profiles = "foo")
+	@ActiveProfiles("foo")
 	static class LocationsFoo {
 	}
 
 	@ContextConfiguration(classes = FooConfig.class, inheritLocations = false)
-	@ActiveProfiles(profiles = "foo")
+	@ActiveProfiles("foo")
 	static class ClassesFoo {
 	}
 
@@ -198,14 +198,14 @@ abstract class AbstractContextConfigurationUtilsTests {
 	}
 
 	@ContextConfiguration(locations = "/foo.properties", loader = GenericPropertiesContextLoader.class)
-	@ActiveProfiles(profiles = "foo")
+	@ActiveProfiles("foo")
 	static class PropertiesLocationsFoo {
 	}
 
 	// Combining @Configuration classes with a Properties based loader doesn't really make
 	// sense, but that's OK for unit testing purposes.
 	@ContextConfiguration(classes = FooConfig.class, loader = GenericPropertiesContextLoader.class)
-	@ActiveProfiles(profiles = "foo")
+	@ActiveProfiles("foo")
 	static class PropertiesClassesFoo {
 	}
 

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

@@ -22,9 +22,7 @@ import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.HashSet;
 import java.util.List;
-import java.util.Set;
 
 import org.junit.Test;
 
@@ -47,27 +45,22 @@ import static org.springframework.test.context.support.ActiveProfilesUtils.*;
 public class ActiveProfilesUtilsTests extends AbstractContextConfigurationUtilsTests {
 
 	private void assertResolvedProfiles(Class<?> testClass, String... expected) {
-		assertNotNull(testClass);
-		assertNotNull(expected);
-		String[] actual = resolveActiveProfiles(testClass);
-		Set<String> expectedSet = new HashSet<String>(Arrays.asList(expected));
-		Set<String> actualSet = new HashSet<String>(Arrays.asList(actual));
-		assertEquals(expectedSet, actualSet);
+		assertArrayEquals(expected, resolveActiveProfiles(testClass));
 	}
 
 	@Test
 	public void resolveActiveProfilesWithoutAnnotation() {
-		assertArrayEquals(EMPTY_STRING_ARRAY, resolveActiveProfiles(Enigma.class));
+		assertResolvedProfiles(Enigma.class, EMPTY_STRING_ARRAY);
 	}
 
 	@Test
 	public void resolveActiveProfilesWithNoProfilesDeclared() {
-		assertArrayEquals(EMPTY_STRING_ARRAY, resolveActiveProfiles(BareAnnotations.class));
+		assertResolvedProfiles(BareAnnotations.class, EMPTY_STRING_ARRAY);
 	}
 
 	@Test
 	public void resolveActiveProfilesWithEmptyProfiles() {
-		assertArrayEquals(EMPTY_STRING_ARRAY, resolveActiveProfiles(EmptyProfiles.class));
+		assertResolvedProfiles(EmptyProfiles.class, EMPTY_STRING_ARRAY);
 	}
 
 	@Test
@@ -75,6 +68,11 @@ public class ActiveProfilesUtilsTests extends AbstractContextConfigurationUtilsT
 		assertResolvedProfiles(DuplicatedProfiles.class, "foo", "bar", "baz");
 	}
 
+	@Test
+	public void resolveActiveProfilesWithLocalAndInheritedDuplicatedProfiles() {
+		assertResolvedProfiles(ExtendedDuplicatedProfiles.class, "foo", "bar", "baz", "cat", "dog");
+	}
+
 	@Test
 	public void resolveActiveProfilesWithLocalAnnotation() {
 		assertResolvedProfiles(LocationsFoo.class, "foo");
@@ -252,6 +250,10 @@ public class ActiveProfilesUtilsTests extends AbstractContextConfigurationUtilsT
 	private static class DuplicatedProfiles {
 	}
 
+	@ActiveProfiles({ "cat", "dog", "  foo", "bar  ", "cat" })
+	private static class ExtendedDuplicatedProfiles extends DuplicatedProfiles {
+	}
+
 	@ActiveProfiles(profiles = { "dog", "cat" }, inheritProfiles = false)
 	private static class Animals extends LocationsBar {
 	}