Skip to content

Remove MatchableHandlerMapping from DelegatingHandlerMapping #2604

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
arimu1 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Remove MatchableHandlerMapping from DelegatingHandlerMapping #2604

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 10 additions & 34 deletions ...c/src/main/java/org/springframework/data/rest/webmvc/config/DelegatingHandlerMapping.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@
import org.springframework.web.bind.UnsatisfiedServletRequestParameterException;
import org.springframework.web.servlet.HandlerExecutionChain;
import org.springframework.web.servlet.HandlerMapping;
import org.springframework.web.servlet.handler.MatchableHandlerMapping;
import org.springframework.web.servlet.handler.RequestMatchResult;
import org.springframework.web.util.pattern.PathPatternParser;

/**
Expand All @@ -42,7 +40,7 @@
* @author Oliver Gierke

@ruthst00 ruthst00 Sep 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add yourself as additional author

* @soundtrack Benny Greb - Stabila (Moving Parts)
*/
class DelegatingHandlerMapping implements MatchableHandlerMapping, Iterable<HandlerMapping>, Ordered {
class DelegatingHandlerMapping implements HandlerMapping, Iterable<HandlerMapping>, Ordered {

private final List<HandlerMapping> delegates;
private final @Nullable PathPatternParser parser;
Expand All @@ -66,7 +64,6 @@ public boolean usesPathPatterns() {
}

@Nullable
@Override
public PathPatternParser getPatternParser() {

@ruthst00 ruthst00 Sep 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider removing or making getPatternParser() package-private since it's no longer part of any interface.

return parser;
}
Expand All @@ -91,20 +88,9 @@ public int getOrder() {
return HandlerSelectionResult.from(request, delegates).resultOrException();
}

@Override
public @Nullable RequestMatchResult match(HttpServletRequest request, String pattern) {

try {
return HandlerSelectionResult.from(request, delegates).match(pattern);
} catch (Exception o_O) {
return null;
}
}

private static class HandlerSelectionResult {

private final HttpServletRequest request;
private final @Nullable HandlerMapping mapping;
private final @Nullable HandlerExecutionChain result;
private final @Nullable Exception ignoredException;

Expand All @@ -120,7 +106,7 @@ public static HandlerSelectionResult from(HttpServletRequest request, Iterable<H
HandlerExecutionChain result = delegate.getHandler(request);

if (result != null) {
return HandlerSelectionResult.forResult(request, delegate, result);
return HandlerSelectionResult.forResult(request, result);
}

} catch (HttpMediaTypeNotSupportedException o_O) {
Expand All @@ -137,13 +123,12 @@ public static HandlerSelectionResult from(HttpServletRequest request, Iterable<H
return HandlerSelectionResult.withoutResult(request, ignoredException);
}

private static HandlerSelectionResult forResult(HttpServletRequest request, HandlerMapping delegate,
HandlerExecutionChain result) {
return new HandlerSelectionResult(request, delegate, result, null);
private static HandlerSelectionResult forResult(HttpServletRequest request, HandlerExecutionChain result) {
return new HandlerSelectionResult(request, result, null);
}

private static HandlerSelectionResult withoutResult(HttpServletRequest request, @Nullable Exception exception) {
return new HandlerSelectionResult(request, null, null, exception);
return new HandlerSelectionResult(request, null, exception);
}

public @Nullable HandlerExecutionChain resultOrException() throws Exception {
Expand All @@ -155,20 +140,12 @@ private static HandlerSelectionResult withoutResult(HttpServletRequest request,
return result;
}

public @Nullable RequestMatchResult match(String pattern) {

return MatchableHandlerMapping.class.isInstance(mapping) //
? ((MatchableHandlerMapping) mapping).match(request, pattern) //
: null;
}

public HandlerSelectionResult(HttpServletRequest request, @Nullable HandlerMapping mapping,
@Nullable HandlerExecutionChain result, @Nullable Exception ignoredException) {
public HandlerSelectionResult(HttpServletRequest request, @Nullable HandlerExecutionChain result,
@Nullable Exception ignoredException) {

Assert.notNull(request, "HttpServletRequest must not be null");

this.request = request;
this.mapping = mapping;
this.result = result;
this.ignoredException = ignoredException;
}
Expand All @@ -187,20 +164,19 @@ public boolean equals(@Nullable Object o) {
HandlerSelectionResult other = (HandlerSelectionResult) o;

return Objects.equals(request, other.request) //
&& Objects.equals(mapping, other.mapping) //
&& Objects.equals(result, other.result) //
&& Objects.equals(ignoredException, other.ignoredException);
}

@Override
public int hashCode() {
return Objects.hash(request, mapping, result, ignoredException);
return Objects.hash(request, result, ignoredException);
}

@Override
public java.lang.String toString() {
return "DelegatingHandlerMapping.HandlerSelectionResult(request=" + request + ", mapping=" + mapping + ", result="
+ result + ", ignoredException=" + ignoredException + ")";
return "DelegatingHandlerMapping.HandlerSelectionResult(request=" + request + ", result=" + result
+ ", ignoredException=" + ignoredException + ")";
}
}
}
22 changes: 0 additions & 22 deletions ...t/java/org/springframework/data/rest/webmvc/config/DelegatingHandlerMappingUnitTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package org.springframework.data.rest.webmvc.config;

import static org.assertj.core.api.Assertions.*;
import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.*;

import jakarta.servlet.http.HttpServletRequest;
Expand All @@ -25,16 +24,13 @@

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Answers;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.web.HttpMediaTypeNotAcceptableException;
import org.springframework.web.HttpMediaTypeNotSupportedException;
import org.springframework.web.HttpRequestMethodNotSupportedException;
import org.springframework.web.bind.UnsatisfiedServletRequestParameterException;
import org.springframework.web.servlet.HandlerMapping;
import org.springframework.web.servlet.handler.MatchableHandlerMapping;
import org.springframework.web.servlet.handler.RequestMatchResult;

/**
* Unit tests for {@link DelegatingHandlerMapping}.
Expand All @@ -59,24 +55,6 @@ void consultsAllHandlerMappingsAndThrowsExceptionEventually() throws Exception {
assertHandlerTriedButExceptionThrown(mapping, HttpMediaTypeNotSupportedException.class); // DATAREST-1387
}

@Test // DATAREST-1193
void exposesMatchabilityOfSelectedMapping() {

// Given:
// A matching mapping that doesn't get selected
MatchableHandlerMapping third = mock(MatchableHandlerMapping.class, withSettings().lenient());
doReturn(mock(RequestMatchResult.class)).when(third).match(any(), any(String.class));

// A matching mapping that gets selected
RequestMatchResult result = mock(RequestMatchResult.class);
MatchableHandlerMapping fourth = mock(MatchableHandlerMapping.class, Answers.RETURNS_MOCKS);
doReturn(result).when(fourth).match(any(), any(String.class));

DelegatingHandlerMapping mapping = new DelegatingHandlerMapping(Arrays.asList(first, second, third, fourth), null);

assertThat(mapping.match(request, "somePattern")).isEqualTo(result);
}

private final void assertHandlerTriedButExceptionThrown(HandlerMapping mapping, Class<? extends Exception> type)
throws Exception {

Expand Down
Loading