From bd2b41f685ee51f5071a0e5fed7b1745debf1364 Mon Sep 17 00:00:00 2001 From: kdelay Date: Tue, 15 Sep 2026 17:15:10 +0900 Subject: [PATCH 1/5] Join collection matrix variable values with a comma MatrixVariableParameterProcessor built the path segment from the raw toString() of each value, so a collection came out bracketed: ;colours=[red, blue] instead of ;colours=red,blue. A matrix variable separates repeated values with a comma, so the receiving side reads "[red" and " blue]" back out of the bracketed form. Both branches of the processor were affected, including the Map> signature used as the @MatrixVariable example in the reference documentation. Signed-off-by: kdelay --- .../MatrixVariableParameterProcessor.java | 23 +++++++++++++----- .../support/SpringMvcContractTests.java | 24 +++++++++++++++++++ 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/annotation/MatrixVariableParameterProcessor.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/annotation/MatrixVariableParameterProcessor.java index bfff4fa5c..ea64af592 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/annotation/MatrixVariableParameterProcessor.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/annotation/MatrixVariableParameterProcessor.java @@ -18,7 +18,9 @@ import java.lang.annotation.Annotation; import java.lang.reflect.Method; +import java.util.Collection; import java.util.Map; +import java.util.Objects; import java.util.stream.Collectors; import feign.MethodMetadata; @@ -32,8 +34,9 @@ /** * {@link MatrixVariable} annotation processor. * - * Can expand maps or single objects. Values are assigned from the objects - * {@code toString()} method. + * Can expand maps or single objects. A value that is a {@link Collection} is joined with + * {@code ,}, which is the separator a matrix variable uses for repeated values; any other + * value is assigned from its {@code toString()} method. * * @author Matt King * @see AnnotatedParameterProcessor @@ -63,7 +66,7 @@ public boolean processArgument(AnnotatedParameterContext context, Annotation ann data.indexToExpander().put(parameterIndex, this::expandMap); } else { - data.indexToExpander().put(parameterIndex, object -> ";" + name + "=" + object.toString()); + data.indexToExpander().put(parameterIndex, object -> ";" + name + "=" + expandValue(object)); } return true; @@ -73,11 +76,19 @@ public boolean processArgument(AnnotatedParameterContext context, Annotation ann private String expandMap(Object object) { Map paramMap = (Map) object; - return paramMap.keySet() + return paramMap.entrySet() .stream() - .filter(key -> paramMap.get(key) != null) - .map(key -> ";" + key + "=" + paramMap.get(key).toString()) + .filter(entry -> entry.getValue() != null) + .map(entry -> ";" + entry.getKey() + "=" + expandValue(entry.getValue())) .collect(Collectors.joining()); } + private String expandValue(Object value) { + if (value instanceof Collection values) { + return values.stream().filter(Objects::nonNull).map(Object::toString).collect(Collectors.joining(",")); + } + + return value.toString(); + } + } diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java index f2b8ccde0..8ccab2dd7 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java @@ -30,6 +30,7 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -819,6 +820,26 @@ void testMatrixVariableWithNoName() throws NoSuchMethodException { assertThat(";param=value").isEqualTo(data.indexToExpander().get(0).expand(testMap)); } + @Test + void testMatrixVariable_MapParamWithCollectionValues() throws Exception { + Method method = TestTemplate_MatrixVariable.class.getDeclaredMethod("matrixVariable", Map.class); + MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + + Map testMap = new LinkedHashMap<>(); + testMap.put("colours", List.of("red", "blue")); + testMap.put("size", "L"); + + assertThat(data.indexToExpander().get(0).expand(testMap)).isEqualTo(";colours=red,blue;size=L"); + } + + @Test + void testMatrixVariable_CollectionParam() throws Exception { + Method method = TestTemplate_MatrixVariable.class.getDeclaredMethod("matrixVariableCollection", List.class); + MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + + assertThat(data.indexToExpander().get(0).expand(List.of("red", "blue"))).isEqualTo(";colours=red,blue"); + } + @Test void testAddingTemplatedParameterWithTheSameKey() throws NoSuchMethodException { Method method = TestTemplate_Advanced.class.getDeclaredMethod("testAddingTemplatedParamForExistingKey", @@ -1184,6 +1205,9 @@ public interface TestTemplate_MatrixVariable { @GetMapping("/matrixVariable/{params}") String matrixVariableNotNamed(@MatrixVariable Map params); + @GetMapping("/matrixVariable/{colours}") + String matrixVariableCollection(@MatrixVariable("colours") List colours); + } @JsonAutoDetect From 4bb1784f601691fc107b4dad227249fc73837121 Mon Sep 17 00:00:00 2001 From: kdelay Date: Fri, 18 Sep 2026 23:22:55 +0900 Subject: [PATCH 2/5] Expand array and nested matrix variable values Use StringUtils.collectionToCommaDelimitedString for the join and flatten arrays and nested collections through the same path. Signed-off-by: kdelay --- .../MatrixVariableParameterProcessor.java | 16 +++++++++---- .../support/SpringMvcContractTests.java | 23 +++++++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/annotation/MatrixVariableParameterProcessor.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/annotation/MatrixVariableParameterProcessor.java index ea64af592..1a6c6b15b 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/annotation/MatrixVariableParameterProcessor.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/annotation/MatrixVariableParameterProcessor.java @@ -26,6 +26,8 @@ import feign.MethodMetadata; import org.springframework.cloud.openfeign.AnnotatedParameterProcessor; +import org.springframework.util.CollectionUtils; +import org.springframework.util.StringUtils; import org.springframework.web.bind.annotation.MatrixVariable; import static feign.Util.checkState; @@ -34,9 +36,10 @@ /** * {@link MatrixVariable} annotation processor. * - * Can expand maps or single objects. A value that is a {@link Collection} is joined with - * {@code ,}, which is the separator a matrix variable uses for repeated values; any other - * value is assigned from its {@code toString()} method. + * Can expand maps or single objects. A value that is a {@link Collection} or an array is + * joined with {@code ,}, which is the separator a matrix variable uses for repeated + * values, and nested collections and arrays are flattened the same way; any other value + * is assigned from its {@code toString()} method. * * @author Matt King * @see AnnotatedParameterProcessor @@ -84,8 +87,13 @@ private String expandMap(Object object) { } private String expandValue(Object value) { + if (value.getClass().isArray()) { + return expandValue(CollectionUtils.arrayToList(value)); + } + if (value instanceof Collection values) { - return values.stream().filter(Objects::nonNull).map(Object::toString).collect(Collectors.joining(",")); + return StringUtils.collectionToCommaDelimitedString( + values.stream().filter(Objects::nonNull).map(this::expandValue).toList()); } return value.toString(); diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java index 8ccab2dd7..8de5ccd9a 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java @@ -840,6 +840,26 @@ void testMatrixVariable_CollectionParam() throws Exception { assertThat(data.indexToExpander().get(0).expand(List.of("red", "blue"))).isEqualTo(";colours=red,blue"); } + @Test + void testMatrixVariable_ArrayParam() throws Exception { + Method method = TestTemplate_MatrixVariable.class.getDeclaredMethod("matrixVariableArray", String[].class); + MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + + assertThat(data.indexToExpander().get(0).expand(new String[] { "red", "blue" })).isEqualTo(";colours=red,blue"); + } + + @Test + void testMatrixVariable_MapParamWithNestedCollectionAndArrayValues() throws Exception { + Method method = TestTemplate_MatrixVariable.class.getDeclaredMethod("matrixVariable", Map.class); + MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + + Map testMap = new LinkedHashMap<>(); + testMap.put("colours", List.of(List.of("red", "blue"), new String[] { "green" })); + testMap.put("sizes", new int[] { 1, 2 }); + + assertThat(data.indexToExpander().get(0).expand(testMap)).isEqualTo(";colours=red,blue,green;sizes=1,2"); + } + @Test void testAddingTemplatedParameterWithTheSameKey() throws NoSuchMethodException { Method method = TestTemplate_Advanced.class.getDeclaredMethod("testAddingTemplatedParamForExistingKey", @@ -1208,6 +1228,9 @@ public interface TestTemplate_MatrixVariable { @GetMapping("/matrixVariable/{colours}") String matrixVariableCollection(@MatrixVariable("colours") List colours); + @GetMapping("/matrixVariable/{colours}") + String matrixVariableArray(@MatrixVariable("colours") String[] colours); + } @JsonAutoDetect From 02fbad76358812efeec84c7bb14177c987cf485c Mon Sep 17 00:00:00 2001 From: kdelay Date: Fri, 18 Sep 2026 23:44:11 +0900 Subject: [PATCH 3/5] Keep matrix variable separators literal in the request URL Feign pct-encodes every value it substitutes into a URI template, so the ';' and '=' produced by the expander left the segment as %3Bname%3Dvalue and the server could not read it as matrix variables. Move the ';name=' prefix into the URI template, where it survives as a literal, and let the expander produce only the value. A collection parameter is expanded element by element by Feign and joined with ',', which now yields ';colours=red,blue'. Signed-off-by: kdelay --- .../MatrixVariableParameterProcessor.java | 19 ++++++++- .../support/SpringMvcContractTests.java | 40 +++++++++++++++++-- 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/annotation/MatrixVariableParameterProcessor.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/annotation/MatrixVariableParameterProcessor.java index 1a6c6b15b..96bf087d8 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/annotation/MatrixVariableParameterProcessor.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/annotation/MatrixVariableParameterProcessor.java @@ -69,12 +69,29 @@ public boolean processArgument(AnnotatedParameterContext context, Annotation ann data.indexToExpander().put(parameterIndex, this::expandMap); } else { - data.indexToExpander().put(parameterIndex, object -> ";" + name + "=" + expandValue(object)); + data.indexToExpander().put(parameterIndex, this::expandValue); + prefixTemplateVariable(data, name); } return true; } + /** + * Moves the {@code ;name=} prefix of the matrix variable out of the expanded value + * and into the URI template, so that it stays a literal. Feign always pct-encodes the + * values it substitutes into a URI template, which would turn the separators into + * {@code %3B} and {@code %3D} and stop the server from reading the segment as matrix + * variables. + */ + private void prefixTemplateVariable(MethodMetadata data, String name) { + String uri = data.template().url(); + String variable = "{" + name + "}"; + + if (uri.contains(variable)) { + data.template().uri(uri.replace(variable, ";" + name + "=" + variable)); + } + } + @SuppressWarnings("unchecked") private String expandMap(Object object) { Map paramMap = (Map) object; diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java index 8de5ccd9a..4c2511451 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java @@ -35,10 +35,15 @@ import java.util.Locale; import java.util.Map; import java.util.Objects; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Consumer; import com.fasterxml.jackson.annotation.JsonAutoDetect; +import feign.Client; +import feign.Feign; import feign.MethodMetadata; import feign.Param; +import feign.Response; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -803,8 +808,8 @@ void testMatrixVariable_ObjectParam() throws Exception { MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); assertThat(data.template().method()).isEqualTo("GET"); - assertThat(data.template().url()).isEqualTo("/matrixVariableObject/{param}"); - assertThat(";param=value").isEqualTo(data.indexToExpander().get(0).expand("value")); + assertThat(data.template().url()).isEqualTo("/matrixVariableObject/;param={param}"); + assertThat("value").isEqualTo(data.indexToExpander().get(0).expand("value")); } @Test @@ -837,7 +842,8 @@ void testMatrixVariable_CollectionParam() throws Exception { Method method = TestTemplate_MatrixVariable.class.getDeclaredMethod("matrixVariableCollection", List.class); MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); - assertThat(data.indexToExpander().get(0).expand(List.of("red", "blue"))).isEqualTo(";colours=red,blue"); + assertThat(data.template().url()).isEqualTo("/matrixVariable/;colours={colours}"); + assertThat(data.indexToExpander().get(0).expand(List.of("red", "blue"))).isEqualTo("red,blue"); } @Test @@ -845,7 +851,8 @@ void testMatrixVariable_ArrayParam() throws Exception { Method method = TestTemplate_MatrixVariable.class.getDeclaredMethod("matrixVariableArray", String[].class); MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); - assertThat(data.indexToExpander().get(0).expand(new String[] { "red", "blue" })).isEqualTo(";colours=red,blue"); + assertThat(data.template().url()).isEqualTo("/matrixVariable/;colours={colours}"); + assertThat(data.indexToExpander().get(0).expand(new String[] { "red", "blue" })).isEqualTo("red,blue"); } @Test @@ -860,6 +867,31 @@ void testMatrixVariable_MapParamWithNestedCollectionAndArrayValues() throws Exce assertThat(data.indexToExpander().get(0).expand(testMap)).isEqualTo(";colours=red,blue,green;sizes=1,2"); } + @Test + void testMatrixVariable_SingleParamKeepsSeparatorsInTheRequestUrl() { + assertThat(captureRequestUrl(api -> api.matrixVariableObject("value"))) + .isEqualTo("http://localhost/matrixVariableObject/;param=value"); + } + + @Test + void testMatrixVariable_CollectionParamKeepsSeparatorsInTheRequestUrl() { + assertThat(captureRequestUrl(api -> api.matrixVariableCollection(List.of("red", "blue")))) + .isEqualTo("http://localhost/matrixVariable/;colours=red,blue"); + } + + private String captureRequestUrl(Consumer call) { + AtomicReference url = new AtomicReference<>(); + Client client = (request, options) -> { + url.set(request.url()); + return Response.builder().status(200).request(request).body(new byte[0]).build(); + }; + call.accept(Feign.builder() + .contract(contract) + .client(client) + .target(TestTemplate_MatrixVariable.class, "http://localhost")); + return url.get(); + } + @Test void testAddingTemplatedParameterWithTheSameKey() throws NoSuchMethodException { Method method = TestTemplate_Advanced.class.getDeclaredMethod("testAddingTemplatedParamForExistingKey", From 94a9ad3a18e603133f8c16a12649e9ba6ad5cfd4 Mon Sep 17 00:00:00 2001 From: kdelay Date: Sat, 19 Sep 2026 09:36:40 +0900 Subject: [PATCH 4/5] Expand map matrix variables with a path-style expression Let Feign expand a Map typed matrix variable through a path-style URI template expression, so the ; and = separators stay literal in the request URL and only the keys and the values are encoded. Signed-off-by: kdelay --- .../MatrixVariableParameterProcessor.java | 32 +++++++++-------- .../support/SpringMvcContractTests.java | 34 +++++++------------ 2 files changed, 31 insertions(+), 35 deletions(-) diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/annotation/MatrixVariableParameterProcessor.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/annotation/MatrixVariableParameterProcessor.java index 96bf087d8..0ac1eb62d 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/annotation/MatrixVariableParameterProcessor.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/annotation/MatrixVariableParameterProcessor.java @@ -21,7 +21,6 @@ import java.util.Collection; import java.util.Map; import java.util.Objects; -import java.util.stream.Collectors; import feign.MethodMetadata; @@ -36,10 +35,11 @@ /** * {@link MatrixVariable} annotation processor. * - * Can expand maps or single objects. A value that is a {@link Collection} or an array is - * joined with {@code ,}, which is the separator a matrix variable uses for repeated - * values, and nested collections and arrays are flattened the same way; any other value - * is assigned from its {@code toString()} method. + * Can expand maps or single objects. A {@link Map} typed variable is expanded by Feign + * itself through a path-style URI template expression. For any other type, a value that + * is a {@link Collection} or an array is joined with {@code ,}, which is the separator a + * matrix variable uses for repeated values, and nested collections and arrays are + * flattened the same way; any other value is assigned from its {@code toString()} method. * * @author Matt King * @see AnnotatedParameterProcessor @@ -66,7 +66,7 @@ public boolean processArgument(AnnotatedParameterContext context, Annotation ann context.setParameterName(name); if (Map.class.isAssignableFrom(parameterType)) { - data.indexToExpander().put(parameterIndex, this::expandMap); + pathStyleTemplateVariable(data, name); } else { data.indexToExpander().put(parameterIndex, this::expandValue); @@ -92,15 +92,19 @@ private void prefixTemplateVariable(MethodMetadata data, String name) { } } - @SuppressWarnings("unchecked") - private String expandMap(Object object) { - Map paramMap = (Map) object; + /** + * Turns the URI template variable of a {@link Map} typed matrix variable into a + * path-style expression, so that Feign expands the map into {@code ;key=value} pairs + * itself and encodes only the keys and the values. Writing the pairs in an expander + * instead would have Feign pct-encode the separators along with them. + */ + private void pathStyleTemplateVariable(MethodMetadata data, String name) { + String uri = data.template().url(); + String variable = "{" + name + "}"; - return paramMap.entrySet() - .stream() - .filter(entry -> entry.getValue() != null) - .map(entry -> ";" + entry.getKey() + "=" + expandValue(entry.getValue())) - .collect(Collectors.joining()); + if (uri.contains(variable)) { + data.template().uri(uri.replace(variable, "{;" + name + "}")); + } } private String expandValue(Object value) { diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java index 4c2511451..82005a2e7 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java @@ -794,12 +794,13 @@ void testMatrixVariable_MapParam() throws Exception { Method method = TestTemplate_MatrixVariable.class.getDeclaredMethod("matrixVariable", Map.class); MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); - Map testMap = new HashMap<>(); + Map testMap = new HashMap<>(); testMap.put("param", "value"); assertThat(data.template().method()).isEqualTo("GET"); - assertThat(data.template().url()).isEqualTo("/matrixVariable/{params}"); - assertThat(";param=value").isEqualTo(data.indexToExpander().get(0).expand(testMap)); + assertThat(data.template().url()).isEqualTo("/matrixVariable/{;params}"); + assertThat(captureRequestUrl(api -> api.matrixVariable(testMap))) + .isEqualTo("http://localhost/matrixVariable/;param=value"); } @Test @@ -816,25 +817,19 @@ void testMatrixVariable_ObjectParam() throws Exception { void testMatrixVariableWithNoName() throws NoSuchMethodException { Method method = TestTemplate_MatrixVariable.class.getDeclaredMethod("matrixVariableNotNamed", Map.class); MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); - Map testMap = new HashMap<>(); - - testMap.put("param", "value"); assertThat(data.template().method()).isEqualTo("GET"); - assertThat(data.template().url()).isEqualTo("/matrixVariable/{params}"); - assertThat(";param=value").isEqualTo(data.indexToExpander().get(0).expand(testMap)); + assertThat(data.template().url()).isEqualTo("/matrixVariable/{;params}"); } @Test - void testMatrixVariable_MapParamWithCollectionValues() throws Exception { - Method method = TestTemplate_MatrixVariable.class.getDeclaredMethod("matrixVariable", Map.class); - MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); - + void testMatrixVariable_MapParamKeepsSeparatorsInTheRequestUrl() { Map testMap = new LinkedHashMap<>(); - testMap.put("colours", List.of("red", "blue")); + testMap.put("colours", "red"); testMap.put("size", "L"); - assertThat(data.indexToExpander().get(0).expand(testMap)).isEqualTo(";colours=red,blue;size=L"); + assertThat(captureRequestUrl(api -> api.matrixVariable(testMap))) + .isEqualTo("http://localhost/matrixVariable/;colours=red;size=L"); } @Test @@ -856,15 +851,12 @@ void testMatrixVariable_ArrayParam() throws Exception { } @Test - void testMatrixVariable_MapParamWithNestedCollectionAndArrayValues() throws Exception { - Method method = TestTemplate_MatrixVariable.class.getDeclaredMethod("matrixVariable", Map.class); + void testMatrixVariable_CollectionParamWithNestedCollectionAndArrayValues() throws Exception { + Method method = TestTemplate_MatrixVariable.class.getDeclaredMethod("matrixVariableCollection", List.class); MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); - Map testMap = new LinkedHashMap<>(); - testMap.put("colours", List.of(List.of("red", "blue"), new String[] { "green" })); - testMap.put("sizes", new int[] { 1, 2 }); - - assertThat(data.indexToExpander().get(0).expand(testMap)).isEqualTo(";colours=red,blue,green;sizes=1,2"); + assertThat(data.indexToExpander().get(0).expand(List.of(List.of("red", "blue"), new String[] { "green" }))) + .isEqualTo("red,blue,green"); } @Test From f404d407d3db22eb9a3fc0c2326daeef52d18617 Mon Sep 17 00:00:00 2001 From: kdelay Date: Tue, 22 Sep 2026 22:18:01 +0900 Subject: [PATCH 5/5] Drop map matrix variable coverage Feign encodes each value of a path-style expression, so a collection valued map entry cannot keep its separators. Leave map expansion as it was and keep this change to the cases it covers. Signed-off-by: kdelay --- .../MatrixVariableParameterProcessor.java | 33 +++++++++---------- .../support/SpringMvcContractTests.java | 24 +++++--------- 2 files changed, 23 insertions(+), 34 deletions(-) diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/annotation/MatrixVariableParameterProcessor.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/annotation/MatrixVariableParameterProcessor.java index 0ac1eb62d..2421cd6c2 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/annotation/MatrixVariableParameterProcessor.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/annotation/MatrixVariableParameterProcessor.java @@ -21,6 +21,7 @@ import java.util.Collection; import java.util.Map; import java.util.Objects; +import java.util.stream.Collectors; import feign.MethodMetadata; @@ -35,8 +36,8 @@ /** * {@link MatrixVariable} annotation processor. * - * Can expand maps or single objects. A {@link Map} typed variable is expanded by Feign - * itself through a path-style URI template expression. For any other type, a value that + * Can expand maps or single objects. For a {@link Map} typed variable, values are + * assigned from the objects {@code toString()} method. For any other type, a value that * is a {@link Collection} or an array is joined with {@code ,}, which is the separator a * matrix variable uses for repeated values, and nested collections and arrays are * flattened the same way; any other value is assigned from its {@code toString()} method. @@ -66,7 +67,7 @@ public boolean processArgument(AnnotatedParameterContext context, Annotation ann context.setParameterName(name); if (Map.class.isAssignableFrom(parameterType)) { - pathStyleTemplateVariable(data, name); + data.indexToExpander().put(parameterIndex, this::expandMap); } else { data.indexToExpander().put(parameterIndex, this::expandValue); @@ -92,21 +93,6 @@ private void prefixTemplateVariable(MethodMetadata data, String name) { } } - /** - * Turns the URI template variable of a {@link Map} typed matrix variable into a - * path-style expression, so that Feign expands the map into {@code ;key=value} pairs - * itself and encodes only the keys and the values. Writing the pairs in an expander - * instead would have Feign pct-encode the separators along with them. - */ - private void pathStyleTemplateVariable(MethodMetadata data, String name) { - String uri = data.template().url(); - String variable = "{" + name + "}"; - - if (uri.contains(variable)) { - data.template().uri(uri.replace(variable, "{;" + name + "}")); - } - } - private String expandValue(Object value) { if (value.getClass().isArray()) { return expandValue(CollectionUtils.arrayToList(value)); @@ -120,4 +106,15 @@ private String expandValue(Object value) { return value.toString(); } + @SuppressWarnings("unchecked") + private String expandMap(Object object) { + Map paramMap = (Map) object; + + return paramMap.keySet() + .stream() + .filter(key -> paramMap.get(key) != null) + .map(key -> ";" + key + "=" + paramMap.get(key).toString()) + .collect(Collectors.joining()); + } + } diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java index 82005a2e7..b1b636f83 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java @@ -30,7 +30,6 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; -import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -794,13 +793,12 @@ void testMatrixVariable_MapParam() throws Exception { Method method = TestTemplate_MatrixVariable.class.getDeclaredMethod("matrixVariable", Map.class); MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); - Map testMap = new HashMap<>(); + Map testMap = new HashMap<>(); testMap.put("param", "value"); assertThat(data.template().method()).isEqualTo("GET"); - assertThat(data.template().url()).isEqualTo("/matrixVariable/{;params}"); - assertThat(captureRequestUrl(api -> api.matrixVariable(testMap))) - .isEqualTo("http://localhost/matrixVariable/;param=value"); + assertThat(data.template().url()).isEqualTo("/matrixVariable/{params}"); + assertThat(";param=value").isEqualTo(data.indexToExpander().get(0).expand(testMap)); } @Test @@ -817,19 +815,13 @@ void testMatrixVariable_ObjectParam() throws Exception { void testMatrixVariableWithNoName() throws NoSuchMethodException { Method method = TestTemplate_MatrixVariable.class.getDeclaredMethod("matrixVariableNotNamed", Map.class); MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + Map testMap = new HashMap<>(); - assertThat(data.template().method()).isEqualTo("GET"); - assertThat(data.template().url()).isEqualTo("/matrixVariable/{;params}"); - } - - @Test - void testMatrixVariable_MapParamKeepsSeparatorsInTheRequestUrl() { - Map testMap = new LinkedHashMap<>(); - testMap.put("colours", "red"); - testMap.put("size", "L"); + testMap.put("param", "value"); - assertThat(captureRequestUrl(api -> api.matrixVariable(testMap))) - .isEqualTo("http://localhost/matrixVariable/;colours=red;size=L"); + assertThat(data.template().method()).isEqualTo("GET"); + assertThat(data.template().url()).isEqualTo("/matrixVariable/{params}"); + assertThat(";param=value").isEqualTo(data.indexToExpander().get(0).expand(testMap)); } @Test