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

mroccyen · velo · web-flow · commit a6fc182d453f · 2022-03-06T21:39:55.000Z
Co-authored-by: Marvin Froeder &lt;velo@users.noreply.github.com&gt;
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());

Original file line number	Diff line number	Diff line change
`@@ -320,11 +320,14 @@ private RequestTemplate addQueryMapQueryParameters(Map<String, Object> queryMap,`
`320`	`320`	`: encoded ? value.toString() : UriUtils.encode(value.toString()));`
`321`	`321`	`}`
`322`	`322`	`} else {`
`323`		`- values.add(currValue == null ? null`
`324`		`- : encoded ? currValue.toString() : UriUtils.encode(currValue.toString()));`
	`323`	`+ if (currValue != null) {`
	`324`	`+ values.add(encoded ? currValue.toString() : UriUtils.encode(currValue.toString()));`
	`325`	`+ }`
`325`	`326`	`}`
`326`	`327`
`327`		`- mutable.query(encoded ? currEntry.getKey() : UriUtils.encode(currEntry.getKey()), values);`
	`328`	`+ if (values.size() > 0) {`
	`329`	`+ mutable.query(encoded ? currEntry.getKey() : UriUtils.encode(currEntry.getKey()), values);`
	`330`	`+ }`
`328`	`331`	`}`
`329`	`332`	`return mutable;`
`330`	`333`	`}`