Supports query params without values

jebeaudet · jebeaudet · commit 45770824957e · 2015-02-21T17:57:26.000Z
Fixes NPE when building a client with a query param with no values
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,7 @@
 * Adds Request.Options support to RibbonClient
 * Updates to Ribbon 2.0-RC13
 * Updates to Jackson 2.5.1
+* Supports query parameters without values
 
 ### Version 7.2
 * Adds `Feign.Builder.build()`
diff --git a/core/src/main/java/feign/RequestTemplate.java b/core/src/main/java/feign/RequestTemplate.java
@@ -145,11 +145,7 @@ private static Map<String, Collection<String>> parseAndDecodeQueries(String quer
       return map;
     }
     if (queryLine.indexOf('&') == -1) {
-      if (queryLine.indexOf('=') != -1) {
-        putKV(queryLine, map);
-      } else {
-        map.put(queryLine, null);
-      }
+      putKV(queryLine, map);
     } else {
       char[] chars = queryLine.toCharArray();
       int start = 0;
@@ -504,7 +500,7 @@ private StringBuilder pullAnyQueriesOutOfUrl(StringBuilder url) {
   }
 
   private boolean allValuesAreNull(Collection<String> values) {
-    if (values.isEmpty()) {
+    if (values == null || values.isEmpty()) {
       return true;
     }
     for (String val : values) {
diff --git a/core/src/main/java/feign/Util.java b/core/src/main/java/feign/Util.java
@@ -139,7 +139,7 @@ public static <T> T[] toArray(Iterable<? extends T> iterable, Class<T> type) {
    * Returns an unmodifiable collection which may be empty, but is never null.
    */
   public static <T> Collection<T> valuesOrEmpty(Map<String, Collection<T>> map, String key) {
-    return map.containsKey(key) ? map.get(key) : Collections.<T>emptyList();
+    return map.containsKey(key) && map.get(key) != null ? map.get(key) : Collections.<T>emptyList();
   }
 
   public static void ensureClosed(Closeable closeable) {
diff --git a/core/src/test/java/feign/DefaultContractTest.java b/core/src/test/java/feign/DefaultContractTest.java
@@ -22,6 +22,7 @@
 import org.junit.rules.ExpectedException;
 
 import java.net.URI;
+import java.util.Collections;
 import java.util.Date;
 import java.util.List;
 
@@ -124,14 +125,31 @@ public void queryParamsInPathExtract() throws Exception {
         );
 
     assertThat(
-        contract.parseAndValidatateMetadata(WithQueryParamsInPath.class.getDeclaredMethod("empty"))
+        contract.parseAndValidatateMetadata(WithQueryParamsInPath.class.getDeclaredMethod("twoAndOneEmpty"))
             .template())
         .hasUrl("/")
         .hasQueries(
             entry("flag", asList(new String[]{null})),
             entry("Action", asList("GetUser")),
             entry("Version", asList("2010-05-08"))
         );
+
+    assertThat(
+        contract.parseAndValidatateMetadata(WithQueryParamsInPath.class.getDeclaredMethod("oneEmpty"))
+            .template())
+        .hasUrl("/")
+        .hasQueries(
+            entry("flag", asList(new String[]{null}))
+        );
+
+    assertThat(
+        contract.parseAndValidatateMetadata(WithQueryParamsInPath.class.getDeclaredMethod("twoEmpty"))
+            .template())
+        .hasUrl("/")
+        .hasQueries(
+            entry("flag", asList(new String[]{null})),
+            entry("NoErrors", asList(new String[]{null}))
+        ); 
   }
 
   @Test
@@ -307,7 +325,13 @@ interface WithQueryParamsInPath {
     Response three();
 
     @RequestLine("GET /?flag&Action=GetUser&Version=2010-05-08")
-    Response empty();
+    Response twoAndOneEmpty();
+
+    @RequestLine("GET /?flag")
+    Response oneEmpty();
+
+    @RequestLine("GET /?flag&NoErrors")
+    Response twoEmpty();
   }
 
   interface BodyWithoutParameters {

Original file line number	Diff line number	Diff line change
`@@ -139,7 +139,7 @@ public static <T> T[] toArray(Iterable<? extends T> iterable, Class<T> type) {`
`139`	`139`	`* Returns an unmodifiable collection which may be empty, but is never null.`
`140`	`140`	`*/`
`141`	`141`	`public static <T> Collection<T> valuesOrEmpty(Map<String, Collection<T>> map, String key) {`
`142`		`- return map.containsKey(key) ? map.get(key) : Collections.<T>emptyList();`
	`142`	`+ return map.containsKey(key) && map.get(key) != null ? map.get(key) : Collections.<T>emptyList();`
`143`	`143`	`}`
`144`	`144`
`145`	`145`	`public static void ensureClosed(Closeable closeable) {`