Skip to content

Commit

Permalink
Ensure ClassFilter and MethodMatcher implementations are cacheable
Browse files Browse the repository at this point in the history
While resolving the regression raised in gh-23571, it came to our
attention that not all of our ClassFilter and MethodMatcher
implementations were properly cacheable with CGLIB generated proxies
due to missing (or improper) equals() and hashCode() implementations.

Although such deficiencies may not manifest themselves as bugs in Core
Spring's default arrangements, these might cause issues in custom
arrangements in user applications.

This commit addresses this by ensuring that ClassFilter and
MethodMatcher implementations properly implement equals() and
hashCode(). In addition, missing toString() implementations have been
added to improve diagnostics for logging and debugging.

Closes gh-23659
  • Loading branch information
sbrannen committed Sep 19, 2019
1 parent b2aad1c commit 8f68468
Show file tree
Hide file tree
Showing 19 changed files with 373 additions and 45 deletions.
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2016 the original author or authors.
* Copyright 2002-2019 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.
Expand All @@ -23,6 +23,11 @@
* <p>Can be used as part of a {@link Pointcut} or for the entire
* targeting of an {@link IntroductionAdvisor}.
*
* <p>Concrete implementations of this interface typically should provide proper
* implementations of {@link Object#equals(Object)} and {@link Object#hashCode()}
* in order to allow the filter to be used in caching scenarios &mdash; for
* example, in proxies generated by CGLIB.
*
* @author Rod Johnson
* @see Pointcut
* @see MethodMatcher
Expand Down
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2019 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.
Expand Down Expand Up @@ -40,6 +40,11 @@
* in an interceptor chain, will have run, so any state changes they have produced in
* parameters or ThreadLocal state will be available at the time of evaluation.
*
* <p>Concrete implementations of this interface typically should provide proper
* implementations of {@link Object#equals(Object)} and {@link Object#hashCode()}
* in order to allow the matcher to be used in caching scenarios &mdash; for
* example, in proxies generated by CGLIB.
*
* @author Rod Johnson
* @since 11.11.2003
* @see Pointcut
Expand Down
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2019 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.
Expand Down Expand Up @@ -735,6 +735,11 @@ public boolean equals(Object other) {
public int hashCode() {
return this.adviceMethod.hashCode();
}

@Override
public String toString() {
return getClass().getName() + ": " + this.adviceMethod;
}
}

}
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2017 the original author or authors.
* Copyright 2002-2019 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.
Expand All @@ -22,13 +22,15 @@
import org.springframework.aop.ClassFilter;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.ObjectUtils;
import org.springframework.util.StringUtils;

/**
* Spring AOP {@link ClassFilter} implementation using AspectJ type matching.
*
* @author Rod Johnson
* @author Juergen Hoeller
* @author Sam Brannen
* @since 2.0
*/
public class TypePatternClassFilter implements ClassFilter {
Expand Down Expand Up @@ -113,4 +115,21 @@ private String replaceBooleanOperators(String pcExpr) {
result = StringUtils.replace(result, " or ", " || ");
return StringUtils.replace(result, " not ", " ! ");
}

@Override
public boolean equals(Object other) {
return (this == other || (other instanceof TypePatternClassFilter &&
ObjectUtils.nullSafeEquals(this.typePattern, ((TypePatternClassFilter) other).typePattern)));
}

@Override
public int hashCode() {
return ObjectUtils.nullSafeHashCode(this.typePattern);
}

@Override
public String toString() {
return getClass().getName() + ": " + this.typePattern;
}

}
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2013 the original author or authors.
* Copyright 2002-2019 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.
Expand All @@ -17,6 +17,7 @@
package org.springframework.aop.support;

import java.io.Serializable;
import java.util.Arrays;

import org.springframework.aop.ClassFilter;
import org.springframework.util.Assert;
Expand All @@ -28,6 +29,7 @@
* @author Rod Johnson
* @author Rob Harrop
* @author Juergen Hoeller
* @author Sam Brannen
* @since 11.11.2003
* @see MethodMatchers
* @see Pointcuts
Expand Down Expand Up @@ -89,9 +91,9 @@ public static ClassFilter intersection(ClassFilter[] classFilters) {
@SuppressWarnings("serial")
private static class UnionClassFilter implements ClassFilter, Serializable {

private ClassFilter[] filters;
private final ClassFilter[] filters;

public UnionClassFilter(ClassFilter[] filters) {
UnionClassFilter(ClassFilter[] filters) {
this.filters = filters;
}

Expand All @@ -115,6 +117,12 @@ public boolean equals(Object other) {
public int hashCode() {
return ObjectUtils.nullSafeHashCode(this.filters);
}

@Override
public String toString() {
return getClass().getName() + ": " + Arrays.toString(this.filters);
}

}


Expand All @@ -124,9 +132,9 @@ public int hashCode() {
@SuppressWarnings("serial")
private static class IntersectionClassFilter implements ClassFilter, Serializable {

private ClassFilter[] filters;
private final ClassFilter[] filters;

public IntersectionClassFilter(ClassFilter[] filters) {
IntersectionClassFilter(ClassFilter[] filters) {
this.filters = filters;
}

Expand All @@ -150,6 +158,12 @@ public boolean equals(Object other) {
public int hashCode() {
return ObjectUtils.nullSafeHashCode(this.filters);
}

@Override
public String toString() {
return getClass().getName() + ": " + Arrays.toString(this.filters);
}

}

}
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2019 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.
Expand All @@ -24,12 +24,15 @@
import org.springframework.util.Assert;

/**
* Convenient class for building up pointcuts. All methods return
* ComposablePointcut, so we can use a concise idiom like:
* Convenient class for building up pointcuts.
*
* {@code
* Pointcut pc = new ComposablePointcut().union(classFilter).intersection(methodMatcher).intersection(pointcut);
* }
* <p>All methods return {@code ComposablePointcut}, so we can use concise idioms
* like in the following example.
*
* <pre class="code">Pointcut pc = new ComposablePointcut()
* .union(classFilter)
* .intersection(methodMatcher)
* .intersection(pointcut);</pre>
*
* @author Rod Johnson
* @author Juergen Hoeller
Expand Down Expand Up @@ -199,7 +202,7 @@ public int hashCode() {

@Override
public String toString() {
return "ComposablePointcut: " + this.classFilter + ", " +this.methodMatcher;
return "ComposablePointcut: " + this.classFilter + ", " + this.methodMatcher;
}

}
Expand Up @@ -34,14 +34,15 @@
* @author Rod Johnson
* @author Rob Harrop
* @author Juergen Hoeller
* @author Sam Brannen
*/
@SuppressWarnings("serial")
public class ControlFlowPointcut implements Pointcut, ClassFilter, MethodMatcher, Serializable {

private Class<?> clazz;
private final Class<?> clazz;

@Nullable
private String methodName;
private final String methodName;

private volatile int evaluations;

Expand Down Expand Up @@ -142,4 +143,9 @@ public int hashCode() {
return code;
}

@Override
public String toString() {
return getClass().getName() + ": class = " + this.clazz.getName() + "; methodName = " + methodName;
}

}
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2019 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.
Expand Down Expand Up @@ -168,7 +168,7 @@ public int hashCode() {

@Override
public String toString() {
return ClassUtils.getShortName(getClass()) + ": advice [" + this.advice + "]; interfaces " +
return getClass().getName() + ": advice [" + this.advice + "]; interfaces " +
ClassUtils.classNamesToString(this.interfaces);
}

Expand Down
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2019 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.
Expand Down Expand Up @@ -34,6 +34,7 @@
* @author Rod Johnson
* @author Rob Harrop
* @author Juergen Hoeller
* @author Sam Brannen
* @since 11.11.2003
* @see ClassFilters
* @see Pointcuts
Expand Down Expand Up @@ -155,6 +156,11 @@ public boolean equals(Object other) {
public int hashCode() {
return 37 * this.mm1.hashCode() + this.mm2.hashCode();
}

@Override
public String toString() {
return getClass().getName() + ": " + this.mm1 + ", " + this.mm2;
}
}


Expand Down Expand Up @@ -229,6 +235,11 @@ public int hashCode() {
// Allow for matching with regular UnionMethodMatcher by providing same hash...
return super.hashCode();
}

@Override
public String toString() {
return getClass().getName() + ": " + this.cf1 + ", " + this.mm1 + ", " + this.cf2 + ", " + this.mm2;
}
}


Expand Down Expand Up @@ -311,6 +322,11 @@ public boolean equals(Object other) {
public int hashCode() {
return 37 * this.mm1.hashCode() + this.mm2.hashCode();
}

@Override
public String toString() {
return getClass().getName() + ": " + this.mm1 + ", " + this.mm2;
}
}


Expand Down
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2019 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.
Expand All @@ -25,8 +25,8 @@
import org.springframework.util.PatternMatchUtils;

/**
* Pointcut bean for simple method name matches, as alternative to regexp patterns.
* Does not handle overloaded methods: all methods with a given name will be eligible.
* Pointcut bean for simple method name matches, as an alternative to regexp patterns.
* <p>Does not handle overloaded methods: all methods with a given name will be eligible.
*
* @author Juergen Hoeller
* @author Rod Johnson
Expand Down Expand Up @@ -108,4 +108,9 @@ public int hashCode() {
return this.mappedNames.hashCode();
}

@Override
public String toString() {
return getClass().getName() + ": " + this.mappedNames;
}

}
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2019 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.
Expand All @@ -26,7 +26,7 @@
/**
* Pointcut constants for matching getters and setters,
* and static methods useful for manipulating and evaluating pointcuts.
* These methods are particularly useful for composing pointcuts
* <p>These methods are particularly useful for composing pointcuts
* using the union and intersection methods.
*
* @author Rod Johnson
Expand Down Expand Up @@ -106,6 +106,11 @@ public boolean matches(Method method, Class<?> targetClass) {
private Object readResolve() {
return INSTANCE;
}

@Override
public String toString() {
return "Pointcuts.SETTERS";
}
}


Expand All @@ -126,6 +131,11 @@ public boolean matches(Method method, Class<?> targetClass) {
private Object readResolve() {
return INSTANCE;
}

@Override
public String toString() {
return "Pointcuts.GETTERS";
}
}

}

0 comments on commit 8f68468

Please sign in to comment.