feign icon indicating copy to clipboard operation
feign copied to clipboard

@QueryMap does not work with POJOs when used after @Param

Open inad9300 opened this issue 7 years ago • 6 comments

This kind of code (from https://github.com/OpenFeign/feign/pull/335/files#diff-13815c678552a275075ab18b17c67784R616):

@RequestLine("GET /?name={name}")
void queryMapWithQueryParams(@Param("name") String name, @QueryMap Map<String, Object> queryMap);

Does not work when @QueryMap is used on a POJO.

In my case, I was trying to use Spring's Pageable like so:

@RequestLine("GET /things?x={x}&y={y}&z={z}")
CustomPageImpl<ThingDto> getElements(
    @Param("x") ZonedDateTime x,
    @Param("y") String y,
    @Param("z") String z,
    @QueryMap Pageable pageable);

By the way... Is there any simpler way to declare query parameters without having to refer four times to their names?

inad9300 avatar Dec 07 '18 09:12 inad9300

This is expected. We do not support mixing @Param and @QueryMap using the Bean specification currently. The models are effectively incompatible. In the meantime, my suggestion is to use a custom Encoder for the parameters you want to control. You can find more information on that in the README - Custom Parameter Expansion

When using the @Param annotation, you must annotate each method parameter. The alternative is to use a @QueryMap, using the Map specification. In your particular case, you could refactor your interface to accomplish what you are asking:

@RequestLine("GET /things")
CustomPageImpl<ThingDto> getElements(
    @QueryMap Map<String, Object> parameters,
    @Param(encoder = "PageableEncoder.class") Pageable pageRequest);

Add all of your parameters to the map and they will be expanded onto the query string. Using the custom encoder, you can control how the Pageable is expanded as well.

Let me know if this helps or if you have more questions.

kdavisk6 avatar Jan 08 '19 18:01 kdavisk6

I disagree the two models are incompatible, but this should be explicitly stated in the documentation, since as it is written now, what I was trying to do initially should be expected to work.

Thanks for your proposed workaround.

inad9300 avatar Jan 13 '19 12:01 inad9300

Just ran into this, IMO it should be clear in the documentation or even better, the Contract should throw an exception when it encounters such a case.

jebeaudet avatar Apr 29 '20 20:04 jebeaudet

Time is a friend here. I’ll revisit this.

kdavisk6 avatar Apr 29 '20 20:04 kdavisk6

Just ran into this, IMO it should be clear in the documentation or even better, the Contract should throw an exception when it encounters such a case.

Sounds like a good plan @jebeaudet, can you help on making it happen?

velo avatar Apr 29 '20 20:04 velo

Hi there, is there is any progress on issue?

DzianisH avatar Jan 18 '21 14:01 DzianisH