From 07cfcbc3a936c2e92d1f64941c79faee48649764 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Wed, 6 Jul 2022 11:42:39 +0200 Subject: [PATCH 1/2] Move convention-based attribute override tests to @Nested class --- .../annotation/MergedAnnotationsTests.java | 127 +++++++++--------- 1 file changed, 65 insertions(+), 62 deletions(-) diff --git a/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java b/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java index bd32cdbe0a2..0b16e2f25c0 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java +++ b/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java @@ -38,6 +38,7 @@ import java.util.stream.Stream; import javax.annotation.Resource; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.springframework.core.Ordered; @@ -75,6 +76,70 @@ import static org.assertj.core.api.Assertions.entry; */ class MergedAnnotationsTests { + @Nested + class ConventionBasedAnnotationAttributeOverrideTests { + + @Test + void getWithInheritedAnnotationsAttributesWithConventionBasedComposedAnnotation() { + MergedAnnotation annotation = + MergedAnnotations.from(ConventionBasedComposedContextConfigurationClass.class, + SearchStrategy.INHERITED_ANNOTATIONS).get(ContextConfiguration.class); + assertThat(annotation.isPresent()).isTrue(); + assertThat(annotation.getStringArray("locations")).containsExactly("explicitDeclaration"); + assertThat(annotation.getStringArray("value")).containsExactly("explicitDeclaration"); + } + + @Test + void getWithInheritedAnnotationsFromHalfConventionBasedAndHalfAliasedComposedAnnotation1() { + // SPR-13554: convention mapping mixed with AliasFor annotations + // xmlConfigFiles can be used because it has an AliasFor annotation + MergedAnnotation annotation = + MergedAnnotations.from(HalfConventionBasedAndHalfAliasedComposedContextConfigurationClass1.class, + SearchStrategy.INHERITED_ANNOTATIONS).get(ContextConfiguration.class); + assertThat(annotation.getStringArray("locations")).containsExactly("explicitDeclaration"); + assertThat(annotation.getStringArray("value")).containsExactly("explicitDeclaration"); + } + + @Test + void withInheritedAnnotationsFromHalfConventionBasedAndHalfAliasedComposedAnnotation2() { + // SPR-13554: convention mapping mixed with AliasFor annotations + // locations doesn't apply because it has no AliasFor annotation + MergedAnnotation annotation = + MergedAnnotations.from(HalfConventionBasedAndHalfAliasedComposedContextConfigurationClass2.class, + SearchStrategy.INHERITED_ANNOTATIONS).get(ContextConfiguration.class); + assertThat(annotation.getStringArray("locations")).isEmpty(); + assertThat(annotation.getStringArray("value")).isEmpty(); + } + + @Test + void getWithInheritedAnnotationsFromInvalidConventionBasedComposedAnnotation() { + assertThatExceptionOfType(AnnotationConfigurationException.class) + .isThrownBy(() -> MergedAnnotations.from(InvalidConventionBasedComposedContextConfigurationClass.class, + SearchStrategy.INHERITED_ANNOTATIONS).get(ContextConfiguration.class)); + } + + @Test + void getWithTypeHierarchyWithSingleElementOverridingAnArrayViaConvention() { + testGetWithTypeHierarchy(ConventionBasedSinglePackageComponentScanClass.class, "com.example.app.test"); + } + + @Test + void getWithTypeHierarchyWithLocalAliasesThatConflictWithAttributesInMetaAnnotationByConvention() { + MergedAnnotation annotation = + MergedAnnotations.from(SpringApplicationConfigurationClass.class, SearchStrategy.TYPE_HIERARCHY) + .get(ContextConfiguration.class); + assertThat(annotation.getStringArray("locations")).isEmpty(); + assertThat(annotation.getStringArray("value")).isEmpty(); + assertThat(annotation.getClassArray("classes")).containsExactly(Number.class); + } + + @Test + void getWithTypeHierarchyOnMethodWithSingleElementOverridingAnArrayViaConvention() throws Exception { + testGetWithTypeHierarchyWebMapping(WebController.class.getMethod("postMappedWithPathAttribute")); + } + + } + @Test void fromPreconditions() { SearchStrategy strategy = SearchStrategy.DIRECT; @@ -347,41 +412,7 @@ class MergedAnnotationsTests { assertThat(annotation.isPresent()).isTrue(); } - @Test - void getWithInheritedAnnotationsAttributesWithConventionBasedComposedAnnotation() { - MergedAnnotation annotation = MergedAnnotations.from( - ConventionBasedComposedContextConfigurationClass.class, - SearchStrategy.INHERITED_ANNOTATIONS).get(ContextConfiguration.class); - assertThat(annotation.isPresent()).isTrue(); - assertThat(annotation.getStringArray("locations")).containsExactly( - "explicitDeclaration"); - assertThat(annotation.getStringArray("value")).containsExactly( - "explicitDeclaration"); - } - @Test - void getWithInheritedAnnotationsFromHalfConventionBasedAndHalfAliasedComposedAnnotation1() { - // SPR-13554: convention mapping mixed with AliasFor annotations - // xmlConfigFiles can be used because it has an AliasFor annotation - MergedAnnotation annotation = MergedAnnotations.from( - HalfConventionBasedAndHalfAliasedComposedContextConfigurationClass1.class, - SearchStrategy.INHERITED_ANNOTATIONS).get(ContextConfiguration.class); - assertThat(annotation.getStringArray("locations")).containsExactly( - "explicitDeclaration"); - assertThat(annotation.getStringArray("value")).containsExactly( - "explicitDeclaration"); - } - - @Test - void withInheritedAnnotationsFromHalfConventionBasedAndHalfAliasedComposedAnnotation2() { - // SPR-13554: convention mapping mixed with AliasFor annotations - // locations doesn't apply because it has no AliasFor annotation - MergedAnnotation annotation = MergedAnnotations.from( - HalfConventionBasedAndHalfAliasedComposedContextConfigurationClass2.class, - SearchStrategy.INHERITED_ANNOTATIONS).get(ContextConfiguration.class); - assertThat(annotation.getStringArray("locations")).isEmpty(); - assertThat(annotation.getStringArray("value")).isEmpty(); - } @Test void withInheritedAnnotationsFromAliasedComposedAnnotation() { @@ -459,13 +490,6 @@ class MergedAnnotationsTests { assertThat(annotation.getClassArray("classes")).isEmpty(); } - @Test - void getWithInheritedAnnotationsFromInvalidConventionBasedComposedAnnotation() { - assertThatExceptionOfType(AnnotationConfigurationException.class).isThrownBy(() -> - MergedAnnotations.from(InvalidConventionBasedComposedContextConfigurationClass.class, - SearchStrategy.INHERITED_ANNOTATIONS).get(ContextConfiguration.class)); - } - @Test void getWithInheritedAnnotationsFromShadowedAliasComposedAnnotation() { MergedAnnotation annotation = MergedAnnotations.from( @@ -628,11 +652,6 @@ class MergedAnnotationsTests { testGetWithTypeHierarchy(ComponentScanWithBasePackagesAndValueAliasClass.class, "com.example.app.test"); } - @Test - void getWithTypeHierarchyWithSingleElementOverridingAnArrayViaConvention() { - testGetWithTypeHierarchy(ConventionBasedSinglePackageComponentScanClass.class, "com.example.app.test"); - } - @Test void getWithTypeHierarchyWithSingleElementOverridingAnArrayViaAliasFor() { testGetWithTypeHierarchy(AliasForBasedSinglePackageComponentScanClass.class, "com.example.app.test"); @@ -661,22 +680,6 @@ class MergedAnnotationsTests { "test.properties"); } - @Test - void getWithTypeHierarchyWithLocalAliasesThatConflictWithAttributesInMetaAnnotationByConvention() { - MergedAnnotation annotation = MergedAnnotations.from( - SpringApplicationConfigurationClass.class, SearchStrategy.TYPE_HIERARCHY).get( - ContextConfiguration.class); - assertThat(annotation.getStringArray("locations")).isEmpty(); - assertThat(annotation.getStringArray("value")).isEmpty(); - assertThat(annotation.getClassArray("classes")).containsExactly(Number.class); - } - - @Test - void getWithTypeHierarchyOnMethodWithSingleElementOverridingAnArrayViaConvention() throws Exception { - testGetWithTypeHierarchyWebMapping( - WebController.class.getMethod("postMappedWithPathAttribute")); - } - @Test void getWithTypeHierarchyOnMethodWithSingleElementOverridingAnArrayViaAliasFor() throws Exception { testGetWithTypeHierarchyWebMapping( From 07960d49187fa447226c749ebd14e234666f1ee4 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Wed, 6 Jul 2022 11:44:15 +0200 Subject: [PATCH 2/2] Polishing --- .../springframework/core/annotation/AnnotationTypeMapping.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMapping.java b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMapping.java index 77302db78ae..4e072238765 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMapping.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMapping.java @@ -271,10 +271,10 @@ final class AnnotationTypeMapping { int[] mappings = this.conventionMappings; for (int i = 0; i < mappings.length; i++) { String name = this.attributes.get(i).getName(); - MirrorSet mirrors = getMirrorSets().getAssigned(i); int mapped = rootAttributes.indexOf(name); if (!MergedAnnotation.VALUE.equals(name) && mapped != -1) { mappings[i] = mapped; + MirrorSet mirrors = getMirrorSets().getAssigned(i); if (mirrors != null) { for (int j = 0; j < mirrors.size(); j++) { mappings[mirrors.getAttributeIndex(j)] = mapped; @@ -518,7 +518,6 @@ final class AnnotationTypeMapping { * @return {@code true} if the value is equivalent to the default value */ boolean isEquivalentToDefaultValue(int attributeIndex, Object value, ValueExtractor valueExtractor) { - Method attribute = this.attributes.get(attributeIndex); return isEquivalentToDefaultValue(attribute, value, valueExtractor); }