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

[Bug] Nested Complex Type Validation Fails #76

Closed
AmirMahdiNassiri opened this issue Jul 20, 2021 · 12 comments · Fixed by #91 · May be fixed by #198
Closed

[Bug] Nested Complex Type Validation Fails #76

AmirMahdiNassiri opened this issue Jul 20, 2021 · 12 comments · Fixed by #91 · May be fixed by #198
Labels
Bug Something isn't working

Comments

@AmirMahdiNassiri
Copy link

Describe the bug

Although FluentValidation supports nested complex types in the model classes (see here), the FluentValidationValidator throws an exception when editing the nested complex type:

System.InvalidOperationException: Cannot validate instances of type 'Address'. This validator can only validate instances of type 'Customer'

Originating from:

Blazored.FluentValidation.EditContextFluentValidationExtensions.ValidateField(EditContext editContext, ValidationMessageStore messages, FieldIdentifier fieldIdentifier, IServiceProvider serviceProvider, Boolean disableAssemblyScanning, IValidator validator)

To Reproduce

Please have a look at the minimal source project I created to reproduce the issue. Please download the repo, build, and execute the sample.

Expected behavior

Nested complex types should be editable and the validation rules should work accordingly.

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

  • Blazor WebAssembly (.Net 5)

Additional context

I guess if the EditContext.Model is passed to the ValidationContext instead of fieldIdentifier.Model, validation should execute successfully. Perhaps such a change in editContext.OnFieldChanged can fix the problem.

Many thanks for the awesome library 👍

@AmirMahdiNassiri AmirMahdiNassiri added Bug Something isn't working Triage Issue needs to be triaged labels Jul 20, 2021
@chrissainty
Copy link
Member

chrissainty commented Jul 23, 2021

Hi @AmirMahdiNassiri, this issue is that you're newing up and passing in the Validator. If you update the FluentValidationValidator component to this:

<Blazored.FluentValidation.FluentValidationValidator />

Removing the Validator parameter, everything works as expected.

What I'm not sure about is where is the problem is. I'm not sure why when Blazored FluentValidation finds your validator it works but when you create it manually it doesn't. Also, the exception is being thrown by FluentValidation itself so that might be something to think about. I'm not sure what to suggest at the moment.

@AmirMahdiNassiri
Copy link
Author

@chrissainty Thanks for the workaround Chris 👍

I tested and realized that if we pass in the nested type to the validator to validate, the exception is thrown by FluentValidation. I think that a validator implemented for a certain type should only receive that specific type as the parameter to validate.

Have a look at this line in the project. This line passes the nested type (fieldIdentifier.Model) to the validator instead of the parent type (accessible by EditContext.Model). Since we already have access to EditContext.Model in the EditContextFluentValidationExtensions.ValidateField function, maybe that would be a potential fix for the issue.

@chrissainty
Copy link
Member

Yep, I see what you're saying. That does seem to be what's causing the issue. That line was based on similar code Microsoft use in the DataAnnotationValidator component. I'm just thinking if there would be any breaking effects of making that change.

@ryanbuening
Copy link

ryanbuening commented Sep 10, 2021

I came across this issue as well and fixed the Cannot validate instances of type... error by removing the Validator parameter.

However, I came across another issue which I'm not sure is related. I'm trying to pass in DateTime.Today as a parameter to my validator:

Foo.razor

<EditForm EditContext="EditContext" OnSubmit="() => Submit()">
	<FluentValidationValidator />
</EditForm>

Foo.razor.cs

public partial class Foo
{
	public FooForm Form { get; }
	public FooForm.Validator Validator { get; }
	public EditContext EditContext { get; }

	public Foo()
	{
		Form = new();
		Validator = new FooForm.Validator(DateTime.Today);
		EditContext = new EditContext(Form);
	}

	public void Submit()
	{
		var isFormValid = EditContext.Validate();
		Console.WriteLine(isFormValid);
	}
}

FooForm.cs

public class FooForm
{
	public List<Contact>? Contacts { get; set; }

	public class Validator : AbstractValidator<FooForm>
	{
		public Validator(DateTime today)
		{
			Console.WriteLine(today.ToString());
			RuleForEach(form => form.Contacts).SetValidator(new ContactValidator());
		}
	}

	public class ContactValidator : AbstractValidator<Contact>
	{
		public ContactValidator()
		{
			RuleFor(contact => contact.PhoneNumber).NotEmpty().WithMessage("Requried");
		}
	}
}

public class Contact
{
	public string PhoneNumber { get; set; } = "";
}

When I submit the form without entering anything in PhoneNumber, isFormValid is logged True (which is incorrect) and an error is thrown:

Error: System.InvalidOperationException: Unable to resolve service for type 'System.DateTime' while attempting to activate 'FooForm+Validator'.
   at Microsoft.Extensions.DependencyInjection.ActivatorUtilities.ConstructorMatcher.CreateInstance(IServiceProvider provider) in Microsoft.Extensions.DependencyInjection.Abstractions.dll:token 0x60000ba+0x5d
   at Microsoft.Extensions.DependencyInjection.ActivatorUtilities.CreateInstance(IServiceProvider provider, Type instanceType, Object[] parameters) in Microsoft.Extensions.DependencyInjection.Abstractions.dll:token 0x600002f+0x9d
   at Blazored.FluentValidation.EditContextFluentValidationExtensions.GetValidatorForModel(IServiceProvider serviceProvider, Object model, Boolean disableAssemblyScanning) in Blazored.FluentValidation.dll:token 0x6000004+0x12d
   at Blazored.FluentValidation.EditContextFluentValidationExtensions.ValidateModel(EditContext editContext, ValidationMessageStore messages, IServiceProvider serviceProvider, Boolean disableAssemblyScanning, FluentValidationValidator fluentValidationValidator, IValidator validator) in Blazored.FluentValidation.dll:token 0x6000002+0x15
   at System.Threading.Tasks.Task.<>c.<ThrowAsync>b__127_0(Object state) in System.Private.CoreLib.dll:token 0x600331c+0x0
   at Microsoft.AspNetCore.Components.Rendering.RendererSynchronizationContext.ExecuteSynchronously(TaskCompletionSource`1 completion, SendOrPostCallback d, Object state) in Microsoft.AspNetCore.Components.dll:token 0x6000338+0x18
   at Microsoft.AspNetCore.Components.Rendering.RendererSynchronizationContext.<>c.<.cctor>b__23_0(Object state) in Microsoft.AspNetCore.Components.dll:token 0x600043e+0x7
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state) in System.Private.CoreLib.dll:token 0x6002ead+0x87

If this is unrelated and a separate issue, let me know and I'll move it.

@CzechsMix
Copy link

How I'm reading the code is this: If you leave the Validator property null, Then the code asks service provider for a validator when it needs one in the case of fields, it determines what kind of validator it needs based on what type of object the bound property belongs to.

So the property belongs to an address, we ask the service provider for an AbstractValidator

and get one, even though to entire form is for a Customer object.

When we provide an AbstractValidator, the code doesn't ask the ServiceProvider for a validator, but the property that triggered the event still belongs to an object of type address, so the passed in AbstractValidator throws an exception when it's asked to validate an Address

I'm not sure I've got it exactly right, but it does seem to explain the issue. I've seen something similar when an input was bound to a component property, which redirected to a property on another object. The validator for the object was provided, but when the field changed, it was the component which was passed into the validator, not the object which actually held the form data.

@chrissainty
Copy link
Member

chrissainty commented Sep 12, 2021

Hi @ryanbuening, I can see a couple of issues with your code design, one of which would remove the problem.

  1. You should never use the constructor of a Razor component, the framework does stuff with it. Generally constructors aren't used in Blazor components. In all my time working with it this is the first code I've seen use one. While initialising properties and fields will work fine, I believe the more Blazor way would be to use OnInitialized instead. This is only called once when the component is initialised and it guarantees all parameters have been initialised and any dependancies injected. So this:
public Foo()
{
	Form = new();
	Validator = new FooForm.Validator(DateTime.Today);
	EditContext = new EditContext(Form);
}

Would be:

protected override void OnInitialized()
{
	Form = new();
	Validator = new FooForm.Validator(DateTime.Today);
	EditContext = new EditContext(Form);
}
  1. I'm not sure why you need to pass in DateTime.Today to the validator? The validator could call this internally as it will always return the same value. So you could change your validator constructor to this and the issue would be resolved.
public Validator()
{
	Console.WriteLine(DateTime.Today.ToString());
	RuleForEach(form => form.Contacts).SetValidator(new ContactValidator());
}

@ryanbuening
Copy link

You should never use the constructor of a Razor component, the framework does stuff with it. Stick to the lifecycle methods.

Good to know! I wasn't able to find any docs on this. Is this outlined somewhere? I made the change of putting the instantiation logic in OnInitialized(), but I still get the same Unable to resolve service for type 'System.DateTime' error when trying to pass DateTime.Today into the validator.

I'm not sure why you need to pass in DateTime.Today to the validator?

I believe passing DateTime.Today into the validator is for making unit testing more reliable, but @CzechsMix can clarify.

@CzechsMix
Copy link

The constructor was used in order to satisfy the compilers nullability checks.

otherwise it's

public EditContext { get; set; } = null!; // I've said it can't be null, but here I am setting it to null to make the compiler happy.
// and now it's possible to throw a null reference exception so I might as well turn nullable references off...

But we can look into other ways to solve this and avoid using the constructor.

DateTime.Today is an Impure call, so to keep the validator pure we provide the current date as a parameter. In this case, it's a really minor risk that a unit test could fail at some point in the future even though nothing changes (a test which makes sure Jan 1 2022 is in the future, ran on jan 2 2022), but it's more about keeping validators and other business logic consistently pure.

icnocop added a commit to icnocop/BlazoredFluentValidation that referenced this issue Oct 3, 2021
…wn by FluentValidation

Thanks to @AmirMahdiNassiri for pointing to the line in the source code with the issue.
Fixes issue Blazored#76
@icnocop
Copy link
Contributor

icnocop commented Oct 3, 2021

Hi.

I created PR #91 to resolve this issue.

Thank you.

@MarvinKlein1508
Copy link

@icnocop thanks! Works perfectly. You've made my day so much better.

@chrissainty chrissainty removed the Triage Issue needs to be triaged label Oct 22, 2021
@ghostinside
Copy link

issue is back in v2.1.0

@icnocop
Copy link
Contributor

icnocop commented Oct 3, 2023

It was reverted in #132 because of #104

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment