Blazored / FluentValidation

A library for using FluentValidation with Blazor
https://blazored.github.io/FluentValidation/
MIT License
597 stars 86 forks source link

[Bug]: .NET 7 - Passing an instance of IValidator throws exception #182

Open manojdcoder opened 1 year ago

manojdcoder commented 1 year ago

Describe the bug After upgrading to .NET 7 from .NET 6, passing the validator instance directly throws InvalidOperationException but works if we let FluentValidationValidator to create the instance internally by scanning assembly.

To Reproduce

Code:

<EditForm EditContext="@EditContext">
    <FluentValidationValidator DisableAssemblyScanning="true" Validator="@validator" />
    ...
</EditForm>

Exception:

System.InvalidOperationException: Cannot validate instances of type '<>c__DisplayClass79_0`1'. This validator can only validate instances of type 'ModelName'.

Expected behavior No exception is thrown when passing validator.

Hosting Model (is this issue happening with a certain hosting model?):

manojdcoder commented 1 year ago

It seems the issue is from value expression, inputs in my form are generated dynamically so I have something like this

<EditForm Model="person">
    <Blazored.FluentValidation.FluentValidationValidator DisableAssemblyScanning="true" validator="@validator" />
    @{
        var value = person.FirstName; 
    }
    <InputText Value="@value"
        ValueChanged="onValueChanged" 
        ValueExpression="@(() => value)" />
    <ValidationSummary />
</EditForm>

With above code, validation only works if assembly scanning is enabled.

manojdcoder commented 1 year ago

I ended up creating field identifier with property path instead of property name.

e.g If the property being validated is SomeProp.ChildProp, I used to create field identifier as

new FieldIdentifier(Model.SomeProp, 'ChildProp')

Now I do,

new FieldIdentifier(Model, 'SomeProp.ChildProp')
maxence51 commented 1 year ago

I have the same issue

manojdcoder commented 1 year ago

@maxence51 The model on the field identifier and editing context should be same, that fixed the issue for me.

But I also needed to clear validation messages to reset the form so I ended up writing my own fluent validator component.

AlexHall1996 commented 1 year ago

Also get this problem. But doesnt seem to happen if I rely on DI and dont pass in an IValidator explicitly

Kazbek commented 1 year ago

Same error when not using DI and set validator directly to FluentValidator if nested collection use custom validator.

RuleForEach(p => p.Users).SetValidator(new CustomUservalidator());
gcalabro-rli commented 3 months ago

Is there any proper resolution to this?

ngold commented 2 months ago

Same for me. It happens only when using an explicit validator but it does not happen with DI. The problem in my case is that I need to send parameters to my validator, hence, I need to use the explicit version.

Is there any update on this? Any upcoming fix?

ngold commented 2 months ago

I've gone through this issue and this is actually happening because when we validate child objects, this is trying to use the validator we provide to validate the fieldIdentifier.Model, which of course will end up getting the error we can see.

This error will only occur when setting the IValidator manually in the component.

I managed to fix it, but it'd be nice to get a proper fix from Blazored :)

rmunson8 commented 2 months ago

Hi @ngold, was your fix something internal to the package by modifying your clone of the repository or a usage workaround? Thanks.

ngold commented 2 months ago

Hi @rmunson8, I ended up just validating the entire form every time any field changes. It might not be the best approach, but the field-specific validation method isn't working well. When you specify a validator, it tends to fail if you try to validate a child object. Initially, I modified the code to check if fieldIdentifier.Model was different from editContext.Model, and then let it scan for the appropriate validator.

It wasn’t pretty, but it worked. However, I later realized that if we have related properties (for example, one property adding validation to another), the clean-up wouldn't work properly because it only clears the modified property.

It’s all a bit of a mess, so I decided to adapt their code to better suit my needs:

editContext.OnValidationRequested += async (, ) => await ValidateModelAsync(editContext, messages, fluentValidationValidator, cancellationTokenSource, validator);

editContext.OnFieldChanged += async (, ) => await ValidateModelAsync(editContext, messages, fluentValidationValidator, cancellationTokenSource, validator);

rmunson8 commented 2 months ago

I ended up creating field identifier with property path instead of property name.

e.g If the property being validated is SomeProp.ChildProp, I used to create field identifier as

new FieldIdentifier(Model.SomeProp, 'ChildProp')

Now I do,

new FieldIdentifier(Model, 'SomeProp.ChildProp')

Hi @manojdcoder, for this change, where are you applying the new way to create a FieldIdentifier. I'm not seeing code that allows us to set this. Thanks.

ngold commented 2 months ago

@rmunson8 I found out the same.. but the problem starts when you have several levels of sub collections and starting to dynamically build the selector is a chaos (using a recursive method). It was all so complex that I decided to go for full validation instead.

manojdcoder commented 2 months ago

As I mentioned above, I ended up writing my own Validator component with inspirations from this package. I needed some additional features that aren't yet available in this package, so it made sense to just write my own.

Regarding FieldIdentifier, this method creates field identifier internally https://github.com/Blazored/FluentValidation/blob/main/src/Blazored.FluentValidation/EditContextFluentValidationExtensions.cs#L153 This had to match my field identifiers tied to my form editors