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 edd2b226..a67da38b 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 @@ -22,11 +22,11 @@ import java.util.Collection; import org.springframework.cloud.netflix.feign.AnnotatedParameterProcessor; import org.springframework.web.bind.annotation.RequestParam; -import feign.MethodMetadata; - import static feign.Util.checkState; import static feign.Util.emptyToNull; +import feign.MethodMetadata; + /** * {@link RequestParam} parameter processor. * @@ -35,23 +35,27 @@ import static feign.Util.emptyToNull; */ public class RequestParamParameterProcessor implements AnnotatedParameterProcessor { - private static final Class ANNOTATION = RequestParam.class; + private static final Class ANNOTATION = RequestParam.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, - "RequestParam.value() was empty on parameter %s", context.getParameterIndex()); - context.setParameterName(name); + @Override + public boolean processArgument(AnnotatedParameterContext context, + Annotation annotation) { + RequestParam requestParam = ANNOTATION.cast(annotation); + String name = requestParam.value(); + checkState(emptyToNull(name) != null, + "RequestParam.value() was empty on parameter %s", + context.getParameterIndex()); + context.setParameterName(name); - MethodMetadata data = context.getMethodMetadata(); - Collection query = context.setTemplateParameter(name, data.template().queries().get(name)); - data.template().query(name, query); - return true; - } + MethodMetadata data = context.getMethodMetadata(); + 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 7dab129c..34b1a643 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 @@ -45,15 +45,15 @@ import org.springframework.util.StringUtils; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; +import static feign.Util.checkState; +import static feign.Util.emptyToNull; +import static org.springframework.core.annotation.AnnotatedElementUtils.findMergedAnnotation; + import feign.Contract; import feign.Feign; import feign.MethodMetadata; import feign.Param; -import static feign.Util.checkState; -import static feign.Util.emptyToNull; -import static org.springframework.core.annotation.AnnotatedElementUtils.findMergedAnnotation; - /** * @author Spencer Gibb */ @@ -237,6 +237,7 @@ public class SpringMvcContract extends Contract.BaseContract } } if (isHttpAnnotation && data.indexToExpander().get(paramIndex) == null + && !isMultiValued(method.getParameterTypes()[paramIndex]) && this.conversionService.canConvert( method.getParameterTypes()[paramIndex], String.class)) { data.indexToExpander().put(paramIndex, this.expander); @@ -244,6 +245,13 @@ public class SpringMvcContract extends Contract.BaseContract return isHttpAnnotation; } + private boolean isMultiValued(Class type) { + // Feign will deal with each element in a collection individually (with no + // expander as of 8.16.2, but we'd rather have no conversion than convert a + // collection to a String (which ends up being a csv). + return Collection.class.isAssignableFrom(type); + } + private void parseProduces(MethodMetadata md, Method method, RequestMapping annotation) { checkAtMostOne(method, annotation.produces(), "produces"); diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/support/SpringEncoderTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/support/SpringEncoderTests.java index a101e691..cfa04cac 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/support/SpringEncoderTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/support/SpringEncoderTests.java @@ -27,13 +27,13 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import org.springframework.test.context.web.WebAppConfiguration; import org.springframework.web.bind.annotation.RestController; -import feign.RequestTemplate; -import lombok.Data; - import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; import static org.junit.Assert.assertThat; +import feign.RequestTemplate; +import lombok.Data; + /** * @author Spencer Gibb */ @@ -50,7 +50,7 @@ public class SpringEncoderTests { @Autowired @Qualifier("myHttpMessageConverter") - private HttpMessageConverter myConverter; + private HttpMessageConverter myConverter; @Test public void testCustomHttpMessageConverter() { @@ -73,14 +73,14 @@ public class SpringEncoderTests { private MediaType mediaType; public MediaTypeMatcher(String type, String subtype) { - mediaType = new MediaType(type, subtype); + this.mediaType = new MediaType(type, subtype); } @Override public boolean matches(Object argument) { if (argument instanceof MediaType) { MediaType other = (MediaType) argument; - return mediaType.equals(other); + return this.mediaType.equals(other); } return false; } @@ -88,7 +88,7 @@ public class SpringEncoderTests { @Override public String toString() { final StringBuffer sb = new StringBuffer("MediaTypeMatcher{"); - sb.append("mediaType=").append(mediaType); + sb.append("mediaType=").append(this.mediaType); sb.append('}'); return sb.toString(); } @@ -109,18 +109,19 @@ public class SpringEncoderTests { protected static class Application implements TestClient { @Bean - HttpMessageConverter myHttpMessageConverter() { + HttpMessageConverter myHttpMessageConverter() { return new MyHttpMessageConverter(); } - private static class MyHttpMessageConverter extends AbstractGenericHttpMessageConverter { + private static class MyHttpMessageConverter + extends AbstractGenericHttpMessageConverter { public MyHttpMessageConverter() { super(new MediaType("application", "mytype")); } @Override - protected boolean supports(Class clazz) { + protected boolean supports(Class clazz) { return false; } @@ -135,17 +136,22 @@ public class SpringEncoderTests { } @Override - protected void writeInternal(Object o, Type type, HttpOutputMessage outputMessage) throws IOException, HttpMessageNotWritableException { + protected void writeInternal(Object o, Type type, + HttpOutputMessage outputMessage) + throws IOException, HttpMessageNotWritableException { } @Override - protected Object readInternal(Class clazz, HttpInputMessage inputMessage) throws IOException, HttpMessageNotReadableException { + protected Object readInternal(Class clazz, HttpInputMessage inputMessage) + throws IOException, HttpMessageNotReadableException { return null; } @Override - public Object read(Type type, Class contextClass, HttpInputMessage inputMessage) throws IOException, HttpMessageNotReadableException { + public Object read(Type type, Class contextClass, + HttpInputMessage inputMessage) + throws IOException, HttpMessageNotReadableException { return 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 1cf2a3fd..a6616361 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,7 @@ package org.springframework.cloud.netflix.feign.support; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.util.List; import org.junit.Before; import org.junit.Test; @@ -34,14 +35,16 @@ import org.springframework.web.bind.annotation.RequestParam; import com.fasterxml.jackson.annotation.JsonAutoDetect; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assume.assumeTrue; + import feign.MethodMetadata; import lombok.AllArgsConstructor; import lombok.NoArgsConstructor; import lombok.ToString; -import static org.junit.Assert.assertEquals; -import static org.junit.Assume.assumeTrue; - /** * @author chadjaros */ @@ -97,10 +100,10 @@ public class SpringMvcContractTests { @Test public void testProcessAnnotations_Class_AnnotationsGetSpecificTest() throws Exception { - Method method = TestTemplate_Class_Annotations.class.getDeclaredMethod( - "getSpecificTest", String.class, String.class); - MethodMetadata data = this.contract.parseAndValidateMetadata( - method.getDeclaringClass(), method); + Method method = TestTemplate_Class_Annotations.class + .getDeclaredMethod("getSpecificTest", String.class, String.class); + MethodMetadata data = this.contract + .parseAndValidateMetadata(method.getDeclaringClass(), method); assertEquals("/prepend/{classId}/test/{testId}", data.template().url()); assertEquals("GET", data.template().method()); @@ -111,10 +114,10 @@ public class SpringMvcContractTests { @Test public void testProcessAnnotations_Class_AnnotationsGetAllTests() throws Exception { - Method method = TestTemplate_Class_Annotations.class.getDeclaredMethod( - "getAllTests", String.class); - MethodMetadata data = this.contract.parseAndValidateMetadata( - method.getDeclaringClass(), method); + Method method = TestTemplate_Class_Annotations.class + .getDeclaredMethod("getAllTests", String.class); + MethodMetadata data = this.contract + .parseAndValidateMetadata(method.getDeclaringClass(), method); assertEquals("/prepend/{classId}", data.template().url()); assertEquals("GET", data.template().method()); @@ -129,10 +132,10 @@ public class SpringMvcContractTests { MethodMetadata extendedData = this.contract.parseAndValidateMetadata( extendedMethod.getDeclaringClass(), extendedMethod); - Method method = TestTemplate_Class_Annotations.class.getDeclaredMethod( - "getAllTests", String.class); - MethodMetadata data = this.contract.parseAndValidateMetadata( - method.getDeclaringClass(), method); + Method method = TestTemplate_Class_Annotations.class + .getDeclaredMethod("getAllTests", String.class); + MethodMetadata data = this.contract + .parseAndValidateMetadata(method.getDeclaringClass(), method); assertEquals(extendedData.template().url(), data.template().url()); assertEquals(extendedData.template().method(), data.template().method()); @@ -193,6 +196,7 @@ public class SpringMvcContractTests { assertEquals("Authorization", data.indexToName().get(0).iterator().next()); assertEquals("id", data.indexToName().get(1).iterator().next()); assertEquals("amount", data.indexToName().get(2).iterator().next()); + assertNotNull(data.indexToExpander().get(2)); assertEquals("{Authorization}", data.template().headers().get("Authorization").iterator().next()); @@ -245,6 +249,19 @@ public class SpringMvcContractTests { data.template().headers().get("Accept").iterator().next()); } + @Test + public void testProcessAnnotations_ListParams() throws Exception { + Method method = TestTemplate_ListParams.class.getDeclaredMethod("getTest", + List.class); + MethodMetadata data = this.contract + .parseAndValidateMetadata(method.getDeclaringClass(), method); + + assertEquals("/test", data.template().url()); + assertEquals("GET", data.template().method()); + assertEquals("[{id}]", data.template().queries().get("id").toString()); + assertNull(data.indexToExpander().get(0)); + } + @Test public void testProcessHeaders() throws Exception { Method method = TestTemplate_Headers.class.getDeclaredMethod("getTest", @@ -339,6 +356,11 @@ public class SpringMvcContractTests { ResponseEntity getTest(@PathVariable("id") String id); } + public interface TestTemplate_ListParams { + @RequestMapping(value = "/test", method = RequestMethod.GET) + ResponseEntity getTest(@RequestParam("id") List id); + } + @JsonAutoDetect @RequestMapping("/advanced") public interface TestTemplate_Advanced { diff --git a/spring-cloud-netflix-dependencies/pom.xml b/spring-cloud-netflix-dependencies/pom.xml index 5b466c56..93dc9d3c 100644 --- a/spring-cloud-netflix-dependencies/pom.xml +++ b/spring-cloud-netflix-dependencies/pom.xml @@ -16,7 +16,7 @@ 1.1.2.BUILD-SNAPSHOT 1.1.2.BUILD-SNAPSHOT - 1.0.2.BUILD-SNAPSHOT + 1.0.3.BUILD-SNAPSHOT 0.7.4 1.4.8 8.16.2