From f7da3503d6360c7b6996871818b3873f5ed1a8d7 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Tue, 18 Sep 2018 11:25:53 -0700 Subject: [PATCH 1/3] Allow Implicit 200 status codes to match Ok result --- .../DeclaredApiResponseMetadata.cs | 10 +- .../ApiConventionAnalyzerIntegrationTest.cs | 4 + .../DeclaredApiResponseMetadataTest.cs | 169 ++++++++++++++++++ .../SymbolApiResponseMetadataProviderTest.cs | 91 ++++++++++ ...sAreReturned_ForOkResultReturningAction.cs | 19 ++ .../TryGetActualResponseMetadataTests.cs | 38 ++++ 6 files changed, 330 insertions(+), 1 deletion(-) create mode 100644 test/Mvc.Api.Analyzers.Test/DeclaredApiResponseMetadataTest.cs create mode 100644 test/Mvc.Api.Analyzers.Test/TestFiles/ApiConventionAnalyzerIntegrationTest/NoDiagnosticsAreReturned_ForOkResultReturningAction.cs create mode 100644 test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/TryGetActualResponseMetadataTests.cs diff --git a/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/DeclaredApiResponseMetadata.cs b/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/DeclaredApiResponseMetadata.cs index a45b1ed573..30819931d0 100644 --- a/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/DeclaredApiResponseMetadata.cs +++ b/src/Microsoft.AspNetCore.Mvc.Api.Analyzers/DeclaredApiResponseMetadata.cs @@ -9,7 +9,7 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers internal readonly struct DeclaredApiResponseMetadata { public static DeclaredApiResponseMetadata ImplicitResponse { get; } = - new DeclaredApiResponseMetadata(statusCode: 0, attributeData: null, attributeSource: null, @implicit: true, @default: false); + new DeclaredApiResponseMetadata(statusCode: 200, attributeData: null, attributeSource: null, @implicit: true, @default: false); public static DeclaredApiResponseMetadata ForProducesResponseType(int statusCode, AttributeData attributeData, IMethodSymbol attributeSource) { @@ -41,8 +41,16 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers public IMethodSymbol AttributeSource { get; } + /// + /// True if this is the implicit 200 associated with an + /// action specifying no metadata. + /// public bool IsImplicit { get; } + /// + /// True if this is from a ProducesDefaultResponseTypeAttribute. + /// Matches all failure (400 and above) status codes. + /// public bool IsDefault { get; } internal static bool Contains(IList declaredApiResponseMetadata, ActualApiResponseMetadata actualMetadata) diff --git a/test/Mvc.Api.Analyzers.Test/ApiConventionAnalyzerIntegrationTest.cs b/test/Mvc.Api.Analyzers.Test/ApiConventionAnalyzerIntegrationTest.cs index f611c8d421..5e68cdbe54 100644 --- a/test/Mvc.Api.Analyzers.Test/ApiConventionAnalyzerIntegrationTest.cs +++ b/test/Mvc.Api.Analyzers.Test/ApiConventionAnalyzerIntegrationTest.cs @@ -26,6 +26,10 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers public Task NoDiagnosticsAreReturned_ForApiController_WithAllDocumentedStatusCodes() => RunNoDiagnosticsAreReturned(); + [Fact] + public Task NoDiagnosticsAreReturned_ForOkResultReturningAction() + => RunNoDiagnosticsAreReturned(); + [Fact] public Task NoDiagnosticsAreReturned_ForApiController_IfStatusCodesCannotBeInferred() => RunNoDiagnosticsAreReturned(); diff --git a/test/Mvc.Api.Analyzers.Test/DeclaredApiResponseMetadataTest.cs b/test/Mvc.Api.Analyzers.Test/DeclaredApiResponseMetadataTest.cs new file mode 100644 index 0000000000..fcfc65ebef --- /dev/null +++ b/test/Mvc.Api.Analyzers.Test/DeclaredApiResponseMetadataTest.cs @@ -0,0 +1,169 @@ +// 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 System.Collections.Generic; +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Moq; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.Api.Analyzers +{ + public class DeclaredApiResponseMetadataTest + { + private readonly ReturnStatementSyntax ReturnStatement = SyntaxFactory.ReturnStatement(); + private readonly AttributeData AttributeData = new TestAttributeData(); + + [Fact] + public void Matches_ReturnsTrue_IfDeclaredMetadataIsImplicit_AndActualMetadataIsDefaultResponse() + { + // Arrange + var declaredMetadata = DeclaredApiResponseMetadata.ImplicitResponse; + var actualMetadata = new ActualApiResponseMetadata(ReturnStatement); + + // Act + var matches = declaredMetadata.Matches(actualMetadata); + + // Assert + Assert.True(matches); + } + + [Fact] + public void Matches_ReturnsTrue_IfDeclaredMetadataIsImplicit_AndActualMetadataReturns200() + { + // Arrange + var declaredMetadata = DeclaredApiResponseMetadata.ImplicitResponse; + var actualMetadata = new ActualApiResponseMetadata(ReturnStatement, 200); + + // Act + var matches = declaredMetadata.Matches(actualMetadata); + + // Assert + Assert.True(matches); + } + + [Fact] + public void Matches_ReturnsTrue_IfDeclaredMetadataIs200_AndActualMetadataIsDefaultResponse() + { + // Arrange + var declaredMetadata = DeclaredApiResponseMetadata.ForProducesResponseType(200, AttributeData, Mock.Of()); + var actualMetadata = new ActualApiResponseMetadata(ReturnStatement); + + // Act + var matches = declaredMetadata.Matches(actualMetadata); + + // Assert + Assert.True(matches); + } + + /// + /// [ProducesResponseType(201)] + /// public IActionResult SomeAction => new Model(); + /// + [Fact] + public void Matches_ReturnsTrue_IfDeclaredMetadataIs201_AndActualMetadataIsDefault() + { + // Arrange + var declaredMetadata = DeclaredApiResponseMetadata.ForProducesResponseType(201, AttributeData, Mock.Of()); + var actualMetadata = new ActualApiResponseMetadata(ReturnStatement); + + // Act + var matches = declaredMetadata.Matches(actualMetadata); + + // Assert + Assert.True(matches); + } + + /// + /// [ProducesResponseType(201)] + /// public IActionResult SomeAction => Ok(new Model()); + /// + [Fact] + public void Matches_ReturnsFalse_IfDeclaredMetadataIs201_AndActualMetadataIs200() + { + // Arrange + var declaredMetadata = DeclaredApiResponseMetadata.ForProducesResponseType(201, AttributeData, Mock.Of()); + var actualMetadata = new ActualApiResponseMetadata(ReturnStatement, 200); + + // Act + var matches = declaredMetadata.Matches(actualMetadata); + + // Assert + Assert.False(matches); + } + + [Fact] + public void Matches_ReturnsTrue_IfDeclaredMetadataAndActualMetadataHaveSameStatusCode() + { + // Arrange + var declaredMetadata = DeclaredApiResponseMetadata.ForProducesResponseType(302, AttributeData, Mock.Of()); + var actualMetadata = new ActualApiResponseMetadata(ReturnStatement, 302); + + // Act + var matches = declaredMetadata.Matches(actualMetadata); + + // Assert + Assert.True(matches); + } + + [Theory] + [InlineData(400)] + [InlineData(409)] + [InlineData(500)] + public void Matches_ReturnsTrue_IfDeclaredMetadataIsDefault_AndActualMetadataIsErrorStatusCode(int actualStatusCode) + { + // Arrange + var declaredMetadata = DeclaredApiResponseMetadata.ForProducesDefaultResponse(AttributeData, Mock.Of()); + var actualMetadata = new ActualApiResponseMetadata(ReturnStatement, actualStatusCode); + + // Act + var matches = declaredMetadata.Matches(actualMetadata); + + // Assert + Assert.True(matches); + } + + [Fact] + public void Matches_ReturnsFalse_IfDeclaredMetadataIsDefault_AndActualMetadataIsNotErrorStatusCode() + { + // Arrange + var declaredMetadata = DeclaredApiResponseMetadata.ForProducesDefaultResponse(AttributeData, Mock.Of()); + var actualMetadata = new ActualApiResponseMetadata(ReturnStatement, 204); + + // Act + var matches = declaredMetadata.Matches(actualMetadata); + + // Assert + Assert.False(matches); + } + + [Fact] + public void Matches_ReturnsFalse_IfDeclaredMetadataIsDefault_AndActualMetadataIsDefaultResponse() + { + // Arrange + var declaredMetadata = DeclaredApiResponseMetadata.ForProducesDefaultResponse(AttributeData, Mock.Of()); + var actualMetadata = new ActualApiResponseMetadata(ReturnStatement); + + // Act + var matches = declaredMetadata.Matches(actualMetadata); + + // Assert + Assert.False(matches); + } + + private class TestAttributeData : AttributeData + { + protected override INamedTypeSymbol CommonAttributeClass => throw new System.NotImplementedException(); + + protected override IMethodSymbol CommonAttributeConstructor => throw new System.NotImplementedException(); + + protected override SyntaxReference CommonApplicationSyntaxReference => throw new System.NotImplementedException(); + + protected override ImmutableArray CommonConstructorArguments => throw new System.NotImplementedException(); + + protected override ImmutableArray> CommonNamedArguments => throw new System.NotImplementedException(); + } + } +} diff --git a/test/Mvc.Api.Analyzers.Test/SymbolApiResponseMetadataProviderTest.cs b/test/Mvc.Api.Analyzers.Test/SymbolApiResponseMetadataProviderTest.cs index 483901ffa1..c417a199fe 100644 --- a/test/Mvc.Api.Analyzers.Test/SymbolApiResponseMetadataProviderTest.cs +++ b/test/Mvc.Api.Analyzers.Test/SymbolApiResponseMetadataProviderTest.cs @@ -1,11 +1,13 @@ // 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 System.Collections.Generic; using System.Linq; using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Analyzer.Testing; +using Microsoft.AspNetCore.Mvc.Api.Analyzers.TestFiles.SymbolApiResponseMetadataProviderTest; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp.Syntax; using Xunit; @@ -433,6 +435,95 @@ namespace Microsoft.AspNetCore.Mvc.Api.Analyzers Assert.True(actualResponseMetadata.Value.IsDefaultResponse); } + [Fact] + public async Task TryGetActualResponseMetadata_ActionWithActionResultOfTReturningOkResult() + { + // Arrange + var typeName = typeof(TryGetActualResponseMetadataController).FullName; + var methodName = nameof(TryGetActualResponseMetadataController.ActionWithActionResultOfTReturningOkResult); + + // Act + var (success, responseMetadatas, _) = await TryGetActualResponseMetadata(typeName, methodName); + + // Assert + Assert.True(success); + Assert.Collection( + responseMetadatas, + metadata => + { + Assert.False(metadata.IsDefaultResponse); + Assert.Equal(200, metadata.StatusCode); + }); + } + + [Fact] + public async Task TryGetActualResponseMetadata_ActionWithActionResultOfTReturningModel() + { + // Arrange + var typeName = typeof(TryGetActualResponseMetadataController).FullName; + var methodName = nameof(TryGetActualResponseMetadataController.ActionWithActionResultOfTReturningModel); + + // Act + var (success, responseMetadatas, _) = await TryGetActualResponseMetadata(typeName, methodName); + + // Assert + Assert.True(success); + Assert.Collection( + responseMetadatas, + metadata => + { + Assert.True(metadata.IsDefaultResponse); + }); + } + + [Fact] + public async Task TryGetActualResponseMetadata_ActionReturningNotFoundAndModel() + { + // Arrange + var typeName = typeof(TryGetActualResponseMetadataController).FullName; + var methodName = nameof(TryGetActualResponseMetadataController.ActionReturningNotFoundAndModel); + + // Act + var (success, responseMetadatas, testSource) = await TryGetActualResponseMetadata(typeName, methodName); + + // Assert + Assert.True(success); + Assert.Collection( + responseMetadatas, + metadata => + { + Assert.False(metadata.IsDefaultResponse); + Assert.Equal(204, metadata.StatusCode); + AnalyzerAssert.DiagnosticLocation(testSource.MarkerLocations["MM1"], metadata.ReturnStatement.GetLocation()); + + }, + metadata => + { + Assert.True(metadata.IsDefaultResponse); + AnalyzerAssert.DiagnosticLocation(testSource.MarkerLocations["MM2"], metadata.ReturnStatement.GetLocation()); + }); + } + + private async Task<(bool result, IList responseMetadatas, TestSource testSource)> TryGetActualResponseMetadata(string typeName, string methodName) + { + var testSource = MvcTestSource.Read(GetType().Name, "TryGetActualResponseMetadataTests"); + var project = DiagnosticProject.Create(GetType().Assembly, new[] { testSource.Source }); + + var compilation = await GetCompilation("TryGetActualResponseMetadataTests"); + + var type = compilation.GetTypeByMetadataName(typeName); + var method = (IMethodSymbol)type.GetMembers(methodName).First(); + var symbolCache = new ApiControllerSymbolCache(compilation); + + var syntaxTree = method.DeclaringSyntaxReferences[0].SyntaxTree; + var methodSyntax = (MethodDeclarationSyntax)syntaxTree.GetRoot().FindNode(method.Locations[0].SourceSpan); + var semanticModel = compilation.GetSemanticModel(syntaxTree); + + var result = SymbolApiResponseMetadataProvider.TryGetActualResponseMetadata(symbolCache, semanticModel, methodSyntax, CancellationToken.None, out var responseMetadatas); + + return (result, responseMetadatas, testSource); + } + private async Task RunInspectReturnStatementSyntax([CallerMemberName]string test = null) { // Arrange diff --git a/test/Mvc.Api.Analyzers.Test/TestFiles/ApiConventionAnalyzerIntegrationTest/NoDiagnosticsAreReturned_ForOkResultReturningAction.cs b/test/Mvc.Api.Analyzers.Test/TestFiles/ApiConventionAnalyzerIntegrationTest/NoDiagnosticsAreReturned_ForOkResultReturningAction.cs new file mode 100644 index 0000000000..e5ae8feb2c --- /dev/null +++ b/test/Mvc.Api.Analyzers.Test/TestFiles/ApiConventionAnalyzerIntegrationTest/NoDiagnosticsAreReturned_ForOkResultReturningAction.cs @@ -0,0 +1,19 @@ +using System.Collections.Generic; +using System.Threading.Tasks; + +namespace Microsoft.AspNetCore.Mvc.Api.Analyzers +{ + [ApiController] + public class NoDiagnosticsAreReturned_ForOkResultReturningAction : ControllerBase + { + public async Task>> Action() + { + await Task.Yield(); + var models = new List(); + + return Ok(models); + } + } + + public class NoDiagnosticsAreReturned_ForOkResultReturningActionModel { } +} diff --git a/test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/TryGetActualResponseMetadataTests.cs b/test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/TryGetActualResponseMetadataTests.cs new file mode 100644 index 0000000000..38e4bc1a81 --- /dev/null +++ b/test/Mvc.Api.Analyzers.Test/TestFiles/SymbolApiResponseMetadataProviderTest/TryGetActualResponseMetadataTests.cs @@ -0,0 +1,38 @@ +using System.Collections.Generic; +using System.Threading.Tasks; + +namespace Microsoft.AspNetCore.Mvc.Api.Analyzers.TestFiles.SymbolApiResponseMetadataProviderTest +{ + public class TryGetActualResponseMetadataController : ControllerBase + { + public async Task>> ActionWithActionResultOfTReturningOkResult() + { + await Task.Yield(); + var models = new List(); + + return Ok(models); + } + + public async Task>> ActionWithActionResultOfTReturningModel() + { + await Task.Yield(); + var models = new List(); + + return models; + } + + public async Task> ActionReturningNotFoundAndModel(int id) + { + await Task.Yield(); + + if (id == 0) + { + /*MM1*/return NoContent(); + } + + /*MM2*/return new TryGetActualResponseMetadataModel(); + } + } + + public class TryGetActualResponseMetadataModel { } +} From 9c424b7b0263dd6c89eb730c7e36b2eb6e3be5e8 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Mon, 17 Sep 2018 14:56:06 -0700 Subject: [PATCH 2/3] 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 From 5c4c7467975a66cc5c1ee2b0562727b0da826a53 Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Wed, 19 Sep 2018 21:55:23 -0700 Subject: [PATCH 3/3] Reaction PR from routing rename --- build/dependencies.props | 4 ++-- .../ApplicationModels/AttributeRouteModel.cs | 4 ++-- .../RouteTokenTransformerConvention.cs | 12 ++++++------ .../Internal/ControllerActionDescriptorBuilder.cs | 4 ++-- .../Internal/MvcEndpointDataSource.cs | 4 ++-- .../RouteTokenTransformerConventionTest.cs | 12 ++++++------ .../Internal/MvcEndpointDataSourceTests.cs | 6 +++--- .../ControllerRouteTokenTransformerConvention.cs | 6 +++--- .../RoutingWebSite/TestParameterTransformer.cs | 6 +++--- 9 files changed, 29 insertions(+), 29 deletions(-) diff --git a/build/dependencies.props b/build/dependencies.props index 8fed603b93..22824154bd 100644 --- a/build/dependencies.props +++ b/build/dependencies.props @@ -48,8 +48,8 @@ 2.2.0-preview3-35252 2.2.0-preview3-35252 2.2.0-preview3-35252 - 2.2.0-preview3-35252 - 2.2.0-preview3-35252 + 2.2.0-a-preview3-outbound-parameter-tranformer-16996 + 2.2.0-a-preview3-outbound-parameter-tranformer-16996 2.2.0-preview3-35252 2.2.0-preview3-35252 2.2.0-preview3-35252 diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/AttributeRouteModel.cs b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/AttributeRouteModel.cs index 3283a630a0..4623d67090 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/AttributeRouteModel.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/AttributeRouteModel.cs @@ -225,7 +225,7 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels return ReplaceTokens(template, values, routeTokenTransformer: null); } - public static string ReplaceTokens(string template, IDictionary values, IParameterTransformer routeTokenTransformer) + public static string ReplaceTokens(string template, IDictionary values, IOutboundParameterTransformer routeTokenTransformer) { var builder = new StringBuilder(); var state = TemplateParserState.Plaintext; @@ -379,7 +379,7 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels if (routeTokenTransformer != null) { - value = routeTokenTransformer.Transform(value); + value = routeTokenTransformer.TransformOutbound(value); } builder.Append(value); diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/RouteTokenTransformerConvention.cs b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/RouteTokenTransformerConvention.cs index d7645077af..c83ba6cc47 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/RouteTokenTransformerConvention.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/RouteTokenTransformerConvention.cs @@ -8,17 +8,17 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels { /// /// An that sets attribute routing token replacement - /// to use the specified on selectors. + /// to use the specified on selectors. /// public class RouteTokenTransformerConvention : IActionModelConvention { - private readonly IParameterTransformer _parameterTransformer; + private readonly IOutboundParameterTransformer _parameterTransformer; /// - /// Creates a new instance of with the specified . + /// Creates a new instance of with the specified . /// - /// The to use with attribute routing token replacement. - public RouteTokenTransformerConvention(IParameterTransformer parameterTransformer) + /// The to use with attribute routing token replacement. + public RouteTokenTransformerConvention(IOutboundParameterTransformer parameterTransformer) { if (parameterTransformer == null) { @@ -32,7 +32,7 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels { if (ShouldApply(action)) { - action.Properties[typeof(IParameterTransformer)] = _parameterTransformer; + action.Properties[typeof(IOutboundParameterTransformer)] = _parameterTransformer; } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionDescriptorBuilder.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionDescriptorBuilder.cs index 174457bbcc..21ec3aee6b 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionDescriptorBuilder.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionDescriptorBuilder.cs @@ -390,8 +390,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal { try { - actionDescriptor.Properties.TryGetValue(typeof(IParameterTransformer), out var transformer); - var routeTokenTransformer = transformer as IParameterTransformer; + actionDescriptor.Properties.TryGetValue(typeof(IOutboundParameterTransformer), out var transformer); + var routeTokenTransformer = transformer as IOutboundParameterTransformer; actionDescriptor.AttributeRouteInfo.Template = AttributeRouteModel.ReplaceTokens( actionDescriptor.AttributeRouteInfo.Template, diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcEndpointDataSource.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcEndpointDataSource.cs index 0c7a0f74f0..0a1aa08e24 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcEndpointDataSource.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcEndpointDataSource.cs @@ -277,9 +277,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Use the first transformer policy for (var k = 0; k < parameterPolicies.Count; k++) { - if (parameterPolicies[k] is IParameterTransformer parameterTransformer) + if (parameterPolicies[k] is IOutboundParameterTransformer parameterTransformer) { - parameterRouteValue = parameterTransformer.Transform(parameterRouteValue); + parameterRouteValue = parameterTransformer.TransformOutbound(parameterRouteValue); break; } } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModels/RouteTokenTransformerConventionTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModels/RouteTokenTransformerConventionTest.cs index aabefa456e..807bb4d41a 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModels/RouteTokenTransformerConventionTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModels/RouteTokenTransformerConventionTest.cs @@ -47,7 +47,7 @@ namespace Microsoft.AspNetCore.Mvc.Test.ApplicationModels convention.Apply(model); // Assert - Assert.True(model.Properties.TryGetValue(typeof(IParameterTransformer), out var routeTokenTransformer)); + Assert.True(model.Properties.TryGetValue(typeof(IOutboundParameterTransformer), out var routeTokenTransformer)); Assert.Equal(transformer, routeTokenTransformer); } @@ -68,7 +68,7 @@ namespace Microsoft.AspNetCore.Mvc.Test.ApplicationModels convention.Apply(model); // Assert - Assert.False(model.Properties.TryGetValue(typeof(IParameterTransformer), out _)); + Assert.False(model.Properties.TryGetValue(typeof(IOutboundParameterTransformer), out _)); } private MethodInfo GetMethodInfo() @@ -76,17 +76,17 @@ namespace Microsoft.AspNetCore.Mvc.Test.ApplicationModels return typeof(RouteTokenTransformerConventionTest).GetMethod(nameof(GetMethodInfo), BindingFlags.NonPublic | BindingFlags.Instance); } - private class TestParameterTransformer : IParameterTransformer + private class TestParameterTransformer : IOutboundParameterTransformer { - public string Transform(string value) + public string TransformOutbound(object value) { - return value; + return value?.ToString(); } } private class CustomRouteTokenTransformerConvention : RouteTokenTransformerConvention { - public CustomRouteTokenTransformerConvention(IParameterTransformer parameterTransformer) : base(parameterTransformer) + public CustomRouteTokenTransformerConvention(IOutboundParameterTransformer parameterTransformer) : base(parameterTransformer) { } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MvcEndpointDataSourceTests.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MvcEndpointDataSourceTests.cs index 21fd57a11b..168ae2950f 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MvcEndpointDataSourceTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MvcEndpointDataSourceTests.cs @@ -771,11 +771,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal return dataSource; } - private class UpperCaseParameterTransform : IParameterTransformer + private class UpperCaseParameterTransform : IOutboundParameterTransformer { - public string Transform(string value) + public string TransformOutbound(object value) { - return value?.ToUpperInvariant(); + return value?.ToString().ToUpperInvariant(); } } diff --git a/test/WebSites/RoutingWebSite/ControllerRouteTokenTransformerConvention.cs b/test/WebSites/RoutingWebSite/ControllerRouteTokenTransformerConvention.cs index 37dca3f259..d2b179a25b 100644 --- a/test/WebSites/RoutingWebSite/ControllerRouteTokenTransformerConvention.cs +++ b/test/WebSites/RoutingWebSite/ControllerRouteTokenTransformerConvention.cs @@ -11,9 +11,9 @@ namespace RoutingWebSite public class ControllerRouteTokenTransformerConvention : IApplicationModelConvention { private readonly Type _controllerType; - private readonly IParameterTransformer _parameterTransformer; + private readonly IOutboundParameterTransformer _parameterTransformer; - public ControllerRouteTokenTransformerConvention(Type controllerType, IParameterTransformer parameterTransformer) + public ControllerRouteTokenTransformerConvention(Type controllerType, IOutboundParameterTransformer parameterTransformer) { if (parameterTransformer == null) { @@ -30,7 +30,7 @@ namespace RoutingWebSite { foreach (var action in controller.Actions) { - action.Properties[typeof(IParameterTransformer)] = _parameterTransformer; + action.Properties[typeof(IOutboundParameterTransformer)] = _parameterTransformer; } } } diff --git a/test/WebSites/RoutingWebSite/TestParameterTransformer.cs b/test/WebSites/RoutingWebSite/TestParameterTransformer.cs index 779d483ca6..a1aa4088f6 100644 --- a/test/WebSites/RoutingWebSite/TestParameterTransformer.cs +++ b/test/WebSites/RoutingWebSite/TestParameterTransformer.cs @@ -7,12 +7,12 @@ using Microsoft.AspNetCore.Routing; namespace RoutingWebSite { - public class SlugifyParameterTransformer : IParameterTransformer + public class SlugifyParameterTransformer : IOutboundParameterTransformer { - public string Transform(string value) + public string TransformOutbound(object value) { // Slugify value - return Regex.Replace(value, "([a-z])([A-Z])", "$1-$2", RegexOptions.None, TimeSpan.FromMilliseconds(100)).ToLower(); + return value == null ? null : Regex.Replace(value.ToString(), "([a-z])([A-Z])", "$1-$2", RegexOptions.None, TimeSpan.FromMilliseconds(100)).ToLower(); } } } \ No newline at end of file