[DATAREST-593] @Valid is not supported on @RepositoryRestController Created: 22/Jun/15  Updated: 07/Aug/18

Status: Waiting for Feedback
Project: Spring Data REST
Component/s: Repositories
Affects Version/s: 2.3 GA (Fowler)
Fix Version/s: None

Type: Bug Priority: Minor
Reporter: Bob Tiernay Assignee: Oliver Drotbohm
Resolution: Unresolved Votes: 12
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Duplicate
is duplicated by DATAREST-1266 @BasePathAwareController disables DTO... Resolved
Last updater: Oliver Drotbohm

 Description   

@Valid annotations are not respected on @RepositoryRestController annotated controllers as they are with @Controller and @RestController classes. This breaks with convention and expectation of developers.



 Comments   
Comment by Oliver Drotbohm [ 22/Jun/15 ]

Do you have a sample project or test case to reproduce the issue?

Comment by Bob Tiernay [ 22/Jun/15 ]

I will create one and attach.

Comment by Bob Tiernay [ 22/Jun/15 ]

Here you are:

https://github.com/btiernay/DATAREST-593

Cheers

Comment by Bob Tiernay [ 23/Jun/15 ]

I should also ask for clarification on the relationship between ValidatingRepositoryEventListener and ValidatingMongoEventListener. When using @RepositoryRestController, it appears as though ValidatingRepositoryEventListener never gets called. However, ValidatingMongoEventListener, if registered, does get called. ValidatingMongoEventListener produces ConstraintViolationException s where as ValidatingRepositoryEventListener produces RepositoryConstraintViolationException s. The former is incompatible with RepositoryRestExceptionHandler. Should both of these exception types be supported in the handler, or should ValidatingMongoEventListener be modified to only throw RepositoryConstraintViolationException?

Comment by Oliver Drotbohm [ 23/Jun/15 ]

This is basically as expected:

  • ValidatingRepositoryEventListener is listening to events triggered by the Spring Data REST controllers. Thus, RepositoryRestExceptionHandler handles these.
  • ValidatingMongoEventListener is implementing validation on the MongoTemplate level. Spring Data REST doesn't know anything about store specifics and thus doesn't handle store specific exceptions.

So what you see is basically caused by the difference between a manually implemented controller using the repositories and thus triggering low level validation and the Spring Data REST controllers that trigger events on the resource level.

Comment by Bob Tiernay [ 23/Jun/15 ]

Okay, that makes a lot of sense. Thanks for explaining.

Comment by Bob Tiernay [ 23/Jun/15 ]

On second thought, shouldn't there be something in the Data REST stack that translates ConstraintViolationException s to proper 400 responses with a payload explaining the error? Right now I'm seeing a 500 because the exception bubbles all the way up and isn't handled.

Comment by Andrea Ratto [ 12/Aug/15 ]

I would like to see this issue accepted and fixed: I refactored a controller to be a @RepositoryRestController and lost request validation without any notice.

In my case validation on db save is too late: the update has been published on a queue.

Comment by Oliver Drotbohm [ 12/Aug/15 ]

I am not sure what you think should be accepted here. As I outlined in my comment above this is basically working as expected. What exactly is it that you think is not working? Are you using `ValidatingRepositoryEventListener` and it's not applied?

Comment by Andrea Ratto [ 12/Aug/15 ]

Basically I think that @Valid should work on @RequestMapping methods, just like normal controllers. That is:

@RequestMapping(value = "/path", method = RequestMethod.POST)
public post(@Valid @RequestBody Enitity entity) {
  doStuff(entity); // this line should not be reached if the entity is not valid.
}
Comment by Oliver Drotbohm [ 12/Aug/15 ]

You're not writing a controller with Spring Data REST, it's all internal. So besides the implementation aspect, what would you like to see happening by default? Currently we require developers to activate JSR-303 support explicitly as SD REST works with domain objects and – while being a decent choice to validate DTOs – JSR-303 is a rather poor one for validating domain objects. The explicit activation requirement is basically a hint in terms of: we know it's a sub-obtimal idea, but you seem to insist on doing this.

Comment by Andrea Ratto [ 12/Aug/15 ]

Specifically I am updating our API to HAL, leveraging Spring Data Rest for reads, but providing all write methods with my custom implementation that involves sending events to rabbitmq, rather than writing directly.

Thanks to @RepositoryRestController I am able to mimick the same output formats (using the ResourceAssembler) and endpoints of SD REST.

I am deleting all our old custom request handlers for reads and porting the remaining code from @RestController to @RepositoryRestController, adding and replacing Spring Data Rest API methods as necessary.

Some of these methods for example take complex request objects and update multiple entities at once. These request object are annotated and were validated thanks to the @Valid annotation in the controller method.

So it surely feels I am writing a controller (in fact I am refactoring controllers) and, just like the bug description says, that an expectation was broken.

If I were to swap back the @RepositoryRestController with @RestController, the @Valid annotation on a request parameter will trigger validation and, if necessary, output a 400 response before invoking any method code. But I would loose the ResourceAssembler.

I would expect @Valid to work by validating the annotated parameter before the method call, just like a normal controller. I hope my explanation was clear and this is a reasonable use case of SDR.
Otherwise I need to find a way to format HAL responses with the ResourceAssembler and a normal @RestController.

Comment by Andrea Ratto [ 12/Aug/15 ]

ERRATA: by ResourceAssembler I meant PersistentEntityResourceAssembler

Comment by Thibaud Lepretre [ 12/Aug/15 ]

Andrea Ratto you can create your own org.springframework.web.servlet.HandlerMapping as workaround (what I did) in order to allow mutual cohabitation between SDR and @RestController.

Comment by Andrea Ratto [ 12/Aug/15 ]

@kakawait: but then I would be without the PersistentEntityResourceAssembler.

Comment by Yuriy Yunikov [ 26/Oct/16 ]

I've created a workaround for the issue using custom RequestBodyAdviceAdapter which works for me well. Check out my answer on StackOverflow.

Comment by Daniele Renda [ 30/Nov/17 ]

Some update on this? I'm experiencing the same problem. I saw the workaround of @Yuriy Yunikov. What is the official position of Spring team about this? Thanks

Comment by Sergei Poznanski [ 30/Nov/17 ]

I agree - this is extremely necessary functionality! It must be implemented.

Often we have to implement custom method in RepositoryRestController (and get access to injected PersistentEntityResourceAssembler), but we cannot do it because Bean Validation doesn't work in RepositoryRestController (as well as in BasePathAwareController).

To workaround this situation we can use InitBinder in our controller:

        @Autowired private LocalValidatorFactoryBean beanValidator;

	@InitBinder
	protected void initBinder(WebDataBinder binder) {
		binder.addValidators(beanValidator);
	}

But in this case the custom validators stop working.

Well, we try to add them to InitBinder:

        @Autowired private LocalValidatorFactoryBean beanValidator;
        @Autowired private CustomValidator customValidator;

	@InitBinder
	protected void initBinder(WebDataBinder binder) {
		binder.addValidators(beanValidator, customValidator);
	}

But in this case Custom Validator is invoked for every DTO object, marked with @Valid annotation, passed to every custom methods in this controller... ((

It's horrible guys!.. )

Comment by Daniele Renda [ 30/Nov/17 ]

@Sergei Poznanski Even using the initBinder anyway, the response I've from the controller in case of error is different from the one returned from SDR repository.

Using the initBinder the response is:

{
    "timestamp": "2017-11-30T15:44:21.066+0000",
    "status": 400,
    "error": "Bad Request",
    "exception": "org.springframework.web.bind.MethodArgumentNotValidException",
    "errors": [
        {
            "codes": [
                "Size.restEmail.to",
                "Size.to",
                "Size.[Ljava.lang.String;",
                "Size"
            ],
            "arguments": [
                {
                    "codes": [
                        "restEmail.to",
                        "to"
                    ],
                    "arguments": null,
                    "defaultMessage": "to",
                    "code": "to"
                },
                2147483647,
                1
            ],
            "defaultMessage": "Il numero di destinatari deve essere compreso maggiore di [1]. Correggere i valori e ripetere l'operazione. ",
            "objectName": "restEmail",
            "field": "to",
            "rejectedValue": [],
            "bindingFailure": false,
            "code": "Size"
        }
    ],
    "message": "Validation failed for object='restEmail'. Error count: 1",
    "path": "/api/v1/emails"
}

instead of the standard SDR response:

{
    "errors": [
        {
            "entity": "Email",
            "property": "to",
            "invalidValue": [],
            "message": "Il numero di destinatari deve essere compreso maggiore di [1]. Correggere i valori e ripetere loperazione. "
        }
    ]
}

Is there a way to uniform the first response to the SDR default one?

Comment by Sergei Poznanski [ 30/Nov/17 ]

@Daniele I created a custom class for user messages and fill it in ExceptionHandler...

Comment by Daniele Renda [ 17/Jul/18 ]

Any update about this issue? Thanks

Comment by Bogdan Samondros [ 31/Jul/18 ]

+1. Was faced with this case today. Very sstrange that no customization for SDR. It's realy painful

Generated at Fri Aug 14 12:07:11 UTC 2020 using Jira 8.5.4#805004-sha1:0444eab799707f9ad7b248d69f858774aadfd250.