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

@Validated validation group not working on request body of List #32886

Closed
anaconda875 opened this issue May 23, 2024 · 16 comments
Closed

@Validated validation group not working on request body of List #32886

anaconda875 opened this issue May 23, 2024 · 16 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: superseded An issue that has been superseded by another

Comments

@anaconda875
Copy link

anaconda875 commented May 23, 2024

Given:

@Data
@NoArgsConstructor
@AllArgsConstructor
@Builder
public class VocabularyRequest {
  @Null(groups = CollectionCreation.class)
  @NotNull(groups = VocabularyCreation.class)
  private Long collectionId;

  @Valid private List<MeaningVariantRequest> meaningVariants;
}

This @Validated works:

  @PostMapping(consumes = APPLICATION_JSON_VALUE)
  @ResponseStatus(HttpStatus.CREATED)
  @Override
  public Mono<VocabularyResponse> create(
      @Validated(VocabularyCreation.class) @RequestBody VocabularyRequest request) {
    return super.create(request);
  }

But this (applied on a List) not:

  @PostMapping(value = "/bulk", consumes = APPLICATION_JSON_VALUE)
  @ResponseStatus(HttpStatus.CREATED)
  @Validated(VocabularyCreation.class)
  public Flux<VocabularyResponse> createBatch(
      @RequestBody List<@Valid VocabularyRequest> requests) {
    return service(VocabularyService.class).createBatch(requests);
  }

Change to this, still not work:

  @PostMapping(value = "/bulk", consumes = APPLICATION_JSON_VALUE)
  @ResponseStatus(HttpStatus.CREATED)
  public Flux<VocabularyResponse> createBatch(
      @Validated(VocabularyCreation.class) @RequestBody List<VocabularyRequest> requests) {
    return service(VocabularyService.class).createBatch(requests);
  }

If changing to @RequestBody List<@Validated(VocabularyCreation.class) VocabularyRequest> requests, got compile error:

'@Validated' not applicable to type use

NOTE: By "not working", I mean validation group not working. Not sure if @Valid works or not since I didnot observe it

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label May 23, 2024
@bclozel bclozel added the in: web Issues in web modules (web, webmvc, webflux, websocket) label May 27, 2024
@anaconda875 anaconda875 changed the title @Validated not working on request body of List @Validated validation group not working on request body of List May 28, 2024
@anaconda875
Copy link
Author

Hi @bclozel, any update on it?

@anaconda875
Copy link
Author

A workarroud:

public class ValidList<E> implements List<E> {

    @Valid
    private List<E> list;

    public ValidList() {
        this.list = new ArrayList<E>();
    }

    public ValidList(List<E> list) {
        this.list = list;
    }

    // Bean-like methods, used by javax.validation but ignored by JSON parsing

    public List<E> getList() {
        return list;
    }

    public void setList(List<E> list) {
        this.list = list;
    }

    // List-like methods, used by JSON parsing but ignored by javax.validation

    @Override
    public int size() {
        return list.size();
    }

    @Override
    public boolean isEmpty() {
        return list.isEmpty();
    }

    // Other list methods ...
}

and
@Validated(VocabularyCreation.class) @RequestBody ValidList<VocabularyRequest> requests

@bclozel
Copy link
Member

bclozel commented Jun 4, 2024

This has been discussed many times in this issue tracker, including in #32807.
If you want method validation to apply to your controller, you should annotate it (the controller class) with @Validated. See https://docs.spring.io/spring-framework/reference/web/webmvc/mvc-controller/ann-validation.html

@bclozel bclozel closed this as not planned Won't fix, can't repro, duplicate, stale Jun 4, 2024
@bclozel bclozel added status: invalid An issue that we don't feel is valid for: stackoverflow A question that's better suited to stackoverflow.com and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jun 4, 2024
@anaconda875
Copy link
Author

anaconda875 commented Jun 4, 2024

This has been discussed many times in this issue tracker, including in #32807. If you want method validation to apply to your controller, you should annotate it (the controller class) with @Validated. See https://docs.spring.io/spring-framework/reference/web/webmvc/mvc-controller/ann-validation.html

@bclozel the issue is not about where to put @validated or @Valid on, but is about @validated not working at all. Please read my description CAREFULLY. Wherever I put the annotation on, the validation did not work. The only way to make it work is to use my workarround
Ps: your github ticket is not relevant to my issue.

@bclozel
Copy link
Member

bclozel commented Jun 4, 2024

Please read my description CAREFULLY.

I did. I have also tried the following:

public class VocabularyRequest {

	@NotEmpty
	private String id;

	public String getId() {
		return id;
	}

	public void setId(String id) {
		this.id = id;
	}
}
@RestController
@Validated
public class TestController {

	@PostMapping(path = "/bulk", consumes = APPLICATION_JSON_VALUE)
	@ResponseStatus(HttpStatus.CREATED)
	public Flux<String> createBatch(
			@RequestBody @Valid List<VocabularyRequest> requests) {
		return Flux.fromIterable(requests).map(VocabularyRequest::getId);
	}

}

When sending [{"id":""},{"id":"2"}] as the request body, I'm getting the following in the console:

jakarta.validation.ConstraintViolationException: createBatch.requests[0].id: must not be empty
	at org.springframework.validation.beanvalidation.MethodValidationInterceptor.invoke(MethodValidationInterceptor.java:170) ~[spring-context-6.1.8.jar:6.1.8]
	Suppressed: reactor.core.publisher.FluxOnAssembly$OnAssemblyException: 
Error has been observed at the following site(s):
	*__checkpoint ⇢ HTTP POST "/bulk" [ExceptionHandlingWebHandler]

Thoughts?

@anaconda875
Copy link
Author

anaconda875 commented Jun 4, 2024

@bclozel then you did not read my desc carefully.

NOTE: By "not working", I mean validation group not working. Not sure if @Valid works or not since I didnot observe it

@validated is useful only when using with validation group. Otherwise why i have to use it while i can just use @Valid?

@bclozel
Copy link
Member

bclozel commented Jun 4, 2024

Does this comment explain better the current situation?

#31775 (comment)

@bclozel bclozel reopened this Jun 4, 2024
@bclozel bclozel added status: waiting-for-triage An issue we've not yet triaged or decided on status: waiting-for-feedback We need additional information before we can continue and removed status: invalid An issue that we don't feel is valid for: stackoverflow A question that's better suited to stackoverflow.com labels Jun 4, 2024
@anaconda875
Copy link
Author

@bclozel maybe I didnot describe the issue clearly. Let me re-describe:
Given:

@Data
@NoArgsConstructor
@AllArgsConstructor
@Builder
public class VocabularyRequest {
  @Null(groups = CollectionCreation.class)
  @NotNull(groups = VocabularyCreation.class)
  private Long collectionId;
}

This @Validated works, because it applied on NORMAL DTO/POJO:

  @PostMapping(consumes = APPLICATION_JSON_VALUE)
  @ResponseStatus(HttpStatus.CREATED)
  @Override
  public Mono<VocabularyResponse> create(
      @Validated(VocabularyCreation.class) @RequestBody VocabularyRequest request) {
    return super.create(request);
  }

By "works", I mean validation group works. For ex I send {"collectionId": null}, Spring will reject because of @NotNull(groups = VocabularyCreation.class). If I change @Validated(VocabularyCreation.class) to @Validated(CollectionCreation.class), Spring will accept because of @Null(groups = CollectionCreation.class)

Now, changing the Request body from NORMAL DTO/POJO to a List, the @Validated will not work:

  @PostMapping(value = "/bulk", consumes = APPLICATION_JSON_VALUE)
  @ResponseStatus(HttpStatus.CREATED)
  @Validated(VocabularyCreation.class)
  public Flux<VocabularyResponse> createBatch(
      @RequestBody List<@Valid VocabularyRequest> requests) {
    return service(VocabularyService.class).createBatch(requests);
  } 

By "not work", I mean validation group does not work. Does not matter I send {"collectionId": null} or {"collectionId": 1}; or changing @Validated(VocabularyCreation.class) to @Validated(CollectionCreation.class) vice versa, Spring will ACCEPT ALL. But I expect it will work like the normal case above

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jun 5, 2024
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Jun 5, 2024

@anaconda875 this could be more of a bean validation question. In the first case, we should be applying bean validation directly on VocabularyRequest from within the @RequestBody argument resolver and passing validation groups with that. In the second case, we should be applying method validation and still passing the validation groups. Assuming it's as expected, I'm not sure why it is not working, and whether it's supposed to work. In other words, applying groups and cascading validation on container elements.

Could you provide a small sample to experiment with?

@bclozel
Copy link
Member

bclozel commented Jun 5, 2024

I have created a sample repository for one of the use cases I've tested, I'm not sure if this covers @anaconda875 's app.
See https://github.com/bclozel/gh-32886/blob/main/src/main/java/org/example/validation/TestController.java

A controller like the following will not validate the parameters, unless you add a @Validated annotation on the controller to trigger method-level validation. Method-level validation is not enabled when looking at the controller method in

MethodValidationInitializer.checkArguments(this.beanType, getMethodParameters()) :

@RestController
public class TestController {

	@PostMapping(path = "/bulk", consumes = APPLICATION_JSON_VALUE)
	@ResponseStatus(HttpStatus.CREATED)
	@Validated(MyGroup.class)
	public Flux<String> createBatch(
			@RequestBody List<@Valid VocabularyRequest> requests) {
		return Flux.fromIterable(requests).map(VocabularyRequest::getId);
	}

}

I have also tried the following variant, and this fails as expected with a HandlerMethodValidationException:

@RestController
public class TestController {

	@PostMapping(path = "/bulk", consumes = APPLICATION_JSON_VALUE)
	@ResponseStatus(HttpStatus.CREATED)
	@Validated(MyGroup.class)
	public Flux<String> createBatch(
			@RequestBody @Valid List<VocabularyRequest> requests) {
		return Flux.fromIterable(requests).map(VocabularyRequest::getId);
	}

}

@anaconda875
Copy link
Author

anaconda875 commented Jun 5, 2024

Hi @bclozel @rstoyanchev I have created a sample project. I also have described very clearly at JAVADOC.
image
Again, it is about @Validated/validation group. Please don't confuse with @Valid

@bclozel bclozel added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Jun 5, 2024
@bclozel bclozel added this to the 6.1.9 milestone Jun 5, 2024
@bclozel
Copy link
Member

bclozel commented Jun 5, 2024

Thanks for the sample @anaconda875

@Valid in container annotations

We've found that indeed the following combination doesn't work, and it should:

@RestController
public class TestController {

	@PostMapping(path = "/bulk", consumes = APPLICATION_JSON_VALUE)
	@ResponseStatus(HttpStatus.CREATED)
	@Validated(MyGroup.class)
	public Flux<String> createBatch(
			@RequestBody List<@Valid VocabularyRequest> requests) {
		return Flux.fromIterable(requests).map(VocabularyRequest::getId);
	}

}

@Validated on collections

You initially mentioned this case in your first comment, but your latest sample doesn't show this and you said:

Again, it is about @Validated/validation group. Please don't confuse with @Valid

So is this issue about the first case, or about the fact that @Validated works for single elements but not for collections?

// @Validated works on a single VocabularyRequest type

@PostMapping(consumes = APPLICATION_JSON_VALUE)
@ResponseStatus(HttpStatus.CREATED)
public Mono<VocabularyResponse> create(
    @Validated(VocabularyCreation.class) @RequestBody VocabularyRequest request) {
  return Mono.empty();
}

// @Validated does not work on a collection of VocabularyRequest
@PostMapping(value = "/bulk", consumes = APPLICATION_JSON_VALUE)
@ResponseStatus(HttpStatus.CREATED)
public Flux<VocabularyResponse> createBatch(
    @Validated(VocabularyCreation.class) @RequestBody List<VocabularyRequest> requests) {
  return Flux.empty();
}

Please let us know.

@anaconda875
Copy link
Author

anaconda875 commented Jun 5, 2024

@bclozel the issue is about @validated does not work for Collection. My sample DO show this.
Take a look at /bulk, the validation group does not works at all as I always receive 201, mean that validation does not run. The /bulk2 is a workaround to make @validated work, at least I could see 400 . My code also contain javadoc, kindly take a look
Do you satify with this answer? Feel free to discuss

@bclozel
Copy link
Member

bclozel commented Jun 5, 2024

Thanks for your feedback and patience.

Instance validation

In the case of a single VocabularyRequest, @Validated triggers instance validation for the provided instance using org.hibernate.validator.internal.engine.ValidatorImpl#validate. This effectively looks at the root instance and its fields for constraint annotations and runs validation accordingly.

@PostMapping(consumes = APPLICATION_JSON_VALUE)
@ResponseStatus(HttpStatus.CREATED)
public Mono<VocabularyResponse> create(
    @Validated(VocabularyCreation.class) @RequestBody VocabularyRequest request) {
  return Mono.empty();
}

In the case of List<VocabularyRequest>, we are also using org.hibernate.validator.internal.engine.ValidatorImpl#validate with the expected input. @Validated does work here, since the validation group is given to the validator. The same algorithm operates and it won't find any constraint annotation on the type itself or its fields. This explains why no validation is performed: no constraints were found.

@PostMapping(value = "/bulk", consumes = APPLICATION_JSON_VALUE)
@ResponseStatus(HttpStatus.CREATED)
public Flux<VocabularyResponse> createBatch(
    @Validated(VocabularyCreation.class) @RequestBody List<VocabularyRequest> requests) {
  return Flux.empty();
}

Again, a signature like @Validated(VocabularyCreation.class) @RequestBody List<@Valid VocabularyRequest> requests would solve the problem here but this is a bug and we'll create another issue for that.

Method validation

This leaves us with method validation, see my previous comment: #32886 (comment)

The method validation case behaves differently and validates parameters in a different way as it uses org.hibernate.validator.internal.engine.ValidatorImpl#validateParameters(T, java.lang.reflect.Method, java.lang.Object[], java.lang.Class<?>...). This variant is working.

At this point, I don't think we can do anything here: instance validation and method validation behave differently. In both cases, we are extracting the right information (including validation groups), the only difference is that the considered root type in the first case is ArrayList and ValidationRequest in the second, within the validator implementation.

Maybe this should be an enhancement request for Hibernate validation? I haven't seen any behavior describing this in the Bean Validation spec so far.

As a result, I'm tempted to decline this issue as I don't see anything actionable from the Spring side.

@anaconda875
Copy link
Author

Got it, thank you

@bclozel
Copy link
Member

bclozel commented Jun 5, 2024

Partially superseded by #32964 for the issue we've found while discussing this ticket.

Correcting my previous statements here. Method validation is not required, only a constraint annotation is required on the parameter to work.

@RestController
@RequestMapping("/vocabularies")
// No @Validated annotation required
public class VocabularyResource {

  @PostMapping(value = "/bulk", consumes = APPLICATION_JSON_VALUE)
  @ResponseStatus(HttpStatus.CREATED)
  // Select validation group
  @Validated(VocabularyCreation.class)
  public Flux<VocabularyResponse> createBatch(
       @RequestBody @Valid List<VocabularyRequest> requests) {
    return Flux.empty();
  }

So the only issue with the problematic use case is that the parameter does not have any constraint annotation on the parameter declaration (in the controller method signature), nor on the type itself (a List).

With that in mind, I don't think it's a problem in Hibernate Validation at all, it's behaving consistently.

@bclozel bclozel closed this as not planned Won't fix, can't repro, duplicate, stale Jun 5, 2024
@bclozel bclozel added status: superseded An issue that has been superseded by another and removed type: bug A general bug labels Jun 5, 2024
@bclozel bclozel self-assigned this Jun 5, 2024
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) status: superseded An issue that has been superseded by another
Projects
None yet
Development

No branches or pull requests

4 participants