InvalidCastException when using ForEach rule

[ArnaudB88]

FluentValidation version

10.0.4

ASP.NET version

.NET Core 3.1

Summary

I have a rulebuilder for a IList typed property with three rules: notEmpty, foreach and mustAsync. The problem is that the foreach rule returns a rulebuilder with out type 'IEnumerable' but the next rule seems to expect the original property type IList since there is an invalid cast exception from ...IList.. to ..IEnumerable..

This rule did not give any exception in FluentValidation version 9.3.0.

Steps to Reproduce

The code
Dto to validate

    public class SaveRegistrationRequestForHardwareUnit : CommandWithTransactionToggle<SuccessOrFailureDto>
    {
        //other props
        public IList<KeyValuePair<Guid,string>> HardwareUnitRoles { get; set; }
    }

Validator

public class SaveRegistrationRequestForHardwareUnitValidator : AbstractValidator<SaveRegistrationRequestForHardwareUnit>
{
    private const string ValidRoleConfigMessage = "ValidRoleConfigMessage";

    public SaveRegistrationRequestForHardwareUnitValidator(IValidator<UserDto> userDtoValidator)
    {
        //Other rules

        RuleFor(r => r.HardwareUnitRoles).Cascade(CascadeMode.Stop)
            .NotEmpty()
            .ForEach(r => r
                .Must(kv => HardwareUnitUserRoles.IsValid(kv.Value))
                    .WithMessage(m => Resources.ValidationMessages.hardwareunit_role_notvalid)
                .Must(kv => kv.Value != HardwareUnitUserRoles.Owner)
                    .WithMessage(m => Resources.ValidationMessages.registrationrequest_hardwareUnitRole_notallowed)
            )
            .MustAsync((roles, token) => ValidRoleConfigAsync(roles))//GIVES EXCEPTION
                .WithMessage(dto => ValidationErrorMessages.GetErrorMessage(ValidRoleConfigMessage));
    }

    private async Task<bool> ValidRoleConfigAsync(IEnumerable<KeyValuePair<Guid, string>> hardwareUnitRoles)
    {
        //logic
    }
}

Exception:

System.InvalidCastException: Unable to cast object
of type 'FluentValidation.Internal.RuleBuilder`2[Foo.Commands.Registration.SaveRegistrationRequestForHardwareUnit,System.Collections.Generic.IList`1[System.Collections.Generic.KeyValuePair`2[System.Guid,System.String]]]'
to type 'FluentValidation.Internal.RuleBuilder`2[Foo.Commands.Registration.SaveRegistrationRequestForHardwareUnit,System.Collections.Generic.IEnumerable`1[System.Collections.Generic.KeyValuePair`2[System.Guid,System.String]]]'.

[JeremySkinner]

Hi, thanks for reporting this, yes that's a bug. At the moment I don't see a good way of fixing this, so for now I'd suggest implementing a workaround and using an InlineValidator to replace the call to ForEach:

public SaveRegistrationRequestForHardwareUnitValidator(IValidator<UserDto> userDtoValidator)
{
  //Other rules

  var unitRoleCollectionValidator = new InlineValidator<IList<KeyValuePair<Guid,string>>>();
  unitRoleCollectionValidator.RuleFor(r => r) 
      .Must(kv => HardwareUnitUserRoles.IsValid(kv.Value))
      .WithMessage(m => Resources.ValidationMessages.hardwareunit_role_notvalid)
      .Must(kv => kv.Value != HardwareUnitUserRoles.Owner)
      .WithMessage(m => Resources.ValidationMessages.registrationrequest_hardwareUnitRole_notallowed)

  RuleFor(r => r.HardwareUnitRoles).Cascade(CascadeMode.Stop)
      .NotEmpty()
      .SetValidator(unitRoleCollectionValidator)
      .MustAsync((roles, token) => ValidRoleConfigAsync(roles))
      .WithMessage(dto => ValidationErrorMessages.GetErrorMessage(ValidRoleConfigMessage));
}

(this is essentially what the ForEach method does internally, but corrects the type mismatch).

I'll post an update here if/when I find a way to fix this, but this should allow you to carry on with upgrading for now.

[ArnaudB88]

Thanks for the feedback Jeremy.
I currently made a workaround by moving the last rule to a dependent rule.

Looking forward to the fix.
Also a big thank you for this great library. We use it for most projects in our company.

[JeremySkinner]

There isn't really any way to fix this now that the internal model is strongly typed (in 9.x and older the internal model wasn't generic, so the return types could be flexible like this). Now, the type returned by each part of the rule chain must be consistent. I think the only options are:

• Deprecate RuleFor().ForEach and require users to either use RuleForEach or RuleFor().SetValidator()
• Make RuleFor().ForEach terminate the rule chain so nothing can be subsequently applied

[ArnaudB88]

Some suggestions

1. make the list type generic. Downside: explicit typing is needed when calling the method:

public static IRuleBuilderOptions<T, TIEnumerable> ForEach<T, TIEnumerable, TElement>(this IRuleBuilder<T, TIEnumerable> ruleBuilder,
    Action<IRuleBuilderInitialCollection<TIEnumerable, TElement>> action) where TIEnumerable : IEnumerable<TElement>
{
    var innerValidator = new InlineValidator<TIEnumerable>();
    action(innerValidator.RuleForEach(x => x));
    return ruleBuilder.SetValidator(innerValidator);
}

2. Add overload method for other list types like IList.

public static IRuleBuilderOptions<T, IList<TElement>> ForEach<T, TElement>(this IRuleBuilder<T, IList<TElement>> ruleBuilder,
    Action<IRuleBuilderInitialCollection<IList<TElement>, TElement>> action)
{
    var innerValidator = new InlineValidator<IList<TElement>>();
    action(innerValidator.RuleForEach(x => x));
    return ruleBuilder.SetValidator(innerValidator);
}

Maybe a combination can be the key, so you only have one (private) implementation like suggestion 1 and many (public) overloads like suggestion 2.

[JeremySkinner]

Unfortunately neither of these options will work properly- I’ve explored both in the past. Option 1 breaks compiler type inference, option 2 has the same problem as the original impl (variance will allow these overloads to be invoked on properties that are of a more derived type, leading to an invalid cast when accessing the internal api. It isn’t possible to cover every possible collection type)

[JeremySkinner]

I think the way to support this properly would be extend the use of variance throughout the internal model rather than limiting it to the fluent API, but this would require several breaking changes so would have to wait for a major release.

I think I'll leave ForEach as it is now but keep this issue open and see if I can get the variance changes in place for 11.0

[JeremySkinner]

I put together a quick prototype in #1713. Will need to think about whether the breaking changes this introduces will be problematic.

[JeremySkinner]

I've committed a change in 6e825d3 which will allow this to work in 10.x without needing to introduce breaking changes (adds an additional interface, which can be removed again in v11). Well, technically there is one breaking change to the return type of one undocumented public method (DefaultValidatorOptions.Configurable), but I think this is worth it to get the fix in place. I'll try and get this pushed out to nuget over the weekend.

With this fix, the underlying problem will still be present if the user calls the Configure method following a call to ForEach, but for all other scenarios this will fix the issue. The use of Configure will need to wait until 11.

I'll let you know once this is pushed to nuget so you can give it a try.

[JeremySkinner]

I've published the 10.1 release to nuget with this fix.

[ArnaudB88]

Updated our solution with the 10.1 version, set original validation code active again: all unit tests pass!
Thanks for the fast fix!