fix: Undefined value in @QueryMap params works not as described (#1585)

Co-authored-by: Marvin Froeder <velo@users.noreply.github.com>
OpenFeign · Mar 6, 2022 · a6fc182 · a6fc182
1 parent 7146e35
commit a6fc182
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 3 deletions.
diff --git a/core/src/main/java/feign/ReflectiveFeign.java b/core/src/main/java/feign/ReflectiveFeign.java
@@ -320,11 +320,14 @@ private RequestTemplate addQueryMapQueryParameters(Map<String, Object> queryMap,
                 : encoded ? value.toString() : UriUtils.encode(value.toString()));
           }
         } else {
-          values.add(currValue == null ? null
-              : encoded ? currValue.toString() : UriUtils.encode(currValue.toString()));
+          if (currValue != null) {
+            values.add(encoded ? currValue.toString() : UriUtils.encode(currValue.toString()));
+          }
         }
 
-        mutable.query(encoded ? currEntry.getKey() : UriUtils.encode(currEntry.getKey()), values);
+        if (values.size() > 0) {
+          mutable.query(encoded ? currEntry.getKey() : UriUtils.encode(currEntry.getKey()), values);
+        }
       }
       return mutable;
     }

diff --git a/core/src/test/java/feign/FeignTest.java b/core/src/test/java/feign/FeignTest.java
@@ -314,6 +314,36 @@ public void queryMap() throws Exception {
         .hasPath("/?name=alice&fooKey=fooValue");
   }
 
+  @Test
+  public void queryMapWithNull() throws Exception {
+    server.enqueue(new MockResponse());
+
+    TestInterface api = new TestInterfaceBuilder().target("http://localhost:" + server.getPort());
+
+    Map<String, Object> queryMap = new LinkedHashMap<>();
+    queryMap.put("name", "alice");
+    queryMap.put("fooKey", null);
+    api.queryMap(queryMap);
+
+    assertThat(server.takeRequest())
+        .hasPath("/?name=alice");
+  }
+
+  @Test
+  public void queryMapWithEmpty() throws Exception {
+    server.enqueue(new MockResponse());
+
+    TestInterface api = new TestInterfaceBuilder().target("http://localhost:" + server.getPort());
+
+    Map<String, Object> queryMap = new LinkedHashMap<>();
+    queryMap.put("name", "alice");
+    queryMap.put("fooKey", "");
+    api.queryMap(queryMap);
+
+    assertThat(server.takeRequest())
+        .hasPath("/?name=alice&fooKey");
+  }
+
   @Test
   public void queryMapIterableValuesExpanded() throws Exception {
     server.enqueue(new MockResponse());