spring-framework icon indicating copy to clipboard operation
spring-framework copied to clipboard

Revisit validation documentation to better explain when method validation is invoked

Open mlichtblau opened this issue 9 months ago • 3 comments

Affects: spring-web-6.1.6

Hi, I encountered some unexpected behavior with the new built-in handler method validation. In my example my handler method has two parameters, one path variable and one request body:

    @PostMapping("/{pathVariable}/test1")
    String test1(@PathVariable("pathVariable") String pathVariable, @RequestBody DemoTransport demoTransport) {
        return "SUCCESS";
    }

The issue occurs when I want to validate the DemoTransport by adding an @Valid annotation. The validation is performed differently depending on whether another constraint annotation is present on any of the paramaters, for example on pathVariable.

I prepared a small demo, here are my two controller methods. Both have the @Valid transportation and one has an additional @NotNull annotation on the pathVariable.

Here are the tests with my expectations. If my requests correctly supplies a non-null pathVariable but an invalid DemoTransport I expect the same HandlerMethodValidationException to be thrown for both controller methods. For the handler method with the @NotNull annotation the handler method is validated and the HandlerMethodValidationException is correctly thrown, because this condition evaluates to true. For the handler method without the @NotNull annotation no hander method validation is performed, only a org.springframework.web.bind.MethodArgumentNotValidException is thrown, because this check does not evaluate to true because DemoTransport is not isIndexOrKeyBasedContainer.

I don't think the presence of an annotation on another parameter should influence whether the validation of the parameter annoted with @Valid is performed as part of the method handler validation or not. I am not sure I understand why, in this case, only an index- or key-based container can be validated? Apparently the parameter is correctly validated as seen by the other controller method.

Is it possible to remove this check and always validate the method when an @Valid is present on one of the parameters?

Consistent behavior is important for us for multiple reasons, for example because we are using ControllerAdvice and want to rely on the HandlerMethodValidationException to be thrown.

Thank you, and please let me know if I can help.

mlichtblau avatar May 13 '24 07:05 mlichtblau

This was discussed in https://github.com/spring-projects/spring-framework/issues/31775#issuecomment-1856654770 and also under #32082. Essentially, arguments may be validated as an individual value when that's sufficient, or with method validation when method parameters have constraints. The former is long standing behavior that would disrupt a lot of applications that don't need to apply method validation. The two exceptions however are designed to be very similar, and can be handled with almost identical code, or likewise one can be adapted to the other.

rstoyanchev avatar May 14 '24 14:05 rstoyanchev

Thanks for your reply and sorry I missed the previous discussion on the topic. Now with the added context, the Validation documentation makes also more sense to me. The mistake on my part was that I assumed @Valid is also an @Constraint validation which it is not.

Maybe it was just confusing to me, but I still think the documentation could be improved in regards to wording:

The validation support works on two levels. First, resolvers for [...] @RequestBody [...] perform validation [...] and raise MethodArgumentNotValidException if necessary. Second, [...] ,then method validation is applied instead, raising HandlerMethodValidationException if necessary.

The "instead" is quite important and I missed it, because otherwise it sounds like there is a temporal relation between these two levels. What I understood intially: First the first level validation is applied and MethodArgumentNotValidException raised and only if the request body is valid then second level method validation is applied for the other parameters with @Constraint raising HandlerMethodValidationException.

This is why I was confused why my first example test didn't throw the MethodArgumentNotValidException.

Otherwise, the ticket can be closed and I will handle both exceptions as the others did, too. Thank you!

mlichtblau avatar May 15 '24 13:05 mlichtblau

Thanks for the feedback. Rossen and I had a quick chat and we're going to look at improving the wording there.

snicoll avatar May 21 '24 09:05 snicoll

I've revised that section of the documentation based on the specific feedback. Hopefully that's better now.

rstoyanchev avatar May 22 '24 16:05 rstoyanchev