From 82f7f2aab88f1b9af15f60df3830ed327b2d5a89 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 --- .../DefaultApiDescriptionProvider.cs | 13 +- .../DefaultApiDescriptionProviderTest.cs | 210 ++++++++++++++++++ 2 files changed, 221 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.ApiExplorer/DefaultApiDescriptionProvider.cs b/src/Microsoft.AspNetCore.Mvc.ApiExplorer/DefaultApiDescriptionProvider.cs index 5da3f8a1cb..2ed34e730a 100644 --- a/src/Microsoft.AspNetCore.Mvc.ApiExplorer/DefaultApiDescriptionProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.ApiExplorer/DefaultApiDescriptionProvider.cs @@ -424,8 +424,17 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer foreach (var metadataAttribute in responseMetadataAttributes) { 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; } diff --git a/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/DefaultApiDescriptionProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/DefaultApiDescriptionProviderTest.cs index d3f0110711..c55d427d9d 100644 --- a/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/DefaultApiDescriptionProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/DefaultApiDescriptionProviderTest.cs @@ -664,6 +664,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))]