Skip to content

Commit aa11721

Browse files
jhoellerjhoeller
committed
AopUtils.getMostSpecificMethod exposes dynamic proxy class methods
Includes efficient canApply check for IntroductionAwareMethodMatcher. Issue: SPR-16757
1 parent 75a41db commit aa11721

File tree

5 files changed

+100
-29
lines changed

5 files changed

+100
-29
lines changed

‎spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,13 @@
4949
import org.springframework.aop.framework.autoproxy.ProxyCreationContext;
5050
import org.springframework.aop.interceptor.ExposeInvocationInterceptor;
5151
import org.springframework.aop.support.AbstractExpressionPointcut;
52+
import org.springframework.aop.support.AopUtils;
5253
import org.springframework.beans.factory.BeanFactory;
5354
import org.springframework.beans.factory.BeanFactoryAware;
5455
import org.springframework.beans.factory.BeanFactoryUtils;
5556
import org.springframework.beans.factory.FactoryBean;
5657
import org.springframework.beans.factory.annotation.BeanFactoryAnnotationUtils;
5758
import org.springframework.beans.factory.config.ConfigurableBeanFactory;
58-
import org.springframework.core.BridgeMethodResolver;
5959
import org.springframework.lang.Nullable;
6060
import org.springframework.util.Assert;
6161
import org.springframework.util.ClassUtils;
@@ -426,19 +426,15 @@ private void bindParameters(ProxyMethodInvocation invocation, JoinPointMatch jpm
426426
}
427427

428428
private ShadowMatch getTargetShadowMatch(Method method, @Nullable Class<?> targetClass) {
429-
Method targetMethod = method;
430-
if (targetClass != null) {
431-
targetMethod = ClassUtils.getMostSpecificMethod(method, ClassUtils.getUserClass(targetClass));
432-
if (targetMethod.getDeclaringClass().isInterface()) {
433-
Set<Class<?>> ifcs = ClassUtils.getAllInterfacesForClassAsSet(targetClass);
434-
if (ifcs.size() > 1) {
435-
Class<?> compositeInterface = ClassUtils.createCompositeInterface(
436-
ClassUtils.toClassArray(ifcs), targetClass.getClassLoader());
437-
targetMethod = ClassUtils.getMostSpecificMethod(targetMethod, compositeInterface);
438-
}
429+
Method targetMethod = AopUtils.getMostSpecificMethod(method, targetClass);
430+
if (targetClass != null && targetMethod.getDeclaringClass().isInterface()) {
431+
Set<Class<?>> ifcs = ClassUtils.getAllInterfacesForClassAsSet(targetClass);
432+
if (ifcs.size() > 1) {
433+
Class<?> compositeInterface = ClassUtils.createCompositeInterface(
434+
ClassUtils.toClassArray(ifcs), targetClass.getClassLoader());
435+
targetMethod = ClassUtils.getMostSpecificMethod(targetMethod, compositeInterface);
439436
}
440437
}
441-
targetMethod = BridgeMethodResolver.findBridgedMethod(targetMethod);
442438
return getShadowMatch(targetMethod, method);
443439
}
444440

‎spring-aop/src/main/java/org/springframework/aop/support/AopUtils.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -192,8 +192,7 @@ public static boolean isFinalizeMethod(@Nullable Method method) {
192192
* @see org.springframework.util.ClassUtils#getMostSpecificMethod
193193
*/
194194
public static Method getMostSpecificMethod(Method method, @Nullable Class<?> targetClass) {
195-
Class<?> specificTargetClass = (targetClass != null && !Proxy.isProxyClass(targetClass) ?
196-
ClassUtils.getUserClass(targetClass) : null);
195+
Class<?> specificTargetClass = (targetClass != null ? ClassUtils.getUserClass(targetClass) : null);
197196
Method resolvedMethod = ClassUtils.getMostSpecificMethod(method, specificTargetClass);
198197
// If we are dealing with method with generic parameters, find the original method.
199198
return BridgeMethodResolver.findBridgedMethod(resolvedMethod);
@@ -247,8 +246,8 @@ public static boolean canApply(Pointcut pc, Class<?> targetClass, boolean hasInt
247246
for (Class<?> clazz : classes) {
248247
Method[] methods = ReflectionUtils.getAllDeclaredMethods(clazz);
249248
for (Method method : methods) {
250-
if ((introductionAwareMethodMatcher != null &&
251-
introductionAwareMethodMatcher.matches(method, targetClass, hasIntroductions)) ||
249+
if (introductionAwareMethodMatcher != null ?
250+
introductionAwareMethodMatcher.matches(method, targetClass, hasIntroductions) :
252251
methodMatcher.matches(method, targetClass)) {
253252
return true;
254253
}

‎spring-aop/src/main/java/org/springframework/aop/support/annotation/AnnotationMethodMatcher.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2017 the original author or authors.
2+
* Copyright 2002-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -18,6 +18,7 @@
1818

1919
import java.lang.annotation.Annotation;
2020
import java.lang.reflect.Method;
21+
import java.lang.reflect.Proxy;
2122

2223
import org.springframework.aop.support.AopUtils;
2324
import org.springframework.aop.support.StaticMethodMatcher;
@@ -71,6 +72,10 @@ public boolean matches(Method method, @Nullable Class<?> targetClass) {
7172
if (matchesMethod(method)) {
7273
return true;
7374
}
75+
// Proxy classes never have annotations on their redeclared methods.
76+
if (targetClass != null && Proxy.isProxyClass(targetClass)) {
77+
return false;
78+
}
7479
// The method may be on an interface, so let's check on the target class as well.
7580
Method specificMethod = AopUtils.getMostSpecificMethod(method, targetClass);
7681
return (specificMethod != method && matchesMethod(specificMethod));

‎spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AnnotationPointcutTests.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2012 the original author or authors.
2+
* Copyright 2002-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -33,14 +33,16 @@ public class AnnotationPointcutTests {
3333

3434
private AnnotatedTestBean testBean;
3535

36+
3637
@Before
37-
public void setUp() {
38+
public void setup() {
3839
ClassPathXmlApplicationContext ctx =
39-
new ClassPathXmlApplicationContext(getClass().getSimpleName() + "-context.xml", getClass());
40+
new ClassPathXmlApplicationContext(getClass().getSimpleName() + "-context.xml", getClass());
4041

4142
testBean = (AnnotatedTestBean) ctx.getBean("testBean");
4243
}
4344

45+
4446
@Test
4547
public void testAnnotationBindingInAroundAdvice() {
4648
assertEquals("this value", testBean.doThis());
@@ -61,4 +63,3 @@ public Object invoke(MethodInvocation methodInvocation) throws Throwable {
6163
return "this value";
6264
}
6365
}
64-

‎spring-tx/src/test/java/org/springframework/transaction/annotation/AnnotationTransactionInterceptorTests.java

Lines changed: 78 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2015 the original author or authors.
2+
* Copyright 2002-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -167,10 +167,13 @@ public void withInterface() {
167167

168168
proxy.doSomething();
169169
assertGetTransactionAndCommitCount(4);
170+
171+
proxy.doSomethingDefault();
172+
assertGetTransactionAndCommitCount(5);
170173
}
171174

172175
@Test
173-
public void crossClassInterfaceMethodLevelOnJdkProxy() throws Exception {
176+
public void crossClassInterfaceMethodLevelOnJdkProxy() {
174177
ProxyFactory proxyFactory = new ProxyFactory();
175178
proxyFactory.setTarget(new SomeServiceImpl());
176179
proxyFactory.addInterface(SomeService.class);
@@ -189,7 +192,7 @@ public void crossClassInterfaceMethodLevelOnJdkProxy() throws Exception {
189192
}
190193

191194
@Test
192-
public void crossClassInterfaceOnJdkProxy() throws Exception {
195+
public void crossClassInterfaceOnJdkProxy() {
193196
ProxyFactory proxyFactory = new ProxyFactory();
194197
proxyFactory.setTarget(new OtherServiceImpl());
195198
proxyFactory.addInterface(OtherService.class);
@@ -201,6 +204,64 @@ public void crossClassInterfaceOnJdkProxy() throws Exception {
201204
assertGetTransactionAndCommitCount(1);
202205
}
203206

207+
@Test
208+
public void withInterfaceOnTargetJdkProxy() {
209+
ProxyFactory targetFactory = new ProxyFactory();
210+
targetFactory.setTarget(new TestWithInterfaceImpl());
211+
targetFactory.addInterface(TestWithInterface.class);
212+
213+
ProxyFactory proxyFactory = new ProxyFactory();
214+
proxyFactory.setTarget(targetFactory.getProxy());
215+
proxyFactory.addInterface(TestWithInterface.class);
216+
proxyFactory.addAdvice(this.ti);
217+
218+
TestWithInterface proxy = (TestWithInterface) proxyFactory.getProxy();
219+
220+
proxy.doSomething();
221+
assertGetTransactionAndCommitCount(1);
222+
223+
proxy.doSomethingElse();
224+
assertGetTransactionAndCommitCount(2);
225+
226+
proxy.doSomethingElse();
227+
assertGetTransactionAndCommitCount(3);
228+
229+
proxy.doSomething();
230+
assertGetTransactionAndCommitCount(4);
231+
232+
proxy.doSomethingDefault();
233+
assertGetTransactionAndCommitCount(5);
234+
}
235+
236+
@Test
237+
public void withInterfaceOnTargetCglibProxy() {
238+
ProxyFactory targetFactory = new ProxyFactory();
239+
targetFactory.setTarget(new TestWithInterfaceImpl());
240+
targetFactory.setProxyTargetClass(true);
241+
242+
ProxyFactory proxyFactory = new ProxyFactory();
243+
proxyFactory.setTarget(targetFactory.getProxy());
244+
proxyFactory.addInterface(TestWithInterface.class);
245+
proxyFactory.addAdvice(this.ti);
246+
247+
TestWithInterface proxy = (TestWithInterface) proxyFactory.getProxy();
248+
249+
proxy.doSomething();
250+
assertGetTransactionAndCommitCount(1);
251+
252+
proxy.doSomethingElse();
253+
assertGetTransactionAndCommitCount(2);
254+
255+
proxy.doSomethingElse();
256+
assertGetTransactionAndCommitCount(3);
257+
258+
proxy.doSomething();
259+
assertGetTransactionAndCommitCount(4);
260+
261+
proxy.doSomethingDefault();
262+
assertGetTransactionAndCommitCount(5);
263+
}
264+
204265
private void assertGetTransactionAndCommitCount(int expectedCount) {
205266
assertEquals(expectedCount, this.ptm.begun);
206267
assertEquals(expectedCount, this.ptm.commits);
@@ -309,13 +370,22 @@ public void doSomethingElseErroneous() {
309370
}
310371

311372

312-
@Transactional
313-
public static interface TestWithInterface {
373+
public interface BaseInterface {
374+
375+
void doSomething();
376+
}
314377

315-
public void doSomething();
378+
379+
@Transactional
380+
public interface TestWithInterface extends BaseInterface {
316381

317382
@Transactional(readOnly = true)
318-
public void doSomethingElse();
383+
void doSomethingElse();
384+
385+
default void doSomethingDefault() {
386+
assertTrue(TransactionSynchronizationManager.isActualTransactionActive());
387+
assertFalse(TransactionSynchronizationManager.isCurrentTransactionReadOnly());
388+
}
319389
}
320390

321391

@@ -335,7 +405,7 @@ public void doSomethingElse() {
335405
}
336406

337407

338-
public static interface SomeService {
408+
public interface SomeService {
339409

340410
void foo();
341411

0 commit comments

Comments
 (0)