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
This commit is contained in:
Sam Brannen 2020-10-26 11:31:54 +01:00
parent 9d70d2023c
commit c840ba9989
7 changed files with 28 additions and 39 deletions

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 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 @@ public @interface ActiveProfiles {
* <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

View File

@ -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 @@ 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
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);
}

View File

@ -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 @@ abstract class ActiveProfilesUtils {
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);
}

View File

@ -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);

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 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.
@ -339,7 +339,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).isNotEqualTo(mergedConfig1);
assertThat(mergedConfig2).isEqualTo(mergedConfig1);
}
@Test

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 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 @@ class ContextCacheTests {
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);

View File

@ -67,12 +67,12 @@ class ActiveProfilesUtilsTests extends AbstractContextConfigurationUtilsTests {
@Test
void resolveActiveProfilesWithDuplicatedProfiles() {
assertResolvedProfiles(DuplicatedProfiles.class, "foo", "bar", "baz");
assertResolvedProfiles(DuplicatedProfiles.class, "bar", "baz", "foo");
}
@Test
void resolveActiveProfilesWithLocalAndInheritedDuplicatedProfiles() {
assertResolvedProfiles(ExtendedDuplicatedProfiles.class, "foo", "bar", "baz", "cat", "dog");
assertResolvedProfiles(ExtendedDuplicatedProfiles.class, "bar", "baz", "cat", "dog", "foo");
}
@Test
@ -92,12 +92,12 @@ class ActiveProfilesUtilsTests extends AbstractContextConfigurationUtilsTests {
@Test
void resolveActiveProfilesWithLocalAndInheritedAnnotations() {
assertResolvedProfiles(LocationsBar.class, "foo", "bar");
assertResolvedProfiles(LocationsBar.class, "bar", "foo");
}
@Test
void resolveActiveProfilesWithOverriddenAnnotation() {
assertResolvedProfiles(Animals.class, "dog", "cat");
assertResolvedProfiles(Animals.class, "cat", "dog");
}
/**
@ -129,7 +129,7 @@ class ActiveProfilesUtilsTests extends AbstractContextConfigurationUtilsTests {
*/
@Test
void resolveActiveProfilesWithLocalAndInheritedMetaAnnotations() {
assertResolvedProfiles(MetaLocationsBar.class, "foo", "bar");
assertResolvedProfiles(MetaLocationsBar.class, "bar", "foo");
}
/**
@ -137,7 +137,7 @@ class ActiveProfilesUtilsTests extends AbstractContextConfigurationUtilsTests {
*/
@Test
void resolveActiveProfilesWithOverriddenMetaAnnotation() {
assertResolvedProfiles(MetaAnimals.class, "dog", "cat");
assertResolvedProfiles(MetaAnimals.class, "cat", "dog");
}
/**
@ -161,7 +161,7 @@ class ActiveProfilesUtilsTests extends AbstractContextConfigurationUtilsTests {
*/
@Test
void resolveActiveProfilesWithMergedInheritedResolver() {
assertResolvedProfiles(MergedInheritedFooActiveProfilesResolverTestCase.class, "foo", "bar");
assertResolvedProfiles(MergedInheritedFooActiveProfilesResolverTestCase.class, "bar", "foo");
}
/**