dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.4k stars 10k forks source link

Missing content element for AcceptedAtAction in PUT and PATCH #18333

Closed knoxi closed 4 years ago

knoxi commented 4 years ago

As already discussed with the team of Swashbuckle, we have the issue that when we are generating a description for a simple CRUD controller, then the content element in PUT and PATCH in swagger is missing.

As Swashbuckle mentioned, there seems that a behavior has changed in asp.net core. Here is the link to our original discussion: https://github.com/domaindrivendev/Swashbuckle.AspNetCore/issues/1470. They suggested to create an issue in this repository.

My controller looks like this:

[Consumes("application/json")]
[Produces("application/json")]
[ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status400BadRequest)]
[ProducesResponseType(typeof(void), StatusCodes.Status404NotFound)]
[ApiController]
[ApiVersion("1.0")]
[Route("api/v{version:apiVersion}/[controller]")]
public class OrderDetailController : ControllerBase
{
    ...

    [HttpPost()]
    [ProducesResponseType(StatusCodes.Status201Created)]
    [SwaggerOperation("CreateOrderDetail")]
    public async Task<ActionResult<OrderDto>> CreateEntity([FromBody]OrderDto dto)
    {
        ...
        return CreatedAtAction(GetByIdName, new { id = createdDto.Id, version = "1.0" }, createdDto);
    }

    [HttpPut("{id}")]
    [ProducesResponseType(StatusCodes.Status202Accepted)]
    [SwaggerOperation("UpdateOrderDetail")]
    public async Task<ActionResult<OrderDto>> UpdateEntity(TKey id, [FromBody] OrderDto dto)
    {
        ...
        return AcceptedAtAction("Id", new { id = createdDto.Id, version = "1.0" }, updatedDto);
    }

    [HttpPatch("{id}")]
    [ProducesResponseType(StatusCodes.Status202Accepted)]
    [SwaggerOperation("PatchOrderDetail")]
    public async Task<ActionResult<OrderDto>> PatchEntity(TKey id, [FromBody] JsonPatchDocument<OrderDto> patchDoc)
    {
        ...
        return AcceptedAtAction("Id", new { id = createdDto.Id, version = "1.0" }, patchedDto);
    }

    ...
}

and the generated swagger.json looks as following:

{
    "openapi": "3.0.1",
    ...
    "paths": {
        "/api/v1.0/OrderDetail": {
            "post": {
              "tags": [
                "OrderDetail"
              ],
              "summary": "CreateOrderDetail",
              "operationId": "CreateOrderDetail",
              "requestBody": {
                "content": {
                  "application/json": {
                    "schema": {
                      "$ref": "#/components/schemas/OrderDetailDto"
                    }
                  }
                }
              },
              "responses": {
                "400": {
                  "description": "Bad Request",
                  "content": {
                    "application/json": {
                      "schema": {
                        "$ref": "#/components/schemas/ProblemDetails"
                      }
                    }
                  }
                },
                "404": {
                  "description": "Not Found",
                  "content": {
                    "application/json": {
                      "schema": {
                        "$ref": "#/components/schemas/ProblemDetails"
                      }
                    }
                  }
                },
                "201": {
                  "description": "Success",
                  "content": {
                    "application/json": {
                      "schema": {
                        "$ref": "#/components/schemas/OrderDetailDto"
                      }
                    }
                  }
                }
              }
            },
            "get": ...
          },
          "/api/v1.0/OrderDetail/{id}": {
            "put": {
              "tags": [
                "OrderDetail"
              ],
              "summary": "UpdateOrderDetail",
              "operationId": "UpdateOrderDetail",
              "parameters": [
                {
                  "name": "id",
                  "in": "path",
                  "required": true,
                  "schema": {
                    "type": "integer",
                    "format": "int32"
                  }
                }
              ],
              "requestBody": {
                "content": {
                  "application/json": {
                    "schema": {
                      "$ref": "#/components/schemas/OrderDetailDto"
                    }
                  }
                }
              },
              "responses": {
                "400": {
                  "description": "Bad Request",
                  "content": {
                    "application/json": {
                      "schema": {
                        "$ref": "#/components/schemas/ProblemDetails"
                      }
                    }
                  }
                },
                "404": {
                  "description": "Not Found",
                  "content": {
                    "application/json": {
                      "schema": {
                        "$ref": "#/components/schemas/ProblemDetails"
                      }
                    }
                  }
                },
                "202": {
                  "description": "Success"
                }
              }
            },
            "patch": {
              "tags": [
                "OrderDetail"
              ],
              "summary": "PatchOrderDetail",
              "operationId": "PatchOrderDetail",
              "parameters": [
                {
                  "name": "id",
                  "in": "path",
                  "required": true,
                  "schema": {
                    "type": "integer",
                    "format": "int32"
                  }
                }
              ],
              "requestBody": {
                "content": {
                  "application/json": {
                    "schema": {
                      "type": "array",
                      "items": {
                        "$ref": "#/components/schemas/Operation"
                      },
                      "nullable": true
                    }
                  }
                }
              },
              "responses": {
                "400": {
                  "description": "Bad Request",
                  "content": {
                    "application/json": {
                      "schema": {
                        "$ref": "#/components/schemas/ProblemDetails"
                      }
                    }
                  }
                },
                "404": {
                  "description": "Not Found",
                  "content": {
                    "application/json": {
                      "schema": {
                        "$ref": "#/components/schemas/ProblemDetails"
                      }
                    }
                  }
                },
                "202": {
                  "description": "Success"
                }
              }
            },
            "delete": ...,
            "get": ...
        }
    }
}        

We are using the latest version of

pranavkm commented 4 years ago

MVC uses the returned value, i.e. T in ActionResult<T>, specifically for 200 and 201 responses. It was based on this pattern for returning either an Ok or Accepted result from an action that peforms long running operations, which we found occurring frequently in a few large API sets: https://github.com/aspnet/Mvc/issues/7871#issuecomment-398144056.

Supplying a [ProducesResponseType(202, typeof(OrderDto))] would be the way to go for now. Right now, there doesn't seem to be compelling evidence we need to change the behavior for 202 responses. We might reconsider it if we get more user feedback for this.

knoxi commented 4 years ago

Because we use a custom generic BaseController class, we unfortunately cannot apply the generic type.

The generic type causes a compiler error:

[ProducesResponseType(statusCode: StatusCodes.Status202Accepted, type: typeof(TDto))]
public class CustomControllerBase<TEntity, TDto, TKey> : ControllerBase
{
    ...

    [HttpPost()]
    [ProducesResponseType(StatusCodes.Status201Created)]
    [SwaggerOperation("CreateOrderDetail")]
    public async Task<ActionResult<TDto>> CreateEntity([FromBody]TDto dto)
    {
        ...
        return CreatedAtAction(...);
    }

    [HttpPut("{id}")]
    [ProducesResponseType(StatusCodes.Status202Accepted)]
    [SwaggerOperation("UpdateOrderDetail")]
    public async Task<ActionResult<TDto>> UpdateEntity(TKey id, [FromBody] TDto dto)
    {
        ...
        return AcceptedAtAction(...);
    }

    [HttpPatch("{id}")]
    [ProducesResponseType(StatusCodes.Status202Accepted)]
    [SwaggerOperation("PatchOrderDetail")]
    public async Task<ActionResult<TDto>> PatchEntity(TKey id, [FromBody] JsonPatchDocument<TDto> patchDoc)
    {
        ...
        return AcceptedAtAction(...);
    }

    ...
}

Do you have any suggestions how I can solve or workaround this problem without override and set this on each derived class?

Simon-Gregory-LG commented 4 years ago

Consider this a +1 vote for including ActionResult<T> types for StatusCodes.Status202Accepted as a response object is typically expected to be returned with this code, as discussed here:

https://restfulapi.net/http-status-202-accepted/

Also, RFC7231 states: (https://tools.ietf.org/html/rfc7231#section-6.3.3)

The 202 (Accepted) status code indicates that the request has been accepted for processing, but the processing has not been completed. The request might or might not eventually be acted upon, as it might be disallowed when processing actually takes place. There is no facility in HTTP for re-sending a status code from an asynchronous operation.

The 202 response is intentionally noncommittal. Its purpose is to allow a server to accept a request for some other process (perhaps a batch-oriented process that is only run once per day) without requiring that the user agent's connection to the server persist until the process is completed. The representation sent with this response ought to describe the request's current status and point to (or embed) a status monitor that can provide the user with an estimate of when the request will be fulfilled.

pranavkm commented 4 years ago

@knoxi, I don't think there's a baked in way to do this. However, you might be able to author an IApiDescriptionProvider that mutates the results of the default one:

public void OnProvidersExecuting(ApiDescriptionProviderContext context)
{
   foreach (var item in context.Results)
   {
       var response202 = item.SupportedResponseTypes.FirstOrDefault(r => r.StatusCode == 202);
        if (response202 != null && 
            !item.SupportedResponseTypes.Any(r => r.StatusCode == 200 || r.StatusCode == 201))
        {
            response202.Type = GetDeclaredReturnType(item.ActionDescriptor);
        }
    }
}

GetDeclaredReturnType would look something like this: https://github.com/dotnet/aspnetcore/blob/master/src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs#L213-L242. I haven't tried this out, but this would be a closer approximation of what you'd need to do. You'd register the type in DI and it should get used by the framework:

services.AddTransient<IApiDescriptionProvider, MyProvider>();

@Simon-Gregory-LG while that's true, the pattern we commonly saw was APIs returning either a 202 or a 200 depending on if the response was ready. In that case, using the result type for a 202 would be incorrect. We could make it conditional if it were the only success status code, but I don't think adding complexity here helps. It would be much more intuitive to document the 202 response type uniformly across the application.

ghost commented 4 years ago

This issue has been resolved and has not had any activity for 1 day. It will be closed for housekeeping purposes.

See our Issue Management Policies for more information.