From f62b69cc329073a631cfedc1e840400cae1bc166 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Garc=C3=ADa=20de=20la=20Noceda=20Arg=C3=BCelles?= Date: Thu, 22 Aug 2024 19:07:16 +0200 Subject: [PATCH 1/7] Support for HttpResults types --- .../src/Routing/ActionEndpointFactory.cs | 10 +++ .../Mvc.FunctionalTests/ApiExplorerTest.cs | 83 ++++++++++++++++++- .../ApiExplorerWebSite.csproj | 2 +- .../ApiExplorerHttpResultsController.cs | 36 ++++++++ .../sample/Controllers/TestController.cs | 10 +++ ...ment_documentName=controllers.verified.txt | 43 ++++++++++ 6 files changed, 180 insertions(+), 4 deletions(-) create mode 100644 src/Mvc/test/WebSites/ApiExplorerWebSite/Controllers/ApiExplorerHttpResultsController.cs diff --git a/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs b/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs index 6b46d0a62bca..d581a57e3e1d 100644 --- a/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs +++ b/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs @@ -405,6 +405,15 @@ private static void AddActionDataToBuilder( builder.Metadata.Add(filter); } } + action.EndpointMetadata ??= []; + foreach (var metadata in builder.Metadata.OfType()) + { + if (action.EndpointMetadata + .FirstOrDefault(e => e is IProducesResponseTypeMetadata p && p.StatusCode == metadata.StatusCode) is null) + { + action.EndpointMetadata.Add(metadata); + } + } if (action.ActionConstraints != null && action.ActionConstraints.Count > 0) { @@ -453,6 +462,7 @@ private static void AddActionDataToBuilder( perRouteConventions[i](builder); } + if (builder.FilterFactories.Count > 0 && controllerActionDescriptor is not null) { var routeHandlerFilters = builder.FilterFactories; diff --git a/src/Mvc/test/Mvc.FunctionalTests/ApiExplorerTest.cs b/src/Mvc/test/Mvc.FunctionalTests/ApiExplorerTest.cs index 2ffebf5af979..0c67a689f82e 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/ApiExplorerTest.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/ApiExplorerTest.cs @@ -3,13 +3,12 @@ using System.Net; using System.Net.Http; +using System.Reflection; using ApiExplorerWebSite; +using Microsoft.AspNetCore.InternalTesting; using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.AspNetCore.Mvc.ModelBinding; -using Microsoft.AspNetCore.InternalTesting; using Newtonsoft.Json; -using Microsoft.Extensions.Logging; -using System.Reflection; using Xunit.Abstractions; namespace Microsoft.AspNetCore.Mvc.FunctionalTests; @@ -1565,6 +1564,84 @@ 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 async Task ApiExplorer_SupportsIResults() + { + // Act + var response = await Client.GetAsync($"http://localhost/ApiExplorerHttpResultsController/GetOfT"); + var responseBody = await response.EnsureSuccessStatusCode().Content.ReadAsStringAsync(); + var result = JsonConvert.DeserializeObject>(responseBody); + + //Assert + var description = Assert.Single(result); + Assert.Collection( + description.SupportedResponseTypes.OrderBy(r => r.StatusCode), + responseType => + { + Assert.Equal(typeof(int).FullName, responseType.ResponseType); + Assert.Equal(200, responseType.StatusCode); + Assert.False(responseType.IsDefaultResponse); + }, + responseType => + { + Assert.Equal(typeof(DateTime).FullName, responseType.ResponseType); + Assert.Equal(404, responseType.StatusCode); + Assert.False(responseType.IsDefaultResponse); + }); + } + + [Fact] + public async Task ApiExplorer_SupportsIResultsWhenNoType() + { + // Act + var response = await Client.GetAsync($"http://localhost/ApiExplorerHttpResultsController/GetWithNotFoundWithNoType"); + var responseBody = await response.EnsureSuccessStatusCode().Content.ReadAsStringAsync(); + var result = JsonConvert.DeserializeObject>(responseBody); + + //Assert + var description = Assert.Single(result); + Assert.Collection( + description.SupportedResponseTypes.OrderBy(r => r.StatusCode), + responseType => + { + Assert.Equal(typeof(int).FullName, responseType.ResponseType); + Assert.Equal(200, responseType.StatusCode); + Assert.False(responseType.IsDefaultResponse); + }, + responseType => + { + Assert.Equal(typeof(void).FullName, responseType.ResponseType); + Assert.Equal(404, responseType.StatusCode); + Assert.False(responseType.IsDefaultResponse); + }); + } + + [Fact] + public async Task ApiExplorer_SupportsIResultsWhenDifferentProduceType() + { + // Act + var response = await Client.GetAsync($"http://localhost/ApiExplorerHttpResultsController/GetWithDifferentProduceType"); + var responseBody = await response.EnsureSuccessStatusCode().Content.ReadAsStringAsync(); + var result = JsonConvert.DeserializeObject>(responseBody); + + //Assert + var description = Assert.Single(result); + Assert.Collection( + description.SupportedResponseTypes.OrderBy(r => r.StatusCode), + responseType => + { + Assert.Equal(typeof(long).FullName, responseType.ResponseType); + Assert.Equal(200, responseType.StatusCode); + Assert.False(responseType.IsDefaultResponse); + }, + responseType => + { + Assert.Equal(typeof(void).FullName, responseType.ResponseType); + Assert.Equal(404, responseType.StatusCode); + Assert.False(responseType.IsDefaultResponse); + }); + } + 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..a0f36112855c 100644 --- a/src/Mvc/test/WebSites/ApiExplorerWebSite/ApiExplorerWebSite.csproj +++ b/src/Mvc/test/WebSites/ApiExplorerWebSite/ApiExplorerWebSite.csproj @@ -8,7 +8,7 @@ - + diff --git a/src/Mvc/test/WebSites/ApiExplorerWebSite/Controllers/ApiExplorerHttpResultsController.cs b/src/Mvc/test/WebSites/ApiExplorerWebSite/Controllers/ApiExplorerHttpResultsController.cs new file mode 100644 index 000000000000..c860936ac70d --- /dev/null +++ b/src/Mvc/test/WebSites/ApiExplorerWebSite/Controllers/ApiExplorerHttpResultsController.cs @@ -0,0 +1,36 @@ +// 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.Controllers; + +public class ApiExplorerHttpResultsController : Controller +{ + [HttpGet("ApiExplorerHttpResultsController/GetOfT")] + public Results, NotFound> Get() + { + return Random.Shared.Next() % 2 == 0 + ? TypedResults.Ok(0) + : TypedResults.NotFound(DateTime.Now); + } + + [HttpGet("ApiExplorerHttpResultsController/GetWithNotFoundWithNoType")] + public async Task, NotFound>> GetWithNotFoundWithNoType() + { + await Task.Delay(1); + return Random.Shared.Next() % 2 == 0 + ? TypedResults.Ok(0) + : TypedResults.NotFound(); + } + [ProducesResponseType(200, Type = typeof(long))] + [HttpGet("ApiExplorerHttpResultsController/GetWithDifferentProduceType")] + public async Task, NotFound>> GetWithDifferentProduceType() + { + await Task.Delay(1); + return Random.Shared.Next() % 2 == 0 + ? TypedResults.Ok(0) + : TypedResults.NotFound(); + } +} diff --git a/src/OpenApi/sample/Controllers/TestController.cs b/src/OpenApi/sample/Controllers/TestController.cs index cf1fed79abb2..dd82526c7500 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,15 @@ public string GetByIdAndName(RouteParamsContainer paramsContainer) return paramsContainer.Id + "_" + paramsContainer.Name; } + [HttpGet] + [Route("/getbyidandnameWithIResults/{id}/{name}")] + public Results, NotFound> GetByIdAndNameWithIResults(RouteParamsContainer paramsContainer) + { + return Random.Shared.Next() % 2 == 0 ? + TypedResults.Ok(paramsContainer.Id + "_" + paramsContainer.Name) + : TypedResults.NotFound(); + } + [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 5f8abe054fd2..a174d458a1b5 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,49 @@ } } }, + "/getbyidandnameWithIResults/{id}/{name}": { + "get": { + "tags": [ + "Test" + ], + "parameters": [ + { + "name": "Id", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int32" + } + }, + { + "name": "Name", + "in": "path", + "required": true, + "schema": { + "minLength": 5, + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "minLength": 5, + "type": "string" + } + } + } + }, + "404": { + "description": "Not Found" + } + } + } + }, "/forms": { "post": { "tags": [ From b86ade686917cc6eca9af7825b612a97964429a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Garc=C3=ADa=20de=20la=20Noceda=20Arg=C3=BCelles?= Date: Thu, 22 Aug 2024 19:17:09 +0200 Subject: [PATCH 2/7] Avoid iterating in EndpointMetadata to many times --- src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs b/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs index d581a57e3e1d..8517d99cc41a 100644 --- a/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs +++ b/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs @@ -406,9 +406,10 @@ private static void AddActionDataToBuilder( } } action.EndpointMetadata ??= []; + var producesResponseMetadataByAttribute = action.EndpointMetadata.OfType().ToList(); foreach (var metadata in builder.Metadata.OfType()) { - if (action.EndpointMetadata + if (producesResponseMetadataByAttribute .FirstOrDefault(e => e is IProducesResponseTypeMetadata p && p.StatusCode == metadata.StatusCode) is null) { action.EndpointMetadata.Add(metadata); From d522a821fbaead5a11ef63461e6db3a72d9acfac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Garc=C3=ADa=20de=20la=20Noceda=20Arg=C3=BCelles?= Date: Thu, 22 Aug 2024 19:42:49 +0200 Subject: [PATCH 3/7] Avoid iterating when there is no change in Builder.Metadata --- .../src/Routing/ActionEndpointFactory.cs | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs b/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs index 8517d99cc41a..dc0d839433ee 100644 --- a/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs +++ b/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs @@ -353,6 +353,7 @@ private static void AddActionDataToBuilder( // Add metadata inferred from the parameter and/or return type before action-specific metadata. // MethodInfo *should* never be null given a ControllerActionDescriptor, but this is unenforced. + var metadataCountBeforePopulating = builder.Metadata.Count; if (controllerActionDescriptor?.MethodInfo is not null) { EndpointMetadataPopulator.PopulateMetadata(controllerActionDescriptor.MethodInfo, builder); @@ -367,6 +368,20 @@ private static void AddActionDataToBuilder( } } + if (builder.Metadata.Count != metadataCountBeforePopulating) + { + action.EndpointMetadata ??= []; + var producesResponseMetadataByAttribute = action.EndpointMetadata.OfType().ToList(); + foreach (var metadata in builder.Metadata.OfType()) + { + if (producesResponseMetadataByAttribute + .FirstOrDefault(e => e is IProducesResponseTypeMetadata p && p.StatusCode == metadata.StatusCode) is null) + { + action.EndpointMetadata.Add(metadata); + } + } + } + builder.Metadata.Add(action); // MVC guarantees that when two of it's endpoints have the same route name they are equivalent. @@ -405,16 +420,6 @@ private static void AddActionDataToBuilder( builder.Metadata.Add(filter); } } - action.EndpointMetadata ??= []; - var producesResponseMetadataByAttribute = action.EndpointMetadata.OfType().ToList(); - foreach (var metadata in builder.Metadata.OfType()) - { - if (producesResponseMetadataByAttribute - .FirstOrDefault(e => e is IProducesResponseTypeMetadata p && p.StatusCode == metadata.StatusCode) is null) - { - action.EndpointMetadata.Add(metadata); - } - } if (action.ActionConstraints != null && action.ActionConstraints.Count > 0) { @@ -463,7 +468,6 @@ private static void AddActionDataToBuilder( perRouteConventions[i](builder); } - if (builder.FilterFactories.Count > 0 && controllerActionDescriptor is not null) { var routeHandlerFilters = builder.FilterFactories; From 17c52acc7d3c397d3491d16f8b7bcdc2b3ed22b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Garc=C3=ADa=20de=20la=20Noceda=20Arg=C3=BCelles?= Date: Thu, 22 Aug 2024 19:44:20 +0200 Subject: [PATCH 4/7] Better after --- src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs b/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs index dc0d839433ee..85d3421981cb 100644 --- a/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs +++ b/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs @@ -368,6 +368,8 @@ private static void AddActionDataToBuilder( } } + builder.Metadata.Add(action); + if (builder.Metadata.Count != metadataCountBeforePopulating) { action.EndpointMetadata ??= []; @@ -382,8 +384,6 @@ private static void AddActionDataToBuilder( } } - builder.Metadata.Add(action); - // MVC guarantees that when two of it's endpoints have the same route name they are equivalent. // // The case for this looks like: From 2ebbe2e36d90d247d17cb37ba8c2ce1099fa0314 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Garc=C3=ADa=20de=20la=20Noceda=20Arg=C3=BCelles?= Date: Thu, 22 Aug 2024 19:45:55 +0200 Subject: [PATCH 5/7] Fix last commit --- src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs b/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs index 85d3421981cb..bd301e6598e8 100644 --- a/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs +++ b/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs @@ -359,6 +359,7 @@ private static void AddActionDataToBuilder( EndpointMetadataPopulator.PopulateMetadata(controllerActionDescriptor.MethodInfo, builder); } + var metadataCountAfterPopulating = builder.Metadata.Count; // Add action-specific metadata early so it has a low precedence if (action.EndpointMetadata != null) { @@ -370,7 +371,7 @@ private static void AddActionDataToBuilder( builder.Metadata.Add(action); - if (builder.Metadata.Count != metadataCountBeforePopulating) + if (metadataCountAfterPopulating != metadataCountBeforePopulating) { action.EndpointMetadata ??= []; var producesResponseMetadataByAttribute = action.EndpointMetadata.OfType().ToList(); From 84ef56d565315c9313a8532a3a3eaa18cb831733 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Garc=C3=ADa=20de=20la=20Noceda=20Arg=C3=BCelles?= Date: Fri, 23 Aug 2024 17:36:59 +0200 Subject: [PATCH 6/7] Avoid Linq methods when searching through EndpointMetadata --- .../src/Routing/ActionEndpointFactory.cs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs b/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs index bd301e6598e8..e18c7f087ff8 100644 --- a/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs +++ b/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs @@ -374,13 +374,22 @@ private static void AddActionDataToBuilder( if (metadataCountAfterPopulating != metadataCountBeforePopulating) { action.EndpointMetadata ??= []; - var producesResponseMetadataByAttribute = action.EndpointMetadata.OfType().ToList(); - foreach (var metadata in builder.Metadata.OfType()) + HashSet producesResponseMetadataByAttribute = []; + foreach (var endpointMetadataInAttributes in action.EndpointMetadata) { - if (producesResponseMetadataByAttribute - .FirstOrDefault(e => e is IProducesResponseTypeMetadata p && p.StatusCode == metadata.StatusCode) is null) + if (endpointMetadataInAttributes is IProducesResponseTypeMetadata metadataInAttributes + && !producesResponseMetadataByAttribute.Contains(metadataInAttributes.StatusCode)) { - action.EndpointMetadata.Add(metadata); + producesResponseMetadataByAttribute.Add(metadataInAttributes.StatusCode); + } + } + + foreach (var endpointMetadata in builder.Metadata) + { + if (endpointMetadata is IProducesResponseTypeMetadata metadata + && !producesResponseMetadataByAttribute.Contains(metadata.StatusCode)) + { + producesResponseMetadataByAttribute.Add(metadata.StatusCode); } } } From 02e3e3190fe2ac3c3b1c5d8245c34ab63a371e11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Garc=C3=ADa=20de=20la=20Noceda=20Arg=C3=BCelles?= Date: Fri, 23 Aug 2024 17:38:01 +0200 Subject: [PATCH 7/7] Fix last commit --- src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs b/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs index e18c7f087ff8..5654976b8510 100644 --- a/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs +++ b/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs @@ -389,7 +389,7 @@ private static void AddActionDataToBuilder( if (endpointMetadata is IProducesResponseTypeMetadata metadata && !producesResponseMetadataByAttribute.Contains(metadata.StatusCode)) { - producesResponseMetadataByAttribute.Add(metadata.StatusCode); + action.EndpointMetadata.Add(metadata); } } }