Skip to content

Commit 9aa707e

Browse files
committed
Polish "Return consistent collection type for matrix variables"
See gh-31483
1 parent ea30c8f commit 9aa707e

File tree

4 files changed

+42
-20
lines changed

4 files changed

+42
-20
lines changed

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

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2018 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,7 +16,6 @@
1616

1717
package org.springframework.web.reactive.result.method.annotation;
1818

19-
import java.util.Collections;
2019
import java.util.List;
2120
import java.util.Map;
2221

@@ -71,20 +70,25 @@ public Object resolveArgumentValue(MethodParameter parameter, BindingContext bin
7170

7271
Map<String, MultiValueMap<String, String>> matrixVariables =
7372
exchange.getAttribute(HandlerMapping.MATRIX_VARIABLES_ATTRIBUTE);
73+
MultiValueMap<String, String> map = mapMatrixVariables(parameter, matrixVariables);
74+
return (isSingleValueMap(parameter) ? map.toSingleValueMap() : map);
75+
}
7476

75-
if (CollectionUtils.isEmpty(matrixVariables)) {
76-
return Collections.emptyMap();
77-
}
77+
private MultiValueMap<String,String> mapMatrixVariables(MethodParameter parameter,
78+
@Nullable Map<String, MultiValueMap<String, String>> matrixVariables) {
7879

7980
MultiValueMap<String, String> map = new LinkedMultiValueMap<>();
81+
if (CollectionUtils.isEmpty(matrixVariables)) {
82+
return map;
83+
}
8084
MatrixVariable annotation = parameter.getParameterAnnotation(MatrixVariable.class);
8185
Assert.state(annotation != null, "No MatrixVariable annotation");
8286
String pathVariable = annotation.pathVar();
8387

8488
if (!pathVariable.equals(ValueConstants.DEFAULT_NONE)) {
8589
MultiValueMap<String, String> mapForPathVariable = matrixVariables.get(pathVariable);
8690
if (mapForPathVariable == null) {
87-
return Collections.emptyMap();
91+
return map;
8892
}
8993
map.putAll(mapForPathVariable);
9094
}
@@ -97,8 +101,7 @@ public Object resolveArgumentValue(MethodParameter parameter, BindingContext bin
97101
});
98102
}
99103
}
100-
101-
return (isSingleValueMap(parameter) ? map.toSingleValueMap() : map);
104+
return map;
102105
}
103106

104107
private boolean isSingleValueMap(MethodParameter parameter) {

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2019 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -22,6 +22,7 @@
2222
import java.util.LinkedHashMap;
2323
import java.util.Map;
2424

25+
import org.assertj.core.api.InstanceOfAssertFactories;
2526
import org.junit.jupiter.api.BeforeEach;
2627
import org.junit.jupiter.api.Test;
2728

@@ -139,6 +140,19 @@ public void resolveArgumentPathVariable() throws Exception {
139140
assertThat(mapAll.get("colors")).isEqualTo("red");
140141
}
141142

143+
@Test
144+
public void resolveMultiValueMapArgumentNoParams() {
145+
146+
MethodParameter param = this.testMethod.annot(matrixAttribute().noPathVar())
147+
.arg(MultiValueMap.class, String.class, String.class);
148+
149+
Object result = this.resolver.resolveArgument(param,
150+
new BindingContext(), this.exchange).block(Duration.ZERO);
151+
152+
assertThat(result).isInstanceOf(MultiValueMap.class)
153+
.asInstanceOf(InstanceOfAssertFactories.MAP).isEmpty();
154+
}
155+
142156
@Test
143157
public void resolveArgumentNoParams() throws Exception {
144158

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

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2018 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,7 +16,6 @@
1616

1717
package org.springframework.web.servlet.mvc.method.annotation;
1818

19-
import java.util.Collections;
2019
import java.util.List;
2120
import java.util.Map;
2221

@@ -68,19 +67,25 @@ public Object resolveArgument(MethodParameter parameter, @Nullable ModelAndViewC
6867
(Map<String, MultiValueMap<String, String>>) request.getAttribute(
6968
HandlerMapping.MATRIX_VARIABLES_ATTRIBUTE, RequestAttributes.SCOPE_REQUEST);
7069

71-
if (CollectionUtils.isEmpty(matrixVariables)) {
72-
return (isSingleValueMap(parameter) ? Collections.emptyMap() : new LinkedMultiValueMap<>(0));
73-
}
70+
MultiValueMap<String, String> map = mapMatrixVariables(parameter, matrixVariables);
71+
return (isSingleValueMap(parameter) ? map.toSingleValueMap() : map);
72+
}
73+
74+
private MultiValueMap<String,String> mapMatrixVariables(MethodParameter parameter,
75+
@Nullable Map<String, MultiValueMap<String, String>> matrixVariables) {
7476

7577
MultiValueMap<String, String> map = new LinkedMultiValueMap<>();
78+
if (CollectionUtils.isEmpty(matrixVariables)) {
79+
return map;
80+
}
7681
MatrixVariable ann = parameter.getParameterAnnotation(MatrixVariable.class);
7782
Assert.state(ann != null, "No MatrixVariable annotation");
7883
String pathVariable = ann.pathVar();
7984

8085
if (!pathVariable.equals(ValueConstants.DEFAULT_NONE)) {
8186
MultiValueMap<String, String> mapForPathVariable = matrixVariables.get(pathVariable);
8287
if (mapForPathVariable == null) {
83-
return Collections.emptyMap();
88+
return map;
8489
}
8590
map.putAll(mapForPathVariable);
8691
}
@@ -93,8 +98,7 @@ public Object resolveArgument(MethodParameter parameter, @Nullable ModelAndViewC
9398
});
9499
}
95100
}
96-
97-
return (isSingleValueMap(parameter) ? map.toSingleValueMap() : map);
101+
return map;
98102
}
99103

100104
private boolean isSingleValueMap(MethodParameter parameter) {

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2019 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -21,6 +21,7 @@
2121
import java.util.LinkedHashMap;
2222
import java.util.Map;
2323

24+
import org.assertj.core.api.InstanceOfAssertFactories;
2425
import org.junit.jupiter.api.BeforeEach;
2526
import org.junit.jupiter.api.Test;
2627

@@ -163,8 +164,8 @@ public void resolveMultiValueMapArgumentNoParams() throws Exception {
163164

164165
Object result = this.resolver.resolveArgument(param, this.mavContainer, this.webRequest, null);
165166

166-
//noinspection unchecked
167-
assertThat(result).isInstanceOfSatisfying(MultiValueMap.class, map -> assertThat(map).isEmpty());
167+
assertThat(result).isInstanceOf(MultiValueMap.class)
168+
.asInstanceOf(InstanceOfAssertFactories.MAP).isEmpty();
168169
}
169170

170171
@Test

0 commit comments

Comments
 (0)