Add @QueryMap mapEncoder attribute (#2098)

* use `mapEncoder` attribute at method level for what encoder to use
* still use builder `QueryMapEncoder` if no attribute specified

Co-authored-by: Pierre Lakreb <pierre.lakreb@smile.fr>
Co-authored-by: Marvin Froeder <velo@users.noreply.github.com>

core/src/main/java/feign/BaseBuilder.java

@@ -21,7 +21,6 @@
 import feign.codec.Decoder;
 import feign.codec.Encoder;
 import feign.codec.ErrorDecoder;
-import feign.querymap.FieldQueryMapEncoder;
 import java.lang.reflect.Field;
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
@@ -44,7 +43,7 @@ public abstract class BaseBuilder<B extends BaseBuilder<B, T>, T> implements Clo
   protected Decoder decoder = new Decoder.Default();
   protected boolean closeAfterDecode = true;
   protected boolean decodeVoid = false;
-  protected QueryMapEncoder queryMapEncoder = new FieldQueryMapEncoder();
+  protected QueryMapEncoder queryMapEncoder = QueryMap.MapEncoder.FIELD.instance();
   protected ErrorDecoder errorDecoder = new ErrorDecoder.Default();
   protected Options options = new Options();
   protected InvocationHandlerFactory invocationHandlerFactory =

core/src/main/java/feign/Contract.java

@@ -341,6 +341,7 @@ public Default() {
                 data.queryMapIndex() == null,
                 "QueryMap annotation was present on multiple parameters.");
             data.queryMapIndex(paramIndex);
+            data.queryMapEncoder(queryMap.mapEncoder().instance());
           });
       super.registerParameterAnnotation(
           HeaderMap.class,

core/src/main/java/feign/MethodMetadata.java

@@ -29,6 +29,7 @@ public final class MethodMetadata implements Serializable {
   private Integer bodyIndex;
   private Integer headerMapIndex;
   private Integer queryMapIndex;
+  private QueryMapEncoder queryMapEncoder;
   private boolean alwaysEncodeBody;
   private transient Type bodyType;
   private final RequestTemplate template = new RequestTemplate();
@@ -109,6 +110,15 @@ public MethodMetadata queryMapIndex(Integer queryMapIndex) {
     return this;
   }
 
+  public QueryMapEncoder queryMapEncoder() {
+    return queryMapEncoder;
+  }
+
+  public MethodMetadata queryMapEncoder(QueryMapEncoder queryMapEncoder) {
+    this.queryMapEncoder = queryMapEncoder;
+    return this;
+  }
+
   @Experimental
   public boolean alwaysEncodeBody() {
     return alwaysEncodeBody;

core/src/main/java/feign/QueryMap.java

@@ -16,6 +16,8 @@
 import static java.lang.annotation.ElementType.PARAMETER;
 import static java.lang.annotation.RetentionPolicy.RUNTIME;
 
+import feign.querymap.BeanQueryMapEncoder;
+import feign.querymap.FieldQueryMapEncoder;
 import java.lang.annotation.Retention;
 import java.util.List;
 import java.util.Map;
@@ -74,4 +76,28 @@
    * @deprecated
    */
   boolean encoded() default false;
+
+  /**
+   * Specifies the QueryMapEncoder implementation to use to transform DTO into query map.
+   *
+   * @return the enum containing the instance of QueryMapEncoder
+   */
+  MapEncoder mapEncoder() default MapEncoder.DEFAULT;
+
+  public enum MapEncoder {
+    // the latter DEFAULT will use BaseBuilder instance
+    BEAN(new BeanQueryMapEncoder()),
+    FIELD(new FieldQueryMapEncoder()),
+    DEFAULT(null);
+
+    private QueryMapEncoder mapEncoder;
+
+    private MapEncoder(QueryMapEncoder mapEncoder) {
+      this.mapEncoder = mapEncoder;
+    }
+
+    public QueryMapEncoder instance() {
+      return mapEncoder;
+    }
+  }
 }

core/src/main/java/feign/RequestTemplateFactoryResolver.java

@@ -107,26 +107,29 @@ public RequestTemplate create(Object[] argv) {
         // add query map parameters after initial resolve so that they take
         // precedence over any predefined values
         Object value = argv[metadata.queryMapIndex()];
-        Map<String, Object> queryMap = toQueryMap(value);
+        Map<String, Object> queryMap = toQueryMap(value, metadata.queryMapEncoder());
         template = addQueryMapQueryParameters(queryMap, template);
       }
 
       if (metadata.headerMapIndex() != null) {
         // add header map parameters for a resolution of the user pojo object
         Object value = argv[metadata.headerMapIndex()];
-        Map<String, Object> headerMap = toQueryMap(value);
+        Map<String, Object> headerMap = toQueryMap(value, metadata.queryMapEncoder());
         template = addHeaderMapHeaders(headerMap, template);
       }
 
       return template;
     }
 
-    private Map<String, Object> toQueryMap(Object value) {
+    private Map<String, Object> toQueryMap(Object value, QueryMapEncoder queryMapEncoder) {
       if (value instanceof Map) {
         return (Map<String, Object>) value;
       }
       try {
-        return queryMapEncoder.encode(value);
+        // encode with @QueryMap annotation if exists otherwise with the one from this resolver
+        return queryMapEncoder != null
+            ? queryMapEncoder.encode(value)
+            : this.queryMapEncoder.encode(value);
       } catch (EncodeException e) {
         throw new IllegalStateException(e);
       }

core/src/test/java/feign/ChildPojo.java

@@ -16,6 +16,15 @@
 class ParentPojo {
   public String parentPublicProperty;
   protected String parentProtectedProperty;
+  private String parentPrivatePropertyAlteredByGetter;
+
+  public String getParentPrivatePropertyAlteredByGetter() {
+    return parentPrivatePropertyAlteredByGetter + "FromGetter";
+  }
+
+  public void setParentPrivatePropertyAlteredByGetter(String parentPrivatePropertyAlteredByGetter) {
+    this.parentPrivatePropertyAlteredByGetter = parentPrivatePropertyAlteredByGetter;
+  }
 
   public String getParentPublicProperty() {
     return parentPublicProperty;

core/src/test/java/feign/FeignTest.java

@@ -26,6 +26,7 @@
 import com.google.gson.Gson;
 import com.google.gson.reflect.TypeToken;
 import feign.Feign.ResponseMappingDecoder;
+import feign.QueryMap.MapEncoder;
 import feign.Request.HttpMethod;
 import feign.Target.HardCodedTarget;
 import feign.codec.DecodeException;
@@ -962,14 +963,40 @@ public void queryMap_with_child_pojo() throws Exception {
     childPojo.setChildPrivateProperty("first");
     childPojo.setParentProtectedProperty("second");
     childPojo.setParentPublicProperty("third");
+    childPojo.setParentPrivatePropertyAlteredByGetter("fourth");
 
     server.enqueue(new MockResponse());
     api.queryMapPropertyInheritence(childPojo);
     assertThat(server.takeRequest())
         .hasQueryParams(
             "parentPublicProperty=third",
             "parentProtectedProperty=second",
-            "childPrivateProperty=first");
+            "childPrivateProperty=first",
+            "parentPrivatePropertyAlteredByGetter=fourth");
+  }
+
+  @Test
+  public void queryMap_with_child_pojo_altered_by_getter_while_using_overriding_encoder()
+      throws Exception {
+    TestInterface api =
+        new TestInterfaceBuilder()
+            .queryMapEncoder(new FieldQueryMapEncoder())
+            .target("http://localhost:" + server.getPort());
+
+    ChildPojo childPojo = new ChildPojo();
+    childPojo.setChildPrivateProperty("first");
+    childPojo.setParentProtectedProperty("second");
+    childPojo.setParentPublicProperty("third");
+    childPojo.setParentPrivatePropertyAlteredByGetter("fourth");
+
+    server.enqueue(new MockResponse());
+    api.queryMapPropertyInheritenceWithBeanMapEncoder(childPojo);
+    assertThat(server.takeRequest())
+        .hasQueryParams(
+            "parentPublicProperty=third",
+            "parentProtectedProperty=second",
+            "childPrivateProperty=first",
+            "parentPrivatePropertyAlteredByGetter=fourthFromGetter");
   }
 
   @Test
@@ -1268,6 +1295,10 @@ void queryMapWithQueryParams(
     @RequestLine("GET /")
     void queryMapPropertyPojo(@QueryMap PropertyPojo object);
 
+    @RequestLine("GET /")
+    void queryMapPropertyInheritenceWithBeanMapEncoder(
+        @QueryMap(mapEncoder = MapEncoder.BEAN) ChildPojo object);
+
     @RequestLine("GET /")
     void queryMapPropertyInheritence(@QueryMap ChildPojo object);