Micronaut-core: HttpRequest isn't filled with request body when no @Body annotation present

[ivyazmitinov]

Steps to Reproduce

1. Create controller with a @Post method, taking HttpRequest as an argument.
   java @Post("/test") public HttpResponse<?> test(HttpRequest<ObjectNode> request) { LOG.info(request.getBody().get().toString()); return HttpResponse.ok(); }
2. Invoke it with any json body

Expected Behaviour

Body should be available via request.getBody()

Actual Behaviour

Body is not available

Environment Information

• Micronaut Version: 1.1.0.RC1

[jameskleeh]

This is essentially a duplicate of https://github.com/micronaut-projects/micronaut-core/issues/1113

[ivyazmitinov]

@jameskleeh

some additional way for you to tell Micronaut you want the body read

I suppose, @Controller's @Post method, taking HttpRequest as an argument would suffice as 'additional way', since it has a .getBody() method, and this is extremely unclear, that it will be empty without some magical annotation.

Documentation also introduces confusion, since it clearly states 'The full HttpRequest', without any conditions.
https://docs.micronaut.io/1.1.0.RC2/guide/index.html#requestResponse

[jameskleeh]

@ivyazmitinov No one is suggesting any of the annotations are magical. It definitely seems like there should be some additional documentation around when and under what conditions the body is read

[graemerocher]

I think this is more of a bug than #1113 because with controllers through generic types you at least have a way to express the body type you require, which is not the case for filters

[jameskleeh]

@graemerocher Agreed - I didn't notice the generic in the argument

[jameskleeh]

This leaves to question what should happen if the body could not be converted to the type requested. Still bind the request object without the body or fail to bind the request at all.

[ivyazmitinov]

@jameskleeh
I advice going with second option. It is much better to fail fast, instead of having source of bugs, which could be quite difficult to track

[jameskleeh]

I agree, however it represents a potentially breaking change

[graemerocher]

@jameskleeh what scenario would this break?

[jameskleeh]

@graemerocher In order to trigger the body reading logic we would have to initially set the state of the request argument to unsatisfied. That would be based off that the body is not present. When the binder gets executed in the future after the body reading logic goes through then we can bind the request. The breaking change is for POST/PUT/etc requests that don't send a body. We would never bind the request object in that case whereas today we would.

[ivyazmitinov]

@jameskleeh
I would recommend also document the behavior of raw HttpRequest as method parameter in here to avoid confusion: I were a user, I would expect body available as Object if no generic provided, or at least, an exception on .getBody() invocation with meaningful exception. Or some kind of warning during startup/annotation processing.
Also, do I understand correctly, that the body will be available as Object if parameter is declared as HttpRequest<?>?

[graemerocher]

The way Micronaut works is to block as little as possible before executing the route. This means you have to be explicit up front about what you want hence the @Body annotation

If you declare ? as a parameter the current assumption is you are not interested in the body. Otherwise we would have to decide on a default for each content type and block unnecessarily

Really the case where you accept the full request is rare and generally you should define explicit @Body, @Header etc parameters

[ivyazmitinov]

@graemerocher
I see. Well, it definitely should be documented because Spring users will be very confused (me included :)).
The problem is in the interface of HttpRequest. It defines .getBody() method, what implies, that body will be available. IMO, from architectural point of view, it would be better to have clear separation of interfaces, like LightHttpRequest, without a .getBody() and existing HttpRequest with .getBody().

[graemerocher]

The getBody method returns Optional for this reason. Certainly more documentation is warranted. Note that Micronaut is definitely not Spring. There are some fundamental differences in the design. This being one of them

[ivyazmitinov]

@graemerocher
I suppose, users would expect body to return empty Optional if the request doesn't contain a body, not because no generic have been provided.

That's why I would add some warning for if .getBody() is invoked on raw HttpRequest, reminding of necessity of concrete type needed for .getBody() to work. Exception will be even more suitable in this situation, since this is clearly an illegal state (although it would violate L from SOLID, IMO).