Avoid cache miss for @activeprofiles w/ same profiles but different order

Prior to this commit, two @activeprofiles declarations with the same
profiles but different order resulted in an identical duplicate
ApplicationContext in the context cache in the Spring TestContext
Framework.

This commit uses a TreeSet to ensure that registered active profiles
are both unique and sorted, thereby avoiding cache misses for
semantically identical active profiles configuration on different test
classes.

Closes gh-25973

Author: sbrannen

spring-test/src/main/java/org/springframework/test/context/ActiveProfiles.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -79,7 +79,7 @@
 	 * <p>The default value is {@code true}, which means that a test
 	 * class will <em>inherit</em> bean definition profiles defined by a
 	 * test superclass. Specifically, the bean definition profiles for a test
-	 * class will be appended to the list of bean definition profiles
+	 * class will be added to the list of bean definition profiles
 	 * defined by a test superclass. Thus, subclasses have the option of
 	 * <em>extending</em> the list of bean definition profiles.
 	 * <p>If {@code inheritProfiles} is set to {@code false}, the bean

spring-test/src/main/java/org/springframework/test/context/MergedContextConfiguration.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2020 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,8 +19,8 @@
 import java.io.Serializable;
 import java.util.Arrays;
 import java.util.Collections;
-import java.util.LinkedHashSet;
 import java.util.Set;
+import java.util.TreeSet;
 
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.ApplicationContextInitializer;
@@ -533,8 +533,8 @@ private static String[] processActiveProfiles(@Nullable String[] activeProfiles)
 			return EMPTY_STRING_ARRAY;
 		}
 
-		// Active profiles must be unique
-		Set<String> profilesSet = new LinkedHashSet<>(Arrays.asList(activeProfiles));
+		// Active profiles must be unique and sorted
+		Set<String> profilesSet = new TreeSet<>(Arrays.asList(activeProfiles));
 		return StringUtils.toStringArray(profilesSet);
 	}
 

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

@@ -16,11 +16,8 @@
 
 package org.springframework.test.context.support;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.LinkedHashSet;
-import java.util.List;
 import java.util.Set;
+import java.util.TreeSet;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -70,7 +67,7 @@ abstract class ActiveProfilesUtils {
 	static String[] resolveActiveProfiles(Class<?> testClass) {
 		Assert.notNull(testClass, "Class must not be null");
 
-		final List<String[]> profileArrays = new ArrayList<>();
+		Set<String> activeProfiles = new TreeSet<>();
 
 		Class<ActiveProfiles> annotationType = ActiveProfiles.class;
 		AnnotationDescriptor<ActiveProfiles> descriptor =
@@ -109,25 +106,17 @@ static String[] resolveActiveProfiles(Class<?> testClass) {
 
 			String[] profiles = resolver.resolve(rootDeclaringClass);
 			if (!ObjectUtils.isEmpty(profiles)) {
-				profileArrays.add(profiles);
+				for (String profile : profiles) {
+					if (StringUtils.hasText(profile)) {
+						activeProfiles.add(profile.trim());
+					}
+				}
 			}
 
 			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<>();
-		for (String[] profiles : profileArrays) {
-			for (String profile : profiles) {
-				if (StringUtils.hasText(profile)) {
-					activeProfiles.add(profile.trim());
-				}
-			}
-		}
-
 		return StringUtils.toStringArray(activeProfiles);
 	}
 

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2020 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,8 +16,8 @@
 
 package org.springframework.test.context.support;
 
-import java.util.LinkedHashSet;
 import java.util.Set;
+import java.util.TreeSet;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -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 LinkedHashSet<>();
+		Set<String> activeProfiles = new TreeSet<>();
 
 		Class<ActiveProfiles> annotationType = ActiveProfiles.class;
 		AnnotationDescriptor<ActiveProfiles> descriptor = findAnnotationDescriptor(testClass, annotationType);

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -143,7 +143,7 @@ public void hashCodeWithSameProfilesReversed() {
 				EMPTY_STRING_ARRAY, EMPTY_CLASS_ARRAY, activeProfiles1, loader);
 		MergedContextConfiguration mergedConfig2 = new MergedContextConfiguration(getClass(),
 				EMPTY_STRING_ARRAY, EMPTY_CLASS_ARRAY, activeProfiles2, loader);
-		assertNotEquals(mergedConfig1.hashCode(), mergedConfig2.hashCode());
+		assertEquals(mergedConfig1.hashCode(), mergedConfig2.hashCode());
 	}
 
 	@Test
@@ -339,7 +339,7 @@ public void equalsWithSameProfilesReversed() {
 				EMPTY_STRING_ARRAY, EMPTY_CLASS_ARRAY, activeProfiles1, loader);
 		MergedContextConfiguration mergedConfig2 = new MergedContextConfiguration(getClass(),
 				EMPTY_STRING_ARRAY, EMPTY_CLASS_ARRAY, activeProfiles2, loader);
-		assertNotEquals(mergedConfig1, mergedConfig2);
+		assertEquals(mergedConfig1, mergedConfig2);
 	}
 
 	@Test

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -90,8 +90,8 @@ public void verifyCacheKeyIsBasedOnActiveProfiles() {
 		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);
+		// Profiles {foo, bar} MUST 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);

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -65,12 +65,12 @@ public void resolveActiveProfilesWithEmptyProfiles() {
 
 	@Test
 	public void resolveActiveProfilesWithDuplicatedProfiles() {
-		assertResolvedProfiles(DuplicatedProfiles.class, "foo", "bar", "baz");
+		assertResolvedProfiles(DuplicatedProfiles.class, "bar", "baz", "foo");
 	}
 
 	@Test
 	public void resolveActiveProfilesWithLocalAndInheritedDuplicatedProfiles() {
-		assertResolvedProfiles(ExtendedDuplicatedProfiles.class, "foo", "bar", "baz", "cat", "dog");
+		assertResolvedProfiles(ExtendedDuplicatedProfiles.class, "bar", "baz", "cat", "dog", "foo");
 	}
 
 	@Test
@@ -90,12 +90,12 @@ public void resolveActiveProfilesWithInheritedAnnotationAndClasses() {
 
 	@Test
 	public void resolveActiveProfilesWithLocalAndInheritedAnnotations() {
-		assertResolvedProfiles(LocationsBar.class, "foo", "bar");
+		assertResolvedProfiles(LocationsBar.class, "bar", "foo");
 	}
 
 	@Test
 	public void resolveActiveProfilesWithOverriddenAnnotation() {
-		assertResolvedProfiles(Animals.class, "dog", "cat");
+		assertResolvedProfiles(Animals.class, "cat", "dog");
 	}
 
 	/**
@@ -127,15 +127,15 @@ public void resolveActiveProfilesWithMetaAnnotationAndOverriddenAttributes() {
 	 */
 	@Test
 	public void resolveActiveProfilesWithLocalAndInheritedMetaAnnotations() {
-		assertResolvedProfiles(MetaLocationsBar.class, "foo", "bar");
+		assertResolvedProfiles(MetaLocationsBar.class, "bar", "foo");
 	}
 
 	/**
 	 * @since 4.0
 	 */
 	@Test
 	public void resolveActiveProfilesWithOverriddenMetaAnnotation() {
-		assertResolvedProfiles(MetaAnimals.class, "dog", "cat");
+		assertResolvedProfiles(MetaAnimals.class, "cat", "dog");
 	}
 
 	/**
@@ -159,7 +159,7 @@ public void resolveActiveProfilesWithInheritedResolver() {
 	 */
 	@Test
 	public void resolveActiveProfilesWithMergedInheritedResolver() {
-		assertResolvedProfiles(MergedInheritedFooActiveProfilesResolverTestCase.class, "foo", "bar");
+		assertResolvedProfiles(MergedInheritedFooActiveProfilesResolverTestCase.class, "bar", "foo");
 	}
 
 	/**