From acbbf61be878f7924e29d71fe95fe001a8d29183 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Mon, 2 Nov 2020 16:32:07 +0100 Subject: [PATCH] Preserve registration order in @ActiveProfiles With this commit, bean definition profiles declared via @ActiveProfiles are once again stored in registration order, in order to support use cases in Spring Boot and other frameworks that depend on the registration order. This effectively reverts the changes made in conjunction with gh-25973. Closes gh-26004 --- .../test/context/ActiveProfiles.java | 4 +-- .../context/MergedContextConfiguration.java | 8 +++--- .../context/support/ActiveProfilesUtils.java | 25 +++++++++++++------ .../MergedContextConfigurationTests.java | 8 +++--- .../test/context/cache/ContextCacheTests.java | 6 ++--- .../support/ActiveProfilesUtilsTests.java | 14 +++++------ .../BootstrapTestUtilsMergedConfigTests.java | 12 +++++---- 7 files changed, 45 insertions(+), 32 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/test/context/ActiveProfiles.java b/spring-test/src/main/java/org/springframework/test/context/ActiveProfiles.java index 1bbaf8bee5..b9b8ddfd64 100644 --- a/spring-test/src/main/java/org/springframework/test/context/ActiveProfiles.java +++ b/spring-test/src/main/java/org/springframework/test/context/ActiveProfiles.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2019 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. @@ -83,7 +83,7 @@ public @interface ActiveProfiles { *

The default value is {@code true}, which means that a test * class will inherit bean definition profiles defined by a * test superclass. Specifically, the bean definition profiles for a test - * class will be added to the list of bean definition profiles + * class will be appended to the list of bean definition profiles * defined by a test superclass. Thus, subclasses have the option of * extending the list of bean definition profiles. *

If {@code inheritProfiles} is set to {@code false}, the bean 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 db37af2f02..f434b14022 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-2020 the original author or authors. + * Copyright 2002-2017 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 @@ 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.TreeSet; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextInitializer; @@ -533,8 +533,8 @@ public class MergedContextConfiguration implements Serializable { return EMPTY_STRING_ARRAY; } - // Active profiles must be unique and sorted - Set profilesSet = new TreeSet<>(Arrays.asList(activeProfiles)); + // 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 901b1d3a72..927cd9acc4 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,8 +16,11 @@ 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,8 +73,8 @@ abstract class ActiveProfilesUtils { static String[] resolveActiveProfiles(Class testClass) { Assert.notNull(testClass, "Class must not be null"); - Set activeProfiles = new TreeSet<>(); AnnotationDescriptor descriptor = findAnnotationDescriptor(testClass, ActiveProfiles.class); + List profileArrays = new ArrayList<>(); if (descriptor == null && logger.isDebugEnabled()) { logger.debug(String.format( @@ -107,16 +110,24 @@ abstract class ActiveProfilesUtils { String[] profiles = resolver.resolve(rootDeclaringClass); if (!ObjectUtils.isEmpty(profiles)) { - for (String profile : profiles) { - if (StringUtils.hasText(profile)) { - activeProfiles.add(profile.trim()); - } - } + profileArrays.add(profiles); } descriptor = (annotation.inheritProfiles() ? descriptor.next() : null); } + // Reverse the list so that we can traverse "down" the hierarchy. + Collections.reverse(profileArrays); + + Set activeProfiles = new LinkedHashSet<>(); + for (String[] profiles : profileArrays) { + for (String profile : profiles) { + if (StringUtils.hasText(profile)) { + activeProfiles.add(profile.trim()); + } + } + } + return StringUtils.toStringArray(activeProfiles); } 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 25c8530c49..002da9c991 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2019 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 @@ class MergedContextConfigurationTests { EMPTY_STRING_ARRAY, EMPTY_CLASS_ARRAY, activeProfiles1, loader); MergedContextConfiguration mergedConfig2 = new MergedContextConfiguration(getClass(), EMPTY_STRING_ARRAY, EMPTY_CLASS_ARRAY, activeProfiles2, loader); - assertThat(mergedConfig2).hasSameHashCodeAs(mergedConfig1); + assertThat(mergedConfig2.hashCode()).isNotEqualTo(mergedConfig1.hashCode()); } @Test @@ -339,13 +339,13 @@ class MergedContextConfigurationTests { EMPTY_STRING_ARRAY, EMPTY_CLASS_ARRAY, activeProfiles1, loader); MergedContextConfiguration mergedConfig2 = new MergedContextConfiguration(getClass(), EMPTY_STRING_ARRAY, EMPTY_CLASS_ARRAY, activeProfiles2, loader); - assertThat(mergedConfig2).isEqualTo(mergedConfig1); + assertThat(mergedConfig2).isNotEqualTo(mergedConfig1); } @Test void equalsWithSameDuplicateProfiles() { String[] activeProfiles1 = new String[] { "catbert", "dogbert" }; - String[] activeProfiles2 = new String[] { "dogbert", "catbert", "dogbert", "catbert" }; + String[] activeProfiles2 = new String[] { "catbert", "dogbert", "catbert", "dogbert", "catbert" }; MergedContextConfiguration mergedConfig1 = new MergedContextConfiguration(getClass(), EMPTY_STRING_ARRAY, EMPTY_CLASS_ARRAY, activeProfiles1, loader); MergedContextConfiguration mergedConfig2 = new MergedContextConfiguration(getClass(), 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 357535ce41..202af4bb4b 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2019 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 @@ class ContextCacheTests { int size = 0, hit = 0, miss = 0; loadCtxAndAssertStats(FooBarProfilesTestCase.class, ++size, hit, ++miss); loadCtxAndAssertStats(FooBarProfilesTestCase.class, size, ++hit, miss); - // Profiles {foo, bar} MUST hash to the same as {bar, foo} - loadCtxAndAssertStats(BarFooProfilesTestCase.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); 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 7228415d14..b192b34ee2 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 @@ -67,12 +67,12 @@ class ActiveProfilesUtilsTests extends AbstractContextConfigurationUtilsTests { @Test void resolveActiveProfilesWithDuplicatedProfiles() { - assertResolvedProfiles(DuplicatedProfiles.class, "bar", "baz", "foo"); + assertResolvedProfiles(DuplicatedProfiles.class, "foo", "bar", "baz"); } @Test void resolveActiveProfilesWithLocalAndInheritedDuplicatedProfiles() { - assertResolvedProfiles(ExtendedDuplicatedProfiles.class, "bar", "baz", "cat", "dog", "foo"); + assertResolvedProfiles(ExtendedDuplicatedProfiles.class, "foo", "bar", "baz", "cat", "dog"); } @Test @@ -92,12 +92,12 @@ class ActiveProfilesUtilsTests extends AbstractContextConfigurationUtilsTests { @Test void resolveActiveProfilesWithLocalAndInheritedAnnotations() { - assertResolvedProfiles(LocationsBar.class, "bar", "foo"); + assertResolvedProfiles(LocationsBar.class, "foo", "bar"); } @Test void resolveActiveProfilesWithOverriddenAnnotation() { - assertResolvedProfiles(Animals.class, "cat", "dog"); + assertResolvedProfiles(Animals.class, "dog", "cat"); } /** @@ -129,7 +129,7 @@ class ActiveProfilesUtilsTests extends AbstractContextConfigurationUtilsTests { */ @Test void resolveActiveProfilesWithLocalAndInheritedMetaAnnotations() { - assertResolvedProfiles(MetaLocationsBar.class, "bar", "foo"); + assertResolvedProfiles(MetaLocationsBar.class, "foo", "bar"); } /** @@ -137,7 +137,7 @@ class ActiveProfilesUtilsTests extends AbstractContextConfigurationUtilsTests { */ @Test void resolveActiveProfilesWithOverriddenMetaAnnotation() { - assertResolvedProfiles(MetaAnimals.class, "cat", "dog"); + assertResolvedProfiles(MetaAnimals.class, "dog", "cat"); } /** @@ -161,7 +161,7 @@ class ActiveProfilesUtilsTests extends AbstractContextConfigurationUtilsTests { */ @Test void resolveActiveProfilesWithMergedInheritedResolver() { - assertResolvedProfiles(MergedInheritedFooActiveProfilesResolverTestCase.class, "bar", "foo"); + assertResolvedProfiles(MergedInheritedFooActiveProfilesResolverTestCase.class, "foo", "bar"); } /** diff --git a/spring-test/src/test/java/org/springframework/test/context/support/BootstrapTestUtilsMergedConfigTests.java b/spring-test/src/test/java/org/springframework/test/context/support/BootstrapTestUtilsMergedConfigTests.java index 4336ec5c83..c44b90e9aa 100644 --- a/spring-test/src/test/java/org/springframework/test/context/support/BootstrapTestUtilsMergedConfigTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/support/BootstrapTestUtilsMergedConfigTests.java @@ -373,6 +373,8 @@ class BootstrapTestUtilsMergedConfigTests extends AbstractContextConfigurationUt void buildMergedConfigWithDuplicateConfigurationOnEnclosingClassAndNestedClass() { compareApplesToApples(AppleConfigTestCase.class, AppleConfigTestCase.Nested.class); compareApplesToApples(AppleConfigTestCase.Nested.class, AppleConfigTestCase.Nested.DoubleNested.class); + compareApplesToOranges(ApplesAndOrangesConfigTestCase.class, ApplesAndOrangesConfigTestCase.Nested.class); + compareApplesToOranges(ApplesAndOrangesConfigTestCase.Nested.class, ApplesAndOrangesConfigTestCase.Nested.DoubleNested.class); } private void compareApplesToApples(Class parent, Class child) { @@ -400,7 +402,7 @@ class BootstrapTestUtilsMergedConfigTests extends AbstractContextConfigurationUt DelegatingSmartContextLoader.class); assertThat(parentMergedConfig.getActiveProfiles()).as("active profiles") - .containsExactly("apples", "oranges") + .containsExactly("oranges", "apples") .isEqualTo(childMergedConfig.getActiveProfiles()); assertThat(parentMergedConfig).isEqualTo(childMergedConfig); } @@ -531,7 +533,7 @@ class BootstrapTestUtilsMergedConfigTests extends AbstractContextConfigurationUt } @ContextConfiguration(classes = AppleConfig.class) - @ActiveProfiles({"apples", "oranges"}) + @ActiveProfiles({"oranges", "apples"}) static class ApplesAndOrangesConfigTestCase { @ContextConfiguration(classes = AppleConfig.class) @@ -539,19 +541,19 @@ class BootstrapTestUtilsMergedConfigTests extends AbstractContextConfigurationUt class Nested { @ContextConfiguration(classes = AppleConfig.class) - @ActiveProfiles(profiles = {"apples", "oranges", "apples"}, inheritProfiles = false) + @ActiveProfiles(profiles = {"oranges", "apples", "oranges"}, inheritProfiles = false) class DoubleNested { } } } @ContextConfiguration(classes = AppleConfig.class) - @ActiveProfiles(profiles = {"oranges", "apples"}, inheritProfiles = false) + @ActiveProfiles(profiles = {"oranges", "apples", "oranges"}, inheritProfiles = false) static class DuplicateConfigApplesAndOrangesConfigTestCase extends ApplesAndOrangesConfigTestCase { } @ContextConfiguration(classes = AppleConfig.class) - @ActiveProfiles(profiles = {"apples", "oranges", "apples"}, inheritProfiles = false) + @ActiveProfiles(profiles = {"oranges", "apples", "oranges"}, inheritProfiles = false) static class SubDuplicateConfigApplesAndOrangesConfigTestCase extends DuplicateConfigApplesAndOrangesConfigTestCase { }