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);