Ensure ClassFilter and MethodMatcher implementations are cacheable

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
This commit is contained in:
Sam Brannen
2019-09-18 17:13:33 +02:00
parent b2aad1c3b1
commit 8f6846827d
19 changed files with 373 additions and 45 deletions

View File

@@ -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.
@@ -34,6 +34,7 @@ import static org.junit.Assert.*;
* @author Rod Johnson
* @author Rick Evans
* @author Chris Beams
* @author Sam Brannen
*/
public class TypePatternClassFilterTests {
@@ -88,4 +89,34 @@ public class TypePatternClassFilterTests {
new TypePatternClassFilter().matches(String.class);
}
@Test
public void testEquals() {
TypePatternClassFilter filter1 = new TypePatternClassFilter("org.springframework.tests.sample.beans.*");
TypePatternClassFilter filter2 = new TypePatternClassFilter("org.springframework.tests.sample.beans.*");
TypePatternClassFilter filter3 = new TypePatternClassFilter("org.springframework.tests.*");
assertEquals(filter1, filter2);
assertNotEquals(filter1, filter3);
}
@Test
public void testHashCode() {
TypePatternClassFilter filter1 = new TypePatternClassFilter("org.springframework.tests.sample.beans.*");
TypePatternClassFilter filter2 = new TypePatternClassFilter("org.springframework.tests.sample.beans.*");
TypePatternClassFilter filter3 = new TypePatternClassFilter("org.springframework.tests.*");
assertEquals(filter1.hashCode(), filter2.hashCode());
assertNotEquals(filter1.hashCode(), filter3.hashCode());
}
@Test
public void testToString() {
TypePatternClassFilter filter1 = new TypePatternClassFilter("org.springframework.tests.sample.beans.*");
TypePatternClassFilter filter2 = new TypePatternClassFilter("org.springframework.tests.sample.beans.*");
assertEquals("org.springframework.aop.aspectj.TypePatternClassFilter: org.springframework.tests.sample.beans.*",
filter1.toString());
assertEquals(filter1.toString(), filter2.toString());
}
}

View File

@@ -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.
@@ -23,39 +23,46 @@ import org.springframework.core.NestedRuntimeException;
import org.springframework.tests.sample.beans.ITestBean;
import org.springframework.tests.sample.beans.TestBean;
import static org.junit.Assert.*;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
/**
* Unit tests for {@link ClassFilters}.
*
* @author Rod Johnson
* @author Chris Beams
* @author Sam Brannen
*/
public class ClassFiltersTests {
private ClassFilter exceptionFilter = new RootClassFilter(Exception.class);
private final ClassFilter exceptionFilter = new RootClassFilter(Exception.class);
private ClassFilter itbFilter = new RootClassFilter(ITestBean.class);
private final ClassFilter interfaceFilter = new RootClassFilter(ITestBean.class);
private final ClassFilter hasRootCauseFilter = new RootClassFilter(NestedRuntimeException.class);
private ClassFilter hasRootCauseFilter = new RootClassFilter(NestedRuntimeException.class);
@Test
public void testUnion() {
public void union() {
assertTrue(exceptionFilter.matches(RuntimeException.class));
assertFalse(exceptionFilter.matches(TestBean.class));
assertFalse(itbFilter.matches(Exception.class));
assertTrue(itbFilter.matches(TestBean.class));
ClassFilter union = ClassFilters.union(exceptionFilter, itbFilter);
assertFalse(interfaceFilter.matches(Exception.class));
assertTrue(interfaceFilter.matches(TestBean.class));
ClassFilter union = ClassFilters.union(exceptionFilter, interfaceFilter);
assertTrue(union.matches(RuntimeException.class));
assertTrue(union.matches(TestBean.class));
assertTrue(union.toString().matches("^.+UnionClassFilter: \\[.+RootClassFilter: .+Exception, .+RootClassFilter: .+TestBean\\]$"));
}
@Test
public void testIntersection() {
public void intersection() {
assertTrue(exceptionFilter.matches(RuntimeException.class));
assertTrue(hasRootCauseFilter.matches(NestedRuntimeException.class));
ClassFilter intersection = ClassFilters.intersection(exceptionFilter, hasRootCauseFilter);
assertFalse(intersection.matches(RuntimeException.class));
assertFalse(intersection.matches(TestBean.class));
assertTrue(intersection.matches(NestedRuntimeException.class));
assertTrue(intersection.toString().matches("^.+IntersectionClassFilter: \\[.+RootClassFilter: .+Exception, .+RootClassFilter: .+NestedRuntimeException\\]$"));
}
}

View File

@@ -100,6 +100,14 @@ public class ControlFlowPointcutTests {
assertFalse(new ControlFlowPointcut(One.class, "getAge").hashCode() == new ControlFlowPointcut(One.class).hashCode());
}
@Test
public void testToString() {
assertEquals(ControlFlowPointcut.class.getName() + ": class = " + One.class.getName() + "; methodName = null",
new ControlFlowPointcut(One.class).toString());
assertEquals(ControlFlowPointcut.class.getName() + ": class = " + One.class.getName() + "; methodName = getAge",
new ControlFlowPointcut(One.class, "getAge").toString());
}
public class One {
int getAge(ITestBean proxied) {
return proxied.getAge();

View File

@@ -0,0 +1,66 @@
/*
* 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.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.aop.support;
import org.junit.Test;
import org.springframework.aop.ClassFilter;
import org.springframework.tests.sample.beans.ITestBean;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertTrue;
/**
* Unit tests for {@link RootClassFilter}.
*
* @author Sam Brannen
* @since 5.1.10
*/
public class RootClassFilterTests {
private final ClassFilter filter1 = new RootClassFilter(Exception.class);
private final ClassFilter filter2 = new RootClassFilter(Exception.class);
private final ClassFilter filter3 = new RootClassFilter(ITestBean.class);
@Test
public void matches() {
assertTrue(filter1.matches(Exception.class));
assertTrue(filter1.matches(RuntimeException.class));
assertFalse(filter1.matches(Error.class));
}
@Test
public void testEquals() {
assertEquals(filter1, filter2);
assertNotEquals(filter1, filter3);
}
@Test
public void testHashCode() {
assertEquals(filter1.hashCode(), filter2.hashCode());
assertNotEquals(filter1.hashCode(), filter3.hashCode());
}
@Test
public void testToString() {
assertEquals("org.springframework.aop.support.RootClassFilter: java.lang.Exception", filter1.toString());
assertEquals(filter1.toString(), filter2.toString());
}
}

View File

@@ -0,0 +1,108 @@
/*
* 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.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.aop.support.annotation;
import org.junit.Test;
import org.springframework.aop.ClassFilter;
import org.springframework.aop.MethodMatcher;
import org.springframework.aop.Pointcut;
import org.springframework.beans.factory.annotation.Qualifier;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertTrue;
/**
* Unit tests for {@link AnnotationMatchingPointcut}.
*
* @author Sam Brannen
* @since 5.1.10
*/
public class AnnotationMatchingPointcutTests {
@Test
public void classLevelPointcuts() {
Pointcut pointcut1 = new AnnotationMatchingPointcut(Qualifier.class, true);
Pointcut pointcut2 = new AnnotationMatchingPointcut(Qualifier.class, true);
Pointcut pointcut3 = new AnnotationMatchingPointcut(Qualifier.class);
assertEquals(AnnotationClassFilter.class, pointcut1.getClassFilter().getClass());
assertEquals(AnnotationClassFilter.class, pointcut2.getClassFilter().getClass());
assertEquals(AnnotationClassFilter.class, pointcut3.getClassFilter().getClass());
assertTrue(pointcut1.getClassFilter().toString().contains(Qualifier.class.getName()));
assertEquals(MethodMatcher.TRUE, pointcut1.getMethodMatcher());
assertEquals(MethodMatcher.TRUE, pointcut2.getMethodMatcher());
assertEquals(MethodMatcher.TRUE, pointcut3.getMethodMatcher());
assertEquals(pointcut1, pointcut2);
assertNotEquals(pointcut1, pointcut3);
assertEquals(pointcut1.hashCode(), pointcut2.hashCode());
// #1 and #3 have equivalent hash codes even though equals() returns false.
assertEquals(pointcut1.hashCode(), pointcut3.hashCode());
assertEquals(pointcut1.toString(), pointcut2.toString());
}
@Test
public void methodLevelPointcuts() {
Pointcut pointcut1 = new AnnotationMatchingPointcut(null, Qualifier.class, true);
Pointcut pointcut2 = new AnnotationMatchingPointcut(null, Qualifier.class, true);
Pointcut pointcut3 = new AnnotationMatchingPointcut(null, Qualifier.class);
assertEquals(ClassFilter.TRUE, pointcut1.getClassFilter());
assertEquals(ClassFilter.TRUE, pointcut2.getClassFilter());
assertEquals(ClassFilter.TRUE, pointcut3.getClassFilter());
assertEquals("ClassFilter.TRUE", pointcut1.getClassFilter().toString());
assertEquals(AnnotationMethodMatcher.class, pointcut1.getMethodMatcher().getClass());
assertEquals(AnnotationMethodMatcher.class, pointcut2.getMethodMatcher().getClass());
assertEquals(AnnotationMethodMatcher.class, pointcut3.getMethodMatcher().getClass());
assertEquals(pointcut1, pointcut2);
assertNotEquals(pointcut1, pointcut3);
assertEquals(pointcut1.hashCode(), pointcut2.hashCode());
// #1 and #3 have equivalent hash codes even though equals() returns false.
assertEquals(pointcut1.hashCode(), pointcut3.hashCode());
assertEquals(pointcut1.toString(), pointcut2.toString());
}
@Test
public void classLevelAndMethodLevelPointcuts() {
Pointcut pointcut1 = new AnnotationMatchingPointcut(Qualifier.class, Qualifier.class, true);
Pointcut pointcut2 = new AnnotationMatchingPointcut(Qualifier.class, Qualifier.class, true);
Pointcut pointcut3 = new AnnotationMatchingPointcut(Qualifier.class, Qualifier.class);
assertEquals(AnnotationClassFilter.class, pointcut1.getClassFilter().getClass());
assertEquals(AnnotationClassFilter.class, pointcut2.getClassFilter().getClass());
assertEquals(AnnotationClassFilter.class, pointcut3.getClassFilter().getClass());
assertTrue(pointcut1.getClassFilter().toString().contains(Qualifier.class.getName()));
assertEquals(AnnotationMethodMatcher.class, pointcut1.getMethodMatcher().getClass());
assertEquals(AnnotationMethodMatcher.class, pointcut2.getMethodMatcher().getClass());
assertEquals(AnnotationMethodMatcher.class, pointcut3.getMethodMatcher().getClass());
assertTrue(pointcut1.getMethodMatcher().toString().contains(Qualifier.class.getName()));
assertEquals(pointcut1, pointcut2);
assertNotEquals(pointcut1, pointcut3);
assertEquals(pointcut1.hashCode(), pointcut2.hashCode());
// #1 and #3 have equivalent hash codes even though equals() returns false.
assertEquals(pointcut1.hashCode(), pointcut3.hashCode());
assertEquals(pointcut1.toString(), pointcut2.toString());
}
}