Support for IDictionary Validation

[mdloss]

I've got a controller function for putting things that looks like this:

public IActionResult UpdateThings([FromBody]IDictionary<int, ThingDto> thingsDictionary) {
	// Call service...
	return Ok();
}

And another that looks something like this:

public IActionResult CreateThings([FromBody]IEnumerable<ThingDto> thingsList) {
	// Call service...
	return Ok();
}

And, lastly, a validator class defined like:

public class ThingDtoValidator: AbstractValidator<ThingDto> {
		public ThingDtoValidator() {
			RuleFor(t => t.Name).NotNull();
		}
	}
}

The validation is only invoked in the second case, when I pass a list, and not for the dictionary.

I thought of defining another class:

public class ThingDtoDictionaryValidator: AbstractValidator<IDictionary<int, ThingDto>> {
	...
}

But that seems like a lot of overhead just for dictionary support. Is there a better to achieve support for the dictionary?

[JeremySkinner]

That's not something that's supported at the moment, and will unfortunately be fairly difficult to add because of the way the MVC validation pipeline works. I'll do some investigation to see what will be involved to add this.

[mdloss]

Thank you for getting back so quickly with a response. I'm on .net core if you can think of solution that circumvents MVC.

[JeremySkinner]

The validation pipleline in .net core is very different to mvc5 (where I believe this works), and it's going to take a lot of work and restructuring to get something like this in place. If you want a quick solution then if possible you'd be better off refactoring to remove the reliance on a dictionary parameter, or just ditch the automatic FluentValidation integration entirely and manually invoke the validator against each object in the collection. As I mentioned, I'll look at trying to add this but can't promise anything, and it won't be for a while I'm afraid.

[wizofaus]

I'm trying to understand this (mainly for my own edification!), and it seems that currently all your Mvc tests use post via FormUrlEncodedContent - but is there any reason not to at least have some that use application/json (or even xml)? Actually until I just looked it up I didn't know it was possible to url-encode a form that represented a list! (apparently the syntax is "[0].property=value1&[1].property=value2&...").

[wizofaus]

Anyway I found the problem:

public class FluentValidationObjectModelValidator {
...
private IValidator BuildCollectionValidator(string prefix, ModelMetadata collectionMetadata, IValidatorFactory validatorFactory) {
var elementValidator = validatorFactory.GetValidator(collectionMetadata.ElementType);

But the ElementType for dictionaries is KeyValuePair<K, V> which itself doesn't have a validator.
But it looks like if I'm able to create a KeyValuePairAdaptor that can delegate to a validator for the value type (I suppose we could support the key too, but I'm not sure how we'd combine the results) then the existing MvcCollectionValidator should work for dictionaries.

[wizofaus]

Alright, got this working...added
class KeyValuePairValidatorAdaptor<K,V> : IValidator<KeyValuePair<K, V>>
that stores the "value" validator, and proxies all calls to that, then changed BuildCollectionValidator to do:

		if (elementValidator == null)
		{
			if (elementType.GetGenericTypeDefinition() != typeof(KeyValuePair<,>))
				return null;
			Type[] types = collectionMetadata.ElementType.GetGenericArguments();
			var valueValidator = validatorFactory.GetValidator(types[1]);
			if (valueValidator == null)
				return null;
			Type validatorType = typeof(KeyValuePairValidatorAdaptor<,>).MakeGenericType(types);
			elementValidator = (IValidator) Activator.CreateInstance(validatorType, valueValidator, prefix);
		}

Seems to work pretty well, even if there are questions over details of the results/messages returned.
But I do wonder why the collection validator support is only available from FluentValidationObjectModelValidator, which is designed to be used with Mvc model validation. Surely the ability to perform validation on a collection is generically useful, whether or not you're using MVC?

Something else that occurred to me - it would seem logically that there would be a good case for a generic memberwise property validator, that could use reflection to handle any type (including KeyValuePair<,>) and individually validate the properties. Because as I understand it currently, if I have

[Validator(typeof(CreditCardDetailsValidator))]
class CreditCardDetails {
  string Number;
  Date ExpiryDate;
}

etc. with a suitable CreditCardDetailsValidator defined, but then tried to pass an object to an HTTP endpoint exposed via MVC like:

class ShoppingCartOrder {
   public CreditCardDetails cardDetails { get; set; }
   public IEnumerable<InvoiceItem> items { get; set; }
}

then currently it's necessary to write an explicit validator for ShoppingCartOrder in order for the cardDetails to get validated.
But if it's not necessary when the argument is IEnumerable, then why should be it necessary for ShoppingCartOrder?
If such a validator did exist I could imagine the function for determining a validator for a given type would simply be:

IValidator GetValidator(Type type) {
    var validator = GetExplicitValidator(type);
    if (validator != null) return validator;
    if (type.IsCollectionType) return BuildCollectionValidator(type); // would use GetValidator(type.ElementType)
    return TryBuildMemberwisePropertyValidator(type);
}

Whereby TryBuildMemberwisePropertyValidator(type) would use reflection to walk through all properties of the type, and if a validator existed for any of those, construct the memberwise property validator, but if not, return null.

(I note currently that the existing BuildCollectionValidator function can't handle collections of collections etc. etc.)

Again, no reason this functionality need be Mvc-specific...

[JeremySkinner]

Yes, that approach would work. Essentially it has the same effect that @cklog suggested of explicitly having an AbstractValidator<IDictionary<K, V>>, but pushes the responsibility of creating this into the ObjectModelValidator rather than having the consumer build the validator definition.

That being said, I'm in the process of rewriting the MVC validation implementation for .NET core to work in a similar way to how the MVC 5 integration worked. This will make use of the ModelValidatorProvider api which is recursive - meaning there's no need to explicitly handle cases like these. When using the ModelValidatorProvider, MVC's ValidationVisitor handles the recursive nature of binding the results and finding the correct validator automatically, as well as handling implicit property validation. Going forward MVC will do all the heavy lifting here and is very reliable.

I have a prototype here: https://github.com/JeremySkinner/FluentValidation/tree/mvcvalidation2

There are lots of problems with this approach which is why I didn't do this originally...the ModelValidatorProvider API is not designed for use with a library like FluentValidation (which validates all properties on the object in one go after it's been set). It's designed primarily to work for DataAnnotations which validates each value in turn before the property is set. It took a while but I've managed to workaround the issues.

But if it's not necessary when the argument is IEnumerable, then why should be it necessary for ShoppingCartOrder?

This is handled in the prototype. It'll be an opt-in setting that you have to turn on. By default, complex child properties will only be validated if you explicitly set up a validator for them using SetValidator, but you can turn on support for validating properties implicitly.

(I note currently that the existing BuildCollectionValidator function can't handle collections of collections etc. etc.)

Correct, this was a "simple workaround". The prototype makes use of MVC's ValidationVisitor to handle this recursively.

Surely the ability to perform validation on a collection is generically useful, whether or not you're using MVC?

Possibly, but typically outside of MVC there isn't a need for this. The usual approach would be to just loop over the collection and call Validate on each item in the collection. In MVC this is an issue as you have no control of when validation occurs - by the time your controller action executes validation has already taken place. Outside of MVC this isn't an issue, you just call Validate explicitly.

Again, no reason this functionality need be Mvc-specific...

Outside of MVC, you usually wouldn't need this kind of recursive way of locating validators - you'd just use SetValidator or SetCollectionValidator inside your validator definitions to handle validation of child properties.

For now, we'll focus on getting this working well inside MVC using the ModelValidatorProvider api. It's worth having the discussion about whether we want to do something like this outside MVC, but I'd like to park that for now and we can re-visit it later.

[wizofaus]

If you have a more complete solution in the works then I'll just park what was I doing here, like I said, was more curiosity and a chance to better understood how various things worked than anything.
Though I'd say you could use more MVC tests - currently there are no tests that pass an IEnumerable< > directly, and as I said, none that use JSON. While obviously it would be nice to think that the serialization format (whether url-encoded, JSON or xml) had no impact on how the validators worked, I wouldn't be so completely sure. Anyway I did write a few if you're interested (new TestController endpoints, tests to call them, and an enhancement to WebAppFixture to call using JSON. The advantage of the latter is that the JSON stringify can work directly on the object you want to pass, rather than needing a dictionary).

Your points about working outside of MVC are true enough, though as I see it the whole advantage of your library is that with very minimal code you can take any object and by defining validators only where necessary/meaningful, still get highly detailed and useful information about whether your model is valid (regardless of where it came from). In fact you could even do without the annotation checks - if you're already using reflection then it's not hard to scan for classes that implement IValidator for any object of type X and use them automatically. If performance was really an issue you could go one step further and write a compile time tool to automatically wire everything together...

[JeremySkinner]

Though I'd say you could use more MVC tests

Agreed

none that use JSON.

Yes, these tests have been ported from one version of MVC to the next over the years. I believe I originally wrote them way back for MVC2. The standard mechanism for representing a form POST was to use the FormCollection class (which was basically just a wrapper around a dictionary). AspNetCore doesn't have FormCollection, so the quickest/easiest solution for porting the tests was just to essentially re-create FormCollection as it made porting the tests easier (developing the AspNetCore integration was a very painful process, so anywhere I could minimize the pain I did!).

As you say, in theory it shouldn't make a difference what format the input takes....MVC should be responsible for ironing out any differences and present a unified model ready for FV to validate. In reality though... ¯\_(ツ)_/¯ I remember in WebApi1 (and possibly 2) there were definitely issues where the formatters behaved inconsistently, but I think those have been fixed in Core.

Anyway I did write a few if you're interested (new TestController endpoints, tests to call them, and an enhancement to WebAppFixture to call using JSON

Would you be up for submitting those as a pull request to the "mvcvalidation2" branch, or will that be too much work?

And thanks again for all your contributions recently....I really appreciate it.

[wizofaus]

I can just attach the code snippets, there's really not much too them.
The tests really should do more to verify the result messages - e.g. for dictionaries I'd expect to get something like
[key].CreditCardNumber must be less than 17 characters

where 'key' was the actual dictionary key used (with my quick-n-dirty solution it's the numerical index into the enumeration of key-pairs).

[JeremySkinner]

Yes if you want to attach the snippets I'll get them added in. I should hopefully have the new mvc validation merged in later this week.

[JeremySkinner]

7.3 beta 1 adds better support for collections (including dictionaries). Please give it a try.