Skip to content
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.

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

Optimize object creation in RequestMappingHandlerMapping#handleNoMatch #29634

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.springframework.http.server.PathContainer;
import org.springframework.http.server.reactive.ServerHttpRequest;
import org.springframework.util.Assert;
import org.springframework.util.CollectionUtils;
import org.springframework.util.MultiValueMap;
import org.springframework.web.bind.annotation.RequestMethod;
import org.springframework.web.filter.reactive.ServerHttpObservationFilter;
Expand Down Expand Up @@ -169,7 +170,7 @@ protected void handleMatch(RequestMappingInfo info, HandlerMethod handlerMethod,
protected HandlerMethod handleNoMatch(Set<RequestMappingInfo> infos,
ServerWebExchange exchange) throws Exception {

PartialMatchHelper helper = new PartialMatchHelper(infos, exchange);
PartialMatchHelper helper = PartialMatchHelper.from(infos, exchange);

if (helper.isEmpty()) {
return null;
Expand Down Expand Up @@ -219,17 +220,27 @@ protected HandlerMethod handleNoMatch(Set<RequestMappingInfo> infos,
/**
* Aggregate all partial matches and expose methods checking across them.
*/
private static class PartialMatchHelper {
private static final class PartialMatchHelper {

private static final PartialMatchHelper EMPTY_HELPER = new PartialMatchHelper(Collections.emptySet(), null);

private final List<PartialMatch> partialMatches = new ArrayList<>();


public PartialMatchHelper(Set<RequestMappingInfo> infos, ServerWebExchange exchange) {
this.partialMatches.addAll(infos.stream()
.filter(info -> info.getPatternsCondition().getMatchingCondition(exchange) != null)
.map(info -> new PartialMatch(info, exchange)).toList());
private PartialMatchHelper(Set<RequestMappingInfo> infos, ServerWebExchange exchange) {
for (RequestMappingInfo info : infos) {
if (info.getPatternsCondition().getMatchingCondition(exchange) != null) {
this.partialMatches.add(new PartialMatch(info, exchange));
}
}
}

public static PartialMatchHelper from(Set<RequestMappingInfo> infos, ServerWebExchange exchange) {
if (CollectionUtils.isEmpty(infos)) {
return EMPTY_HELPER;
}
return new PartialMatchHelper(infos, exchange);
}

/**
* Whether there are any partial matches.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -345,6 +346,17 @@ public void handlePatchUnsupportedMediaType() {

}

@Test
public void handleNoMatchEmptyRequestMappingInfo() throws Exception {
ServerWebExchange exchange = MockServerWebExchange.from(post("/bar"));

HandlerMethod handlerMethod = this.handlerMapping.handleNoMatch(new HashSet<>(), exchange);
assertThat(handlerMethod).isNull();

handlerMethod = this.handlerMapping.handleNoMatch(null, exchange);
assertThat(handlerMethod).isNull();
}


@SuppressWarnings("unchecked")
private <T> void assertError(Mono<Object> mono, final Class<T> exceptionClass, final Consumer<T> consumer) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ private Map<String, MultiValueMap<String, String>> extractMatrixVariables(
protected HandlerMethod handleNoMatch(
Set<RequestMappingInfo> infos, String lookupPath, HttpServletRequest request) throws ServletException {

PartialMatchHelper helper = new PartialMatchHelper(infos, request);
PartialMatchHelper helper = PartialMatchHelper.from(infos, request);
if (helper.isEmpty()) {
return null;
}
Expand Down Expand Up @@ -293,18 +293,27 @@ protected HandlerMethod handleNoMatch(
/**
* Aggregate all partial matches and expose methods checking across them.
*/
private static class PartialMatchHelper {
private static final class PartialMatchHelper {

private static final PartialMatchHelper EMPTY_HELPER = new PartialMatchHelper(Collections.emptySet(), null);

private final List<PartialMatch> partialMatches = new ArrayList<>();

public PartialMatchHelper(Set<RequestMappingInfo> infos, HttpServletRequest request) {
private PartialMatchHelper(Set<RequestMappingInfo> infos, HttpServletRequest request) {
for (RequestMappingInfo info : infos) {
if (info.getActivePatternsCondition().getMatchingCondition(request) != null) {
this.partialMatches.add(new PartialMatch(info, request));
}
}
}

public static PartialMatchHelper from(Set<RequestMappingInfo> infos, HttpServletRequest request) {
if (CollectionUtils.isEmpty(infos)) {
return EMPTY_HELPER;
}
return new PartialMatchHelper(infos, request);
}

/**
* Whether there are any partial matches.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServletRequest;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -397,6 +398,17 @@ void handleMatchMatrixVariablesDecoding(TestRequestMappingInfoHandlerMapping map
assertThat(uriVariables.get("cars")).isEqualTo("cars");
}

@PathPatternsParameterizedTest
void handleNoMatchEmptyRequestMappingInfo(TestRequestMappingInfoHandlerMapping mapping) throws ServletException {
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/cars;color=green");

HandlerMethod handlerMethod = mapping.handleNoMatch(new HashSet<>(), "/{cars}", request);
assertThat(handlerMethod).isNull();

handlerMethod = mapping.handleNoMatch(null, "/{cars}", request);
assertThat(handlerMethod).isNull();
}

private HandlerMethod getHandler(
TestRequestMappingInfoHandlerMapping mapping, MockHttpServletRequest request) throws Exception {

Expand Down