Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
spring-issuemaster opened this issue Jan 21, 2010 · 11 comments
Assignees
Milestone

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Jan 21, 2010

Keith Donald opened SPR-6740 and commented

Differences I've noticed so far:

  • When using @RequestBody to bind the request body to a JavaBean, you cannot also declare a BindingResult parameter to trap bind failures. Any failures result seems to result in an exception (500) being thrown. For example, when Jackson fails to bind a 500 error is returned and you have no control over this like you do when failures are first added to the BindingResult context.

  • When using @RequestBody you cannot trigger execution of a validation step post binding with the @Valid annotation.

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:

  • #11715 Allow usage of ConversionService for Jackson HttpMessageConverter

4 votes, 7 watchers

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jan 22, 2010

Keith Donald commented

Other questions to answer:

  • How should Jackson binding exceptions be handled today, in general? Can this improved by addressing the above issues?
@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Mar 22, 2010

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-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jun 4, 2010

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-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jun 7, 2011

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
Copy link
Contributor

@kicktipp kicktipp commented Jan 31, 2019

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:

  • We don't know which field had errors without parsing the JSON error message
  • We don't know if other fields had bind problems, too, as the process is stopped on the first error
  • We can't run the validation process to check the validation on other fields
  • We can't even show a proper error message as no internationalization is possible.

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
Copy link
Contributor

@rstoyanchev rstoyanchev commented Jan 31, 2019

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
Copy link
Contributor

@kicktipp kicktipp commented Feb 1, 2019

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
Copy link
Contributor

@rstoyanchev rstoyanchev commented Feb 1, 2019

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
Copy link
Contributor

@kicktipp kicktipp commented Feb 1, 2019

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
Copy link
Contributor

@rstoyanchev rstoyanchev commented Feb 4, 2019

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
Copy link

@apoorvkapil apoorvkapil commented Dec 20, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.