Skip to content

Commit

Permalink
Support header map and query map (#1361)
Browse files Browse the repository at this point in the history 
Adds support for Map types in feign for `@RequestHeader` and `@RequestParam`.

Fixes gh-1360
  • Loading branch information
@asarkar @spencergibb
asarkar authored and spencergibb committed Jan 10, 2017
1 parent af03247 commit 45d769b
Show file tree
Hide file tree
Showing 6 changed files with 163 additions and 66 deletions.
5 changes: 4 additions & 1 deletion ...re/src/main/java/org/springframework/cloud/netflix/feign/AnnotatedParameterProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -25,6 +26,7 @@
* Feign contract method parameter processor.
*
* @author Jakub Narloch
* @author Abhijit Sarkar
*/
public interface AnnotatedParameterProcessor {

Expand All @@ -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.
Expand Down
66 changes: 34 additions & 32 deletions ...va/org/springframework/cloud/netflix/feign/annotation/PathVariableParameterProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -32,44 +33,45 @@
* {@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;
}
}
51 changes: 32 additions & 19 deletions ...a/org/springframework/cloud/netflix/feign/annotation/RequestHeaderParameterProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -31,27 +33,38 @@
* {@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;
}
}
35 changes: 22 additions & 13 deletions ...va/org/springframework/cloud/netflix/feign/annotation/RequestParamParameterProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -31,6 +33,7 @@
* {@link RequestParam} parameter processor.
*
* @author Jakub Narloch
* @author Abhijit Sarkar
* @see AnnotatedParameterProcessor
*/
public class RequestParamParameterProcessor implements AnnotatedParameterProcessor {
Expand All @@ -43,22 +46,28 @@ public Class<? extends Annotation> getAnnotationType() {
}

@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;
}
}
3 changes: 2 additions & 1 deletion ...core/src/main/java/org/springframework/cloud/netflix/feign/support/SpringMvcContract.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@

/**
* @author Spencer Gibb
* @author Abhijit Sarkar
*/
public class SpringMvcContract extends Contract.BaseContract
implements ResourceLoaderAware {
Expand Down Expand Up @@ -235,7 +236,7 @@ protected boolean processAnnotationsOnParameter(MethodMetadata data,
processParameterAnnotation = synthesizeWithMethodParameterNameAsFallbackValue(
parameterAnnotation, method, paramIndex);
isHttpAnnotation |= processor.processArgument(context,
processParameterAnnotation);
processParameterAnnotation, method);
}
}
if (isHttpAnnotation && data.indexToExpander().get(paramIndex) == null
Expand Down
69 changes: 69 additions & 0 deletions ...src/test/java/org/springframework/cloud/netflix/feign/support/SpringMvcContractTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,16 @@

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;

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;
Expand Down Expand Up @@ -340,6 +343,48 @@ private static boolean hasJava8ParameterNames(Method m) {
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);
Expand Down Expand Up @@ -380,6 +425,30 @@ public interface TestTemplate_MapParams {
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 {
Expand Down

0 comments on commit 45d769b

Please sign in to comment.