From d510b738f45865ac09b8097ead7d31c84a00df07 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Fri, 4 Jul 2025 18:11:12 +0200 Subject: [PATCH] Match if empty by default in InstanceFilter and ExceptionTypeFilter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prior to this commit, the constructors for InstanceFilter and ExceptionTypeFilter required one to supply the matchIfEmpty flag. However, users will typically want that to be true. Moreover, we always supply true for the matchIfEmpty flag within the Spring Framework. This commit therefore makes the matchIfEmpty flag optional by introducing overloaded constructors for InstanceFilter and ExceptionTypeFilter that only accept the includes and excludes collections. In addition, this commit overhauls the Javadoc for InstanceFilter and ExceptionTypeFilter, fixing several issues in the documentation. Furthermore, this commit applies consistent @⁠Nullable declarations in ExceptionTypeFilter. Closes gh-35158 --- .../interceptor/AbstractJCacheOperation.java | 2 +- .../resilience/retry/MethodRetrySpec.java | 2 +- .../core/retry/DefaultRetryPolicy.java | 2 +- .../util/ExceptionTypeFilter.java | 50 ++++++++++++++-- .../springframework/util/InstanceFilter.java | 58 +++++++++++++------ .../util/ExceptionTypeFilterTests.java | 2 +- .../util/InstanceFilterTests.java | 20 +++---- 7 files changed, 97 insertions(+), 39 deletions(-) diff --git a/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/AbstractJCacheOperation.java b/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/AbstractJCacheOperation.java index 461b187cc5..f30d5aa567 100644 --- a/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/AbstractJCacheOperation.java +++ b/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/AbstractJCacheOperation.java @@ -134,7 +134,7 @@ abstract class AbstractJCacheOperation implements JCacheOp protected ExceptionTypeFilter createExceptionTypeFilter( Class[] includes, Class[] excludes) { - return new ExceptionTypeFilter(Arrays.asList(includes), Arrays.asList(excludes), true); + return new ExceptionTypeFilter(Arrays.asList(includes), Arrays.asList(excludes)); } diff --git a/spring-context/src/main/java/org/springframework/resilience/retry/MethodRetrySpec.java b/spring-context/src/main/java/org/springframework/resilience/retry/MethodRetrySpec.java index 515d9e4190..2dd82577e4 100644 --- a/spring-context/src/main/java/org/springframework/resilience/retry/MethodRetrySpec.java +++ b/spring-context/src/main/java/org/springframework/resilience/retry/MethodRetrySpec.java @@ -64,7 +64,7 @@ public record MethodRetrySpec( MethodRetryPredicate combinedPredicate() { - ExceptionTypeFilter exceptionFilter = new ExceptionTypeFilter(this.includes, this.excludes, true); + ExceptionTypeFilter exceptionFilter = new ExceptionTypeFilter(this.includes, this.excludes); return (method, throwable) -> exceptionFilter.match(throwable.getClass()) && this.predicate.shouldRetry(method, throwable); } diff --git a/spring-core/src/main/java/org/springframework/core/retry/DefaultRetryPolicy.java b/spring-core/src/main/java/org/springframework/core/retry/DefaultRetryPolicy.java index ead33d292b..34a3e30c64 100644 --- a/spring-core/src/main/java/org/springframework/core/retry/DefaultRetryPolicy.java +++ b/spring-core/src/main/java/org/springframework/core/retry/DefaultRetryPolicy.java @@ -51,7 +51,7 @@ class DefaultRetryPolicy implements RetryPolicy { this.includes = includes; this.excludes = excludes; - this.exceptionFilter = new ExceptionTypeFilter(this.includes, this.excludes, true); + this.exceptionFilter = new ExceptionTypeFilter(this.includes, this.excludes); this.predicate = predicate; this.backOff = backOff; } diff --git a/spring-core/src/main/java/org/springframework/util/ExceptionTypeFilter.java b/spring-core/src/main/java/org/springframework/util/ExceptionTypeFilter.java index d907979102..3bb0c4295f 100644 --- a/spring-core/src/main/java/org/springframework/util/ExceptionTypeFilter.java +++ b/spring-core/src/main/java/org/springframework/util/ExceptionTypeFilter.java @@ -18,21 +18,63 @@ package org.springframework.util; import java.util.Collection; +import org.jspecify.annotations.Nullable; + /** - * An {@link InstanceFilter} implementation that handles exception types. A type - * will match against a given candidate if it is assignable to that candidate. + * An {@link InstanceFilter} that handles exception types. + * + *

An exception type will match against a given candidate if it is assignable + * to that candidate. * * @author Stephane Nicoll + * @author Sam Brannen * @since 4.1 */ public class ExceptionTypeFilter extends InstanceFilter> { - public ExceptionTypeFilter(Collection> includes, - Collection> excludes, boolean matchIfEmpty) { + /** + * Create a new {@code ExceptionTypeFilter} based on include and exclude + * collections, with the {@code matchIfEmpty} flag set to {@code true}. + *

See {@link #ExceptionTypeFilter(Collection, Collection, boolean)} for + * details. + * @param includes the collection of includes + * @param excludes the collection of excludes + * @since 7.0 + */ + public ExceptionTypeFilter(@Nullable Collection> includes, + @Nullable Collection> excludes) { + + super(includes, excludes); + } + + /** + * Create a new {@code ExceptionTypeFilter} based on include and exclude + * collections. + *

See {@link InstanceFilter#InstanceFilter(Collection, Collection, boolean) + * InstanceFilter} for details. + * @param includes the collection of includes + * @param excludes the collection of excludes + * @param matchIfEmpty the matching result if the includes and the excludes + * collections are both {@code null} or empty + */ + public ExceptionTypeFilter(@Nullable Collection> includes, + @Nullable Collection> excludes, boolean matchIfEmpty) { super(includes, excludes, matchIfEmpty); } + + /** + * Determine if the specified {@code instance} matches the specified + * {@code candidate}. + *

By default, the two instances match if the {@code candidate} type is + * {@linkplain Class#isAssignableFrom(Class) is assignable from} the + * {@code instance} type. + *

Can be overridden by subclasses. + * @param instance the instance to check + * @param candidate a candidate defined by this filter + * @return {@code true} if the instance matches the candidate + */ @Override protected boolean match(Class instance, Class candidate) { return candidate.isAssignableFrom(instance); diff --git a/spring-core/src/main/java/org/springframework/util/InstanceFilter.java b/spring-core/src/main/java/org/springframework/util/InstanceFilter.java index d4a15a1d56..4765837f17 100644 --- a/spring-core/src/main/java/org/springframework/util/InstanceFilter.java +++ b/spring-core/src/main/java/org/springframework/util/InstanceFilter.java @@ -22,13 +22,14 @@ import java.util.Collections; import org.jspecify.annotations.Nullable; /** - * A simple instance filter that checks if a given instance match based on - * a collection of includes and excludes element. + * A simple instance filter that checks if a given instance matches based on + * collections of includes and excludes. * - *

Subclasses may want to override {@link #match(Object, Object)} to provide - * a custom matching algorithm. + *

Subclasses may override {@link #match(Object, Object)} to provide a custom + * matching algorithm. * * @author Stephane Nicoll + * @author Sam Brannen * @since 4.1 * @param the instance type */ @@ -42,17 +43,33 @@ public class InstanceFilter { /** - * Create a new instance based on includes/excludes collections. - *

A particular element will match if it "matches" the one of the element in the - * includes list and does not match one of the element in the excludes list. - *

Subclasses may redefine what matching means. By default, an element match with - * another if it is equals according to {@link Object#equals(Object)} - *

If both collections are empty, {@code matchIfEmpty} defines if - * an element matches or not. + * Create a new {@code InstanceFilter} based on include and exclude collections, + * with the {@code matchIfEmpty} flag set to {@code true}. + *

See {@link #InstanceFilter(Collection, Collection, boolean)} for details. * @param includes the collection of includes * @param excludes the collection of excludes - * @param matchIfEmpty the matching result if both the includes and the excludes - * collections are empty + * @since 7.0 + */ + public InstanceFilter(@Nullable Collection includes, + @Nullable Collection excludes) { + + this(includes, excludes, true); + } + + /** + * Create a new {@code InstanceFilter} based on include and exclude collections. + *

A particular element will match if it matches one of the elements + * in the {@code includes} list and does not match one of the elements in the + * {@code excludes} list. + *

Subclasses may redefine what matching means. By default, an element + * {@linkplain #match(Object, Object) matches} another if the two elements are + * {@linkplain Object#equals(Object) equal}. + *

If both collections are empty, {@code matchIfEmpty} defines if an element + * matches or not. + * @param includes the collection of includes + * @param excludes the collection of excludes + * @param matchIfEmpty the matching result if the includes and the excludes + * collections are both {@code null} or empty */ public InstanceFilter(@Nullable Collection includes, @Nullable Collection excludes, boolean matchIfEmpty) { @@ -87,9 +104,12 @@ public class InstanceFilter { } /** - * Determine if the specified {@code instance} is equal to the - * specified {@code candidate}. - * @param instance the instance to handle + * Determine if the specified {@code instance} matches the specified + * {@code candidate}. + *

By default, the two instances match if they are + * {@linkplain Object#equals(Object) equal}. + *

Can be overridden by subclasses. + * @param instance the instance to check * @param candidate a candidate defined by this filter * @return {@code true} if the instance matches the candidate */ @@ -99,10 +119,10 @@ public class InstanceFilter { /** * Determine if the specified {@code instance} matches one of the candidates. - *

If the candidates collection is {@code null}, returns {@code false}. * @param instance the instance to check - * @param candidates a list of candidates - * @return {@code true} if the instance match or the candidates collection is null + * @param candidates the collection of candidates + * @return {@code true} if the instance matches; {@code false} if the + * candidates collection is empty or there is no match */ protected boolean match(T instance, Collection candidates) { for (T candidate : candidates) { diff --git a/spring-core/src/test/java/org/springframework/util/ExceptionTypeFilterTests.java b/spring-core/src/test/java/org/springframework/util/ExceptionTypeFilterTests.java index c5f4673f38..222b8d4ab9 100644 --- a/spring-core/src/test/java/org/springframework/util/ExceptionTypeFilterTests.java +++ b/spring-core/src/test/java/org/springframework/util/ExceptionTypeFilterTests.java @@ -29,7 +29,7 @@ class ExceptionTypeFilterTests { @Test void subClassMatch() { - ExceptionTypeFilter filter = new ExceptionTypeFilter(List.of(RuntimeException.class), null, true); + ExceptionTypeFilter filter = new ExceptionTypeFilter(List.of(RuntimeException.class), null); assertThat(filter.match(RuntimeException.class)).isTrue(); assertThat(filter.match(IllegalStateException.class)).isTrue(); } diff --git a/spring-core/src/test/java/org/springframework/util/InstanceFilterTests.java b/spring-core/src/test/java/org/springframework/util/InstanceFilterTests.java index ad49689647..9cb54b8042 100644 --- a/spring-core/src/test/java/org/springframework/util/InstanceFilterTests.java +++ b/spring-core/src/test/java/org/springframework/util/InstanceFilterTests.java @@ -20,7 +20,6 @@ import java.util.List; import org.junit.jupiter.api.Test; -import static java.util.Arrays.asList; import static org.assertj.core.api.Assertions.assertThat; /** @@ -30,47 +29,44 @@ class InstanceFilterTests { @Test void emptyFilterApplyMatchIfEmpty() { - InstanceFilter filter = new InstanceFilter<>(null, null, true); + InstanceFilter filter = new InstanceFilter<>(null, null); match(filter, "foo"); match(filter, "bar"); } @Test void includesFilter() { - InstanceFilter filter = new InstanceFilter<>( - asList("First", "Second"), null, true); + InstanceFilter filter = new InstanceFilter<>(List.of("First", "Second"), null); match(filter, "Second"); doNotMatch(filter, "foo"); } @Test void excludesFilter() { - InstanceFilter filter = new InstanceFilter<>( - null, asList("First", "Second"), true); + InstanceFilter filter = new InstanceFilter<>(null, List.of("First", "Second")); doNotMatch(filter, "Second"); match(filter, "foo"); } @Test void includesAndExcludesFilters() { - InstanceFilter filter = new InstanceFilter<>( - asList("foo", "Bar"), asList("First", "Second"), true); + InstanceFilter filter = new InstanceFilter<>(List.of("foo", "Bar"), List.of("First", "Second")); doNotMatch(filter, "Second"); match(filter, "foo"); } @Test void includesAndExcludesFiltersConflict() { - InstanceFilter filter = new InstanceFilter<>( - List.of("First"), List.of("First"), true); + InstanceFilter filter = new InstanceFilter<>(List.of("First"), List.of("First")); doNotMatch(filter, "First"); } - private void match(InstanceFilter filter, T candidate) { + + private static void match(InstanceFilter filter, T candidate) { assertThat(filter.match(candidate)).as("filter '" + filter + "' should match " + candidate).isTrue(); } - private void doNotMatch(InstanceFilter filter, T candidate) { + private static void doNotMatch(InstanceFilter filter, T candidate) { assertThat(filter.match(candidate)).as("filter '" + filter + "' should not match " + candidate).isFalse(); }