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 e14f254612..882116ddbc 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2013 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. @@ -42,6 +42,7 @@ 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; @@ -55,6 +56,7 @@ import org.springframework.beans.factory.BeanFactoryAware; import org.springframework.beans.factory.BeanFactoryUtils; import org.springframework.beans.factory.FactoryBean; import org.springframework.beans.factory.config.ConfigurableBeanFactory; +import org.springframework.util.ClassUtils; import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; @@ -98,11 +100,11 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut private static final Log logger = LogFactory.getLog(AspectJExpressionPointcut.class); - private Class pointcutDeclarationScope; + private Class pointcutDeclarationScope; private String[] pointcutParameterNames = new String[0]; - private Class[] pointcutParameterTypes = new Class[0]; + private Class[] pointcutParameterTypes = new Class[0]; private BeanFactory beanFactory; @@ -123,7 +125,7 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut * @param paramNames the parameter names for the pointcut * @param paramTypes the parameter types for the pointcut */ - public AspectJExpressionPointcut(Class declarationScope, String[] paramNames, Class[] paramTypes) { + public AspectJExpressionPointcut(Class declarationScope, String[] paramNames, Class[] paramTypes) { this.pointcutDeclarationScope = declarationScope; if (paramNames.length != paramTypes.length) { throw new IllegalStateException( @@ -137,21 +139,21 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut /** * Set the declaration scope for the pointcut. */ - public void setPointcutDeclarationScope(Class pointcutDeclarationScope) { + public void setPointcutDeclarationScope(Class pointcutDeclarationScope) { this.pointcutDeclarationScope = pointcutDeclarationScope; } /** * Set the parameter names for the pointcut. */ - public void setParameterNames(String[] names) { + public void setParameterNames(String... names) { this.pointcutParameterNames = names; } /** * Set the parameter types for the pointcut. */ - public void setParameterTypes(Class[] types) { + public void setParameterTypes(Class... types) { this.pointcutParameterTypes = types; } @@ -188,9 +190,9 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut * Build the underlying AspectJ pointcut expression. */ private PointcutExpression buildPointcutExpression() { - ClassLoader cl = (this.beanFactory instanceof ConfigurableBeanFactory ? ((ConfigurableBeanFactory) this.beanFactory) - .getBeanClassLoader() : Thread.currentThread() - .getContextClassLoader()); + ClassLoader cl = (this.beanFactory instanceof ConfigurableBeanFactory ? + ((ConfigurableBeanFactory) this.beanFactory).getBeanClassLoader() : + ClassUtils.getDefaultClassLoader()); return buildPointcutExpression(cl); } @@ -202,8 +204,7 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut 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()), @@ -244,20 +245,19 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut return this.pointcutExpression; } - public boolean matches(Class targetClass) { + public boolean matches(Class targetClass) { checkReadyToMatch(); try { return this.pointcutExpression.couldMatchJoinPointsInType(targetClass); - } catch (ReflectionWorldException e) { - logger.debug("PointcutExpression matching rejected target class", e); + } + catch (ReflectionWorldException rwe) { + logger.debug("PointcutExpression matching rejected target class", rwe); try { - // Actually this is still a "maybe" - treat the pointcut as dynamic if we - // don't know enough yet + // 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); + } + catch (BCException bce) { + logger.debug("Fallback PointcutExpression matching rejected target class", bce); return false; } } @@ -267,7 +267,7 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut } } - public boolean matches(Method method, Class targetClass, boolean beanHasIntroductions) { + public boolean matches(Method method, Class targetClass, boolean beanHasIntroductions) { checkReadyToMatch(); Method targetMethod = AopUtils.getMostSpecificMethod(method, targetClass); ShadowMatch shadowMatch = getShadowMatch(targetMethod, method); @@ -283,11 +283,19 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut } else { // the maybe case - return (beanHasIntroductions || matchesIgnoringSubtypes(shadowMatch) || matchesTarget(shadowMatch, targetClass)); + if (beanHasIntroductions) { + return true; + } + // 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 + // we say this is not a match as in Spring there will never be a different + // runtime subtype. + RuntimeTestWalker walker = getRuntimeTestWalker(shadowMatch); + return (!walker.testsSubtypeSensitiveVars() || walker.testTargetInstanceOfResidue(targetClass)); } } - public boolean matches(Method method, Class targetClass) { + public boolean matches(Method method, Class targetClass) { return matches(method, targetClass, false); } @@ -296,7 +304,7 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut return this.pointcutExpression.mayNeedDynamicTest(); } - public boolean matches(Method method, Class targetClass, Object[] args) { + public boolean matches(Method method, Class targetClass, Object[] args) { checkReadyToMatch(); ShadowMatch shadowMatch = getShadowMatch(AopUtils.getMostSpecificMethod(method, targetClass), method); ShadowMatch originalShadowMatch = getShadowMatch(method, method); @@ -336,14 +344,13 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut if (!originalMethodResidueTest.testThisInstanceOfResidue(thisObject.getClass())) { return false; } - } - if (joinPointMatch.matches() && pmi != null) { - bindParameters(pmi, joinPointMatch); + if (joinPointMatch.matches()) { + bindParameters(pmi, joinPointMatch); + } } return joinPointMatch.matches(); } - protected String getCurrentProxiedBeanName() { return ProxyCreationContext.getCurrentProxiedBeanName(); } @@ -353,29 +360,14 @@ 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) { + 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 - * we say this is not a match as in Spring there will never be a different - * runtime subtype. - */ - private boolean matchesIgnoringSubtypes(ShadowMatch shadowMatch) { - return !(getRuntimeTestWalker(shadowMatch).testsSubtypeSensitiveVars()); - } - - private boolean matchesTarget(ShadowMatch shadowMatch, Class targetClass) { - return getRuntimeTestWalker(shadowMatch).testTargetInstanceOfResidue(targetClass); + return (classLoader != null ? buildPointcutExpression(classLoader) : this.pointcutExpression); } private RuntimeTestWalker getRuntimeTestWalker(ShadowMatch shadowMatch) { if (shadowMatch instanceof DefensiveShadowMatch) { - return new RuntimeTestWalker(((DefensiveShadowMatch)shadowMatch).primary); + return new RuntimeTestWalker(((DefensiveShadowMatch) shadowMatch).primary); } return new RuntimeTestWalker(shadowMatch); } @@ -409,7 +401,8 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut try { fallbackPointcutExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass()); shadowMatch = fallbackPointcutExpression.matchesMethodExecution(methodToMatch); - } catch (ReflectionWorld.ReflectionWorldException e) { + } + catch (ReflectionWorld.ReflectionWorldException ex2) { if (targetMethod == originalMethod) { shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null); } @@ -417,21 +410,22 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut try { shadowMatch = this.pointcutExpression.matchesMethodExecution(originalMethod); } - catch (ReflectionWorld.ReflectionWorldException ex2) { + catch (ReflectionWorld.ReflectionWorldException ex3) { // 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) { + } + catch (ReflectionWorld.ReflectionWorldException ex4) { shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null); } } } } } - if (shadowMatch.maybeMatches() && fallbackPointcutExpression!=null) { + if (shadowMatch.maybeMatches() && fallbackPointcutExpression != null) { shadowMatch = new DefensiveShadowMatch(shadowMatch, fallbackPointcutExpression.matchesMethodExecution(methodToMatch)); } @@ -601,9 +595,11 @@ 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) { @@ -612,31 +608,30 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut } public boolean alwaysMatches() { - return primary.alwaysMatches(); + return this.primary.alwaysMatches(); } public boolean maybeMatches() { - return primary.maybeMatches(); + return this.primary.maybeMatches(); } public boolean neverMatches() { - return primary.neverMatches(); + return this.primary.neverMatches(); } - public JoinPointMatch matchesJoinPoint(Object thisObject, - Object targetObject, Object[] args) { + 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); + return this.primary.matchesJoinPoint(thisObject, targetObject, args); + } + catch (ReflectionWorldException ex) { + return this.other.matchesJoinPoint(thisObject, targetObject, args); } } public void setMatchingContext(MatchingContext aMatchContext) { - primary.setMatchingContext(aMatchContext); - other.setMatchingContext(aMatchContext); + this.primary.setMatchingContext(aMatchContext); + this.other.setMatchingContext(aMatchContext); } - } } diff --git a/spring-aop/src/main/java/org/springframework/aop/aspectj/RuntimeTestWalker.java b/spring-aop/src/main/java/org/springframework/aop/aspectj/RuntimeTestWalker.java index fa470759cd..0b20e8dc6e 100644 --- a/spring-aop/src/main/java/org/springframework/aop/aspectj/RuntimeTestWalker.java +++ b/spring-aop/src/main/java/org/springframework/aop/aspectj/RuntimeTestWalker.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2013 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,8 @@ package org.springframework.aop.aspectj; import java.lang.reflect.Field; +import org.aspectj.weaver.ReferenceType; +import org.aspectj.weaver.ReferenceTypeDelegate; import org.aspectj.weaver.ResolvedType; import org.aspectj.weaver.ast.And; import org.aspectj.weaver.ast.Call; @@ -30,6 +32,7 @@ import org.aspectj.weaver.ast.Not; import org.aspectj.weaver.ast.Or; import org.aspectj.weaver.ast.Test; import org.aspectj.weaver.internal.tools.MatchingContextBasedTest; +import org.aspectj.weaver.reflect.ReflectionBasedReferenceTypeDelegate; import org.aspectj.weaver.reflect.ReflectionVar; import org.aspectj.weaver.reflect.ShadowMatchImpl; import org.aspectj.weaver.tools.ShadowMatch; @@ -55,25 +58,36 @@ import org.springframework.util.ReflectionUtils; */ class RuntimeTestWalker { + private static final Field residualTestField; + + private static final Field varTypeField; + + private static final Field myClassField; + + + static { + try { + residualTestField = ShadowMatchImpl.class.getDeclaredField("residualTest"); + varTypeField = ReflectionVar.class.getDeclaredField("varType"); + myClassField = ReflectionBasedReferenceTypeDelegate.class.getDeclaredField("myClass"); + } + catch (NoSuchFieldException ex) { + throw new IllegalStateException("The version of aspectjtools.jar / aspectjweaver.jar " + + "on the classpath is incompatible with this version of Spring: " + ex); + } + } + + private final Test runtimeTest; public RuntimeTestWalker(ShadowMatch shadowMatch) { - ShadowMatchImpl shadowMatchImplementation = (ShadowMatchImpl) shadowMatch; try { - Field testField = shadowMatchImplementation.getClass().getDeclaredField("residualTest"); - ReflectionUtils.makeAccessible(testField); - this.runtimeTest = (Test) testField.get(shadowMatch); + ReflectionUtils.makeAccessible(residualTestField); + this.runtimeTest = (Test) residualTestField.get(shadowMatch); } - catch (NoSuchFieldException noSuchFieldEx) { - throw new IllegalStateException("The version of aspectjtools.jar / aspectjweaver.jar " + - "on the classpath is incompatible with this version of Spring: Expected field " + - "'runtimeTest' is not present on ShadowMatchImpl class."); - } - catch (IllegalAccessException illegalAccessEx) { - // Famous last words... but I don't see how this can happen given the - // makeAccessible call above - throw new IllegalStateException("Unable to access ShadowMatchImpl.residualTest field"); + catch (IllegalAccessException ex) { + throw new IllegalStateException(ex); } } @@ -87,12 +101,12 @@ class RuntimeTestWalker { new SubtypeSensitiveVarTypeTestVisitor().testsSubtypeSensitiveVars(this.runtimeTest)); } - public boolean testThisInstanceOfResidue(Class thisClass) { + public boolean testThisInstanceOfResidue(Class thisClass) { return (this.runtimeTest != null && new ThisInstanceOfResidueTestVisitor(thisClass).thisInstanceOfMatches(this.runtimeTest)); } - public boolean testTargetInstanceOfResidue(Class targetClass) { + public boolean testTargetInstanceOfResidue(Class targetClass) { return (this.runtimeTest != null && new TargetInstanceOfResidueTestVisitor(targetClass).targetInstanceOfMatches(this.runtimeTest)); } @@ -140,19 +154,11 @@ class RuntimeTestWalker { protected int getVarType(ReflectionVar v) { try { - Field varTypeField = ReflectionVar.class.getDeclaredField("varType"); ReflectionUtils.makeAccessible(varTypeField); return (Integer) varTypeField.get(v); } - catch (NoSuchFieldException noSuchFieldEx) { - throw new IllegalStateException("the version of aspectjtools.jar / aspectjweaver.jar " + - "on the classpath is incompatible with this version of Spring:- expected field " + - "'varType' is not present on ReflectionVar class"); - } - catch (IllegalAccessException illegalAccessEx) { - // Famous last words... but I don't see how this can happen given the - // makeAccessible call above - throw new IllegalStateException("Unable to access ReflectionVar.varType field"); + catch (IllegalAccessException ex) { + throw new IllegalStateException(ex); } } } @@ -160,11 +166,13 @@ class RuntimeTestWalker { private static abstract class InstanceOfResidueTestVisitor extends TestVisitorAdapter { - private Class matchClass; - private boolean matches; - private int matchVarType; + private final Class matchClass; - public InstanceOfResidueTestVisitor(Class matchClass, boolean defaultMatches, int matchVarType) { + private boolean matches; + + private final int matchVarType; + + public InstanceOfResidueTestVisitor(Class matchClass, boolean defaultMatches, int matchVarType) { this.matchClass = matchClass; this.matches = defaultMatches; this.matchVarType = matchVarType; @@ -172,19 +180,34 @@ class RuntimeTestWalker { public boolean instanceOfMatches(Test test) { test.accept(this); - return matches; + return this.matches; } @Override public void visit(Instanceof i) { - ResolvedType type = (ResolvedType) i.getType(); int varType = getVarType((ReflectionVar) i.getVar()); if (varType != this.matchVarType) { return; } + Class typeClass = null; + ResolvedType type = (ResolvedType) i.getType(); + if (type instanceof ReferenceType) { + ReferenceTypeDelegate delegate = ((ReferenceType) type).getDelegate(); + if (delegate instanceof ReflectionBasedReferenceTypeDelegate) { + try { + ReflectionUtils.makeAccessible(myClassField); + typeClass = (Class) myClassField.get(delegate); + } + catch (IllegalAccessException ex) { + throw new IllegalStateException(ex); + } + } + } try { - Class typeClass = ClassUtils.forName(type.getName(), this.matchClass.getClassLoader()); - // Don't use ReflectionType.isAssignableFrom() as it won't be aware of (Spring) mixins + // Don't use ResolvedType.isAssignableFrom() as it won't be aware of (Spring) mixins + if (typeClass == null) { + typeClass = ClassUtils.forName(type.getName(), this.matchClass.getClassLoader()); + } this.matches = typeClass.isAssignableFrom(this.matchClass); } catch (ClassNotFoundException ex) { @@ -199,7 +222,7 @@ class RuntimeTestWalker { */ private static class TargetInstanceOfResidueTestVisitor extends InstanceOfResidueTestVisitor { - public TargetInstanceOfResidueTestVisitor(Class targetClass) { + public TargetInstanceOfResidueTestVisitor(Class targetClass) { super(targetClass, false, TARGET_VAR); } @@ -214,7 +237,7 @@ class RuntimeTestWalker { */ private static class ThisInstanceOfResidueTestVisitor extends InstanceOfResidueTestVisitor { - public ThisInstanceOfResidueTestVisitor(Class thisClass) { + public ThisInstanceOfResidueTestVisitor(Class thisClass) { super(thisClass, true, THIS_VAR); } @@ -228,8 +251,11 @@ class RuntimeTestWalker { private static class SubtypeSensitiveVarTypeTestVisitor extends TestVisitorAdapter { private final Object thisObj = new Object(); + private final Object targetObj = new Object(); + private final Object[] argsObjs = new Object[0]; + private boolean testsSubtypeSensitiveVars = false; public boolean testsSubtypeSensitiveVars(Test aTest) { @@ -240,8 +266,8 @@ class RuntimeTestWalker { @Override public void visit(Instanceof i) { ReflectionVar v = (ReflectionVar) i.getVar(); - Object varUnderTest = v.getBindingAtJoinPoint(thisObj,targetObj,argsObjs); - if ((varUnderTest == thisObj) || (varUnderTest == targetObj)) { + Object varUnderTest = v.getBindingAtJoinPoint(this.thisObj, this.targetObj, this.argsObjs); + if (varUnderTest == this.thisObj || varUnderTest == this.targetObj) { this.testsSubtypeSensitiveVars = true; } } @@ -251,7 +277,7 @@ class RuntimeTestWalker { // If you thought things were bad before, now we sink to new levels of horror... ReflectionVar v = (ReflectionVar) hasAnn.getVar(); int varType = getVarType(v); - if ((varType == AT_THIS_VAR) || (varType == AT_TARGET_VAR) || (varType == AT_ANNOTATION_VAR)) { + if (varType == AT_THIS_VAR || varType == AT_TARGET_VAR || varType == AT_ANNOTATION_VAR) { this.testsSubtypeSensitiveVars = true; } }