Browse Source

Accept ajc-compiled @Aspect classes for Spring AOP proxy usage

AspectJExpressionPointcut leniently ignores unsupported expression.

Closes gh-32793
pull/33048/head
Juergen Hoeller 2 years ago
parent
commit
e81c788274
  1. 23
      spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java
  2. 32
      spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/AbstractAspectJAdvisorFactory.java
  3. 27
      spring-aop/src/test/java/org/springframework/aop/aspectj/AspectJExpressionPointcutTests.java

23
spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java

@ -169,25 +169,30 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
@Override @Override
public ClassFilter getClassFilter() { public ClassFilter getClassFilter() {
obtainPointcutExpression(); checkExpression();
return this; return this;
} }
@Override @Override
public MethodMatcher getMethodMatcher() { public MethodMatcher getMethodMatcher() {
obtainPointcutExpression(); checkExpression();
return this; return this;
} }
/** /**
* Check whether this pointcut is ready to match, * Check whether this pointcut is ready to match.
* lazily building the underlying AspectJ pointcut expression.
*/ */
private PointcutExpression obtainPointcutExpression() { private void checkExpression() {
if (getExpression() == null) { if (getExpression() == null) {
throw new IllegalStateException("Must set property 'expression' before attempting to match"); 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) { if (this.pointcutExpression == null) {
this.pointcutClassLoader = determinePointcutClassLoader(); this.pointcutClassLoader = determinePointcutClassLoader();
this.pointcutExpression = buildPointcutExpression(this.pointcutClassLoader); this.pointcutExpression = buildPointcutExpression(this.pointcutClassLoader);
@ -264,10 +269,9 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
@Override @Override
public boolean matches(Class<?> targetClass) { public boolean matches(Class<?> targetClass) {
PointcutExpression pointcutExpression = obtainPointcutExpression();
try { try {
try { try {
return pointcutExpression.couldMatchJoinPointsInType(targetClass); return obtainPointcutExpression().couldMatchJoinPointsInType(targetClass);
} }
catch (ReflectionWorldException ex) { catch (ReflectionWorldException ex) {
logger.debug("PointcutExpression matching rejected target class - trying fallback expression", ex); logger.debug("PointcutExpression matching rejected target class - trying fallback expression", ex);
@ -278,6 +282,9 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
} }
} }
} }
catch (IllegalArgumentException | IllegalStateException ex) {
throw ex;
}
catch (Throwable ex) { catch (Throwable ex) {
logger.debug("PointcutExpression matching rejected target class", ex); logger.debug("PointcutExpression matching rejected target class", ex);
} }
@ -286,7 +293,6 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
@Override @Override
public boolean matches(Method method, Class<?> targetClass, boolean hasIntroductions) { public boolean matches(Method method, Class<?> targetClass, boolean hasIntroductions) {
obtainPointcutExpression();
ShadowMatch shadowMatch = getTargetShadowMatch(method, targetClass); ShadowMatch shadowMatch = getTargetShadowMatch(method, targetClass);
// Special handling for this, target, @this, @target, @annotation // Special handling for this, target, @this, @target, @annotation
@ -324,7 +330,6 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
@Override @Override
public boolean matches(Method method, Class<?> targetClass, Object... args) { public boolean matches(Method method, Class<?> targetClass, Object... args) {
obtainPointcutExpression();
ShadowMatch shadowMatch = getTargetShadowMatch(method, targetClass); ShadowMatch shadowMatch = getTargetShadowMatch(method, targetClass);
// Bind Spring AOP proxy to AspectJ "this" and Spring AOP target to AspectJ target, // Bind Spring AOP proxy to AspectJ "this" and Spring AOP target to AspectJ target,

32
spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/AbstractAspectJAdvisorFactory.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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with 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.annotation.Annotation;
import java.lang.reflect.Constructor; import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.Method; import java.lang.reflect.Method;
import java.util.Map; import java.util.Map;
import java.util.StringTokenizer; import java.util.StringTokenizer;
@ -56,8 +55,6 @@ import org.springframework.lang.Nullable;
*/ */
public abstract class AbstractAspectJAdvisorFactory implements AspectJAdvisorFactory { public abstract class AbstractAspectJAdvisorFactory implements AspectJAdvisorFactory {
private static final String AJC_MAGIC = "ajc$";
private static final Class<?>[] ASPECTJ_ANNOTATION_CLASSES = new Class<?>[] { private static final Class<?>[] ASPECTJ_ANNOTATION_CLASSES = new Class<?>[] {
Pointcut.class, Around.class, Before.class, After.class, AfterReturning.class, AfterThrowing.class}; Pointcut.class, Around.class, Before.class, After.class, AfterReturning.class, AfterThrowing.class};
@ -68,37 +65,11 @@ public abstract class AbstractAspectJAdvisorFactory implements AspectJAdvisorFac
protected final ParameterNameDiscoverer parameterNameDiscoverer = new AspectJAnnotationParameterNameDiscoverer(); 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 @Override
public boolean isAspect(Class<?> clazz) { public boolean isAspect(Class<?> clazz) {
return (hasAspectAnnotation(clazz) && !compiledByAjc(clazz));
}
private boolean hasAspectAnnotation(Class<?> clazz) {
return (AnnotationUtils.findAnnotation(clazz, Aspect.class) != null); 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 @Override
public void validate(Class<?> aspectClass) throws AopConfigException { public void validate(Class<?> aspectClass) throws AopConfigException {
AjType<?> ajType = AjTypeSystem.getAjType(aspectClass); AjType<?> ajType = AjTypeSystem.getAjType(aspectClass);
@ -115,6 +86,7 @@ public abstract class AbstractAspectJAdvisorFactory implements AspectJAdvisorFac
} }
} }
/** /**
* Find and return the first AspectJ annotation on the given method * Find and return the first AspectJ annotation on the given method
* (there <i>should</i> only be one anyway...). * (there <i>should</i> only be one anyway...).

27
spring-aop/src/test/java/org/springframework/aop/aspectj/AspectJExpressionPointcutTests.java

@ -23,8 +23,6 @@ import java.util.Map;
import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInterceptor;
import org.aopalliance.intercept.MethodInvocation; import org.aopalliance.intercept.MethodInvocation;
import org.aspectj.weaver.tools.PointcutPrimitive;
import org.aspectj.weaver.tools.UnsupportedPointcutPrimitiveException;
import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import test.annotation.EmptySpringAnnotation; import test.annotation.EmptySpringAnnotation;
@ -41,7 +39,6 @@ import org.springframework.beans.testfixture.beans.TestBean;
import org.springframework.beans.testfixture.beans.subpkg.DeepBean; import org.springframework.beans.testfixture.beans.subpkg.DeepBean;
import static org.assertj.core.api.Assertions.assertThat; 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.assertThatIllegalArgumentException;
import static org.assertj.core.api.Assertions.assertThatIllegalStateException; import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
@ -174,24 +171,24 @@ public class AspectJExpressionPointcutTests {
@Test @Test
public void testFriendlyErrorOnNoLocationClassMatching() { public void testFriendlyErrorOnNoLocationClassMatching() {
AspectJExpressionPointcut pc = new AspectJExpressionPointcut(); AspectJExpressionPointcut pc = new AspectJExpressionPointcut();
assertThatIllegalStateException().isThrownBy(() -> assertThatIllegalStateException()
pc.matches(ITestBean.class)) .isThrownBy(() -> pc.getClassFilter().matches(ITestBean.class))
.withMessageContaining("expression"); .withMessageContaining("expression");
} }
@Test @Test
public void testFriendlyErrorOnNoLocation2ArgMatching() { public void testFriendlyErrorOnNoLocation2ArgMatching() {
AspectJExpressionPointcut pc = new AspectJExpressionPointcut(); AspectJExpressionPointcut pc = new AspectJExpressionPointcut();
assertThatIllegalStateException().isThrownBy(() -> assertThatIllegalStateException()
pc.matches(getAge, ITestBean.class)) .isThrownBy(() -> pc.getMethodMatcher().matches(getAge, ITestBean.class))
.withMessageContaining("expression"); .withMessageContaining("expression");
} }
@Test @Test
public void testFriendlyErrorOnNoLocation3ArgMatching() { public void testFriendlyErrorOnNoLocation3ArgMatching() {
AspectJExpressionPointcut pc = new AspectJExpressionPointcut(); AspectJExpressionPointcut pc = new AspectJExpressionPointcut();
assertThatIllegalStateException().isThrownBy(() -> assertThatIllegalStateException()
pc.matches(getAge, ITestBean.class, (Object[]) null)) .isThrownBy(() -> pc.getMethodMatcher().matches(getAge, ITestBean.class, (Object[]) null))
.withMessageContaining("expression"); .withMessageContaining("expression");
} }
@ -209,8 +206,10 @@ public class AspectJExpressionPointcutTests {
// not currently testable in a reliable fashion // not currently testable in a reliable fashion
//assertDoesNotMatchStringClass(classFilter); //assertDoesNotMatchStringClass(classFilter);
assertThat(methodMatcher.matches(setSomeNumber, TestBean.class, 12D)).as("Should match with setSomeNumber with Double input").isTrue(); assertThat(methodMatcher.matches(setSomeNumber, TestBean.class, 12D))
assertThat(methodMatcher.matches(setSomeNumber, TestBean.class, 11)).as("Should not match setSomeNumber with Integer input").isFalse(); .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.matches(getAge, TestBean.class)).as("Should not match getAge").isFalse();
assertThat(methodMatcher.isRuntime()).as("Should be a runtime match").isTrue(); assertThat(methodMatcher.isRuntime()).as("Should be a runtime match").isTrue();
} }
@ -245,7 +244,7 @@ public class AspectJExpressionPointcutTests {
@Test @Test
public void testInvalidExpression() { public void testInvalidExpression() {
String expression = "execution(void org.springframework.beans.testfixture.beans.TestBean.setSomeNumber(Number) && args(Double)"; 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) { private TestBean getAdvisedProxy(String pointcutExpression, CallCountingInterceptor interceptor) {
@ -275,9 +274,7 @@ public class AspectJExpressionPointcutTests {
@Test @Test
public void testWithUnsupportedPointcutPrimitive() { public void testWithUnsupportedPointcutPrimitive() {
String expression = "call(int org.springframework.beans.testfixture.beans.TestBean.getAge())"; String expression = "call(int org.springframework.beans.testfixture.beans.TestBean.getAge())";
assertThatExceptionOfType(UnsupportedPointcutPrimitiveException.class) assertThat(getPointcut(expression).getClassFilter().matches(Object.class)).isFalse();
.isThrownBy(() -> getPointcut(expression).getClassFilter()) // call to getClassFilter forces resolution...
.satisfies(ex -> assertThat(ex.getUnsupportedPrimitive()).isEqualTo(PointcutPrimitive.CALL));
} }
@Test @Test

Loading…
Cancel
Save