From d10460d77576c49a19166efb421f46746802c36a Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Mon, 30 Mar 2026 17:17:07 +0200 Subject: [PATCH] Track class loading exceptions in MergedAnnotation.asMap() Spring Framework 5.2 introduced a regression in our annotation processing support when the MergedAnnotations API was introduced. Consequently, prior to this commit, our "annotation attributes as a Map or AnnotationAttributes instance" support no longer stored exceptions thrown while attempting to load a type referenced by an annotation attribute. Instead, the exception was thrown immediately. To address that, this commit revises our MergedAnnotation.asMap() support so that it now tracks such exceptions in the map instead of immediately throwing them. This allows map functionality such as contains(attributeName), keySet(), etc. to continue to function properly. In addition, the internal getRequiredAttribute() method in AnnotationAttributes once again properly throws the original exception wrapped in an IllegalArgumentException whenever a caller invokes one of the convenience methods such as getClass() and getClassArray(). Note that this affects both asMap() variants as well as asAnnotationAttributes(). In addition, this commit reverts the fix applied in 00fbd91cca0 since it is no longer necessary. See gh-36524 Closes gh-36586 --- .../AnnotationBeanNameGenerator.java | 12 +- .../core/annotation/MergedAnnotation.java | 38 ++++-- .../core/annotation/TypeMappedAnnotation.java | 18 ++- .../AnnotationIntrospectionFailureTests.java | 35 +++++ .../annotation/TypeMappedAnnotationTests.java | 121 ++++++++++++++++++ 5 files changed, 202 insertions(+), 22 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/context/annotation/AnnotationBeanNameGenerator.java b/spring-context/src/main/java/org/springframework/context/annotation/AnnotationBeanNameGenerator.java index 3eedc57b9b2..19ef319fd48 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/AnnotationBeanNameGenerator.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/AnnotationBeanNameGenerator.java @@ -139,17 +139,7 @@ public class AnnotationBeanNameGenerator implements BeanNameGenerator { Set visited = new HashSet<>(); for (MergedAnnotation mergedAnnotation : mergedAnnotations) { - AnnotationAttributes attributes = null; - try { - attributes = mergedAnnotation.asAnnotationAttributes(ADAPTATIONS); - } - catch (Throwable ex) { - // Ignore exception and current MergedAnnotation, assuming that values of the - // MergedAnnotation could not be adapted to a Map/AnnotationAttributes due to - // missing types referenced via annotation attributes. - continue; - } - + AnnotationAttributes attributes = mergedAnnotation.asAnnotationAttributes(ADAPTATIONS); if (visited.add(attributes)) { String annotationType = mergedAnnotation.getType().getName(); Set metaAnnotationTypes = this.metaAnnotationTypesCache.computeIfAbsent(annotationType, diff --git a/spring-core/src/main/java/org/springframework/core/annotation/MergedAnnotation.java b/spring-core/src/main/java/org/springframework/core/annotation/MergedAnnotation.java index e316d7d4e76..3c0cf910f17 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/MergedAnnotation.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/MergedAnnotation.java @@ -449,33 +449,53 @@ public interface MergedAnnotation { MergedAnnotation withNonMergedAttributes(); /** - * Create a new mutable {@link AnnotationAttributes} instance from this - * merged annotation. + * Create a mutable {@link AnnotationAttributes} map that contains all annotation + * attributes from this merged annotation. *

The {@linkplain Adapt adaptations} may be used to change the way that * values are added. + *

As of Spring Framework 7.0.7, annotation attributes of type {@code Class} + * or {@code Class[]} in the returned {@code AnnotationAttributes} map may have + * their values replaced by an {@linkplain Throwable exception} if an error + * occurred while attempting to load the respective type via reflection. + * Accessing such an attribute via {@link AnnotationAttributes#getClass(String)} + * or {@link AnnotationAttributes#getClassArray(String)} will throw an + * {@link IllegalArgumentException} which includes the original exception as + * the cause. * @param adaptations the adaptations that should be applied to the annotation values - * @return a mutable {@code AnnotationAttributes} instance containing the attributes - * and values + * @return a mutable {@code AnnotationAttributes} map containing the attributes + * and their values */ AnnotationAttributes asAnnotationAttributes(Adapt... adaptations); /** - * Create an immutable {@link Map} that contains all the annotation attributes. + * Create an immutable {@link Map} that contains all annotation attributes + * from this merged annotation. *

The {@linkplain Adapt adaptations} may be used to change the way that * values are added. + *

As of Spring Framework 7.0.7, annotation attributes of type {@code Class} + * or {@code Class[]} in the returned map may have their values replaced by an + * {@linkplain Throwable exception} if an error occurred while attempting to + * load the respective type via reflection. * @param adaptations the adaptations that should be applied to the annotation values - * @return an immutable map containing the attributes and values + * @return an immutable map containing the attributes and their values + * @see #asAnnotationAttributes(Adapt...) */ Map asMap(Adapt... adaptations); /** - * Create a new {@link Map} of the given type that contains all the annotation - * attributes. + * Create a {@link Map} using the supplied factory and populate it with all + * annotation attributes from this merged annotation. *

The {@linkplain Adapt adaptations} may be used to change the way that * values are added. + *

As of Spring Framework 7.0.7, annotation attributes of type {@code Class} + * or {@code Class[]} in the returned map may have their values replaced by an + * {@linkplain Throwable exception} if an error occurred while attempting to + * load the respective type via reflection. * @param factory a map factory * @param adaptations the adaptations that should be applied to the annotation values - * @return a map containing the attributes and values + * @return a map containing the attributes and their values + * @see #asAnnotationAttributes(Adapt...) + * @see #asMap(Adapt...) */ > T asMap(Function, T> factory, Adapt... adaptations); 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 97c69679bde..b7b7bdf1ddc 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 @@ -269,8 +269,22 @@ final class TypeMappedAnnotation extends AbstractMergedAnn AttributeMethods attributes = this.mapping.getAttributes(); for (int i = 0; i < attributes.size(); i++) { Method attribute = attributes.get(i); - Object value = (isFiltered(attribute.getName()) ? null : - getValue(i, getTypeForMapOptions(attribute, adaptations))); + if (isFiltered(attribute.getName())) { + continue; + } + Object value; + try { + value = getValue(i, getTypeForMapOptions(attribute, adaptations)); + } + catch (Throwable ex) { + // If the value for the current annotation attribute cannot be resolved + // (for example, a class attribute referencing a type that is absent from + // the classpath), store the exception as the value and skip the "adapt" + // step. This allows us to track the exception internally and only throw it + // if the user actually requests the value via the AnnotationAttributes API. + map.put(attribute.getName(), ex); + continue; + } if (value != null) { map.put(attribute.getName(), adaptValueForMapOptions(attribute, value, map.getClass(), factory, adaptations)); diff --git a/spring-core/src/test/java/org/springframework/core/annotation/AnnotationIntrospectionFailureTests.java b/spring-core/src/test/java/org/springframework/core/annotation/AnnotationIntrospectionFailureTests.java index d8323266de4..2c4679993da 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/AnnotationIntrospectionFailureTests.java +++ b/spring-core/src/test/java/org/springframework/core/annotation/AnnotationIntrospectionFailureTests.java @@ -24,11 +24,14 @@ import java.lang.reflect.Method; import org.junit.jupiter.api.Test; import org.springframework.core.OverridingClassLoader; +import org.springframework.core.annotation.MergedAnnotation.Adapt; import org.springframework.util.ClassUtils; import org.springframework.util.ReflectionUtils; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.assertj.core.api.InstanceOfAssertFactories.throwable; /** * Tests that trigger annotation introspection failures and ensure that they are @@ -84,6 +87,38 @@ class AnnotationIntrospectionFailureTests { assertThat(annotations.isPresent(annotationClass)).isFalse(); } + @Test // gh-36586 + void filteredTypeInAnnotationAttributeDoesNotThrowWhenCallingAsAnnotationAttributes() throws Exception { + FilteringClassLoader classLoader = new FilteringClassLoader(getClass().getClassLoader()); + Class withAnnotation = ClassUtils.forName(WithExampleAnnotation.class.getName(), classLoader); + Annotation annotation = withAnnotation.getAnnotations()[0]; + MergedAnnotation mergedAnnotation = MergedAnnotation.from(null, annotation); + + // 0) Sanity check MergedAnnotation.getClass() behavior. + assertThatExceptionOfType(TypeNotPresentException.class) + .isThrownBy(() -> mergedAnnotation.getClass("value")) + .withCauseInstanceOf(ClassNotFoundException.class); + + AnnotationAttributes attributes = mergedAnnotation.asAnnotationAttributes(Adapt.values(false, true)); + + // 1) Attribute should be present, even though its value is an exception. + assertThat(attributes).containsKey("value"); + assertThat(attributes.get("value")).asInstanceOf(throwable(TypeNotPresentException.class)) + .hasMessageContaining(FilteredType.class.getName()) + .hasCauseInstanceOf(ClassNotFoundException.class); + + // 2) Accessing the attribute via AnnotationAttributes.getClass() should throw an + // IllegalArgumentException with the TypeNotPresentException as its cause. + assertThatIllegalArgumentException() + .isThrownBy(() -> attributes.getClass("value")) + .withMessageMatching(""" + Attribute 'value' for annotation \\[.+?\\] was not resolvable \ + due to exception \\[.+?TypeNotPresentException.+?\\]""") + .havingCause() + .isExactlyInstanceOf(TypeNotPresentException.class) + .withMessageContaining(FilteredType.class.getName()); + } + static class FilteringClassLoader extends OverridingClassLoader { diff --git a/spring-core/src/test/java/org/springframework/core/annotation/TypeMappedAnnotationTests.java b/spring-core/src/test/java/org/springframework/core/annotation/TypeMappedAnnotationTests.java index 2e61f557078..e2d7f4f049c 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/TypeMappedAnnotationTests.java +++ b/spring-core/src/test/java/org/springframework/core/annotation/TypeMappedAnnotationTests.java @@ -24,9 +24,15 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; +import org.assertj.core.api.ThrowableAssert.ThrowingCallable; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.springframework.core.annotation.MergedAnnotation.Adapt; + import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.assertj.core.api.InstanceOfAssertFactories.throwable; /** * Tests for {@link TypeMappedAnnotation}. See also {@link MergedAnnotationsTests} @@ -126,6 +132,121 @@ class TypeMappedAnnotationTests { assertThat(annotation.getClassArray("classArrayValue")).containsExactly(InputStream.class); } + + @Nested + class AsMapTests { + + @Test // gh-36586 + void fromStringToUnresolvableClass() { + var attributeName = "classValue"; + var mergedAnnotation = MergedAnnotation.of(null, null, ClassAttributes.class, + Map.of(attributeName, "com.example.DoesNotExist")); + + // 0) Sanity check MergedAnnotation.getClass() behavior. + assertThatIllegalArgumentException() + .isThrownBy(() -> mergedAnnotation.getClass(attributeName)) + .withMessage("Could not find class [com.example.DoesNotExist]") + .withCauseInstanceOf(ClassNotFoundException.class); + + var map = mergedAnnotation.asMap(Adapt.values(false, true)); + + // 1) Attribute should be present, even though its value is an exception. + assertThat(map).containsKey(attributeName); + assertThat(map.get(attributeName)).asInstanceOf(throwable(IllegalArgumentException.class)) + .hasMessageContaining("com.example.DoesNotExist"); + } + + @Test // gh-36586 + void fromStringArrayToUnresolvableClass() { + var attributeName = "classArrayValue"; + var mergedAnnotation = MergedAnnotation.of(null, null, ClassAttributes.class, + Map.of(attributeName, new String[] { "com.example.DoesNotExist" })); + + // 0) Sanity check MergedAnnotation.getClassArray() behavior. + assertThatIllegalArgumentException() + .isThrownBy(() -> mergedAnnotation.getClassArray(attributeName)) + .withMessage("Could not find class [com.example.DoesNotExist]") + .withCauseExactlyInstanceOf(ClassNotFoundException.class); + + var map = mergedAnnotation.asMap(Adapt.values(false, true)); + + // 1) Attribute should be present, even though its value is an exception. + assertThat(map).containsKey(attributeName); + assertThat(map.get(attributeName)).asInstanceOf(throwable(IllegalArgumentException.class)) + .hasMessageContaining("com.example.DoesNotExist"); + } + } + + + @Nested + class AsAnnotationAttributesTests { + + @Test // gh-36586 + void fromStringToUnresolvableClass() { + var attributeName = "classValue"; + var mergedAnnotation = MergedAnnotation.of(null, null, ClassAttributes.class, + Map.of(attributeName, "com.example.DoesNotExist")); + + // 0) Sanity check MergedAnnotation.getClass() behavior. + assertThatIllegalArgumentException() + .isThrownBy(() -> mergedAnnotation.getClass(attributeName)) + .withMessage("Could not find class [com.example.DoesNotExist]") + .withCauseInstanceOf(ClassNotFoundException.class); + + var attributes = mergedAnnotation.asAnnotationAttributes(Adapt.values(false, true)); + + // 1) Attribute should be present, even though its value is an exception. + assertThat(attributes).containsKey(attributeName); + assertThat(attributes.get(attributeName)).asInstanceOf(throwable(IllegalArgumentException.class)) + .hasMessageContaining("com.example.DoesNotExist"); + + // 2) Accessing the attribute via AnnotationAttributes.getClassArray() should throw an + // IllegalArgumentException with the IllegalArgumentException from ClassUtils.resolveClassName() + // as its cause. + assertAttributeAccessException(attributeName, () -> attributes.getClass(attributeName)); + } + + @Test // gh-36586 + void fromStringArrayToUnresolvableClass() { + var attributeName = "classArrayValue"; + var mergedAnnotation = MergedAnnotation.of(null, null, ClassAttributes.class, + Map.of(attributeName, new String[] { "com.example.DoesNotExist" })); + + // 0) Sanity check MergedAnnotation.getClassArray() behavior. + assertThatIllegalArgumentException() + .isThrownBy(() -> mergedAnnotation.getClassArray(attributeName)) + .withMessage("Could not find class [com.example.DoesNotExist]") + .withCauseInstanceOf(ClassNotFoundException.class); + + var attributes = mergedAnnotation.asAnnotationAttributes(Adapt.values(false, true)); + + // 1) Attribute should be present, even though its value is an exception. + assertThat(attributes).containsKey(attributeName); + assertThat(attributes.get(attributeName)).asInstanceOf(throwable(IllegalArgumentException.class)) + .hasMessageContaining("com.example.DoesNotExist"); + + // 2) Accessing the attribute via AnnotationAttributes.getClassArray() should throw an + // IllegalArgumentException with the IllegalArgumentException from ClassUtils.resolveClassName() + // as its cause. + assertAttributeAccessException(attributeName, () -> attributes.getClassArray(attributeName)); + } + + private static void assertAttributeAccessException(String attributeName, ThrowingCallable throwingCallable) { + assertThatIllegalArgumentException() + .isThrownBy(throwingCallable) + .withMessageMatching(""" + Attribute '%s' for annotation \\[.+?\\] was not resolvable \ + due to exception \\[.+?IllegalArgumentException.+?\\]""".formatted(attributeName)) + .havingCause() + .isExactlyInstanceOf(IllegalArgumentException.class) + .withMessageContaining("com.example.DoesNotExist") + .havingCause() + .isExactlyInstanceOf(ClassNotFoundException.class) + .withMessageContaining("com.example.DoesNotExist"); + } + } + + private TypeMappedAnnotation getTypeMappedAnnotation( Class source, Class annotationType) { return getTypeMappedAnnotation(source, annotationType, annotationType);