From d9cb44527c1f06d6055b805446a38c8817981aaf Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 16 Apr 2014 18:07:10 +0200 Subject: [PATCH] Backported tests for package-visible methods with CGLIB proxies Issue: SPR-11618 (cherry picked from commit 90309ab) --- .../aop/framework/CglibAopProxy.java | 22 ++-- .../aop/framework/CglibProxyTests.java | 103 +++++++++++++----- 2 files changed, 85 insertions(+), 40 deletions(-) diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java b/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java index 90e7080fd9..85d249ac20 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java @@ -261,7 +261,7 @@ class CglibAopProxy implements AopProxy, Serializable { if (!Object.class.equals(method.getDeclaringClass()) && !Modifier.isStatic(method.getModifiers()) && Modifier.isFinal(method.getModifiers())) { logger.warn("Unable to proxy method [" + method + "] because it is final: " + - "All calls to this method via a proxy will be routed directly to the proxy."); + "All calls to this method via a proxy will NOT be routed to the target instance."); } } } @@ -604,7 +604,7 @@ class CglibAopProxy implements AopProxy, Serializable { */ private static class DynamicAdvisedInterceptor implements MethodInterceptor, Serializable { - private AdvisedSupport advised; + private final AdvisedSupport advised; public DynamicAdvisedInterceptor(AdvisedSupport advised) { this.advised = advised; @@ -622,8 +622,8 @@ class CglibAopProxy implements AopProxy, Serializable { oldProxy = AopContext.setCurrentProxy(proxy); setProxyContext = true; } - // May be null Get as late as possible to minimize the time we - // "own" the target, in case it comes from a pool. + // May be null. Get as late as possible to minimize the time we + // "own" the target, in case it comes from a pool... target = getTarget(); if (target != null) { targetClass = target.getClass(); @@ -689,13 +689,13 @@ class CglibAopProxy implements AopProxy, Serializable { private final MethodProxy methodProxy; - private boolean protectedMethod; + private final boolean publicMethod; public CglibMethodInvocation(Object proxy, Object target, Method method, Object[] arguments, Class targetClass, List interceptorsAndDynamicMethodMatchers, MethodProxy methodProxy) { super(proxy, target, method, arguments, targetClass, interceptorsAndDynamicMethodMatchers); this.methodProxy = methodProxy; - this.protectedMethod = Modifier.isProtected(method.getModifiers()); + this.publicMethod = Modifier.isPublic(method.getModifiers()); } /** @@ -704,11 +704,11 @@ class CglibAopProxy implements AopProxy, Serializable { */ @Override protected Object invokeJoinpoint() throws Throwable { - if (this.protectedMethod) { - return super.invokeJoinpoint(); + if (this.publicMethod) { + return this.methodProxy.invoke(this.target, this.arguments); } else { - return this.methodProxy.invoke(this.target, this.arguments); + return super.invokeJoinpoint(); } } } @@ -829,8 +829,8 @@ class CglibAopProxy implements AopProxy, Serializable { // of the target type. If so we know it never needs to have return type // massage and can use a dispatcher. // If the proxy is being exposed, then must use the interceptor the - // correct one is already configured. If the target is not static cannot - // use a Dispatcher because the target can not then be released. + // correct one is already configured. If the target is not static, then + // cannot use a dispatcher because the target cannot be released. if (exposeProxy || !isStatic) { return INVOKE_TARGET; } diff --git a/spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java b/spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java index 8ef8305d9e..af22633fbd 100644 --- a/spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java +++ b/spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java @@ -102,6 +102,7 @@ public final class CglibProxyTests extends AbstractAopProxyTests implements Seri @Test public void testProtectedMethodInvocation() { ProtectedMethodTestBean bean = new ProtectedMethodTestBean(); + bean.value = "foo"; mockTargetSource.setTarget(bean); AdvisedSupport as = new AdvisedSupport(new Class[]{}); @@ -109,8 +110,47 @@ public final class CglibProxyTests extends AbstractAopProxyTests implements Seri as.addAdvice(new NopInterceptor()); AopProxy aop = new CglibAopProxy(as); - Object proxy = aop.getProxy(); + ProtectedMethodTestBean proxy = (ProtectedMethodTestBean) aop.getProxy(); assertTrue(AopUtils.isCglibProxy(proxy)); + assertEquals(proxy.getClass().getClassLoader(), bean.getClass().getClassLoader()); + assertEquals("foo", proxy.getString()); + } + + @Test + public void testPackageMethodInvocation() { + PackageMethodTestBean bean = new PackageMethodTestBean(); + bean.value = "foo"; + mockTargetSource.setTarget(bean); + + AdvisedSupport as = new AdvisedSupport(new Class[]{}); + as.setTargetSource(mockTargetSource); + as.addAdvice(new NopInterceptor()); + AopProxy aop = new CglibAopProxy(as); + + PackageMethodTestBean proxy = (PackageMethodTestBean) aop.getProxy(); + assertTrue(AopUtils.isCglibProxy(proxy)); + assertEquals(proxy.getClass().getClassLoader(), bean.getClass().getClassLoader()); + assertEquals("foo", proxy.getString()); + } + + @Test + public void testPackageMethodInvocationWithDifferentClassLoader() { + ClassLoader child = new ClassLoader(getClass().getClassLoader()) { + }; + + PackageMethodTestBean bean = new PackageMethodTestBean(); + bean.value = "foo"; + mockTargetSource.setTarget(bean); + + AdvisedSupport as = new AdvisedSupport(new Class[]{}); + as.setTargetSource(mockTargetSource); + as.addAdvice(new NopInterceptor()); + AopProxy aop = new CglibAopProxy(as); + + PackageMethodTestBean proxy = (PackageMethodTestBean) aop.getProxy(child); + assertTrue(AopUtils.isCglibProxy(proxy)); + assertNotEquals(proxy.getClass().getClassLoader(), bean.getClass().getClassLoader()); + assertNull(proxy.getString()); // we're stuck in the proxy instance } @Test @@ -410,9 +450,40 @@ public final class CglibProxyTests extends AbstractAopProxyTests implements Seri } - public static class HasFinalMethod { + public static class NoArgCtorTestBean { - public final void foo() { + private boolean called = false; + + public NoArgCtorTestBean(String x, int y) { + called = true; + } + + public boolean wasCalled() { + return called; + } + + public void reset() { + called = false; + } + } + + + public static class ProtectedMethodTestBean { + + public String value; + + protected String getString() { + return this.value; + } + } + + + public static class PackageMethodTestBean { + + public String value; + + String getString() { + return this.value; } } } @@ -436,32 +507,6 @@ class CglibTestBean { } -class NoArgCtorTestBean { - - private boolean called = false; - - public NoArgCtorTestBean(String x, int y) { - called = true; - } - - public boolean wasCalled() { - return called; - } - - public void reset() { - called = false; - } -} - - -class ProtectedMethodTestBean { - - protected String getString() { - return "foo"; - } -} - - class UnsupportedInterceptor implements MethodInterceptor { @Override