Mvc: ProducesResponseType doesn't infer the type from ActionResult<T>

Created on 5 Jun 2018  路  8Comments  路  Source: aspnet/Mvc

Is this a Bug or Feature request?:

I think it's a bug but it's also possible the behavior is by design.

Steps to reproduce (preferrably a link to a GitHub repo with a repro project):

  1. Create a simple API project and wire up Swashbuckle as described here:
    https://docs.microsoft.com/en-us/aspnet/core/tutorials/web-api-help-pages-using-swagger?view=aspnetcore-2.1
  2. Create a controller with a single action that a) returns the ActionResult<T> type introduced in 2.1 and b) is annotated with ProducesResponseType to indicate that the action returns a 201 code instead of the default 200:
// ProductsController.cs
public class ProductsController
{
    [HttpPost("api/producs")]
    [ProducesResponseType(201)]
    public ActionResult<Product> Create([FromBody, Required]Product product)
    {
        ...
    }
}

public class Product
{
    ...
}
  1. Navigate to the Swagger UI and note that the 201 response doesn't display a corresponding schema for the Product object:
    screenshot

Description of the problem:

This is happening because ApiExplorer surfaces an ApiResponseType.Type value of System.Void for the 201 response rather than Product. Although this is a little counter intuitive, it may be by design. However, if it's by design, then I think the following content should be updated in the ASP.NET Core docs:
https://docs.microsoft.com/en-us/aspnet/core/web-api/action-return-types?view=aspnetcore-2.1#actionresultt-type

ActionResult<T> offers the following benefits over the IActionResult type:

  • The ProducesResponseType attribute's Type property can be excluded

Version of Microsoft.AspNetCore.Mvc or Microsoft.AspNetCore.App or Microsoft.AspNetCore.All:

2.1.0

3 - Done 1 - Required S enhancement

Most helpful comment

All 8 comments

Hi @domaindrivendev.
The behavior you're observing here is by design. However, we still plan to add support for it in the upcoming 2.2 release. @pranavkm, do we have an issue tracking the swagger work to reference from here?

There's a PR out for this and it seems like a pretty decent fix. I asked the powers to be if we could consider this for a patch, but unfortunately this wouldn't meet the bar. We'll just get https://github.com/aspnet/Mvc/pull/7873 in for now.

If I may, I don't think the current behavior is by design according to multiple examples in the doc such as :

They explicitly states that removing the type in the ProducesResponseType whether it's a 200 or above will produce the right return type inferred from ActionResult.
It would be weird to go let the 2.1 as LTS with a feature documented that doesn't work as expected ;)

Even the sample here doesn't work as expected.
IMHO, something has to be corrected for 2.1: the docs or the code, it's up to you I don't own the product :)

@NatMarchand we're consider addressing this in a 2.1.x patch release (See https://github.com/aspnet/Mvc/issues/7875). We'll hold off on updating the docs until triage has had a chance to decide.

Summarizing some offline conversations I had with @rynowak

1) The behavior isn't specific to ActionResult<T>. You could get this as a result of returning regular POCO types too e.g. [ProducesResponseType(200)] public Person GetPerson() would have missing metadata for the response type. Inferring the response type is the correct thing to do in this scenarios too. I'd treat this as a regular ol' bug in ApiExplorer sans the need for a compat switch.

2) We might need to be smarter than what's currently in the PR, https://github.com/aspnet/Mvc/pull/7873, in the event multiple success status codes are returned from an action. Consider the shape of the action described here: https://developer.github.com/v3/repos/statistics:

Computing repository statistics is an expensive operation, so we try to return cached data whenever possible. If the data hasn't been cached when you query a repository's statistics, you'll receive a 202 response; a background job is also fired to start compiling these statistics. Give the job a few moments to complete, and then submit the request again. If the job has completed, that request will receive a 200 response with the statistics in the response body.

```C#
[ProducesResponseType(200)]
[ProducesResponseType(202)]
public async Task> GetStatistics(string repo, string owner)
{
var statsTask = Repo.GetStatisticsAsync(repo, owner);
if (stats.IsCompleted)
{
return await stats;
}

return Accepted();

}
```

In this event, the 200 is the only status code that would have a body. To allow for this, my suggestion is to only apply the return code if

a) The action has at most one 200-203 response type attribute
b) If multiple success response type attributes are found, apply it to specifically to the 200 status code (if present).

*Opting out: * There really isn't a good way to opt-out of this behavior short of writing a custom IApiResponseMetadataProvider attrbute. But it's essentially the same behavior as using the return type for actions without any attributes, so this change in behavior is OK.

cc @rynowak

Maybe we can only consider 200 and 201 ?
Those two seems to be the only case where the payload is meaningful.

Maybe we can only consider 200 and 201 ?

That sounds great.

Was this page helpful?
0 / 5 - 0 ratings