From 1f6fd16dae388e4d5a727f0e1fc4721de682ad7e Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Mon, 28 Aug 2023 15:01:17 +0200 Subject: [PATCH] Polish "Introduce reverse on ClassFilter and MethodFilter" See gh-26725 --- .../aop/support/ClassFilters.java | 40 ++++++----- .../aop/support/MethodMatchers.java | 51 ++++++------- .../aop/support/ClassFiltersTests.java | 71 ++++++++++++++++--- .../aop/support/MethodMatchersTests.java | 69 +++++++++++++++--- 4 files changed, 165 insertions(+), 66 deletions(-) diff --git a/spring-aop/src/main/java/org/springframework/aop/support/ClassFilters.java b/spring-aop/src/main/java/org/springframework/aop/support/ClassFilters.java index 028c07f83c2..fa24bb10504 100644 --- a/spring-aop/src/main/java/org/springframework/aop/support/ClassFilters.java +++ b/spring-aop/src/main/java/org/springframework/aop/support/ClassFilters.java @@ -18,6 +18,7 @@ package org.springframework.aop.support; import java.io.Serializable; import java.util.Arrays; +import java.util.Objects; import org.springframework.aop.ClassFilter; import org.springframework.lang.Nullable; @@ -86,14 +87,15 @@ public abstract class ClassFilters { } /** - * reverse the given ClassFilter match. - * @param cf the ClassFilter - * @return a distinct ClassFilter that not matches classes - * of the given ClassFilter match + * Return a class filter that represents the logical negation of the specified + * filter instance. + * @param classFilter the {@link ClassFilter} to negate + * @return a filter that represents the logical negation of the specified filter + * @since 6.1 */ - public static ClassFilter reversion(ClassFilter cf) { - Assert.notNull(cf, "ClassFilter must not be null"); - return new ReversionClassFilter(cf); + public static ClassFilter negate(ClassFilter classFilter) { + Assert.notNull(classFilter, "ClassFilter must not be null"); + return new NegateClassFilter(classFilter); } @@ -180,38 +182,38 @@ public abstract class ClassFilters { /** - * ClassFilter implementation for an reversion of the given ClassFilter. + * ClassFilter implementation for a logical negation of the given ClassFilter. */ @SuppressWarnings("serial") - private static class ReversionClassFilter implements ClassFilter, Serializable { + private static class NegateClassFilter implements ClassFilter, Serializable { - private final ClassFilter filter; + private final ClassFilter original; - ReversionClassFilter(ClassFilter filter) { - this.filter = filter; + NegateClassFilter(ClassFilter original) { + this.original = original; } @Override public boolean matches(Class clazz) { - return !this.filter.matches(clazz); + return !this.original.matches(clazz); } @Override - public boolean equals(@Nullable Object other) { - return (this == other || (other instanceof ReversionClassFilter && - this.filter.equals(((ReversionClassFilter)other).filter))); + public boolean equals(Object other) { + return (this == other || (other instanceof NegateClassFilter that + && this.original.equals(that.original))); } @Override public int hashCode() { - return 37 * this.filter.hashCode(); + return Objects.hash(getClass(), this.original); } @Override public String toString() { - return getClass().getName() + ": " + this.filter; + return "Negate " + this.original; } } - + } diff --git a/spring-aop/src/main/java/org/springframework/aop/support/MethodMatchers.java b/spring-aop/src/main/java/org/springframework/aop/support/MethodMatchers.java index 37d764662cb..f2d226adfb2 100644 --- a/spring-aop/src/main/java/org/springframework/aop/support/MethodMatchers.java +++ b/spring-aop/src/main/java/org/springframework/aop/support/MethodMatchers.java @@ -18,6 +18,7 @@ package org.springframework.aop.support; import java.io.Serializable; import java.lang.reflect.Method; +import java.util.Objects; import org.springframework.aop.ClassFilter; import org.springframework.aop.IntroductionAwareMethodMatcher; @@ -82,13 +83,15 @@ public abstract class MethodMatchers { } /** - * reverse the given MethodMatcher match. - * @param mm the MethodMatcher - * @return a distinct MethodMatcher that not matches methods - * of the given MethodMatcher match + * Return a method matcher that represents the logical negation of the specified + * matcher instance. + * @param methodMatcher the {@link MethodMatcher} to negate + * @return a matcher that represents the logical negation of the specified matcher + * @since 6.1 */ - public static MethodMatcher reversion(MethodMatcher mm) { - return new ReversionMethodMatcher(mm); + public static MethodMatcher negate(MethodMatcher methodMatcher) { + Assert.notNull(methodMatcher, "MethodMatcher must not be null"); + return new NegateMethodMatcher(methodMatcher); } /** @@ -349,56 +352,46 @@ public abstract class MethodMatchers { } - /** - * MethodMatcher implementation for an reversion of the given MethodMatcher. - */ @SuppressWarnings("serial") - private static class ReversionMethodMatcher implements MethodMatcher, Serializable { + private static class NegateMethodMatcher implements MethodMatcher, Serializable { - protected final MethodMatcher mm; + private final MethodMatcher original; - public ReversionMethodMatcher(MethodMatcher mm) { - Assert.notNull(mm, "MethodMatcher must not be null"); - this.mm = mm; + NegateMethodMatcher(MethodMatcher original) { + this.original = original; } @Override public boolean matches(Method method, Class targetClass) { - return !this.mm.matches(method, targetClass); + return !this.original.matches(method, targetClass); } @Override public boolean isRuntime() { - return this.mm.isRuntime(); + return this.original.isRuntime(); } @Override public boolean matches(Method method, Class targetClass, Object... args) { - return !this.mm.matches(method, targetClass, args); + return !this.original.matches(method, targetClass, args); } @Override - public boolean equals(@Nullable Object other) { - if (this == other) { - return true; - } - if (!(other instanceof ReversionMethodMatcher)) { - return false; - } - ReversionMethodMatcher that = (ReversionMethodMatcher) other; - return this.mm.equals(that.mm); + public boolean equals(Object other) { + return (this == other || (other instanceof NegateMethodMatcher that + && this.original.equals(that.original))); } @Override public int hashCode() { - return 37 * this.mm.hashCode(); + return Objects.hash(getClass(), this.original); } @Override public String toString() { - return getClass().getName() + ": " + this.mm; + return "Negate " + this.original; } - + } } diff --git a/spring-aop/src/test/java/org/springframework/aop/support/ClassFiltersTests.java b/spring-aop/src/test/java/org/springframework/aop/support/ClassFiltersTests.java index 3f0872eb327..b108eab36ac 100644 --- a/spring-aop/src/test/java/org/springframework/aop/support/ClassFiltersTests.java +++ b/spring-aop/src/test/java/org/springframework/aop/support/ClassFiltersTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2023 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. @@ -24,6 +24,9 @@ import org.springframework.beans.testfixture.beans.TestBean; import org.springframework.core.NestedRuntimeException; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; /** * Unit tests for {@link ClassFilters}. @@ -67,12 +70,64 @@ class ClassFiltersTests { } @Test - void reversion() { - assertThat(exceptionFilter.matches(RuntimeException.class)).isTrue(); - ClassFilter reversion = ClassFilters.reversion(exceptionFilter); - assertThat(reversion.matches(RuntimeException.class)).isFalse(); - assertThat(reversion.toString()) - .matches("^.+ReversionClassFilter: .+RootClassFilter: .+Exception$"); + void negateClassFilter() { + ClassFilter filter = mock(ClassFilter.class); + given(filter.matches(String.class)).willReturn(true); + ClassFilter negate = ClassFilters.negate(filter); + assertThat(negate.matches(String.class)).isFalse(); + verify(filter).matches(String.class); + } + + @Test + void negateTrueClassFilter() { + ClassFilter negate = ClassFilters.negate(ClassFilter.TRUE); + assertThat(negate.matches(String.class)).isFalse(); + assertThat(negate.matches(Object.class)).isFalse(); + assertThat(negate.matches(Integer.class)).isFalse(); + } + + @Test + void negateTrueClassFilterAppliedTwice() { + ClassFilter negate = ClassFilters.negate(ClassFilters.negate(ClassFilter.TRUE)); + assertThat(negate.matches(String.class)).isTrue(); + assertThat(negate.matches(Object.class)).isTrue(); + assertThat(negate.matches(Integer.class)).isTrue(); + } + + @Test + void negateIsNotEqualsToOriginalFilter() { + ClassFilter original = ClassFilter.TRUE; + ClassFilter negate = ClassFilters.negate(original); + assertThat(original).isNotEqualTo(negate); } - + + @Test + void negateOnSameFilterIsEquals() { + ClassFilter original = ClassFilter.TRUE; + ClassFilter first = ClassFilters.negate(original); + ClassFilter second = ClassFilters.negate(original); + assertThat(first).isEqualTo(second); + } + + @Test + void negateHasNotSameHashCodeAsOriginalFilter() { + ClassFilter original = ClassFilter.TRUE; + ClassFilter negate = ClassFilters.negate(original); + assertThat(original).doesNotHaveSameHashCodeAs(negate); + } + + @Test + void negateOnSameFilterHasSameHashCode() { + ClassFilter original = ClassFilter.TRUE; + ClassFilter first = ClassFilters.negate(original); + ClassFilter second = ClassFilters.negate(original); + assertThat(first).hasSameHashCodeAs(second); + } + + @Test + void toStringIncludesRepresentationOfOriginalFilter() { + ClassFilter original = ClassFilter.TRUE; + assertThat(ClassFilters.negate(original)).hasToString("Negate " + original); + } + } diff --git a/spring-aop/src/test/java/org/springframework/aop/support/MethodMatchersTests.java b/spring-aop/src/test/java/org/springframework/aop/support/MethodMatchersTests.java index 7c0084d63e7..c33a8b548d0 100644 --- a/spring-aop/src/test/java/org/springframework/aop/support/MethodMatchersTests.java +++ b/spring-aop/src/test/java/org/springframework/aop/support/MethodMatchersTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2023 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. @@ -28,6 +28,7 @@ import org.springframework.core.testfixture.io.SerializationTestUtils; import org.springframework.lang.Nullable; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; /** * @author Juergen Hoeller @@ -35,6 +36,8 @@ import static org.assertj.core.api.Assertions.assertThat; */ public class MethodMatchersTests { + private final static Method TEST_METHOD = mock(Method.class); + private final Method EXCEPTION_GETMESSAGE; private final Method ITESTBEAN_SETAGE; @@ -112,16 +115,62 @@ public class MethodMatchersTests { } @Test - public void testDynamicAndStaticMethodMatcherReversion() throws Exception { + void negateMethodMatcher() { MethodMatcher getterMatcher = new StartsWithMatcher("get"); - MethodMatcher reversion = MethodMatchers.reversion(getterMatcher); - assertThat(reversion.isRuntime()).as("Reversion is a static matcher").isFalse(); - assertThat(reversion.matches(ITESTBEAN_GETAGE, TestBean.class)).as("Didn't matched getAge method").isFalse(); - assertThat(reversion.matches(IOTHER_ABSQUATULATE, TestBean.class)).as("Matched absquatulate method").isTrue(); - reversion = MethodMatchers.reversion(new TestDynamicMethodMatcherWhichDoesNotMatch()); - assertThat(reversion.isRuntime()).as("Intersection is a dynamic matcher").isTrue(); - assertThat(reversion.matches(ITESTBEAN_SETAGE, TestBean.class)).as("Didn't matched setAge method").isFalse(); - assertThat(reversion.matches(ITESTBEAN_SETAGE, TestBean.class, 5)).as("Matched setAge method").isTrue(); + MethodMatcher negate = MethodMatchers.negate(getterMatcher); + assertThat(negate.matches(ITESTBEAN_SETAGE, int.class)).isTrue(); + } + + @Test + void negateTrueMethodMatcher() { + MethodMatcher negate = MethodMatchers.negate(MethodMatcher.TRUE); + assertThat(negate.matches(TEST_METHOD, String.class)).isFalse(); + assertThat(negate.matches(TEST_METHOD, Object.class)).isFalse(); + assertThat(negate.matches(TEST_METHOD, Integer.class)).isFalse(); + } + + @Test + void negateTrueMethodMatcherAppliedTwice() { + MethodMatcher negate = MethodMatchers.negate(MethodMatchers.negate(MethodMatcher.TRUE)); + assertThat(negate.matches(TEST_METHOD, String.class)).isTrue(); + assertThat(negate.matches(TEST_METHOD, Object.class)).isTrue(); + assertThat(negate.matches(TEST_METHOD, Integer.class)).isTrue(); + } + + @Test + void negateIsNotEqualsToOriginalMatcher() { + MethodMatcher original = MethodMatcher.TRUE; + MethodMatcher negate = MethodMatchers.negate(original); + assertThat(original).isNotEqualTo(negate); + } + + @Test + void negateOnSameMatcherIsEquals() { + MethodMatcher original = MethodMatcher.TRUE; + MethodMatcher first = MethodMatchers.negate(original); + MethodMatcher second = MethodMatchers.negate(original); + assertThat(first).isEqualTo(second); + } + + @Test + void negateHasNotSameHashCodeAsOriginalMatcher() { + MethodMatcher original = MethodMatcher.TRUE; + MethodMatcher negate = MethodMatchers.negate(original); + assertThat(original).doesNotHaveSameHashCodeAs(negate); + } + + @Test + void negateOnSameMatcherHasSameHashCode() { + MethodMatcher original = MethodMatcher.TRUE; + MethodMatcher first = MethodMatchers.negate(original); + MethodMatcher second = MethodMatchers.negate(original); + assertThat(first).hasSameHashCodeAs(second); + } + + @Test + void toStringIncludesRepresentationOfOriginalMatcher() { + MethodMatcher original = MethodMatcher.TRUE; + assertThat(MethodMatchers.negate(original)).hasToString("Negate " + original); }