diff --git a/spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java b/spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java index 7b56b21a907..b0473cf1a74 100644 --- a/spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java +++ b/spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java @@ -49,13 +49,13 @@ import org.springframework.aop.ProxyMethodInvocation; import org.springframework.aop.framework.autoproxy.ProxyCreationContext; import org.springframework.aop.interceptor.ExposeInvocationInterceptor; import org.springframework.aop.support.AbstractExpressionPointcut; +import org.springframework.aop.support.AopUtils; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; import org.springframework.beans.factory.BeanFactoryUtils; import org.springframework.beans.factory.FactoryBean; import org.springframework.beans.factory.annotation.BeanFactoryAnnotationUtils; import org.springframework.beans.factory.config.ConfigurableBeanFactory; -import org.springframework.core.BridgeMethodResolver; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -426,19 +426,15 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut } private ShadowMatch getTargetShadowMatch(Method method, @Nullable Class targetClass) { - Method targetMethod = method; - if (targetClass != null) { - targetMethod = ClassUtils.getMostSpecificMethod(method, ClassUtils.getUserClass(targetClass)); - if (targetMethod.getDeclaringClass().isInterface()) { - Set> ifcs = ClassUtils.getAllInterfacesForClassAsSet(targetClass); - if (ifcs.size() > 1) { - Class compositeInterface = ClassUtils.createCompositeInterface( - ClassUtils.toClassArray(ifcs), targetClass.getClassLoader()); - targetMethod = ClassUtils.getMostSpecificMethod(targetMethod, compositeInterface); - } + Method targetMethod = AopUtils.getMostSpecificMethod(method, targetClass); + if (targetClass != null && targetMethod.getDeclaringClass().isInterface()) { + Set> ifcs = ClassUtils.getAllInterfacesForClassAsSet(targetClass); + if (ifcs.size() > 1) { + Class compositeInterface = ClassUtils.createCompositeInterface( + ClassUtils.toClassArray(ifcs), targetClass.getClassLoader()); + targetMethod = ClassUtils.getMostSpecificMethod(targetMethod, compositeInterface); } } - targetMethod = BridgeMethodResolver.findBridgedMethod(targetMethod); return getShadowMatch(targetMethod, method); } diff --git a/spring-aop/src/main/java/org/springframework/aop/support/AopUtils.java b/spring-aop/src/main/java/org/springframework/aop/support/AopUtils.java index a6f87f573e3..d7e3665f1b6 100644 --- a/spring-aop/src/main/java/org/springframework/aop/support/AopUtils.java +++ b/spring-aop/src/main/java/org/springframework/aop/support/AopUtils.java @@ -192,8 +192,7 @@ public abstract class AopUtils { * @see org.springframework.util.ClassUtils#getMostSpecificMethod */ public static Method getMostSpecificMethod(Method method, @Nullable Class targetClass) { - Class specificTargetClass = (targetClass != null && !Proxy.isProxyClass(targetClass) ? - ClassUtils.getUserClass(targetClass) : null); + Class specificTargetClass = (targetClass != null ? ClassUtils.getUserClass(targetClass) : null); Method resolvedMethod = ClassUtils.getMostSpecificMethod(method, specificTargetClass); // If we are dealing with method with generic parameters, find the original method. return BridgeMethodResolver.findBridgedMethod(resolvedMethod); @@ -247,8 +246,8 @@ public abstract class AopUtils { for (Class clazz : classes) { Method[] methods = ReflectionUtils.getAllDeclaredMethods(clazz); for (Method method : methods) { - if ((introductionAwareMethodMatcher != null && - introductionAwareMethodMatcher.matches(method, targetClass, hasIntroductions)) || + if (introductionAwareMethodMatcher != null ? + introductionAwareMethodMatcher.matches(method, targetClass, hasIntroductions) : methodMatcher.matches(method, targetClass)) { return 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 90836d9af39..63caee265e7 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-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -18,6 +18,7 @@ package org.springframework.aop.support.annotation; import java.lang.annotation.Annotation; import java.lang.reflect.Method; +import java.lang.reflect.Proxy; import org.springframework.aop.support.AopUtils; import org.springframework.aop.support.StaticMethodMatcher; @@ -71,6 +72,10 @@ public class AnnotationMethodMatcher extends StaticMethodMatcher { if (matchesMethod(method)) { return true; } + // Proxy classes never have annotations on their redeclared methods. + if (targetClass != null && Proxy.isProxyClass(targetClass)) { + return false; + } // 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 && matchesMethod(specificMethod)); diff --git a/spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AnnotationPointcutTests.java b/spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AnnotationPointcutTests.java index 85578d5ffa2..e34b795c77a 100644 --- a/spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AnnotationPointcutTests.java +++ b/spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AnnotationPointcutTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2018 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. @@ -33,14 +33,16 @@ public class AnnotationPointcutTests { private AnnotatedTestBean testBean; + @Before - public void setUp() { + public void setup() { ClassPathXmlApplicationContext ctx = - new ClassPathXmlApplicationContext(getClass().getSimpleName() + "-context.xml", getClass()); + new ClassPathXmlApplicationContext(getClass().getSimpleName() + "-context.xml", getClass()); testBean = (AnnotatedTestBean) ctx.getBean("testBean"); } + @Test public void testAnnotationBindingInAroundAdvice() { assertEquals("this value", testBean.doThis()); @@ -61,4 +63,3 @@ class TestMethodInterceptor implements MethodInterceptor { return "this value"; } } - diff --git a/spring-tx/src/test/java/org/springframework/transaction/annotation/AnnotationTransactionInterceptorTests.java b/spring-tx/src/test/java/org/springframework/transaction/annotation/AnnotationTransactionInterceptorTests.java index 8cdec71df9c..f35559342d3 100644 --- a/spring-tx/src/test/java/org/springframework/transaction/annotation/AnnotationTransactionInterceptorTests.java +++ b/spring-tx/src/test/java/org/springframework/transaction/annotation/AnnotationTransactionInterceptorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2018 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. @@ -167,10 +167,13 @@ public class AnnotationTransactionInterceptorTests { proxy.doSomething(); assertGetTransactionAndCommitCount(4); + + proxy.doSomethingDefault(); + assertGetTransactionAndCommitCount(5); } @Test - public void crossClassInterfaceMethodLevelOnJdkProxy() throws Exception { + public void crossClassInterfaceMethodLevelOnJdkProxy() { ProxyFactory proxyFactory = new ProxyFactory(); proxyFactory.setTarget(new SomeServiceImpl()); proxyFactory.addInterface(SomeService.class); @@ -189,7 +192,7 @@ public class AnnotationTransactionInterceptorTests { } @Test - public void crossClassInterfaceOnJdkProxy() throws Exception { + public void crossClassInterfaceOnJdkProxy() { ProxyFactory proxyFactory = new ProxyFactory(); proxyFactory.setTarget(new OtherServiceImpl()); proxyFactory.addInterface(OtherService.class); @@ -201,6 +204,64 @@ public class AnnotationTransactionInterceptorTests { assertGetTransactionAndCommitCount(1); } + @Test + public void withInterfaceOnTargetJdkProxy() { + ProxyFactory targetFactory = new ProxyFactory(); + targetFactory.setTarget(new TestWithInterfaceImpl()); + targetFactory.addInterface(TestWithInterface.class); + + ProxyFactory proxyFactory = new ProxyFactory(); + proxyFactory.setTarget(targetFactory.getProxy()); + proxyFactory.addInterface(TestWithInterface.class); + proxyFactory.addAdvice(this.ti); + + TestWithInterface proxy = (TestWithInterface) proxyFactory.getProxy(); + + proxy.doSomething(); + assertGetTransactionAndCommitCount(1); + + proxy.doSomethingElse(); + assertGetTransactionAndCommitCount(2); + + proxy.doSomethingElse(); + assertGetTransactionAndCommitCount(3); + + proxy.doSomething(); + assertGetTransactionAndCommitCount(4); + + proxy.doSomethingDefault(); + assertGetTransactionAndCommitCount(5); + } + + @Test + public void withInterfaceOnTargetCglibProxy() { + ProxyFactory targetFactory = new ProxyFactory(); + targetFactory.setTarget(new TestWithInterfaceImpl()); + targetFactory.setProxyTargetClass(true); + + ProxyFactory proxyFactory = new ProxyFactory(); + proxyFactory.setTarget(targetFactory.getProxy()); + proxyFactory.addInterface(TestWithInterface.class); + proxyFactory.addAdvice(this.ti); + + TestWithInterface proxy = (TestWithInterface) proxyFactory.getProxy(); + + proxy.doSomething(); + assertGetTransactionAndCommitCount(1); + + proxy.doSomethingElse(); + assertGetTransactionAndCommitCount(2); + + proxy.doSomethingElse(); + assertGetTransactionAndCommitCount(3); + + proxy.doSomething(); + assertGetTransactionAndCommitCount(4); + + proxy.doSomethingDefault(); + assertGetTransactionAndCommitCount(5); + } + private void assertGetTransactionAndCommitCount(int expectedCount) { assertEquals(expectedCount, this.ptm.begun); assertEquals(expectedCount, this.ptm.commits); @@ -309,13 +370,22 @@ public class AnnotationTransactionInterceptorTests { } - @Transactional - public static interface TestWithInterface { + public interface BaseInterface { + + void doSomething(); + } - public void doSomething(); + + @Transactional + public interface TestWithInterface extends BaseInterface { @Transactional(readOnly = true) - public void doSomethingElse(); + void doSomethingElse(); + + default void doSomethingDefault() { + assertTrue(TransactionSynchronizationManager.isActualTransactionActive()); + assertFalse(TransactionSynchronizationManager.isCurrentTransactionReadOnly()); + } } @@ -335,7 +405,7 @@ public class AnnotationTransactionInterceptorTests { } - public static interface SomeService { + public interface SomeService { void foo();