dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.23k stars 9.95k forks source link

ModelState.AddModelError problemdetails resulting object doesn't match that of model validation #17856

Open bbarry opened 4 years ago

bbarry commented 4 years ago

Given a fairly simple mvc action such as:

[ApiController]
public class MyController : ControllerBase
{
    [HttpPost]
    public IActionResult Foo([FromBody] Command request)
    {
        ModelState.AddModelError<Command>(a => a.Bar, "Invalid.");
        return ValidationProblem();
    }
}
public class Command
{
    [BindProperty(Name = "bar")]
    [BindRequired]
    [Required]
    public string Bar { get; set; }
}

Submitting a post to the action which would trigger a model validation error, such as posting:

{
  "bar": ""
}

presents a ProblemDetails response:

{
  "errors": {
    "bar": [
      "The Bar field is required."
    ]
  },
...
}

But any validations which do not flow through the built in validation framework are unable to utilize the logic for naming the error properties. They instead result in:

{
  "errors": {
    "Bar": [
      "Invalid."
    ]
  },
...
}
mkArtakMSFT commented 4 years ago

Thanks for contacting us. This is not an issue we've heard much about so we're going to park it in our backlog to accumulate more feedback about. We will review it during our next release planning and act accordingly.

rynowak commented 4 years ago

This happens because the errors in model state produced by validation reflect the shape of the input. Your invalid JSON input looked like: {... "bar": "invalid", ... }.

This is annoying, but there's no solution to it.

bbarry commented 4 years ago

As far as I can tell it is a difference in the generation of the model state dictionary key in validation vs the AddModelError extension method that accepts a lambda expression. The validation logic utilizes the shape of the input and the extension method simply serializes the expression tree.

I'm not really sure what the solution is, but I believe I can clearly articulate the problem:

As an api provider I have a need to present my clients with a mechanism for discovering which segments of their inputs are causing problems in my call stack. This mechanism must be javascript tooling friendly because my users are writing web applications that call my api. It must enable those users to display error messages related to input data when their users perform operations on those sites.

I am unable to find any api in asp.net core that enables me to provide the same logic for determining the key for an error message in an automatic model validation error as in a business logic validation error directly attributable to model inputs.

rynowak commented 4 years ago

I am unable to find any api in asp.net core that enables me to provide the same logic for determining the key for an error message in an automatic model validation error as in a business logic validation error directly attributable to model inputs.

I agree, we do not have this.

ghost commented 2 years ago

Thanks for contacting us. We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. Because it's not immediately obvious that this is a bug in our framework, we would like to keep this around to collect more feedback, which can later help us determine the impact of it. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

captainsafia commented 2 years ago

Putting this back into .NET 7 milestone for now.

We'll want to add a new AddModelError overload to ModelStateDictionaryExtensions that takes (perhaps) a delegate that gives users access to the ModelStateDictionary for resolving the keys with an attribute.

ghost commented 1 year ago

Thanks for contacting us. We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. Because it's not immediately obvious that this is a bug in our framework, we would like to keep this around to collect more feedback, which can later help us determine the impact of it. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

AnthonyMastrean commented 5 months ago

We're using the IValidationMetadataProvider to enforce the required JSON naming strategy when the framework-provided binding/validation is executed.

builder.Services.AddControllers(options =>
{
    options.ModelMetadataDetailsProviders.Add(new SystemTextJsonValidationMetadataProvider());
});

https://learn.microsoft.com/en-us/aspnet/core/mvc/models/validation?view=aspnetcore-8.0#use-json-property-names-in-validation-errors

But that doesn't apply to ModelState errors added in the body of the endpoint.

The SystemTextJsonValidationMetadataProvider provides a ValidationMetadataName metadata that is used during the validation process and not during the ProblemDetails serialization.

https://github.com/dotnet/aspnetcore/issues/43524#issuecomment-1227541995

Is there any way to make that happen? Or some way we can use that provider in the endpoint to ensure those keys are properly transformed?