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..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 @@ -18,12 +18,16 @@ 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; 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; @@ -32,8 +36,11 @@ /** * {@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. 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. * * @author Matt King * @see AnnotatedParameterProcessor @@ -63,12 +70,42 @@ 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, 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)); + } + } + + private String expandValue(Object value) { + if (value.getClass().isArray()) { + return expandValue(CollectionUtils.arrayToList(value)); + } + + if (value instanceof Collection values) { + return StringUtils.collectionToCommaDelimitedString( + values.stream().filter(Objects::nonNull).map(this::expandValue).toList()); + } + + return value.toString(); + } + @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 f2b8ccde0..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 @@ -34,10 +34,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; @@ -802,8 +807,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 @@ -819,6 +824,58 @@ void testMatrixVariableWithNoName() throws NoSuchMethodException { assertThat(";param=value").isEqualTo(data.indexToExpander().get(0).expand(testMap)); } + @Test + void testMatrixVariable_CollectionParam() throws Exception { + Method method = TestTemplate_MatrixVariable.class.getDeclaredMethod("matrixVariableCollection", List.class); + MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + + assertThat(data.template().url()).isEqualTo("/matrixVariable/;colours={colours}"); + assertThat(data.indexToExpander().get(0).expand(List.of("red", "blue"))).isEqualTo("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.template().url()).isEqualTo("/matrixVariable/;colours={colours}"); + assertThat(data.indexToExpander().get(0).expand(new String[] { "red", "blue" })).isEqualTo("red,blue"); + } + + @Test + void testMatrixVariable_CollectionParamWithNestedCollectionAndArrayValues() 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(List.of("red", "blue"), new String[] { "green" }))) + .isEqualTo("red,blue,green"); + } + + @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", @@ -1184,6 +1241,12 @@ public interface TestTemplate_MatrixVariable { @GetMapping("/matrixVariable/{params}") String matrixVariableNotNamed(@MatrixVariable Map params); + @GetMapping("/matrixVariable/{colours}") + String matrixVariableCollection(@MatrixVariable("colours") List colours); + + @GetMapping("/matrixVariable/{colours}") + String matrixVariableArray(@MatrixVariable("colours") String[] colours); + } @JsonAutoDetect