spring-projects / spring-framework

Spring Framework
https://spring.io/projects/spring-framework
Apache License 2.0
56.64k stars 38.14k forks source link

Spring MVC Binding Lifecycle differs between @RequestBody arguments and plain JavaBean arguments [SPR-6740] #11406

Closed spring-projects-issues closed 12 years ago

spring-projects-issues commented 14 years ago

Keith Donald opened SPR-6740 and commented

Differences I've noticed so far:

We should look at getting consistency between the "traditional" DataBinder mechanism and the @RequestBody mechanism where it is possible. If it's not possible or practical, we should at least document the differences between traditional binding and @RequestBody/@ResponseBody usage.


Affects: 3.0 GA

Issue Links:

4 votes, 7 watchers

spring-projects-issues commented 14 years ago

Keith Donald commented

Other questions to answer:

spring-projects-issues commented 14 years ago

Simon Wong commented

I have read the source code of the mvc-ajax spring samples. How it deals with validation exception is putting the ValidationResult in Map and returned it (with the HTTP status code is a CLIENT_ERROR code). I think setting the return type as a combination (in Map<String, ?>) of ValidationResult/domain object will cause the @Controller API not that intuitive.

I guess the binding exceptions should be handled automatically (maybe configurable), maybe output to @ResponseBody (through HttpMessageConverter) by default.

spring-projects-issues commented 14 years ago

Keith Donald commented

This issue has been brought up multiple times on our blog here: http://blog.springsource.com/2010/01/25/ajax-simplifications-in-spring-3-0/#comment-173191

spring-projects-issues commented 13 years ago

Rossen Stoyanchev commented

I'm resolving this issue as complete. Use of @Valid @RequestBody is now supported, see #11375 for details. Jackson failures result in HttpMessageNotReadableException, which is resolved by the DefaultHandlerExceptionResolver by setting the status code to 400.

We don't support a BindingResult argument after @RequestBody indeed. I think handling HttpMessageNotReadableException or RequestBodyNotValidException centrally from an exception resolver should be adequate. If there are good use cases we can consider it of course.

kicktipp commented 5 years ago

We are switching from @ModelAttribute to @RequestBody right now and we find it very strange not getting access to the BindingResult. At the moment we are not doing any client side validation and just parse the default JSON error messages on validation errors to show which fields had errors. We have a few simple integer fields. If the user accidentally types a string value into the input filed, we have the following problems on the client side:

I understand the reason behind this as JacksonMapping is a complete different process then Spring Binding. But it is annoying to have @ModelAttribute and @RequestBody behave so different.

At least this behavior is documented here: https://docs.spring.io/spring/docs/current/spring-framework-reference/web.html#mvc-ann-methods

And discussed here: https://stackoverflow.com/questions/34728144/spring-boot-binding-and-validation-error-handling-in-rest-controller/54464613#54464613

rstoyanchev commented 5 years ago

This is an 8 year old issue and the information in it is equally old. Please make sure you've read the latest documentation. Keep in mind the issue tracker here is for issues and not for questions.

kicktipp commented 5 years ago

Sorry for this. I still think this is an issue as you have access to the BindingResult with @ModelAttribute but not with @RequestBody. I have read the latest documentation and linked to it in my comment. Should I just open a new issue or should I just keep calm and carry on as this is not going to change?

rstoyanchev commented 5 years ago

as you have access to the BindingResult with @ModelAttribute but not with @requestbody

How so? If you actually click the link I provided, which I will assume you did not, it's not the same as the one you provided, and the very second example has BindingResult.

kicktipp commented 5 years ago

I have read it all, of course. Of course, you can add a BindingResult object to the method parameter, but it does not include Binding problems within Jackson. This differs from @ModelAttribute. But to make my words understandable I wrote a small test project.

https://github.com/kicktipp/requestbody-demo

./mvnw test and the failing test shows the problem.

rstoyanchev commented 5 years ago

In the case @ModelAttribute, we are in control of creating the object and populating it from request parameter name-value pairs. We can loop over all and record any errors.

In the case of @RequestBody we rely on Jackson create and populate the target object by parsing JSON from the request body. Only then we can be in control of validation and the BindingResult reflects the validation phase only. I'm not aware of anything we could do differently. This is what we get from Jackson and in any case parser errors represent malformed input, so continuing to parse may not even be feasible depending on the parse error.

apoorvkapil commented 4 years ago

In case of HttpMessageNotReadableException we get return jackson parser error message. which is too much info to provide to an Consumer (e.g. your Backend stack), I am sure this can be customized with some Generic Static messages. so seems fine. However issue is with the getPathReference() or getPath() which doesn't return the exact Path which has the validation issue. It would be good if it could return the exact path.