Merge pull request #31483 from bernie-schelberg-invicara

* pr/31483:
  Polish "Return consistent collection type for matrix variables"
  Return consistent collection type for matrix variables

Closes gh-31483

Author: snicoll

spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/MatrixVariableMapMethodArgumentResolver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2023 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.
@@ -16,7 +16,6 @@
 
 package org.springframework.web.reactive.result.method.annotation;
 
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
@@ -71,20 +70,25 @@ public Object resolveArgumentValue(MethodParameter parameter, BindingContext bin
 
 		Map<String, MultiValueMap<String, String>> matrixVariables =
 				exchange.getAttribute(HandlerMapping.MATRIX_VARIABLES_ATTRIBUTE);
+		MultiValueMap<String, String> map = mapMatrixVariables(parameter, matrixVariables);
+		return (isSingleValueMap(parameter) ? map.toSingleValueMap() : map);
+	}
 
-		if (CollectionUtils.isEmpty(matrixVariables)) {
-			return Collections.emptyMap();
-		}
+	private MultiValueMap<String,String> mapMatrixVariables(MethodParameter parameter,
+			@Nullable Map<String, MultiValueMap<String, String>> matrixVariables) {
 
 		MultiValueMap<String, String> map = new LinkedMultiValueMap<>();
+		if (CollectionUtils.isEmpty(matrixVariables)) {
+			return map;
+		}
 		MatrixVariable annotation = parameter.getParameterAnnotation(MatrixVariable.class);
 		Assert.state(annotation != null, "No MatrixVariable annotation");
 		String pathVariable = annotation.pathVar();
 
 		if (!pathVariable.equals(ValueConstants.DEFAULT_NONE)) {
 			MultiValueMap<String, String> mapForPathVariable = matrixVariables.get(pathVariable);
 			if (mapForPathVariable == null) {
-				return Collections.emptyMap();
+				return map;
 			}
 			map.putAll(mapForPathVariable);
 		}
@@ -97,8 +101,7 @@ public Object resolveArgumentValue(MethodParameter parameter, BindingContext bin
 				});
 			}
 		}
-
-		return (isSingleValueMap(parameter) ? map.toSingleValueMap() : map);
+		return map;
 	}
 
 	private boolean isSingleValueMap(MethodParameter parameter) {

spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/MatrixVariablesMapMethodArgumentResolverTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2023 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.
@@ -22,6 +22,7 @@
 import java.util.LinkedHashMap;
 import java.util.Map;
 
+import org.assertj.core.api.InstanceOfAssertFactories;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -139,6 +140,19 @@ public void resolveArgumentPathVariable() throws Exception {
 		assertThat(mapAll.get("colors")).isEqualTo("red");
 	}
 
+	@Test
+	public void resolveMultiValueMapArgumentNoParams() {
+
+		MethodParameter param = this.testMethod.annot(matrixAttribute().noPathVar())
+				.arg(MultiValueMap.class, String.class, String.class);
+
+		Object result = this.resolver.resolveArgument(param,
+				new BindingContext(), this.exchange).block(Duration.ZERO);
+
+		assertThat(result).isInstanceOf(MultiValueMap.class)
+				.asInstanceOf(InstanceOfAssertFactories.MAP).isEmpty();
+	}
+
 	@Test
 	public void resolveArgumentNoParams() throws Exception {
 

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/MatrixVariableMapMethodArgumentResolver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2023 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.
@@ -16,7 +16,6 @@
 
 package org.springframework.web.servlet.mvc.method.annotation;
 
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
@@ -68,19 +67,25 @@ public Object resolveArgument(MethodParameter parameter, @Nullable ModelAndViewC
 				(Map<String, MultiValueMap<String, String>>) request.getAttribute(
 						HandlerMapping.MATRIX_VARIABLES_ATTRIBUTE, RequestAttributes.SCOPE_REQUEST);
 
-		if (CollectionUtils.isEmpty(matrixVariables)) {
-			return Collections.emptyMap();
-		}
+		MultiValueMap<String, String> map = mapMatrixVariables(parameter, matrixVariables);
+		return (isSingleValueMap(parameter) ? map.toSingleValueMap() : map);
+	}
+
+	private MultiValueMap<String,String> mapMatrixVariables(MethodParameter parameter,
+			@Nullable Map<String, MultiValueMap<String, String>> matrixVariables) {
 
 		MultiValueMap<String, String> map = new LinkedMultiValueMap<>();
+		if (CollectionUtils.isEmpty(matrixVariables)) {
+			return map;
+		}
 		MatrixVariable ann = parameter.getParameterAnnotation(MatrixVariable.class);
 		Assert.state(ann != null, "No MatrixVariable annotation");
 		String pathVariable = ann.pathVar();
 
 		if (!pathVariable.equals(ValueConstants.DEFAULT_NONE)) {
 			MultiValueMap<String, String> mapForPathVariable = matrixVariables.get(pathVariable);
 			if (mapForPathVariable == null) {
-				return Collections.emptyMap();
+				return map;
 			}
 			map.putAll(mapForPathVariable);
 		}
@@ -93,8 +98,7 @@ public Object resolveArgument(MethodParameter parameter, @Nullable ModelAndViewC
 				});
 			}
 		}
-
-		return (isSingleValueMap(parameter) ? map.toSingleValueMap() : map);
+		return map;
 	}
 
 	private boolean isSingleValueMap(MethodParameter parameter) {

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MatrixVariablesMapMethodArgumentResolverTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2023 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,6 +21,7 @@
 import java.util.LinkedHashMap;
 import java.util.Map;
 
+import org.assertj.core.api.InstanceOfAssertFactories;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -155,6 +156,18 @@ public void resolveArgumentNoParams() throws Exception {
 		assertThat(map).isEqualTo(Collections.emptyMap());
 	}
 
+	@Test
+	public void resolveMultiValueMapArgumentNoParams() throws Exception {
+
+		MethodParameter param = this.testMethod.annot(matrixAttribute().noPathVar())
+				.arg(MultiValueMap.class, String.class, String.class);
+
+		Object result = this.resolver.resolveArgument(param, this.mavContainer, this.webRequest, null);
+
+		assertThat(result).isInstanceOf(MultiValueMap.class)
+				.asInstanceOf(InstanceOfAssertFactories.MAP).isEmpty();
+	}
+
 	@Test
 	public void resolveArgumentNoMatch() throws Exception {
 		MultiValueMap<String, String> params2 = getVariablesFor("planes");