From 3a2e2cd5f2e2580c44204caead2c11b3626c98dd Mon Sep 17 00:00:00 2001 From: Laurent MARTELLI Date: Wed, 5 Apr 2023 00:16:12 +0200 Subject: [PATCH 1/2] ImportsContextCustomizer uses MergedAnnotation --- .../context/ImportsContextCustomizer.java | 170 +++++------------- .../ImportsContextCustomizerTests.java | 95 ++++++++++ 2 files changed, 140 insertions(+), 125 deletions(-) diff --git a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/ImportsContextCustomizer.java b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/ImportsContextCustomizer.java index 039ebbc76e1a..2fbe44555c49 100644 --- a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/ImportsContextCustomizer.java +++ b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/ImportsContextCustomizer.java @@ -16,13 +16,13 @@ package org.springframework.boot.test.context; -import java.lang.annotation.Annotation; -import java.lang.reflect.AnnotatedElement; import java.lang.reflect.Constructor; import java.util.Collections; import java.util.HashSet; -import java.util.LinkedHashSet; import java.util.Set; +import java.util.function.Predicate; +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanFactory; @@ -42,7 +42,9 @@ import org.springframework.context.annotation.ImportSelector; import org.springframework.context.support.AbstractApplicationContext; import org.springframework.core.Ordered; -import org.springframework.core.annotation.AnnotationUtils; +import org.springframework.core.annotation.AnnotatedElementUtils; +import org.springframework.core.annotation.MergedAnnotation; +import org.springframework.core.annotation.MergedAnnotations; import org.springframework.core.annotation.Order; import org.springframework.core.style.ToStringCreator; import org.springframework.core.type.AnnotationMetadata; @@ -214,80 +216,40 @@ public void postProcessBeanDefinitionRegistry(BeanDefinitionRegistry registry) t */ static class ContextCustomizerKey { - private static final Class[] NO_IMPORTS = {}; + private static final Predicate IGNORE_ANNOTATION = or(packages("java.lang.annotation"), + packages("org.spockframework", "spock"), + or(Predicate.isEqual("kotlin.Metadata"), isInKotlinAnnotationPackage()), packages(("org.junit"))); - private static final Set ANNOTATION_FILTERS; - - static { - Set filters = new HashSet<>(); - filters.add(new JavaLangAnnotationFilter()); - filters.add(new KotlinAnnotationFilter()); - filters.add(new SpockAnnotationFilter()); - filters.add(new JUnitAnnotationFilter()); - ANNOTATION_FILTERS = Collections.unmodifiableSet(filters); - } - - private final Set key; + private final Object key; ContextCustomizerKey(Class testClass) { - Set annotations = new HashSet<>(); - Set> seen = new HashSet<>(); - collectClassAnnotations(testClass, annotations, seen); - Set determinedImports = determineImports(annotations, testClass); - this.key = Collections.unmodifiableSet((determinedImports != null) ? determinedImports : annotations); - } - - private void collectClassAnnotations(Class classType, Set annotations, Set> seen) { - if (seen.add(classType)) { - collectElementAnnotations(classType, annotations, seen); - for (Class interfaceType : classType.getInterfaces()) { - collectClassAnnotations(interfaceType, annotations, seen); - } - if (classType.getSuperclass() != null) { - collectClassAnnotations(classType.getSuperclass(), annotations, seen); - } + var mergedAnnotations = MergedAnnotations.search(MergedAnnotations.SearchStrategy.TYPE_HIERARCHY) + .withAnnotationFilter(IGNORE_ANNOTATION::test) + .from(testClass); + var determinedImports = determineImports(mergedAnnotations, testClass); + if (determinedImports != null) { + this.key = determinedImports; } - } - - private void collectElementAnnotations(AnnotatedElement element, Set annotations, - Set> seen) { - for (Annotation annotation : element.getDeclaredAnnotations()) { - if (!isIgnoredAnnotation(annotation)) { - annotations.add(annotation); - collectClassAnnotations(annotation.annotationType(), annotations, seen); - } + else { + this.key = AnnotatedElementUtils.findAllMergedAnnotations(testClass, + mergedAnnotations.stream().map(MergedAnnotation::getType).collect(Collectors.toSet())); } } - private boolean isIgnoredAnnotation(Annotation annotation) { - for (AnnotationFilter annotationFilter : ANNOTATION_FILTERS) { - if (annotationFilter.isIgnored(annotation)) { - return true; - } - } - return false; - } - - private Set determineImports(Set annotations, Class testClass) { - Set determinedImports = new LinkedHashSet<>(); + private Set determineImports(MergedAnnotations mergedAnnotations, Class testClass) { AnnotationMetadata testClassMetadata = AnnotationMetadata.introspect(testClass); - for (Annotation annotation : annotations) { - for (Class source : getImports(annotation)) { - Set determinedSourceImports = determineImports(source, testClassMetadata); - if (determinedSourceImports == null) { + return mergedAnnotations.stream(Import.class) + .flatMap((ma) -> Stream.of(ma.getClassArray("value"))) + .map((source) -> determineImports(source, testClassMetadata)) + .reduce(new HashSet<>(), (a, b) -> { + if (a == null || b == null) { return null; } - determinedImports.addAll(determinedSourceImports); - } - } - return determinedImports; - } - - private Class[] getImports(Annotation annotation) { - if (annotation instanceof Import importAnnotation) { - return importAnnotation.value(); - } - return NO_IMPORTS; + else { + a.add(b); + return a; + } + }); } private Set determineImports(Class source, AnnotationMetadata metadata) { @@ -333,67 +295,25 @@ public String toString() { return this.key.toString(); } - } - - /** - * Filter used to limit considered annotations. - */ - private interface AnnotationFilter { - - boolean isIgnored(Annotation annotation); - - } - - /** - * {@link AnnotationFilter} for {@literal java.lang} annotations. - */ - private static final class JavaLangAnnotationFilter implements AnnotationFilter { - - @Override - public boolean isIgnored(Annotation annotation) { - return AnnotationUtils.isInJavaLangAnnotationPackage(annotation); - } - - } - - /** - * {@link AnnotationFilter} for Kotlin annotations. - */ - private static final class KotlinAnnotationFilter implements AnnotationFilter { - - @Override - public boolean isIgnored(Annotation annotation) { - return "kotlin.Metadata".equals(annotation.annotationType().getName()) - || isInKotlinAnnotationPackage(annotation); - } - - private boolean isInKotlinAnnotationPackage(Annotation annotation) { - return annotation.annotationType().getName().startsWith("kotlin.annotation."); + private static Predicate isInKotlinAnnotationPackage() { + return packages("kotlin.annotation"); } - } - - /** - * {@link AnnotationFilter} for Spock annotations. - */ - private static final class SpockAnnotationFilter implements AnnotationFilter { - - @Override - public boolean isIgnored(Annotation annotation) { - return annotation.annotationType().getName().startsWith("org.spockframework.") - || annotation.annotationType().getName().startsWith("spock."); + @SafeVarargs + private static Predicate or(Predicate... predicates) { + return (x) -> { + for (var predicate : predicates) { + if (predicate.test(x)) { + return true; + } + } + return false; + }; } - } - - /** - * {@link AnnotationFilter} for JUnit annotations. - */ - private static final class JUnitAnnotationFilter implements AnnotationFilter { - - @Override - public boolean isIgnored(Annotation annotation) { - return annotation.annotationType().getName().startsWith("org.junit."); + private static Predicate packages(String... packages) { + var starts = Stream.of(packages).map((p) -> p + ".").toList(); + return (packageName) -> starts.stream().anyMatch(packageName::startsWith); } } diff --git a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/ImportsContextCustomizerTests.java b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/ImportsContextCustomizerTests.java index f047bf98406d..5f36e1a05c3b 100644 --- a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/ImportsContextCustomizerTests.java +++ b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/ImportsContextCustomizerTests.java @@ -33,6 +33,7 @@ import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; import org.springframework.context.annotation.ImportSelector; +import org.springframework.core.annotation.AliasFor; import org.springframework.core.type.AnnotationMetadata; import static org.assertj.core.api.Assertions.assertThat; @@ -80,6 +81,30 @@ void customizersForTestClassesWithDifferentJUnitAnnotationsAreEqual() { .isEqualTo(new ImportsContextCustomizer(SecondJUnitAnnotatedTestClass.class)); } + @Test + void customizersForClassesWithDifferentImportsAreNotEqual() { + assertThat(new ImportsContextCustomizer(FirstAnnotatedTestClass.class)) + .isNotEqualTo(new ImportsContextCustomizer(SecondAnnotatedTestClass.class)); + } + + @Test + void customizersForClassesWithDifferentMetaImportsAreNotEqual() { + assertThat(new ImportsContextCustomizer(FirstMetaAnnotatedTestClass.class)) + .isNotEqualTo(new ImportsContextCustomizer(SecondMetaAnnotatedTestClass.class)); + } + + @Test + void customizersForClassesWithDifferentAliasedImportsAreNotEqual() { + assertThat(new ImportsContextCustomizer(FirstAliasAnnotatedTestClass.class)) + .isNotEqualTo(new ImportsContextCustomizer(SecondAliasAnnotatedTestClass.class)); + } + + @Test + void importsCanBeScatteredOnMultipleAnnotations() { + assertThat(new ImportsContextCustomizer(SingleImportAnnotationTestClass.class)) + .isEqualTo(new ImportsContextCustomizer(MultipleImportAnnotationTestClass.class)); + } + @Import(TestImportSelector.class) @Indicator1 static class FirstImportSelectorAnnotatedClass { @@ -152,6 +177,17 @@ static class SecondJUnitAnnotatedTestClass { } + @Import({ FirstImportedClass.class, SecondImportedClass.class }) + static class SingleImportAnnotationTestClass { + + } + + @FirstMetaImport + @Import(SecondImportedClass.class) + static class MultipleImportAnnotationTestClass { + + } + @Retention(RetentionPolicy.RUNTIME) @interface Indicator1 { @@ -162,6 +198,65 @@ static class SecondJUnitAnnotatedTestClass { } + @Retention(RetentionPolicy.RUNTIME) + @Import(AliasFor.class) + public @interface AliasedImport { + + @AliasFor(annotation = Import.class) + Class[] value(); + + } + + @Retention(RetentionPolicy.RUNTIME) + @Import(FirstImportedClass.class) + public @interface FirstMetaImport { + + } + + @Retention(RetentionPolicy.RUNTIME) + @Import(SecondImportedClass.class) + public @interface SecondMetaImport { + + } + + static class FirstImportedClass { + + } + + static class SecondImportedClass { + + } + + @AliasedImport(FirstImportedClass.class) + static class FirstAliasAnnotatedTestClass { + + } + + @AliasedImport(SecondImportedClass.class) + static class SecondAliasAnnotatedTestClass { + + } + + @FirstMetaImport + static class FirstMetaAnnotatedTestClass { + + } + + @SecondMetaImport + static class SecondMetaAnnotatedTestClass { + + } + + @Import(FirstImportedClass.class) + static class FirstAnnotatedTestClass { + + } + + @Import(SecondImportedClass.class) + static class SecondAnnotatedTestClass { + + } + static class TestImportSelector implements ImportSelector { @Override From 0f4b30f068926721e8fdf77354b83e61e5e38f1a Mon Sep 17 00:00:00 2001 From: Laurent MARTELLI Date: Sat, 15 Apr 2023 00:20:47 +0200 Subject: [PATCH 2/2] Use AnnotationFilter instead of Predicate, explicit types instead of "var" --- .../context/ImportsContextCustomizer.java | 47 ++++++++----------- 1 file changed, 20 insertions(+), 27 deletions(-) diff --git a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/ImportsContextCustomizer.java b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/ImportsContextCustomizer.java index 2fbe44555c49..bc0bdb55d9d8 100644 --- a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/ImportsContextCustomizer.java +++ b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/ImportsContextCustomizer.java @@ -20,7 +20,6 @@ import java.util.Collections; import java.util.HashSet; import java.util.Set; -import java.util.function.Predicate; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -43,6 +42,7 @@ import org.springframework.context.support.AbstractApplicationContext; import org.springframework.core.Ordered; import org.springframework.core.annotation.AnnotatedElementUtils; +import org.springframework.core.annotation.AnnotationFilter; import org.springframework.core.annotation.MergedAnnotation; import org.springframework.core.annotation.MergedAnnotations; import org.springframework.core.annotation.Order; @@ -52,6 +52,8 @@ import org.springframework.test.context.MergedContextConfiguration; import org.springframework.util.ReflectionUtils; +import static org.springframework.core.annotation.AnnotationFilter.packages; + /** * {@link ContextCustomizer} to allow {@code @Import} annotations to be used directly on * test classes. @@ -216,17 +218,20 @@ public void postProcessBeanDefinitionRegistry(BeanDefinitionRegistry registry) t */ static class ContextCustomizerKey { - private static final Predicate IGNORE_ANNOTATION = or(packages("java.lang.annotation"), + private static final AnnotationFilter ANNOTATION_FILTERS = or( + packages("java.lang.annotation"), packages("org.spockframework", "spock"), - or(Predicate.isEqual("kotlin.Metadata"), isInKotlinAnnotationPackage()), packages(("org.junit"))); + or(isEqualTo("kotlin.Metadata"), packages("kotlin.annotation")), + packages(("org.junit"))); private final Object key; ContextCustomizerKey(Class testClass) { - var mergedAnnotations = MergedAnnotations.search(MergedAnnotations.SearchStrategy.TYPE_HIERARCHY) - .withAnnotationFilter(IGNORE_ANNOTATION::test) - .from(testClass); - var determinedImports = determineImports(mergedAnnotations, testClass); + MergedAnnotations mergedAnnotations = + MergedAnnotations.search(MergedAnnotations.SearchStrategy.TYPE_HIERARCHY) + .withAnnotationFilter(ANNOTATION_FILTERS) + .from(testClass); + Set determinedImports = determineImports(mergedAnnotations, testClass); if (determinedImports != null) { this.key = determinedImports; } @@ -295,27 +300,15 @@ public String toString() { return this.key.toString(); } - private static Predicate isInKotlinAnnotationPackage() { - return packages("kotlin.annotation"); - } - - @SafeVarargs - private static Predicate or(Predicate... predicates) { - return (x) -> { - for (var predicate : predicates) { - if (predicate.test(x)) { - return true; - } - } - return false; - }; - } - - private static Predicate packages(String... packages) { - var starts = Stream.of(packages).map((p) -> p + ".").toList(); - return (packageName) -> starts.stream().anyMatch(packageName::startsWith); - } + } + static AnnotationFilter or(AnnotationFilter... filters) { + return typeName -> + Stream.of(filters) + .anyMatch(filter -> filter.matches(typeName)); } + static AnnotationFilter isEqualTo(String expectedTypeName) { + return typeName -> typeName.equals(expectedTypeName); + } }