Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RestClientInvoker does not serialize multiple query parameter values (List<String>) #12

Open
bachmankobts opened this issue May 11, 2020 · 5 comments

Comments

@bachmankobts
Copy link

When a rest client API specifies multiple query parameter values, e.g.:

@Path("/rest/v1")
@RegisterRestClient
public interface MyRestClient {

  @GET
  @Path("/query")
  @Consumes("application/json")
  public String query(@QueryParam("filter") List<String> values);

The URI is built as "/rest/v1/query?filter=[1,2,3]" instead of "/rest/v1/query?filter=1&filter=2&filter=3"

The RestClientInvoker.invoke() method should add param values as array in case of list:

ParamInfo paramInfo = determineParamInfo(method, args);
        UriBuilder uriBuilder = UriBuilder.fromUri(serverURL.toString());
        for (Map.Entry<String, Object> entry : paramInfo.getQueryParameterValues().entrySet()) {
            if (entry.getValue() != null) {
                uriBuilder.queryParam(entry.getKey(), **entry.getValue()**);
            }
        }
@Jamsek-m
Copy link
Member

Hello, this is default behaviour of JAX-RS client and as such I am not inclined to change it, as I would prefer to keep consistency between the two.

If you need URL that you wrote, you can define your interface to accept array instead of list and simply when calling interface method pass list.toArray() as a parameter.

@bachmankobts
Copy link
Author

Hello, I used array in my interface, but it did not help. I have extended the kumuluzee sample rest applications (attached) with new Customer API method accepting array:

public interface CustomerApi {
@GET
@Path("/query")
Customer getCustomers(@QueryParam("customerId") String[] ids);

public class RestResource {
@GET
@Path("query")
public Response getListOfCustomers() {
return Response.ok(customerApi.getCustomers(new String[]{"1", "2", "3"})).build();
}

but serialized query string was not as expected (customerId=1&customerId=2&customerId=3):
/v1/customers/query?customerId=%5BLjava.lang.String%3B%40440b42e9

example:
kumuluzee-rest.zip
kumuluzee-rest-client.zip

to execute the code, query the client application:
GET http://localhost:8082/v1/operations/query

If you have a working sample of the query parameter array, I will appreciate it.

@bachmankobts
Copy link
Author

none of this client definitions is working:

1. Customer getCustomers(@QueryParam("customerId") String[] ids);
2. Customer getCustomers(@QueryParam("customerId") String... ids);
3. Customer getCustomers(@QueryParam("customerId") List<String> ids);

the problem is with Java, that does not pass array correctly into the method with varialble parameters, when the array is stored in the variable of Type Object:

for (Map.Entry<String, Object> entry : paramInfo.getQueryParameterValues().entrySet()) {
   uriBuilder.queryParam(entry.getKey(), entry.getValue());
}

the entry.getValue() is an array, but passed to queryParam(String name, Object... values) as Object type, instead of Object[] (queryParam(String, Object))

@Jamsek-m
Copy link
Member

Hi,
It looks like you are using older version of rest client. In your project you have 1.3.0-SNAPSHOT, but there is already version 1.4.1 (and 1.5.0-SNAPSHOT).

There was a bug when mapping generic collections, which was fixed in version 1.4.1 (note that this version requires json-b, alongside of json-p).

Can you try upgrading the version if possible?

@gpor0
Copy link
Member

gpor0 commented Nov 26, 2021

I had the same issue today on latest snapshot version of kumuluz rest client. The expected behavior is explained in mp rest client 2.0 specification:
https://download.eclipse.org/microprofile/microprofile-rest-client-2.0/microprofile-rest-client-spec-2.0.html

I think this issue should be closed (due to inactivity) and the problem should be fixed in newer mp client integration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants