From 974cacac31a2e2d7fa784e962b6a66e6e72dfff4 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Sun, 19 Jan 2020 16:55:46 +0100 Subject: [PATCH] Support nested annotations in ASM-based processing again Spring Framework 5.0 introduced a regression in ASM-based annotation processing. Specifically, nested annotations were no longer supported, and component scanning resulted in an exception if a candidate component was annotated with an annotation that contained nested annotations. This commit fixes this regression by introducing special handling in AnnotationTypeMapping that supports extracting values from objects of type TypeMappedAnnotation when necessary. Closes gh-24375 --- ...Component.java => AnnotatedComponent.java} | 4 +-- .../{A.java => EnclosingAnnotation.java} | 11 ++++--- .../gh24375/{B.java => NestedAnnotation.java} | 3 +- ...anningCandidateComponentProviderTests.java | 10 +++--- .../annotation/AnnotationTypeMapping.java | 14 +++++--- .../core/annotation/TypeMappedAnnotation.java | 2 +- .../java/example/type/AnnotatedComponent.java | 21 ++++++++++++ .../example/type/EnclosingAnnotation.java | 33 +++++++++++++++++++ .../java/example/type/NestedAnnotation.java | 29 ++++++++++++++++ .../AnnotationTypeMappingsTests.java | 20 ++++++----- .../type/AbstractMethodMetadataTests.java | 11 ++++++- 11 files changed, 130 insertions(+), 28 deletions(-) rename spring-context/src/test/java/example/gh24375/{MyComponent.java => AnnotatedComponent.java} (89%) rename spring-context/src/test/java/example/gh24375/{A.java => EnclosingAnnotation.java} (82%) rename spring-context/src/test/java/example/gh24375/{B.java => NestedAnnotation.java} (96%) create mode 100644 spring-core/src/test/java/example/type/AnnotatedComponent.java create mode 100644 spring-core/src/test/java/example/type/EnclosingAnnotation.java create mode 100644 spring-core/src/test/java/example/type/NestedAnnotation.java diff --git a/spring-context/src/test/java/example/gh24375/MyComponent.java b/spring-context/src/test/java/example/gh24375/AnnotatedComponent.java similarity index 89% rename from spring-context/src/test/java/example/gh24375/MyComponent.java rename to spring-context/src/test/java/example/gh24375/AnnotatedComponent.java index ce21ae567ef..4eb7fdefb73 100644 --- a/spring-context/src/test/java/example/gh24375/MyComponent.java +++ b/spring-context/src/test/java/example/gh24375/AnnotatedComponent.java @@ -19,6 +19,6 @@ package example.gh24375; import org.springframework.stereotype.Component; @Component -@A(other = @B) -public class MyComponent { +@EnclosingAnnotation(nested2 = @NestedAnnotation) +public class AnnotatedComponent { } diff --git a/spring-context/src/test/java/example/gh24375/A.java b/spring-context/src/test/java/example/gh24375/EnclosingAnnotation.java similarity index 82% rename from spring-context/src/test/java/example/gh24375/A.java rename to spring-context/src/test/java/example/gh24375/EnclosingAnnotation.java index a55f3406c24..1a925de59ae 100644 --- a/spring-context/src/test/java/example/gh24375/A.java +++ b/spring-context/src/test/java/example/gh24375/EnclosingAnnotation.java @@ -25,11 +25,12 @@ import org.springframework.core.annotation.AliasFor; @Target(ElementType.TYPE) @Retention(RetentionPolicy.RUNTIME) -public @interface A { +public @interface EnclosingAnnotation { - @AliasFor("value") - B other() default @B; + @AliasFor("nested2") + NestedAnnotation nested1() default @NestedAnnotation; + + @AliasFor("nested1") + NestedAnnotation nested2() default @NestedAnnotation; - @AliasFor("other") - B value() default @B; } diff --git a/spring-context/src/test/java/example/gh24375/B.java b/spring-context/src/test/java/example/gh24375/NestedAnnotation.java similarity index 96% rename from spring-context/src/test/java/example/gh24375/B.java rename to spring-context/src/test/java/example/gh24375/NestedAnnotation.java index 96f3e9c94aa..531de72d6bf 100644 --- a/spring-context/src/test/java/example/gh24375/B.java +++ b/spring-context/src/test/java/example/gh24375/NestedAnnotation.java @@ -23,7 +23,8 @@ import java.lang.annotation.Target; @Target(ElementType.ANNOTATION_TYPE) @Retention(RetentionPolicy.RUNTIME) -public @interface B { +public @interface NestedAnnotation { String name() default ""; + } diff --git a/spring-context/src/test/java/org/springframework/context/annotation/ClassPathScanningCandidateComponentProviderTests.java b/spring-context/src/test/java/org/springframework/context/annotation/ClassPathScanningCandidateComponentProviderTests.java index 9d46c7d3f16..f4809a51d59 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/ClassPathScanningCandidateComponentProviderTests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/ClassPathScanningCandidateComponentProviderTests.java @@ -21,7 +21,7 @@ import java.lang.annotation.RetentionPolicy; import java.util.Set; import java.util.regex.Pattern; -import example.gh24375.MyComponent; +import example.gh24375.AnnotatedComponent; import example.profilescan.DevComponent; import example.profilescan.ProfileAnnotatedComponent; import example.profilescan.ProfileMetaAnnotatedComponent; @@ -39,7 +39,6 @@ import example.scannable.ServiceInvocationCounter; import example.scannable.StubFooDao; import example.scannable.sub.BarComponent; import org.aspectj.lang.annotation.Aspect; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.AnnotatedGenericBeanDefinition; @@ -503,12 +502,11 @@ public class ClassPathScanningCandidateComponentProviderTests { } @Test - @Disabled("Disabled until gh-24375 is resolved") - public void gh24375() { + public void componentScanningFindsComponentsAnnotatedWithAnnotationsContainingNestedAnnotations() { ClassPathScanningCandidateComponentProvider provider = new ClassPathScanningCandidateComponentProvider(true); - Set components = provider.findCandidateComponents(MyComponent.class.getPackage().getName()); + Set components = provider.findCandidateComponents(AnnotatedComponent.class.getPackage().getName()); assertThat(components).hasSize(1); - assertThat(components.iterator().next().getBeanClassName()).isEqualTo(MyComponent.class.getName()); + assertThat(components.iterator().next().getBeanClassName()).isEqualTo(AnnotatedComponent.class.getName()); } 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 f788127cc7f..19a3189564a 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 @@ -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. @@ -47,7 +47,6 @@ import org.springframework.util.StringUtils; */ final class AnnotationTypeMapping { - private static final MirrorSet[] EMPTY_MIRROR_SETS = new MirrorSet[0]; @@ -534,8 +533,15 @@ final class AnnotationTypeMapping { AttributeMethods attributes = AttributeMethods.forAnnotationType(annotation.annotationType()); for (int i = 0; i < attributes.size(); i++) { Method attribute = attributes.get(i); - if (!areEquivalent(ReflectionUtils.invokeMethod(attribute, annotation), - valueExtractor.apply(attribute, extractedValue), valueExtractor)) { + Object value1 = ReflectionUtils.invokeMethod(attribute, annotation); + Object value2; + if (extractedValue instanceof TypeMappedAnnotation) { + value2 = ((TypeMappedAnnotation) extractedValue).getValue(attribute.getName()).orElse(null); + } + else { + value2 = valueExtractor.apply(attribute, extractedValue); + } + if (!areEquivalent(value1, value2, valueExtractor)) { return false; } } diff --git a/spring-core/src/main/java/org/springframework/core/annotation/TypeMappedAnnotation.java b/spring-core/src/main/java/org/springframework/core/annotation/TypeMappedAnnotation.java index 8975dc3f63e..cc612a1b9b3 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/TypeMappedAnnotation.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/TypeMappedAnnotation.java @@ -685,7 +685,7 @@ final class TypeMappedAnnotation extends AbstractMergedAnn @SuppressWarnings("unchecked") @Nullable - private static Object extractFromMap(Method attribute, @Nullable Object map) { + static Object extractFromMap(Method attribute, @Nullable Object map) { return (map != null ? ((Map) map).get(attribute.getName()) : null); } diff --git a/spring-core/src/test/java/example/type/AnnotatedComponent.java b/spring-core/src/test/java/example/type/AnnotatedComponent.java new file mode 100644 index 00000000000..33fab84028e --- /dev/null +++ b/spring-core/src/test/java/example/type/AnnotatedComponent.java @@ -0,0 +1,21 @@ +/* + * 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package example.type; + +@EnclosingAnnotation(nested2 = @NestedAnnotation) +public class AnnotatedComponent { +} diff --git a/spring-core/src/test/java/example/type/EnclosingAnnotation.java b/spring-core/src/test/java/example/type/EnclosingAnnotation.java new file mode 100644 index 00000000000..8c9bbd25d2c --- /dev/null +++ b/spring-core/src/test/java/example/type/EnclosingAnnotation.java @@ -0,0 +1,33 @@ +/* + * 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package example.type; + +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +import org.springframework.core.annotation.AliasFor; + +@Retention(RetentionPolicy.RUNTIME) +public @interface EnclosingAnnotation { + + @AliasFor("nested2") + NestedAnnotation nested1() default @NestedAnnotation; + + @AliasFor("nested1") + NestedAnnotation nested2() default @NestedAnnotation; + +} diff --git a/spring-core/src/test/java/example/type/NestedAnnotation.java b/spring-core/src/test/java/example/type/NestedAnnotation.java new file mode 100644 index 00000000000..8d3789fa52d --- /dev/null +++ b/spring-core/src/test/java/example/type/NestedAnnotation.java @@ -0,0 +1,29 @@ +/* + * 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package example.type; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Target(ElementType.ANNOTATION_TYPE) +@Retention(RetentionPolicy.RUNTIME) +public @interface NestedAnnotation { + + String name() default ""; +} diff --git a/spring-core/src/test/java/org/springframework/core/annotation/AnnotationTypeMappingsTests.java b/spring-core/src/test/java/org/springframework/core/annotation/AnnotationTypeMappingsTests.java index f5cb39ec6d2..412e9c51a97 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/AnnotationTypeMappingsTests.java +++ b/spring-core/src/test/java/org/springframework/core/annotation/AnnotationTypeMappingsTests.java @@ -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. @@ -45,6 +45,7 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType; * Tests for {@link AnnotationTypeMappings} and {@link AnnotationTypeMapping}. * * @author Phillip Webb + * @author Sam Brannen */ class AnnotationTypeMappingsTests { @@ -440,10 +441,18 @@ class AnnotationTypeMappingsTests { } @Test - void isEquivalentToDefaultValueWhenNestedAnnotationAndExtractedValuesMatchReturnsTrue() { + void isEquivalentToDefaultValueWhenNestedAnnotationAndExtractedValuesMatchReturnsTrueAndValueSuppliedAsMap() { AnnotationTypeMapping mapping = AnnotationTypeMappings.forAnnotationType(NestedValue.class).get(0); Map value = Collections.singletonMap("value", "java.io.InputStream"); - assertThat(mapping.isEquivalentToDefaultValue(0, value, this::extractFromMap)).isTrue(); + assertThat(mapping.isEquivalentToDefaultValue(0, value, TypeMappedAnnotation::extractFromMap)).isTrue(); + } + + @Test // gh-24375 + void isEquivalentToDefaultValueWhenNestedAnnotationAndExtractedValuesMatchReturnsTrueAndValueSuppliedAsTypeMappedAnnotation() { + AnnotationTypeMapping mapping = AnnotationTypeMappings.forAnnotationType(NestedValue.class).get(0); + Map attributes = Collections.singletonMap("value", "java.io.InputStream"); + MergedAnnotation value = TypeMappedAnnotation.of(getClass().getClassLoader(), null, ClassValue.class, attributes); + assertThat(mapping.isEquivalentToDefaultValue(0, value, TypeMappedAnnotation::extractFromMap)).isTrue(); } @Test @@ -504,11 +513,6 @@ class AnnotationTypeMappingsTests { return names; } - @SuppressWarnings("unchecked") - private Object extractFromMap(Method attribute, Object map) { - return map != null ? ((Map) map).get(attribute.getName()) : null; - } - @Retention(RetentionPolicy.RUNTIME) @interface SimpleAnnotation { diff --git a/spring-core/src/test/java/org/springframework/core/type/AbstractMethodMetadataTests.java b/spring-core/src/test/java/org/springframework/core/type/AbstractMethodMetadataTests.java index e1190866a94..07d48e47967 100644 --- a/spring-core/src/test/java/org/springframework/core/type/AbstractMethodMetadataTests.java +++ b/spring-core/src/test/java/org/springframework/core/type/AbstractMethodMetadataTests.java @@ -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. @@ -19,6 +19,8 @@ package org.springframework.core.type; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; +import example.type.AnnotatedComponent; +import example.type.EnclosingAnnotation; import org.junit.jupiter.api.Test; import org.springframework.core.annotation.MergedAnnotation; @@ -31,6 +33,7 @@ import static org.assertj.core.api.Assertions.entry; * Base class for {@link MethodMetadata} tests. * * @author Phillip Webb + * @author Sam Brannen */ public abstract class AbstractMethodMetadataTests { @@ -138,6 +141,12 @@ public abstract class AbstractMethodMetadataTests { assertThat(attributes.get("size")).containsExactlyInAnyOrder(1, 2); } + @Test // gh-24375 + public void metadataLoadsForNestedAnnotations() { + AnnotationMetadata annotationMetadata = get(AnnotatedComponent.class); + assertThat(annotationMetadata.getAnnotationTypes()).containsExactly(EnclosingAnnotation.class.getName()); + } + protected MethodMetadata getTagged(Class source) { return get(source, Tag.class.getName()); }