Spring Cloud Netflix Spring Cloud Feign Client @RequestParam with Collection parameter creates a wrong request

Here is the problem my colleague Sergey faced yesterday (http://stackoverflow.com/questions/41744542/spring-cloud-feign-client-requestparam-with-list-parameter-creates-a-wrong-requ)

We have a Spring Clound Feign Client mapping defined as following

@RequestMapping(method = RequestMethod.GET, value = "/search/findByIdIn")
Resources<MyClass> get(@RequestParam("ids") List<Long> ids);

by calling

feignClient.get(Arrays.asList(1L,2L,3L))

according to what I can see in the debugger, the feign-core library forms the following request:

/search/findByIdIn?ids=1&ids=2&ids=3

instead of expected

/search/findByIdIn?ids=1,2,3 which would be correct for the server Spring Data REST endpoint declared in the same way as my Feign client method.

Spring Data REST Service does not accept parameters like ids=1&ids=2 and throws an Exception. But the multiple sort parameters will be accepted.

We've tried both versions Camden.RELEASE and Dalston.BUILD-SNAPSHOT without success.

Here you can find both client and server to reproduce the problem:

https://github.com/abinet/demo

Comment From: ryanjbaxter

It looks like the logic in RequestTemplate.queryLine is what is creating the query string. Maybe @adriancole or @spencergibb know why we decided to create the string this way. https://github.com/OpenFeign/feign/blob/master/core/src/main/java/feign/RequestTemplate.java#L653

Comment From: spencergibb

It's not built for Spring Data, it's built for general HTTP requests. A list of parameters to a get is naturally split into repeated query parameters, which are allowed.

Comment From: codefromthecrypt

what @spencergibb said :P repeated query parameters are permitted, and while there's no standard, many libraries repeat rather than assume the other side uses comma encoding.

In feign upstream there's Param.Encoder annotation which would allow you to customize a list and decide to join on string. This is set in contract parsing time in the MethodMetadata

Comment From: ryanjbaxter

Thanks @spencergibb and @adriancole!

@adriancole when you say Param.Encoder do you mean this Param class?

Comment From: codefromthecrypt

no feign.Param.Expander which ends up in MethodMetadata.indexToExpanderClass

Comment From: abinet

@adriancole Param.Expander does not help here. It will be applied to each the element of the collection but not to collection itself.

Comment From: codefromthecrypt

well, at the moment, the only way I can think of is to use a request interceptor to rewrite the queries

ex.

for each entry with more than one value in template.queries() template.query(entry.key, joinOnComma(entry.value))

Comment From: ryanjbaxter

@adriancole after I shut down my computer last night I had the same idea. I will try the request interceptor approach later on.

Comment From: abinet

thank you guys. the solution with a request interceptor works like a charm.

Comment From: postalservice14

Here is my request interceptor that worked. In case someone else is trying to solve this:

public class ListToStringCommaRequestInterceptor implements RequestInterceptor {
    @Override
    public void apply(RequestTemplate template) {
        template.queries().forEach((key, value) -> {
            if (value.size() > 1) {
                template.query(key, String.join(",", value));
            }
        });
    }
}

Comment From: BohdanKorinnyi

@postalservice14 this request interceptor produces duplicates of data in 10.7.4 version. I added some logs and it appeared that values are not replaced for given key but they just appended to query.

public void apply(RequestTemplate requestTemplate) {
    log.info("Before RequestInterceptor query line: {}", requestTemplate.path() + requestTemplate.queryLine());
    requestTemplate.queries().forEach((key, value) -> {
        if (value.size() > 1) {
            requestTemplate.query(key, String.join(",", value));
        }
    });
    log.info("After RequestInterceptor query line: {}", requestTemplate.path() + requestTemplate.queryLine());
}

As a result the query line looks like:

Before RequestInterceptor query line: /path?params=param1&params=param2
After RequestInterceptor query line: /path?params=param1&params=param2&params=param1,param2

Comment From: Alexalexale

var queries = template.queries().entrySet().stream().collect(Collectors.toMap(
    Map.Entry::getKey,
    entry -> {
        if (entry.getValue().size() > 1) {
            return List.of(String.join(",", entry.getValue()));
        }
        return entry.getValue();
    }
));

template.queries(null);
template.queries(queries);