From 8e9b11ffdf4a118f4febc4dd7816c904e1a7d16d Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Mon, 6 Mar 2023 14:17:59 +0100 Subject: [PATCH] Fix pageable processing when querymap present. (#843) --- .../openfeign/support/SpringMvcContract.java | 27 +++++++++++++++-- .../support/SpringMvcContractTests.java | 30 +++++++++++++++---- 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringMvcContract.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringMvcContract.java index 430bb44c..746cec5c 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringMvcContract.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringMvcContract.java @@ -34,12 +34,14 @@ import feign.Contract; import feign.Feign; import feign.MethodMetadata; import feign.Param; +import feign.QueryMap; import feign.Request; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.cloud.openfeign.AnnotatedParameterProcessor; import org.springframework.cloud.openfeign.CollectionFormat; +import org.springframework.cloud.openfeign.SpringQueryMap; import org.springframework.cloud.openfeign.annotation.CookieValueParameterProcessor; import org.springframework.cloud.openfeign.annotation.MatrixVariableParameterProcessor; import org.springframework.cloud.openfeign.annotation.PathVariableParameterProcessor; @@ -67,6 +69,7 @@ import org.springframework.util.Assert; import org.springframework.util.StringUtils; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; +import org.springframework.web.bind.annotation.RequestParam; import static feign.Util.checkState; import static feign.Util.emptyToNull; @@ -158,7 +161,7 @@ public class SpringMvcContract extends Contract.BaseContract implements Resource private static TypeDescriptor getElementTypeDescriptor(TypeDescriptor typeDescriptor) { TypeDescriptor elementTypeDescriptor = typeDescriptor.getElementTypeDescriptor(); - // that means it's not a collection but it is iterable, gh-135 + // that means it's not a collection, but it is iterable, gh-135 if (elementTypeDescriptor == null && Iterable.class.isAssignableFrom(typeDescriptor.getType())) { ResolvableType type = typeDescriptor.getResolvableType().as(Iterable.class).getGeneric(0); if (type.resolve() == null) { @@ -268,8 +271,12 @@ public class SpringMvcContract extends Contract.BaseContract implements Resource try { if (Pageable.class.isAssignableFrom(data.method().getParameterTypes()[paramIndex])) { - data.queryMapIndex(paramIndex); - return false; + // do not set a Pageable as QueryMap if there's an actual QueryMap param + // present + if (!queryMapParamPresent(data)) { + data.queryMapIndex(paramIndex); + return false; + } } } catch (NoClassDefFoundError ignored) { @@ -304,6 +311,20 @@ public class SpringMvcContract extends Contract.BaseContract implements Resource return isHttpAnnotation; } + private boolean queryMapParamPresent(MethodMetadata data) { + Annotation[][] paramsAnnotations = data.method().getParameterAnnotations(); + for (int i = 0; i < paramsAnnotations.length; i++) { + Annotation[] paramAnnotations = paramsAnnotations[i]; + Class parameterType = data.method().getParameterTypes()[i]; + if (Arrays.stream(paramAnnotations).anyMatch( + annotation -> Map.class.isAssignableFrom(parameterType) && annotation instanceof RequestParam + || annotation instanceof SpringQueryMap || annotation instanceof QueryMap)) { + return true; + } + } + return false; + } + private void parseProduces(MethodMetadata md, Method method, RequestMapping annotation) { String[] serverProduces = annotation.produces(); String clientAccepts = serverProduces.length == 0 ? null : emptyToNull(serverProduces[0]); 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 919e85c8..6c2a1156 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 @@ -631,10 +631,20 @@ class SpringMvcContractTests { void shouldNotFailWhenBothPageableAndRequestBodyParamsInPostRequest() { List data = contract.parseAndValidateMetadata(TestTemplate_PageablePost.class); - assertThat(data.get(0).queryMapIndex().intValue()).isEqualTo(0); - assertThat(data.get(0).bodyIndex().intValue()).isEqualTo(1); - assertThat(data.get(1).queryMapIndex().intValue()).isEqualTo(1); - assertThat(data.get(1).bodyIndex().intValue()).isEqualTo(0); + assertThat(data.get(0).queryMapIndex()).isEqualTo(0); + assertThat(data.get(0).bodyIndex()).isEqualTo(1); + assertThat(data.get(1).queryMapIndex()).isEqualTo(1); + assertThat(data.get(1).bodyIndex()).isEqualTo(0); + } + + @Test + void shouldSetPageableAsBodyWhenQueryMapParamPresent() { + List data = contract.parseAndValidateMetadata(TestTemplate_PageablePostWithQueryMap.class); + + assertThat(data.get(0).queryMapIndex()).isEqualTo(0); + assertThat(data.get(0).bodyIndex()).isEqualTo(1); + assertThat(data.get(1).queryMapIndex()).isEqualTo(1); + assertThat(data.get(1).bodyIndex()).isEqualTo(0); } private ConversionService getConversionService() { @@ -838,7 +848,7 @@ class SpringMvcContractTests { } - public interface TestTemplate_PageablePost { + interface TestTemplate_PageablePost { @PostMapping Page getPage(Pageable pageable, @RequestBody String body); @@ -848,6 +858,16 @@ class SpringMvcContractTests { } + interface TestTemplate_PageablePostWithQueryMap { + + @PostMapping + Page getPage(@SpringQueryMap TestObject pojo, Pageable pageable); + + @PostMapping + Page getPage(Pageable pageable, @SpringQueryMap TestObject pojo); + + } + @JsonAutoDetect(fieldVisibility = ANY, getterVisibility = NONE, setterVisibility = NONE) public class TestObject {