Produces and Accepts should target IEndpointConventionBuilder

[halter73]

Background and Motivation

This is a continuation of #41428. I first proposed this API there, but it only got partially approved. #43675 demonstrates there are scenarios when we want to apply this kind of metadata to a group.ProducesMetadata, but calling an extension method on group might be more convenient for app developers.

Proposed API

namespace Microsoft.AspNetCore.Http;

public static class OpenApiRouteHandlerBuilderExtensions
{
    public static RouteHandlerBuilder Produces<TResponse>(
        this RouteHandlerBuilder builder,
        int statusCode = StatusCodes.Status200OK,
        string? contentType = null,
        params string[] additionalContentTypes);
    public static RouteHandlerBuilder Produces(
        this RouteHandlerBuilder builder,
        int statusCode,
        Type? responseType = null,
        string? contentType = null,
        params string[] additionalContentTypes);
+    public static TBuilder Produces<TBuilder>(
+        this TBuilder builder,
+        Type? responseType = null,
+        int statusCode = StatusCodes.Status200OK,
+        string? contentType = null,
+        params string[] additionalContentTypes) where TBuilder : IEndpointConventionBuilder;

    public static RouteHandlerBuilder ProducesProblem(
        this RouteHandlerBuilder builder,
        int statusCode,
        string? contentType = null);
+    public static TBuilder ProducesProblem<TBuilder>(
+        this TBuilder builder,
+        int statusCode,
+        string? contentType = null) where TBuilder : IEndpointConventionBuilder;

    public static RouteHandlerBuilder ProducesValidationProblem(
        this RouteHandlerBuilder builder,
        int statusCode = StatusCodes.Status400BadRequest,
        string? contentType = null);
+    public static TBuilder ProducesValidationProblem<TBuilder>(
+        this TBuilder builder,
+        int statusCode = StatusCodes.Status400BadRequest,
+        string? contentType = null) where TBuilder : IEndpointConventionBuilder;

    public static RouteHandlerBuilder Accepts<TRequest>(
        this RouteHandlerBuilder builder,
        bool isOptional,
        string contentType,
        params string[] additionalContentTypes) where TRequest : notnull;
    public static RouteHandlerBuilder Accepts(
        this RouteHandlerBuilder builder,
        Type requestType,
        bool isOptional,
        string contentType,
        params string[] additionalContentTypes);
    public static RouteHandlerBuilder Accepts(
        this RouteHandlerBuilder builder,
        Type requestType,
        string contentType,
        params string[] additionalContentTypes);
+    public static TBuilder Accepts<TBuilder>(
+        this TBuilder builder,
+        Type requestType,
+        bool isOptional,
+        string contentType,
+        params string[] additionalContentTypes) where TBuilder : IEndpointConventionBuilder;
+    public static TBuilder Accepts<TBuilder>(
+        this TBuilder builder,
+        Type requestType,
+        string contentType,
+        params string[] additionalContentTypes) where TBuilder : IEndpointConventionBuilder;

Usage Examples

Now you're forced to manually add attributes using WithMetadata():

// Describe that all APIs can return errors as JSON or plain text
examples.WithMetadata(new ProducesResponseTypeAttribute(typeof(ProblemDetails), 401, "application/problem+json", "text/plain"));
examples.WithMetadata(new ProducesResponseTypeAttribute(typeof(ProblemDetails), 403, "application/problem+json", "text/plain"));

Rather than

// Describe that all APIs can return errors as JSON or plain text
examples.ProducesProblem(401);
examples.ProducesProblem(403);

I don't have as concrete a scenario for adding accepts metadata to an entire group. We could consider skipping this again, but I like having both for consistency. In theory, middleware or BindAsync could be adding additional accepted content types to an entire group somehow. There are alternatives for types implementing BindAsync like also implementing PopulateMetadata, but who knows? Maybe it's too dynamic to be implemented via a static interface.

Alternative Designs

• Only target IEndpointConventionBuilder for Produces, ProducesProblem and ProducesValidationProblem, but not Accepts.
• Add an overload for RouteGroupBuilder rather than IEndpointConventionBuilder even though that's less extensible.

Risks

These are just more extension methods that now show up in intellisense for all IEndpointConventionBuilder types.

[ghost]

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]

Here's a more in-depth explanation I gave in another issues filed by @commonsensesoftware for why we rejected this the first time.

The Produces, ProducesProblem, ProducesValidationProblem and Accepts extension methods were notably excluded from this treatment even though I originally proposed we update these too. The main issue with updating these APIs were the type arguments. Having an additional type argument on top of TBuilder is annoying because then you have to explicitly pass the TBuilder argument then. And as mentioned in the API Review Notes, WithMetadata can be used manually with attributes if needed. But as you note above, there is no public IProducesResponseTypeMetadata implementation at the moment, so setting a response type on a non-RouteHandlerBuilder is even less convenient than it otherwise would be.

I'll admit that this API review was focused on the MapGroup use case and less on the API Versioning use case. I realize that the extension methods we left out are exactly the methods you proposed updating in this issue. Would you be okay with having to write something like .Accepts<VersionedEndpointMetadataBuilder, RequestType>(...)? Or is there something else you would prefer? Are there any other methods you'd like to see use this TBuilder pattern?

#39604 (comment)

[ghost]

Thanks for contacting us.

We're moving this issue to the .NET 8 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.

[murshex]

Any update?