From 80bdec06a4cda5bb457783e9b4f569efe0d1ce9a Mon Sep 17 00:00:00 2001 From: Pierre Lakreb <2006408+pilak@users.noreply.github.com> Date: Wed, 4 Oct 2023 09:06:19 +0200 Subject: [PATCH] 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 Co-authored-by: Marvin Froeder --- core/src/main/java/feign/BaseBuilder.java | 2 +- core/src/main/java/feign/Contract.java | 1 + core/src/main/java/feign/MethodMetadata.java | 10 ++++++ core/src/main/java/feign/QueryMap.java | 24 ++++++++++++++ .../feign/RequestTemplateFactoryResolver.java | 10 +++--- core/src/test/java/feign/ChildPojo.java | 9 ++++++ core/src/test/java/feign/FeignTest.java | 32 ++++++++++++++++++- 7 files changed, 82 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/feign/BaseBuilder.java b/core/src/main/java/feign/BaseBuilder.java index b30b245bf..e89681476 100644 --- a/core/src/main/java/feign/BaseBuilder.java +++ b/core/src/main/java/feign/BaseBuilder.java @@ -44,7 +44,7 @@ public abstract class BaseBuilder, 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 = diff --git a/core/src/main/java/feign/Contract.java b/core/src/main/java/feign/Contract.java index cf966a6f6..225664687 100644 --- a/core/src/main/java/feign/Contract.java +++ b/core/src/main/java/feign/Contract.java @@ -315,6 +315,7 @@ public Default() { 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, diff --git a/core/src/main/java/feign/MethodMetadata.java b/core/src/main/java/feign/MethodMetadata.java index 899f5571b..ef002e695 100644 --- a/core/src/main/java/feign/MethodMetadata.java +++ b/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; diff --git a/core/src/main/java/feign/QueryMap.java b/core/src/main/java/feign/QueryMap.java index eef0a4320..91b127eee 100644 --- a/core/src/main/java/feign/QueryMap.java +++ b/core/src/main/java/feign/QueryMap.java @@ -16,6 +16,8 @@ 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 @@ * @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; + } + } } diff --git a/core/src/main/java/feign/RequestTemplateFactoryResolver.java b/core/src/main/java/feign/RequestTemplateFactoryResolver.java index 84743b04d..6dd69de89 100644 --- a/core/src/main/java/feign/RequestTemplateFactoryResolver.java +++ b/core/src/main/java/feign/RequestTemplateFactoryResolver.java @@ -108,26 +108,28 @@ 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 queryMap = toQueryMap(value); + Map 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 headerMap = toQueryMap(value); + Map headerMap = toQueryMap(value, metadata.queryMapEncoder()); template = addHeaderMapHeaders(headerMap, template); } return template; } - private Map toQueryMap(Object value) { + private Map toQueryMap(Object value, QueryMapEncoder queryMapEncoder) { if (value instanceof Map) { return (Map) 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); } diff --git a/core/src/test/java/feign/ChildPojo.java b/core/src/test/java/feign/ChildPojo.java index a6bf2ea39..f620724e2 100644 --- a/core/src/test/java/feign/ChildPojo.java +++ b/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; diff --git a/core/src/test/java/feign/FeignTest.java b/core/src/test/java/feign/FeignTest.java index af6724f4e..cdc5358a7 100755 --- a/core/src/test/java/feign/FeignTest.java +++ b/core/src/test/java/feign/FeignTest.java @@ -16,6 +16,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; @@ -924,6 +925,7 @@ 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); @@ -931,7 +933,31 @@ public void queryMap_with_child_pojo() throws Exception { .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 @@ void queryMapWithQueryParams(@Param("name") String name, @RequestLine("GET /") void queryMapPropertyPojo(@QueryMap PropertyPojo object); + @RequestLine("GET /") + void queryMapPropertyInheritenceWithBeanMapEncoder(@QueryMap( + mapEncoder = MapEncoder.BEAN) ChildPojo object); + @RequestLine("GET /") void queryMapPropertyInheritence(@QueryMap ChildPojo object);