diff --git a/spring-web/src/main/java/org/springframework/web/bind/annotation/ControllerAdvice.java b/spring-web/src/main/java/org/springframework/web/bind/annotation/ControllerAdvice.java index b65bcef1aa3..e5bac276c21 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/annotation/ControllerAdvice.java +++ b/spring-web/src/main/java/org/springframework/web/bind/annotation/ControllerAdvice.java @@ -16,6 +16,7 @@ package org.springframework.web.bind.annotation; +import java.lang.annotation.Annotation; import java.lang.annotation.Documented; import java.lang.annotation.ElementType; import java.lang.annotation.Retention; @@ -60,29 +61,44 @@ public @interface ControllerAdvice { /** * Alias for the {@link #basePackages()} attribute. * Allows for more concise annotation declarations e.g.: - * {@code @ControllerAdvice("org.my.pkg")} instead of + * {@code @ControllerAdvice("org.my.pkg")} is equivalent to * {@code @ControllerAdvice(basePackages="org.my.pkg")}. + * * @since 4.0 */ String[] value() default {}; /** * Array of base packages. - * Controllers that belong to those base packages will be selected - * to be assisted by the annotated class, e.g.: - * {@code @ControllerAdvice(basePackages="org.my.pkg")} + * Controllers that belong to those base packages will be included, e.g.: + * {@code @ControllerAdvice(basePackages="org.my.pkg")} or * {@code @ControllerAdvice(basePackages={"org.my.pkg","org.my.other.pkg"})} * *

{@link #value()} is an alias for this attribute. - *

Use {@link #basePackageClasses()} for a type-safe alternative to String-based package names. + *

Also consider using {@link #basePackageClasses()} as a type-safe + * alternative to String-based package names. + * * @since 4.0 */ String[] basePackages() default {}; + /** + * Type-safe alternative to {@link #value()} for specifying the packages + * to select Controllers to be assisted by the {@code @ControllerAdvice} + * annotated class. + * + *

Consider creating a special no-op marker class or interface in each package + * that serves no purpose other than being referenced by this attribute. + * + * @since 4.0 + */ + Class[] basePackageClasses() default {}; + /** * Array of classes. * Controllers that are assignable to at least one of the given types * will be assisted by the {@code @ControllerAdvice} annotated class. + * * @since 4.0 */ Class[] assignableTypes() default {}; @@ -95,18 +111,9 @@ public @interface ControllerAdvice { * *

Consider creating a special annotation or use a predefined one, * like {@link RestController @RestController}. - * @since 4.0 - */ - Class[] annotations() default {}; - - /** - * Type-safe alternative to {@link #value()} for specifying the packages - * to select Controllers to be assisted by the {@code @ControllerAdvice} - * annotated class. * - *

Consider creating a special no-op marker class or interface in each package - * that serves no purpose other than being referenced by this attribute. * @since 4.0 */ - Class[] basePackageClasses() default {}; + Class[] annotations() default {}; + } diff --git a/spring-web/src/main/java/org/springframework/web/method/ControllerAdviceBean.java b/spring-web/src/main/java/org/springframework/web/method/ControllerAdviceBean.java index 7724796bd2d..b82f31bd36f 100644 --- a/spring-web/src/main/java/org/springframework/web/method/ControllerAdviceBean.java +++ b/spring-web/src/main/java/org/springframework/web/method/ControllerAdviceBean.java @@ -46,6 +46,8 @@ import org.springframework.web.bind.annotation.ControllerAdvice; */ public class ControllerAdviceBean implements Ordered { + private static final Log logger = LogFactory.getLog(ControllerAdviceBean.class); + private final Object bean; private final int order; @@ -54,11 +56,10 @@ public class ControllerAdviceBean implements Ordered { private final List basePackages = new ArrayList(); - private final List> annotations = new ArrayList>(); + private final List> annotations = new ArrayList>(); private final List> assignableTypes = new ArrayList>(); - private static final Log logger = LogFactory.getLog(ControllerAdviceBean.class); /** * Create an instance using the given bean name. @@ -70,13 +71,19 @@ public class ControllerAdviceBean implements Ordered { Assert.notNull(beanFactory, "'beanFactory' must not be null"); Assert.isTrue(beanFactory.containsBean(beanName), "Bean factory [" + beanFactory + "] does not contain bean " + "with name [" + beanName + "]"); + this.bean = beanName; this.beanFactory = beanFactory; + Class beanType = this.beanFactory.getType(beanName); this.order = initOrderFromBeanType(beanType); - this.basePackages.addAll(initBasePackagesFromBeanType(beanType)); - this.annotations.addAll(initAnnotationsFromBeanType(beanType)); - this.assignableTypes.addAll(initAssignableTypesFromBeanType(beanType)); + + ControllerAdvice annotation = AnnotationUtils.findAnnotation(beanType,ControllerAdvice.class); + Assert.notNull(annotation, "BeanType [" + beanType.getName() + "] is not annotated @ControllerAdvice"); + + this.basePackages.addAll(initBasePackagesFromBeanType(beanType, annotation)); + this.annotations.addAll(Arrays.asList(annotation.annotations())); + this.assignableTypes.addAll(Arrays.asList(annotation.assignableTypes())); } private static int initOrderFromBeanType(Class beanType) { @@ -84,56 +91,35 @@ public class ControllerAdviceBean implements Ordered { return (annot != null) ? annot.value() : Ordered.LOWEST_PRECEDENCE; } - private static List initBasePackagesFromBeanType(Class beanType) { + private static List initBasePackagesFromBeanType(Class beanType, ControllerAdvice annotation) { List basePackages = new ArrayList(); - ControllerAdvice annotation = AnnotationUtils.findAnnotation(beanType,ControllerAdvice.class); - Assert.notNull(annotation,"BeanType ["+beanType.getName()+"] is not annotated @ControllerAdvice"); - for (String pkgName : (String[])AnnotationUtils.getValue(annotation)) { + List basePackageNames = new ArrayList(); + basePackageNames.addAll(Arrays.asList(annotation.value())); + basePackageNames.addAll(Arrays.asList(annotation.basePackages())); + for (String pkgName : basePackageNames) { if (StringUtils.hasText(pkgName)) { - Package pack = Package.getPackage(pkgName); - if(pack != null) { - basePackages.add(pack); - } else { - logger.warn("Package [" + pkgName + "] was not found, see [" - + beanType.getName() + "]"); + Package pkg = Package.getPackage(pkgName); + if(pkg != null) { + basePackages.add(pkg); + } + else { + logger.warn("Package [" + pkgName + "] was not found, see [" + beanType.getName() + "]"); } } } - for (String pkgName : (String[])AnnotationUtils.getValue(annotation,"basePackages")) { - if (StringUtils.hasText(pkgName)) { - Package pack = Package.getPackage(pkgName); - if(pack != null) { - basePackages.add(pack); - } else { - logger.warn("Package [" + pkgName + "] was not found, see [" - + beanType.getName() + "]"); - } + for (Class markerClass : annotation.basePackageClasses()) { + Package pack = markerClass.getPackage(); + if (pack != null) { + basePackages.add(pack); + } + else { + logger.warn("Package was not found for class [" + markerClass.getName() + + "], see [" + beanType.getName() + "]"); } - } - for (Class markerClass : (Class[])AnnotationUtils.getValue(annotation,"basePackageClasses")) { - Package pack = markerClass.getPackage(); - if(pack != null) { - basePackages.add(pack); - } else { - logger.warn("Package was not found for class [" + markerClass.getName() - + "], see [" + beanType.getName() + "]"); - } } return basePackages; } - private static List> initAnnotationsFromBeanType(Class beanType) { - ControllerAdvice annotation = AnnotationUtils.findAnnotation(beanType,ControllerAdvice.class); - Class[] annotations = (Class[])AnnotationUtils.getValue(annotation,"annotations"); - return Arrays.asList(annotations); - } - - private static List> initAssignableTypesFromBeanType(Class beanType) { - ControllerAdvice annotation = AnnotationUtils.findAnnotation(beanType,ControllerAdvice.class); - Class[] assignableTypes = (Class[])AnnotationUtils.getValue(annotation,"assignableTypes"); - return Arrays.asList(assignableTypes); - } - /** * Create an instance using the given bean instance. * @param bean the bean @@ -142,9 +128,14 @@ public class ControllerAdviceBean implements Ordered { Assert.notNull(bean, "'bean' must not be null"); this.bean = bean; this.order = initOrderFromBean(bean); - this.basePackages.addAll(initBasePackagesFromBeanType(bean.getClass())); - this.annotations.addAll(initAnnotationsFromBeanType(bean.getClass())); - this.assignableTypes.addAll(initAssignableTypesFromBeanType(bean.getClass())); + + Class beanType = bean.getClass(); + ControllerAdvice annotation = AnnotationUtils.findAnnotation(beanType,ControllerAdvice.class); + Assert.notNull(annotation, "BeanType [" + beanType.getName() + "] is not annotated @ControllerAdvice"); + + this.basePackages.addAll(initBasePackagesFromBeanType(beanType, annotation)); + this.annotations.addAll(Arrays.asList(annotation.annotations())); + this.assignableTypes.addAll(Arrays.asList(annotation.assignableTypes())); this.beanFactory = null; } @@ -195,31 +186,31 @@ public class ControllerAdviceBean implements Ordered { } /** - * Checks whether the bean type given as a parameter should be assisted by - * the current {@code @ControllerAdvice} annotated bean. + * Checks whether the given bean type should be assisted by this + * {@code @ControllerAdvice} instance. * - * @param beanType the type of the bean + * @param beanType the type of the bean to check * @see org.springframework.web.bind.annotation.ControllerAdvice * @since 4.0 */ public boolean isApplicableToBeanType(Class beanType) { - if(hasNoSelector()) { + if(!hasSelectors()) { return true; } - else if(beanType != null) { - String packageName = beanType.getPackage().getName(); - for(Package basePackage : this.basePackages) { - if(packageName.startsWith(basePackage.getName())) { + else if (beanType != null) { + for (Class clazz : this.assignableTypes) { + if(ClassUtils.isAssignable(clazz, beanType)) { return true; } } - for(Class annotationClass : this.annotations) { + for (Class annotationClass : this.annotations) { if(AnnotationUtils.findAnnotation(beanType, annotationClass) != null) { return true; } } - for(Class clazz : this.assignableTypes) { - if(ClassUtils.isAssignable(clazz, beanType)) { + String packageName = beanType.getPackage().getName(); + for (Package basePackage : this.basePackages) { + if(packageName.startsWith(basePackage.getName())) { return true; } } @@ -227,9 +218,8 @@ public class ControllerAdviceBean implements Ordered { return false; } - private boolean hasNoSelector() { - return this.basePackages.isEmpty() && this.annotations.isEmpty() - && this.assignableTypes.isEmpty(); + private boolean hasSelectors() { + return (!this.basePackages.isEmpty() || !this.annotations.isEmpty() || !this.assignableTypes.isEmpty()); } diff --git a/spring-web/src/test/java/org/springframework/web/method/ControllerAdviceBeanTests.java b/spring-web/src/test/java/org/springframework/web/method/ControllerAdviceBeanTests.java index 918b146da66..25f0ccefde2 100644 --- a/spring-web/src/test/java/org/springframework/web/method/ControllerAdviceBeanTests.java +++ b/spring-web/src/test/java/org/springframework/web/method/ControllerAdviceBeanTests.java @@ -1,12 +1,12 @@ package org.springframework.web.method; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + import org.junit.Test; import org.springframework.web.bind.annotation.ControllerAdvice; import org.springframework.web.bind.annotation.RestController; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; - import static org.junit.Assert.*; /** @@ -116,12 +116,10 @@ public class ControllerAdviceBeanTests { @ControllerAdvice("org.springframework.web.method") static class BasePackageValueSupport {} - @ControllerAdvice(annotations = ControllerAnnotation.class, - assignableTypes = ControllerInterface.class) + @ControllerAdvice(annotations = ControllerAnnotation.class, assignableTypes = ControllerInterface.class) static class MultipleSelectorsSupport {} - @ControllerAdvice(basePackages = "java.util", - annotations = RestController.class) + @ControllerAdvice(basePackages = "java.util", annotations = {RestController.class}) static class ShouldNotMatch {} // Support classes