Skip to content

Conversation

icnocop
Copy link
Contributor

@icnocop icnocop commented Jan 19, 2025

Fixes #53

@icnocop icnocop force-pushed the TryValidateModel branch 2 times, most recently from 5654e44 to 945cab4 Compare January 20, 2025 07:07
@icnocop icnocop force-pushed the TryValidateModel branch from 736f137 to 1daba17 Compare March 1, 2025 01:13
this.disableBuiltInModelValidation = disableBuiltInModelValidation;
}

public override bool Validate(ModelMetadata? metadata, string? key, object? model, bool alwaysValidateAtTopLevel)
{
// If built in model validation is disabled return true for later validation in the action filter.
return disableBuiltInModelValidation || base.Validate(metadata, key, model, alwaysValidateAtTopLevel);
bool isBaseValid = disableBuiltInModelValidation || base.Validate(metadata, key, model, alwaysValidateAtTopLevel);
return ValidateAsync(isBaseValid, key, model).GetAwaiter().GetResult();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@icnocop not sure about this one, won't this result in deadlocks?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In ASP.NET Core, it should not cause a deadlock, according to ChatGPT 4.1.

image

In which scenario(s) are you thinking it could cause a deadlock?
Maybe a test can be written to verify it.

@@ -9,19 +9,29 @@ public class FluentValidationAutoValidationObjectModelValidator : ObjectModelVal
{
private readonly bool disableBuiltInModelValidation;

public FluentValidationAutoValidationObjectModelValidator(IModelMetadataProvider modelMetadataProvider, IList<IModelValidatorProvider> validatorProviders, bool disableBuiltInModelValidation)
public FluentValidationAutoValidationObjectModelValidator(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@icnocop keep existing formatting

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in fd34ec6

Thank you.

IModelValidatorProvider validatorProvider,
ValidatorCache validatorCache,
IModelMetadataProvider metadataProvider,
ValidationStateDictionary? validationState)
{
return new FluentValidationAutoValidationValidationVisitor(actionContext, validatorProvider, validatorCache, metadataProvider, validationState, disableBuiltInModelValidation);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@icnocop keep existing formatting

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in fd34ec6

Thank you.

{
public static class FluentValidationHelper
{
public static async Task<ValidationResult?> ValidateWithFluentValidationAsync(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@icnocop this method feels kinda hacky by taking a IServiceProvider as an argument, what do you think?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What else do you recommend?

IServiceProvider is needed to dynamically get the validator based on the type of the model.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TryValidateModel does not work as expected
3 participants