Skip to content

Aggregate multiple Produces for same status code but different content-types #56177

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

Open
1 task done
mikekistler opened this issue Jun 10, 2024 · 0 comments · May be fixed by #62055
Open
1 task done

Aggregate multiple Produces for same status code but different content-types #56177

mikekistler opened this issue Jun 10, 2024 · 0 comments · May be fixed by #62055
Assignees
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-openapi

Comments

@mikekistler
Copy link
Contributor

mikekistler commented Jun 10, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe the problem.

OpenApi v3.0 and later can describe multiple response "contents" for a single status code differentiated by the response content-type. Each "contents" definition can have its own schema. A common case may be:

responses:
  '200':
    description: OK
    content:
      application/json:
        schema:
          type: object
          properties:
            id:
              type: integer
              format: int32
            title:
              type: string
            content:
              type: string
      text/html:
        schema:
          type: string

Currently this response description can't be created just using Produces because each produces can specify only one response type (schema), so two Produces are needed for the case above, but the second Produces for a given status code overrides the information of the first, even if it is for a distinct content type.

Implementation Plan For Copilot

We need to add support for multiple response types with the same status code in ApiExplorer. This limitation currently exists because our ApiResponseTypeProvider only maintains a list of types based on status code. When two attributes specify the same code, the second entry overrides the first. This happens in ApiResponseTypeProvider.ReadResponseMetadata. We need to update the code to support deduplicating based on status-code and content-type.

The relevant files are:

File Purpose
ApiResponseTypeProvider.cs Change collection semantics and add helper record
ApiResponseMetadataProviderContext.cs XML-doc only, note new behaviour
ApiResponseTypeProviderTest.cs Regression tests for duplicate-status scenarios
(No public APIs change; everything modified is internal to Mvc.ApiExplorer.)

We should solve this by completing the following steps:

  1. Add a compound key record

    private readonly record struct ResponseKey(
        int    StatusCode,
        Type?  DeclaredType,
        string? ContentType);
  2. Replace the status-code dictionary in ApiResponseTypeProvider

    - var results = new Dictionary<int, ApiResponseType>();
    + var results = new Dictionary<ResponseKey, ApiResponseType>();
  3. Build a key for every discovered response

    var key = new ResponseKey(
        apiResponseType.StatusCode,
        apiResponseType.Type,                         // may be null
        apiResponseType.ContentTypes.FirstOrDefault() // may be null
    );
    results.TryAdd(key, apiResponseType);            // preserves duplicates

    Optional: if true duplicates arise (same code + type + content type) you may merge their ContentTypes collections instead of discarding either.

  4. Populate SupportedResponseTypes

    action.SupportedResponseTypes = results.Values.ToList();
  5. (Nice-to-have) deterministic ordering

    action.SupportedResponseTypes = results.Values
        .OrderBy(r => r.StatusCode)
        .ThenBy(r => r.Type?.Name)
        .ThenBy(r => r.ContentTypes.FirstOrDefault())
        .ToList();
  6. Unit tests

    • Two [ProducesResponseType] attributes with identical status code but distinct content-types types ➜ assert SupportedResponseTypes.Count == 2.
    • Verify ordering and ContentTypes merging logic if implemented.
    • Verify that multiple Produces calls on minimal API with identical status codes and different content-types produces multiple responses.
    • Verify that the Microsoft.AspNetCore.OpenApi generator produces the correct OpenAPI document for each scenario.
@ghost ghost added the needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically label Jun 10, 2024
@mikekistler mikekistler added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically labels Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-openapi
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants