From 25d47445dc57ce3ad0a12c0f14c5eb67eb9d4c7b Mon Sep 17 00:00:00 2001
From: adriancole <adrian.f.cole@gmail.com>
Date: Wed, 14 Aug 2013 12:25:41 -0700
Subject: [PATCH] Skip query template parameters when corresponding java arg is
 null

---
 CHANGES.md                                    |  1 +
 .../src/main/java/feign/RequestTemplate.java  | 37 +++++++++++++++++--
 .../test/java/feign/RequestTemplateTest.java  | 32 +++++++++++++++-
 feign-jaxrs/README.md                         |  2 +-
 4 files changed, 66 insertions(+), 6 deletions(-)

diff --git a/CHANGES.md b/CHANGES.md
index 5e5c24ca..6f156f77 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -1,5 +1,6 @@
 ### Version 4.2/3.3
 * Document and enforce JAX-RS annotation processing from server POV
+* Skip query template parameters when corresponding java arg is null
 
 ### Version 4.1/3.2
 * update to dagger 1.1
diff --git a/feign-core/src/main/java/feign/RequestTemplate.java b/feign-core/src/main/java/feign/RequestTemplate.java
index 5b2b9a46..f3081a19 100644
--- a/feign-core/src/main/java/feign/RequestTemplate.java
+++ b/feign-core/src/main/java/feign/RequestTemplate.java
@@ -23,6 +23,7 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -98,9 +99,7 @@ public final class RequestTemplate implements Serializable {
     for (Entry<String, ?> entry : unencoded.entrySet()) {
       encoded.put(entry.getKey(), urlEncode(String.valueOf(entry.getValue())));
     }
-    String queryLine = expand(queryLine(), encoded);
-    queries.clear();
-    pullAnyQueriesOutOfUrl(new StringBuilder(queryLine));
+    replaceQueryValues(encoded);
     String resolvedUrl = expand(url.toString(), encoded).replace("%2F", "/");
     url = new StringBuilder(resolvedUrl);
 
@@ -505,6 +504,37 @@ public final class RequestTemplate implements Serializable {
     return request().toString();
   }
 
+  /**
+   * Replaces query values which are templated with corresponding values from the {@code unencoded} map.
+   * Any unresolved queries are removed.
+   */
+  public void replaceQueryValues(Map<String, ?> unencoded) {
+    Iterator<Entry<String, Collection<String>>> iterator = queries.entrySet().iterator();
+    while (iterator.hasNext()) {
+      Entry<String, Collection<String>> entry = iterator.next();
+      if (entry.getValue() == null) {
+        continue;
+      }
+      Collection<String> values = new ArrayList<String>();
+      for (String value : entry.getValue()) {
+        if (value.indexOf('{') == 0 && value.indexOf('}') == value.length() - 1) {
+          Object variableValue = unencoded.get(value.substring(1, value.length() - 1));
+          // only add non-null expressions
+          if (variableValue != null) {
+            values.add(String.valueOf(variableValue));
+          }
+        } else {
+          values.add(value);
+        }
+      }
+      if (values.isEmpty()) {
+        iterator.remove();
+      } else {
+        entry.setValue(values);
+      }
+    }
+  }
+
   public String queryLine() {
     if (queries.isEmpty())
       return "";
@@ -524,6 +554,5 @@ public final class RequestTemplate implements Serializable {
     return queryBuilder.insert(0, '?').toString();
   }
 
-
   private static final long serialVersionUID = 1L;
 }
diff --git a/feign-core/src/test/java/feign/RequestTemplateTest.java b/feign-core/src/test/java/feign/RequestTemplateTest.java
index 173ce535..ffc13e9d 100644
--- a/feign-core/src/test/java/feign/RequestTemplateTest.java
+++ b/feign-core/src/test/java/feign/RequestTemplateTest.java
@@ -18,7 +18,6 @@ package feign;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableListMultimap;
 import com.google.common.collect.ImmutableMap;
-
 import org.testng.annotations.Test;
 
 import static feign.RequestTemplate.expand;
@@ -133,4 +132,35 @@ public class RequestTemplateTest {
         + "\n" //
         + "{\"customer_name\": \"netflix\", \"user_name\": \"denominator\", \"password\": \"password\"}");
   }
+
+  @Test public void skipUnresolvedQueries() throws Exception {
+    RequestTemplate template = new RequestTemplate().method("GET")//
+        .append("/domains/{domainId}/records")//
+        .query("optional", "{optional}")//
+        .query("name", "{nameVariable}");
+
+    template = template.resolve(ImmutableMap.<String, Object>builder()//
+        .put("domainId", 1001)//
+        .put("nameVariable", "denominator.io")//
+        .build()
+    );
+
+    assertEquals(template.toString(), ""//
+        + "GET /domains/1001/records?name=denominator.io HTTP/1.1\n");
+  }
+
+  @Test public void allQueriesUnresolvable() throws Exception {
+    RequestTemplate template = new RequestTemplate().method("GET")//
+        .append("/domains/{domainId}/records")//
+        .query("optional", "{optional}")//
+        .query("optional2", "{optional2}");
+
+    template = template.resolve(ImmutableMap.<String, Object>builder()//
+        .put("domainId", 1001)//
+        .build()
+    );
+
+    assertEquals(template.toString(), ""//
+        + "GET /domains/1001/records HTTP/1.1\n");
+  }
 }
diff --git a/feign-jaxrs/README.md b/feign-jaxrs/README.md
index 5f53d92f..5026c7ac 100644
--- a/feign-jaxrs/README.md
+++ b/feign-jaxrs/README.md
@@ -30,7 +30,7 @@ Adds the first value as the `Content-Type` header.
 #### `@PathParam`
 Links the value of the corresponding parameter to a template variable declared in the path.
 #### `@QueryParam`
-Links the value of the corresponding parameter to a query parameter.
+Links the value of the corresponding parameter to a query parameter.  When invoked, null will skip the query param.
 #### `@HeaderParam`
 Links the value of the corresponding parameter to a header.
 #### `@FormParam`