dotnet / dotnet/aspnetcore

Expose IoC to IPropertyValidationFilter.ShouldValidateEntry for more flexible ModelState validation

Open
#26,580 3 comments 0 reactions 0 assignees View on GitHub
affected-few area-mvc enhancement feature-model-binding severity-minor
Dominant language
C#
Stars
38.4k
Forks
10.9k
Avg merge
2d 10h
Merged PRs (30d)
281

Description

## The problem
In the [JsonApiDotNetCore framework](https://github.com/json-api-dotnet/JsonApiDotNetCore) which I'm maintaining, there is a problem with ModelState validation when using the [RequiredAttribute](https://docs.microsoft.com/en-us/dotnet/api/system.componentmodel.dataannotations.requiredattribute?view=netcore-3.1). The framework implements the [json:api specification](https://jsonapi.org/format/#crud-updating-resource-attributes) 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](https://jsonapi.org/format/#crud-updating-resource-relationships) 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)](https://github.com/dotnet/aspnetcore/blob/master/src/Mvc/Mvc.Abstractions/src/ModelBinding/Validation/IPropertyValidationFilter.cs) (this is my [custom IModelMetadataProvider](https://github.com/json-api-dotnet/JsonApiDotNetCore/blob/feat/834/src/JsonApiDotNetCore/Configuration/JsonApiMetaDataProvider.cs) that does the trick).

Currently [IPropertyValidationFilter.ShouldValidateEntry(ValidationEntry entry, ValidationEntry parentEntry)](https://github.com/dotnet/aspnetcore/blob/master/src/Mvc/Mvc.Abstractions/src/ModelBinding/Validation/IPropertyValidationFilter.cs) allows me to deduce where I am in the object model graph through the [Entry.Key](https://github.com/dotnet/aspnetcore/blob/ca1f9f532f540f16732e8d8b4990fbae1578786b/src/Mvc/Mvc.Abstractions/src/ModelBinding/Validation/ValidationEntry.cs#L72) 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](https://jsonapi.org/format/#crud-updating-to-one-relationships), 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](https://github.com/dotnet/aspnetcore/blob/master/src/Mvc/Mvc.Abstractions/src/ModelBinding/Validation/IPropertyValidationFilter.cs):
```c#
public interface IPropertyValidationFilter
{
bool ShouldValidateEntry(PropertyValidationFilterContext filterContext);
}
```

Then, `PropertyValidationFilterContext` would look something like:
```c#
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() => _serviceProvider.GetService();
}
```

And for [ValidationVisitor.VisitChildren(IValidationStrategy)](https://github.com/dotnet/aspnetcore/blob/ca1f9f532f540f16732e8d8b4990fbae1578786b/src/Mvc/Mvc.Core/src/ModelBinding/Validation/ValidationVisitor.cs#L453):
```c#
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](https://github.com/json-api-dotnet/JsonApiDotNetCore/blob/feat/834/src/JsonApiDotNetCore/Configuration/JsonApiValidationVisitor.cs) 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](https://github.com/json-api-dotnet/JsonApiDotNetCore/blob/feat/834/src/JsonApiDotNetCore/Configuration/JsonApiObjectValidator.cs). For this implementation I need pretty much everything from [DefaultObjectValidator](https://github.com/dotnet/aspnetcore/blob/v3.1.8/src/Mvc/Mvc.Core/src/ModelBinding/Validation/DefaultObjectValidator.cs) 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: https://github.com/json-api-dotnet/JsonApiDotNetCore/pull/847

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.