From bd0f68d095ba881621b8499925334de38a6b1051 Mon Sep 17 00:00:00 2001 From: David Syer Date: Fri, 24 Jun 2011 17:23:43 +0000 Subject: [PATCH] SPR-5749: Add defensive matching using target class loader * Changes to ASpectJExpressionPointcut plus some tests in Spring AOP * plus some tests in groovy support --- .../aspectj/AspectJExpressionPointcut.java | 145 ++++++++++++--- .../TrickyAspectJPointcutExpressionTests.java | 174 ++++++++++++++++++ ...ntegrationTests-groovy-dynamic-context.xml | 21 +++ ...egrationTests-groovy-interface-context.xml | 20 ++ ...ests-groovy-proxy-target-class-context.xml | 20 ++ ...ovyAspectIntegrationTests-java-context.xml | 20 ++ .../groovy/GroovyAspectIntegrationTests.java | 100 ++++++++++ .../scripting/groovy/GroovyAspectTests.java | 101 ++++++++++ .../scripting/groovy/GroovyServiceImpl.grv | 11 ++ .../springframework/scripting/groovy/Log.java | 15 ++ .../scripting/groovy/LogUserAdvice.java | 41 +++++ .../scripting/groovy/TestException.java | 29 +++ .../scripting/groovy/TestService.java | 5 + .../scripting/groovy/TestServiceImpl.java | 8 + 14 files changed, 685 insertions(+), 25 deletions(-) create mode 100644 org.springframework.aop/src/test/java/org/springframework/aop/aspectj/TrickyAspectJPointcutExpressionTests.java create mode 100644 org.springframework.context/src/test/java/org/springframework/scripting/groovy/GroovyAspectIntegrationTests-groovy-dynamic-context.xml create mode 100644 org.springframework.context/src/test/java/org/springframework/scripting/groovy/GroovyAspectIntegrationTests-groovy-interface-context.xml create mode 100644 org.springframework.context/src/test/java/org/springframework/scripting/groovy/GroovyAspectIntegrationTests-groovy-proxy-target-class-context.xml create mode 100644 org.springframework.context/src/test/java/org/springframework/scripting/groovy/GroovyAspectIntegrationTests-java-context.xml create mode 100644 org.springframework.context/src/test/java/org/springframework/scripting/groovy/GroovyAspectIntegrationTests.java create mode 100644 org.springframework.context/src/test/java/org/springframework/scripting/groovy/GroovyAspectTests.java create mode 100644 org.springframework.context/src/test/java/org/springframework/scripting/groovy/GroovyServiceImpl.grv create mode 100644 org.springframework.context/src/test/java/org/springframework/scripting/groovy/Log.java create mode 100644 org.springframework.context/src/test/java/org/springframework/scripting/groovy/LogUserAdvice.java create mode 100644 org.springframework.context/src/test/java/org/springframework/scripting/groovy/TestException.java create mode 100644 org.springframework.context/src/test/java/org/springframework/scripting/groovy/TestService.java create mode 100644 org.springframework.context/src/test/java/org/springframework/scripting/groovy/TestServiceImpl.java diff --git a/org.springframework.aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java b/org.springframework.aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java index db9c8bc43f3..348b8449c37 100644 --- a/org.springframework.aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java +++ b/org.springframework.aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java @@ -30,6 +30,7 @@ import org.apache.commons.logging.LogFactory; import org.aspectj.weaver.BCException; import org.aspectj.weaver.patterns.NamePattern; import org.aspectj.weaver.reflect.ReflectionWorld; +import org.aspectj.weaver.reflect.ReflectionWorld.ReflectionWorldException; import org.aspectj.weaver.reflect.ShadowMatchImpl; import org.aspectj.weaver.tools.ContextBasedMatcher; import org.aspectj.weaver.tools.FuzzyBoolean; @@ -41,7 +42,6 @@ import org.aspectj.weaver.tools.PointcutParameter; import org.aspectj.weaver.tools.PointcutParser; import org.aspectj.weaver.tools.PointcutPrimitive; import org.aspectj.weaver.tools.ShadowMatch; - import org.springframework.aop.ClassFilter; import org.springframework.aop.IntroductionAwareMethodMatcher; import org.springframework.aop.MethodMatcher; @@ -73,6 +73,7 @@ import org.springframework.util.StringUtils; * @author Rod Johnson * @author Juergen Hoeller * @author Ramnivas Laddad + * @author Dave Syer * @since 2.0 */ public class AspectJExpressionPointcut extends AbstractExpressionPointcut @@ -186,30 +187,40 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut * Build the underlying AspectJ pointcut expression. */ private PointcutExpression buildPointcutExpression() { - PointcutParser parser = initializePointcutParser(); + ClassLoader cl = (this.beanFactory instanceof ConfigurableBeanFactory ? ((ConfigurableBeanFactory) this.beanFactory) + .getBeanClassLoader() : Thread.currentThread() + .getContextClassLoader()); + return buildPointcutExpression(cl); + } + + /** + * Build the underlying AspectJ pointcut expression. + */ + private PointcutExpression buildPointcutExpression(ClassLoader classLoader) { + PointcutParser parser = initializePointcutParser(classLoader); PointcutParameter[] pointcutParameters = new PointcutParameter[this.pointcutParameterNames.length]; for (int i = 0; i < pointcutParameters.length; i++) { pointcutParameters[i] = parser.createPointcutParameter( - this.pointcutParameterNames[i], this.pointcutParameterTypes[i]); + this.pointcutParameterNames[i], + this.pointcutParameterTypes[i]); } return parser.parsePointcutExpression( - replaceBooleanOperators(getExpression()), this.pointcutDeclarationScope, pointcutParameters); + replaceBooleanOperators(getExpression()), + this.pointcutDeclarationScope, pointcutParameters); } /** * Initialize the underlying AspectJ pointcut parser. */ - private PointcutParser initializePointcutParser() { - ClassLoader cl = (this.beanFactory instanceof ConfigurableBeanFactory ? - ((ConfigurableBeanFactory) this.beanFactory).getBeanClassLoader() : - Thread.currentThread().getContextClassLoader()); - PointcutParser parser = - PointcutParser.getPointcutParserSupportingSpecifiedPrimitivesAndUsingSpecifiedClassLoaderForResolution( + private PointcutParser initializePointcutParser(ClassLoader cl) { + PointcutParser parser = PointcutParser + .getPointcutParserSupportingSpecifiedPrimitivesAndUsingSpecifiedClassLoaderForResolution( SUPPORTED_PRIMITIVES, cl); parser.registerPointcutDesignatorHandler(new BeanNamePointcutDesignatorHandler()); return parser; } + /** * If a pointcut expression has been specified in XML, the user cannot * write and as "&&" (though && will work). @@ -236,7 +247,19 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut checkReadyToMatch(); try { return this.pointcutExpression.couldMatchJoinPointsInType(targetClass); - } + } catch (ReflectionWorldException e) { + logger.debug("PointcutExpression matching rejected target class", e); + try { + // Actually this is still a "maybe" - treat the pointcut as dynamic if we + // don't know enough yet + return getFallbackPointcutExpression(targetClass).couldMatchJoinPointsInType(targetClass); + } catch (BCException ex) { + logger.debug( + "Fallback PointcutExpression matching rejected target class", + ex); + return false; + } + } catch (BCException ex) { logger.debug("PointcutExpression matching rejected target class", ex); return false; @@ -308,7 +331,7 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut *

See SPR-2979 for the original bug. */ if (pmi != null) { // there is a current invocation - RuntimeTestWalker originalMethodResidueTest = new RuntimeTestWalker(originalShadowMatch); + RuntimeTestWalker originalMethodResidueTest = getRuntimeTestWalker(originalShadowMatch); if (!originalMethodResidueTest.testThisInstanceOfResidue(thisObject.getClass())) { return false; } @@ -325,6 +348,16 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut } + /** + * Get a new pointcut expression based on a target class's loader, rather + * than the default. + */ + private PointcutExpression getFallbackPointcutExpression( + Class targetClass) { + ClassLoader classLoader = targetClass.getClassLoader(); + return classLoader == null ? this.pointcutExpression : buildPointcutExpression(classLoader); + } + /** * A match test returned maybe - if there are any subtype sensitive variables * involved in the test (this, target, at_this, at_target, at_annotation) then @@ -332,11 +365,18 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut * runtime subtype. */ private boolean matchesIgnoringSubtypes(ShadowMatch shadowMatch) { - return !(new RuntimeTestWalker(shadowMatch).testsSubtypeSensitiveVars()); + return !(getRuntimeTestWalker(shadowMatch).testsSubtypeSensitiveVars()); } private boolean matchesTarget(ShadowMatch shadowMatch, Class targetClass) { - return new RuntimeTestWalker(shadowMatch).testTargetInstanceOfResidue(targetClass); + return getRuntimeTestWalker(shadowMatch).testTargetInstanceOfResidue(targetClass); + } + + private RuntimeTestWalker getRuntimeTestWalker(ShadowMatch shadowMatch) { + if (shadowMatch instanceof DefensiveShadowMatch) { + return new RuntimeTestWalker(((DefensiveShadowMatch)shadowMatch).primary); + } + return new RuntimeTestWalker(shadowMatch); } private void bindParameters(ProxyMethodInvocation invocation, JoinPointMatch jpm) { @@ -355,7 +395,9 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut if (shadowMatch == null) { synchronized (this.shadowMatchCache) { // Not found - now check again with full lock... - shadowMatch = this.shadowMatchCache.get(targetMethod); + Method methodToMatch = targetMethod; + PointcutExpression fallbackPointcutExpression = null; + shadowMatch = this.shadowMatchCache.get(methodToMatch); if (shadowMatch == null) { try { shadowMatch = this.pointcutExpression.matchesMethodExecution(targetMethod); @@ -363,20 +405,35 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut catch (ReflectionWorld.ReflectionWorldException ex) { // Failed to introspect target method, probably because it has been loaded // in a special ClassLoader. Let's try the original method instead... - if (targetMethod == originalMethod) { - shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null); - } - else { - try { - shadowMatch = this.pointcutExpression.matchesMethodExecution(originalMethod); - } - catch (ReflectionWorld.ReflectionWorldException ex2) { - // Could neither introspect the target class nor the proxy class -> - // let's simply consider this method as non-matching. + try { + fallbackPointcutExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass()); + shadowMatch = fallbackPointcutExpression.matchesMethodExecution(methodToMatch); + } catch (ReflectionWorld.ReflectionWorldException e) { + if (targetMethod == originalMethod) { shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null); } + else { + try { + shadowMatch = this.pointcutExpression.matchesMethodExecution(originalMethod); + } + catch (ReflectionWorld.ReflectionWorldException ex2) { + // Could neither introspect the target class nor the proxy class -> + // let's simply consider this method as non-matching. + methodToMatch = originalMethod; + fallbackPointcutExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass()); + try { + shadowMatch = fallbackPointcutExpression.matchesMethodExecution(methodToMatch); + } catch (ReflectionWorld.ReflectionWorldException e2) { + shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null); + } + } + } } } + if (shadowMatch.maybeMatches() && fallbackPointcutExpression!=null) { + shadowMatch = new DefensiveShadowMatch(shadowMatch, + fallbackPointcutExpression.matchesMethodExecution(methodToMatch)); + } this.shadowMatchCache.put(targetMethod, shadowMatch); } } @@ -543,4 +600,42 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut this.shadowMatchCache = new ConcurrentHashMap(32); } + private static class DefensiveShadowMatch implements ShadowMatch { + + private final ShadowMatch primary; + private final ShadowMatch other; + + public DefensiveShadowMatch(ShadowMatch primary, ShadowMatch other) { + this.primary = primary; + this.other = other; + } + + public boolean alwaysMatches() { + return primary.alwaysMatches(); + } + + public boolean maybeMatches() { + return primary.maybeMatches(); + } + + public boolean neverMatches() { + return primary.neverMatches(); + } + + public JoinPointMatch matchesJoinPoint(Object thisObject, + Object targetObject, Object[] args) { + try { + return primary.matchesJoinPoint(thisObject, targetObject, args); + } catch (ReflectionWorldException e) { + return other.matchesJoinPoint(thisObject, targetObject, args); + } + } + + public void setMatchingContext(MatchingContext aMatchContext) { + primary.setMatchingContext(aMatchContext); + other.setMatchingContext(aMatchContext); + } + + } + } diff --git a/org.springframework.aop/src/test/java/org/springframework/aop/aspectj/TrickyAspectJPointcutExpressionTests.java b/org.springframework.aop/src/test/java/org/springframework/aop/aspectj/TrickyAspectJPointcutExpressionTests.java new file mode 100644 index 00000000000..be1fb06e37f --- /dev/null +++ b/org.springframework.aop/src/test/java/org/springframework/aop/aspectj/TrickyAspectJPointcutExpressionTests.java @@ -0,0 +1,174 @@ +package org.springframework.aop.aspectj; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Inherited; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import java.lang.reflect.Method; + +import org.junit.Test; +import org.springframework.aop.Advisor; +import org.springframework.aop.MethodBeforeAdvice; +import org.springframework.aop.ThrowsAdvice; +import org.springframework.aop.framework.ProxyFactory; +import org.springframework.aop.support.DefaultPointcutAdvisor; +import org.springframework.core.OverridingClassLoader; + +/** + * @author Dave Syer + */ +public class TrickyAspectJPointcutExpressionTests { + + @Test + public void testManualProxyJavaWithUnconditionalPointcut() throws Exception { + TestService target = new TestServiceImpl(); + LogUserAdvice logAdvice = new LogUserAdvice(); + testAdvice(new DefaultPointcutAdvisor(logAdvice), logAdvice, target, "TestServiceImpl"); + } + + @Test + public void testManualProxyJavaWithStaticPointcut() throws Exception { + TestService target = new TestServiceImpl(); + LogUserAdvice logAdvice = new LogUserAdvice(); + AspectJExpressionPointcut pointcut = new AspectJExpressionPointcut(); + pointcut.setExpression(String.format("execution(* %s.TestService.*(..))", getClass().getName())); + testAdvice(new DefaultPointcutAdvisor(pointcut, logAdvice), logAdvice, target, "TestServiceImpl"); + } + + @Test + public void testManualProxyJavaWithDynamicPointcut() throws Exception { + TestService target = new TestServiceImpl(); + LogUserAdvice logAdvice = new LogUserAdvice(); + AspectJExpressionPointcut pointcut = new AspectJExpressionPointcut(); + pointcut.setExpression(String.format("@within(%s.Log)", getClass().getName())); + testAdvice(new DefaultPointcutAdvisor(pointcut, logAdvice), logAdvice, target, "TestServiceImpl"); + } + + @Test + public void testManualProxyJavaWithDynamicPointcutAndProxyTargetClass() throws Exception { + TestService target = new TestServiceImpl(); + LogUserAdvice logAdvice = new LogUserAdvice(); + AspectJExpressionPointcut pointcut = new AspectJExpressionPointcut(); + pointcut.setExpression(String.format("@within(%s.Log)", getClass().getName())); + testAdvice(new DefaultPointcutAdvisor(pointcut, logAdvice), logAdvice, target, "TestServiceImpl", true); + } + + @Test + public void testManualProxyJavaWithStaticPointcutAndTwoClassLoaders() throws Exception { + + LogUserAdvice logAdvice = new LogUserAdvice(); + AspectJExpressionPointcut pointcut = new AspectJExpressionPointcut(); + pointcut.setExpression(String.format("execution(* %s.TestService.*(..))", getClass().getName())); + + // Test with default class loader first... + testAdvice(new DefaultPointcutAdvisor(pointcut, logAdvice), logAdvice, new TestServiceImpl(), "TestServiceImpl"); + + // Then try again with a different class loader on the target... + SimpleThrowawayClassLoader loader = new SimpleThrowawayClassLoader(new TestServiceImpl().getClass().getClassLoader()); + // Make sure the interface is loaded from the parent class loader + loader.excludeClass(TestService.class.getName()); + loader.excludeClass(TestException.class.getName()); + TestService other = (TestService) loader.loadClass(TestServiceImpl.class.getName()).newInstance(); + testAdvice(new DefaultPointcutAdvisor(pointcut, logAdvice), logAdvice, other, "TestServiceImpl"); + + } + + private void testAdvice(Advisor advisor, LogUserAdvice logAdvice, TestService target, String message) + throws Exception { + testAdvice(advisor, logAdvice, target, message, false); + } + + private void testAdvice(Advisor advisor, LogUserAdvice logAdvice, TestService target, String message, + boolean proxyTargetClass) throws Exception { + + logAdvice.reset(); + + ProxyFactory factory = new ProxyFactory(target); + factory.setProxyTargetClass(proxyTargetClass); + factory.addAdvisor(advisor); + TestService bean = (TestService) factory.getProxy(); + + assertEquals(0, logAdvice.getCountThrows()); + try { + bean.sayHello(); + fail("Expected exception"); + } catch (TestException e) { + assertEquals(message, e.getMessage()); + } + assertEquals(1, logAdvice.getCountThrows()); + } + + public static class SimpleThrowawayClassLoader extends OverridingClassLoader { + + /** + * Create a new SimpleThrowawayClassLoader for the given class loader. + * @param parent the ClassLoader to build a throwaway ClassLoader for + */ + public SimpleThrowawayClassLoader(ClassLoader parent) { + super(parent); + } + + } + + public static class TestException extends RuntimeException { + + public TestException(String string) { + super(string); + } + + } + + @Target({ ElementType.METHOD, ElementType.TYPE }) + @Retention(RetentionPolicy.RUNTIME) + @Documented + @Inherited + public static @interface Log { + } + + public static interface TestService { + public String sayHello(); + } + + @Log + public static class TestServiceImpl implements TestService{ + public String sayHello() { + throw new TestException("TestServiceImpl"); + } + } + + public class LogUserAdvice implements MethodBeforeAdvice, ThrowsAdvice { + + private int countBefore = 0; + + private int countThrows = 0; + + public void before(Method method, Object[] objects, Object o) throws Throwable { + countBefore++; + } + + public void afterThrowing(Exception e) throws Throwable { + countThrows++; + throw e; + } + + public int getCountBefore() { + return countBefore; + } + + public int getCountThrows() { + return countThrows; + } + + public void reset() { + countThrows = 0; + countBefore = 0; + } + + } + +} diff --git a/org.springframework.context/src/test/java/org/springframework/scripting/groovy/GroovyAspectIntegrationTests-groovy-dynamic-context.xml b/org.springframework.context/src/test/java/org/springframework/scripting/groovy/GroovyAspectIntegrationTests-groovy-dynamic-context.xml new file mode 100644 index 00000000000..835f7ba933f --- /dev/null +++ b/org.springframework.context/src/test/java/org/springframework/scripting/groovy/GroovyAspectIntegrationTests-groovy-dynamic-context.xml @@ -0,0 +1,21 @@ + + + + + + + + + + + + + \ No newline at end of file diff --git a/org.springframework.context/src/test/java/org/springframework/scripting/groovy/GroovyAspectIntegrationTests-groovy-interface-context.xml b/org.springframework.context/src/test/java/org/springframework/scripting/groovy/GroovyAspectIntegrationTests-groovy-interface-context.xml new file mode 100644 index 00000000000..bd1457c6f36 --- /dev/null +++ b/org.springframework.context/src/test/java/org/springframework/scripting/groovy/GroovyAspectIntegrationTests-groovy-interface-context.xml @@ -0,0 +1,20 @@ + + + + + + + + + + + + \ No newline at end of file diff --git a/org.springframework.context/src/test/java/org/springframework/scripting/groovy/GroovyAspectIntegrationTests-groovy-proxy-target-class-context.xml b/org.springframework.context/src/test/java/org/springframework/scripting/groovy/GroovyAspectIntegrationTests-groovy-proxy-target-class-context.xml new file mode 100644 index 00000000000..b483ad880f0 --- /dev/null +++ b/org.springframework.context/src/test/java/org/springframework/scripting/groovy/GroovyAspectIntegrationTests-groovy-proxy-target-class-context.xml @@ -0,0 +1,20 @@ + + + + + + + + + + + + \ No newline at end of file diff --git a/org.springframework.context/src/test/java/org/springframework/scripting/groovy/GroovyAspectIntegrationTests-java-context.xml b/org.springframework.context/src/test/java/org/springframework/scripting/groovy/GroovyAspectIntegrationTests-java-context.xml new file mode 100644 index 00000000000..ce87a94630c --- /dev/null +++ b/org.springframework.context/src/test/java/org/springframework/scripting/groovy/GroovyAspectIntegrationTests-java-context.xml @@ -0,0 +1,20 @@ + + + + + + + + + + + + \ No newline at end of file diff --git a/org.springframework.context/src/test/java/org/springframework/scripting/groovy/GroovyAspectIntegrationTests.java b/org.springframework.context/src/test/java/org/springframework/scripting/groovy/GroovyAspectIntegrationTests.java new file mode 100644 index 00000000000..4bcc10e1a69 --- /dev/null +++ b/org.springframework.context/src/test/java/org/springframework/scripting/groovy/GroovyAspectIntegrationTests.java @@ -0,0 +1,100 @@ +package org.springframework.scripting.groovy; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +import org.junit.After; +import org.junit.Test; +import org.springframework.context.support.GenericXmlApplicationContext; + +/** + * @author Dave Syer + */ +public class GroovyAspectIntegrationTests { + + private GenericXmlApplicationContext context; + + @After + public void close() { + if (context != null) { + context.close(); + } + } + + @Test + public void testJavaBean() { + + context = new GenericXmlApplicationContext(getClass(), getClass().getSimpleName()+"-java-context.xml"); + + TestService bean = context.getBean("javaBean", TestService.class); + LogUserAdvice logAdvice = context.getBean(LogUserAdvice.class); + + assertEquals(0, logAdvice.getCountThrows()); + try { + bean.sayHello(); + fail("Expected exception"); + } catch (RuntimeException e) { + assertEquals("TestServiceImpl", e.getMessage()); + } + assertEquals(1, logAdvice.getCountThrows()); + + } + + @Test + public void testGroovyBeanInterface() { + context = new GenericXmlApplicationContext(getClass(), getClass().getSimpleName()+"-groovy-interface-context.xml"); + + TestService bean = context.getBean("groovyBean", TestService.class); + LogUserAdvice logAdvice = context.getBean(LogUserAdvice.class); + + assertEquals(0, logAdvice.getCountThrows()); + try { + bean.sayHello(); + fail("Expected exception"); + } catch (RuntimeException e) { + assertEquals("GroovyServiceImpl", e.getMessage()); + } + assertEquals(1, logAdvice.getCountThrows()); + } + + + @Test + public void testGroovyBeanDynamic() { + + context = new GenericXmlApplicationContext(getClass(), getClass().getSimpleName()+"-groovy-dynamic-context.xml"); + + TestService bean = context.getBean("groovyBean", TestService.class); + LogUserAdvice logAdvice = context.getBean(LogUserAdvice.class); + + assertEquals(0, logAdvice.getCountThrows()); + try { + bean.sayHello(); + fail("Expected exception"); + } catch (RuntimeException e) { + assertEquals("GroovyServiceImpl", e.getMessage()); + } + // No proxy here because the pointcut only applies to the concrete class, not the interface + assertEquals(0, logAdvice.getCountThrows()); + assertEquals(0, logAdvice.getCountBefore()); + } + + @Test + public void testGroovyBeanProxyTargetClass() { + + context = new GenericXmlApplicationContext(getClass(), getClass().getSimpleName()+"-groovy-proxy-target-class-context.xml"); + + TestService bean = context.getBean("groovyBean", TestService.class); + LogUserAdvice logAdvice = context.getBean(LogUserAdvice.class); + + assertEquals(0, logAdvice.getCountThrows()); + try { + bean.sayHello(); + fail("Expected exception"); + } catch (TestException e) { + assertEquals("GroovyServiceImpl", e.getMessage()); + } + assertEquals(1, logAdvice.getCountBefore()); + assertEquals(1, logAdvice.getCountThrows()); + } + +} diff --git a/org.springframework.context/src/test/java/org/springframework/scripting/groovy/GroovyAspectTests.java b/org.springframework.context/src/test/java/org/springframework/scripting/groovy/GroovyAspectTests.java new file mode 100644 index 00000000000..124842916bc --- /dev/null +++ b/org.springframework.context/src/test/java/org/springframework/scripting/groovy/GroovyAspectTests.java @@ -0,0 +1,101 @@ +package org.springframework.scripting.groovy; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +import org.junit.Test; +import org.springframework.aop.Advisor; +import org.springframework.aop.aspectj.AspectJExpressionPointcut; +import org.springframework.aop.framework.ProxyFactory; +import org.springframework.aop.support.DefaultPointcutAdvisor; +import org.springframework.core.io.ClassPathResource; +import org.springframework.scripting.groovy.GroovyScriptFactory; +import org.springframework.scripting.support.ResourceScriptSource; +import org.springframework.util.ClassUtils; + +/** + * @author Dave Syer + */ +public class GroovyAspectTests { + + @Test + public void testManualGroovyBeanWithUnconditionalPointcut() throws Exception { + + LogUserAdvice logAdvice = new LogUserAdvice(); + + GroovyScriptFactory scriptFactory = new GroovyScriptFactory("GroovyServiceImpl.grv"); + TestService target = (TestService) scriptFactory.getScriptedObject(new ResourceScriptSource( + new ClassPathResource("GroovyServiceImpl.grv", getClass())), null); + + testAdvice(new DefaultPointcutAdvisor(logAdvice), logAdvice, target, "GroovyServiceImpl"); + + } + + @Test + public void testManualGroovyBeanWithStaticPointcut() throws Exception { + LogUserAdvice logAdvice = new LogUserAdvice(); + + GroovyScriptFactory scriptFactory = new GroovyScriptFactory("GroovyServiceImpl.grv"); + TestService target = (TestService) scriptFactory.getScriptedObject(new ResourceScriptSource( + new ClassPathResource("GroovyServiceImpl.grv", getClass())), null); + + AspectJExpressionPointcut pointcut = new AspectJExpressionPointcut(); + pointcut.setExpression(String.format("execution(* %s.TestService+.*(..))", ClassUtils.getPackageName(getClass()))); + testAdvice(new DefaultPointcutAdvisor(pointcut, logAdvice), logAdvice, target, "GroovyServiceImpl", true); + } + + @Test + public void testManualGroovyBeanWithDynamicPointcut() throws Exception { + + LogUserAdvice logAdvice = new LogUserAdvice(); + + GroovyScriptFactory scriptFactory = new GroovyScriptFactory("GroovyServiceImpl.grv"); + TestService target = (TestService) scriptFactory.getScriptedObject(new ResourceScriptSource( + new ClassPathResource("GroovyServiceImpl.grv", getClass())), null); + + AspectJExpressionPointcut pointcut = new AspectJExpressionPointcut(); + pointcut.setExpression(String.format("@within(%s.Log)", ClassUtils.getPackageName(getClass()))); + testAdvice(new DefaultPointcutAdvisor(pointcut, logAdvice), logAdvice, target, "GroovyServiceImpl", false); + + } + + @Test + public void testManualGroovyBeanWithDynamicPointcutProxyTargetClass() throws Exception { + + LogUserAdvice logAdvice = new LogUserAdvice(); + + GroovyScriptFactory scriptFactory = new GroovyScriptFactory("GroovyServiceImpl.grv"); + TestService target = (TestService) scriptFactory.getScriptedObject(new ResourceScriptSource( + new ClassPathResource("GroovyServiceImpl.grv", getClass())), null); + + AspectJExpressionPointcut pointcut = new AspectJExpressionPointcut(); + pointcut.setExpression(String.format("@within(%s.Log)", ClassUtils.getPackageName(getClass()))); + testAdvice(new DefaultPointcutAdvisor(pointcut, logAdvice), logAdvice, target, "GroovyServiceImpl", true); + + } + + private void testAdvice(Advisor advisor, LogUserAdvice logAdvice, TestService target, String message) + throws Exception { + testAdvice(advisor, logAdvice, target, message, false); + } + + private void testAdvice(Advisor advisor, LogUserAdvice logAdvice, TestService target, String message, + boolean proxyTargetClass) throws Exception { + + logAdvice.reset(); + + ProxyFactory factory = new ProxyFactory(target); + factory.setProxyTargetClass(proxyTargetClass); + factory.addAdvisor(advisor); + TestService bean = (TestService) factory.getProxy(); + + assertEquals(0, logAdvice.getCountThrows()); + try { + bean.sayHello(); + fail("Expected exception"); + } catch (TestException e) { + assertEquals(message, e.getMessage()); + } + assertEquals(1, logAdvice.getCountThrows()); + } +} diff --git a/org.springframework.context/src/test/java/org/springframework/scripting/groovy/GroovyServiceImpl.grv b/org.springframework.context/src/test/java/org/springframework/scripting/groovy/GroovyServiceImpl.grv new file mode 100644 index 00000000000..ba7a1e15110 --- /dev/null +++ b/org.springframework.context/src/test/java/org/springframework/scripting/groovy/GroovyServiceImpl.grv @@ -0,0 +1,11 @@ +package org.springframework.scripting.groovy; + +@Log +public class GroovyServiceImpl implements TestService { + + public String sayHello() { + throw new TestException("GroovyServiceImpl"); + } + + +} \ No newline at end of file diff --git a/org.springframework.context/src/test/java/org/springframework/scripting/groovy/Log.java b/org.springframework.context/src/test/java/org/springframework/scripting/groovy/Log.java new file mode 100644 index 00000000000..2d23dfc682e --- /dev/null +++ b/org.springframework.context/src/test/java/org/springframework/scripting/groovy/Log.java @@ -0,0 +1,15 @@ +package org.springframework.scripting.groovy; + +import java.lang.annotation.Inherited; +import java.lang.annotation.Target; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Documented; + +@Target({ ElementType.METHOD, ElementType.TYPE }) +@Retention(RetentionPolicy.RUNTIME) +@Documented +@Inherited +public @interface Log { +} diff --git a/org.springframework.context/src/test/java/org/springframework/scripting/groovy/LogUserAdvice.java b/org.springframework.context/src/test/java/org/springframework/scripting/groovy/LogUserAdvice.java new file mode 100644 index 00000000000..2a553c504e2 --- /dev/null +++ b/org.springframework.context/src/test/java/org/springframework/scripting/groovy/LogUserAdvice.java @@ -0,0 +1,41 @@ +package org.springframework.scripting.groovy; + +import java.lang.reflect.Method; + +import org.springframework.aop.MethodBeforeAdvice; +import org.springframework.aop.ThrowsAdvice; + +public class LogUserAdvice implements MethodBeforeAdvice, ThrowsAdvice { + + private int countBefore = 0; + + private int countThrows = 0; + + public void before(Method method, Object[] objects, Object o) throws Throwable { + countBefore++; + System.out.println("Method:"+method.getName()); + } + + public void afterThrowing(Exception e) throws Throwable { + countThrows++; + System.out.println("***********************************************************************************"); + System.out.println("Exception caught:"); + System.out.println("***********************************************************************************"); + e.printStackTrace(); + throw e; + } + + public int getCountBefore() { + return countBefore; + } + + public int getCountThrows() { + return countThrows; + } + + public void reset() { + countThrows = 0; + countBefore = 0; + } + +} diff --git a/org.springframework.context/src/test/java/org/springframework/scripting/groovy/TestException.java b/org.springframework.context/src/test/java/org/springframework/scripting/groovy/TestException.java new file mode 100644 index 00000000000..d5b47efb9ff --- /dev/null +++ b/org.springframework.context/src/test/java/org/springframework/scripting/groovy/TestException.java @@ -0,0 +1,29 @@ +/* + * Copyright 2002-2011 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.scripting.groovy; + +/** + * @author Dave Syer + * + */ +public class TestException extends RuntimeException { + + public TestException(String string) { + super(string); + } + +} diff --git a/org.springframework.context/src/test/java/org/springframework/scripting/groovy/TestService.java b/org.springframework.context/src/test/java/org/springframework/scripting/groovy/TestService.java new file mode 100644 index 00000000000..33db91aaa79 --- /dev/null +++ b/org.springframework.context/src/test/java/org/springframework/scripting/groovy/TestService.java @@ -0,0 +1,5 @@ +package org.springframework.scripting.groovy; + +public interface TestService { + public String sayHello(); +} diff --git a/org.springframework.context/src/test/java/org/springframework/scripting/groovy/TestServiceImpl.java b/org.springframework.context/src/test/java/org/springframework/scripting/groovy/TestServiceImpl.java new file mode 100644 index 00000000000..f51a623836d --- /dev/null +++ b/org.springframework.context/src/test/java/org/springframework/scripting/groovy/TestServiceImpl.java @@ -0,0 +1,8 @@ +package org.springframework.scripting.groovy; + +@Log +public class TestServiceImpl implements TestService{ + public String sayHello() { + throw new TestException("TestServiceImpl"); + } +}