From d3fba6d49b0ec21361e9b8a95842d3b5548fea59 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Wed, 25 Oct 2023 17:40:58 +0200 Subject: [PATCH] Support pattern matching for method names in ControlFlowPointcut Prior to this commit, ControlFlowPointcut supported a single method name which was matched exactly. Although it was possible to extend ControlFlowPointcut to add support for pattern matching, it was a bit cumbersome. To address that, this commit introduces built-in pattern matching support for method names in ControlFlowPointcut, analogous to the pattern matching support in NameMatchMethodPointcut. Specifically, a user can provide one or more method name patterns, and the patterns will be matched against candidate method names using OR semantics. By default, the matching algorithm delegates to PatternMatchUtils.simpleMatch(), but this can be overridden in subclasses by overriding the new protected isMatch() method. Closes gh-31435 --- .../aop/support/ControlFlowPointcut.java | 102 +++++++++--- .../aop/support/ControlFlowPointcutTests.java | 147 ++++++++++++++---- 2 files changed, 195 insertions(+), 54 deletions(-) diff --git a/spring-aop/src/main/java/org/springframework/aop/support/ControlFlowPointcut.java b/spring-aop/src/main/java/org/springframework/aop/support/ControlFlowPointcut.java index cb2b368fd264..7f56a9c81f0e 100644 --- a/spring-aop/src/main/java/org/springframework/aop/support/ControlFlowPointcut.java +++ b/spring-aop/src/main/java/org/springframework/aop/support/ControlFlowPointcut.java @@ -18,6 +18,9 @@ import java.io.Serializable; import java.lang.reflect.Method; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; import java.util.concurrent.atomic.AtomicInteger; import org.springframework.aop.ClassFilter; @@ -25,11 +28,15 @@ import org.springframework.aop.Pointcut; import org.springframework.lang.Nullable; import org.springframework.util.Assert; -import org.springframework.util.ObjectUtils; +import org.springframework.util.PatternMatchUtils; /** * Pointcut and method matcher for use as a simple cflow-style pointcut. * + *

Each configured method name pattern can be an exact method name or a + * pattern (see {@link #isMatch(String, String)} for details on the supported + * pattern styles). + * *

Note that evaluating such pointcuts is 10-15 times slower than evaluating * normal pointcuts, but they are useful in some cases. * @@ -37,6 +44,9 @@ * @author Rob Harrop * @author Juergen Hoeller * @author Sam Brannen + * @see #isMatch + * @see NameMatchMethodPointcut + * @see JdkRegexpMethodPointcut */ @SuppressWarnings("serial") public class ControlFlowPointcut implements Pointcut, ClassFilter, MethodMatcher, Serializable { @@ -48,11 +58,10 @@ public class ControlFlowPointcut implements Pointcut, ClassFilter, MethodMatcher protected final Class clazz; /** - * The method against which to match, potentially {@code null}. - *

Available for use in subclasses since 6.1. + * An immutable list of method name patterns against which to match. + * @since 6.1 */ - @Nullable - protected final String methodName; + protected final List methodNamePatterns; private final AtomicInteger evaluationCount = new AtomicInteger(); @@ -62,21 +71,52 @@ public class ControlFlowPointcut implements Pointcut, ClassFilter, MethodMatcher * @param clazz the class */ public ControlFlowPointcut(Class clazz) { - this(clazz, null); + this(clazz, (String) null); + } + + /** + * Construct a new pointcut that matches all calls below a method matching + * the given method name pattern in the given class. + *

If no method name pattern is given, the pointcut matches all control flows + * below the given class. + * @param clazz the class + * @param methodNamePattern the method name pattern (may be {@code null}) + */ + public ControlFlowPointcut(Class clazz, @Nullable String methodNamePattern) { + Assert.notNull(clazz, "Class must not be null"); + this.clazz = clazz; + this.methodNamePatterns = (methodNamePattern != null ? + Collections.singletonList(methodNamePattern) : Collections.emptyList()); + } + + /** + * Construct a new pointcut that matches all calls below a method matching + * one of the given method name patterns in the given class. + *

If no method name pattern is given, the pointcut matches all control flows + * below the given class. + * @param clazz the class + * @param methodNamePatterns the method name patterns (potentially empty) + * @since 6.1 + */ + public ControlFlowPointcut(Class clazz, String... methodNamePatterns) { + this(clazz, Arrays.asList(methodNamePatterns)); } /** - * Construct a new pointcut that matches all calls below the given method - * in the given class. - *

If no method name is given, the pointcut matches all control flows + * Construct a new pointcut that matches all calls below a method matching + * one of the given method name patterns in the given class. + *

If no method name pattern is given, the pointcut matches all control flows * below the given class. * @param clazz the class - * @param methodName the name of the method (may be {@code null}) + * @param methodNamePatterns the method name patterns (potentially empty) + * @since 6.1 */ - public ControlFlowPointcut(Class clazz, @Nullable String methodName) { + public ControlFlowPointcut(Class clazz, List methodNamePatterns) { Assert.notNull(clazz, "Class must not be null"); + Assert.notNull(methodNamePatterns, "List of method name patterns must not be null"); + Assert.noNullElements(methodNamePatterns, "List of method name patterns must not contain null elements"); this.clazz = clazz; - this.methodName = methodName; + this.methodNamePatterns = methodNamePatterns.stream().distinct().toList(); } @@ -108,9 +148,15 @@ public boolean matches(Method method, Class targetClass, Object... args) { incrementEvaluationCount(); for (StackTraceElement element : new Throwable().getStackTrace()) { - if (element.getClassName().equals(this.clazz.getName()) && - (this.methodName == null || element.getMethodName().equals(this.methodName))) { - return true; + if (element.getClassName().equals(this.clazz.getName())) { + if (this.methodNamePatterns.isEmpty()) { + return true; + } + for (String methodNamePattern : this.methodNamePatterns) { + if (isMatch(element.getMethodName(), methodNamePattern)) { + return true; + } + } } } return false; @@ -134,6 +180,23 @@ protected final void incrementEvaluationCount() { this.evaluationCount.incrementAndGet(); } + /** + * Determine if the given method name matches the method name pattern. + *

The default implementation checks for direct equality as well as + * {@code xxx*}, {@code *xxx}, {@code *xxx*}, and {@code xxx*yyy} matches. + *

Can be overridden in subclasses. + * @param methodName the method name to check + * @param methodNamePattern the method name pattern + * @return {@code true} if the method name matches the pattern + * @since 6.1 + * @see #matches(Method, Class, Object...) + * @see PatternMatchUtils#simpleMatch(String, String) + */ + protected boolean isMatch(String methodName, String methodNamePattern) { + return (methodName.equals(methodNamePattern) || + PatternMatchUtils.simpleMatch(methodNamePattern, methodName)); + } + @Override public ClassFilter getClassFilter() { @@ -149,22 +212,19 @@ public MethodMatcher getMethodMatcher() { @Override public boolean equals(@Nullable Object other) { return (this == other || (other instanceof ControlFlowPointcut that && - this.clazz.equals(that.clazz)) && - ObjectUtils.nullSafeEquals(this.methodName, that.methodName)); + this.clazz.equals(that.clazz)) && this.methodNamePatterns.equals(that.methodNamePatterns)); } @Override public int hashCode() { int code = this.clazz.hashCode(); - if (this.methodName != null) { - code = 37 * code + this.methodName.hashCode(); - } + code = 37 * code + this.methodNamePatterns.hashCode(); return code; } @Override public String toString() { - return getClass().getName() + ": class = " + this.clazz.getName() + "; methodName = " + this.methodName; + return getClass().getName() + ": class = " + this.clazz.getName() + "; methodNamePatterns = " + this.methodNamePatterns; } } diff --git a/spring-aop/src/test/java/org/springframework/aop/support/ControlFlowPointcutTests.java b/spring-aop/src/test/java/org/springframework/aop/support/ControlFlowPointcutTests.java index ab70cd7255aa..d9cb49c1a217 100644 --- a/spring-aop/src/test/java/org/springframework/aop/support/ControlFlowPointcutTests.java +++ b/spring-aop/src/test/java/org/springframework/aop/support/ControlFlowPointcutTests.java @@ -17,6 +17,7 @@ package org.springframework.aop.support; import java.lang.reflect.Method; +import java.util.List; import org.junit.jupiter.api.Test; @@ -39,36 +40,69 @@ class ControlFlowPointcutTests { @Test void matchesExactMethodName() { + MyComponent component = new MyComponent(); TestBean target = new TestBean("Jane", 27); - ControlFlowPointcut cflow = new ControlFlowPointcut(MyComponent.class, "getAge"); + ControlFlowPointcut cflow = pointcut("getAge"); NopInterceptor nop = new NopInterceptor(); ProxyFactory pf = new ProxyFactory(target); pf.addAdvisor(new DefaultPointcutAdvisor(cflow, nop)); ITestBean proxy = (ITestBean) pf.getProxy(); - // Not advised, not under MyComponent + // Will not be advised: not under MyComponent assertThat(proxy.getAge()).isEqualTo(target.getAge()); assertThat(nop.getCount()).isEqualTo(0); assertThat(cflow.getEvaluations()).isEqualTo(1); - // Will be advised - assertThat(new MyComponent().getAge(proxy)).isEqualTo(target.getAge()); + // Will be advised due to "getAge" pattern: the proxy is invoked under MyComponent#getAge + assertThat(component.getAge(proxy)).isEqualTo(target.getAge()); assertThat(nop.getCount()).isEqualTo(1); assertThat(cflow.getEvaluations()).isEqualTo(2); - // Won't be advised - assertThat(new MyComponent().nomatch(proxy)).isEqualTo(target.getAge()); + // Will not be advised: the proxy is invoked under MyComponent, but there is no match for "nomatch" + assertThat(component.nomatch(proxy)).isEqualTo(target.getAge()); assertThat(nop.getCount()).isEqualTo(1); assertThat(cflow.getEvaluations()).isEqualTo(3); } + @Test + void matchesMethodNamePatterns() { + MyComponent component = new MyComponent(); + TestBean target = new TestBean("Jane", 27); + ControlFlowPointcut cflow = pointcut("foo", "get*", "bar", "*se*", "baz"); + NopInterceptor nop = new NopInterceptor(); + ProxyFactory pf = new ProxyFactory(target); + pf.addAdvisor(new DefaultPointcutAdvisor(cflow, nop)); + ITestBean proxy = (ITestBean) pf.getProxy(); + + // Will not be advised: not under MyComponent + assertThat(proxy.getAge()).isEqualTo(target.getAge()); + assertThat(nop.getCount()).isEqualTo(0); + assertThat(cflow.getEvaluations()).isEqualTo(1); + + // Will be advised due to "get*" pattern: the proxy is invoked under MyComponent#getAge + assertThat(component.getAge(proxy)).isEqualTo(target.getAge()); + assertThat(nop.getCount()).isEqualTo(1); + assertThat(cflow.getEvaluations()).isEqualTo(2); + + // Will be advised due to "*se*" pattern: the proxy is invoked under MyComponent#set + component.set(proxy); + assertThat(proxy.getAge()).isEqualTo(5); + assertThat(nop.getCount()).isEqualTo(2); + assertThat(cflow.getEvaluations()).isEqualTo(4); + + // Will not be advised: the proxy is invoked under MyComponent, but there is no match for "nomatch" + assertThat(component.nomatch(proxy)).isEqualTo(target.getAge()); + assertThat(nop.getCount()).isEqualTo(2); + assertThat(cflow.getEvaluations()).isEqualTo(5); + } + @Test void controlFlowPointcutIsExtensible() { @SuppressWarnings("serial") class CustomControlFlowPointcut extends ControlFlowPointcut { - CustomControlFlowPointcut(Class clazz, String methodName) { - super(clazz, methodName); + CustomControlFlowPointcut(Class clazz, String... methodNamePatterns) { + super(clazz, methodNamePatterns); } @Override @@ -81,40 +115,41 @@ Class trackedClass() { return super.clazz; } - String trackedMethod() { - return super.methodName; + List trackedMethodNamePatterns() { + return super.methodNamePatterns; } } - CustomControlFlowPointcut cflow = new CustomControlFlowPointcut(MyComponent.class, "getAge"); + CustomControlFlowPointcut cflow = new CustomControlFlowPointcut(MyComponent.class, "set*", "getAge", "set*", "set*"); assertThat(cflow.trackedClass()).isEqualTo(MyComponent.class); - assertThat(cflow.trackedMethod()).isEqualTo("getAge"); + assertThat(cflow.trackedMethodNamePatterns()).containsExactly("set*", "getAge"); + MyComponent component = new MyComponent(); TestBean target = new TestBean("Jane", 27); NopInterceptor nop = new NopInterceptor(); ProxyFactory pf = new ProxyFactory(target); pf.addAdvisor(new DefaultPointcutAdvisor(cflow, nop)); ITestBean proxy = (ITestBean) pf.getProxy(); - // Not advised: the proxy is not invoked under MyComponent#getAge + // Will not be advised: the proxy is not invoked under MyComponent#getAge assertThat(proxy.getAge()).isEqualTo(target.getAge()); assertThat(nop.getCount()).isEqualTo(0); assertThat(cflow.getEvaluations()).isEqualTo(2); // intentional double increment // Will be advised: the proxy is invoked under MyComponent#getAge - assertThat(new MyComponent().getAge(proxy)).isEqualTo(target.getAge()); + assertThat(component.getAge(proxy)).isEqualTo(target.getAge()); assertThat(nop.getCount()).isEqualTo(1); assertThat(cflow.getEvaluations()).isEqualTo(4); // intentional double increment - // Won't be advised: the proxy is not invoked under MyComponent#getAge - assertThat(new MyComponent().nomatch(proxy)).isEqualTo(target.getAge()); + // Will not be advised: the proxy is invoked under MyComponent, but there is no match for "nomatch" + assertThat(component.nomatch(proxy)).isEqualTo(target.getAge()); assertThat(nop.getCount()).isEqualTo(1); assertThat(cflow.getEvaluations()).isEqualTo(6); // intentional double increment } /** - * Check that we can use a cflow pointcut only in conjunction with + * Check that we can use a cflow pointcut in conjunction with * a static pointcut: e.g. all setter methods that are invoked under * a particular class. This greatly reduces the number of calls * to the cflow pointcut, meaning that it's not so prohibitively @@ -122,24 +157,28 @@ String trackedMethod() { */ @Test void selectiveApplication() { + MyComponent component = new MyComponent(); TestBean target = new TestBean("Jane", 27); - ControlFlowPointcut cflow = new ControlFlowPointcut(MyComponent.class); + ControlFlowPointcut cflow = pointcut(); NopInterceptor nop = new NopInterceptor(); Pointcut settersUnderMyComponent = Pointcuts.intersection(Pointcuts.SETTERS, cflow); ProxyFactory pf = new ProxyFactory(target); pf.addAdvisor(new DefaultPointcutAdvisor(settersUnderMyComponent, nop)); ITestBean proxy = (ITestBean) pf.getProxy(); - // Not advised, not under MyComponent + // Will not be advised: not under MyComponent target.setAge(16); assertThat(nop.getCount()).isEqualTo(0); + assertThat(cflow.getEvaluations()).isEqualTo(0); - // Not advised; under MyComponent but not a setter - assertThat(new MyComponent().getAge(proxy)).isEqualTo(16); + // Will not be advised: under MyComponent but not a setter + assertThat(component.getAge(proxy)).isEqualTo(16); assertThat(nop.getCount()).isEqualTo(0); + assertThat(cflow.getEvaluations()).isEqualTo(0); - // Won't be advised - new MyComponent().set(proxy); + // Will be advised due to Pointcuts.SETTERS: the proxy is invoked under MyComponent#set + component.set(proxy); + assertThat(proxy.getAge()).isEqualTo(5); assertThat(nop.getCount()).isEqualTo(1); // We saved most evaluations @@ -148,21 +187,63 @@ void selectiveApplication() { @Test void equalsAndHashCode() { - assertThat(new ControlFlowPointcut(MyComponent.class)).isEqualTo(new ControlFlowPointcut(MyComponent.class)); - assertThat(new ControlFlowPointcut(MyComponent.class, "getAge")).isEqualTo(new ControlFlowPointcut(MyComponent.class, "getAge")); - assertThat(new ControlFlowPointcut(MyComponent.class, "getAge")).isNotEqualTo(new ControlFlowPointcut(MyComponent.class)); + assertThat(pointcut()).isEqualTo(pointcut()); + assertThat(pointcut()).hasSameHashCodeAs(pointcut()); + + assertThat(pointcut("getAge")).isEqualTo(pointcut("getAge")); + assertThat(pointcut("getAge")).hasSameHashCodeAs(pointcut("getAge")); + + assertThat(pointcut("getAge")).isNotEqualTo(pointcut()); + assertThat(pointcut("getAge")).doesNotHaveSameHashCodeAs(pointcut()); - assertThat(new ControlFlowPointcut(MyComponent.class)).hasSameHashCodeAs(new ControlFlowPointcut(MyComponent.class)); - assertThat(new ControlFlowPointcut(MyComponent.class, "getAge")).hasSameHashCodeAs(new ControlFlowPointcut(MyComponent.class, "getAge")); - assertThat(new ControlFlowPointcut(MyComponent.class, "getAge")).doesNotHaveSameHashCodeAs(new ControlFlowPointcut(MyComponent.class)); + assertThat(pointcut("get*", "set*")).isEqualTo(pointcut("get*", "set*")); + assertThat(pointcut("get*", "set*")).isEqualTo(pointcut("get*", "set*", "set*", "get*")); + assertThat(pointcut("get*", "set*")).hasSameHashCodeAs(pointcut("get*", "get*", "set*")); + + assertThat(pointcut("get*", "set*")).isNotEqualTo(pointcut("set*", "get*")); + assertThat(pointcut("get*", "set*")).doesNotHaveSameHashCodeAs(pointcut("set*", "get*")); + + assertThat(pointcut("get*", "set*")).isEqualTo(pointcut(List.of("get*", "set*"))); + assertThat(pointcut("get*", "set*")).isEqualTo(pointcut(List.of("get*", "set*", "set*", "get*"))); + assertThat(pointcut("get*", "set*")).hasSameHashCodeAs(pointcut(List.of("get*", "get*", "set*"))); } @Test void testToString() { - assertThat(new ControlFlowPointcut(MyComponent.class)).asString() - .isEqualTo(ControlFlowPointcut.class.getName() + ": class = " + MyComponent.class.getName() + "; methodName = null"); - assertThat(new ControlFlowPointcut(MyComponent.class, "getAge")).asString() - .isEqualTo(ControlFlowPointcut.class.getName() + ": class = " + MyComponent.class.getName() + "; methodName = getAge"); + String pointcutType = ControlFlowPointcut.class.getName(); + String componentType = MyComponent.class.getName(); + + assertThat(pointcut()).asString() + .startsWith(pointcutType) + .contains(componentType) + .endsWith("[]"); + + assertThat(pointcut("getAge")).asString() + .startsWith(pointcutType) + .contains(componentType) + .endsWith("[getAge]"); + + assertThat(pointcut("get*", "set*", "get*")).asString() + .startsWith(pointcutType) + .contains(componentType) + .endsWith("[get*, set*]"); + } + + + private static ControlFlowPointcut pointcut() { + return new ControlFlowPointcut(MyComponent.class); + } + + private static ControlFlowPointcut pointcut(String methodNamePattern) { + return new ControlFlowPointcut(MyComponent.class, methodNamePattern); + } + + private static ControlFlowPointcut pointcut(String... methodNamePatterns) { + return new ControlFlowPointcut(MyComponent.class, methodNamePatterns); + } + + private static ControlFlowPointcut pointcut(List methodNamePatterns) { + return new ControlFlowPointcut(MyComponent.class, methodNamePatterns); }