-
Notifications
You must be signed in to change notification settings - Fork 38.3k
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 and WebFlux docs need to say method validation applies if any method parameter has constraint annotations #32082
Comments
@NotNull
@Valid
private String target; You should remove |
@quaff without |
I think instead of MethodArgumentNotValidException the HandlerMethodValidationException is thrown. Is your Controller annotated with |
@ChrAh88 Class-level Method validation on SF 6.1 should not need the As said. I'm also totally fine with having this as a documentation-only issue, but it clearly needs clarification what is supported and what is not. |
Same issue after update to 6.1.3 instead of MethodArgumentNotValidException the HandlerMethodValidationException is thrown. |
According to the Javadoc, Have you tried to remove public String hello(@RequestBody @Valid /*@NotNull*/ Body body) {
return "Hello " + body.target;
}
public static class Body {
@NotNull
/*@Valid*/
private String target;
public Body() {}
public void setTarget(String target) {
this.target = target;
}
} |
I think it's expected if you remove |
|
@quaff I don't want to repeat myself, but if this is a documentation-only issue I'm totally fine with that. But I think it needs documentation what the preferred way is and what is not supported. |
@dreis2211 it's documented here: https://docs.spring.io/spring-framework/reference/web/webmvc/mvc-controller/ann-validation.html With the fix of #31711, also SmartValidators with contained jakarta validator are respected. In case of spring boot the validator "ValidatorAdapter" is a SmartValidator with a contained jakarta validator. But yes, spring boots upgrade instructions to version 3.2 or the release notes for 3.2.2 should be updated in my opinion. |
@ChrAh88 From documentation it seems that if only |
@rstoyanchev What is your recommendation for the use-case at hand? Dropping the |
@dreis2211, I would suggest dropping the |
Thanks. Exactly what I was hoping for (because I did it already :D ) |
Hi,
when upgrading to Spring-Boot 3.2.2 and therefore SF 6.1.3 we noticed a change in behaviour that causes one of our
@ExceptionHandler
s not to be triggered anymore, when the@RequestBody
fails validation (in this case whenBody#target
is null).Interestingly, this can be worked around by removing the
@NotNull
from the method parameter.I think this is somewhat related to #31711 but without the indication that the fix causes the additional annotations not to work anymore or respectively breaking the validation somehow if present.
I've created a minimal reproducer under https://github.com/dreis2211/method-argument-validation-bug to ease testing. Rolling back to SB 3.2.1 fixes it, similar to removing the
@NotNull
annotation. I'd appreciate if this is either fixed or explicitly documented. Use cases withList
request bodies in combination with a@NotEmpty
seem to be unaffected because they don't seem to throw aMethodArgumentNotValidException
in the first place (also with SB 3.2.1).In case the
@NotNull
annotation is not supported under certain circumstances, feel free to turn this into a documentation issue.Let me know if you need anything.
Cheers,
Christoph
The text was updated successfully, but these errors were encountered: