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 8cb21661cb..52b652001c 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; @@ -144,14 +146,14 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut /** * 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; } @@ -191,9 +193,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); } @@ -205,8 +207,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()), @@ -252,16 +253,15 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut 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; } } @@ -288,7 +288,15 @@ 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)); } } @@ -344,14 +352,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(); } @@ -361,29 +368,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); } @@ -417,7 +409,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); } @@ -425,21 +418,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)); } @@ -620,9 +614,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) { @@ -632,35 +628,34 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut @Override public boolean alwaysMatches() { - return primary.alwaysMatches(); + return this.primary.alwaysMatches(); } @Override public boolean maybeMatches() { - return primary.maybeMatches(); + return this.primary.maybeMatches(); } @Override public boolean neverMatches() { - return primary.neverMatches(); + return this.primary.neverMatches(); } @Override - 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); } } @Override 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 e8e38e0179..237ebe245a 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 final Test runtimeTest; + private static final Field residualTestField; + private static final Field varTypeField; - public RuntimeTestWalker(ShadowMatch shadowMatch) { - ShadowMatchImpl shadowMatchImplementation = (ShadowMatchImpl) shadowMatch; + private static final Field myClassField; + + + static { try { - Field testField = shadowMatchImplementation.getClass().getDeclaredField("residualTest"); - ReflectionUtils.makeAccessible(testField); - this.runtimeTest = (Test) testField.get(shadowMatch); + residualTestField = ShadowMatchImpl.class.getDeclaredField("residualTest"); + varTypeField = ReflectionVar.class.getDeclaredField("varType"); + myClassField = ReflectionBasedReferenceTypeDelegate.class.getDeclaredField("myClass"); } - catch (NoSuchFieldException noSuchFieldEx) { + catch (NoSuchFieldException ex) { 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."); + "on the classpath is incompatible with this version of Spring: " + ex); + } + } + + + private final Test runtimeTest; + + + public RuntimeTestWalker(ShadowMatch shadowMatch) { + try { + ReflectionUtils.makeAccessible(residualTestField); + this.runtimeTest = (Test) residualTestField.get(shadowMatch); } - 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); } } @@ -149,19 +163,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); } } } @@ -169,9 +175,11 @@ class RuntimeTestWalker { private static abstract class InstanceOfResidueTestVisitor extends TestVisitorAdapter { - private Class matchClass; + private final Class matchClass; + private boolean matches; - private int matchVarType; + + private final int matchVarType; public InstanceOfResidueTestVisitor(Class matchClass, boolean defaultMatches, int matchVarType) { this.matchClass = matchClass; @@ -181,19 +189,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) { @@ -237,8 +260,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) { @@ -249,8 +275,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; } } @@ -260,7 +286,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; } }