From bf9f261b9512a3d0f84844dfa75943a231ed5a87 Mon Sep 17 00:00:00 2001 From: Arjen Poutsma Date: Tue, 14 Jun 2022 13:03:29 +0200 Subject: [PATCH] Revert "Support recursive annotations in merged annotations" This reverts commit 3ec612aaf8611d2ad6e6f7f3d5868feee5024477. --- .../annotation/AnnotationTypeMapping.java | 25 ++--- .../annotation/AnnotationTypeMappings.java | 77 ++++--------- .../AnnotationTypeMappingsTests.java | 4 +- .../springframework/core/annotation/Filter.kt | 35 ------ .../core/annotation/Filters.kt | 29 ----- .../KotlinMergedAnnotationsTests.kt | 105 ------------------ .../springframework/core/annotation/Person.kt | 35 ------ 7 files changed, 31 insertions(+), 279 deletions(-) delete mode 100644 spring-core/src/test/kotlin/org/springframework/core/annotation/Filter.kt delete mode 100644 spring-core/src/test/kotlin/org/springframework/core/annotation/Filters.kt delete mode 100644 spring-core/src/test/kotlin/org/springframework/core/annotation/KotlinMergedAnnotationsTests.kt delete mode 100644 spring-core/src/test/kotlin/org/springframework/core/annotation/Person.kt 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 fd90775eb996..3ebcefc84818 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-2022 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. @@ -82,8 +82,8 @@ final class AnnotationTypeMapping { private final Set claimedAliases = new HashSet<>(); - AnnotationTypeMapping(@Nullable AnnotationTypeMapping source, Class annotationType, - @Nullable Annotation annotation, Set> visitedAnnotationTypes) { + AnnotationTypeMapping(@Nullable AnnotationTypeMapping source, + Class annotationType, @Nullable Annotation annotation) { this.source = source; this.root = (source != null ? source.getRoot() : this); @@ -103,7 +103,7 @@ final class AnnotationTypeMapping { processAliases(); addConventionMappings(); addConventionAnnotationValues(); - this.synthesizable = computeSynthesizableFlag(visitedAnnotationTypes); + this.synthesizable = computeSynthesizableFlag(); } @@ -311,10 +311,7 @@ private boolean isBetterConventionAnnotationValue(int index, boolean isValueAttr } @SuppressWarnings("unchecked") - private boolean computeSynthesizableFlag(Set> visitedAnnotationTypes) { - // Track that we have visited the current annotation type. - visitedAnnotationTypes.add(this.annotationType); - + private boolean computeSynthesizableFlag() { // Uses @AliasFor for local aliases? for (int index : this.aliasMappings) { if (index != -1) { @@ -343,15 +340,9 @@ private boolean computeSynthesizableFlag(Set> visite if (type.isAnnotation() || (type.isArray() && type.getComponentType().isAnnotation())) { Class annotationType = (Class) (type.isAnnotation() ? type : type.getComponentType()); - // Ensure we have not yet visited the current nested annotation type, in order - // to avoid infinite recursion for JVM languages other than Java that support - // recursive annotation definitions. - if (visitedAnnotationTypes.add(annotationType)) { - AnnotationTypeMapping mapping = - AnnotationTypeMappings.forAnnotationType(annotationType, visitedAnnotationTypes).get(0); - if (mapping.isSynthesizable()) { - return true; - } + AnnotationTypeMapping mapping = AnnotationTypeMappings.forAnnotationType(annotationType).get(0); + if (mapping.isSynthesizable()) { + return true; } } } diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMappings.java b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMappings.java index a676583d570f..7bd535086ee5 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMappings.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMappings.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 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. @@ -20,10 +20,8 @@ import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Deque; -import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Set; import org.springframework.lang.Nullable; import org.springframework.util.ConcurrentReferenceHashMap; @@ -42,7 +40,6 @@ * be searched once, regardless of how many times they are actually used. * * @author Phillip Webb - * @author Sam Brannen * @since 5.2 * @see AnnotationTypeMapping */ @@ -63,22 +60,19 @@ final class AnnotationTypeMappings { private AnnotationTypeMappings(RepeatableContainers repeatableContainers, - AnnotationFilter filter, Class annotationType, - Set> visitedAnnotationTypes) { + AnnotationFilter filter, Class annotationType) { this.repeatableContainers = repeatableContainers; this.filter = filter; this.mappings = new ArrayList<>(); - addAllMappings(annotationType, visitedAnnotationTypes); + addAllMappings(annotationType); this.mappings.forEach(AnnotationTypeMapping::afterAllMappingsSet); } - private void addAllMappings(Class annotationType, - Set> visitedAnnotationTypes) { - + private void addAllMappings(Class annotationType) { Deque queue = new ArrayDeque<>(); - addIfPossible(queue, null, annotationType, null, visitedAnnotationTypes); + addIfPossible(queue, null, annotationType, null); while (!queue.isEmpty()) { AnnotationTypeMapping mapping = queue.removeFirst(); this.mappings.add(mapping); @@ -108,15 +102,14 @@ private void addMetaAnnotationsToQueue(Deque queue, Annot } private void addIfPossible(Deque queue, AnnotationTypeMapping source, Annotation ann) { - addIfPossible(queue, source, ann.annotationType(), ann, new HashSet<>()); + addIfPossible(queue, source, ann.annotationType(), ann); } private void addIfPossible(Deque queue, @Nullable AnnotationTypeMapping source, - Class annotationType, @Nullable Annotation ann, - Set> visitedAnnotationTypes) { + Class annotationType, @Nullable Annotation ann) { try { - queue.addLast(new AnnotationTypeMapping(source, annotationType, ann, visitedAnnotationTypes)); + queue.addLast(new AnnotationTypeMapping(source, annotationType, ann)); } catch (Exception ex) { AnnotationUtils.rethrowAnnotationConfigurationException(ex); @@ -173,37 +166,20 @@ AnnotationTypeMapping get(int index) { * @return type mappings for the annotation type */ static AnnotationTypeMappings forAnnotationType(Class annotationType) { - return forAnnotationType(annotationType, new HashSet<>()); - } - - /** - * Create {@link AnnotationTypeMappings} for the specified annotation type. - * @param annotationType the source annotation type - * @param visitedAnnotationTypes the set of annotations that we have already - * visited; used to avoid infinite recursion for recursive annotations which - * some JVM languages support (such as Kotlin) - * @return type mappings for the annotation type - */ - static AnnotationTypeMappings forAnnotationType(Class annotationType, - Set> visitedAnnotationTypes) { - - return forAnnotationType(annotationType, RepeatableContainers.standardRepeatables(), - AnnotationFilter.PLAIN, visitedAnnotationTypes); + return forAnnotationType(annotationType, AnnotationFilter.PLAIN); } /** * Create {@link AnnotationTypeMappings} for the specified annotation type. * @param annotationType the source annotation type - * @param repeatableContainers the repeatable containers that may be used by - * the meta-annotations * @param annotationFilter the annotation filter used to limit which * annotations are considered * @return type mappings for the annotation type */ - static AnnotationTypeMappings forAnnotationType(Class annotationType, - RepeatableContainers repeatableContainers, AnnotationFilter annotationFilter) { + static AnnotationTypeMappings forAnnotationType( + Class annotationType, AnnotationFilter annotationFilter) { - return forAnnotationType(annotationType, repeatableContainers, annotationFilter, new HashSet<>()); + return forAnnotationType(annotationType, RepeatableContainers.standardRepeatables(), annotationFilter); } /** @@ -213,24 +189,20 @@ static AnnotationTypeMappings forAnnotationType(Class anno * the meta-annotations * @param annotationFilter the annotation filter used to limit which * annotations are considered - * @param visitedAnnotationTypes the set of annotations that we have already - * visited; used to avoid infinite recursion for recursive annotations which - * some JVM languages support (such as Kotlin) * @return type mappings for the annotation type */ - private static AnnotationTypeMappings forAnnotationType(Class annotationType, - RepeatableContainers repeatableContainers, AnnotationFilter annotationFilter, - Set> visitedAnnotationTypes) { + static AnnotationTypeMappings forAnnotationType(Class annotationType, + RepeatableContainers repeatableContainers, AnnotationFilter annotationFilter) { if (repeatableContainers == RepeatableContainers.standardRepeatables()) { return standardRepeatablesCache.computeIfAbsent(annotationFilter, - key -> new Cache(repeatableContainers, key)).get(annotationType, visitedAnnotationTypes); + key -> new Cache(repeatableContainers, key)).get(annotationType); } if (repeatableContainers == RepeatableContainers.none()) { return noRepeatablesCache.computeIfAbsent(annotationFilter, - key -> new Cache(repeatableContainers, key)).get(annotationType, visitedAnnotationTypes); + key -> new Cache(repeatableContainers, key)).get(annotationType); } - return new AnnotationTypeMappings(repeatableContainers, annotationFilter, annotationType, visitedAnnotationTypes); + return new AnnotationTypeMappings(repeatableContainers, annotationFilter, annotationType); } static void clearCache() { @@ -263,21 +235,14 @@ private static class Cache { /** * Get or create {@link AnnotationTypeMappings} for the specified annotation type. * @param annotationType the annotation type - * @param visitedAnnotationTypes the set of annotations that we have already - * visited; used to avoid infinite recursion for recursive annotations which - * some JVM languages support (such as Kotlin) * @return a new or existing {@link AnnotationTypeMappings} instance */ - AnnotationTypeMappings get(Class annotationType, - Set> visitedAnnotationTypes) { - - return this.mappings.computeIfAbsent(annotationType, key -> createMappings(key, visitedAnnotationTypes)); + AnnotationTypeMappings get(Class annotationType) { + return this.mappings.computeIfAbsent(annotationType, this::createMappings); } - private AnnotationTypeMappings createMappings(Class annotationType, - Set> visitedAnnotationTypes) { - - return new AnnotationTypeMappings(this.repeatableContainers, this.filter, annotationType, visitedAnnotationTypes); + AnnotationTypeMappings createMappings(Class annotationType) { + return new AnnotationTypeMappings(this.repeatableContainers, this.filter, annotationType); } } 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 5afcc174e8e2..1a440efbf636 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-2022 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. @@ -80,7 +80,7 @@ void forAnnotationTypeWhenHasRepeatingMetaAnnotationReturnsMapping() { @Test void forAnnotationTypeWhenRepeatableMetaAnnotationIsFiltered() { AnnotationTypeMappings mappings = AnnotationTypeMappings.forAnnotationType(WithRepeatedMetaAnnotations.class, - RepeatableContainers.standardRepeatables(), Repeating.class.getName()::equals); + Repeating.class.getName()::equals); assertThat(getAll(mappings)).flatExtracting(AnnotationTypeMapping::getAnnotationType) .containsExactly(WithRepeatedMetaAnnotations.class); } diff --git a/spring-core/src/test/kotlin/org/springframework/core/annotation/Filter.kt b/spring-core/src/test/kotlin/org/springframework/core/annotation/Filter.kt deleted file mode 100644 index 2755ba4d9b2f..000000000000 --- a/spring-core/src/test/kotlin/org/springframework/core/annotation/Filter.kt +++ /dev/null @@ -1,35 +0,0 @@ -/* - * Copyright 2002-2022 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 org.springframework.core.annotation - -/** - * @author Sam Brannen - * @since 5.3.16 - */ -@Target(AnnotationTarget.FUNCTION) -@Retention(AnnotationRetention.RUNTIME) -public annotation class Filter( - - @get:AliasFor("name") - val value: String = "", - - @get:AliasFor("value") - val name: String = "", - - val and: Filters = Filters() - -) diff --git a/spring-core/src/test/kotlin/org/springframework/core/annotation/Filters.kt b/spring-core/src/test/kotlin/org/springframework/core/annotation/Filters.kt deleted file mode 100644 index 7a044fa2bb5c..000000000000 --- a/spring-core/src/test/kotlin/org/springframework/core/annotation/Filters.kt +++ /dev/null @@ -1,29 +0,0 @@ -/* - * Copyright 2002-2022 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 org.springframework.core.annotation - -/** - * @author Sam Brannen - * @since 5.3.16 - */ -@Target(AnnotationTarget.FUNCTION) -@Retention(AnnotationRetention.RUNTIME) -public annotation class Filters( - - vararg val value: Filter - -) diff --git a/spring-core/src/test/kotlin/org/springframework/core/annotation/KotlinMergedAnnotationsTests.kt b/spring-core/src/test/kotlin/org/springframework/core/annotation/KotlinMergedAnnotationsTests.kt deleted file mode 100644 index dedd3ced4166..000000000000 --- a/spring-core/src/test/kotlin/org/springframework/core/annotation/KotlinMergedAnnotationsTests.kt +++ /dev/null @@ -1,105 +0,0 @@ -/* - * Copyright 2002-2022 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 org.springframework.core.annotation - -import org.assertj.core.api.Assertions.assertThat -import org.junit.jupiter.api.Test -import org.springframework.core.annotation.MergedAnnotations -import org.springframework.core.annotation.MergedAnnotation - -/** - * Tests for {@link MergedAnnotations} and {@link MergedAnnotation} in Kotlin. - * - * @author Sam Brannen - * @since 5.3.16 - */ -class KotlinMergedAnnotationsTests { - - @Test // gh-28012 - fun recursiveAnnotation() { - val method = javaClass.getMethod("personMethod") - - // MergedAnnotations - val mergedAnnotations = MergedAnnotations.from(method) - assertThat(mergedAnnotations.isPresent(Person::class.java)).isTrue(); - - // MergedAnnotation - val mergedAnnotation = MergedAnnotation.from(method.getAnnotation(Person::class.java)) - assertThat(mergedAnnotation).isNotNull(); - - // Synthesized Annotations - val jane = mergedAnnotation.synthesize() - assertThat(jane).isInstanceOf(SynthesizedAnnotation::class.java) - assertThat(jane.value).isEqualTo("jane") - assertThat(jane.name).isEqualTo("jane") - val synthesizedFriends = jane.friends - assertThat(synthesizedFriends).hasSize(2) - - val john = synthesizedFriends[0] - assertThat(john).isInstanceOf(SynthesizedAnnotation::class.java) - assertThat(john.value).isEqualTo("john") - assertThat(john.name).isEqualTo("john") - - val sally = synthesizedFriends[1] - assertThat(sally).isInstanceOf(SynthesizedAnnotation::class.java) - assertThat(sally.value).isEqualTo("sally") - assertThat(sally.name).isEqualTo("sally") - } - - @Test // gh-28012 - fun recursiveNestedAnnotation() { - val method = javaClass.getMethod("filterMethod") - - // MergedAnnotations - val mergedAnnotations = MergedAnnotations.from(method) - assertThat(mergedAnnotations.isPresent(Filter::class.java)).isTrue(); - - // MergedAnnotation - val mergedAnnotation = MergedAnnotation.from(method.getAnnotation(Filter::class.java)) - assertThat(mergedAnnotation).isNotNull(); - - // Synthesized Annotations - val fooFilter = mergedAnnotation.synthesize() - assertThat(fooFilter).isInstanceOf(SynthesizedAnnotation::class.java) - assertThat(fooFilter.value).isEqualTo("foo") - assertThat(fooFilter.name).isEqualTo("foo") - val filters = fooFilter.and - assertThat(filters.value).hasSize(2) - - val barFilter = filters.value[0] - assertThat(barFilter).isInstanceOf(SynthesizedAnnotation::class.java) - assertThat(barFilter.value).isEqualTo("bar") - assertThat(barFilter.name).isEqualTo("bar") - assertThat(barFilter.and.value).isEmpty() - - val bazFilter = filters.value[1] - assertThat(bazFilter).isInstanceOf(SynthesizedAnnotation::class.java) - assertThat(bazFilter.value).isEqualTo("baz") - assertThat(bazFilter.name).isEqualTo("baz") - assertThat(bazFilter.and.value).isEmpty() - } - - - @Person("jane", friends = [Person("john"), Person("sally")]) - fun personMethod() { - } - - @Filter("foo", and = Filters(Filter("bar"), Filter("baz"))) - fun filterMethod() { - } - -} diff --git a/spring-core/src/test/kotlin/org/springframework/core/annotation/Person.kt b/spring-core/src/test/kotlin/org/springframework/core/annotation/Person.kt deleted file mode 100644 index 28d67c4a2364..000000000000 --- a/spring-core/src/test/kotlin/org/springframework/core/annotation/Person.kt +++ /dev/null @@ -1,35 +0,0 @@ -/* - * Copyright 2002-2022 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 org.springframework.core.annotation - -/** - * @author Sam Brannen - * @since 5.3.16 - */ -@Target(AnnotationTarget.FUNCTION) -@Retention(AnnotationRetention.RUNTIME) -public annotation class Person( - - @get:AliasFor("name") - val value: String = "", - - @get:AliasFor("value") - val name: String = "", - - vararg val friends: Person = [] - -)