From 45d769b05bdb0073a4941f1f9473716fb2826b01 Mon Sep 17 00:00:00 2001 From: Abhijit Sarkar Date: Mon, 9 Jan 2017 19:45:03 -0800 Subject: [PATCH] Support header map and query map (#1361) Adds support for Map types in feign for `@RequestHeader` and `@RequestParam`. Fixes gh-1360 --- .../feign/AnnotatedParameterProcessor.java | 5 +- .../PathVariableParameterProcessor.java | 66 +++++++++--------- .../RequestHeaderParameterProcessor.java | 45 +++++++----- .../RequestParamParameterProcessor.java | 35 ++++++---- .../feign/support/SpringMvcContract.java | 3 +- .../feign/support/SpringMvcContractTests.java | 69 +++++++++++++++++++ 6 files changed, 160 insertions(+), 63 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/AnnotatedParameterProcessor.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/AnnotatedParameterProcessor.java index acfccef0..c612a8cc 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/AnnotatedParameterProcessor.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/AnnotatedParameterProcessor.java @@ -17,6 +17,7 @@ package org.springframework.cloud.netflix.feign; import java.lang.annotation.Annotation; +import java.lang.reflect.Method; import java.util.Collection; import feign.MethodMetadata; @@ -25,6 +26,7 @@ import feign.MethodMetadata; * Feign contract method parameter processor. * * @author Jakub Narloch + * @author Abhijit Sarkar */ public interface AnnotatedParameterProcessor { @@ -40,9 +42,10 @@ public interface AnnotatedParameterProcessor { * * @param context the parameter context * @param annotation the annotation instance + * @param method the method that contains the annotation * @return whether the parameter is http */ - boolean processArgument(AnnotatedParameterContext context, Annotation annotation); + boolean processArgument(AnnotatedParameterContext context, Annotation annotation, Method method); /** * Specifies the parameter context. diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/annotation/PathVariableParameterProcessor.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/annotation/PathVariableParameterProcessor.java index cb7848ba..c1dba1c9 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/annotation/PathVariableParameterProcessor.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/annotation/PathVariableParameterProcessor.java @@ -17,6 +17,7 @@ package org.springframework.cloud.netflix.feign.annotation; import java.lang.annotation.Annotation; +import java.lang.reflect.Method; import java.util.Collection; import java.util.Map; @@ -32,44 +33,45 @@ import static feign.Util.emptyToNull; * {@link PathVariable} parameter processor. * * @author Jakub Narloch + * @author Abhijit Sarkar * @see AnnotatedParameterProcessor */ public class PathVariableParameterProcessor implements AnnotatedParameterProcessor { - private static final Class ANNOTATION = PathVariable.class; + private static final Class ANNOTATION = PathVariable.class; - @Override - public Class getAnnotationType() { - return ANNOTATION; - } + @Override + public Class getAnnotationType() { + return ANNOTATION; + } - @Override - public boolean processArgument(AnnotatedParameterContext context, Annotation annotation) { - String name = ANNOTATION.cast(annotation).value(); - checkState(emptyToNull(name) != null, - "PathVariable annotation was empty on param %s.", context.getParameterIndex()); - context.setParameterName(name); + @Override + public boolean processArgument(AnnotatedParameterContext context, Annotation annotation, Method method) { + String name = ANNOTATION.cast(annotation).value(); + checkState(emptyToNull(name) != null, + "PathVariable annotation was empty on param %s.", context.getParameterIndex()); + context.setParameterName(name); - MethodMetadata data = context.getMethodMetadata(); - String varName = '{' + name + '}'; - if (!data.template().url().contains(varName) - && !searchMapValues(data.template().queries(), varName) - && !searchMapValues(data.template().headers(), varName)) { - data.formParams().add(name); - } - return true; - } + MethodMetadata data = context.getMethodMetadata(); + String varName = '{' + name + '}'; + if (!data.template().url().contains(varName) + && !searchMapValues(data.template().queries(), varName) + && !searchMapValues(data.template().headers(), varName)) { + data.formParams().add(name); + } + return true; + } - private boolean searchMapValues(Map> map, V search) { - Collection> values = map.values(); - if (values == null) { - return false; - } - for (Collection entry : values) { - if (entry.contains(search)) { - return true; - } - } - return false; - } + private boolean searchMapValues(Map> map, V search) { + Collection> values = map.values(); + if (values == null) { + return false; + } + for (Collection entry : values) { + if (entry.contains(search)) { + return true; + } + } + return false; + } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/annotation/RequestHeaderParameterProcessor.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/annotation/RequestHeaderParameterProcessor.java index a5a3942d..111bf00f 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/annotation/RequestHeaderParameterProcessor.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/annotation/RequestHeaderParameterProcessor.java @@ -17,7 +17,9 @@ package org.springframework.cloud.netflix.feign.annotation; import java.lang.annotation.Annotation; +import java.lang.reflect.Method; import java.util.Collection; +import java.util.Map; import org.springframework.cloud.netflix.feign.AnnotatedParameterProcessor; import org.springframework.web.bind.annotation.RequestHeader; @@ -31,27 +33,38 @@ import static feign.Util.emptyToNull; * {@link RequestHeader} parameter processor. * * @author Jakub Narloch + * @author Abhijit Sarkar * @see AnnotatedParameterProcessor */ public class RequestHeaderParameterProcessor implements AnnotatedParameterProcessor { - private static final Class ANNOTATION = RequestHeader.class; + private static final Class ANNOTATION = RequestHeader.class; - @Override - public Class getAnnotationType() { - return ANNOTATION; - } + @Override + public Class getAnnotationType() { + return ANNOTATION; + } - @Override - public boolean processArgument(AnnotatedParameterContext context, Annotation annotation) { - String name = ANNOTATION.cast(annotation).value(); - checkState(emptyToNull(name) != null, - "RequestHeader.value() was empty on parameter %s", context.getParameterIndex()); - context.setParameterName(name); + @Override + public boolean processArgument(AnnotatedParameterContext context, Annotation annotation, Method method) { + int parameterIndex = context.getParameterIndex(); + Class parameterType = method.getParameterTypes()[parameterIndex]; + MethodMetadata data = context.getMethodMetadata(); - MethodMetadata data = context.getMethodMetadata(); - Collection header = context.setTemplateParameter(name, data.template().headers().get(name)); - data.template().header(name, header); - return true; - } + if (Map.class.isAssignableFrom(parameterType)) { + checkState(data.headerMapIndex() == null, "Header map can only be present once."); + data.headerMapIndex(parameterIndex); + + return true; + } + + String name = ANNOTATION.cast(annotation).value(); + checkState(emptyToNull(name) != null, + "RequestHeader.value() was empty on parameter %s", parameterIndex); + context.setParameterName(name); + + Collection header = context.setTemplateParameter(name, data.template().headers().get(name)); + data.template().header(name, header); + return true; + } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/annotation/RequestParamParameterProcessor.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/annotation/RequestParamParameterProcessor.java index 950f91cc..ba996ac2 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/annotation/RequestParamParameterProcessor.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/annotation/RequestParamParameterProcessor.java @@ -17,7 +17,9 @@ package org.springframework.cloud.netflix.feign.annotation; import java.lang.annotation.Annotation; +import java.lang.reflect.Method; import java.util.Collection; +import java.util.Map; import org.springframework.cloud.netflix.feign.AnnotatedParameterProcessor; import org.springframework.web.bind.annotation.RequestParam; @@ -31,6 +33,7 @@ import feign.MethodMetadata; * {@link RequestParam} parameter processor. * * @author Jakub Narloch + * @author Abhijit Sarkar * @see AnnotatedParameterProcessor */ public class RequestParamParameterProcessor implements AnnotatedParameterProcessor { @@ -43,22 +46,28 @@ public class RequestParamParameterProcessor implements AnnotatedParameterProcess } @Override - public boolean processArgument(AnnotatedParameterContext context, - Annotation annotation) { + public boolean processArgument(AnnotatedParameterContext context, Annotation annotation, Method method) { + int parameterIndex = context.getParameterIndex(); + Class parameterType = method.getParameterTypes()[parameterIndex]; + MethodMetadata data = context.getMethodMetadata(); + + if (Map.class.isAssignableFrom(parameterType)) { + checkState(data.queryMapIndex() == null, "Query map can only be present once."); + data.queryMapIndex(parameterIndex); + + return true; + } + RequestParam requestParam = ANNOTATION.cast(annotation); String name = requestParam.value(); - if (emptyToNull(name) != null) { - context.setParameterName(name); + checkState(emptyToNull(name) != null, + "RequestParam.value() was empty on parameter %s", + parameterIndex); + context.setParameterName(name); - MethodMetadata data = context.getMethodMetadata(); - Collection query = context.setTemplateParameter(name, - data.template().queries().get(name)); - data.template().query(name, query); - } else { - // supports `Map` types - MethodMetadata data = context.getMethodMetadata(); - data.queryMapIndex(context.getParameterIndex()); - } + Collection query = context.setTemplateParameter(name, + data.template().queries().get(name)); + data.template().query(name, query); return true; } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/SpringMvcContract.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/SpringMvcContract.java index 0afec31a..f0018c8b 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/SpringMvcContract.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/SpringMvcContract.java @@ -58,6 +58,7 @@ import feign.Param; /** * @author Spencer Gibb + * @author Abhijit Sarkar */ public class SpringMvcContract extends Contract.BaseContract implements ResourceLoaderAware { @@ -235,7 +236,7 @@ public class SpringMvcContract extends Contract.BaseContract processParameterAnnotation = synthesizeWithMethodParameterNameAsFallbackValue( parameterAnnotation, method, paramIndex); isHttpAnnotation |= processor.processArgument(context, - processParameterAnnotation); + processParameterAnnotation, method); } } if (isHttpAnnotation && data.indexToExpander().get(paramIndex) == null diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/support/SpringMvcContractTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/support/SpringMvcContractTests.java index 139f38ce..624f8928 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/support/SpringMvcContractTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/support/SpringMvcContractTests.java @@ -18,6 +18,8 @@ package org.springframework.cloud.netflix.feign.support; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.text.CollationElementIterator; +import java.util.Collection; import java.util.List; import java.util.Map; @@ -25,6 +27,7 @@ import org.junit.Before; import org.junit.Test; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; +import org.springframework.util.MultiValueMap; import org.springframework.util.ReflectionUtils; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.PathVariable; @@ -340,6 +343,48 @@ public class SpringMvcContractTests { return false; } + @Test + public void testProcessHeaderMap() throws Exception { + Method method = TestTemplate_HeaderMap.class.getDeclaredMethod("headerMap", + MultiValueMap.class, String.class); + MethodMetadata data = this.contract + .parseAndValidateMetadata(method.getDeclaringClass(), method); + + assertEquals("/headerMap", data.template().url()); + assertEquals("GET", data.template().method()); + assertEquals(0, data.headerMapIndex().intValue()); + Map> headers = data.template().headers(); + assertEquals("{aHeader}", headers.get("aHeader").iterator().next()); + } + + @Test(expected = IllegalStateException.class) + public void testProcessHeaderMapMoreThanOnce() throws Exception { + Method method = TestTemplate_HeaderMap.class.getDeclaredMethod( + "headerMapMoreThanOnce", MultiValueMap.class, MultiValueMap.class); + this.contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + } + + @Test + public void testProcessQueryMap() throws Exception { + Method method = TestTemplate_QueryMap.class.getDeclaredMethod("queryMap", + MultiValueMap.class, String.class); + MethodMetadata data = this.contract + .parseAndValidateMetadata(method.getDeclaringClass(), method); + + assertEquals("/queryMap", data.template().url()); + assertEquals("GET", data.template().method()); + assertEquals(0, data.queryMapIndex().intValue()); + Map> params = data.template().queries(); + assertEquals("{aParam}", params.get("aParam").iterator().next()); + } + + @Test(expected = IllegalStateException.class) + public void testProcessQueryMapMoreThanOnce() throws Exception { + Method method = TestTemplate_QueryMap.class.getDeclaredMethod( + "queryMapMoreThanOnce", MultiValueMap.class, MultiValueMap.class); + this.contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + } + public interface TestTemplate_Simple { @RequestMapping(value = "/test/{id}", method = RequestMethod.GET, produces = MediaType.APPLICATION_JSON_VALUE) ResponseEntity getTest(@PathVariable("id") String id); @@ -380,6 +425,30 @@ public class SpringMvcContractTests { ResponseEntity getTest(@RequestParam Map params); } + public interface TestTemplate_HeaderMap { + @RequestMapping(path = "/headerMap") + String headerMap( + @RequestHeader MultiValueMap headerMap, + @RequestHeader(name = "aHeader") String aHeader); + + @RequestMapping(path = "/headerMapMoreThanOnce") + String headerMapMoreThanOnce( + @RequestHeader MultiValueMap headerMap1, + @RequestHeader MultiValueMap headerMap2); + } + + public interface TestTemplate_QueryMap { + @RequestMapping(path = "/queryMap") + String queryMap( + @RequestParam MultiValueMap queryMap, + @RequestParam(name = "aParam") String aParam); + + @RequestMapping(path = "/queryMapMoreThanOnce") + String queryMapMoreThanOnce( + @RequestParam MultiValueMap queryMap1, + @RequestParam MultiValueMap queryMap2); + } + @JsonAutoDetect @RequestMapping("/advanced") public interface TestTemplate_Advanced {