From 656fe93d93d98286afe1da7c35b0363129848e6a Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Tue, 8 Apr 2025 14:19:03 -0700 Subject: [PATCH 1/3] fix: #58329 merge multiple consumes content types into one operation --- .../src/Services/OpenApiDocumentService.cs | 19 ++++++++++- ...OpenApiDocumentServiceTests.RequestBody.cs | 34 +++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/src/OpenApi/src/Services/OpenApiDocumentService.cs b/src/OpenApi/src/Services/OpenApiDocumentService.cs index a358f56d08a9..bf62cc6f1e42 100644 --- a/src/OpenApi/src/Services/OpenApiDocumentService.cs +++ b/src/OpenApi/src/Services/OpenApiDocumentService.cs @@ -278,7 +278,6 @@ private async Task> GetOperationsAsy }; _operationTransformerContextCache.TryAdd(description.ActionDescriptor.Id, operationContext); - operations[description.GetOperationType()] = operation; // Use index-based for loop to avoid allocating an enumerator with a foreach. for (var i = 0; i < operationTransformers.Length; i++) @@ -295,6 +294,24 @@ private async Task> GetOperationsAsy { await endpointOperationTransformer.TransformAsync(operation, operationContext, cancellationToken); } + + var operationType = description.GetOperationType(); + if ( + operations.TryGetValue(operationType, out var existingOperation) && + existingOperation.RequestBody?.Content is not null && + operation.RequestBody is { Content.Count: > 0 } + ) + { + // Merge additional accepted content types into the existing operation. + foreach (var body in operation.RequestBody.Content) + { + existingOperation.RequestBody.Content.Add(body); + } + } + else + { + operations[operationType] = operation; + } } return operations; } diff --git a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.RequestBody.cs b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.RequestBody.cs index 032720da5cc0..5066a207cbbf 100644 --- a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.RequestBody.cs +++ b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.RequestBody.cs @@ -259,6 +259,40 @@ await VerifyOpenApiDocument(builder, document => }); } + /// + /// Tests documented behavior at https://learn.microsoft.com/en-us/aspnet/core/web-api/?view=aspnetcore-9.0#define-supported-request-content-types-with-the-consumes-attribute-1 + /// + [Fact] + public async Task GetRequestBody_HandlesMultipleAcceptedContentType() + { + // Arrange + var builder = CreateBuilder(); + + // Act + builder.MapPost("/", [Consumes("application/json")] (TodoWithDueDate name) => { }); + builder.MapPost("/", [Consumes("application/x-www-form-urlencoded")] ([FromForm] TodoWithDueDate name) => { }); + + // Assert + await VerifyOpenApiDocument(builder, document => + { + var paths = Assert.Single(document.Paths.Values); + var operation = paths.Operations[OperationType.Post]; + Assert.NotNull(operation.RequestBody); + + Assert.Collection(operation.RequestBody.Content, + pair => + { + Assert.Equal("application/json", pair.Key); + Assert.Equal("TodoWithDueDate", pair.Value.Schema.Annotations["x-schema-id"]); + }, + pair => + { + Assert.Equal("application/x-www-form-urlencoded", pair.Key); + Assert.Equal("TodoWithDueDate", pair.Value.Schema.Annotations["x-schema-id"]); + }); + }); + } + [Fact] public async Task GetRequestBody_HandlesJsonBody() { From 5722bc523c70f34afc8842fd5cdf84d8d518e265 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Wed, 9 Apr 2025 09:30:36 -0700 Subject: [PATCH 2/3] refactor to better support operation transformers --- .../sample/Controllers/XmlController.cs | 16 ++--- src/OpenApi/src/PublicAPI.Unshipped.txt | 2 + .../src/Services/OpenApiDocumentService.cs | 64 ++++++++++++------- .../OpenApiOperationTransformerContext.cs | 7 +- .../Transformers/OperationTransformerTests.cs | 50 +++++++++++++++ 5 files changed, 106 insertions(+), 33 deletions(-) diff --git a/src/OpenApi/sample/Controllers/XmlController.cs b/src/OpenApi/sample/Controllers/XmlController.cs index d2849c6b304c..e3c6475ca0ae 100644 --- a/src/OpenApi/sample/Controllers/XmlController.cs +++ b/src/OpenApi/sample/Controllers/XmlController.cs @@ -8,6 +8,14 @@ [ApiExplorerSettings(GroupName = "xml")] public class XmlController : ControllerBase { + /// The name of the person. + /// Returns the greeting. + [HttpGet] + public string Get1(string name) + { + return $"Hello, {name}!"; + } + /// /// A summary of the action. /// @@ -20,14 +28,6 @@ public string Get() return "Hello, World!"; } - /// The name of the person. - /// Returns the greeting. - [HttpGet] - public string Get1(string name) - { - return $"Hello, {name}!"; - } - /// The todo to insert into the database. [HttpPost] public string Post(Todo todo) diff --git a/src/OpenApi/src/PublicAPI.Unshipped.txt b/src/OpenApi/src/PublicAPI.Unshipped.txt index 629fbbb86f29..fb732aff0b2e 100644 --- a/src/OpenApi/src/PublicAPI.Unshipped.txt +++ b/src/OpenApi/src/PublicAPI.Unshipped.txt @@ -3,6 +3,8 @@ static Microsoft.AspNetCore.Builder.OpenApiEndpointConventionBuilderExtensions.A Microsoft.AspNetCore.OpenApi.OpenApiDocumentTransformerContext.GetOrCreateSchemaAsync(System.Type! type, Microsoft.AspNetCore.Mvc.ApiExplorer.ApiParameterDescription? parameterDescription = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! Microsoft.AspNetCore.OpenApi.OpenApiOperationTransformerContext.Document.get -> Microsoft.OpenApi.Models.OpenApiDocument? Microsoft.AspNetCore.OpenApi.OpenApiOperationTransformerContext.Document.init -> void +Microsoft.AspNetCore.OpenApi.OpenApiOperationTransformerContext.AllDescriptions.get -> System.Collections.Generic.IReadOnlyList! +Microsoft.AspNetCore.OpenApi.OpenApiOperationTransformerContext.AllDescriptions.init -> void Microsoft.AspNetCore.OpenApi.OpenApiOperationTransformerContext.GetOrCreateSchemaAsync(System.Type! type, Microsoft.AspNetCore.Mvc.ApiExplorer.ApiParameterDescription? parameterDescription = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! Microsoft.AspNetCore.OpenApi.OpenApiSchemaTransformerContext.Document.get -> Microsoft.OpenApi.Models.OpenApiDocument? Microsoft.AspNetCore.OpenApi.OpenApiSchemaTransformerContext.Document.init -> void diff --git a/src/OpenApi/src/Services/OpenApiDocumentService.cs b/src/OpenApi/src/Services/OpenApiDocumentService.cs index bf62cc6f1e42..f63448e1fb86 100644 --- a/src/OpenApi/src/Services/OpenApiDocumentService.cs +++ b/src/OpenApi/src/Services/OpenApiDocumentService.cs @@ -262,9 +262,18 @@ private async Task> GetOperationsAsy CancellationToken cancellationToken) { var operations = new Dictionary(); - foreach (var description in descriptions) + foreach (var opTypeDescriptions in descriptions.GroupBy(d => d.GetOperationType())) { - var operation = await GetOperationAsync(description, document, scopedServiceProvider, schemaTransformers, cancellationToken); + var operationType = opTypeDescriptions.Key; + + // `description` is the first description for a given Route + HttpMethod. + // There may be additional descriptions if the endpoint has additional definitions + // with different [Consumes] definitions. We merge in the bodies of these additional endpoints, + // but currently don't merge any other parts of the definition. + IReadOnlyList allDescriptions = [.. opTypeDescriptions]; + var description = allDescriptions.First(); + + var operation = await GetOperationAsync(allDescriptions, document, scopedServiceProvider, schemaTransformers, cancellationToken); operation.Annotations ??= new Dictionary(); operation.Annotations.Add(OpenApiConstants.DescriptionId, description.ActionDescriptor.Id); @@ -272,12 +281,14 @@ private async Task> GetOperationsAsy { DocumentName = documentName, Description = description, + AllDescriptions = [.. allDescriptions], ApplicationServices = scopedServiceProvider, Document = document, SchemaTransformers = schemaTransformers }; _operationTransformerContextCache.TryAdd(description.ActionDescriptor.Id, operationContext); + operations[operationType] = operation; // Use index-based for loop to avoid allocating an enumerator with a foreach. for (var i = 0; i < operationTransformers.Length; i++) @@ -288,41 +299,25 @@ private async Task> GetOperationsAsy // Apply any endpoint-specific operation transformers registered via // the AddOpenApiOperationTransformer extension method. - var endpointOperationTransformers = description.ActionDescriptor.EndpointMetadata - .OfType(); + var endpointOperationTransformers = allDescriptions + .SelectMany(d => d.ActionDescriptor.EndpointMetadata + .OfType()); foreach (var endpointOperationTransformer in endpointOperationTransformers) { await endpointOperationTransformer.TransformAsync(operation, operationContext, cancellationToken); } - - var operationType = description.GetOperationType(); - if ( - operations.TryGetValue(operationType, out var existingOperation) && - existingOperation.RequestBody?.Content is not null && - operation.RequestBody is { Content.Count: > 0 } - ) - { - // Merge additional accepted content types into the existing operation. - foreach (var body in operation.RequestBody.Content) - { - existingOperation.RequestBody.Content.Add(body); - } - } - else - { - operations[operationType] = operation; - } } return operations; } private async Task GetOperationAsync( - ApiDescription description, + IReadOnlyList descriptions, OpenApiDocument document, IServiceProvider scopedServiceProvider, IOpenApiSchemaTransformer[] schemaTransformers, CancellationToken cancellationToken) { + var description = descriptions.First(); var tags = GetTags(description, document); var operation = new OpenApiOperation { @@ -331,9 +326,30 @@ private async Task GetOperationAsync( Description = GetDescription(description), Responses = await GetResponsesAsync(document, description, scopedServiceProvider, schemaTransformers, cancellationToken), Parameters = await GetParametersAsync(document, description, scopedServiceProvider, schemaTransformers, cancellationToken), - RequestBody = await GetRequestBodyAsync(document, description, scopedServiceProvider, schemaTransformers, cancellationToken), Tags = tags, }; + + foreach (var bodyDescription in descriptions) + { + var requestBody = await GetRequestBodyAsync(document, bodyDescription, scopedServiceProvider, schemaTransformers, cancellationToken); + if (operation.RequestBody is null) + { + operation.RequestBody = requestBody; + } + else if (requestBody is not null) + { + // Merge additional accepted content types that are defined by additional endpoint descriptions. + var existingContent = operation.RequestBody.Content; + foreach (var additionalContent in requestBody.Content) + { + if (!existingContent.ContainsKey(additionalContent.Key)) + { + existingContent.Add(additionalContent); + } + } + } + } + return operation; } diff --git a/src/OpenApi/src/Transformers/OpenApiOperationTransformerContext.cs b/src/OpenApi/src/Transformers/OpenApiOperationTransformerContext.cs index 8e070cc2c92f..71742c05424d 100644 --- a/src/OpenApi/src/Transformers/OpenApiOperationTransformerContext.cs +++ b/src/OpenApi/src/Transformers/OpenApiOperationTransformerContext.cs @@ -18,10 +18,15 @@ public sealed class OpenApiOperationTransformerContext public required string DocumentName { get; init; } /// - /// Gets the API description associated with target operation. + /// Gets the primary API description associated with target operation. /// public required ApiDescription Description { get; init; } + /// + /// Gets all API descriptions that were merged to create the target operation. + /// + public required IReadOnlyList AllDescriptions { get; init; } + /// /// Gets the application services associated with the current document the target operation is in. /// diff --git a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Transformers/OperationTransformerTests.cs b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Transformers/OperationTransformerTests.cs index f172ac364f55..ce093173e67d 100644 --- a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Transformers/OperationTransformerTests.cs +++ b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Transformers/OperationTransformerTests.cs @@ -3,6 +3,7 @@ using System.Globalization; using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.OpenApi; using Microsoft.Extensions.DependencyInjection; using Microsoft.OpenApi.Models; @@ -657,6 +658,55 @@ public async Task OperationTransformer_ExecutesAsynchronously() Assert.Equal([1, 2, 3], transformerOrder); } + [Fact] + public async Task OperationTransformer_ExecutesOncePerSetOfMergedEndpoints() + { + // Arrange + var builder = CreateBuilder(); + + // Act + builder.MapPost("/overload", [Consumes("application/json")] (TodoWithDueDate name) => { }); + builder.MapPost("/overload", [Consumes("application/x-www-form-urlencoded")] ([FromForm] TodoWithDueDate name) => { }); + + var options = new OpenApiOptions(); + int executionCount = 0; + options.AddOperationTransformer((operation, context, cancellationToken) => + { + executionCount++; + + Assert.Collection(context.AllDescriptions, + description => + { + Assert.Equal("application/json", description.SupportedRequestFormats.Single().MediaType); + // The primary description (the first declared endpoint) should be first in AllDescriptions. + Assert.Equal(context.Description, description); + }, + description => + { + Assert.Equal("application/x-www-form-urlencoded", description.SupportedRequestFormats.Single().MediaType); + }); + + operation.Description = "overloaded x" + context.AllDescriptions.Count; + return Task.CompletedTask; + }); + + // Assert + await VerifyOpenApiDocument(builder, options, document => + { + Assert.Equal(1, executionCount); + + var paths = Assert.Single(document.Paths.Values); + var operation = paths.Operations[OperationType.Post]; + Assert.NotNull(operation.RequestBody); + + Assert.Equal("overloaded x2", operation.Description); + Assert.Collection(operation.RequestBody.Content, + pair => Assert.Equal("application/json", pair.Key), + pair => Assert.Equal("application/x-www-form-urlencoded", pair.Key) + ); + }); + } + private class ActivatedTransformer : IOpenApiOperationTransformer { public Task TransformAsync(OpenApiOperation operation, OpenApiOperationTransformerContext context, CancellationToken cancellationToken) From 07d563be7998e377af0ed26f002e85b49d35826a Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Wed, 9 Apr 2025 09:35:26 -0700 Subject: [PATCH 3/3] adjust xml comment tests so endpoint isn't overloaded --- .../sample/Controllers/XmlController.cs | 16 +++--- ...nApiDocument_documentName=xml.verified.txt | 56 +++++++++++++++---- ...nApiDocument_documentName=xml.verified.txt | 56 +++++++++++++++---- 3 files changed, 96 insertions(+), 32 deletions(-) diff --git a/src/OpenApi/sample/Controllers/XmlController.cs b/src/OpenApi/sample/Controllers/XmlController.cs index e3c6475ca0ae..0181638195db 100644 --- a/src/OpenApi/sample/Controllers/XmlController.cs +++ b/src/OpenApi/sample/Controllers/XmlController.cs @@ -8,14 +8,6 @@ [ApiExplorerSettings(GroupName = "xml")] public class XmlController : ControllerBase { - /// The name of the person. - /// Returns the greeting. - [HttpGet] - public string Get1(string name) - { - return $"Hello, {name}!"; - } - /// /// A summary of the action. /// @@ -28,6 +20,14 @@ public string Get() return "Hello, World!"; } + /// The name of the person. + /// Returns the greeting. + [HttpGet("{name}")] + public string Get1(string name) + { + return $"Hello, {name}!"; + } + /// The todo to insert into the database. [HttpPost] public string Post(Todo todo) diff --git a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApi3_0/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=xml.verified.txt b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApi3_0/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=xml.verified.txt index 9eed2206b116..39cf0179861b 100644 --- a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApi3_0/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=xml.verified.txt +++ b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApi3_0/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=xml.verified.txt @@ -149,19 +149,11 @@ "tags": [ "Xml" ], - "parameters": [ - { - "name": "name", - "in": "query", - "description": "The name of the person.", - "schema": { - "type": "string" - } - } - ], + "summary": "A summary of the action.", + "description": "A description of the action.", "responses": { "200": { - "description": "Returns the greeting.", + "description": "OK", "content": { "text/plain": { "schema": { @@ -230,6 +222,46 @@ } } } + }, + "/Xml/{name}": { + "get": { + "tags": [ + "Xml" + ], + "parameters": [ + { + "name": "name", + "in": "path", + "description": "The name of the person.", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "Returns the greeting.", + "content": { + "text/plain": { + "schema": { + "type": "string" + } + }, + "application/json": { + "schema": { + "type": "string" + } + }, + "text/json": { + "schema": { + "type": "string" + } + } + } + } + } + } } }, "components": { @@ -394,4 +426,4 @@ "name": "Xml" } ] -} \ No newline at end of file +} diff --git a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApi3_1/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=xml.verified.txt b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApi3_1/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=xml.verified.txt index 4a8829575928..de0b6ecf9f51 100644 --- a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApi3_1/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=xml.verified.txt +++ b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApi3_1/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=xml.verified.txt @@ -149,19 +149,11 @@ "tags": [ "Xml" ], - "parameters": [ - { - "name": "name", - "in": "query", - "description": "The name of the person.", - "schema": { - "type": "string" - } - } - ], + "summary": "A summary of the action.", + "description": "A description of the action.", "responses": { "200": { - "description": "Returns the greeting.", + "description": "OK", "content": { "text/plain": { "schema": { @@ -230,6 +222,46 @@ } } } + }, + "/Xml/{name}": { + "get": { + "tags": [ + "Xml" + ], + "parameters": [ + { + "name": "name", + "in": "path", + "description": "The name of the person.", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "Returns the greeting.", + "content": { + "text/plain": { + "schema": { + "type": "string" + } + }, + "application/json": { + "schema": { + "type": "string" + } + }, + "text/json": { + "schema": { + "type": "string" + } + } + } + } + } + } } }, "components": { @@ -394,4 +426,4 @@ "name": "Xml" } ] -} \ No newline at end of file +}