Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Suppress default status code response type in api descriptions when e… #4831

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -358,13 +358,6 @@ private IReadOnlyList<ApiResponseType> GetApiResponseTypes(
// Build list of all possible return types (and status codes) for an action.
var objectTypes = new Dictionary<int, Type>();

if (type != null && type != typeof(void))
{
// This return type can be overriden by any response metadata
// attributes later if the user wishes to.
objectTypes[StatusCodes.Status200OK] = type;
}

// Get the content type that the action explicitly set to support.
// Walk through all 'filter' attributes in order, and allow each one to see or override
// the results of the previous ones. This is similar to the execution path for content-negotiation.
Expand All @@ -382,6 +375,14 @@ private IReadOnlyList<ApiResponseType> GetApiResponseTypes(
}
}

// Set the default status only when no status has already been set explicitly
if (objectTypes.Count == 0
&& type != null
&& type != typeof(void))
{
objectTypes[StatusCodes.Status200OK] = type;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this all seems contingent on the objectTypes what will happen by default if the method returns void

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if an action returns void, then it would by default not have any apidescriptions...metadata needs to be explicitly decorated ex: https://github.com/aspnet/Mvc/blob/dev/test/WebSites/ApiExplorerWebSite/Controllers/ApiExplorerResponseTypeWithoutAttributeController.cs#L14

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you open a bug for this? I don't want to bloat this PR, but it seems wrong that we infer 'default 200' when you a DTO as your return type and 'nothing' when you have void as your return type.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure: #4838


if (contentTypes.Count == 0)
{
contentTypes.Add((string)null);
Expand Down
127 changes: 127 additions & 0 deletions test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiExplorerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,133 @@ public async Task ApiExplorer_ResponseType_KnownWithAttribute(string action, str
Assert.Equal("application/json", responseFormat.MediaType);
}

[Fact]
public async Task ExplicitResponseTypeDecoration_SuppressesDefaultStatus()
{
// Arrange
var type1 = typeof(ApiExplorerWebSite.Product).FullName;
var type2 = typeof(ModelStateDictionary).FullName;
var expectedMediaTypes = new[] { "application/json", "text/json", "application/xml", "text/xml" };

// Act
var response = await Client.GetAsync(
"http://localhost/ApiExplorerResponseTypeWithAttribute/CreateProductWithDefaultResponseContentTypes");

var body = await response.Content.ReadAsStringAsync();
var result = JsonConvert.DeserializeObject<List<ApiExplorerData>>(body);

// Assert
var description = Assert.Single(result);
Assert.Equal(2, description.SupportedResponseTypes.Count);
var responseType = description.SupportedResponseTypes[0];
Assert.Equal(type1, responseType.ResponseType);
Assert.Equal(201, responseType.StatusCode);
Assert.Equal(
expectedMediaTypes,
responseType.ResponseFormats.Select(responseFormat => responseFormat.MediaType).ToArray());
responseType = description.SupportedResponseTypes[1];
Assert.Equal(type2, responseType.ResponseType);
Assert.Equal(400, responseType.StatusCode);
Assert.Equal(
expectedMediaTypes,
responseType.ResponseFormats.Select(responseFormat => responseFormat.MediaType).ToArray());
}

[Fact]
public async Task ExplicitResponseTypeDecoration_SuppressesDefaultStatus_AlsoHonorsProducesContentTypes()
{
// Arrange
var type1 = typeof(ApiExplorerWebSite.Product).FullName;
var type2 = typeof(ModelStateDictionary).FullName;
var expectedMediaTypes = new[] { "text/xml" };

// Act
var response = await Client.GetAsync(
"http://localhost/ApiExplorerResponseTypeWithAttribute/CreateProductWithLimitedResponseContentTypes");

var body = await response.Content.ReadAsStringAsync();
var result = JsonConvert.DeserializeObject<List<ApiExplorerData>>(body);

// Assert
var description = Assert.Single(result);
Assert.Equal(2, description.SupportedResponseTypes.Count);
var responseType = description.SupportedResponseTypes[0];
Assert.Equal(type1, responseType.ResponseType);
Assert.Equal(201, responseType.StatusCode);
Assert.Equal(
expectedMediaTypes,
responseType.ResponseFormats.Select(responseFormat => responseFormat.MediaType).ToArray());
responseType = description.SupportedResponseTypes[1];
Assert.Equal(type2, responseType.ResponseType);
Assert.Equal(400, responseType.StatusCode);
Assert.Equal(
expectedMediaTypes,
responseType.ResponseFormats.Select(responseFormat => responseFormat.MediaType).ToArray());
}

[Fact]
public async Task ExplicitResponseTypeDecoration_WithExplicitDefaultStatus()
{
// Arrange
var type1 = typeof(ApiExplorerWebSite.Product).FullName;
var type2 = typeof(ModelStateDictionary).FullName;
var expectedMediaTypes = new[] { "application/json", "text/json", "application/xml", "text/xml" };

// Act
var response = await Client.GetAsync(
"http://localhost/ApiExplorerResponseTypeWithAttribute/UpdateProductWithDefaultResponseContentTypes");

var body = await response.Content.ReadAsStringAsync();
var result = JsonConvert.DeserializeObject<List<ApiExplorerData>>(body);

// Assert
var description = Assert.Single(result);
Assert.Equal(2, description.SupportedResponseTypes.Count);
var responseType = description.SupportedResponseTypes[0];
Assert.Equal(type1, responseType.ResponseType);
Assert.Equal(200, responseType.StatusCode);
Assert.Equal(
expectedMediaTypes,
responseType.ResponseFormats.Select(responseFormat => responseFormat.MediaType).ToArray());
responseType = description.SupportedResponseTypes[1];
Assert.Equal(type2, responseType.ResponseType);
Assert.Equal(400, responseType.StatusCode);
Assert.Equal(
expectedMediaTypes,
responseType.ResponseFormats.Select(responseFormat => responseFormat.MediaType).ToArray());
}

[Fact]
public async Task ExplicitResponseTypeDecoration_WithExplicitDefaultStatus_SpecifiedViaProducesAttribute()
{
// Arrange
var type1 = typeof(ApiExplorerWebSite.Product).FullName;
var type2 = typeof(ModelStateDictionary).FullName;
var expectedMediaTypes = new[] { "text/xml" };

// Act
var response = await Client.GetAsync(
"http://localhost/ApiExplorerResponseTypeWithAttribute/UpdateProductWithLimitedResponseContentTypes");

var body = await response.Content.ReadAsStringAsync();
var result = JsonConvert.DeserializeObject<List<ApiExplorerData>>(body);

// Assert
var description = Assert.Single(result);
Assert.Equal(2, description.SupportedResponseTypes.Count);
var responseType = description.SupportedResponseTypes[0];
Assert.Equal(type1, responseType.ResponseType);
Assert.Equal(200, responseType.StatusCode);
Assert.Equal(
expectedMediaTypes,
responseType.ResponseFormats.Select(responseFormat => responseFormat.MediaType).ToArray());
responseType = description.SupportedResponseTypes[1];
Assert.Equal(type2, responseType.ResponseType);
Assert.Equal(400, responseType.StatusCode);
Assert.Equal(
expectedMediaTypes,
responseType.ResponseFormats.Select(responseFormat => responseFormat.MediaType).ToArray());
}
[Fact]
public async Task ApiExplorer_ResponseType_InheritingFromController()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@

using System.Threading.Tasks;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.ModelBinding;

namespace ApiExplorerWebSite
{
[Route("ApiExplorerResponseTypeWithAttribute/[Action]")]
[Route("[controller]/[Action]")]
public class ApiExplorerResponseTypeWithAttributeController : Controller
{
[HttpGet]
Expand Down Expand Up @@ -42,5 +43,34 @@ public Product GetProduct()
{
return null;
}

[ProducesResponseType(typeof(Product), 201)]
[ProducesResponseType(typeof(ModelStateDictionary), 400)]
public Product CreateProductWithDefaultResponseContentTypes(Product product)
{
return null;
}

[Produces("text/xml")] // Has status code as 200 but is not applied as it does not set 'Type'
[ProducesResponseType(typeof(Product), 201)]
[ProducesResponseType(typeof(ModelStateDictionary), 400)]
public Product CreateProductWithLimitedResponseContentTypes(Product product)
{
return null;
}

[ProducesResponseType(typeof(Product), 200)]
[ProducesResponseType(typeof(ModelStateDictionary), 400)]
public Product UpdateProductWithDefaultResponseContentTypes(Product product)
{
return null;
}

[Produces("text/xml", Type = typeof(Product))] // Has status code as 200
[ProducesResponseType(typeof(ModelStateDictionary), 400)]
public Product UpdateProductWithLimitedResponseContentTypes(Product product)
{
return null;
}
}
}