From aedef43a9a2b88bb4e44362708c24ea0da8000ee Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 17 Jul 2015 15:24:04 +0200 Subject: [PATCH] Exception fine-tuning and general polishing Issue: SPR-13067 --- ...liasAwareAnnotationAttributeExtractor.java | 34 ++++++++----------- .../AnnotationConfigurationException.java | 4 ++- .../DefaultAnnotationAttributeExtractor.java | 14 ++++---- .../MapAnnotationAttributeExtractor.java | 22 +++++++----- .../SqlScriptsTestExecutionListenerTests.java | 6 ++-- 5 files changed, 41 insertions(+), 39 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AbstractAliasAwareAnnotationAttributeExtractor.java b/spring-core/src/main/java/org/springframework/core/annotation/AbstractAliasAwareAnnotationAttributeExtractor.java index e9f68bc2a2..b97d82b9b1 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/AbstractAliasAwareAnnotationAttributeExtractor.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/AbstractAliasAwareAnnotationAttributeExtractor.java @@ -55,6 +55,7 @@ abstract class AbstractAliasAwareAnnotationAttributeExtractor implements Annotat */ AbstractAliasAwareAnnotationAttributeExtractor(Class annotationType, AnnotatedElement annotatedElement, Object source) { + Assert.notNull(annotationType, "annotationType must not be null"); Assert.notNull(source, "source must not be null"); this.annotationType = annotationType; @@ -63,6 +64,7 @@ abstract class AbstractAliasAwareAnnotationAttributeExtractor implements Annotat this.attributeAliasMap = AnnotationUtils.getAttributeAliasMap(annotationType); } + @Override public final Class getAnnotationType() { return this.annotationType; @@ -84,25 +86,24 @@ abstract class AbstractAliasAwareAnnotationAttributeExtractor implements Annotat Object attributeValue = getRawAttributeValue(attributeMethod); String aliasName = this.attributeAliasMap.get(attributeName); - if ((aliasName != null)) { - + if (aliasName != null) { Object aliasValue = getRawAttributeValue(aliasName); Object defaultValue = AnnotationUtils.getDefaultValue(getAnnotationType(), attributeName); - if (!nullSafeEquals(attributeValue, aliasValue) && !nullSafeEquals(attributeValue, defaultValue) - && !nullSafeEquals(aliasValue, defaultValue)) { - String elementName = (getAnnotatedElement() == null ? "unknown element" - : getAnnotatedElement().toString()); - String msg = String.format("In annotation [%s] declared on [%s] and synthesized from [%s], " - + "attribute [%s] and its alias [%s] are present with values of [%s] and [%s], " - + "but only one is permitted.", getAnnotationType().getName(), elementName, getSource(), - attributeName, aliasName, nullSafeToString(attributeValue), nullSafeToString(aliasValue)); - throw new AnnotationConfigurationException(msg); + if (!ObjectUtils.nullSafeEquals(attributeValue, aliasValue) && + !ObjectUtils.nullSafeEquals(attributeValue, defaultValue) && + !ObjectUtils.nullSafeEquals(aliasValue, defaultValue)) { + String elementName = (getAnnotatedElement() != null ? getAnnotatedElement().toString() : "unknown element"); + throw new AnnotationConfigurationException(String.format( + "In annotation [%s] declared on %s and synthesized from [%s], attribute '%s' and its " + + "alias '%s' are present with values of [%s] and [%s], but only one is permitted.", + getAnnotationType().getName(), elementName, getSource(), attributeName, aliasName, + ObjectUtils.nullSafeToString(attributeValue), ObjectUtils.nullSafeToString(aliasValue))); } // If the user didn't declare the annotation with an explicit value, // return the value of the alias. - if (nullSafeEquals(attributeValue, defaultValue)) { + if (ObjectUtils.nullSafeEquals(attributeValue, defaultValue)) { attributeValue = aliasValue; } } @@ -110,6 +111,7 @@ abstract class AbstractAliasAwareAnnotationAttributeExtractor implements Annotat return attributeValue; } + /** * Get the raw, unmodified attribute value from the underlying * {@linkplain #getSource source} that corresponds to the supplied @@ -124,12 +126,4 @@ abstract class AbstractAliasAwareAnnotationAttributeExtractor implements Annotat */ protected abstract Object getRawAttributeValue(String attributeName); - private static boolean nullSafeEquals(Object o1, Object o2) { - return ObjectUtils.nullSafeEquals(o1, o2); - } - - private static String nullSafeToString(Object obj) { - return ObjectUtils.nullSafeToString(obj); - } - } diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationConfigurationException.java b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationConfigurationException.java index 7e444d838b..0b318077c1 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationConfigurationException.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationConfigurationException.java @@ -16,6 +16,8 @@ package org.springframework.core.annotation; +import org.springframework.core.NestedRuntimeException; + /** * Thrown by {@link AnnotationUtils} and synthesized annotations * if an annotation is improperly configured. @@ -26,7 +28,7 @@ package org.springframework.core.annotation; * @see SynthesizedAnnotation */ @SuppressWarnings("serial") -public class AnnotationConfigurationException extends RuntimeException { +public class AnnotationConfigurationException extends NestedRuntimeException { /** * Construct a new {@code AnnotationConfigurationException} with the diff --git a/spring-core/src/main/java/org/springframework/core/annotation/DefaultAnnotationAttributeExtractor.java b/spring-core/src/main/java/org/springframework/core/annotation/DefaultAnnotationAttributeExtractor.java index 4232c05e29..827e63b4c1 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/DefaultAnnotationAttributeExtractor.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/DefaultAnnotationAttributeExtractor.java @@ -46,20 +46,22 @@ class DefaultAnnotationAttributeExtractor extends AbstractAliasAwareAnnotationAt super(annotation.annotationType(), annotatedElement, annotation); } + + @Override + public Annotation getSource() { + return (Annotation) super.getSource(); + } + @Override protected Object getRawAttributeValue(Method attributeMethod) { ReflectionUtils.makeAccessible(attributeMethod); - return ReflectionUtils.invokeMethod(attributeMethod, getAnnotation()); + return ReflectionUtils.invokeMethod(attributeMethod, getSource()); } @Override protected Object getRawAttributeValue(String attributeName) { - Method attributeMethod = ReflectionUtils.findMethod(getAnnotation().annotationType(), attributeName); + Method attributeMethod = ReflectionUtils.findMethod(getSource().annotationType(), attributeName); return getRawAttributeValue(attributeMethod); } - private Annotation getAnnotation() { - return (Annotation) getSource(); - } - } diff --git a/spring-core/src/main/java/org/springframework/core/annotation/MapAnnotationAttributeExtractor.java b/spring-core/src/main/java/org/springframework/core/annotation/MapAnnotationAttributeExtractor.java index d217d5adbb..05ab3bd185 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/MapAnnotationAttributeExtractor.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/MapAnnotationAttributeExtractor.java @@ -52,23 +52,26 @@ class MapAnnotationAttributeExtractor extends AbstractAliasAwareAnnotationAttrib */ MapAnnotationAttributeExtractor(Map attributes, Class annotationType, AnnotatedElement annotatedElement) { - super(annotationType, annotatedElement, enrichAndValidateAttributes(new HashMap(attributes), annotationType)); + + super(annotationType, annotatedElement, enrichAndValidateAttributes(attributes, annotationType)); + } + + + @SuppressWarnings("unchecked") + public Map getSource() { + return (Map) super.getSource(); } @Override protected Object getRawAttributeValue(Method attributeMethod) { - return getMap().get(attributeMethod.getName()); + return getSource().get(attributeMethod.getName()); } @Override protected Object getRawAttributeValue(String attributeName) { - return getMap().get(attributeName); + return getSource().get(attributeName); } - @SuppressWarnings("unchecked") - private Map getMap() { - return (Map) getSource(); - } /** * Enrich and validate the supplied {@code attributes} map by ensuring @@ -81,9 +84,10 @@ class MapAnnotationAttributeExtractor extends AbstractAliasAwareAnnotationAttrib * an {@link IllegalArgumentException} will be thrown. * @see AliasFor */ - private static Map enrichAndValidateAttributes(Map attributes, - Class annotationType) { + private static Map enrichAndValidateAttributes( + Map original, Class annotationType) { + Map attributes = new HashMap(original); Map attributeAliasMap = getAttributeAliasMap(annotationType); for (Method attributeMethod : getAttributeMethods(annotationType)) { diff --git a/spring-test/src/test/java/org/springframework/test/context/jdbc/SqlScriptsTestExecutionListenerTests.java b/spring-test/src/test/java/org/springframework/test/context/jdbc/SqlScriptsTestExecutionListenerTests.java index 7267b1acf1..306223f8f1 100644 --- a/spring-test/src/test/java/org/springframework/test/context/jdbc/SqlScriptsTestExecutionListenerTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/jdbc/SqlScriptsTestExecutionListenerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 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. @@ -75,8 +75,8 @@ public class SqlScriptsTestExecutionListenerTests { exception.expect(AnnotationConfigurationException.class); exception.expectMessage(either( - containsString("attribute [value] and its alias [scripts]")).or( - containsString("attribute [scripts] and its alias [value]"))); + containsString("attribute 'value' and its alias 'scripts'")).or( + containsString("attribute 'scripts' and its alias 'value'"))); exception.expectMessage(either(containsString("values of [{foo}] and [{bar}]")).or( containsString("values of [{bar}] and [{foo}]"))); exception.expectMessage(containsString("but only one is permitted"));