Support Conditional Parameter Expansion

[bsideup]

Hi!

We define our Spring MVC APIs with @RequestParam Optional<Type> something.

It looks like Feign doesn't handle it well and uses "toString()" version so that Spring fails to convert String -> Optional

[spencergibb]

see #294

[bsideup]

hey @spencergibb,
I'm afraid this is not the same, #294 was about the decoder, and it's not hard to use a custom decoder to make it work, but, in case of the request parameters, I don't know any good way to make it work :(

[spencergibb]

Yeah, I specifically didn't say it was a duplicate, just regarding Optional.

[codefromthecrypt]

right now, the work around is param parser

[bsideup]

@adriancole sounds exactly what I want. Where can I find it? Thanks!

[codefromthecrypt]

oh.. you are asking about spring mvc annotations. sorry I was answering about feign.Param.Expander

[codefromthecrypt]

if one were hacking things.. they could change the contract so that it populates MethodMetadata.indexToExpanderClass (this is what the default contract does, handling Param.Expander.

[Hronom]

Same here, optional parameter is very useful if you want to reset optional parameter, otherwise you just can't do that.

It's already supported by Spring MVC:

@GetMapping(path = "/test")
    public void test(
        @RequestParam(value = "name", required = false) Optional<String> name
    ) {
        System.out.println("is present " + name.isPresent());
    }

Request set name http://localhost:8132/test?name=hello will have true.
Reset name request http://localhost:8132/test?name will have true.
No name reset request http://localhost:8132/test? will have false.

[velo]

Well, feign core now is on java 8, so, this should be doable

[Hronom]

I think I was missing something, it seems that contract in Spring MVC is next:
Request set name http://localhost:8132/test?name=hello will have hello.
Reset name request http://localhost:8132/test?name will have `` - empty String.
No name reset request http://localhost:8132/test? will have `null` - null String.

Seems like optional adds nothing in my case.

For my case I think I need custom class that will be able to show when parameter actually is in request. Like ResponseEntity for returned body.

Example:

   @GetMapping(path = "/test")
    public void test(
        @RequestParam(value = "name", required = false) RequestParamEntity<String> name
    ) {
        System.out.println("is present " + name.isPresent());
        System.out.println("has value " + name.hasValue());
    }

For request http://localhost:8132/test?name=hello output will be:

is present true
has value true

For request http://localhost:8132/test?name= (questionable) output will be:

is present true
has value true

For request http://localhost:8132/test?name output will be:

is present true
has value false

For request http://localhost:8132/test? output will be:

is present false
has value false

But this also should be implemented in Spring MVC.

[kdavisk6]

We can support Optional<T> as a return type. but we do not support is as a @Param. I'll add this to the backlog for us to consider, however; feel free to submit a PR for this if you cannot wait.

[RicardoRdzG]

I would like to help add Optional support in @param, just need some direction on where to start looking

[kdavisk6]

I think that we will first need to agree on a specification. These should focus on the result of Optional.isPresent(). We would also need to limit the scope to Feign's default contract and annotation, @Param, which does not support a required attribute. Given these restrictions, I see the following use cases for us to define:

• isPresent is true
• isPresent is false
• Parameter is not provided or null

Thoughts?

[Hronom]

@kdavisk6 I think Optional will adds nothing from what actually needed, check my previous post for suggested custom class RequestParamEntity(name is questionable) and suggested behaviour of it.

That behaviour you can't implement using Optional.