From 9c424b7b0263dd6c89eb730c7e36b2eb6e3be5e8 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Mon, 17 Sep 2018 14:56:06 -0700 Subject: [PATCH] Use content-type specified by ProducesAttribute if no formatter supports it This allows users to use `ProducesAttribute` to specify the content-type for action results such as FileStreamResult where the result determines the content type and the specified value is informational. Fixes https://github.com/aspnet/Mvc/issues/5701 --- .../ApiResponseTypeProvider.cs | 40 ++++++++++++-- .../ApiResponseTypeProviderTest.cs | 54 +++++++++++++++++-- .../ApiExplorerTest.cs | 51 +++++++++++++++--- .../Controllers/ApiExplorerApiController.cs | 6 ++- 4 files changed, 135 insertions(+), 16 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.ApiExplorer/ApiResponseTypeProvider.cs b/src/Microsoft.AspNetCore.Mvc.ApiExplorer/ApiResponseTypeProvider.cs index cd42b8a157..0761a7950a 100644 --- a/src/Microsoft.AspNetCore.Mvc.ApiExplorer/ApiResponseTypeProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.ApiExplorer/ApiResponseTypeProvider.cs @@ -135,12 +135,33 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer if (contentTypes.Count == 0) { + // None of the IApiResponseMetadataProvider specified a content type. This is common for actions that + // specify one or more ProducesResponseType but no ProducesAttribute. In this case, formatters will participate in conneg + // and respond to the incoming request. + // Querying IApiResponseTypeMetadataProvider.GetSupportedContentTypes with "null" should retrieve all supported + // content types that each formatter may respond in. contentTypes.Add((string)null); } + var responseTypes = results.Values; + CalculateResponseFormats(responseTypes, contentTypes); + return responseTypes; + } + + private void CalculateResponseFormats(ICollection responseTypes, MediaTypeCollection declaredContentTypes) + { var responseTypeMetadataProviders = _mvcOptions.OutputFormatters.OfType(); - foreach (var apiResponse in results.Values) + // Given the content-types that were declared for this action, determine the formatters that support the content-type for the given + // response type. + // 1. Responses that do not specify an type do not have any associated content-type. This usually is meant for status-code only responses such + // as return NotFound(); + // 2. When a type is specified, use GetSupportedContentTypes to expand wildcards and get the range of content-types formatters support. + // 3. When no formatter supports the specified content-type, use the user specified value as is. This is useful in actions where the user + // dictates the content-type. + // e.g. [Produces("application/pdf")] Action() => FileStream("somefile.pdf", "applicaiton/pdf"); + + foreach (var apiResponse in responseTypes) { var responseType = apiResponse.Type; if (responseType == null || responseType == typeof(void)) @@ -150,8 +171,10 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer apiResponse.ModelMetadata = _modelMetadataProvider.GetMetadataForType(responseType); - foreach (var contentType in contentTypes) + foreach (var contentType in declaredContentTypes) { + var isSupportedContentType = false; + foreach (var responseTypeMetadataProvider in responseTypeMetadataProviders) { var formatterSupportedContentTypes = responseTypeMetadataProvider.GetSupportedContentTypes( @@ -163,6 +186,8 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer continue; } + isSupportedContentType = true; + foreach (var formatterSupportedContentType in formatterSupportedContentTypes) { apiResponse.ApiResponseFormats.Add(new ApiResponseFormat @@ -172,10 +197,17 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer }); } } + + if (!isSupportedContentType && contentType != null) + { + // No output formatter was found that supports this content type. Add the user specified content type as-is to the result. + apiResponse.ApiResponseFormats.Add(new ApiResponseFormat + { + MediaType = contentType, + }); + } } } - - return results.Values; } private Type GetDeclaredReturnType(ControllerActionDescriptor action) diff --git a/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/ApiResponseTypeProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/ApiResponseTypeProviderTest.cs index 5ea1f29b9f..d990cdd2ff 100644 --- a/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/ApiResponseTypeProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/ApiResponseTypeProviderTest.cs @@ -45,7 +45,11 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer Assert.False(responseType.IsDefaultResponse); Assert.Collection( responseType.ApiResponseFormats, - format => Assert.Equal("application/json", format.MediaType)); + format => + { + Assert.Equal("application/json", format.MediaType); + Assert.IsType(format.Formatter); + }); }, responseType => { @@ -106,7 +110,11 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer Assert.False(responseType.IsDefaultResponse); Assert.Collection( responseType.ApiResponseFormats, - format => Assert.Equal("application/json", format.MediaType)); + format => + { + Assert.Equal("application/json", format.MediaType); + Assert.IsType(format.Formatter); + }); }, responseType => { @@ -115,7 +123,11 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer Assert.False(responseType.IsDefaultResponse); Assert.Collection( responseType.ApiResponseFormats, - format => Assert.Equal("application/json", format.MediaType)); + format => + { + Assert.Equal("application/json", format.MediaType); + Assert.IsType(format.Formatter); + }); }, responseType => { @@ -156,7 +168,11 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer Assert.False(responseType.IsDefaultResponse); Assert.Collection( responseType.ApiResponseFormats, - format => Assert.Equal("application/json", format.MediaType)); + format => + { + Assert.Equal("application/json", format.MediaType); + Assert.IsType(format.Formatter); + }); }, responseType => { @@ -663,6 +679,36 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer }); } + [Fact] + public void GetApiResponseTypes_UsesContentTypeWithoutWildCard_WhenNoFormatterSupportsIt() + { + // Arrange + var actionDescriptor = GetControllerActionDescriptor(typeof(TestController), nameof(TestController.GetUser)); + actionDescriptor.FilterDescriptors.Add(new FilterDescriptor(new ProducesAttribute("application/pdf"), FilterScope.Action)); + + var provider = GetProvider(); + + // Act + var result = provider.GetApiResponseTypes(actionDescriptor); + + // Assert + Assert.Collection( + result.OrderBy(r => r.StatusCode), + responseType => + { + Assert.Equal(200, responseType.StatusCode); + Assert.Equal(typeof(DerivedModel), responseType.Type); + Assert.False(responseType.IsDefaultResponse); + Assert.Collection( + responseType.ApiResponseFormats, + format => + { + Assert.Equal("application/pdf", format.MediaType); + Assert.Null(format.Formatter); + }); + }); + } + private static ApiResponseTypeProvider GetProvider() { var mvcOptions = new MvcOptions diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiExplorerTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiExplorerTest.cs index a4db6dd1ee..e333821046 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiExplorerTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiExplorerTest.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.IO; using System.Linq; using System.Net; using System.Net.Http; @@ -829,17 +830,27 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests // Assert var description = Assert.Single(result); var responseType = Assert.Single(description.SupportedResponseTypes); - Assert.Equal(1, responseType.ResponseFormats.Count); - - var responseFormat = responseType.ResponseFormats[0]; - Assert.Equal("application/hal+json", responseFormat.MediaType); - Assert.Equal(typeof(JsonOutputFormatter).FullName, responseFormat.FormatterType); + Assert.Collection( + responseType.ResponseFormats, + responseFormat => + { + Assert.Equal("application/hal+custom", responseFormat.MediaType); + Assert.Null(responseFormat.FormatterType); + }, + responseFormat => + { + Assert.Equal("application/hal+json", responseFormat.MediaType); + Assert.Equal(typeof(JsonOutputFormatter).FullName, responseFormat.FormatterType); + }); } [Fact] public async Task ApiExplorer_ResponseContentType_NoMatch() { - // Arrange & Act + // Arrange + var expectedMediaTypes = new[] { "application/custom", "text/hal+bson" }; + + // Act var response = await Client.GetAsync("http://localhost/ApiExplorerResponseContentType/NoMatch"); var body = await response.Content.ReadAsStringAsync(); @@ -848,7 +859,11 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests // Assert var description = Assert.Single(result); var responseType = Assert.Single(description.SupportedResponseTypes); - Assert.Empty(responseType.ResponseFormats); + + + Assert.Equal(typeof(Product).FullName, responseType.ResponseType); + Assert.Equal(200, responseType.StatusCode); + Assert.Equal(expectedMediaTypes, GetSortedMediaTypes(responseType)); } [ConditionalTheory] @@ -1147,6 +1162,28 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests Assert.Equal("multipart/form-data", requestFormat.MediaType); } + [Fact] + public async Task ApiBehavior_UsesContentTypeFromProducesAttribute_WhenNoFormatterSupportsIt() + { + // Arrange + var expectedMediaTypes = new[] { "application/pdf" }; + + // Act + var body = await Client.GetStringAsync("ApiExplorerApiController/ProducesWithUnsupportedContentType"); + var result = JsonConvert.DeserializeObject>(body); + + // Assert + var description = Assert.Single(result); + Assert.Collection( + description.SupportedResponseTypes.OrderBy(r => r.StatusCode), + responseType => + { + Assert.Equal(typeof(Stream).FullName, responseType.ResponseType); + Assert.Equal(200, responseType.StatusCode); + Assert.Equal(expectedMediaTypes, GetSortedMediaTypes(responseType)); + }); + } + [Fact] public Task ApiConvention_ForGetMethod_ReturningModel() => ApiConvention_ForGetMethod("GetProduct"); diff --git a/test/WebSites/ApiExplorerWebSite/Controllers/ApiExplorerApiController.cs b/test/WebSites/ApiExplorerWebSite/Controllers/ApiExplorerApiController.cs index d45f0bb07a..a32ebed154 100644 --- a/test/WebSites/ApiExplorerWebSite/Controllers/ApiExplorerApiController.cs +++ b/test/WebSites/ApiExplorerWebSite/Controllers/ApiExplorerApiController.cs @@ -1,8 +1,9 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using Microsoft.AspNetCore.Mvc; +using System.IO; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; namespace ApiExplorerWebSite { @@ -27,5 +28,8 @@ namespace ApiExplorerWebSite public void ActionWithFormFileCollectionParameter(IFormFileCollection formFile) { } + + [Produces("application/pdf", Type = typeof(Stream))] + public IActionResult ProducesWithUnsupportedContentType() => null; } } \ No newline at end of file