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

Built-in method validation does not work with Boot auto-configuration #31082

Closed
sephiroth-j opened this issue Aug 21, 2023 · 3 comments
Closed
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug
Milestone

Comments

@sephiroth-j
Copy link
Contributor

There are two issues with the new feature of validating method parameters.

  1. Validation happens only if @Validated is used on the class level. If it is used on method level, nothing happens at all. The documentation suggest to remove class level annotation and use a method level annotation instead.
    NOTE: If a controller has a class level `@Validated`, then
    xref:core/validation/beanvalidation.adoc#validation-beanvalidation-spring-method[method validation is applied]
    through an AOP proxy. In order to take advantage of the Spring MVC built-in support for
    method validation added in Spring Framework 6.1, you need to remove the class level
    `@Validated` annotation from the controller.
2023-08-21T09:39:56.099+02:00 DEBUG 11436 --- [           main] o.s.w.r.f.client.ExchangeFunctions       : [2f04993d] HTTP GET /method/-1
2023-08-21T09:39:56.111+02:00 DEBUG 11436 --- [     parallel-1] o.s.t.w.r.server.HttpHandlerConnector    : Writing client request for  GET "/method/-1"
2023-08-21T09:39:56.118+02:00 DEBUG 11436 --- [     parallel-1] o.s.t.w.r.server.HttpHandlerConnector    : Invoking HttpHandler for  GET "/method/-1"
2023-08-21T09:39:56.126+02:00 DEBUG 11436 --- [     parallel-1] o.s.w.s.adapter.HttpWebHandlerAdapter    : [5d04b1ce] HTTP GET "/method/-1"
2023-08-21T09:39:56.143+02:00 DEBUG 11436 --- [     parallel-1] s.w.r.r.m.a.RequestMappingHandlerMapping : [5d04b1ce] Mapped to com.example.demo.MethodLevelController#getEntity(Long)
2023-08-21T09:39:56.164+02:00 DEBUG 11436 --- [     parallel-1] o.s.w.s.adapter.HttpWebHandlerAdapter    : [5d04b1ce] Completed 200 OK
2023-08-21T09:39:56.170+02:00 DEBUG 11436 --- [     parallel-1] o.s.t.w.r.server.HttpHandlerConnector    : Creating client response for  GET "/method/-1"
2023-08-21T09:39:56.176+02:00 DEBUG 11436 --- [     parallel-1] o.s.w.r.f.client.ExchangeFunctions       : [2f04993d] [7eae50ac] Response 200 OK
  1. On class level: Instead of an expected bad request response, an internal server error is thrown.
2023-08-21T09:36:05.610+02:00 DEBUG 11860 --- [           main] o.s.w.r.f.client.ExchangeFunctions       : [200d1a3d] HTTP GET /class/-1
2023-08-21T09:36:05.610+02:00 DEBUG 11860 --- [     parallel-2] o.s.t.w.r.server.HttpHandlerConnector    : Writing client request for  GET "/class/-1"
2023-08-21T09:36:05.610+02:00 DEBUG 11860 --- [     parallel-2] o.s.t.w.r.server.HttpHandlerConnector    : Invoking HttpHandler for  GET "/class/-1"
2023-08-21T09:36:05.610+02:00 DEBUG 11860 --- [     parallel-2] o.s.w.s.adapter.HttpWebHandlerAdapter    : [70770775] HTTP GET "/class/-1"
2023-08-21T09:36:05.611+02:00 DEBUG 11860 --- [     parallel-2] s.w.r.r.m.a.RequestMappingHandlerMapping : [70770775] Mapped to com.example.demo.ClassLevelController#getEntity(Long)
2023-08-21T09:36:05.618+02:00 DEBUG 11860 --- [     parallel-2] o.h.v.r.PlatformResourceBundleLocator    : ValidationMessages not found.
2023-08-21T09:36:05.619+02:00 DEBUG 11860 --- [     parallel-2] o.h.v.r.PlatformResourceBundleLocator    : ContributorValidationMessages not found.
2023-08-21T09:36:05.621+02:00 DEBUG 11860 --- [     parallel-2] o.h.v.r.PlatformResourceBundleLocator    : org.hibernate.validator.ValidationMessages found.
2023-08-21T09:36:05.642+02:00 DEBUG 11860 --- [     parallel-2] a.w.r.e.AbstractErrorWebExceptionHandler : [70770775] Resolved [ConstraintViolationException: getEntity.id: muss größer als 0 sein] for HTTP GET /class/-1
2023-08-21T09:36:05.643+02:00 ERROR 11860 --- [     parallel-2] a.w.r.e.AbstractErrorWebExceptionHandler : [70770775]  500 Server Error for HTTP GET "/class/-1"

jakarta.validation.ConstraintViolationException: getEntity.id: muss größer als 0 sein
	at org.springframework.validation.beanvalidation.MethodValidationInterceptor.invoke(MethodValidationInterceptor.java:146) ~[spring-context-6.1.0-M4.jar:6.1.0-M4]
	Suppressed: reactor.core.publisher.FluxOnAssembly$OnAssemblyException: 
Error has been observed at the following site(s):
	*__checkpoint ? HTTP GET "/class/-1" [ExceptionHandlingWebHandler]
Original Stack Trace:
		at org.springframework.validation.beanvalidation.MethodValidationInterceptor.invoke(MethodValidationInterceptor.java:146) ~[spring-context-6.1.0-M4.jar:6.1.0-M4]
		at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:184) ~[spring-aop-6.1.0-M4.jar:6.1.0-M4]
		at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.proceed(CglibAopProxy.java:751) ~[spring-aop-6.1.0-M4.jar:6.1.0-M4]
		at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:703) ~[spring-aop-6.1.0-M4.jar:6.1.0-M4]
		at com.example.demo.ClassLevelController$$SpringCGLIB$$0.getEntity(<generated>) ~[classes/:na]
		at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:na]
		at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77) ~[na:na]
		at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:na]
		at java.base/java.lang.reflect.Method.invoke(Method.java:568) ~[na:na]
		at org.springframework.web.reactive.result.method.InvocableHandlerMethod.lambda$invoke$0(InvocableHandlerMethod.java:178) ~[spring-webflux-6.1.0-M4.jar:6.1.0-M4]

Tested with Spring Boot 3.2.0-M1 and Spring Framework 6.1.0-M3 and 6.1.0-M4. Sample project is attached.

spring-boot-reactive-demo.zip

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Aug 21, 2023
@sdeleuze
Copy link
Contributor

@rstoyanchev See #31045 where I worked on WebFlux type conversion refinements to get a bad request response instead of an internal server error.

@rstoyanchev rstoyanchev self-assigned this Aug 22, 2023
@rstoyanchev rstoyanchev added in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Aug 22, 2023
@rstoyanchev rstoyanchev added this to the 6.1.0-RC1 milestone Aug 22, 2023
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Aug 22, 2023

Thanks for the sample.

For built-in method validation to work, there needs be a globally configured jakarta.validation.Validator. However, it looks like WebFluxAutoConfigurationSupport overrides the webFluxValidator bean method, and adds a ValidatorAdapter wrapper which intentionally hides the jakarta.validation.Validator. As a result, HandlerMethodValidator#from does not see the validator as a Jakarta Validator and method validation is not applied. The same is done in WebMvcAutoConfigurationSupport so the issue applies to Spring MVC too.

ValidatorAdapter has a getTarget method, but the Spring Framework can't depend on a Spring Boot class. We need to figure something out.

By the way, the built-in method validation does not need a method-level @Validated. In other words the following is sufficient (once this issue is resolved) because there is a constraint annotation:

@RestController
@RequestMapping(produces = MediaType.APPLICATION_JSON_VALUE, path = "/method")
public class MethodLevelController {

	@GetMapping(path = "/{id}")
	public Mono<Void> getEntity(@PathVariable @Positive Long id) {
		return Mono.empty();
	}
}

@rstoyanchev rstoyanchev changed the title WebFlux: Validation of method parameters not working Built-in method validation does not work with Boot auto-configuration Aug 22, 2023
rstoyanchev added a commit to rstoyanchev/spring-framework that referenced this issue Aug 23, 2023
rstoyanchev added a commit to rstoyanchev/spring-framework that referenced this issue Aug 23, 2023
rstoyanchev added a commit to rstoyanchev/spring-framework that referenced this issue Aug 23, 2023
rstoyanchev added a commit to rstoyanchev/spring-framework that referenced this issue Aug 23, 2023
rstoyanchev added a commit to rstoyanchev/spring-framework that referenced this issue Aug 23, 2023
rstoyanchev added a commit to rstoyanchev/spring-framework that referenced this issue Aug 23, 2023
rstoyanchev added a commit to rstoyanchev/spring-framework that referenced this issue Aug 23, 2023
rstoyanchev added a commit to rstoyanchev/spring-framework that referenced this issue Aug 23, 2023
rstoyanchev added a commit to rstoyanchev/spring-framework that referenced this issue Aug 23, 2023
rstoyanchev added a commit to rstoyanchev/spring-framework that referenced this issue Aug 23, 2023
rstoyanchev added a commit to rstoyanchev/spring-framework that referenced this issue Aug 23, 2023
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Aug 23, 2023

I've made updates on the Spring Framework side with 832b49f, c441fc7, and 4bf54a2, which completes the work necessary on the Spring Framework side. There is also a follow-up change required on the Spring Boot side, see spring-projects/spring-boot#37081. I expect that will be done for 3.2 M3.

I am closing this issue for now, but I'll test with the sample after the Boot issue is done. If necessary we can reopen this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants