Improve OpenAPI/Swagger response metadata for returned IResults
halter73 opened this issue · 10 comments
Today, when you return an IResult from a minimal action, you lose all metadata about the type of the response (see #33433). You can add the metadata back with something like [ProducesResponseType(typeof(Person), 201], but in many cases the response type could be inferred from the IResult implementation if we designed a way for this to work.
The most obvious idea is to do like ActionResult<TValue> though implicit conversions with interfaces might make this tricky.
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.
Another option is adding extension methods on MinimalActionEndpointConventionBuilder that adds the Produces*Attributes as endpoint metadata.
Here's an example from @DamianEdwards showing how this could look.
app.MapPost("/todos", async (Todo todo, TodoDb db) =>
{
if (!MinimalValidation.TryValidate(todo, out var errors))
return Results.ValidationProblem(errors);
db.Todos.Add(todo);
await db.SaveChangesAsync();
return Results.Created($"/todos/{todo.Id}", todo);
})
.ProducesValidationProblem()
.Produces<Todo>();That example (and a few more) are from a functional prototype/example at https://github.com/DamianEdwards/MinimalApiPlayground/blob/main/src/MinimalApiPlayground/Program.cs
Something like that would be good.
I've done a similar thing with the project I've been trying minimal actions out with: https://github.com/martincostello/dotnet-minimal-api-integration-testing/blob/2c9bf003d8213e73d06729d0001d48952f6fa741/src/TodoApp/ApiModule.cs#L31-L32
I've suggested a similar thing for ignoring the endpoints too in #34068.
Though at the same time it's made me wonder at what point the dividing line between minimal actions and "growing up" to moving "back" to a controller is?
Not a criticism as it's a new different style of implementing APIs, it's just that with a bunch of annotations on an action that's only a few lines of code it's suddenly a lot less...minimal 🙂
@martincostello I actually think one of the benefits of the minimal API style is that the extension methods are (admittedly subjectively) less ceremony and far easier to discover than the attributes. The attribute names are somewhat fixed now but it's easy for us to add method overloads that do more/less/allow differing expression.
Also I do think at some point we'll enable hopefully much of this as a convention/automatically so that one doesn't need to describe the response shape manually at all, e.g. via a source generator.
Strawman set of extension methods for setting endpoint metadata. Additional context provided in the doc/code comments. These methods and their new supporting types (where required) can also be seen in the playground repo.
using Microsoft.AspNetCore.Http.Extensions;
using Microsoft.AspNetCore.Mvc;
namespace Microsoft.AspNetCore.Http;
public static class OpenApiEndpointConventionBuilderExtensions
{
/// <summary>
/// Adds an EndpointNameMetadata item to the Metadata for all endpoints produced by the builder.<br />
/// The name is used to lookup the endpoint during link generation and as an operationId when generating OpenAPI documentation.<br />
/// The name must be unique per endpoint.
/// </summary>
/// <param name="builder">The Microsoft.AspNetCore.Builder.IEndpointConventionBuilder.</param>
/// <param name="name">The endpoint name.</param>
/// <returns>The Microsoft.AspNetCore.Builder.IEndpointConventionBuilder.</returns>
public static IEndpointConventionBuilder WithName(this IEndpointConventionBuilder builder, string name)
{
// Once Swashbuckle issue is fixed this will set operationId in the swagger doc
// https://github.com/domaindrivendev/Swashbuckle.AspNetCore/issues/2165
builder.WithMetadata(new EndpointNameMetadata(name));
return builder;
}
/// <summary>
/// Adds an EndpointGroupNameMetadata item to the Metadata for all endpoints produced by the builder.
/// </summary>
/// <param name="builder"></param>
/// <param name="groupName"></param>
/// <returns>The Microsoft.AspNetCore.Builder.IEndpointConventionBuilder.</returns>
public static IEndpointConventionBuilder WithGroupName(this IEndpointConventionBuilder builder, string groupName)
{
// Swashbuckle uses group name to match APIs with OpenApi documents by default
// See https://github.com/domaindrivendev/Swashbuckle.AspNetCore/blob/master/src/Swashbuckle.AspNetCore.SwaggerGen/SwaggerGenerator/SwaggerGeneratorOptions.cs#L59
// Minimal APIs currently doesn't populate the ApiDescription with a group name but we will change that so this can work as intended.
// Note that EndpointGroupNameMetadata doesn't exist in ASP.NET Core today so we'll have to add that too.
builder.WithMetadata(new EndpointGroupNameMetadata(groupName));
return builder;
}
/// <summary>
/// Adds metadata indicating the type of response an endpoint produces.
/// </summary>
/// <typeparam name="TResponse">The type of the response.</typeparam>
/// <param name="builder">The Microsoft.AspNetCore.Builder.IEndpointConventionBuilder.</param>
/// <param name="statusCode">The response status code. Defatuls to StatusCodes.Status200OK.</param>
/// <param name="contentType">The response content type. Defaults to "application/json"</param>
/// <param name="additionalContentTypes">Additional response content types the endpoint produces for the supplied status code.</param>
/// <returns>The Microsoft.AspNetCore.Builder.IEndpointConventionBuilder.</returns>
public static IEndpointConventionBuilder Produces<TResponse>(this IEndpointConventionBuilder builder,
int statusCode = StatusCodes.Status200OK,
string? contentType = "application/json",
params string[] additionalContentTypes)
{
return Produces(builder, statusCode, typeof(TResponse), contentType, additionalContentTypes);
}
/// <summary>
/// Adds metadata indicating the type of response an endpoint produces.
/// </summary>
/// <param name="builder">The Microsoft.AspNetCore.Builder.IEndpointConventionBuilder.</param>
/// <param name="statusCode">The response status code. Defatuls to StatusCodes.Status200OK.</param>
/// <param name="responseType">The type of the response. Defaults to null.</param>
/// <param name="contentType">The response content type. Defaults to "application/json" if responseType is not null, otherwise defaults to null.</param>
/// <param name="additionalContentTypes">Additional response content types the endpoint produces for the supplied status code.</param>
/// <returns>The Microsoft.AspNetCore.Builder.IEndpointConventionBuilder.</returns>
public static IEndpointConventionBuilder Produces(this IEndpointConventionBuilder builder,
int statusCode = StatusCodes.Status200OK,
Type? responseType = null,
string? contentType = null,
params string[] additionalContentTypes)
{
if (responseType is Type && string.IsNullOrEmpty(contentType))
{
contentType = "application/json";
}
builder.WithMetadata(new ProducesMetadataAttribute(responseType, statusCode, contentType, additionalContentTypes));
return builder;
}
/// <summary>
/// Adds metadata indicating that the endpoint produces a Problem Details response.
/// </summary>
/// <param name="builder">The Microsoft.AspNetCore.Builder.IEndpointConventionBuilder.</param>
/// <param name="statusCode">The response status code. Defatuls to StatusCodes.Status500InternalServerError.</param>
/// <param name="contentType">The response content type. Defaults to "application/problem+json".</param>
/// <returns>The Microsoft.AspNetCore.Builder.IEndpointConventionBuilder.</returns>
public static IEndpointConventionBuilder ProducesProblem(this IEndpointConventionBuilder builder,
int statusCode = StatusCodes.Status500InternalServerError,
string contentType = "application/problem+json")
{
return Produces<ProblemDetails>(builder, statusCode, contentType);
}
/// <summary>
/// Adds metadata indicating that the endpoint produces a Problem Details response including validation errors.
/// </summary>
/// <param name="builder">The Microsoft.AspNetCore.Builder.IEndpointConventionBuilder.</param>
/// <param name="statusCode">The response status code. Defatuls to StatusCodes.Status400BadRequest.</param>
/// <param name="contentType">The response content type. Defaults to "application/problem+json".</param>
/// <returns>The Microsoft.AspNetCore.Builder.IEndpointConventionBuilder.</returns>
public static IEndpointConventionBuilder ProducesValidationProblem(this IEndpointConventionBuilder builder,
int statusCode = StatusCodes.Status400BadRequest,
string contentType = "application/problem+json")
{
return Produces<HttpValidationProblemDetails>(builder, statusCode, contentType);
}
}Design considerations
Further details that were considered during the design of this proposal:
- Having specific methods for each response type that could be produced by the methods on
Microsoft.AspNetCore.Http.Results, e.g.ProducesCreated,ProducesConflict,ProducesNotFound, etc. As the eventual goal is for the experience to determine the correct information RE what endpoints produce and set the relevant metadata by default, thus requiring these methods to not be required to be called in the vast majority of cases, it was decided that we shouldn't create such a large set of API surface area only to have it not be used (generally) in the future. - After the methods were reduced to just a minimal set, as detailed in the previous point, it was apparent that specifying that an API produces Problem Details required an extremely verbose and likely unintuitive call, i.e.
Produces<ProblemDetails>(500, "application/problem+json")andProduces<HttpValidationProblemDetails>(400, "application/problem+json"). For this reason it was decided that higher level methods for problem details should be included, e.g.ProducesProblem,ProducesValidationProblem. These offer a nice symmetry with theResults.ProblemandResults.ValidationProblemresults methods too.
Ignored method get a random group name?
@davidfowl I'm not recommending that, but that would make Swashbuckle's existing logic work.
We should instead create a new first-class metadata type for indicating an endpoint should be ignored from ApiExplorer's POV (#34068) and either not publish endpoints with that metadata into ApiExplorer or update Swashbuckle and nSwag to not include those endpoints in document/code-generation if the metadata is present.
I've removed that from the code block above as it was causing confusion.
Some related detail coming together in #34514
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.