mirror of
https://github.com/spring-projects/spring-framework
synced 2026-06-08 17:33:33 +00:00
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 00fbd91cca since
it is no longer necessary.
See gh-36524
Closes gh-36586
This commit is contained in:
+1
-11
@@ -139,17 +139,7 @@ public class AnnotationBeanNameGenerator implements BeanNameGenerator {
|
||||
Set<AnnotationAttributes> visited = new HashSet<>();
|
||||
|
||||
for (MergedAnnotation<Annotation> 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<String> metaAnnotationTypes = this.metaAnnotationTypesCache.computeIfAbsent(annotationType,
|
||||
|
||||
+29
-9
@@ -449,33 +449,53 @@ public interface MergedAnnotation<A extends Annotation> {
|
||||
MergedAnnotation<A> 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.
|
||||
* <p>The {@linkplain Adapt adaptations} may be used to change the way that
|
||||
* values are added.
|
||||
* <p>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.
|
||||
* <p>The {@linkplain Adapt adaptations} may be used to change the way that
|
||||
* values are added.
|
||||
* <p>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<String, Object> 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.
|
||||
* <p>The {@linkplain Adapt adaptations} may be used to change the way that
|
||||
* values are added.
|
||||
* <p>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 extends Map<String, Object>> T asMap(Function<MergedAnnotation<?>, T> factory, Adapt... adaptations);
|
||||
|
||||
|
||||
+16
-2
@@ -269,8 +269,22 @@ final class TypeMappedAnnotation<A extends Annotation> 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));
|
||||
|
||||
+35
@@ -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 {
|
||||
|
||||
|
||||
+121
@@ -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 <A extends Annotation> TypeMappedAnnotation<A> getTypeMappedAnnotation(
|
||||
Class<?> source, Class<A> annotationType) {
|
||||
return getTypeMappedAnnotation(source, annotationType, annotationType);
|
||||
|
||||
Reference in New Issue
Block a user