com.kiwigrid.micronaut.optional.RestClientTest test and observe the DEBUG log entries of the client.When i use a declarative client as showed in the test case i would expect Micronaut to handle the java.util.Optional parameters gracefully. This means an Optional.EMPTY parameter is omitted in the request generated by the client. And if the value of an Optional is present it should unwrap the value before using it in the client request.
Currently the Optionals seem to be always directly used via toString conversion. (see ReadMe of the showcase).
The server side implementation of Micronaut does already handle Optional the way i would expect it. You can see this as well in the showcase project, as the controller part is also using Optional for the parameters.
Hi @graemerocher,
I'm not sure if that's an issue or Micronaut is working as designed, but after a debug session I've found out that a String with Optional.empty() gets wrapped in another optional at:
io.micronaut.web.router.AbstractRouteMatch.java:386
For an Optional.empty() where the type is Integer, the type and value match at the if-statement at line 386, on the other hand, an Optional.empty() for a String is going into the else (line 388), which in turn takes the value to the conversion service, wrapping it in another Optional after the assignment (line 390).
To find out why the String is being considered of a different type, we must look at:
io.micronaut.http.server.binding.RequestArgumentSatisfier.java:86
It looks like that for the Optional Integer the result of bindingResult.getValue() (line 85) is not Present, therefore "value" gets assigned to "optionalValue" (line 89). For an Optional String, bindingResult.getValue() is Present, which results in the "optionalValue" being extracted at line 87.
I believe that the behaviour above changes the wrapping object of the return at RequestArgumentSatisfier.java:108. which will then create the difference in the if-statement at AbstractRouteMatch.java:386. For the sake of experimentation, I changed this statement in my local machine and confirmed that this is where it's being triggered. Also, I could confirm that it's the root of the behaviour stated by @mscheibler in this issue.
Well, hypothetically, a change in the class io.micronaut.core.bind.ArgumentBinder would do the trick and change it to fulfil @mscheibler's expectations. However, this interface has 28 implementors and I believe that it's too hefty of a change, especially for a first committer at Micronaut like myself.
What are your thoughts? Is that something that we should change or leave as is?
I can contribute myself.
@allanneves Is that related to this ticket? Seems like you're referring to the server and this issue is for the client.
Hi @jameskleeh,
Thanks for the prompt response. I cloned @mscheibler's repo and debugged it from there.
There is a test that creates a Client (interface annotated with @Client) injected by javax.Inject. He asserts in the test that actual and expected String will be the same. what happens is that actual is wrapped in another Optional when the response body is called.
Did I look into the wrong place?
@allanneves I'm not sure. I didn't need to look at the example app as the issue was clear for me. You can see the commit above to see the added test that uses optional in client methods and passes.
Thanks for looking into it.
Most helpful comment
@allanneves I'm not sure. I didn't need to look at the example app as the issue was clear for me. You can see the commit above to see the added test that uses optional in client methods and passes.