GitHub-Spring
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

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

AspectJExpressionPointcut leniently ignores unsupported expression.

Closes gh-32793
pull/33211/head
Juergen Hoeller 2 years ago
parent
ca9eb57c94
commit
0c65a5e04a
3 changed files with 52 additions and 80 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Unified View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 25
      spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java
  2. 34
      spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/AbstractAspectJAdvisorFactory.java
  3. 73
      spring-aop/src/test/java/org/springframework/aop/aspectj/AspectJExpressionPointcutTests.java

25
spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java
Unescape View File

@ -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.
@ -174,25 +174,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);
@ -269,10 +274,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);
@ -283,6 +287,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);
} }
@ -291,7 +298,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
@ -329,7 +335,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,

34
spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/AbstractAspectJAdvisorFactory.java
Unescape View File

@ -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"); * 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.lang.reflect.Modifier; import java.lang.reflect.Modifier;
import java.util.HashMap; import java.util.HashMap;
@ -57,8 +56,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};
@ -69,37 +66,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 {
// If the parent has the annotation and isn't abstract it's an error // 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 * 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...).
@ -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. * pointcut String.
* @param <A> the annotation type * @param <A> the annotation type
*/ */

73
spring-aop/src/test/java/org/springframework/aop/aspectj/AspectJExpressionPointcutTests.java
Unescape View File

@ -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"); * 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.
@ -23,9 +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.PointcutExpression;
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;
@ -42,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;
@ -65,7 +61,7 @@ public class AspectJExpressionPointcutTests {
@BeforeEach @BeforeEach
public void setUp() throws NoSuchMethodException { public void setup() throws NoSuchMethodException {
getAge = TestBean.class.getMethod("getAge"); getAge = TestBean.class.getMethod("getAge");
setAge = TestBean.class.getMethod("setAge", int.class); setAge = TestBean.class.getMethod("setAge", int.class);
setSomeNumber = TestBean.class.getMethod("setSomeNumber", Number.class); setSomeNumber = TestBean.class.getMethod("setSomeNumber", Number.class);
@ -175,25 +171,25 @@ 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");
} }
@ -210,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();
} }
@ -246,14 +244,13 @@ 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( assertThatIllegalArgumentException().isThrownBy(() -> getPointcut(expression).getClassFilter().matches(Object.class));
getPointcut(expression)::getClassFilter); // call to getClassFilter forces resolution
} }
private TestBean getAdvisedProxy(String pointcutExpression, CallCountingInterceptor interceptor) { private TestBean getAdvisedProxy(String pointcutExpression, CallCountingInterceptor interceptor) {
TestBean target = new TestBean(); TestBean target = new TestBean();
Pointcut pointcut = getPointcut(pointcutExpression); AspectJExpressionPointcut pointcut = getPointcut(pointcutExpression);
DefaultPointcutAdvisor advisor = new DefaultPointcutAdvisor(); DefaultPointcutAdvisor advisor = new DefaultPointcutAdvisor();
advisor.setAdvice(interceptor); advisor.setAdvice(interceptor);
@ -277,40 +274,29 @@ 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).isThrownBy(() -> assertThat(getPointcut(expression).getClassFilter().matches(Object.class)).isFalse();
getPointcut(expression).getClassFilter()) // call to getClassFilter forces resolution...
.satisfies(ex -> assertThat(ex.getUnsupportedPrimitive()).isEqualTo(PointcutPrimitive.CALL));
} }
@Test @Test
public void testAndSubstitution() { public void testAndSubstitution() {
Pointcut pc = getPointcut("execution(* *(..)) and args(String)"); AspectJExpressionPointcut pc = getPointcut("execution(* *(..)) and args(String)");
PointcutExpression expr = ((AspectJExpressionPointcut) pc).getPointcutExpression(); String expr = pc.getPointcutExpression().getPointcutExpression();
assertThat(expr.getPointcutExpression()).isEqualTo("execution(* *(..)) && args(String)"); assertThat(expr).isEqualTo("execution(* *(..)) && args(String)");
} }
@Test @Test
public void testMultipleAndSubstitutions() { public void testMultipleAndSubstitutions() {
Pointcut pc = getPointcut("execution(* *(..)) and args(String) and this(Object)"); AspectJExpressionPointcut pc = getPointcut("execution(* *(..)) and args(String) and this(Object)");
PointcutExpression expr = ((AspectJExpressionPointcut) pc).getPointcutExpression(); String expr = pc.getPointcutExpression().getPointcutExpression();
assertThat(expr.getPointcutExpression()).isEqualTo("execution(* *(..)) && args(String) && this(Object)"); assertThat(expr).isEqualTo("execution(* *(..)) && args(String) && this(Object)");
} }
private Pointcut getPointcut(String expression) { private AspectJExpressionPointcut getPointcut(String expression) {
AspectJExpressionPointcut pointcut = new AspectJExpressionPointcut(); AspectJExpressionPointcut pointcut = new AspectJExpressionPointcut();
pointcut.setExpression(expression); pointcut.setExpression(expression);
return pointcut; return pointcut;
} }
public static class OtherIOther implements IOther {
@Override
public void absquatulate() {
// Empty
}
}
@Test @Test
public void testMatchGenericArgument() { public void testMatchGenericArgument() {
String expression = "execution(* set*(java.util.List<org.springframework.beans.testfixture.beans.TestBean>) )"; String expression = "execution(* set*(java.util.List<org.springframework.beans.testfixture.beans.TestBean>) )";
@ -505,6 +491,15 @@ public class AspectJExpressionPointcutTests {
} }
public static class OtherIOther implements IOther {
@Override
public void absquatulate() {
// Empty
}
}
public static class HasGeneric { public static class HasGeneric {
public void setFriends(List<TestBean> friends) { public void setFriends(List<TestBean> friends) {

Write Preview
Loading…
Cancel
Save