From ea30c8fb5b3018b28746d0096640e2ab0175adf1 Mon Sep 17 00:00:00 2001 From: Bernie Schelberg Date: Tue, 24 Oct 2023 17:08:13 +1000 Subject: [PATCH 1/2] Return consistent collection type for matrix variables See gh-31483 --- .../MatrixVariableMapMethodArgumentResolver.java | 2 +- ...atrixVariablesMapMethodArgumentResolverTests.java | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/MatrixVariableMapMethodArgumentResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/MatrixVariableMapMethodArgumentResolver.java index 313e1b9cd1..13d0446fea 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/MatrixVariableMapMethodArgumentResolver.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/MatrixVariableMapMethodArgumentResolver.java @@ -69,7 +69,7 @@ public class MatrixVariableMapMethodArgumentResolver implements HandlerMethodArg HandlerMapping.MATRIX_VARIABLES_ATTRIBUTE, RequestAttributes.SCOPE_REQUEST); if (CollectionUtils.isEmpty(matrixVariables)) { - return Collections.emptyMap(); + return (isSingleValueMap(parameter) ? Collections.emptyMap() : new LinkedMultiValueMap<>(0)); } MultiValueMap map = new LinkedMultiValueMap<>(); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MatrixVariablesMapMethodArgumentResolverTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MatrixVariablesMapMethodArgumentResolverTests.java index b33e15cf1c..95d21fd05e 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MatrixVariablesMapMethodArgumentResolverTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MatrixVariablesMapMethodArgumentResolverTests.java @@ -155,6 +155,18 @@ public class MatrixVariablesMapMethodArgumentResolverTests { 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); + + //noinspection unchecked + assertThat(result).isInstanceOfSatisfying(MultiValueMap.class, map -> assertThat(map).isEmpty()); + } + @Test public void resolveArgumentNoMatch() throws Exception { MultiValueMap params2 = getVariablesFor("planes"); From 9aa707ec4b04fe5a27d93143a42bec8cd30c7666 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Nicoll?= Date: Tue, 24 Oct 2023 10:49:41 +0200 Subject: [PATCH 2/2] Polish "Return consistent collection type for matrix variables" See gh-31483 --- ...trixVariableMapMethodArgumentResolver.java | 19 ++++++++++-------- ...riablesMapMethodArgumentResolverTests.java | 16 ++++++++++++++- ...trixVariableMapMethodArgumentResolver.java | 20 +++++++++++-------- ...riablesMapMethodArgumentResolverTests.java | 7 ++++--- 4 files changed, 42 insertions(+), 20 deletions(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/MatrixVariableMapMethodArgumentResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/MatrixVariableMapMethodArgumentResolver.java index bd2ff501bd..a9d179d0aa 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/MatrixVariableMapMethodArgumentResolver.java +++ b/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,12 +70,17 @@ public class MatrixVariableMapMethodArgumentResolver extends HandlerMethodArgume Map> matrixVariables = exchange.getAttribute(HandlerMapping.MATRIX_VARIABLES_ATTRIBUTE); + MultiValueMap map = mapMatrixVariables(parameter, matrixVariables); + return (isSingleValueMap(parameter) ? map.toSingleValueMap() : map); + } - if (CollectionUtils.isEmpty(matrixVariables)) { - return Collections.emptyMap(); - } + private MultiValueMap mapMatrixVariables(MethodParameter parameter, + @Nullable Map> matrixVariables) { MultiValueMap 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(); @@ -84,7 +88,7 @@ public class MatrixVariableMapMethodArgumentResolver extends HandlerMethodArgume if (!pathVariable.equals(ValueConstants.DEFAULT_NONE)) { MultiValueMap mapForPathVariable = matrixVariables.get(pathVariable); if (mapForPathVariable == null) { - return Collections.emptyMap(); + return map; } map.putAll(mapForPathVariable); } @@ -97,8 +101,7 @@ public class MatrixVariableMapMethodArgumentResolver extends HandlerMethodArgume }); } } - - return (isSingleValueMap(parameter) ? map.toSingleValueMap() : map); + return map; } private boolean isSingleValueMap(MethodParameter parameter) { diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/MatrixVariablesMapMethodArgumentResolverTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/MatrixVariablesMapMethodArgumentResolverTests.java index a6dfd8554b..5cc897ce32 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/MatrixVariablesMapMethodArgumentResolverTests.java +++ b/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.Collections; 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 class MatrixVariablesMapMethodArgumentResolverTests { 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 { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/MatrixVariableMapMethodArgumentResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/MatrixVariableMapMethodArgumentResolver.java index 13d0446fea..c60de4a425 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/MatrixVariableMapMethodArgumentResolver.java +++ b/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,11 +67,17 @@ public class MatrixVariableMapMethodArgumentResolver implements HandlerMethodArg (Map>) request.getAttribute( HandlerMapping.MATRIX_VARIABLES_ATTRIBUTE, RequestAttributes.SCOPE_REQUEST); - if (CollectionUtils.isEmpty(matrixVariables)) { - return (isSingleValueMap(parameter) ? Collections.emptyMap() : new LinkedMultiValueMap<>(0)); - } + MultiValueMap map = mapMatrixVariables(parameter, matrixVariables); + return (isSingleValueMap(parameter) ? map.toSingleValueMap() : map); + } + + private MultiValueMap mapMatrixVariables(MethodParameter parameter, + @Nullable Map> matrixVariables) { MultiValueMap 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(); @@ -80,7 +85,7 @@ public class MatrixVariableMapMethodArgumentResolver implements HandlerMethodArg if (!pathVariable.equals(ValueConstants.DEFAULT_NONE)) { MultiValueMap mapForPathVariable = matrixVariables.get(pathVariable); if (mapForPathVariable == null) { - return Collections.emptyMap(); + return map; } map.putAll(mapForPathVariable); } @@ -93,8 +98,7 @@ public class MatrixVariableMapMethodArgumentResolver implements HandlerMethodArg }); } } - - return (isSingleValueMap(parameter) ? map.toSingleValueMap() : map); + return map; } private boolean isSingleValueMap(MethodParameter parameter) { diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MatrixVariablesMapMethodArgumentResolverTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MatrixVariablesMapMethodArgumentResolverTests.java index 95d21fd05e..5555121256 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MatrixVariablesMapMethodArgumentResolverTests.java +++ b/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.Collections; 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; @@ -163,8 +164,8 @@ public class MatrixVariablesMapMethodArgumentResolverTests { Object result = this.resolver.resolveArgument(param, this.mavContainer, this.webRequest, null); - //noinspection unchecked - assertThat(result).isInstanceOfSatisfying(MultiValueMap.class, map -> assertThat(map).isEmpty()); + assertThat(result).isInstanceOf(MultiValueMap.class) + .asInstanceOf(InstanceOfAssertFactories.MAP).isEmpty(); } @Test