From 43b5fbf51b0ee1effe57116afc39c2898a94f2c6 Mon Sep 17 00:00:00 2001 From: Artak <34246760+mkArtakMSFT@users.noreply.github.com> Date: Mon, 3 Feb 2025 21:39:52 -0800 Subject: [PATCH] Fixed the issue. Will be adding the tests as a separate commit. (#60096) # Include typed result metadata in the action descriptors for MVC Controller actions ## Description The endpoint metadata wasn't being captured in ActionModel during the ApplicationModel creation. This is addressed by the change in the DefaultApplicationModelProvider class. The metadata is now being extracted based on the return type of the action and then the selectors associated with the action are populated with that metadata, as that's what is being used later on for populating the ActionDescriptor's EndpointMetadata. Fixes #44988 --- .../DefaultApplicationModelProvider.cs | 33 +++++++++++++ ...spNetCore.Mvc.Core.WarningSuppressions.xml | 18 +++++++ .../src/Routing/ActionEndpointFactory.cs | 8 --- .../src/Routing/InertEndpointBuilder.cs | 15 ++++++ .../DefaultApplicationModelProviderTest.cs | 49 +++++++++++++++++++ .../Microsoft.AspNetCore.Mvc.Core.Test.csproj | 1 + .../Mvc.FunctionalTests/ApiExplorerTest.cs | 25 ++++++++++ .../ApiExplorerWebSite.csproj | 1 + .../ApiExplorerWithTypedResultController.cs | 14 ++++++ .../sample/Controllers/TestController.cs | 8 +++ ...ment_documentName=controllers.verified.txt | 42 ++++++++++++++++ src/Shared/EndpointMetadataPopulator.cs | 2 +- 12 files changed, 207 insertions(+), 9 deletions(-) create mode 100644 src/Mvc/Mvc.Core/src/Routing/InertEndpointBuilder.cs create mode 100644 src/Mvc/test/WebSites/ApiExplorerWebSite/Controllers/ApiExplorerWithTypedResultController.cs diff --git a/src/Mvc/Mvc.Core/src/ApplicationModels/DefaultApplicationModelProvider.cs b/src/Mvc/Mvc.Core/src/ApplicationModels/DefaultApplicationModelProvider.cs index 829ceef8f59b..9e19c9c66289 100644 --- a/src/Mvc/Mvc.Core/src/ApplicationModels/DefaultApplicationModelProvider.cs +++ b/src/Mvc/Mvc.Core/src/ApplicationModels/DefaultApplicationModelProvider.cs @@ -4,6 +4,7 @@ using System.Linq; using System.Reflection; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Metadata; using Microsoft.AspNetCore.Mvc.ActionConstraints; using Microsoft.AspNetCore.Mvc.ApiExplorer; using Microsoft.AspNetCore.Mvc.Filters; @@ -349,9 +350,41 @@ internal PropertyModel CreatePropertyModel(PropertyInfo propertyInfo) applicableAttributes.AddRange(routeAttributes); AddRange(actionModel.Selectors, CreateSelectors(applicableAttributes)); + AddReturnTypeMetadata(actionModel.Selectors, methodInfo); + return actionModel; } + internal static void AddReturnTypeMetadata(IList selectors, MethodInfo methodInfo) + { + // Get metadata from return type + var returnType = methodInfo.ReturnType; + if (CoercedAwaitableInfo.IsTypeAwaitable(returnType, out var coercedAwaitableInfo)) + { + returnType = coercedAwaitableInfo.AwaitableInfo.ResultType; + } + + if (returnType is not null && typeof(IEndpointMetadataProvider).IsAssignableFrom(returnType)) + { + // Return type implements IEndpointMetadataProvider + var builder = new InertEndpointBuilder(); + var invokeArgs = new object[2]; + invokeArgs[0] = methodInfo; + invokeArgs[1] = builder; + EndpointMetadataPopulator.PopulateMetadataForEndpointMethod.MakeGenericMethod(returnType).Invoke(null, invokeArgs); + + // The metadata is added to the builder's metadata collection. + // We need to populate the selectors with that metadata. + foreach (var metadata in builder.Metadata) + { + foreach (var selector in selectors) + { + selector.EndpointMetadata.Add(metadata); + } + } + } + } + private string CanonicalizeActionName(string actionName) { const string Suffix = "Async"; diff --git a/src/Mvc/Mvc.Core/src/Microsoft.AspNetCore.Mvc.Core.WarningSuppressions.xml b/src/Mvc/Mvc.Core/src/Microsoft.AspNetCore.Mvc.Core.WarningSuppressions.xml index 450566e81932..c581d374a6b5 100644 --- a/src/Mvc/Mvc.Core/src/Microsoft.AspNetCore.Mvc.Core.WarningSuppressions.xml +++ b/src/Mvc/Mvc.Core/src/Microsoft.AspNetCore.Mvc.Core.WarningSuppressions.xml @@ -37,6 +37,12 @@ member M:Microsoft.AspNetCore.Mvc.AcceptedAtRouteResult.#ctor(System.String,System.Object,System.Object) + + ILLink + IL2026 + member + M:Microsoft.AspNetCore.Mvc.ApplicationModels.DefaultApplicationModelProvider.AddReturnTypeMetadata(System.Collections.Generic.IList{Microsoft.AspNetCore.Mvc.ApplicationModels.SelectorModel},System.Reflection.MethodInfo) + ILLink IL2026 @@ -517,6 +523,12 @@ member M:Microsoft.AspNetCore.Mvc.ApplicationParts.ProvideApplicationPartFactoryAttribute.GetFactoryType + + ILLink + IL2060 + member + M:Microsoft.AspNetCore.Mvc.ApplicationModels.DefaultApplicationModelProvider.AddReturnTypeMetadata(System.Collections.Generic.IList{Microsoft.AspNetCore.Mvc.ApplicationModels.SelectorModel},System.Reflection.MethodInfo) + ILLink IL2060 @@ -637,6 +649,12 @@ member M:Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.DefaultBindingMetadataProvider.GetBoundConstructor(System.Type) + + ILLink + IL2072 + member + M:Microsoft.AspNetCore.Mvc.ApplicationModels.DefaultApplicationModelProvider.AddReturnTypeMetadata(System.Collections.Generic.IList{Microsoft.AspNetCore.Mvc.ApplicationModels.SelectorModel},System.Reflection.MethodInfo) + ILLink IL2072 diff --git a/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs b/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs index 6b46d0a62bca..3b803e72e340 100644 --- a/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs +++ b/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs @@ -543,12 +543,4 @@ private static RequestDelegate CreateRequestDelegate() return invoker!.InvokeAsync(); }; } - - private sealed class InertEndpointBuilder : EndpointBuilder - { - public override Endpoint Build() - { - return new Endpoint(RequestDelegate, new EndpointMetadataCollection(Metadata), DisplayName); - } - } } diff --git a/src/Mvc/Mvc.Core/src/Routing/InertEndpointBuilder.cs b/src/Mvc/Mvc.Core/src/Routing/InertEndpointBuilder.cs new file mode 100644 index 000000000000..b37312e5be92 --- /dev/null +++ b/src/Mvc/Mvc.Core/src/Routing/InertEndpointBuilder.cs @@ -0,0 +1,15 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; + +namespace Microsoft.AspNetCore.Mvc.Routing; + +internal sealed class InertEndpointBuilder : EndpointBuilder +{ + public override Endpoint Build() + { + return new Endpoint(RequestDelegate, new EndpointMetadataCollection(Metadata), DisplayName); + } +} diff --git a/src/Mvc/Mvc.Core/test/ApplicationModels/DefaultApplicationModelProviderTest.cs b/src/Mvc/Mvc.Core/test/ApplicationModels/DefaultApplicationModelProviderTest.cs index eee240402991..19aea8a971b2 100644 --- a/src/Mvc/Mvc.Core/test/ApplicationModels/DefaultApplicationModelProviderTest.cs +++ b/src/Mvc/Mvc.Core/test/ApplicationModels/DefaultApplicationModelProviderTest.cs @@ -1243,6 +1243,45 @@ public void CreateActionModel_InheritedAttributeRoutesOverridden() Assert.Contains(selectorModel.AttributeRouteModel.Attribute, action.Attributes); } + [Fact] + public void CreateActionModel_PopulatesReturnTypeEndpointMetadata() { + // Arrange + var builder = new TestApplicationModelProvider(); + var typeInfo = typeof(TypedResultsReturningActionsController).GetTypeInfo(); + var actionName = nameof(TypedResultsReturningActionsController.Get); + + // Act + var action = builder.CreateActionModel(typeInfo, typeInfo.AsType().GetMethod(actionName)); + + // Assert + Assert.NotNull(action.Selectors); + Assert.All(action.Selectors, selector => + { + Assert.NotNull(selector.EndpointMetadata); + Assert.Contains(selector.EndpointMetadata, m => m is ProducesResponseTypeMetadata); + }); + var metadata = action.Selectors[0].EndpointMetadata.OfType().Single(); + Assert.Equal(200, metadata.StatusCode); + } + + [Fact] + public void AddReturnTypeMetadata_ExtractsMetadataFromReturnType() + { + // Arrange + var selector = new SelectorModel(); + var selectors = new List { selector }; + var actionMethod = typeof(TypedResultsReturningActionsController).GetMethod(nameof(TypedResultsReturningActionsController.Get)); + + // Act + DefaultApplicationModelProvider.AddReturnTypeMetadata(selectors, actionMethod); + + // Assert + Assert.NotNull(selector.EndpointMetadata); + Assert.Single(selector.EndpointMetadata); + Assert.IsType(selector.EndpointMetadata.Single()); + Assert.Equal(200, ((ProducesResponseTypeMetadata)selector.EndpointMetadata[0]).StatusCode); + } + [Fact] public void ControllerDispose_ExplicitlyImplemented_IDisposableMethods_AreTreatedAs_NonActions() { @@ -1711,6 +1750,16 @@ public void Details() { } public void List() { } } + private class TypedResultsReturningActionsController : Controller + { + [HttpGet] + public Http.HttpResults.Ok Get() => TypedResults.Ok(new Foo { Info = "Hello" }); + } + + public class Foo { + public required string Info { get; set; } + } + private class CustomHttpMethodsAttribute : Attribute, IActionHttpMethodProvider { private readonly string[] _methods; diff --git a/src/Mvc/Mvc.Core/test/Microsoft.AspNetCore.Mvc.Core.Test.csproj b/src/Mvc/Mvc.Core/test/Microsoft.AspNetCore.Mvc.Core.Test.csproj index e7ccf9267d05..2d050ebe7717 100644 --- a/src/Mvc/Mvc.Core/test/Microsoft.AspNetCore.Mvc.Core.Test.csproj +++ b/src/Mvc/Mvc.Core/test/Microsoft.AspNetCore.Mvc.Core.Test.csproj @@ -12,6 +12,7 @@ + diff --git a/src/Mvc/test/Mvc.FunctionalTests/ApiExplorerTest.cs b/src/Mvc/test/Mvc.FunctionalTests/ApiExplorerTest.cs index 2ffebf5af979..3386a5aabb6d 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/ApiExplorerTest.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/ApiExplorerTest.cs @@ -11,6 +11,11 @@ using Microsoft.Extensions.Logging; using System.Reflection; using Xunit.Abstractions; +using Microsoft.AspNetCore.Mvc.ApplicationModels; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.AspNetCore.Mvc.ApiExplorer; +using Microsoft.AspNetCore.Http.HttpResults; +using Microsoft.AspNetCore.Http; namespace Microsoft.AspNetCore.Mvc.FunctionalTests; @@ -1565,6 +1570,26 @@ public async Task ApiExplorer_LogsInvokedDescriptionProvidersOnStartup() Assert.Contains(TestSink.Writes, w => w.Message.Equals("Executing API description provider 'JsonPatchOperationsArrayProvider' from assembly Microsoft.AspNetCore.Mvc.NewtonsoftJson v42.42.42.42.", StringComparison.Ordinal)); } + [Fact] + public void ApiExplorer_BuildsMetadataForActionWithTypedResult() + { + var apiDescCollectionProvider = Factory.Server.Services.GetService(); + var testGroupName = nameof(ApiExplorerWithTypedResultController).Replace("Controller", string.Empty); + var group = apiDescCollectionProvider.ApiDescriptionGroups.Items.Where(i => i.GroupName == testGroupName).SingleOrDefault(); + Assert.NotNull(group); + var apiDescription = Assert.Single(group.Items); + + var responseType = Assert.Single(apiDescription.SupportedResponseTypes); + Assert.Equal(StatusCodes.Status200OK, responseType.StatusCode); + Assert.Equal(typeof(Product), responseType.Type); + + Assert.NotNull(apiDescription.ActionDescriptor.EndpointMetadata); + var producesResponseTypeMetadata = apiDescription.ActionDescriptor.EndpointMetadata.OfType().SingleOrDefault(); + Assert.NotNull(producesResponseTypeMetadata); + Assert.Equal(StatusCodes.Status200OK, producesResponseTypeMetadata.StatusCode); + Assert.Equal(typeof(Product), producesResponseTypeMetadata.Type); + } + private IEnumerable GetSortedMediaTypes(ApiExplorerResponseType apiResponseType) { return apiResponseType.ResponseFormats diff --git a/src/Mvc/test/WebSites/ApiExplorerWebSite/ApiExplorerWebSite.csproj b/src/Mvc/test/WebSites/ApiExplorerWebSite/ApiExplorerWebSite.csproj index b1fcdbf3ffe6..539f0477a1fd 100644 --- a/src/Mvc/test/WebSites/ApiExplorerWebSite/ApiExplorerWebSite.csproj +++ b/src/Mvc/test/WebSites/ApiExplorerWebSite/ApiExplorerWebSite.csproj @@ -7,6 +7,7 @@ + diff --git a/src/Mvc/test/WebSites/ApiExplorerWebSite/Controllers/ApiExplorerWithTypedResultController.cs b/src/Mvc/test/WebSites/ApiExplorerWebSite/Controllers/ApiExplorerWithTypedResultController.cs new file mode 100644 index 000000000000..2bd1c52c3386 --- /dev/null +++ b/src/Mvc/test/WebSites/ApiExplorerWebSite/Controllers/ApiExplorerWithTypedResultController.cs @@ -0,0 +1,14 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.AspNetCore.Http.HttpResults; +using Microsoft.AspNetCore.Mvc; + +namespace ApiExplorerWebSite; + +[Route("ApiExplorerWithTypedResult/[Action]")] +public class ApiExplorerWithTypedResultController : Controller +{ + [HttpGet] + public Ok GetProduct() => TypedResults.Ok(new Product { Name = "Test product" }); +} diff --git a/src/OpenApi/sample/Controllers/TestController.cs b/src/OpenApi/sample/Controllers/TestController.cs index cf1fed79abb2..fdc398987a35 100644 --- a/src/OpenApi/sample/Controllers/TestController.cs +++ b/src/OpenApi/sample/Controllers/TestController.cs @@ -3,6 +3,7 @@ using System.ComponentModel.DataAnnotations; using System.Diagnostics.CodeAnalysis; +using Microsoft.AspNetCore.Http.HttpResults; using Microsoft.AspNetCore.Mvc; [ApiController] @@ -17,6 +18,13 @@ public string GetByIdAndName(RouteParamsContainer paramsContainer) return paramsContainer.Id + "_" + paramsContainer.Name; } + [HttpGet] + [Route("/gettypedresult")] + public Ok GetTypedResult() + { + return TypedResults.Ok(new MvcTodo("Title", "Description", true)); + } + [HttpPost] [Route("/forms")] public IActionResult PostForm([FromForm] MvcTodo todo) diff --git a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=controllers.verified.txt b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=controllers.verified.txt index 9eb758a76e24..efb88cb71d5e 100644 --- a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=controllers.verified.txt +++ b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=controllers.verified.txt @@ -54,6 +54,25 @@ } } }, + "/gettypedresult": { + "get": { + "tags": [ + "Test" + ], + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/MvcTodo" + } + } + } + } + } + } + }, "/forms": { "post": { "tags": [ @@ -88,6 +107,29 @@ } } }, + "components": { + "schemas": { + "MvcTodo": { + "required": [ + "title", + "description", + "isCompleted" + ], + "type": "object", + "properties": { + "title": { + "type": "string" + }, + "description": { + "type": "string" + }, + "isCompleted": { + "type": "boolean" + } + } + } + } + }, "tags": [ { "name": "Test" diff --git a/src/Shared/EndpointMetadataPopulator.cs b/src/Shared/EndpointMetadataPopulator.cs index 2c8fe12d3e9e..3087f6b2715d 100644 --- a/src/Shared/EndpointMetadataPopulator.cs +++ b/src/Shared/EndpointMetadataPopulator.cs @@ -16,7 +16,7 @@ namespace Microsoft.AspNetCore.Http; internal static class EndpointMetadataPopulator { private static readonly MethodInfo PopulateMetadataForParameterMethod = typeof(EndpointMetadataPopulator).GetMethod(nameof(PopulateMetadataForParameter), BindingFlags.NonPublic | BindingFlags.Static)!; - private static readonly MethodInfo PopulateMetadataForEndpointMethod = typeof(EndpointMetadataPopulator).GetMethod(nameof(PopulateMetadataForEndpoint), BindingFlags.NonPublic | BindingFlags.Static)!; + internal static readonly MethodInfo PopulateMetadataForEndpointMethod = typeof(EndpointMetadataPopulator).GetMethod(nameof(PopulateMetadataForEndpoint), BindingFlags.NonPublic | BindingFlags.Static)!; public static void PopulateMetadata(MethodInfo methodInfo, EndpointBuilder builder, IEnumerable? parameters = null) {