Deprecate QueryMap.encode, remove processing of the "encode" parameter (

#1551) Co-authored-by: Marvin Froeder <velo@users.noreply.github.com>
OpenFeign · Mar 20, 2022 · 55b35f7 · 55b35f7
1 parent 04a85e6
commit 55b35f7
Show file tree

Hide file tree

Showing 6 changed files with 13 additions and 79 deletions.
diff --git a/core/src/main/java/feign/Contract.java b/core/src/main/java/feign/Contract.java
@@ -310,7 +310,6 @@ public Default() {
         checkState(data.queryMapIndex() == null,
             "QueryMap annotation was present on multiple parameters.");
         data.queryMapIndex(paramIndex);
-        data.queryMapEncoded(queryMap.encoded());
       });
       super.registerParameterAnnotation(HeaderMap.class, (queryMap, data, paramIndex) -> {
         checkState(data.headerMapIndex() == null,

diff --git a/core/src/main/java/feign/MethodMetadata.java b/core/src/main/java/feign/MethodMetadata.java
@@ -29,7 +29,6 @@ public final class MethodMetadata implements Serializable {
   private Integer bodyIndex;
   private Integer headerMapIndex;
   private Integer queryMapIndex;
-  private boolean queryMapEncoded;
   private boolean alwaysEncodeBody;
   private transient Type bodyType;
   private final RequestTemplate template = new RequestTemplate();
@@ -110,15 +109,6 @@ public MethodMetadata queryMapIndex(Integer queryMapIndex) {
     return this;
   }
 
-  public boolean queryMapEncoded() {
-    return queryMapEncoded;
-  }
-
-  public MethodMetadata queryMapEncoded(boolean queryMapEncoded) {
-    this.queryMapEncoded = queryMapEncoded;
-    return this;
-  }
-
   @Experimental
   public boolean alwaysEncodeBody() {
     return alwaysEncodeBody;

diff --git a/core/src/main/java/feign/QueryMap.java b/core/src/main/java/feign/QueryMap.java
@@ -53,15 +53,21 @@
  * Once this conversion is applied, the query keys and resulting String values follow the same
  * contract as if they were set using {@link RequestTemplate#query(String, String...)}.
  */
-@SuppressWarnings("deprecation")
 @Retention(RUNTIME)
 @java.lang.annotation.Target(PARAMETER)
 public @interface QueryMap {
 
   /**
    * Specifies whether parameter names and values are already encoded.
+   * <p>
+   * Deprecation: there are two options
+   * <ul>
+   * <li>if name or value are already encoded we do nothing;</li>
+   * <li>if name or value are not encoded we encode them.</li>
+   * </ul>
    *
    * @see Param#encoded
+   * @deprecated
    */
   boolean encoded() default false;
 }
diff --git a/core/src/main/java/feign/ReflectiveFeign.java b/core/src/main/java/feign/ReflectiveFeign.java
@@ -304,29 +304,25 @@ private RequestTemplate addQueryMapQueryParameters(Map<String, Object> queryMap,
       for (Entry<String, Object> currEntry : queryMap.entrySet()) {
         Collection<String> values = new ArrayList<String>();
 
-        boolean encoded = metadata.queryMapEncoded();
         Object currValue = currEntry.getValue();
         if (currValue instanceof Iterable<?>) {
           Iterator<?> iter = ((Iterable<?>) currValue).iterator();
           while (iter.hasNext()) {
             Object nextObject = iter.next();
-            values.add(nextObject == null ? null
-                : encoded ? nextObject.toString()
-                    : UriUtils.encode(nextObject.toString()));
+            values.add(nextObject == null ? null : UriUtils.encode(nextObject.toString()));
           }
         } else if (currValue instanceof Object[]) {
           for (Object value : (Object[]) currValue) {
-            values.add(value == null ? null
-                : encoded ? value.toString() : UriUtils.encode(value.toString()));
+            values.add(value == null ? null : UriUtils.encode(value.toString()));
           }
         } else {
           if (currValue != null) {
-            values.add(encoded ? currValue.toString() : UriUtils.encode(currValue.toString()));
+            values.add(UriUtils.encode(currValue.toString()));
           }
         }
 
         if (values.size() > 0) {
-          mutable.query(encoded ? currEntry.getKey() : UriUtils.encode(currEntry.getKey()), values);
+          mutable.query(UriUtils.encode(currEntry.getKey()), values);
         }
       }
       return mutable;

diff --git a/core/src/test/java/feign/DefaultContractTest.java b/core/src/test/java/feign/DefaultContractTest.java
@@ -326,30 +326,6 @@ public void queryMap() throws Exception {
     assertThat(md.queryMapIndex()).isEqualTo(0);
   }
 
-  @Test
-  public void queryMapEncodedDefault() throws Exception {
-    final MethodMetadata md =
-        parseAndValidateMetadata(QueryMapTestInterface.class, "queryMap", Map.class);
-
-    assertThat(md.queryMapEncoded()).isFalse();
-  }
-
-  @Test
-  public void queryMapEncodedTrue() throws Exception {
-    final MethodMetadata md =
-        parseAndValidateMetadata(QueryMapTestInterface.class, "queryMapEncoded", Map.class);
-
-    assertThat(md.queryMapEncoded()).isTrue();
-  }
-
-  @Test
-  public void queryMapEncodedFalse() throws Exception {
-    final MethodMetadata md =
-        parseAndValidateMetadata(QueryMapTestInterface.class, "queryMapNotEncoded", Map.class);
-
-    assertThat(md.queryMapEncoded()).isFalse();
-  }
-
   @Test
   public void queryMapMapSubclass() throws Exception {
     final MethodMetadata md =
@@ -388,24 +364,6 @@ public void queryMapPojoObject() throws Exception {
     assertThat(md.queryMapIndex()).isEqualTo(0);
   }
 
-  @Test
-  public void queryMapPojoObjectEncoded() throws Exception {
-    final MethodMetadata md =
-        parseAndValidateMetadata(QueryMapTestInterface.class, "pojoObjectEncoded", Object.class);
-
-    assertThat(md.queryMapIndex()).isEqualTo(0);
-    assertThat(md.queryMapEncoded()).isTrue();
-  }
-
-  @Test
-  public void queryMapPojoObjectNotEncoded() throws Exception {
-    final MethodMetadata md =
-        parseAndValidateMetadata(QueryMapTestInterface.class, "pojoObjectNotEncoded", Object.class);
-
-    assertThat(md.queryMapIndex()).isEqualTo(0);
-    assertThat(md.queryMapEncoded()).isFalse();
-  }
-
   @Test
   public void slashAreEncodedWhenNeeded() throws Exception {
     MethodMetadata md = parseAndValidateMetadata(SlashNeedToBeEncoded.class,
@@ -645,21 +603,9 @@ interface QueryMapTestInterface {
     @RequestLine("POST /")
     void queryMapMapSubclass(@QueryMap SortedMap<String, String> queryMap);
 
-    @RequestLine("POST /")
-    void queryMapEncoded(@QueryMap(encoded = true) Map<String, String> queryMap);
-
-    @RequestLine("POST /")
-    void queryMapNotEncoded(@QueryMap(encoded = false) Map<String, String> queryMap);
-
     @RequestLine("POST /")
     void pojoObject(@QueryMap Object object);
 
-    @RequestLine("POST /")
-    void pojoObjectEncoded(@QueryMap(encoded = true) Object object);
-
-    @RequestLine("POST /")
-    void pojoObjectNotEncoded(@QueryMap(encoded = false) Object object);
-
     // invalid
     @RequestLine("POST /")
     void multipleQueryMap(@QueryMap Map<String, String> mapOne,

diff --git a/core/src/test/java/feign/FeignTest.java b/core/src/test/java/feign/FeignTest.java
@@ -412,14 +412,14 @@ public void queryMapValueStartingWithBrace() throws Exception {
     server.enqueue(new MockResponse());
     queryMap = new LinkedHashMap<String, Object>();
     queryMap.put("name", "%7Balice");
-    api.queryMapEncoded(queryMap);
+    api.queryMap(queryMap);
     assertThat(server.takeRequest())
         .hasPath("/?name=%7Balice");
 
     server.enqueue(new MockResponse());
     queryMap = new LinkedHashMap<String, Object>();
     queryMap.put("%7Bname", "%7Balice");
-    api.queryMapEncoded(queryMap);
+    api.queryMap(queryMap);
     assertThat(server.takeRequest())
         .hasPath("/?%7Bname=%7Balice");
   }
@@ -1021,9 +1021,6 @@ void form(
     @RequestLine("GET /")
     void queryMap(@QueryMap Map<String, Object> queryMap);
 
-    @RequestLine("GET /")
-    void queryMapEncoded(@QueryMap(encoded = true) Map<String, Object> queryMap);
-
     @RequestLine("GET /?name={name}")
     void queryMapWithQueryParams(@Param("name") String name,
                                  @QueryMap Map<String, Object> queryMap);