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 5c79bb5aa70..c4a3aa4f508 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-2023 the original author or authors. + * Copyright 2002-2024 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. @@ -174,25 +174,30 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut @Override public ClassFilter getClassFilter() { - obtainPointcutExpression(); + checkExpression(); return this; } @Override public MethodMatcher getMethodMatcher() { - obtainPointcutExpression(); + checkExpression(); return this; } /** - * Check whether this pointcut is ready to match, - * lazily building the underlying AspectJ pointcut expression. + * Check whether this pointcut is ready to match. */ - private PointcutExpression obtainPointcutExpression() { + private void checkExpression() { if (getExpression() == null) { throw new IllegalStateException("Must set property 'expression' before attempting to match"); } + } + + /** + * Lazily build the underlying AspectJ pointcut expression. + */ + private PointcutExpression obtainPointcutExpression() { if (this.pointcutExpression == null) { this.pointcutClassLoader = determinePointcutClassLoader(); this.pointcutExpression = buildPointcutExpression(this.pointcutClassLoader); @@ -269,10 +274,9 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut @Override public boolean matches(Class targetClass) { - PointcutExpression pointcutExpression = obtainPointcutExpression(); try { try { - return pointcutExpression.couldMatchJoinPointsInType(targetClass); + return obtainPointcutExpression().couldMatchJoinPointsInType(targetClass); } catch (ReflectionWorldException ex) { logger.debug("PointcutExpression matching rejected target class - trying fallback expression", ex); @@ -283,6 +287,9 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut } } } + catch (IllegalArgumentException | IllegalStateException ex) { + throw ex; + } catch (Throwable ex) { logger.debug("PointcutExpression matching rejected target class", ex); } @@ -291,7 +298,6 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut @Override public boolean matches(Method method, Class targetClass, boolean hasIntroductions) { - obtainPointcutExpression(); ShadowMatch shadowMatch = getTargetShadowMatch(method, targetClass); // Special handling for this, target, @this, @target, @annotation @@ -329,7 +335,6 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut @Override public boolean matches(Method method, Class targetClass, Object... args) { - obtainPointcutExpression(); ShadowMatch shadowMatch = getTargetShadowMatch(method, targetClass); // Bind Spring AOP proxy to AspectJ "this" and Spring AOP target to AspectJ target, diff --git a/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/AbstractAspectJAdvisorFactory.java b/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/AbstractAspectJAdvisorFactory.java index 58f3c23b459..efdd136628b 100644 --- a/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/AbstractAspectJAdvisorFactory.java +++ b/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/AbstractAspectJAdvisorFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2024 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,7 +18,6 @@ package org.springframework.aop.aspectj.annotation; import java.lang.annotation.Annotation; import java.lang.reflect.Constructor; -import java.lang.reflect.Field; import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.util.HashMap; @@ -57,8 +56,6 @@ import org.springframework.lang.Nullable; */ public abstract class AbstractAspectJAdvisorFactory implements AspectJAdvisorFactory { - private static final String AJC_MAGIC = "ajc$"; - private static final Class[] ASPECTJ_ANNOTATION_CLASSES = new Class[] { Pointcut.class, Around.class, Before.class, After.class, AfterReturning.class, AfterThrowing.class}; @@ -69,37 +66,11 @@ public abstract class AbstractAspectJAdvisorFactory implements AspectJAdvisorFac protected final ParameterNameDiscoverer parameterNameDiscoverer = new AspectJAnnotationParameterNameDiscoverer(); - /** - * We consider something to be an AspectJ aspect suitable for use by the Spring AOP system - * if it has the @Aspect annotation, and was not compiled by ajc. The reason for this latter test - * is that aspects written in the code-style (AspectJ language) also have the annotation present - * when compiled by ajc with the -1.5 flag, yet they cannot be consumed by Spring AOP. - */ @Override public boolean isAspect(Class clazz) { - return (hasAspectAnnotation(clazz) && !compiledByAjc(clazz)); - } - - private boolean hasAspectAnnotation(Class clazz) { return (AnnotationUtils.findAnnotation(clazz, Aspect.class) != null); } - /** - * We need to detect this as "code-style" AspectJ aspects should not be - * interpreted by Spring AOP. - */ - private boolean compiledByAjc(Class clazz) { - // The AJTypeSystem goes to great lengths to provide a uniform appearance between code-style and - // annotation-style aspects. Therefore there is no 'clean' way to tell them apart. Here we rely on - // an implementation detail of the AspectJ compiler. - for (Field field : clazz.getDeclaredFields()) { - if (field.getName().startsWith(AJC_MAGIC)) { - return true; - } - } - return false; - } - @Override public void validate(Class aspectClass) throws AopConfigException { // If the parent has the annotation and isn't abstract it's an error @@ -124,6 +95,7 @@ public abstract class AbstractAspectJAdvisorFactory implements AspectJAdvisorFac } } + /** * Find and return the first AspectJ annotation on the given method * (there should only be one anyway...). @@ -163,7 +135,7 @@ public abstract class AbstractAspectJAdvisorFactory implements AspectJAdvisorFac /** - * Class modelling an AspectJ annotation, exposing its type enumeration and + * Class modeling an AspectJ annotation, exposing its type enumeration and * pointcut String. * @param the annotation type */ diff --git a/spring-aop/src/test/java/org/springframework/aop/aspectj/AspectJExpressionPointcutTests.java b/spring-aop/src/test/java/org/springframework/aop/aspectj/AspectJExpressionPointcutTests.java index e1b2a93b958..4602a1e8a9f 100644 --- a/spring-aop/src/test/java/org/springframework/aop/aspectj/AspectJExpressionPointcutTests.java +++ b/spring-aop/src/test/java/org/springframework/aop/aspectj/AspectJExpressionPointcutTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2024 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. @@ -23,9 +23,6 @@ import java.util.Map; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; -import org.aspectj.weaver.tools.PointcutExpression; -import org.aspectj.weaver.tools.PointcutPrimitive; -import org.aspectj.weaver.tools.UnsupportedPointcutPrimitiveException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import test.annotation.EmptySpringAnnotation; @@ -42,7 +39,6 @@ import org.springframework.beans.testfixture.beans.TestBean; import org.springframework.beans.testfixture.beans.subpkg.DeepBean; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; @@ -65,7 +61,7 @@ public class AspectJExpressionPointcutTests { @BeforeEach - public void setUp() throws NoSuchMethodException { + public void setup() throws NoSuchMethodException { getAge = TestBean.class.getMethod("getAge"); setAge = TestBean.class.getMethod("setAge", int.class); setSomeNumber = TestBean.class.getMethod("setSomeNumber", Number.class); @@ -175,25 +171,25 @@ public class AspectJExpressionPointcutTests { @Test public void testFriendlyErrorOnNoLocationClassMatching() { AspectJExpressionPointcut pc = new AspectJExpressionPointcut(); - assertThatIllegalStateException().isThrownBy(() -> - pc.matches(ITestBean.class)) - .withMessageContaining("expression"); + assertThatIllegalStateException() + .isThrownBy(() -> pc.getClassFilter().matches(ITestBean.class)) + .withMessageContaining("expression"); } @Test public void testFriendlyErrorOnNoLocation2ArgMatching() { AspectJExpressionPointcut pc = new AspectJExpressionPointcut(); - assertThatIllegalStateException().isThrownBy(() -> - pc.matches(getAge, ITestBean.class)) - .withMessageContaining("expression"); + assertThatIllegalStateException() + .isThrownBy(() -> pc.getMethodMatcher().matches(getAge, ITestBean.class)) + .withMessageContaining("expression"); } @Test public void testFriendlyErrorOnNoLocation3ArgMatching() { AspectJExpressionPointcut pc = new AspectJExpressionPointcut(); - assertThatIllegalStateException().isThrownBy(() -> - pc.matches(getAge, ITestBean.class, (Object[]) null)) - .withMessageContaining("expression"); + assertThatIllegalStateException() + .isThrownBy(() -> pc.getMethodMatcher().matches(getAge, ITestBean.class, (Object[]) null)) + .withMessageContaining("expression"); } @@ -210,8 +206,10 @@ public class AspectJExpressionPointcutTests { // not currently testable in a reliable fashion //assertDoesNotMatchStringClass(classFilter); - assertThat(methodMatcher.matches(setSomeNumber, TestBean.class, 12D)).as("Should match with setSomeNumber with Double input").isTrue(); - assertThat(methodMatcher.matches(setSomeNumber, TestBean.class, 11)).as("Should not match setSomeNumber with Integer input").isFalse(); + assertThat(methodMatcher.matches(setSomeNumber, TestBean.class, 12D)) + .as("Should match with setSomeNumber with Double input").isTrue(); + assertThat(methodMatcher.matches(setSomeNumber, TestBean.class, 11)) + .as("Should not match setSomeNumber with Integer input").isFalse(); assertThat(methodMatcher.matches(getAge, TestBean.class)).as("Should not match getAge").isFalse(); assertThat(methodMatcher.isRuntime()).as("Should be a runtime match").isTrue(); } @@ -246,14 +244,13 @@ public class AspectJExpressionPointcutTests { @Test public void testInvalidExpression() { String expression = "execution(void org.springframework.beans.testfixture.beans.TestBean.setSomeNumber(Number) && args(Double)"; - assertThatIllegalArgumentException().isThrownBy( - getPointcut(expression)::getClassFilter); // call to getClassFilter forces resolution + assertThatIllegalArgumentException().isThrownBy(() -> getPointcut(expression).getClassFilter().matches(Object.class)); } private TestBean getAdvisedProxy(String pointcutExpression, CallCountingInterceptor interceptor) { TestBean target = new TestBean(); - Pointcut pointcut = getPointcut(pointcutExpression); + AspectJExpressionPointcut pointcut = getPointcut(pointcutExpression); DefaultPointcutAdvisor advisor = new DefaultPointcutAdvisor(); advisor.setAdvice(interceptor); @@ -277,40 +274,29 @@ public class AspectJExpressionPointcutTests { @Test public void testWithUnsupportedPointcutPrimitive() { String expression = "call(int org.springframework.beans.testfixture.beans.TestBean.getAge())"; - assertThatExceptionOfType(UnsupportedPointcutPrimitiveException.class).isThrownBy(() -> - getPointcut(expression).getClassFilter()) // call to getClassFilter forces resolution... - .satisfies(ex -> assertThat(ex.getUnsupportedPrimitive()).isEqualTo(PointcutPrimitive.CALL)); + assertThat(getPointcut(expression).getClassFilter().matches(Object.class)).isFalse(); } @Test public void testAndSubstitution() { - Pointcut pc = getPointcut("execution(* *(..)) and args(String)"); - PointcutExpression expr = ((AspectJExpressionPointcut) pc).getPointcutExpression(); - assertThat(expr.getPointcutExpression()).isEqualTo("execution(* *(..)) && args(String)"); + AspectJExpressionPointcut pc = getPointcut("execution(* *(..)) and args(String)"); + String expr = pc.getPointcutExpression().getPointcutExpression(); + assertThat(expr).isEqualTo("execution(* *(..)) && args(String)"); } @Test public void testMultipleAndSubstitutions() { - Pointcut pc = getPointcut("execution(* *(..)) and args(String) and this(Object)"); - PointcutExpression expr = ((AspectJExpressionPointcut) pc).getPointcutExpression(); - assertThat(expr.getPointcutExpression()).isEqualTo("execution(* *(..)) && args(String) && this(Object)"); + AspectJExpressionPointcut pc = getPointcut("execution(* *(..)) and args(String) and this(Object)"); + String expr = pc.getPointcutExpression().getPointcutExpression(); + assertThat(expr).isEqualTo("execution(* *(..)) && args(String) && this(Object)"); } - private Pointcut getPointcut(String expression) { + private AspectJExpressionPointcut getPointcut(String expression) { AspectJExpressionPointcut pointcut = new AspectJExpressionPointcut(); pointcut.setExpression(expression); return pointcut; } - - public static class OtherIOther implements IOther { - - @Override - public void absquatulate() { - // Empty - } - } - @Test public void testMatchGenericArgument() { String expression = "execution(* set*(java.util.List) )"; @@ -505,6 +491,15 @@ public class AspectJExpressionPointcutTests { } + public static class OtherIOther implements IOther { + + @Override + public void absquatulate() { + // Empty + } + } + + public static class HasGeneric { public void setFriends(List friends) {