spring-data-rest icon indicating copy to clipboard operation
spring-data-rest copied to clipboard

@Valid is not supported on @RepositoryRestController [DATAREST-593]

Open spring-projects-issues opened this issue 9 years ago • 27 comments

Bob Tiernay opened DATAREST-593 and commented

@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.


Affects: 2.3 GA (Fowler)

Issue Links:

  • DATAREST-1266 @BasePathAwareController disables DTO validation ("is duplicated by")

15 votes, 18 watchers

spring-projects-issues avatar Jun 22 '15 06:06 spring-projects-issues

Oliver Drotbohm commented

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

spring-projects-issues avatar Jun 22 '15 07:06 spring-projects-issues

Bob Tiernay commented

I will create one and attach

spring-projects-issues avatar Jun 22 '15 07:06 spring-projects-issues

Bob Tiernay commented

Here you are:

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

Cheers

spring-projects-issues avatar Jun 22 '15 08:06 spring-projects-issues

Bob Tiernay commented

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?

spring-projects-issues avatar Jun 23 '15 08:06 spring-projects-issues

Oliver Drotbohm commented

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

spring-projects-issues avatar Jun 23 '15 09:06 spring-projects-issues

Bob Tiernay commented

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

spring-projects-issues avatar Jun 23 '15 09:06 spring-projects-issues

Bob Tiernay commented

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

spring-projects-issues avatar Jun 23 '15 10:06 spring-projects-issues

Andrea Ratto commented

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

spring-projects-issues avatar Aug 12 '15 02:08 spring-projects-issues

Oliver Drotbohm commented

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?

spring-projects-issues avatar Aug 12 '15 03:08 spring-projects-issues

Andrea Ratto commented

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.
}

spring-projects-issues avatar Aug 12 '15 04:08 spring-projects-issues

Oliver Drotbohm commented

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

spring-projects-issues avatar Aug 12 '15 05:08 spring-projects-issues

Andrea Ratto commented

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

spring-projects-issues avatar Aug 12 '15 06:08 spring-projects-issues

Andrea Ratto commented

ERRATA: by ResourceAssembler I meant PersistentEntityResourceAssembler

spring-projects-issues avatar Aug 12 '15 06:08 spring-projects-issues

Thibaud Lepretre commented

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

spring-projects-issues avatar Aug 12 '15 08:08 spring-projects-issues

Andrea Ratto commented

@kakawait: but then I would be without the PersistentEntityResourceAssembler

spring-projects-issues avatar Aug 12 '15 09:08 spring-projects-issues

Yuriy Yunikov commented

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

spring-projects-issues avatar Oct 26 '16 07:10 spring-projects-issues

Daniele Renda commented

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

spring-projects-issues avatar Nov 30 '17 14:11 spring-projects-issues

Sergei Poznanski commented

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!.. )

spring-projects-issues avatar Nov 30 '17 15:11 spring-projects-issues

Daniele Renda commented

@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?

spring-projects-issues avatar Nov 30 '17 15:11 spring-projects-issues

Sergei Poznanski commented

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

spring-projects-issues avatar Nov 30 '17 16:11 spring-projects-issues

Daniele Renda commented

Any update about this issue? Thanks

spring-projects-issues avatar Jul 17 '18 14:07 spring-projects-issues

Bogdan Samondros commented

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

spring-projects-issues avatar Jul 31 '18 21:07 spring-projects-issues

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

spring-projects-issues avatar Jan 07 '21 10:01 spring-projects-issues

Doesn't work for @BasePathAwareController as well. Looks like Spring Data REST doesn't inject LocalValidatorFactoryBean instance into ExtendedServletRequestDataBinder while it resolves arguments and creates new binder.

emaysyuk avatar Jan 29 '21 15:01 emaysyuk

Faced the same issue

hmnshgpt455 avatar May 19 '21 06:05 hmnshgpt455

Really painful issue. We are thinking to not use spring data rest because of this issue.

dhunganawork avatar Aug 27 '21 15:08 dhunganawork

  1. WebMvcConfigurationSupport#requestMappingHandlerAdapter, Autowired mvcValidator
  2. WebMvcConfigurationSupport#getConfigurableWebBindingInitializer, setValidator

@Bean public RequestMappingHandlerAdapter requestMappingHandlerAdapter( @Qualifier("mvcContentNegotiationManager") ContentNegotiationManager contentNegotiationManager, @Qualifier("mvcConversionService") FormattingConversionService conversionService, @Qualifier("mvcValidator") Validator validator) {

RequestMappingHandlerAdapter adapter = createRequestMappingHandlerAdapter();
adapter.setContentNegotiationManager(contentNegotiationManager);
adapter.setMessageConverters(getMessageConverters());
adapter.setWebBindingInitializer(getConfigurableWebBindingInitializer(conversionService, validator));
adapter.setCustomArgumentResolvers(getArgumentResolvers());
adapter.setCustomReturnValueHandlers(getReturnValueHandlers());

if (jackson2Present) {
	adapter.setRequestBodyAdvice(Collections.singletonList(new JsonViewRequestBodyAdvice()));
	adapter.setResponseBodyAdvice(Collections.singletonList(new JsonViewResponseBodyAdvice()));
}

AsyncSupportConfigurer configurer = new AsyncSupportConfigurer();
configureAsyncSupport(configurer);
if (configurer.getTaskExecutor() != null) {
	adapter.setTaskExecutor(configurer.getTaskExecutor());
}
if (configurer.getTimeout() != null) {
	adapter.setAsyncRequestTimeout(configurer.getTimeout());
}
adapter.setCallableInterceptors(configurer.getCallableInterceptors());
adapter.setDeferredResultInterceptors(configurer.getDeferredResultInterceptors());

return adapter;

}

protected ConfigurableWebBindingInitializer getConfigurableWebBindingInitializer( FormattingConversionService mvcConversionService, Validator mvcValidator) {

ConfigurableWebBindingInitializer initializer = new ConfigurableWebBindingInitializer();
initializer.setConversionService(mvcConversionService);
**_initializer.setValidator(mvcValidator);_**
MessageCodesResolver messageCodesResolver = getMessageCodesResolver();
if (messageCodesResolver != null) {
	initializer.setMessageCodesResolver(messageCodesResolver);
}
return initializer;

}


// RepositoryRestMvcConfiguration#repositoryExporterHandlerAdapter, not autowired

@Bean public RequestMappingHandlerAdapter repositoryExporterHandlerAdapter() {

// Forward conversion service to handler adapter
ConfigurableWebBindingInitializer initializer = new ConfigurableWebBindingInitializer();
initializer.setConversionService(defaultConversionService());

RepositoryRestHandlerAdapter handlerAdapter = new RepositoryRestHandlerAdapter(defaultMethodArgumentResolvers());
handlerAdapter.setWebBindingInitializer(initializer);
handlerAdapter.setMessageConverters(defaultMessageConverters());

if (repositoryRestConfiguration().getMetadataConfiguration().alpsEnabled()) {
	handlerAdapter.setResponseBodyAdvice(Arrays.<ResponseBodyAdvice<?>> asList(alpsJsonHttpMessageConverter()));
}

return handlerAdapter;

}


My solution is:

@Configuration @RequiredArgsConstructor public class RepositoryRestMvcValidatorConfiguration implements InitializingBean {

@Resource(name = "mvcValidator")
private final Validator validator;

@Resource(name = "repositoryExporterHandlerAdapter")
private final RequestMappingHandlerAdapter repositoryExporterHandlerAdapter;

@Override
public void afterPropertiesSet() {
    ConfigurableWebBindingInitializer configurableWebBindingInitializer =
        (ConfigurableWebBindingInitializer) repositoryExporterHandlerAdapter.getWebBindingInitializer();
    if (configurableWebBindingInitializer != null) {
        configurableWebBindingInitializer.setValidator(validator);
    }
}

}

its work!


Is it possible to modify the source code in this way:

@Bean
public RequestMappingHandlerAdapter repositoryExporterHandlerAdapter(**_@Qualifier("mvcValidator") Validator validator_**) {

	// Forward conversion service to handler adapter
	ConfigurableWebBindingInitializer initializer = new ConfigurableWebBindingInitializer();
	initializer.setConversionService(defaultConversionService());

	RepositoryRestHandlerAdapter handlerAdapter = new RepositoryRestHandlerAdapter(defaultMethodArgumentResolvers());
	handlerAdapter.setWebBindingInitializer(initializer);
	handlerAdapter.setMessageConverters(defaultMessageConverters());

	if (repositoryRestConfiguration().getMetadataConfiguration().alpsEnabled()) {
		handlerAdapter.setResponseBodyAdvice(Arrays.<ResponseBodyAdvice<?>> asList(alpsJsonHttpMessageConverter()));
	}

	return handlerAdapter;
}

senekis316 avatar Feb 07 '22 10:02 senekis316