Skip to content

Expose IoC to IPropertyValidationFilter.ShouldValidateEntry for more flexible ModelState validation #26580

Open
@maurei

Description

@maurei

The problem

In the JsonApiDotNetCore framework which I'm maintaining, there is a problem with ModelState validation when using the RequiredAttribute. The framework implements the json:api specification which allows for partial patching, and validation does not work well with that.

Consider the model Article with a to-one relationship to Author, with RequiredAttributes on all properties. Then, for example, in a PATCH /articles request , we require the following behaviour

  • Properties Article.* should ONLY be validated if they are targeted explicitly by the request body
  • Properties Article.Author.* should NEVER be validated because in the json:api spec it is supported to assign relationships up to 1 layer deep, but it is not allowed to simultaneously update properties of that relationship

On the other hand, for a POST /articles request, all Article.* properties should always be validated.

I have tried to implement this behaviour by using IPropertyValidationFilter.ShouldValidateEntry(ValidationEntry entry, ValidationEntry parentEntry) (this is my custom IModelMetadataProvider that does the trick).

Currently IPropertyValidationFilter.ShouldValidateEntry(ValidationEntry entry, ValidationEntry parentEntry) allows me to deduce where I am in the object model graph through the Entry.Key property. With this I can successfully ignore Article.Author.*. But I also need access to HttpContextAccessor to figure out if the request is PATCH/POST. This is currently not possible because the IoC is not passed along.

Additionally I also need access to IoC for other json:api specific metadata about the request scope. Eg. if the endpoint is a ../relationships/... endpoint, in which case validation should never occur, regardless if POST or PATCH.

Solution

I would love to see the IoC being exposed to IPropertyValidationFilter.

Proposed approach:

In IPropertyValidationFilter.cs:

public interface IPropertyValidationFilter
{
    bool ShouldValidateEntry(PropertyValidationFilterContext filterContext);
}

Then, PropertyValidationFilterContext would look something like:

public class PropertyValidationFilterContext
{
    private readonly IServiceProvider _serviceProvider;
    public ValidationEntry Entry { get; }
    public ValidationEntry ParentyEntry { get; }

    public PropertyValidationFilterContext(ValidationEntry entry, ValidationEntry parentyEntry, ActionContext actionContext)
    {
        Entry = entry;
        ParentyEntry = parentyEntry;
        _serviceProvider = actionContext?.HttpContext?.RequestServices;
    }

    public TService GetService<TService>() => _serviceProvider.GetService<TService>();
}

And for ValidationVisitor.VisitChildren(IValidationStrategy):

protected override bool VisitChildren(IValidationStrategy strategy)
{
    var isValid = true;
    var enumerator = strategy.GetChildren(Metadata, Key, Model);
    var parentEntry = new ValidationEntry(Metadata, Key, Model);

    while (enumerator.MoveNext())
    {
        var entry = enumerator.Current;
        var metadata = entry.Metadata;
        var key = entry.Key;

        if (metadata.PropertyValidationFilter?.ShouldValidateEntry(new PropertyValidationFilterContext(entry, parentEntry, Context)) == false)
        {
            SuppressValidation(key);
            continue;
        }

        isValid &= Visit(metadata, key, entry.Model);
    }

    return isValid;
}

Additional context

Currently I can work around this problem by using a custom ValidationVisitor that calls my IPropertyValidationFilter with a reference to IServiceProvider. This is a bit tedious because the only way to have my application use this one instead of the built-in visitor implementation requires me to register a custom ObjectModelValidator. For this implementation I need pretty much everything from DefaultObjectValidator but this type is internal, so I need to copy-paste its internals which I think is not a good thing to do.

If the idea is approved, I would love to make a PR for this myself.

Related issue in JADNC framework: json-api-dotnet/JsonApiDotNetCore#847

Metadata

Metadata

Assignees

No one assigned

    Labels

    affected-fewThis issue impacts only small number of customersarea-mvcIncludes: MVC, Actions and Controllers, Localization, CORS, most templatesenhancementThis issue represents an ask for new feature or an enhancement to an existing onefeature-model-bindingseverity-minorThis label is used by an internal tool

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions