Set best matching pattern attribute in WebMvc.fn

Prior to this commit, the `RouterFunctionMapping` WebFlux.fn variant
would set the `HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE` as an
exchange attribute. This is useful for instrumentation purposes.
The WebMvc.fn variant would not do the same; this would lead to
"UNKNOWN" path metrics tags.

This commit ensures that the `RouterFunctionMapping` WebMvc.fn variant
does set the `BEST_MATCHING_PATTERN_ATTRIBUTE` and
`BEST_MATCHING_HANDLER_ATTRIBUTE` request attributes.

Closes gh-26963

spring-webflux/src/test/java/org/springframework/web/reactive/function/server/support/RouterFunctionMappingTests.java

 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2021 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -21,33 +21,37 @@ import reactor.core.publisher.Mono;
 import reactor.test.StepVerifier;
 
 import org.springframework.http.codec.ServerCodecConfigurer;
+import org.springframework.web.reactive.HandlerMapping;
 import org.springframework.web.reactive.function.server.HandlerFunction;
 import org.springframework.web.reactive.function.server.RouterFunction;
 import org.springframework.web.reactive.function.server.RouterFunctions;
+import org.springframework.web.reactive.function.server.ServerRequest;
 import org.springframework.web.reactive.function.server.ServerResponse;
 import org.springframework.web.server.ServerWebExchange;
 import org.springframework.web.testfixture.http.server.reactive.MockServerHttpRequest;
 import org.springframework.web.testfixture.server.MockServerWebExchange;
+import org.springframework.web.util.pattern.PathPattern;
+
+import static org.assertj.core.api.Assertions.assertThat;
 
 /**
  * @author Arjen Poutsma
+ * @author Brian Clozel
  */
 public class RouterFunctionMappingTests {
 
-	private final ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("https://example.com/match"));
-
 	private final ServerCodecConfigurer codecConfigurer = ServerCodecConfigurer.create();
 
 
 	@Test
-	public void normal() {
+	void normal() {
 		HandlerFunction<ServerResponse> handlerFunction = request -> ServerResponse.ok().build();
 		RouterFunction<ServerResponse> routerFunction = request -> Mono.just(handlerFunction);
 
 		RouterFunctionMapping mapping = new RouterFunctionMapping(routerFunction);
 		mapping.setMessageReaders(this.codecConfigurer.getReaders());
 
-		Mono<Object> result = mapping.getHandler(this.exchange);
+		Mono<Object> result = mapping.getHandler(createExchange("https://example.com/match"));
 
 		StepVerifier.create(result)
 				.expectNext(handlerFunction)
@@ -56,12 +60,12 @@ public class RouterFunctionMappingTests {
 	}
 
 	@Test
-	public void noMatch() {
+	void noMatch() {
 		RouterFunction<ServerResponse> routerFunction = request -> Mono.empty();
 		RouterFunctionMapping mapping = new RouterFunctionMapping(routerFunction);
 		mapping.setMessageReaders(this.codecConfigurer.getReaders());
 
-		Mono<Object> result = mapping.getHandler(this.exchange);
+		Mono<Object> result = mapping.getHandler(createExchange("https://example.com/match"));
 
 		StepVerifier.create(result)
 				.expectComplete()
@@ -69,7 +73,7 @@ public class RouterFunctionMappingTests {
 	}
 
 	@Test
-	public void changeParser() throws Exception {
+	void changeParser() throws Exception {
 		HandlerFunction<ServerResponse> handlerFunction = request -> ServerResponse.ok().build();
 		RouterFunction<ServerResponse> routerFunction = RouterFunctions.route()
 				.GET("/foo", handlerFunction)
@@ -90,4 +94,36 @@ public class RouterFunctionMappingTests {
 
 	}
 
+	@Test
+	void mappedRequestShouldHoldAttributes() {
+		HandlerFunction<ServerResponse> handlerFunction = request -> ServerResponse.ok().build();
+		RouterFunction<ServerResponse> routerFunction = RouterFunctions.route()
+				.GET("/match", handlerFunction).build();
+
+		RouterFunctionMapping mapping = new RouterFunctionMapping(routerFunction);
+		mapping.setMessageReaders(this.codecConfigurer.getReaders());
+
+		ServerWebExchange exchange = createExchange("https://example.com/match");
+		Mono<Object> result = mapping.getHandler(exchange);
+
+		StepVerifier.create(result)
+				.expectNext(handlerFunction)
+				.expectComplete()
+				.verify();
+
+		PathPattern matchingPattern = exchange.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE);
+		assertThat(matchingPattern).isNotNull();
+		assertThat(matchingPattern.getPatternString()).isEqualTo("/match");
+
+		ServerRequest serverRequest = exchange.getAttribute(RouterFunctions.REQUEST_ATTRIBUTE);
+		assertThat(serverRequest).isNotNull();
+
+		HandlerFunction<?> handler = exchange.getAttribute(HandlerMapping.BEST_MATCHING_HANDLER_ATTRIBUTE);
+		assertThat(handler).isEqualTo(handlerFunction);
+	}
+
+	private ServerWebExchange createExchange(String urlTemplate) {
+		return MockServerWebExchange.from(MockServerHttpRequest.get(urlTemplate));
+	}
+
 }

spring-webmvc/src/main/java/org/springframework/web/servlet/function/support/RouterFunctionMapping.java

@@ -34,10 +34,12 @@ import org.springframework.http.converter.support.AllEncompassingFormHttpMessage
 import org.springframework.http.converter.xml.SourceHttpMessageConverter;
 import org.springframework.lang.Nullable;
 import org.springframework.util.CollectionUtils;
+import org.springframework.web.servlet.function.HandlerFunction;
 import org.springframework.web.servlet.function.RouterFunction;
 import org.springframework.web.servlet.function.RouterFunctions;
 import org.springframework.web.servlet.function.ServerRequest;
 import org.springframework.web.servlet.handler.AbstractHandlerMapping;
+import org.springframework.web.util.pattern.PathPattern;
 import org.springframework.web.util.pattern.PathPatternParser;
 
 /**
@@ -50,6 +52,7 @@ import org.springframework.web.util.pattern.PathPatternParser;
  *
  * @author Arjen Poutsma
  * @author Sebastien Deleuze
+ * @author Brian Clozel
  * @since 5.2
  */
 public class RouterFunctionMapping extends AbstractHandlerMapping implements InitializingBean {
@@ -206,16 +209,23 @@ public class RouterFunctionMapping extends AbstractHandlerMapping implements Ini
 	protected Object getHandlerInternal(HttpServletRequest servletRequest) throws Exception {
 		if (this.routerFunction != null) {
 			ServerRequest request = ServerRequest.create(servletRequest, this.messageConverters);
-			setAttributes(servletRequest, request);
-			return this.routerFunction.route(request).orElse(null);
+			HandlerFunction<?> handlerFunction = this.routerFunction.route(request).orElse(null);
+			setAttributes(servletRequest, request, handlerFunction);
+			return handlerFunction;
 		}
 		else {
 			return null;
 		}
 	}
 
-	private void setAttributes(HttpServletRequest servletRequest, ServerRequest request) {
-		servletRequest.removeAttribute(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE);
+	private void setAttributes(HttpServletRequest servletRequest, ServerRequest request,
+			HandlerFunction<?> handlerFunction) {
+		PathPattern matchingPattern = (PathPattern) servletRequest.getAttribute(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE);
+		if (matchingPattern != null) {
+			servletRequest.removeAttribute(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE);
+			servletRequest.setAttribute(BEST_MATCHING_PATTERN_ATTRIBUTE, matchingPattern.getPatternString());
+		}
+		servletRequest.setAttribute(BEST_MATCHING_HANDLER_ATTRIBUTE, handlerFunction);
 		servletRequest.setAttribute(RouterFunctions.REQUEST_ATTRIBUTE, request);
 	}
 

spring-webmvc/src/test/java/org/springframework/web/servlet/function/support/RouterFunctionMappingTests.java

 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2021 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -24,6 +24,7 @@ import org.junit.jupiter.api.Test;
 
 import org.springframework.http.converter.HttpMessageConverter;
 import org.springframework.web.servlet.HandlerExecutionChain;
+import org.springframework.web.servlet.HandlerMapping;
 import org.springframework.web.servlet.function.HandlerFunction;
 import org.springframework.web.servlet.function.RouterFunction;
 import org.springframework.web.servlet.function.RouterFunctions;
@@ -36,43 +37,42 @@ import static org.assertj.core.api.Assertions.assertThat;
 
 /**
  * @author Arjen Poutsma
+ * @author Brian Clozel
  */
 class RouterFunctionMappingTests {
 
-	private final MockHttpServletRequest request = new MockHttpServletRequest("GET", "/match");
-
 	private List<HttpMessageConverter<?>> messageConverters = Collections.emptyList();
 
 	@Test
-	public void normal() throws Exception {
+	void normal() throws Exception {
 		HandlerFunction<ServerResponse> handlerFunction = request -> ServerResponse.ok().build();
 		RouterFunction<ServerResponse> routerFunction = request -> Optional.of(handlerFunction);
 
 		RouterFunctionMapping mapping = new RouterFunctionMapping(routerFunction);
 		mapping.setMessageConverters(this.messageConverters);
-		ServletRequestPathUtils.parseAndCache(this.request);
 
-		HandlerExecutionChain result = mapping.getHandler(this.request);
+		MockHttpServletRequest request = createTestRequest("/match");
+		HandlerExecutionChain result = mapping.getHandler(request);
 
 		assertThat(result).isNotNull();
 		assertThat(result.getHandler()).isSameAs(handlerFunction);
 	}
 
 	@Test
-	public void noMatch() throws Exception {
+	void noMatch() throws Exception {
 		RouterFunction<ServerResponse> routerFunction = request -> Optional.empty();
 
 		RouterFunctionMapping mapping = new RouterFunctionMapping(routerFunction);
 		mapping.setMessageConverters(this.messageConverters);
-		ServletRequestPathUtils.parseAndCache(this.request);
 
-		HandlerExecutionChain result = mapping.getHandler(this.request);
+		MockHttpServletRequest request = createTestRequest("/match");
+		HandlerExecutionChain result = mapping.getHandler(request);
 
 		assertThat(result).isNull();
 	}
 
 	@Test
-	public void changeParser() throws Exception {
+	void changeParser() throws Exception {
 		HandlerFunction<ServerResponse> handlerFunction = request -> ServerResponse.ok().build();
 		RouterFunction<ServerResponse> routerFunction = RouterFunctions.route()
 				.GET("/foo", handlerFunction)
@@ -86,13 +86,35 @@ class RouterFunctionMappingTests {
 		mapping.setPatternParser(patternParser);
 		mapping.afterPropertiesSet();
 
-		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/FOO");
-		ServletRequestPathUtils.parseAndCache(request);
-
+		MockHttpServletRequest request = createTestRequest("/FOO");
 		HandlerExecutionChain result = mapping.getHandler(request);
 
 		assertThat(result).isNotNull();
 		assertThat(result.getHandler()).isSameAs(handlerFunction);
 	}
 
+	@Test
+	void mappedRequestShouldHoldAttributes() throws Exception {
+		HandlerFunction<ServerResponse> handlerFunction = request -> ServerResponse.ok().build();
+		RouterFunction<ServerResponse> routerFunction = RouterFunctions.route()
+				.GET("/match", handlerFunction)
+				.build();
+
+		RouterFunctionMapping mapping = new RouterFunctionMapping(routerFunction);
+		mapping.setMessageConverters(this.messageConverters);
+
+		MockHttpServletRequest request = createTestRequest("/match");
+		HandlerExecutionChain result = mapping.getHandler(request);
+
+		assertThat(result).isNotNull();
+		assertThat(request.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE)).isEqualTo("/match");
+		assertThat(request.getAttribute(HandlerMapping.BEST_MATCHING_HANDLER_ATTRIBUTE)).isEqualTo(handlerFunction);
+	}
+
+	private MockHttpServletRequest createTestRequest(String path) {
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", path);
+		ServletRequestPathUtils.parseAndCache(request);
+		return request;
+	}
+
 }