Skip to content

Allow parameter and return types of route handler delegates (Minimal APIs) to contribute to endpoint metadata #40646

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
DamianEdwards opened this issue Mar 10, 2022 · 6 comments · Fixed by #40926
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-minimal-actions Controller-like actions for endpoint routing feature-openapi old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels
Milestone

Comments

@DamianEdwards
Copy link
Member

DamianEdwards commented Mar 10, 2022

Overview

Introduce the capability for types used for parameters and return values for route handler delegates (Minimal APIs) to contribute to metadata of the endpoint they're mapped to. This will allow types that implement custom binding logic via TryParse or BindAsync, and IResult types, to add metadata that describes the API parameters and responses in Swagger UI and OpenAPI documents.

This, along with some other capabilities, will better allow route handler delegates to self-describe themselves from just the type information in their signature, reducing the need for the developer to manually provide the same type information about the API in the endpoint metadata.

A functionally equivalent version of this feature is already implemented in the MinimalApis.Extensions library.

Problem

Today, route handler delegates in Minimal APIs are auto-described in ApiExplorer and thus Swagger UI and OpenAPI documents (through libraries like Swashbuckle) including details of the parameters they accept and response they return, e.g.

Program.cs

var todos = new Dictionary<int, Todo>
{
    { 1, new Todo(1, "Buy the groceries") },
    { 2, new Todo(2, "Wash the car") },
    { 3, new Todo(3, "Put out the garbage") }
};

app.MapGet("/todos/{id}", (int id) => todos[id]);

app.Run();

record Todo(int Id, string Title, bool IsCompleted = false);

Swagger UI
image

Note that the input parameter id is correctly named and typed based on the delegate parameter, and the 200 response has a media type of application/json and a body schema based on the Todo type that the delegate returns.

This auto-describing of the API breaks down though when the extensibility options for parameter binding or result processing are utilized, namely TryParse or BindAsync on parameter types, and IResult for return types. e.g.:

Program.cs

app.MapGet("/todos/customid/{id}", (TodoId id) =>
    todos.SingleOrDefault(t => t.Key == id.Id).Value switch
    {
        Todo todo => Results.Ok(todo),
        _ => Results.NotFound()
    });

app.Run();

record Todo(int Id, string Title, bool IsCompleted = false);
record struct TodoId(int Id)
{
    public static bool TryParse(string value, out TodoId result)
    {
        if (int.TryParse(value, out int id))
        {
            result = new TodoId(id);
            return true;
        }

        result = new();
        return false;
    }
}

Swagger UI
image

Note this time that the id parameter is typed as "string" and the 200 response has no media type or schema information.

To restore the API description details, the user is required to manually describe the API via extension methods (or attributes) that add endpoint metadata about the responses, e.g.:

Program.cs

app.MapGet("/todos/customid/{id}", (TodoId id) =>
    todos.SingleOrDefault(t => t.Key == id.Id).Value switch
    {
        Todo todo => Results.Ok(todo),
        _ => Results.NotFound()
    })
    .Produces<Todo>()
    .Produces(StatusCodes.Status404NotFound);

Swagger UI
image

Note that today there is no endpoint metadata supported that will describe API parameters, so the id parameter is still typed incorrectly as string, but the 200 and 404 responses are now described.

Proposed Solution

We should introduce a way for types used in the signature of route handlers (parameters and return values) to add metadata to the associated endpoint. This metadata should be featured enough to properly contribute all API parameter and response details in ApiExplorer and thus Swagger UI and OpenAPI documents.

Interfaces

Two new interfaces will be introduced, one for parameter types and another for return value types. The presence of these interfaces on a type indicates they can contribute to the endpoint metadata.

/// <summary>
/// Marker interface that indicates a type provides a static method that returns <see cref="Endpoint"/> metadata for a
/// parameter on a route handler delegate.
/// </summary>
public interface IProvideEndpointParameterMetadata
{
    static abstract IEnumerable<object> GetMetadata(ParameterInfo parameter, IServiceProvider services);
}

/// <summary>
/// Marker interface that indicates a type provides a static method that returns <see cref="Endpoint"/> metadata for the
/// returned value from a given <see cref="Endpoint"/> route handler delegate.
/// </summary>
public interface IProvideEndpointMetadata
{
    static abstract IEnumerable<object> GetMetadata(MethodInfo methodInfo, IServiceProvider services);
}

Each interface has a single method that will be called by the framework when the endpoint is being built, with the returned objects being added to the endpoint metadata.

The IProvideEndpointParameterMetadata interface's GetMetadata method accepts a ParameterInfo representing the relevant route handler delegate parameter, such that the parameter name, type details, etc. are available, along with the IServiceProvider for the application's DI container.

The IProvideEndpointMetadata interface's GetMetadata method accepts the MethodInfo instance for the endpoint that the returned metadata will be applied to, such that the details of the method are available, along with the IServiceProvider for the application's DI container.

Example

The following is an example of an API that accepts a parameter utilizing custom binding via BindAsync and returns a custom IResult type that describes itself in Swagger UI/OpenAPI by emitting metadata via the
IProvideEndpointParameterMetadata and IProvideEndpointMetadata interfaces:

Program.cs

app.MapGet("/todos", async (TodoDb db) => await db.Todos.ToListAsync());
app.MapGet("/todos/{id}", async (int id, TodoDb db) => await db.Todos.FindAsync(id));
app.MapPost("/todos", async (NewTodo newTodo, TodoDb db) =>
{
    if (newTodo.IsValid && newTodo.Todo is not null)
    {
        db.Todos.Add(newTodo.Todo);
        await db.SaveChangesAsync();
        return new CreateResult<Todo>(newTodo.Todo);
    }
    return new CreateResult<Todo>(newTodo.ValidationErrors);
});

app.Run();

class Todo
{
    public Todo(string? title, bool isComplete = false)
    {
        Title = title;
        IsComplete = isComplete;
    }

    public int Id { get; set; }
    public string? Title { get; set; }
    public bool IsComplete { get; set; }
}

class NewTodo : IProvideEndpointParameterMetadata
{
    private static readonly Todo EmptyTodo = new(default);

    private NewTodo() { }

    private NewTodo(Todo todo)
    {
        switch(todo)
        {
            case { Title: null }:
            case { Title.Length: 0 }:
                ValidationErrors.Add(nameof(Todo.Title), new[] { $"The {nameof(Todo.Title)} field is required." });
                break;
            default:
                Todo = todo;
                break;
        }
    }

    public Todo Todo { get; } = EmptyTodo;

    public bool IsValid => ValidationErrors.Count > 0;

    public Dictionary<string, string[]> ValidationErrors { get; } = new();

    public static async ValueTask<NewTodo> BindAsync(HttpContext context) =>
        await context.Request.ReadFromJsonAsync<Todo>() switch
        {
            Todo todo => new NewTodo(todo),
            _ => new NewTodo()
        };

    public static IEnumerable<object> GetMetadata(ParameterInfo parameter, IServiceProvider services)
    {
        yield return new ConsumesAttribute(typeof(Todo), "application/json");
    }
}

public class GetResult<TResponse> : IResult, IProvideEndpointMetadata
{
    public GetResult(TResponse? response) => Response = response;

    public TResponse? Response { get; set; }

    public async Task ExecuteAsync(HttpContext httpContext) =>
        await (Response switch
        {
            not null => Results.Ok(Response).ExecuteAsync(httpContext),
            _ => Results.NotFound().ExecuteAsync(httpContext)
        });

    public static IEnumerable<object> GetMetadata(MethodInfo methodInfo, IServiceProvider services)
    {
        yield return new ProducesResponseTypeAttribute(typeof(TResponse), StatusCodes.Status200OK, "application/json");
        yield return new ProducesResponseTypeAttribute(StatusCodes.Status404NotFound);
    }
}

public class CreateResult<TResponse> : IResult, IProvideEndpointMetadata
{
    public CreateResult(TResponse response) => Response = response;

    public CreateResult(IDictionary<string, string[]> errors) => ProblemDetails = new(errors);

    public TResponse? Response { get; }

    public HttpValidationProblemDetails? ProblemDetails { get; }

    public async Task ExecuteAsync(HttpContext httpContext) =>
        await (Response switch
        {
            TResponse => Results.Ok(Response).ExecuteAsync(httpContext),
            _ => Results.Problem(ProblemDetails!).ExecuteAsync(httpContext)
        });

    public static IEnumerable<object> GetMetadata(MethodInfo methodInfo, IServiceProvider services)
    {
        yield return new ProducesResponseTypeAttribute(typeof(TResponse), StatusCodes.Status201Created, "application/json");
        yield return new ProducesResponseTypeAttribute(typeof(HttpValidationProblemDetails), StatusCodes.Status400BadRequest, "application/problem+json");
    }
}

Challenges/Open Questions

Existing result types

The existing IResult implementations in the framework are currently internal, although that will likely change as part of #37502. For this feature to be useful out-of-the-box, these result types must be public and be updated (where applicable) to implement IProvideEndpointMetadata so that they can contribute to endpoint metadata.

Result types that represent a response with a body that's based on a type need to be able to convey the schema of the body in their type signature, meaning they should need to be generic, where the generic type argument is the type that represents the resource being returned, e.g. OkResult<Todo>, CreatedAtResult<Todo>, etc.

Also each type needs to completely represent a distinct HTTP result without relying on runtime/instance data, e.g. StatusCodeResult is ambiguous when statically observed as to what the actual HTTP result is and thus will not work, whereas a NotFoundResult can wholly represents an HTTP 404 Not Found result statically.

Results class static factory methods

The current Results static factory methods that all return IResult do not allow the framework to observe the actual concrete types being returned, and thus we'll need to consider adding new methods to create the result types that preserve the concrete type information, or some other solution, e.g.

  • Introduce a new static class, e.g. HttpResults, which has factory methods for the public result types that return the concrete types
  • Introduce a new set of methods on the existing Results class but under a new property, e.g. Results.Typed.NotFound(), etc. (this one is my preference right now)
  • Change the existing static methods to return the now public concrete types instead of IResult (Note along with this being a binary-breaking change, this has implications on the compiler's ability to infer the return type of anonymous lambdas when there's more than one return type in the method so is likely a source-breaking change too)
  • Add overloads to the existing static methods that differ only by return type. This is not possible in C# but can be expressed in MSIL. Then have the .NET 7 ref assemblies only expose the new concrete-type returning overloads while the implementation assembly has both overloads, keeping existing binaries bound to the .NET 6 IResult-returning overloads (this is likely crazy but possibly crazy like a fox, so...). (Note this has the same issue as the point above RE the compiler's ability to infer anonymous lambda return type)
  • Make the in-box result types constructable so that they can be returned directly, e.g. return new NotFoundHttpResult(); (Note this has the same issue as the point above RE the compiler's ability to infer anonymous lambda return type)
  • Add static factory methods to the in-box result types so they can be returned directly, e.g. return NotFoundHttpResult.Create(); (Note this has the same issue as the point above RE the compiler's ability to infer anonymous lambda return type)
  • Something else?

Multiple return types

Most APIs end up having multiple return paths that return different result types depending on the outcome of the API invocation, e.g. returning an OkResult if the resource is found, and a NotFoundResult if it isn't. C# doesn't currently have a language construct that allows a method to declare it returns multiple types (see discriminated unions at dotnet/csharplang#113) but this can be achieved using the type system thanks to generics, e.g.

TodosApi.cs

app.MapGet("/todos/{id}", GetTodoById);

async Task<Results<Ok<Todo>, NotFound>> GetTodoById(int id, IDbConnection db) =>
    await db.QuerySingleOrDefaultAsync<Todo>("SELECT * FROM Todos WHERE Id = @id", new { id })
    is Todo todo
        ? Results.Extensions.Ok(todo)
        : Results.Extensions.NotFound();

See #40672 for details of a proposal to support multiple return types on route handler delegates.

@DamianEdwards DamianEdwards added feature-openapi feature-minimal-actions Controller-like actions for endpoint routing old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels labels Mar 10, 2022
@DamianEdwards DamianEdwards changed the title WIP: Allow parameter and return types of route handler delegates (Minimal APIs) to contribute to endpoint metadata Allow parameter and return types of route handler delegates (Minimal APIs) to contribute to endpoint metadata Mar 10, 2022
@DamianEdwards DamianEdwards added the enhancement This issue represents an ask for new feature or an enhancement to an existing one label Mar 10, 2022
@rafikiassumani-msft rafikiassumani-msft added this to the 7.0-preview4 milestone Mar 15, 2022
DamianEdwards added a commit that referenced this issue Mar 28, 2022
- IProvideEndpointParameterMetadata
- IProvideEndpointResponseMetadata
- Contributes to #40646
DamianEdwards added a commit that referenced this issue Mar 28, 2022
@DamianEdwards DamianEdwards added the api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews label Mar 29, 2022
@ghost
Copy link

ghost commented Mar 29, 2022

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

@halter73
Copy link
Member

halter73 commented Mar 29, 2022

Changing the ParameterInfo parameter, IServiceProvider services parameters to an EndpointParameterMetadataContext type will make it easier to avoid breaking changes in the future when we decide to provide more details to the GetMetadata method. Maybe something like:

public sealed class EndpointParameterMetadataContext
{
    public ParameterInfo Parameter { init; }
    public IServiceProvider Services { init; }
    // Possible additions
    public RoutePattern RoutePattern { init; }
    public IList<object> EndpointMetadata { init; }
}

Then we could do something similar with a EndpointMetadataContext:

public sealed class EndpointMetadataContext
{
    public MethodInfo Method { init; }
    public IServiceProvider Services { init; }
    // Possible additions
    public RoutePattern RoutePattern { init; }
    publice IList<object> EndpointMetadata { init; }
}
public interface IProvideEndpointParameterMetadata
{
    static abstract void PopulateMetadata(EndpointParameterMetadataContext parameterContext);
}

public interface IProvideEndpointMetadata
{
    static abstract void PopulateMetadata(EndpointMetadataContext context);
}

The return becomes void since metadata can be added to the IList<object> just like with an IEndpointConventionBuilder.

@davidfowl
Copy link
Member

It's not GetMetadata if it's populating a context. Maybe PopulateMetadata

@DamianEdwards
Copy link
Member Author

DamianEdwards commented Mar 29, 2022

Changing to a context definitely adds flexibility to change in the future. Are you expecting that the mutable properties on the context would be preserved as each type's method is called? If so, that introduces a potential reliance on ordering that we shouldn't be guaranteeing. We could of course not preserve those values but that seems different to how we use context types in other places. I guess we could make the properties settable and be IReadOnlyList<object> or just IEnumerable<object> too.

@halter73
Copy link
Member

halter73 commented Apr 4, 2022

API Review:

  • Do we only look for this in minimal route handler parameters and return values? What about attributes?
    • Just parameter and return types.
  • Do we need an analyzer to tell people to return the specific type with GetMetadata instead of a plain IResult?
    • Yes, as a follow up.
  • We will require the interface is implemented rather than just looking for the right static method signature like TryParse and BindAsync. Static abstract interfaces are no longer experimental.
  • IProvide... -> I...Provider
namespace Microsoft.AspNetCore.Http;

public sealed class EndpointParameterMetadataContext
{
    public ParameterInfo Parameter { init; }
    public IServiceProvider Services { init; }

    public IList<object> EndpointMetadata { init; }
}

public sealed class EndpointMetadataContext
{
    public MethodInfo Method { init; }
    public IServiceProvider Services { init; }

    public IList<object> EndpointMetadata { init; }
}

public interface IEndpointParameterMetadataProvider
{
    static abstract void PopulateMetadata(EndpointParameterMetadataContext parameterContext);
}

public interface IEndpointMetadataProvider
{
    static abstract void PopulateMetadata(EndpointMetadataContext context);
}

@halter73 halter73 added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews labels Apr 4, 2022
@DamianEdwards
Copy link
Member Author

API changes since API review based on implementation details and PR feedback:

-namespace Microsoft.AspNetCore.Http;
+namespace Microsoft.AspNetCore.Http.Metadata;

public sealed class EndpointParameterMetadataContext
{
-    public ParameterInfo Parameter { get; init; }
+    public ParameterInfo Parameter { get; internal set; } // internal set to allow re-use
    public IServiceProvider Services { get; init; }
    public IList<object>? EndpointMetadata { get; init; }
}

public sealed class EndpointMetadataContext
{
    public MethodInfo Method { get; init; }
    public IServiceProvider Services { get; init; }
    public IList<object>? EndpointMetadata { get; init; }
}

public interface IEndpointParameterMetadataProvider
{
    static abstract void PopulateMetadata(EndpointParameterMetadataContext parameterContext);
}

public interface IEndpointMetadataProvider
{
    static abstract void PopulateMetadata(EndpointMetadataContext context);
}

public sealed class RequestDelegateFactoryOptions
{
    public IServiceProvider? ServiceProvider { get; init; }

    public IEnumerable<string>? RouteParameterNames { get; init; }

    public bool ThrowOnBadRequest { get; init; }

    public bool DisableInferBodyFromParameters { get; init; }
    
    public IReadOnlyList<Func<RouteHandlerContext, RouteHandlerFilterDelegate, RouteHandlerFilterDelegate>>? RouteHandlerFilterFactories { get; init; }

+    public IEnumerable<object>? AdditionalEndpointMetadata { get; init; }
}

@ghost ghost locked as resolved and limited conversation to collaborators May 8, 2022
@amcasey amcasey added the area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc label Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-minimal-actions Controller-like actions for endpoint routing feature-openapi old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants