Design validation for Minimal APIs

[halter73]

Idea:

// Step 5: Input validation
app.MapPost("/todos", async (Validated<Todo> inputTodo, TodoDb db) =>
{
    var (todo, isValid) = inputTodo;
    if (!isValid) return Problem(inputTodo);

    db.Todos.Add(todo);
    await db.SaveChangesAsync();

    return CreatedAt(routes.GetTodo.Url(todo.Id), todo);
};

From: #30580

[ghost]

Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

[ghost]

Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

[jchannon]

Can I propose instead of Validated<ToDo> we have ValidationResult<ToDo>, you can then either use the deconstructor like you show:

var (todo, isValid) = inputTodo;
if (!isValid) return Problem(inputTodo);

db.Todos.Add(todo);

or use something like this where you have explicit properties on the type:

if (!inputTodo.IsValid) return Problem(inputTodo.Data);
db.Todos.Add(inputTodo.Data);

[ghost]

Thanks for contacting us.

We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. 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.

[DamianEdwards]

New idea for this that effectively uses the existing validation feature in System.ComponentModel but creates a thin static wrapper for the call-site and result type. The app calls the validate method rather than the framework doing it for you.

// Taken from https://github.com/DamianEdwards/MinimalApiPlayground
// In Program.cs
using static ResultHelpers;
using static ValidationHelpers;

// startup code...

app.MapPost("/todos", async (Todo todo, TodoDb db) =>
{
    if (!TryValidate(todo, out var errors)) return BadRequest(errors);

    db.Todos.Add(todo);
    await db.SaveChangesAsync();

    return CreatedAt($"/todos/{todo.Id}", todo);
});

// In ASP.NET Core somewhere, named better
static class ResultHelpers
{
    public static IResult BadRequest(IDictionary<string, string[]> errors)
    {
        var problem = new ValidationProblemDetails(errors)
        {
            Status = StatusCodes.Status400BadRequest
        };
        return new ProblemDetailsResult(problem);
    }
}

class ProblemDetailsResult : IResult
{
    private readonly ProblemDetails _problem;

    public ProblemDetailsResult(ProblemDetails problem)
    {
        _problem = problem ?? new ProblemDetails { Status = StatusCodes.Status400BadRequest };
    }

    public async Task ExecuteAsync(HttpContext httpContext)
    {
        if (_problem.Status.HasValue)
        {
            httpContext.Response.StatusCode = _problem.Status.Value;
        }
        await httpContext.Response.WriteAsJsonAsync(_problem, _problem.GetType());
    }
}

static class ValidationHelpers
{
    public static bool TryValidate<T>(T target, out IDictionary<string, string[]> errors) where T : class
    {
        // TODO: Make recursive

        if (target == null)
        {
            throw new ArgumentNullException(nameof(target));
        }

        var validationContext = new ValidationContext(target);
        var validationResults = new List<ValidationResult>();

        var isValid = Validator.TryValidateObject(target, validationContext, validationResults);

        var errorsList = new Dictionary<string, List<string>>();
        foreach (var result in validationResults)
        {
            foreach (var name in result.MemberNames)
            {
                List<string> fieldErrors;
                if (errorsList.ContainsKey(name))
                {
                    fieldErrors = errorsList[name];
                }
                else
                {
                    fieldErrors = new List<string>();
                    errorsList.Add(name, fieldErrors);
                }
                fieldErrors.Add(result.ErrorMessage);
            }
        }

        errors = new Dictionary<string, string[]>(errorsList.Count);
        foreach (var error in errorsList)
        {
            errors.Add(error.Key, error.Value.ToArray());
        }

        return isValid;
    }
}

[ghost]

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

[DamianEdwards]

I implemented my suggestion above in a library called MinimalValidation so that the idea can be explored in a flexible fashion outside the timelines of .NET 6.

Looks like this in a minimal APIs context:

using System.ComponentModel.DataAnnotations;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Hosting;

var builder = WebApplication.CreateBuilder(args);
var app = builder.Build();

if (app.Environment.IsDevelopment())
{
    app.UseDeveloperExceptionPage();
}

app.MapGet("/", () => "Hello World");

app.MapGet("/widgets", () =>
    new[] {
        new Widget { Name = "Shinerizer" },
        new Widget { Name = "Sparklizer" }
    });

app.MapGet("/widgets/{name}", (string name) =>
    new Widget { Name = name });

app.MapPost("/widgets", (Widget widget) =>
    !MinimalValidation.TryValidate(widget, out var errors)
        ? Results.BadRequest(errors)
        : Results.Created($"/widgets/{widget.Name}", widget));

app.Run();

class Widget
{
    [Required, MinLength(3)]
    public string? Name { get; set; }

    public override string? ToString() => Name;
}

[DamianEdwards]

Doing some "customer research" https://twitter.com/DamianEdwards/status/1486871181252833284

Result:

[mottaghipour]

If we have Data Annotations Extensions(#41836), we can implement some extensions for minimal Apis like:

public static class MinimalValidationExtensions
{
    public static bool ValidateAsync(this HttpContext context, object model) { ... }
}

Minimal Apis:

app.MapPost("/todos", async (Todo todo, HttpContext context) =>
{
    var isValid = await context.ValidateAsync(todo);
    if (isValid)
    {
        // do something with the todo
    }
});

If model is not valid, extension send bad request then closed request and otherwise continue tasks.

bad request have response like ApiController Validation result:

{
    ...
    "meesages" : [  
        {
            "name" : "DisplayName",
            "message" : "Message",
        },
    ],
    ...
}

@halter73 @davidfowl

[halter73]

@mottaghipour Is the idea that ValidateAsync would write a 4xx ProblemDetails response when it returns false? Is that why it needs the HttpContext?

Have you looked at MiniValidation library created by @DamianEdwards mentioned earlier in the issue? I like how the design of MiniValidation makes it usable in non-web contexts when there's no HttpContext available.

if (!MiniValidator.TryValidate(todo, out var errors)) return Results.ValidationProblem(errors);

It's also nice not to have to add an HttpContext parameter to the route handler and to not introduce async (which has some perf impact) to route handlers that don't need them.

We may not ship a whole lot in the framework for minimal validation in .NET 7. For things like enforcing System.ComponentModel annotations, that can already be done in a third-party library like MiniValidation. Even your proposed ValidateAsync method looks like it could easily be added via a third-party NuGet package.

If we get feedback that something based on System.ComponentModel like MiniValidation or ValidateAsync fully meets peoples' needs and fills a gap, we might include something like it in the framework. Also, if there was some pluggable design that needs the framework to add API to hook into, that might compel us to put something in the framework as opposed to just recommending a package.

[davidfowl]

I think we should close this issue. If we end up doing anything here for recursive validation, it shouldn't be part of ASP.NET Core (dotnet/runtime#69834). Folks tend to use something like minivalidation, fluent validation or another validation library. The only reason to have anything baked in at the ASP.NET Core level is if we decide to have another abstraction (like MVC does today) and I think we should avoid that.

[mottaghipour]

@halter73 I think I should reconsider. Users do not want to have error messages, They just want to make sure objects is valid, If it was invalid, the framework returned error messages.

I don't like MiniValidation package because it makes the methods not clean And I also agree that the existence of HttpContext in the way I suggested is not interesting at all.

[davidfowl]

@mottaghipour Not sure what you mean.

I don't like MinimalValidation package because it makes the methods not clean And I also agree that the existence of HttpContext in the way I suggested is not interesting at all.

What doesn't MiniValidation solve?

[mottaghipour]

MiniValidation is not good because I want clean code but we have this:

app.MapPost("/widgets", (Widget widget) =>
    !MiniValidator.TryValidate(widget, out var errors)
        ? Results.ValidationProblem(errors)
        : Results.Created($"/widgets/{widget.Name}", widget));

I think MinimalApis should have simpler validation.

[mottaghipour]

.NET have ApiController attr and All models validate before process actions.
Why in that section everything is related to the framework, but now you are talking about the external package؟

[davidfowl]

Because the performance is not ideal. Minimal APIs are... minimal, behavior needs to be opted into. That's why it won't be on by default:

• We don't want to add a validation abstraction.
• We don't want add the overhead of validation per parameter by default. It needs to be opted into.

Model binding, validation and the filter pipeline are some of the reasons its so hard optimize MVC, they are always on, even if your controller action doesn't need it. We're not doing the same with minimal APIs, everything needs to be opted into.

[mottaghipour]

I was convinced, In your opinion, if we change MVC validations In a way that only validate IValidatable objects and optimize this changes in MinimalApis Is this possible?

[mottaghipour]

@davidfowl If we apply validation to parts of the framework in such a way that only IValidatable objects are validated, both validation becomes easier and users have a choice and performance is optimized.

[davidfowl]

In .NET 7 you'll be able to write a filter that can run validation on all the parameters trivially. In fact @DamianEdwards, you should add that filter to the Minimal.Extensions.

I was convinced, In your opinion, if we change MVC validations In a way that only validate IValidatable objects and optimize this changes in MinimalApis Is this possible?

MVC already has a validation system so we wouldn't change it. It does what it does. As for minimal APIs, doing it by default means we need to have an abstraction so others can plug in (for things like fluent validation etc) or a way to turn it off. The way I see it the extensibility point for plugging in validation is filters, so you can plug in to do it automagically.

[davidfowl]

@davidfowl If we apply validation to parts of the framework in such a way that only IValidatable objects are validated, both validation becomes easier and users have a choice and performance is optimized.

You also need to support validating objects you didn't create and can't implement interfaces on. This is why Validated<T> exists.

[mottaghipour]

This is the best way. That's great. I think you should close this issue.

[DamianEdwards]

DamianEdwards/MinimalApis.Extensions#26