From 5ac97b16a83fe32439adbe4852a9fb9b6254efb6 Mon Sep 17 00:00:00 2001 From: "koo.taejin" Date: Sun, 4 Dec 2022 15:18:22 +0900 Subject: [PATCH] Optimize object creation PartialMatchHelper See gh-29634 --- .../RequestMappingInfoHandlerMapping.java | 23 ++++++++++++++----- ...RequestMappingInfoHandlerMappingTests.java | 12 ++++++++++ .../RequestMappingInfoHandlerMapping.java | 15 +++++++++--- ...RequestMappingInfoHandlerMappingTests.java | 12 ++++++++++ 4 files changed, 53 insertions(+), 9 deletions(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMapping.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMapping.java index a12fa48cdd6e..49570506759d 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMapping.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMapping.java @@ -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; @@ -169,7 +170,7 @@ protected void handleMatch(RequestMappingInfo info, HandlerMethod handlerMethod, protected HandlerMethod handleNoMatch(Set infos, ServerWebExchange exchange) throws Exception { - PartialMatchHelper helper = new PartialMatchHelper(infos, exchange); + PartialMatchHelper helper = PartialMatchHelper.from(infos, exchange); if (helper.isEmpty()) { return null; @@ -219,17 +220,27 @@ protected HandlerMethod handleNoMatch(Set 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 partialMatches = new ArrayList<>(); - public PartialMatchHelper(Set 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 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 infos, ServerWebExchange exchange) { + if (CollectionUtils.isEmpty(infos)) { + return EMPTY_HELPER; + } + return new PartialMatchHelper(infos, exchange); + } /** * Whether there are any partial matches. diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMappingTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMappingTests.java index 2628e361db14..1fd5babe236b 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMappingTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMappingTests.java @@ -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; @@ -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 void assertError(Mono mono, final Class exceptionClass, final Consumer consumer) { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMapping.java index 362f98314855..c576725fa2c2 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMapping.java @@ -246,7 +246,7 @@ private Map> extractMatrixVariables( protected HandlerMethod handleNoMatch( Set infos, String lookupPath, HttpServletRequest request) throws ServletException { - PartialMatchHelper helper = new PartialMatchHelper(infos, request); + PartialMatchHelper helper = PartialMatchHelper.from(infos, request); if (helper.isEmpty()) { return null; } @@ -293,11 +293,13 @@ 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 partialMatches = new ArrayList<>(); - public PartialMatchHelper(Set infos, HttpServletRequest request) { + private PartialMatchHelper(Set infos, HttpServletRequest request) { for (RequestMappingInfo info : infos) { if (info.getActivePatternsCondition().getMatchingCondition(request) != null) { this.partialMatches.add(new PartialMatch(info, request)); @@ -305,6 +307,13 @@ public PartialMatchHelper(Set infos, HttpServletRequest requ } } + public static PartialMatchHelper from(Set infos, HttpServletRequest request) { + if (CollectionUtils.isEmpty(infos)) { + return EMPTY_HELPER; + } + return new PartialMatchHelper(infos, request); + } + /** * Whether there are any partial matches. */ diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java index d5b84d4b1b31..33dbfa50cef8 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java @@ -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; @@ -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 {