Merge pull request #1626 from ryanjbaxter/apply-formatter-to-collections-1526

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/SpringMvcContract.java

@@ -241,7 +241,6 @@ 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);
@@ -249,13 +248,6 @@ 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");

spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/support/SpringMvcContractTests.java

@@ -294,7 +294,7 @@ public class SpringMvcContractTests {
 		assertEquals("/test", data.template().url());
 		assertEquals("GET", data.template().method());
 		assertEquals("[{id}]", data.template().queries().get("id").toString());
-		assertNull(data.indexToExpander().get(0));
+		assertNotNull(data.indexToExpander().get(0));
 	}
 
 	@Test

spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignClientTests.java

@@ -29,6 +29,7 @@ import java.lang.reflect.Proxy;
 import java.text.ParseException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.List;
 import java.util.Locale;
 import java.util.concurrent.Future;
@@ -138,6 +139,11 @@ public class FeignClientTests {
 		public OtherArg(String value) {
 			this.value = value;
 		}
+
+		@Override
+		public String toString() {
+			return value;
+		}
 	}
 
 	@FeignClient(name = "localapp", configuration = TestClientConfig.class)
@@ -186,6 +192,9 @@ public class FeignClientTests {
 
 		@RequestMapping(method = RequestMethod.GET, path = "/tostring2")
 		String getToString(@RequestParam("arg") OtherArg arg);
+
+		@RequestMapping(method = RequestMethod.GET, path = "/tostringcollection")
+		Collection<String> getToString(@RequestParam("arg") Collection<OtherArg> args);
 	}
 
 	public static class TestClientConfig {
@@ -325,6 +334,9 @@ public class FeignClientTests {
 
 						@Override
 						public String print(OtherArg object, Locale locale) {
+							if("foo".equals(object.value)) {
+								return "bar";
+							}
 							return object.value;
 						}
 
@@ -417,6 +429,15 @@ public class FeignClientTests {
 			return arg.value;
 		}
 
+		@RequestMapping(method = RequestMethod.GET, path = "/tostringcollection")
+		Collection<String> getToString(@RequestParam("arg") Collection<OtherArg> args) {
+			List<String> result = new ArrayList<>();
+			for(OtherArg arg : args) {
+				result.add(arg.value);
+			}
+			return result;
+		}
+
 		public static void main(String[] args) {
 			new SpringApplicationBuilder(Application.class)
 					.properties("spring.application.name=feignclienttest",
@@ -570,7 +591,14 @@ public class FeignClientTests {
 		assertEquals(Arg.A.toString(), testClient.getToString(Arg.A));
 		assertEquals(Arg.B.toString(), testClient.getToString(Arg.B));
 
-		assertEquals("foo", testClient.getToString(new OtherArg("foo")));
+		assertEquals("bar", testClient.getToString(new OtherArg("foo")));
+		List<OtherArg> args = new ArrayList<>();
+		args.add(new OtherArg("foo"));
+		args.add(new OtherArg("goo"));
+		List<String> expectedResult = new ArrayList<>();
+		expectedResult.add("bar");
+		expectedResult.add("goo");
+		assertEquals(expectedResult, testClient.getToString(args));
 	}
 
 	@Test