From 68a704373dfcc3438eabb98f0767aed9cec4f95b Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Sun, 14 Jun 2015 23:35:23 +0200 Subject: [PATCH] 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 --- .../context/MergedContextConfiguration.java | 18 +++++++------- .../context/support/ActiveProfilesUtils.java | 21 ++++++++++++---- .../DefaultActiveProfilesResolver.java | 4 ++-- .../MergedContextConfigurationTests.java | 4 ++-- .../test/context/cache/ContextCacheTests.java | 17 ++++++------- ...bstractContextConfigurationUtilsTests.java | 10 ++++---- .../support/ActiveProfilesUtilsTests.java | 24 ++++++++++--------- 7 files changed, 55 insertions(+), 43 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/test/context/MergedContextConfiguration.java b/spring-test/src/main/java/org/springframework/test/context/MergedContextConfiguration.java index f72dce31da..5212a04071 100644 --- a/spring-test/src/main/java/org/springframework/test/context/MergedContextConfiguration.java +++ b/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; *

A {@link SmartContextLoader} uses {@code MergedContextConfiguration} * to load an {@link org.springframework.context.ApplicationContext ApplicationContext}. * - *

{@code MergedContextConfiguration} is also used by the {@link TestContext} - * as the context cache key for caching an + *

{@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 sortedProfilesSet = new TreeSet(Arrays.asList(activeProfiles)); - return StringUtils.toStringArray(sortedProfilesSet); + // Active profiles must be unique + Set profilesSet = new LinkedHashSet(Arrays.asList(activeProfiles)); + return StringUtils.toStringArray(profilesSet); } /** diff --git a/spring-test/src/main/java/org/springframework/test/context/support/ActiveProfilesUtils.java b/spring-test/src/main/java/org/springframework/test/context/support/ActiveProfilesUtils.java index 15a281a4de..c2025424c5 100644 --- a/spring-test/src/main/java/org/springframework/test/context/support/ActiveProfilesUtils.java +++ b/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 activeProfiles = new HashSet(); + final List profileArrays = new ArrayList(); Class annotationType = ActiveProfiles.class; AnnotationDescriptor 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 activeProfiles = new LinkedHashSet(); + 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); diff --git a/spring-test/src/main/java/org/springframework/test/context/support/DefaultActiveProfilesResolver.java b/spring-test/src/main/java/org/springframework/test/context/support/DefaultActiveProfilesResolver.java index d03f0f1c1a..e1de32a8cc 100644 --- a/spring-test/src/main/java/org/springframework/test/context/support/DefaultActiveProfilesResolver.java +++ b/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 activeProfiles = new HashSet(); + final Set activeProfiles = new LinkedHashSet(); Class annotationType = ActiveProfiles.class; AnnotationDescriptor descriptor = findAnnotationDescriptor(testClass, annotationType); diff --git a/spring-test/src/test/java/org/springframework/test/context/MergedContextConfigurationTests.java b/spring-test/src/test/java/org/springframework/test/context/MergedContextConfigurationTests.java index ea0521afc3..bf8eba0403 100644 --- a/spring-test/src/test/java/org/springframework/test/context/MergedContextConfigurationTests.java +++ b/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 diff --git a/spring-test/src/test/java/org/springframework/test/context/cache/ContextCacheTests.java b/spring-test/src/test/java/org/springframework/test/context/cache/ContextCacheTests.java index be0440a504..5d518d3c8f 100644 --- a/spring-test/src/test/java/org/springframework/test/context/cache/ContextCacheTests.java +++ b/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 diff --git a/spring-test/src/test/java/org/springframework/test/context/support/AbstractContextConfigurationUtilsTests.java b/spring-test/src/test/java/org/springframework/test/context/support/AbstractContextConfigurationUtilsTests.java index 6663128e8b..23eade78e0 100644 --- a/spring-test/src/test/java/org/springframework/test/context/support/AbstractContextConfigurationUtilsTests.java +++ b/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 { } diff --git a/spring-test/src/test/java/org/springframework/test/context/support/ActiveProfilesUtilsTests.java b/spring-test/src/test/java/org/springframework/test/context/support/ActiveProfilesUtilsTests.java index ceb19dde6d..f9f4013c39 100644 --- a/spring-test/src/test/java/org/springframework/test/context/support/ActiveProfilesUtilsTests.java +++ b/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 expectedSet = new HashSet(Arrays.asList(expected)); - Set actualSet = new HashSet(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 { }