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..ad87d300 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) { @@ -292,7 +299,11 @@ public class SpringMvcContract extends Contract.BaseContract implements Resource } } - if (!isMultipartFormData(data) && isHttpAnnotation && data.indexToExpander().get(paramIndex) == null) { + if (! + + isMultipartFormData(data) && isHttpAnnotation && data.indexToExpander(). + + get(paramIndex) == null) { TypeDescriptor typeDescriptor = createTypeDescriptor(method, paramIndex); if (conversionService.canConvert(typeDescriptor, STRING_TYPE_DESCRIPTOR)) { Param.Expander expander = convertingExpanderFactory.getExpander(typeDescriptor); @@ -304,6 +315,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..fc080ed5 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 @@ -117,7 +117,7 @@ class SpringMvcContractTests { Method isNamePresent = ReflectionUtils.findMethod(parameters[0].getClass(), "isNamePresent"); return Boolean.TRUE.equals(isNamePresent.invoke(parameters[0])); } - catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException ex) { + catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException ignored) { } } return false; @@ -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(2).queryMapIndex()).isEqualTo(1); + assertThat(data.get(2).bodyIndex()).isEqualTo(0); + assertThat(data.get(3).queryMapIndex()).isEqualTo(0); + assertThat(data.get(3).bodyIndex()).isEqualTo(1); + } + + @Test + void shouldSetPageableAsBodyWhenQueryMapParamPresent() { + List data = contract.parseAndValidateMetadata(TestTemplate_PageablePost.class); + + assertThat(data.get(0).queryMapIndex()).isEqualTo(1); + assertThat(data.get(0).bodyIndex()).isEqualTo(0); + assertThat(data.get(1).queryMapIndex()).isEqualTo(0); + assertThat(data.get(1).bodyIndex()).isEqualTo(1); } private ConversionService getConversionService() { @@ -846,6 +856,12 @@ class SpringMvcContractTests { @PostMapping Page getPage(@RequestBody String body, Pageable pageable); + @PostMapping + Page getPage(@SpringQueryMap TestObject pojo, Pageable pageable); + + @PostMapping + Page getPage(Pageable pageable, @SpringQueryMap TestObject pojo); + } @JsonAutoDetect(fieldVisibility = ANY, getterVisibility = NONE, setterVisibility = NONE)