From 01868096a30df950a7116a7f228d5dd18ff5c9a5 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 30 Nov 2016 22:24:14 +0100 Subject: [PATCH] Consistent catching of Throwable for introspection failures Issue: SPR-12889 --- .../annotation/AnnotatedElementUtils.java | 40 +++++++++---------- .../core/annotation/AnnotationUtils.java | 38 +++++++++--------- .../ComposedRepeatableAnnotationsTests.java | 2 +- 3 files changed, 39 insertions(+), 41 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedElementUtils.java b/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedElementUtils.java index e19015f1ee..2593799c5b 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedElementUtils.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedElementUtils.java @@ -151,7 +151,7 @@ public class AnnotatedElementUtils { */ public static Set getMetaAnnotationTypes(AnnotatedElement element, Class annotationType) { Assert.notNull(element, "AnnotatedElement must not be null"); - Assert.notNull(annotationType, "annotationType must not be null"); + Assert.notNull(annotationType, "'annotationType' must not be null"); return getMetaAnnotationTypes(element, element.getAnnotation(annotationType)); } @@ -213,7 +213,7 @@ public class AnnotatedElementUtils { */ public static boolean hasMetaAnnotationTypes(AnnotatedElement element, Class annotationType) { Assert.notNull(element, "AnnotatedElement must not be null"); - Assert.notNull(annotationType, "annotationType must not be null"); + Assert.notNull(annotationType, "'annotationType' must not be null"); return hasMetaAnnotationTypes(element, annotationType, null); } @@ -266,7 +266,7 @@ public class AnnotatedElementUtils { */ public static boolean isAnnotated(AnnotatedElement element, Class annotationType) { Assert.notNull(element, "AnnotatedElement must not be null"); - Assert.notNull(annotationType, "annotationType must not be null"); + Assert.notNull(annotationType, "'annotationType' must not be null"); // Shortcut: directly present on the element, with no processing needed? if (element.isAnnotationPresent(annotationType)) { @@ -315,7 +315,7 @@ public class AnnotatedElementUtils { public static AnnotationAttributes getMergedAnnotationAttributes( AnnotatedElement element, Class annotationType) { - Assert.notNull(annotationType, "annotationType must not be null"); + Assert.notNull(annotationType, "'annotationType' must not be null"); AnnotationAttributes attributes = searchWithGetSemantics(element, annotationType, null, new MergedAnnotationAttributesProcessor()); AnnotationUtils.postProcessAnnotationAttributes(element, attributes, false, false); @@ -399,7 +399,7 @@ public class AnnotatedElementUtils { * @see AnnotationUtils#synthesizeAnnotation(Map, Class, AnnotatedElement) */ public static A getMergedAnnotation(AnnotatedElement element, Class annotationType) { - Assert.notNull(annotationType, "annotationType must not be null"); + Assert.notNull(annotationType, "'annotationType' must not be null"); // Shortcut: directly present on the element, with no merging needed? if (!(element instanceof Class)) { @@ -440,7 +440,7 @@ public class AnnotatedElementUtils { Class annotationType) { Assert.notNull(element, "AnnotatedElement must not be null"); - Assert.notNull(annotationType, "annotationType must not be null"); + Assert.notNull(annotationType, "'annotationType' must not be null"); MergedAnnotationAttributesProcessor processor = new MergedAnnotationAttributesProcessor(false, false, true); searchWithGetSemantics(element, annotationType, null, processor); @@ -507,7 +507,7 @@ public class AnnotatedElementUtils { Class annotationType, Class containerType) { Assert.notNull(element, "AnnotatedElement must not be null"); - Assert.notNull(annotationType, "annotationType must not be null"); + Assert.notNull(annotationType, "'annotationType' must not be null"); if (containerType == null) { containerType = resolveContainerType(annotationType); @@ -593,7 +593,7 @@ public class AnnotatedElementUtils { */ public static boolean hasAnnotation(AnnotatedElement element, Class annotationType) { Assert.notNull(element, "AnnotatedElement must not be null"); - Assert.notNull(annotationType, "annotationType must not be null"); + Assert.notNull(annotationType, "'annotationType' must not be null"); // Shortcut: directly present on the element, with no processing needed? if (element.isAnnotationPresent(annotationType)) { @@ -696,7 +696,7 @@ public class AnnotatedElementUtils { * @see #getMergedAnnotationAttributes(AnnotatedElement, Class) */ public static A findMergedAnnotation(AnnotatedElement element, Class annotationType) { - Assert.notNull(annotationType, "annotationType must not be null"); + Assert.notNull(annotationType, "'annotationType' must not be null"); // Shortcut: directly present on the element, with no merging needed? if (!(element instanceof Class)) { @@ -736,7 +736,7 @@ public class AnnotatedElementUtils { Class annotationType) { Assert.notNull(element, "AnnotatedElement must not be null"); - Assert.notNull(annotationType, "annotationType must not be null"); + Assert.notNull(annotationType, "'annotationType' must not be null"); MergedAnnotationAttributesProcessor processor = new MergedAnnotationAttributesProcessor(false, false, true); searchWithFindSemantics(element, annotationType, null, processor); @@ -803,7 +803,7 @@ public class AnnotatedElementUtils { Class annotationType, Class containerType) { Assert.notNull(element, "AnnotatedElement must not be null"); - Assert.notNull(annotationType, "annotationType must not be null"); + Assert.notNull(annotationType, "'annotationType' must not be null"); if (containerType == null) { containerType = resolveContainerType(annotationType); @@ -910,7 +910,7 @@ public class AnnotatedElementUtils { } } } - catch (Exception ex) { + catch (Throwable ex) { AnnotationUtils.handleIntrospectionFailure(element, ex); } } @@ -1199,7 +1199,7 @@ public class AnnotatedElementUtils { } } } - catch (Exception ex) { + catch (Throwable ex) { AnnotationUtils.handleIntrospectionFailure(element, ex); } } @@ -1241,7 +1241,7 @@ public class AnnotatedElementUtils { try { return (A[]) AnnotationUtils.getValue(container); } - catch (Exception ex) { + catch (Throwable ex) { AnnotationUtils.handleIntrospectionFailure(element, ex); } // Unable to read value from repeating annotation container -> ignore it. @@ -1260,8 +1260,8 @@ public class AnnotatedElementUtils { Class containerType = AnnotationUtils.resolveContainerAnnotationType(annotationType); if (containerType == null) { throw new IllegalArgumentException( - "annotationType must be a repeatable annotation: failed to resolve container type for " - + annotationType.getName()); + "Annotation type must be a repeatable annotation: failed to resolve container type for " + + annotationType.getName()); } return containerType; } @@ -1283,15 +1283,15 @@ public class AnnotatedElementUtils { Class returnType = method.getReturnType(); if (!returnType.isArray() || returnType.getComponentType() != annotationType) { String msg = String.format( - "Container type [%s] must declare a 'value' attribute for an array of type [%s]", - containerType.getName(), annotationType.getName()); + "Container type [%s] must declare a 'value' attribute for an array of type [%s]", + containerType.getName(), annotationType.getName()); throw new AnnotationConfigurationException(msg); } } - catch (Exception ex) { + catch (Throwable ex) { AnnotationUtils.rethrowAnnotationConfigurationException(ex); String msg = String.format("Invalid declaration of container type [%s] for repeatable annotation [%s]", - containerType.getName(), annotationType.getName()); + containerType.getName(), annotationType.getName()); throw new AnnotationConfigurationException(msg, ex); } } diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java index 49bc2d9a6e..c1a98a81e8 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java @@ -158,7 +158,7 @@ public abstract class AnnotationUtils { try { return synthesizeAnnotation(annotatedElement.getAnnotation(annotationType), annotatedElement); } - catch (Exception ex) { + catch (Throwable ex) { handleIntrospectionFailure(annotatedElement, ex); } return null; @@ -189,7 +189,7 @@ public abstract class AnnotationUtils { } return synthesizeAnnotation(annotation, annotatedElement); } - catch (Exception ex) { + catch (Throwable ex) { handleIntrospectionFailure(annotatedElement, ex); } return null; @@ -229,7 +229,7 @@ public abstract class AnnotationUtils { try { return synthesizeAnnotationArray(annotatedElement.getAnnotations(), annotatedElement); } - catch (Exception ex) { + catch (Throwable ex) { handleIntrospectionFailure(annotatedElement, ex); } return null; @@ -251,7 +251,7 @@ public abstract class AnnotationUtils { try { return synthesizeAnnotationArray(BridgeMethodResolver.findBridgedMethod(method).getAnnotations(), method); } - catch (Exception ex) { + catch (Throwable ex) { handleIntrospectionFailure(method, ex); } return null; @@ -440,7 +440,7 @@ public abstract class AnnotationUtils { } return new AnnotationCollector<>(annotationType, containerAnnotationType, declaredMode).getResult(annotatedElement); } - catch (Exception ex) { + catch (Throwable ex) { handleIntrospectionFailure(annotatedElement, ex); } return Collections.emptySet(); @@ -503,7 +503,7 @@ public abstract class AnnotationUtils { } } } - catch (Exception ex) { + catch (Throwable ex) { handleIntrospectionFailure(annotatedElement, ex); } return null; @@ -602,7 +602,7 @@ public abstract class AnnotationUtils { break; } } - catch (Exception ex) { + catch (Throwable ex) { handleIntrospectionFailure(ifcMethod, ex); } } @@ -692,7 +692,7 @@ public abstract class AnnotationUtils { } } } - catch (Exception ex) { + catch (Throwable ex) { handleIntrospectionFailure(clazz, ex); return null; } @@ -809,7 +809,7 @@ public abstract class AnnotationUtils { } } } - catch (Exception ex) { + catch (Throwable ex) { handleIntrospectionFailure(clazz, ex); } return false; @@ -872,12 +872,11 @@ public abstract class AnnotationUtils { /** * Determine if the supplied {@link Annotation} is defined in the core JDK * {@code java.lang.annotation} package. - * @param annotation the annotation to check (never {@code null}) + * @param annotation the annotation to check * @return {@code true} if the annotation is in the {@code java.lang.annotation} package */ public static boolean isInJavaLangAnnotationPackage(Annotation annotation) { - Assert.notNull(annotation, "Annotation must not be null"); - return isInJavaLangAnnotationPackage(annotation.annotationType().getName()); + return (annotation != null && isInJavaLangAnnotationPackage(annotation.annotationType().getName())); } /** @@ -888,8 +887,7 @@ public abstract class AnnotationUtils { * @since 4.2 */ public static boolean isInJavaLangAnnotationPackage(String annotationType) { - Assert.hasText(annotationType, "annotationType must not be null or empty"); - return annotationType.startsWith("java.lang.annotation"); + return (annotationType != null && annotationType.startsWith("java.lang.annotation")); } /** @@ -1046,7 +1044,7 @@ public abstract class AnnotationUtils { attributes.put(method.getName(), adaptValue(annotatedElement, attributeValue, classValuesAsString, nestedAnnotationsAsMap)); } - catch (Exception ex) { + catch (Throwable ex) { if (ex instanceof InvocationTargetException) { Throwable targetException = ((InvocationTargetException) ex).getTargetException(); rethrowAnnotationConfigurationException(targetException); @@ -1461,7 +1459,7 @@ public abstract class AnnotationUtils { public static A synthesizeAnnotation(Map attributes, Class annotationType, AnnotatedElement annotatedElement) { - Assert.notNull(annotationType, "annotationType must not be null"); + Assert.notNull(annotationType, "'annotationType' must not be null"); if (attributes == null) { return null; } @@ -1541,7 +1539,7 @@ public abstract class AnnotationUtils { */ @SuppressWarnings("unchecked") static A[] synthesizeAnnotationArray(Map[] maps, Class annotationType) { - Assert.notNull(annotationType, "annotationType must not be null"); + Assert.notNull(annotationType, "'annotationType' must not be null"); if (maps == null) { return null; } @@ -1809,7 +1807,7 @@ public abstract class AnnotationUtils { * @param ex the exception that we encountered * @see #rethrowAnnotationConfigurationException */ - static void handleIntrospectionFailure(AnnotatedElement element, Exception ex) { + static void handleIntrospectionFailure(AnnotatedElement element, Throwable ex) { rethrowAnnotationConfigurationException(ex); Log loggerToUse = logger; @@ -1921,7 +1919,7 @@ public abstract class AnnotationUtils { } } } - catch (Exception ex) { + catch (Throwable ex) { handleIntrospectionFailure(element, ex); } } @@ -1936,7 +1934,7 @@ public abstract class AnnotationUtils { } return synthesizedAnnotations; } - catch (Exception ex) { + catch (Throwable ex) { handleIntrospectionFailure(element, ex); } // Unable to read value from repeating annotation container -> ignore it. diff --git a/spring-core/src/test/java/org/springframework/core/annotation/ComposedRepeatableAnnotationsTests.java b/spring-core/src/test/java/org/springframework/core/annotation/ComposedRepeatableAnnotationsTests.java index 5cbc10d9a8..f33f71d6ac 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/ComposedRepeatableAnnotationsTests.java +++ b/spring-core/src/test/java/org/springframework/core/annotation/ComposedRepeatableAnnotationsTests.java @@ -186,7 +186,7 @@ public class ComposedRepeatableAnnotationsTests { private void expectNonRepeatableAnnotation() { exception.expect(IllegalArgumentException.class); - exception.expectMessage(startsWith("annotationType must be a repeatable annotation")); + exception.expectMessage(startsWith("Annotation type must be a repeatable annotation")); exception.expectMessage(containsString("failed to resolve container type for")); exception.expectMessage(containsString(NonRepeatable.class.getName())); }