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

#1551) Co-authored-by: Marvin Froeder <velo@users.noreply.github.com>
OpenFeign · Mar 20, 2022 · 7eb3dc6 · 7eb3dc6
1 parent 6072c13
commit 7eb3dc6
Show file tree

Hide file tree

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

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
@@ -56,15 +56,22 @@
  * 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>if name or value are not encoded we encode them.
+   * </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
@@ -313,32 +313,25 @@ private RequestTemplate addQueryMapQueryParameters(
       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
@@ -313,30 +313,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 =
@@ -375,24 +351,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 =
@@ -631,21 +589,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(

diff --git a/core/src/test/java/feign/FeignTest.java b/core/src/test/java/feign/FeignTest.java
@@ -403,13 +403,13 @@ 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");
   }
 
@@ -1055,9 +1055,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);