From ac4d6366b70b4adb3b18e07084c3eb4c01bf1161 Mon Sep 17 00:00:00 2001 From: Nathanael Marchand Date: Tue, 5 Jun 2018 14:24:42 +0200 Subject: [PATCH] Fix Api Explorer not returning type with ActionResult and no type in ProducesResponseTypeAttribute --- .../ApiResponseTypeProvider.cs | 13 +- .../ApiResponseTypeProviderTest.cs | 6 +- .../DefaultApiDescriptionProviderTest.cs | 210 ++++++++++++++++++ .../ApiExplorerTest.cs | 10 +- 4 files changed, 233 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.ApiExplorer/ApiResponseTypeProvider.cs b/src/Microsoft.AspNetCore.Mvc.ApiExplorer/ApiResponseTypeProvider.cs index e0953ca6e2..d6c7cd7661 100644 --- a/src/Microsoft.AspNetCore.Mvc.ApiExplorer/ApiResponseTypeProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.ApiExplorer/ApiResponseTypeProvider.cs @@ -86,13 +86,24 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer { metadataAttribute.SetContentTypes(contentTypes); - if (metadataAttribute.Type != null) + if (metadataAttribute.Type == typeof(void) && + type != null && + (metadataAttribute.StatusCode == StatusCodes.Status200OK || metadataAttribute.StatusCode == StatusCodes.Status201Created)) + { + // ProducesResponseTypeAttribute's constructor defaults to setting "Type" to void when no value is specified. + // In this event, use the action's return type for 200 or 201 status codes. This lets you decorate an action with a + // [ProducesResponseType(201)] instead of [ProducesResponseType(201, typeof(Person)] when typeof(Person) can be inferred + // from the return type. + objectTypes[metadataAttribute.StatusCode] = type; + } + else if (metadataAttribute.Type != null) { objectTypes[metadataAttribute.StatusCode] = metadataAttribute.Type; } } } + // Set the default status only when no status has already been set explicitly if (objectTypes.Count == 0 && type != null) { diff --git a/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/ApiResponseTypeProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/ApiResponseTypeProviderTest.cs index 99e862dcf4..7f3247deac 100644 --- a/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/ApiResponseTypeProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/ApiResponseTypeProviderTest.cs @@ -98,9 +98,11 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer responseType => { Assert.Equal(200, responseType.StatusCode); - Assert.Equal(typeof(void), responseType.Type); + Assert.Equal(typeof(BaseModel), responseType.Type); Assert.False(responseType.IsDefaultResponse); - Assert.Empty(responseType.ApiResponseFormats); + Assert.Collection( + responseType.ApiResponseFormats, + format => Assert.Equal("application/json", format.MediaType)); }, responseType => { diff --git a/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/DefaultApiDescriptionProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/DefaultApiDescriptionProviderTest.cs index b661be6034..5caceeaa02 100644 --- a/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/DefaultApiDescriptionProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/DefaultApiDescriptionProviderTest.cs @@ -665,6 +665,216 @@ namespace Microsoft.AspNetCore.Mvc.Description }); } + [Theory] + [InlineData(nameof(ReturnsActionResultOfProduct))] + [InlineData(nameof(ReturnsTaskOfActionResultOfProduct))] + public void GetApiDescription_ReturnsActionResultOfTWithProducesContentType( + string methodName) + { + // Arrange + var action = CreateActionDescriptor(methodName); + action.FilterDescriptors = new List() + { + // Since action is returning Void or Task, it does not make sense to provide a value for the + // 'Type' property to ProducesAttribute. But the same action could return other types of data + // based on runtime conditions. + new FilterDescriptor( + new ProducesAttribute("text/json", "application/json"), + FilterScope.Action), + new FilterDescriptor( + new ProducesResponseTypeAttribute(200), + FilterScope.Action), + new FilterDescriptor( + new ProducesResponseTypeAttribute(202), + FilterScope.Action), + new FilterDescriptor( + new ProducesResponseTypeAttribute(typeof(BadData), 400), + FilterScope.Action), + new FilterDescriptor( + new ProducesResponseTypeAttribute(typeof(ErrorDetails), 500), + FilterScope.Action) + }; + var expectedMediaTypes = new[] { "application/json", "text/json" }; + + // Act + var descriptions = GetApiDescriptions(action); + + // Assert + var description = Assert.Single(descriptions); + Assert.Equal(4, description.SupportedResponseTypes.Count); + + Assert.Collection( + description.SupportedResponseTypes.OrderBy(responseType => responseType.StatusCode), + responseType => + { + Assert.Equal(typeof(Product), responseType.Type); + Assert.Equal(200, responseType.StatusCode); + Assert.NotNull(responseType.ModelMetadata); + Assert.Equal(expectedMediaTypes, GetSortedMediaTypes(responseType)); + }, + responseType => + { + Assert.Equal(typeof(void), responseType.Type); + Assert.Equal(202, responseType.StatusCode); + Assert.Null(responseType.ModelMetadata); + Assert.Empty(GetSortedMediaTypes(responseType)); + }, + responseType => + { + Assert.Equal(typeof(BadData), responseType.Type); + Assert.Equal(400, responseType.StatusCode); + Assert.NotNull(responseType.ModelMetadata); + Assert.Equal(expectedMediaTypes, GetSortedMediaTypes(responseType)); + }, + responseType => + { + Assert.Equal(typeof(ErrorDetails), responseType.Type); + Assert.Equal(500, responseType.StatusCode); + Assert.NotNull(responseType.ModelMetadata); + Assert.Equal(expectedMediaTypes, GetSortedMediaTypes(responseType)); + }); + } + + [Theory] + [InlineData(nameof(ReturnsActionResultOfProduct))] + [InlineData(nameof(ReturnsTaskOfActionResultOfProduct))] + public void GetApiDescription_ReturnsActionResultOfTWithProducesContentType_ForStatusCode201( + string methodName) + { + // Arrange + var action = CreateActionDescriptor(methodName); + action.FilterDescriptors = new List() + { + // Since action is returning Void or Task, it does not make sense to provide a value for the + // 'Type' property to ProducesAttribute. But the same action could return other types of data + // based on runtime conditions. + new FilterDescriptor( + new ProducesAttribute("text/json", "application/json"), + FilterScope.Action), + new FilterDescriptor( + new ProducesResponseTypeAttribute(201), + FilterScope.Action), + new FilterDescriptor( + new ProducesResponseTypeAttribute(204), + FilterScope.Action), + new FilterDescriptor( + new ProducesResponseTypeAttribute(typeof(BadData), 400), + FilterScope.Action), + new FilterDescriptor( + new ProducesResponseTypeAttribute(typeof(ErrorDetails), 500), + FilterScope.Action) + }; + var expectedMediaTypes = new[] { "application/json", "text/json" }; + + // Act + var descriptions = GetApiDescriptions(action); + + // Assert + var description = Assert.Single(descriptions); + Assert.Equal(4, description.SupportedResponseTypes.Count); + + Assert.Collection( + description.SupportedResponseTypes.OrderBy(responseType => responseType.StatusCode), + responseType => + { + Assert.Equal(typeof(Product), responseType.Type); + Assert.Equal(201, responseType.StatusCode); + Assert.NotNull(responseType.ModelMetadata); + Assert.Equal(expectedMediaTypes, GetSortedMediaTypes(responseType)); + }, + responseType => + { + Assert.Equal(typeof(void), responseType.Type); + Assert.Equal(204, responseType.StatusCode); + Assert.Null(responseType.ModelMetadata); + Assert.Empty(GetSortedMediaTypes(responseType)); + }, + responseType => + { + Assert.Equal(typeof(BadData), responseType.Type); + Assert.Equal(400, responseType.StatusCode); + Assert.NotNull(responseType.ModelMetadata); + Assert.Equal(expectedMediaTypes, GetSortedMediaTypes(responseType)); + }, + responseType => + { + Assert.Equal(typeof(ErrorDetails), responseType.Type); + Assert.Equal(500, responseType.StatusCode); + Assert.NotNull(responseType.ModelMetadata); + Assert.Equal(expectedMediaTypes, GetSortedMediaTypes(responseType)); + }); + } + + [Theory] + [InlineData(nameof(ReturnsActionResultOfSequenceOfProducts))] + [InlineData(nameof(ReturnsTaskOfActionResultOfSequenceOfProducts))] + public void GetApiDescription_ReturnsActionResultOfSequenceOfTWithProducesContentType( + string methodName) + { + // Arrange + var action = CreateActionDescriptor(methodName); + action.FilterDescriptors = new List() + { + // Since action is returning Void or Task, it does not make sense to provide a value for the + // 'Type' property to ProducesAttribute. But the same action could return other types of data + // based on runtime conditions. + new FilterDescriptor( + new ProducesAttribute("text/json", "application/json"), + FilterScope.Action), + new FilterDescriptor( + new ProducesResponseTypeAttribute(200), + FilterScope.Action), + new FilterDescriptor( + new ProducesResponseTypeAttribute(201), + FilterScope.Action), + new FilterDescriptor( + new ProducesResponseTypeAttribute(typeof(BadData), 400), + FilterScope.Action), + new FilterDescriptor( + new ProducesResponseTypeAttribute(typeof(ErrorDetails), 500), + FilterScope.Action) + }; + var expectedMediaTypes = new[] { "application/json", "text/json" }; + + // Act + var descriptions = GetApiDescriptions(action); + + // Assert + var description = Assert.Single(descriptions); + Assert.Equal(4, description.SupportedResponseTypes.Count); + + Assert.Collection( + description.SupportedResponseTypes.OrderBy(responseType => responseType.StatusCode), + responseType => + { + Assert.Equal(typeof(IEnumerable), responseType.Type); + Assert.Equal(200, responseType.StatusCode); + Assert.NotNull(responseType.ModelMetadata); + Assert.Equal(expectedMediaTypes, GetSortedMediaTypes(responseType)); + }, + responseType => + { + Assert.Equal(typeof(IEnumerable), responseType.Type); + Assert.Equal(201, responseType.StatusCode); + Assert.NotNull(responseType.ModelMetadata); + Assert.Equal(expectedMediaTypes, GetSortedMediaTypes(responseType)); + }, + responseType => + { + Assert.Equal(typeof(BadData), responseType.Type); + Assert.Equal(400, responseType.StatusCode); + Assert.NotNull(responseType.ModelMetadata); + Assert.Equal(expectedMediaTypes, GetSortedMediaTypes(responseType)); + }, + responseType => + { + Assert.Equal(typeof(ErrorDetails), responseType.Type); + Assert.Equal(500, responseType.StatusCode); + Assert.NotNull(responseType.ModelMetadata); + Assert.Equal(expectedMediaTypes, GetSortedMediaTypes(responseType)); + }); + } + [Theory] [InlineData(nameof(ReturnsVoid))] [InlineData(nameof(ReturnsTask))] diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiExplorerTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiExplorerTest.cs index 7490fe2f7f..e575967651 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiExplorerTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiExplorerTest.cs @@ -7,6 +7,7 @@ using System.Linq; using System.Net; using System.Net.Http; using System.Threading.Tasks; +using ApiExplorerWebSite; using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Testing.xunit; @@ -1156,6 +1157,9 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests private async Task ApiConvention_ForGetMethod(string action) { + // Arrange + var expectedMediaTypes = new[] { "application/json", "application/xml", "text/json", "text/xml" }; + // Act var response = await Client.GetStringAsync( $"ApiExplorerResponseTypeWithApiConventionController/{action}"); @@ -1168,9 +1172,9 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests description.SupportedResponseTypes.OrderBy(r => r.StatusCode), responseType => { - Assert.Equal(typeof(void).FullName, responseType.ResponseType); + Assert.Equal(typeof(Product).FullName, responseType.ResponseType); Assert.Equal(200, responseType.StatusCode); - Assert.Empty(responseType.ResponseFormats); + Assert.Equal(expectedMediaTypes, GetSortedMediaTypes(responseType)); }, responseType => { @@ -1198,7 +1202,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests description.SupportedResponseTypes.OrderBy(r => r.StatusCode), responseType => { - Assert.Equal(typeof(IEnumerable).FullName, responseType.ResponseType); + Assert.Equal(typeof(IEnumerable).FullName, responseType.ResponseType); Assert.Equal(200, responseType.StatusCode); var actualMediaTypes = responseType.ResponseFormats.Select(r => r.MediaType).OrderBy(r => r); Assert.Equal(expectedMediaTypes, actualMediaTypes);