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

Implicit validation of root collection models #1585

Closed
notnotchris opened this issue Nov 18, 2020 · 3 comments · Fixed by #1586
Closed

Implicit validation of root collection models #1585

notnotchris opened this issue Nov 18, 2020 · 3 comments · Fixed by #1586
Milestone

Comments

@notnotchris
Copy link
Contributor

notnotchris commented Nov 18, 2020

Is your feature request related to a problem? Please describe.
I would like to be able to validate the elements of a root model that is a collection type without enabling implicit validation of child properties or having to define an explicit collection validator.

Example model and controller action method:

public class Model { 
    public string Name { get; set; }
}

public class ModelValidator : AbstractValidator<Model> {
    public ModelValidator() => RuleFor(m => m.Name).NotNull().WithMessage("NotNull");
}

[ApiController]
public class TestController : ControllerBase {
    [HttpPost]
    public IActionResult ControllerValidation(IEnumerable<Model> model) => Ok();
}

Given the model and controller above and the equivalent rule implemented using a [Required("NotNull")] data model annotation, the errors returned by the default model validation framework would be of the following format.

Example (partial) error response:

{ "[0].Name": ["NotNull"] }

While it is possible to produce the same response by enabling implicit validation of child properties, it obviously has the side effect that any child properties of Model will also be automatically validated – however, I'd like to control this validation explicitly.

I can define an explicit collection validator like so:

public class ListOfModelValidator : AbstractValidator<List<Model>> {
    public ListOfModelValidator() => RuleForEach(model => model).SetValidator(new ModelValidator());
}

However, this has the side effect that the response will now be as below, where the lambda parameter name is used as the property name.

{ "model[0].Name": ["NotNull"] }

Of course, WithName can be used to override the property name but not to set it to a null or an empty string. I think this behaviour makes sense when using RuleForEach for a child property but I'm not sure it does when it is the root model that is the collection. Equally, while [0].Name may not seem like an ideal naming convention, it matches the ASP.NET Core default.

Describe the solution you'd like
One option might be to introduce a ImplicitlyValidateRootCollectionElements property to FluentValidationMvcConfiguration. It would default to false but when set to true validation of collection elements would occur the same way it does when ImplicitlyValidateChildProperties is set to true. If both ImplicitlyValidateRootCollectionElements and ImplicitlyValidateChildProperties are set to true then ImplicitlyValidateRootCollectionElements would simply be ignored and the behaviour of ImplicitlyValidateChildProperties would be as it is now.

Describe alternatives you've considered

  1. Rewrite the property name after validation by overriding Validate or by implementing IValidatorInterceptor. However, using either of those options to rewrite the property name after validation just feels wrong.
  2. Redesign the model and/or response (by using WithName perhaps). However, this may not be an option where an API is public and/or already in production.

Additional context
This may be a bit of an edge case but I think it may be useful for developers inheriting an already published endpoint and/or converting annotation-based validation to FluentValidation (as is my case).

I'd be happy to contribute a PR (either for what I've suggested above or an alternative) if the suggestion fits with the existing design and roadmap.

@JeremySkinner
Copy link
Member

Hi, thanks for the suggestion.

Usually I'd suggest rewriting the property name after validation occurs as you've described (either overriding validate, or I expect you could use an action filter too), but I like your idea of a ImplicitlyValidateRootCollectionElements property, this feels like a good solution without having to enable full implicit validation.

If you'd like to put together a PR that puts together an initial implementation then I'd be happy to review that. It'll need to include tests too- there are some helpers in the test project for spinning up different web applications with different configuration, let me know if there's anything in there that you'd like me to explain.

Thanks for wanting to be involved!

@notnotchris
Copy link
Contributor Author

Thanks for your quick response @JeremySkinner. I'll put together a PR – hopefully I might have one ready later today.

@JeremySkinner
Copy link
Member

This is implemented and part of the 9.4 release

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants