From 7f2a64e32beb9d08cfcb5e2158605ce84a886c95 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Fri, 29 Jun 2018 09:41:41 -0700 Subject: [PATCH 1/2] Razor runtime compilation produces errors if running on a shared runtime that's rolled forward Do not provide compilation references from runtime MVC assemblies. This avoids cases where the app is compiled against an older MVC but running against a newer one (e.g. shared fx roll forward) resulting in compiling against multiple versions of MVC assemblies Fixes #7969 --- .../MvcServiceCollectionExtensions.cs | 23 +++++++++++++++---- .../Controllers/HomeController.cs | 2 +- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc/MvcServiceCollectionExtensions.cs b/src/Microsoft.AspNetCore.Mvc/MvcServiceCollectionExtensions.cs index b7baee8634..037bcdd2d3 100644 --- a/src/Microsoft.AspNetCore.Mvc/MvcServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc/MvcServiceCollectionExtensions.cs @@ -2,6 +2,8 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using System.Reflection; using Microsoft.AspNetCore.Mvc; @@ -59,15 +61,15 @@ namespace Microsoft.Extensions.DependencyInjection private static void AddDefaultFrameworkParts(ApplicationPartManager partManager) { var mvcTagHelpersAssembly = typeof(InputTagHelper).GetTypeInfo().Assembly; - if(!partManager.ApplicationParts.OfType().Any(p => p.Assembly == mvcTagHelpersAssembly)) + if (!partManager.ApplicationParts.OfType().Any(p => p.Assembly == mvcTagHelpersAssembly)) { - partManager.ApplicationParts.Add(new AssemblyPart(mvcTagHelpersAssembly)); + partManager.ApplicationParts.Add(new FrameworkAssemblyPart(mvcTagHelpersAssembly)); } - + var mvcRazorAssembly = typeof(UrlResolutionTagHelper).GetTypeInfo().Assembly; - if(!partManager.ApplicationParts.OfType().Any(p => p.Assembly == mvcRazorAssembly)) + if (!partManager.ApplicationParts.OfType().Any(p => p.Assembly == mvcRazorAssembly)) { - partManager.ApplicationParts.Add(new AssemblyPart(mvcRazorAssembly)); + partManager.ApplicationParts.Add(new FrameworkAssemblyPart(mvcRazorAssembly)); } } @@ -94,5 +96,16 @@ namespace Microsoft.Extensions.DependencyInjection return builder; } + + [DebuggerDisplay("{Name}")] + private class FrameworkAssemblyPart : AssemblyPart, ICompilationReferencesProvider + { + public FrameworkAssemblyPart(Assembly assembly) + : base(assembly) + { + } + + IEnumerable ICompilationReferencesProvider.GetReferencePaths() => Enumerable.Empty(); + } } } diff --git a/test/WebSites/BasicWebSite/Controllers/HomeController.cs b/test/WebSites/BasicWebSite/Controllers/HomeController.cs index 454aca34b8..5d5fb11525 100644 --- a/test/WebSites/BasicWebSite/Controllers/HomeController.cs +++ b/test/WebSites/BasicWebSite/Controllers/HomeController.cs @@ -129,7 +129,7 @@ namespace BasicWebSite.Controllers // Ensures that the entry assembly part is marked correctly. var assemblyPartMetadata = applicationPartManager .ApplicationParts - .Where(part => part.GetType() == typeof(AssemblyPart)) + .OfType() .Select(part => part.Name) .ToArray(); From 133d49c57eefadfd22d6c1c2820612c3ba4542bd Mon Sep 17 00:00:00 2001 From: Nathanael Marchand Date: Tue, 5 Jun 2018 14:24:42 +0200 Subject: [PATCH 2/2] 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);