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

@@ -44,7 +44,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

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

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 final class MethodMetadata implements Serializable {
     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 @@ package feign;
 import java.lang.annotation.Retention;
 import java.util.List;
 import java.util.Map;
+import feign.querymap.BeanQueryMapEncoder;
+import feign.querymap.FieldQueryMapEncoder;
 import static java.lang.annotation.ElementType.PARAMETER;
 import static java.lang.annotation.RetentionPolicy.RUNTIME;
 
@@ -70,4 +72,26 @@ public @interface QueryMap {
    * @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

@@ -108,26 +108,28 @@ final class RequestTemplateFactoryResolver {
         // 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 @@ package feign;
 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

@@ -16,6 +16,7 @@ package feign;
 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;
@@ -924,6 +925,7 @@ public class FeignTest {
     childPojo.setChildPrivateProperty("first");
     childPojo.setParentProtectedProperty("second");
     childPojo.setParentPublicProperty("third");
+    childPojo.setParentPrivatePropertyAlteredByGetter("fourth");
 
     server.enqueue(new MockResponse());
     api.queryMapPropertyInheritence(childPojo);
@@ -931,7 +933,31 @@ public class FeignTest {
         .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
@@ -1210,6 +1236,10 @@ public class FeignTest {
     @RequestLine("GET /")
     void queryMapPropertyPojo(@QueryMap PropertyPojo object);
 
+    @RequestLine("GET /")
+    void queryMapPropertyInheritenceWithBeanMapEncoder(@QueryMap(
+        mapEncoder = MapEncoder.BEAN) ChildPojo object);
+
     @RequestLine("GET /")
     void queryMapPropertyInheritence(@QueryMap ChildPojo object);