From 6c43d14a77287bbf3272251f6e50563357a6c8c3 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 26 Apr 2017 18:16:40 +0200 Subject: [PATCH] Async annotations on interface methods with CGLIB proxies Issue: SPR-14949 --- .../annotation/AnnotationClassFilter.java | 9 ++--- .../AnnotationMatchingPointcut.java | 33 ++++++++++++++----- .../annotation/AnnotationMethodMatcher.java | 30 +++++++++++++++-- .../annotation/AsyncAnnotationAdvisor.java | 8 ++--- .../annotation/EnableAsyncTests.java | 3 +- 5 files changed, 61 insertions(+), 22 deletions(-) diff --git a/spring-aop/src/main/java/org/springframework/aop/support/annotation/AnnotationClassFilter.java b/spring-aop/src/main/java/org/springframework/aop/support/annotation/AnnotationClassFilter.java index 96f1cc9e0e1..bb7f1967af9 100644 --- a/spring-aop/src/main/java/org/springframework/aop/support/annotation/AnnotationClassFilter.java +++ b/spring-aop/src/main/java/org/springframework/aop/support/annotation/AnnotationClassFilter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2017 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. @@ -48,9 +48,10 @@ public class AnnotationClassFilter implements ClassFilter { /** * Create a new AnnotationClassFilter for the given annotation type. * @param annotationType the annotation type to look for - * @param checkInherited whether to explicitly check the superclasses and - * interfaces for the annotation type as well (even if the annotation type - * is not marked as inherited itself) + * @param checkInherited whether to also check the superclasses and + * interfaces as well as meta-annotations for the annotation type + * (i.e. whether to use {@link AnnotationUtils#findAnnotation(Class, Class)} + * semantics instead of standard Java {@link Class#isAnnotationPresent}) */ public AnnotationClassFilter(Class annotationType, boolean checkInherited) { Assert.notNull(annotationType, "Annotation type must not be null"); diff --git a/spring-aop/src/main/java/org/springframework/aop/support/annotation/AnnotationMatchingPointcut.java b/spring-aop/src/main/java/org/springframework/aop/support/annotation/AnnotationMatchingPointcut.java index 59619830d92..68a77c8503f 100644 --- a/spring-aop/src/main/java/org/springframework/aop/support/annotation/AnnotationMatchingPointcut.java +++ b/spring-aop/src/main/java/org/springframework/aop/support/annotation/AnnotationMatchingPointcut.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2017 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. @@ -46,16 +46,15 @@ public class AnnotationMatchingPointcut implements Pointcut { * @param classAnnotationType the annotation type to look for at the class level */ public AnnotationMatchingPointcut(Class classAnnotationType) { - this.classFilter = new AnnotationClassFilter(classAnnotationType); - this.methodMatcher = MethodMatcher.TRUE; + this(classAnnotationType, false); } /** * Create a new AnnotationMatchingPointcut for the given annotation type. * @param classAnnotationType the annotation type to look for at the class level - * @param checkInherited whether to explicitly check the superclasses and - * interfaces for the annotation type as well (even if the annotation type - * is not marked as inherited itself) + * @param checkInherited whether to also check the superclasses and interfaces + * as well as meta-annotations for the annotation type + * @see AnnotationClassFilter#AnnotationClassFilter(Class, boolean) */ public AnnotationMatchingPointcut(Class classAnnotationType, boolean checkInherited) { this.classFilter = new AnnotationClassFilter(classAnnotationType, checkInherited); @@ -72,18 +71,36 @@ public class AnnotationMatchingPointcut implements Pointcut { public AnnotationMatchingPointcut( Class classAnnotationType, Class methodAnnotationType) { + this(classAnnotationType, methodAnnotationType, false); + } + + /** + * Create a new AnnotationMatchingPointcut for the given annotation type. + * @param classAnnotationType the annotation type to look for at the class level + * (can be {@code null}) + * @param methodAnnotationType the annotation type to look for at the method level + * (can be {@code null}) + * @param checkInherited whether to also check the superclasses and interfaces + * as well as meta-annotations for the annotation type + * @since 5.0 + * @see AnnotationClassFilter#AnnotationClassFilter(Class, boolean) + * @see AnnotationMethodMatcher#AnnotationMethodMatcher(Class, boolean) + */ + public AnnotationMatchingPointcut(Class classAnnotationType, + Class methodAnnotationType, boolean checkInherited) { + Assert.isTrue((classAnnotationType != null || methodAnnotationType != null), "Either Class annotation type or Method annotation type needs to be specified (or both)"); if (classAnnotationType != null) { - this.classFilter = new AnnotationClassFilter(classAnnotationType); + this.classFilter = new AnnotationClassFilter(classAnnotationType, checkInherited); } else { this.classFilter = ClassFilter.TRUE; } if (methodAnnotationType != null) { - this.methodMatcher = new AnnotationMethodMatcher(methodAnnotationType); + this.methodMatcher = new AnnotationMethodMatcher(methodAnnotationType, checkInherited); } else { this.methodMatcher = MethodMatcher.TRUE; diff --git a/spring-aop/src/main/java/org/springframework/aop/support/annotation/AnnotationMethodMatcher.java b/spring-aop/src/main/java/org/springframework/aop/support/annotation/AnnotationMethodMatcher.java index d304b4212f7..ec0c371bbe7 100644 --- a/spring-aop/src/main/java/org/springframework/aop/support/annotation/AnnotationMethodMatcher.java +++ b/spring-aop/src/main/java/org/springframework/aop/support/annotation/AnnotationMethodMatcher.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2017 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. @@ -21,6 +21,7 @@ import java.lang.reflect.Method; import org.springframework.aop.support.AopUtils; import org.springframework.aop.support.StaticMethodMatcher; +import org.springframework.core.annotation.AnnotationUtils; import org.springframework.util.Assert; /** @@ -36,25 +37,48 @@ public class AnnotationMethodMatcher extends StaticMethodMatcher { private final Class annotationType; + private final boolean checkInherited; + /** * Create a new AnnotationClassFilter for the given annotation type. * @param annotationType the annotation type to look for */ public AnnotationMethodMatcher(Class annotationType) { + this(annotationType, false); + } + + /** + * Create a new AnnotationClassFilter for the given annotation type. + * @param annotationType the annotation type to look for + * @param checkInherited whether to also check the superclasses and + * interfaces as well as meta-annotations for the annotation type + * (i.e. whether to use {@link AnnotationUtils#findAnnotation(Method, Class)} + * semantics instead of standard Java {@link Method#isAnnotationPresent}) + * @since 5.0 + */ + public AnnotationMethodMatcher(Class annotationType, boolean checkInherited) { Assert.notNull(annotationType, "Annotation type must not be null"); this.annotationType = annotationType; + this.checkInherited = checkInherited; } + @Override public boolean matches(Method method, Class targetClass) { - if (method.isAnnotationPresent(this.annotationType)) { + if (matchesMethod(method)) { return true; } // The method may be on an interface, so let's check on the target class as well. Method specificMethod = AopUtils.getMostSpecificMethod(method, targetClass); - return (specificMethod != method && specificMethod.isAnnotationPresent(this.annotationType)); + return (specificMethod != method && matchesMethod(specificMethod)); + } + + private boolean matchesMethod(Method method) { + return (this.checkInherited ? + (AnnotationUtils.findAnnotation(method, this.annotationType) != null) : + method.isAnnotationPresent(this.annotationType)); } @Override diff --git a/spring-context/src/main/java/org/springframework/scheduling/annotation/AsyncAnnotationAdvisor.java b/spring-context/src/main/java/org/springframework/scheduling/annotation/AsyncAnnotationAdvisor.java index 9e034f6e240..15dea168d7c 100644 --- a/spring-context/src/main/java/org/springframework/scheduling/annotation/AsyncAnnotationAdvisor.java +++ b/spring-context/src/main/java/org/springframework/scheduling/annotation/AsyncAnnotationAdvisor.java @@ -47,10 +47,8 @@ import org.springframework.util.ClassUtils; * * @author Juergen Hoeller * @since 3.0 - * @see org.springframework.dao.annotation.PersistenceExceptionTranslationAdvisor - * @see org.springframework.stereotype.Repository - * @see org.springframework.dao.DataAccessException - * @see org.springframework.dao.support.PersistenceExceptionTranslator + * @see Async + * @see AnnotationAsyncExecutionInterceptor */ @SuppressWarnings("serial") public class AsyncAnnotationAdvisor extends AbstractPointcutAdvisor implements BeanFactoryAware { @@ -157,7 +155,7 @@ public class AsyncAnnotationAdvisor extends AbstractPointcutAdvisor implements B ComposablePointcut result = null; for (Class asyncAnnotationType : asyncAnnotationTypes) { Pointcut cpc = new AnnotationMatchingPointcut(asyncAnnotationType, true); - Pointcut mpc = AnnotationMatchingPointcut.forMethodAnnotation(asyncAnnotationType); + Pointcut mpc = new AnnotationMatchingPointcut(null, asyncAnnotationType, true); if (result == null) { result = new ComposablePointcut(cpc); } diff --git a/spring-context/src/test/java/org/springframework/scheduling/annotation/EnableAsyncTests.java b/spring-context/src/test/java/org/springframework/scheduling/annotation/EnableAsyncTests.java index 3e5cf952f85..f6bc283f9f5 100644 --- a/spring-context/src/test/java/org/springframework/scheduling/annotation/EnableAsyncTests.java +++ b/spring-context/src/test/java/org/springframework/scheduling/annotation/EnableAsyncTests.java @@ -25,7 +25,6 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.Executor; import java.util.concurrent.Future; -import org.junit.Ignore; import org.junit.Test; import org.mockito.Mockito; @@ -215,7 +214,7 @@ public class EnableAsyncTests { ctx.close(); } - @Test @Ignore // TODO + @Test public void spr14949FindsOnInterfaceWithCglibProxy() throws InterruptedException { AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(Spr14949ConfigB.class);