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

When is the ValidationPipelineBehavior ever used? #24

Open
VictorioBerra opened this issue Feb 23, 2018 · 3 comments
Open

When is the ValidationPipelineBehavior ever used? #24

VictorioBerra opened this issue Feb 23, 2018 · 3 comments

Comments

@VictorioBerra
Copy link

I noticed that all the validators run as expected during Model Binding (probably because of .AddFluentValidation(cfg => { cfg.RegisterValidatorsFromAssemblyContaining<Startup>(); });) and they never even have a chance to make it to the ValidationPipelineBehavior. I assume that is why we just throw in there because something is very wrong if we got that far.

Why even have this? Just as a safety net or are there situations where this comes in handy?

@domas-st
Copy link

I ran into the same issue. I debugged into it and recognized that there are two separate validations being made using the same validators. The first one is done by the mvc pipeline using FluentValidation and the ValidatorActionFilter. The second one is done by the mediatR pipeline initiated by ValidationPipelineBehavior while using the same validator objects being injected into it. Taking UsersController.Create as an example:
Before UsersController.Create is called, the mvc validation validates the Conduit.Features.Users.Create.Command object. If it fails ValidatorActionFilter will respond with Statuscode 422. However, if it does not fail, the object is being handed over to the mediatR by calling Send(). Now the second validation issued by ValidationPipelineBehavior is taking place. As this validates the same object using the same validators, the validation will never fail.
So yes in this case, ValidationPipelineBehavior is useless. However, if you pass a different object to mediatR than that being passed to the controller, the second validation makes sence again.
But here, imho, lies the actual problem: ValidationPipelineBehavior throws ValidationExceptions which are not being handled appropriately. Instead, the exception ends up being handled in the ErrorHandlingMiddleware which would respond with Statuscode 500.

TL;DR: ValidationPipelineBehavior makes sence if you pass a different object to mediatR than that being passed to the controller. Instead of throwing ValidationExceptions, I suggest doing the same thing as in ValidatorActionFilter: respond with Statuscode 422 and an appropriate payload explaining the problem.

@VictorioBerra
Copy link
Author

I'm not sure if we should respond with a 422 in this situation. Mediatr is designed to have multiple handlers file for a single command and commands might not always be triggered by a controller. I'm still new with Mediatr but do events use the same pipeline?

@domas-st
Copy link

domas-st commented Apr 1, 2018

Good point, I didn't consider the cases where MediatR is used outside of controllers. So maybe the best solution is to throw in ValidationPipelineBehavior as is and explicitly handle ValidationExceptions in ErrorHandlingMiddleware or even a dedicated middleware for this concern. This provides you with the opportunity to handle the ValidationException differently in non-controller situations. However, in such situations, you might not use any validation at all and simply would not provide any validator for this kind of command type.
Btw, MediatR doesn't use PipelineBehaviors when you call Publish() instead of Send(). Just compare NotificationHandlerWrapper and RequestHandlerWrapper.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants