Support header map and query map (#1361)

Adds support for Map types in feign for `@RequestHeader` and `@RequestParam`.

Fixes gh-1360

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.

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<PathVariable> ANNOTATION = PathVariable.class;
+	private static final Class<PathVariable> ANNOTATION = PathVariable.class;
 
-    @Override
-    public Class<? extends Annotation> getAnnotationType() {
-        return ANNOTATION;
-    }
+	@Override
+	public Class<? extends Annotation> 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 <K, V> boolean searchMapValues(Map<K, Collection<V>> map, V search) {
-        Collection<Collection<V>> values = map.values();
-        if (values == null) {
-            return false;
-        }
-        for (Collection<V> entry : values) {
-            if (entry.contains(search)) {
-                return true;
-            }
-        }
-        return false;
-    }
+	private <K, V> boolean searchMapValues(Map<K, Collection<V>> map, V search) {
+		Collection<Collection<V>> values = map.values();
+		if (values == null) {
+			return false;
+		}
+		for (Collection<V> entry : values) {
+			if (entry.contains(search)) {
+				return true;
+			}
+		}
+		return false;
+	}
 }

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<RequestHeader> ANNOTATION = RequestHeader.class;
-
-    @Override
-    public Class<? extends Annotation> 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);
-
-        MethodMetadata data = context.getMethodMetadata();
-        Collection<String> header = context.setTemplateParameter(name, data.template().headers().get(name));
-        data.template().header(name, header);
-        return true;
-    }
+	private static final Class<RequestHeader> ANNOTATION = RequestHeader.class;
+
+	@Override
+	public Class<? extends Annotation> getAnnotationType() {
+		return ANNOTATION;
+	}
+
+	@Override
+	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.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<String> header = context.setTemplateParameter(name, data.template().headers().get(name));
+		data.template().header(name, header);
+		return true;
+	}
 }

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<String> 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<String> query = context.setTemplateParameter(name,
+				data.template().queries().get(name));
+		data.template().query(name, query);
 		return true;
 	}
 }

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

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<String, Collection<String>> 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<String, Collection<String>> 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<TestObject> getTest(@PathVariable("id") String id);
@@ -380,6 +425,30 @@ public class SpringMvcContractTests {
 		ResponseEntity<TestObject> getTest(@RequestParam Map<String, String> params);
 	}
 
+	public interface TestTemplate_HeaderMap {
+		@RequestMapping(path = "/headerMap")
+		String headerMap(
+				@RequestHeader MultiValueMap<String, String> headerMap,
+				@RequestHeader(name = "aHeader") String aHeader);
+
+		@RequestMapping(path = "/headerMapMoreThanOnce")
+		String headerMapMoreThanOnce(
+				@RequestHeader MultiValueMap<String, String> headerMap1,
+				@RequestHeader MultiValueMap<String, String> headerMap2);
+	}
+
+	public interface TestTemplate_QueryMap {
+		@RequestMapping(path = "/queryMap")
+		String queryMap(
+				@RequestParam MultiValueMap<String, String> queryMap,
+				@RequestParam(name = "aParam") String aParam);
+
+		@RequestMapping(path = "/queryMapMoreThanOnce")
+		String queryMapMoreThanOnce(
+				@RequestParam MultiValueMap<String, String> queryMap1,
+				@RequestParam MultiValueMap<String, String> queryMap2);
+	}
+
 	@JsonAutoDetect
 	@RequestMapping("/advanced")
 	public interface TestTemplate_Advanced {