Merge pull request #10 from CheKazan/master

Fix inserting @QueryParam to request body

pom.xml

@@ -22,7 +22,7 @@
 
     <groupId>ru.i-novus.components</groupId>
     <artifactId>feign-jaxrs</artifactId>
-    <version>3.1.2</version>
+    <version>3.1.3</version>
 
     <properties>
         <java.version>17</java.version>

src/main/java/com/qualys/feign/jaxrs/BeanParamEncoder.java

@@ -23,6 +23,7 @@
 import feign.codec.Encoder;
 import feign.template.*;
 import jakarta.ws.rs.PathParam;
+import jakarta.ws.rs.QueryParam;
 
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Type;
@@ -50,46 +51,54 @@ public void encode(Object object, Type bodyType, RequestTemplate template) throw
         if (template.methodMetadata().indexToExpander() == null)
             template.methodMetadata().indexToExpander(new HashMap<>());
 
-        boolean resolved = false;
         if (object instanceof Object[] objects && objects.length > 0) {
-            Object[] withoutPathParam = withoutPathParam(objects, template);
-            if (withoutPathParam.length < 1)
-                return;
-            for (Object internalObject : withoutPathParam) {
-                if (internalObject instanceof EncoderContext ctx) {
-                    if (ctx.values.size() == 1 && ctx.values.get(ctx.values.keySet().iterator().next()) instanceof Map<?,?> map) {
-                        encodeQueryMapParam(template, map, true);
-                    } else {
-                        resolve(template, ctx);
-                    }
-                    resolved = true;
-                }
-            }
-            if (!resolved) {
-                if (withoutPathParam[0] != null)
-                    this.delegate.encode(withoutPathParam[0], withoutPathParam[0].getClass(), template);
-                if (!template.queries().isEmpty() && withoutPathParam[0] instanceof Map<?,?> map) {
-                    encodeQueryMapParam(template, map, false);
+            Object[] withoutPathAndQueryParams = getWithoutPathAndQueryParams(objects, template);
+            if (withoutPathAndQueryParams.length < 1) return;
+            for (Object param : withoutPathAndQueryParams) {
+                if (param != null) {
+                    if (param instanceof EncoderContext ctx)
+                        resolveEncoderContext(ctx, template);
+                    else resolve(param, template);
                 }
             }
         }
     }
 
-    private Object[] withoutPathParam(Object[] objects, RequestTemplate template) {
-        List<Object> noPathParam = new ArrayList<>();
-        List<List<Annotation>> parameterAnnotations = new ArrayList<>();
+    private void resolveEncoderContext(EncoderContext ctx, RequestTemplate template) {
+        if (ctx.values.size() == 1 && ctx.values.get(ctx.values.keySet().iterator().next()) instanceof Map<?, ?> map) {
+            encodeQueryMapParam(template, map, true);
+        } else {
+            resolve(template, ctx);
+        }
+    }
 
+    private void resolve(Object param, RequestTemplate template) {
+        this.delegate.encode(param, param.getClass(), template);
+        if (template.queries().size() == 1 && param instanceof Map<?, ?> map) {
+            encodeQueryMapParam(template, map, false);
+        }
+    }
+
+    private Object[] getWithoutPathAndQueryParams(Object[] params, RequestTemplate template) {
+        List<Object> noPathAndQueryParams = new ArrayList<>();
+        List<List<Annotation>> paramAnnotations = getAnnotations(template);
+        for (int i = 0; i < params.length; i++) {
+            Object param = params[i];
+            if (paramAnnotations.get(i).stream().noneMatch(obj -> obj instanceof PathParam ||
+                    (obj instanceof QueryParam && !(param instanceof Map))))
+                noPathAndQueryParams.add(params[i]);
+        }
+        return noPathAndQueryParams.toArray(new Object[0]);
+    }
+
+    private List<List<Annotation>> getAnnotations(RequestTemplate template) {
+        List<List<Annotation>> parameterAnnotations = new ArrayList<>();
         for (Annotation[] annotations : template.methodMetadata().method().getParameterAnnotations())
             parameterAnnotations.add(Arrays.asList(annotations));
-
-        for (int i = 0; i < objects.length; i++)
-            if (parameterAnnotations.get(i).stream().noneMatch(PathParam.class::isInstance))
-                noPathParam.add(objects[i]);
-
-        return noPathParam.toArray(new Object[0]);
+        return parameterAnnotations;
     }
 
-    private void encodeQueryMapParam(RequestTemplate template, Map<?,?> params, boolean runEncoder) {
+    private void encodeQueryMapParam(RequestTemplate template, Map<?, ?> params, boolean runEncoder) {
         if (runEncoder)
             this.delegate.encode(params, Map.class, template);
 

src/test/groovy/com/qualys/feign/jaxrs/BeanParamTest.groovy

@@ -98,7 +98,10 @@ class BeanParamTest extends Specification {
         client.mapQueryParam(Map.of("testParam1", "ing", "testParam2", "ing2"))
 
         then:
-        sent.url() == "http://localhost/mapQueryParam?map=%7B%0D%0A%20%20%22testParam2%22%20%3A%20%22ing2%22%2C%0D%0A%20%20%22testParam1%22%20%3A%20%22ing%22%0D%0A%7D"
+        URLDecoder.decode(sent.url(), "UTF-8") == "http://localhost/mapQueryParam?map={\n" +
+                "  \"testParam1\" : \"ing\",\n" +
+                "  \"testParam2\" : \"ing2\"\n" +
+                "}"
     }
 
     def "first null header param not sent"() {
@@ -121,10 +124,10 @@ class BeanParamTest extends Specification {
 
     def "mixed param"() {
         when:
-        client.withMixed(5, "one", new QueryResource.MixedBeanParam(id: 10, param: "two", header: "headerTwo"), "headerOne")
+        client.withMixed(5, "one", "three", new QueryResource.MixedBeanParam(id: 10, param: "two", header: "headerTwo"), "headerOne")
 
         then:
-        sent.url() == "http://localhost/path1/5/path2/10?param1=one&param2=two"
+        sent.url() == "http://localhost/path1/5/path2/10?param1=one&param3=three&param2=two"
         sent.headers().get("header1")[0] == ("headerOne")
         sent.headers().get("header2")[0] == ("headerTwo")
     }
@@ -138,19 +141,37 @@ class BeanParamTest extends Specification {
         sent.body() == null
     }
 
+    def "single query param string"() {
+        when:
+        client.testQueryString("test")
+
+        then:
+        sent.url() == "http://localhost/path1?param1=test"
+        sent.body() == null
+    }
+
+    def "post"() {
+        when:
+        client.postModel(new QueryResource.PostModelParam(id: 1, name: "test"))
+
+        then:
+        sent.url() == "http://localhost"
+        sent.body() != null
+    }
+
     def "mixed param with special characters"() {
         when:
-        client.withMixed(5, "o{n}e", new QueryResource.MixedBeanParam(id: 10, param: "t{w}o", header: "headerTwo"), "heade{rOne}")
+        client.withMixed(5, "o{n}e", "t{h}ree", new QueryResource.MixedBeanParam(id: 10, param: "t{w}o", header: "headerTwo"), "heade{rOne}")
 
         then:
-        sent.url() == "http://localhost/path1/5/path2/10?param1=o%7Bn%7De&param2=t%7Bw%7Do"
+        URLDecoder.decode(sent.url(), "UTF-8") == "http://localhost/path1/5/path2/10?param1=o{n}e&param3=t{h}ree&param2=t{w}o"
         sent.headers().get("header1")[0] == ("heade{rOne}")
         sent.headers().get("header2")[0] == ("headerTwo") /// headers in BeanParam doesn't support values with curly braces
     }
 
     def "mixed param only path"() {
         when:
-        client.withMixed(5, null, new QueryResource.MixedBeanParam(id: 10), null)
+        client.withMixed(5, null, null, new QueryResource.MixedBeanParam(id: 10), null)
 
         then:
         sent.url() == "http://localhost/path1/5/path2/10"

src/test/java/com/qualys/feign/jaxrs/QueryResource.java

@@ -44,9 +44,13 @@ public interface QueryResource {
     @Path("{id}")
     String withPathString(@PathParam("id") String id);
 
+    @GET
+    @Path("/path1")
+    String testQueryString(@QueryParam("param1") String param);
+
     @GET
     @Path("path1/{id1}/path2/{id2}")
-    String withMixed(@PathParam("id1") int id, @QueryParam("param1") String param,
+    String withMixed(@PathParam("id1") int id, @QueryParam("param1") String param, @QueryParam("param3") String param3,
                      @BeanParam MixedBeanParam bean, @HeaderParam("header1") String header);
 
     @GET
@@ -124,10 +128,12 @@ class ExtendedSetterQueryParamBean extends ParamBeanForExtend {
         public void setParam1(String param1) {
             this.param1 = param1;
         }
+
         @QueryParam("two")
         public void setParam2(String param2) {
             this.param2 = param2;
         }
+
         @QueryParam("three")
         public void setParam3(String param3) {
             this.param3 = param3;