Controller method argument initialized either from the request body or from request parameters [SPR-13742]

[spring-projects-issues]

Rossen Stoyanchev opened SPR-13742 and commented

@RequestBody converts the body of the request to an object while @ModelAttribute binds request parameters to an object. The former is common with REST APIs while the latter is common with HTML forms in browser applications.

JavaScript UI components however may send HTTP GET requests via Ajax with data represented either as JSON input in the body or appended as query parameters. See relevant discussion under SPR-13679. It would be useful to have a single controller method argument that supports both.

One way to do this could be to delegate to either @RequestBody or @ModelAttribute argument resolution. However for the given use case it may make more sense to somehow fold this together with @RequestBody handling such that we either read the body or treat request parameters as form data. This is something we already do for HTTP POST albeit for a different reason (i.e. to avoid issues where a call to getParameter from a Filter causes the request body to be consumed before we try to read it). This would also be a more consistent and simpler to understand approach, effectively it can still be thought of as @RequestBody handling. It would also be suited for the given use case, i.e. Ajax requests where for example a 400 response is perfectly okay for error responses (vs rendering an HTML error page).

---

Affects: 4.2.3

Issue Links:

• @RestController avoid declare @RequestBody too [SPR-13868] #18441 @RestController avoid declare @RequestBody too
• True Content-Negotiation [SPR-13679] #18254 True Content-Negotiation ("supersedes")

Referenced from: commits spring-attic/spring-framework-issues@cdde05b

2 votes, 8 watchers

[spring-projects-issues]

Rossen Stoyanchev commented

Enrique Ruiz (DiSiD), looking for the best approach for the programming model and implementation. I've discounted adding a new annotation. Doing that would be helpful in terms of making it an explicit choice. However I can't think of one that makes sense intuitively and doesn't bring more complications than the problems it solves.

That leaves @RequestBody or @ModelAttribute to enhance. @RequestBody is arguably the better fit. Conceptually it supports consuming different mime types already while @ModelAttribute is essentially all about consuming form data (name-value pairs) which may come from the body or the query string.

Furthermore consider the definition of the form submission method. GET is expected for idempotent forms (e.g. search) and for that case input data is appended to the URI. This seems to fit well the use case with jQuery (and other plugins) that append data to the query string. We can support this for @RequestBody in combination when also mapped to GET with consumes="application/x-www-form-urlencoded":

@RequestMapping(path="/accounts", method = GET, consumes={"application/json", "application/x-www-form-urlencoded"})
public String list(@RequestBody Customer customer) {
        // ... 
}

It seems like a contradiction to have GET and consumes application/x-www-form-urlencoded. However it's quite intuitive and a natural extension of the idea of a GET form submission. Internally we would turn request parameters into form data and treat as the body as we already do here (to make sure the body isn't parsed accidentally).

What do you think?

[spring-projects-issues]

Enrique Ruiz (DiSiD) commented

It seems good.

Summarizing, this enhancement will give us 2 options:

1. Using the ContentNegotiatingViewResolver plus creating an own JsonViewResolver (as shown in Content Negotiation using Views), a single controller method could handle any request and generate any content: HTML, JSON, etc.:

@RequestMapping(path="/accounts", method = GET, consumes={"application/json", "application/x-www-form-urlencoded"})
public String list(@RequestBody Customer customer) {

  // ...

  // Return the view name for rendering the response
  return "accounts/list";

}

2. Using the ContentNegotiatingViewResolver without a JsonViewResolver, two controller methods could handle all the requests and generate HTML responses plus RESTful responses:

@RequestMapping(path="/accounts", method = GET, consumes={"application/json", "application/x-www-form-urlencoded"})
public String list(@RequestBody Customer customer) {

  // ...
  model.addAttribute( listWithMarshalling(customer) );

  // Return the view name for rendering the response
  return "accounts/list";

}

@RequestMapping(path="/accounts", method = GET, produces="application/json", consumes={"application/json", "application/x-www-form-urlencoded"})
@ResponseStatus(HttpStatus.OK)
public @ResponseBody List<Account> listWithMarshalling(@RequestBody Customer customer) {

  return accountService.findAllAccounts(customer.getName());
}

Is this correct?

[spring-projects-issues]

Rossen Stoyanchev commented

Yes this sounds correct. I only qualify it a little. This is specifically for HTTP GET requests where you can alternate between accepting input as a JSON request body or form data in the query string and also alternate between producing JSON or HTML, or other content types. I would say the first of the two options above is well suited for cases with HTML rendering.

I will work on something concrete to try out. Thanks!

[spring-projects-issues]

Rossen Stoyanchev commented

Enrique Ruiz (DiSiD), initializing the request body from request parameters didn't work out as expected. First the absence of a Content-Type request header precludes the selection of an HttpMesageConverter. We could fall back on "application/x-www-form-urlencoded" based on the presence of a consumes condition, which would be a bit of a stretch. Even so the FormHttpMessageConverter only supports method arguments of type MultiValueMap so that's a dead end. Doh!

Back to where we started under #18254 you mentioned there are various Ajax components that send query parameters (search-related, data table filtering, etc). That seems fine and it's what led me to think of this as support for @RequestBody + GET requests with form data. Under #18254 you also pointed to jQuery.load() as example of GET with body but that's a generic method that accepts an HTTP method, URL, and body. It doesn't mean that all combinations make sense. To be clear the HTTP spec does not preclude GET with body but is ambiguous at best ("a payload within a GET request message has no defined semantics"). In particular a search screen or data table refresh query with a request body would be a bad idea since it will produce different results for the same URL and that's an issue for caching.

So the idea that this is useful for GET requests that may accept a body or query parameters doesn't seem to add up. Looking back in an earlier comment you mentioned desktop apps vs native mobile applications and CRUD using JSON. Could you comment on that? In a CRUD scenario where you post JSON (instead of form data) what would be the response in case of success, or case of error? As far as I can tell you have to render HTML with errors (desktop) or turn the BindingResult into a JSON error response (mobile), or on success redirect (desktop) vs return 201 (mobile) so that points to two separate controller methods.

As far as I can see it doesn't make sense to support this for HTTP GET explicitly, at least not in the framework. That said I did go back to the original idea of creating a composite resolver that combines @RequestBody and @ModelAttribute semantics just to see how easy it would be. It proved to be very easy indeed. See this project where I created a new annotation called @RequestBodyModelAttribute for lack of a better name. There is one test with a JSON body and another with query string input.

This could be improved further but it's enough for now as a proof of concept. What I'm lacking is a good understanding of the usage scenarios for which we would promote using this as well as a better name, which might come from the former.

[spring-projects-issues]

Rossen Stoyanchev commented

Enrique Ruiz (DiSiD), have you had a chance to check the progress on this ticket? The POC code shows creating a resolver for @RequestBody with @ModelAttribute is the easy part and for that matter it's something you can do. In order for this to be a framework feature however there needs to be a good mental model. GET forms isn't a good model here since GET with a body is rather unusual and works against HTTP caching.

[spring-projects-issues]

Enrique Ruiz (DiSiD) commented

Looking back in an earlier comment you mentioned desktop apps vs native mobile applications and CRUD using JSON. Could you comment on that? In a CRUD scenario where you post JSON (instead of form data) what would be the response in case of success, or case of error? As far as I can tell you have to render HTML with errors (desktop) or turn the BindingResult into a JSON error response (mobile), or on success redirect (desktop) vs return 201 (mobile) so that points to two separate controller methods.

A typical use case is to POST JSON Data using AJAX from mobile devices and to POST Form Data from web browsers at desktops.

We have two applications:

1. The first app is an Spring MVC application that generates a CRUD views to manage the data via a web browser in a desktop. Moreover it publishes the CRUD actions via REST.
2. The second app is an AngularJS application to manage the data in mobile devices. This application send the CRUD requests to the first app via AngularJS forms that send JSON Data (POST) to the REST services.

The forms of the first application are the typical HTTP Form Data. The forms of the AngularJS application are similar to this example: https://hello-angularjs.appspot.com/angularjs-http-service-ajax-post-json-data-code-example

The first application generates the HTTP response by using a ContentNegotiatingViewResolver and related ViewResolvers that dinamically generates HTML or the JSON message, so in terms of response, we could generate it with only one handler method

But we have two handler methods: one for @RequestBody and another for @ModelAttribute semantics, but both have the same logic and generates a view-based response. Note we don't use the POST-REDIRECT-GET pattern.

About the errors, the handler method gets the data inside the BindingResult and it copies the data in our own bean. Later, the controller method inserts this bean in the Model in order to be able to send binding errors to mobile devices.

It would be great some type of improvement to include the BindingResult in JSON responses automatically .. but this could be another issue.

As far as I can see it doesn't make sense to support this for HTTP GET explicitly, at least not in the framework. That said I did go back to the original idea of creating a composite resolver that combines @RequestBody and @ModelAttribute semantics just to see how easy it would be. It proved to be very easy indeed. See this project where I created a new annotation called @RequestBodyModelAttribute for lack of a better name. There is one test with a JSON body and another with query string input.

This could be improved further but it's enough for now as a proof of concept. What I'm lacking is a good understanding of the usage scenarios for which we would promote using this as well as a better name, which might come from the former.

I hope the above use case has clarified a bit.

[spring-projects-issues]

Rossen Stoyanchev commented

The POST-REDIRECT-GET and the BindingResult -> REST error object are two ways in which the @RequestBody and @ModelAttribute methods are not the same. I find it hard to promote a new annotation in the programming model whose most basic usage requires such qualifications and trade-offs. Furthermore considering how easy it is to create a custom annotation of your own, and if you really want to consolidate methods this way, I tend to lean towards leaving this as a case for a custom annotation.

[spring-projects-issues]

Rossen Stoyanchev commented

After a discussion with the team I've decided to close this for now. It is trivial to achieve technically (see demo project) but it involves trade-offs so we'd rather leave it at that.