From 39d4d2041a2d40e94f62fcbcc8ac0151c0fc462f Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Fri, 9 Dec 2022 10:24:42 +0000 Subject: [PATCH] Polishing contribution Closes gh-29634 --- .../RequestMappingInfoHandlerMapping.java | 16 +++++----------- .../RequestMappingInfoHandlerMappingTests.java | 6 +++--- .../RequestMappingInfoHandlerMapping.java | 17 ++++++----------- .../RequestMappingInfoHandlerMappingTests.java | 11 ++++++----- 4 files changed, 20 insertions(+), 30 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 49570506759d..f4b8a60a6dfc 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 @@ -170,8 +170,11 @@ protected void handleMatch(RequestMappingInfo info, HandlerMethod handlerMethod, protected HandlerMethod handleNoMatch(Set infos, ServerWebExchange exchange) throws Exception { - PartialMatchHelper helper = PartialMatchHelper.from(infos, exchange); + if (CollectionUtils.isEmpty(infos)) { + return null; + } + PartialMatchHelper helper = new PartialMatchHelper(infos, exchange); if (helper.isEmpty()) { return null; } @@ -222,12 +225,10 @@ protected HandlerMethod handleNoMatch(Set infos, */ private static final class PartialMatchHelper { - private static final PartialMatchHelper EMPTY_HELPER = new PartialMatchHelper(Collections.emptySet(), null); - private final List partialMatches = new ArrayList<>(); - private PartialMatchHelper(Set infos, ServerWebExchange exchange) { + PartialMatchHelper(Set infos, ServerWebExchange exchange) { for (RequestMappingInfo info : infos) { if (info.getPatternsCondition().getMatchingCondition(exchange) != null) { this.partialMatches.add(new PartialMatch(info, exchange)); @@ -235,13 +236,6 @@ private PartialMatchHelper(Set infos, ServerWebExchange exch } } - 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 1fd5babe236b..f3d12370a03c 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 @@ -346,9 +346,9 @@ public void handlePatchUnsupportedMediaType() { } - @Test - public void handleNoMatchEmptyRequestMappingInfo() throws Exception { - ServerWebExchange exchange = MockServerWebExchange.from(post("/bar")); + @Test // gh-29611 + public void handleNoMatchWithoutPartialMatches() throws Exception { + ServerWebExchange exchange = MockServerWebExchange.from(post("/non-existent")); HandlerMethod handlerMethod = this.handlerMapping.handleNoMatch(new HashSet<>(), exchange); assertThat(handlerMethod).isNull(); 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 c576725fa2c2..ec92c8abd26d 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,11 @@ private Map> extractMatrixVariables( protected HandlerMethod handleNoMatch( Set infos, String lookupPath, HttpServletRequest request) throws ServletException { - PartialMatchHelper helper = PartialMatchHelper.from(infos, request); + if (CollectionUtils.isEmpty(infos)) { + return null; + } + + PartialMatchHelper helper = new PartialMatchHelper(infos, request); if (helper.isEmpty()) { return null; } @@ -295,11 +299,9 @@ protected HandlerMethod handleNoMatch( */ private static final class PartialMatchHelper { - private static final PartialMatchHelper EMPTY_HELPER = new PartialMatchHelper(Collections.emptySet(), null); - private final List partialMatches = new ArrayList<>(); - private PartialMatchHelper(Set infos, HttpServletRequest request) { + PartialMatchHelper(Set infos, HttpServletRequest request) { for (RequestMappingInfo info : infos) { if (info.getActivePatternsCondition().getMatchingCondition(request) != null) { this.partialMatches.add(new PartialMatch(info, request)); @@ -307,13 +309,6 @@ private PartialMatchHelper(Set infos, HttpServletRequest req } } - 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 33dbfa50cef8..b77fe1fe76b2 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 @@ -398,14 +398,15 @@ 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"); + @PathPatternsParameterizedTest // gh-29611 + void handleNoMatchWithoutPartialMatches(TestRequestMappingInfoHandlerMapping mapping) throws ServletException { + String path = "/non-existent"; + MockHttpServletRequest request = new MockHttpServletRequest("GET", path); - HandlerMethod handlerMethod = mapping.handleNoMatch(new HashSet<>(), "/{cars}", request); + HandlerMethod handlerMethod = mapping.handleNoMatch(new HashSet<>(), path, request); assertThat(handlerMethod).isNull(); - handlerMethod = mapping.handleNoMatch(null, "/{cars}", request); + handlerMethod = mapping.handleNoMatch(null, path, request); assertThat(handlerMethod).isNull(); }