osu-tournament-rating / otr-api

API powering osu! Tournament Rating
https://otr.stagec.xyz/
GNU General Public License v3.0
8 stars 5 forks source link

Determine and implement a strategy for "merging" response metadata when redirecting to a route for swagger #526

Open myssto opened 1 week ago

myssto commented 1 week ago

Tasks:

Synopsis

Through testing I have noticed a quirk of swagger that causes endpoints that redirect to not populate the forwarded response codes / response types.

For example, GET /me will redirect to GET /users/{id} by forwarding the authorized user's id

    /// <summary>
    /// Get the currently logged in user
    /// </summary>
    /// <response code="302">Redirects to `GET` `/users/{id}`</response>
    [HttpGet]
    [Authorize(Roles = OtrClaims.Roles.User)]
    [ProducesResponseType(StatusCodes.Status302Found)]
    public IActionResult Get() =>
        RedirectToAction("Get", "Users", new { id = User.GetSubjectId() });

Objectively, this documentation is completely correct since redirects are separate requests, meaning a request to this endpoint (without following the redirect) will return a response code of 302 with no body.

However, this setup causes swagger to not generate response type information because it does not "look forward" into what the redirected request will return so to speak:

{
  "paths": {
    "/api/v1/me": {
      "get": {
        "summary": "Get the currently logged in user",
        "description": "\n\nRequires Authorization:\n\nClaim(s): user",
        "operationId": "Me_get",
        "responses": {
          "302": {
            "description": "Redirects to `GET` `/users/{id}`"
          }
       }
    }
}

This is a problem because any code generator will not be able to infer the response type of this operation. The above definition creates the following client code with nswag:

    /**
    * Get the currently logged in user
    *
    * Requires Authorization:
    * 
    * Claim(s): user
    * @return Returns the currently logged in user
    */
    // ** Notice the response type of OtrApiResponse<void> instead of OtrApiResponse<UserDTO> **
    get(cancelToken?: CancelToken): Promise<OtrApiResponse<void>> { }

Solution

At this time, since we only use redirects in a limited capacity I have simply just copy / pasted the ProducesResponseTypeAttribute<> and <response code=""> of the end destination onto the original request. This achieves the desired result for now but it is not refactor proof and will surely cause issues in the future with any schema changes or changes to the end destination's response codes.

For example (copying the response attributes from GET /users/{id} to GET /me):

    /// <summary>
    /// Get the currently logged in user
    /// </summary>
    /// <response code="302">Redirects to `GET` `/users/{id}`</response>
    // ** Copied from `/users/{id}`
    /// <response code="200">Returns the currently logged in user</response>
    [HttpGet]
    [Authorize(Roles = OtrClaims.Roles.User)]
    [ProducesResponseType(StatusCodes.Status302Found)]
    // ** Copied from `/users/{id}`
    [ProducesResponseType<UserDTO>(StatusCodes.Status200OK)]
    public IActionResult Get() =>
        RedirectToAction("Get", "Users", new { id = User.GetSubjectId() });

Produces the desired definition with populated response references:

{
  "paths": {
    "/api/v1/me": {
      "get": {
        "summary": "Get the currently logged in user",
        "description": "\n\nRequires Authorization:\n\nClaim(s): user",
        "operationId": "Me_get",
        "responses": {
          "302": {
            "description": "Redirects to `GET` `/users/{id}`"
          },
          "200": {
            "description": "Returns the currently logged in user"
            "content": {
              "text/plain": {
                "schema": {
                  "$ref": "#/components/schemas/UserDTO"
                }
              },
              "application/json": {
                "schema": {
                  "$ref": "#/components/schemas/UserDTO"
                }
              },
              "text/json": {
                "schema": {
                  "$ref": "#/components/schemas/UserDTO"
                }
              }
            }
          },
       }
    }
}

A desired permanent fix would alter the swagger generator to look ahead into the redirected action and replicate its potential response metadata onto the original action. This can likely be achieved with a custom Swashbuckle.AspNetCore.SwaggerGen.IOperationFilter. The operation filter exposes the entire swagger document as well as the MethodInfo for the current operation which can likely be reflected on to determine if a redirect will occur.

myssto commented 1 week ago

I realize this is super wordy and seems pointless to write out since I'll probably tackle this myself anyways but it helps me to walk myself through the problem :ehW: