Micronaut-core: HttpRequest<Optional<ByteBuffer<*>>> still requires a request body when it should be optional

[apottere]

Per a conversation in gitter, HttpRequest<Optional<ByteBuffer<*>>> should not require a request body in order to call the controller endpoint.

Steps to Reproduce

Controller:

@Post("/test")
fun post(request: HttpRequest<Optional<ByteBuffer<*>>>) = request.body.isPresent

curl with empty body:

$ curl localhost:8080/test -XPOST

Expected Behavior

false

Actual Behavior

{"message":"Required argument [HttpRequest request] not specified","path":"/request","_links":{"self":{"href":"/test","templated":false}}}

Environment Information

• Operating System: MacOS Catalina
• Micronaut Version: 1.3.2
• JDK Version: 11

[apottere]

I created a sample project that shows the behavior of different combinations (https://github.com/apottere/micronaut-optional-body)

Copying the controller definition here for reference:

@Post("/test1")
fun post1(request: HttpRequest<Optional<ByteBuffer<*>>>) = request.body.isPresent

@Post("/test2")
fun post2(request: HttpRequest<ByteBuffer<*>?>) = request.body.isPresent

@Post("/test3")
fun post3(request: HttpRequest<*>, @Body body: Optional<ByteBuffer<*>>) = request.body.isPresent

@Post("/test4")
fun post4(request: HttpRequest<*>, @Body body: ByteBuffer<*>?) = request.body.isPresent

@Post("/test5")
fun post5(request: HttpRequest<ByteBuffer<*>?>, @Body body: ByteBuffer<*>?) = request.body.isPresent

Running the app will produce the following results:

/test1: no body: Required argument [HttpRequest request] not specified
/test1:    body: Required argument [HttpRequest request] not specified
/test2: no body: Required argument [HttpRequest request] not specified
/test2:    body: 200 true
/test3: no body: Required Body [body] not specified
/test3:    body: Required Body [body] not specified
/test4: no body: 200 false
/test4:    body: 200 true
/test5: no body: Required argument [HttpRequest request] not specified
/test5:    body: 200 true

Based on these results (and reading some of the micronaut code), it looks like there's a discrepancy between how the generic on HttpRequest and the @Body-annotated argument are handled. /test4 is the only one that works properly, because the body argument is technically annotated with @Nullable, and there's a check in the DefaultBodyAnnotationBinder that returns BindingResult.EMPTY if the body is missing, which satisfies the requirement for a nullable argument. This doesn't seem to satisfy the requirement for an Optional, though.

Binding an HttpRequest, on the other hand, goes through en entirely different path in DefaultRequestBinderRegistry. The binder there returns ArgumentBinder.BindingResult.UNSATISFIED if there is a generic type specified and HttpMethod.permitsRequestBody is true, but source.getBody().isPresent() is false. It doesn't seem to have any check to see if the type is Optional/nullable. Nullability also doesn't seem to be supported by the argument metadata generator.

It seems like it would be a good idea to:

1. Unify the code path for binding @Body and HttpRequest
2. Support Optional for both
3. Support kotlin nullable types for HttpRequest?

I'm not sure if 3 is even possible, I took a look at the inject and inject-java repo and I cut my losses before I spent too much time on it. If it is possible, though, it would be a great improvement, since @Body body: Any? and HttpRequest<Any?> would then behave similarly.

[graemerocher]

Will review, thanks for the feedback