From fe5ff3412b7bb3bd5425a43efa01dc93a7b7672c Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Tue, 23 Apr 2024 13:08:28 -0700 Subject: [PATCH 1/4] Fix form handling and add NuGet pkg docs --- .../sample/Controllers/TestController.cs | 9 + src/OpenApi/sample/Program.cs | 2 + src/OpenApi/src/PACKAGE.md | 54 ++++ .../src/Services/OpenApiDocumentService.cs | 75 ++++- ...piDocument_documentName=forms.verified.txt | 193 +++++++++++-- ...cument_documentName=responses.verified.txt | 40 +++ ...enApiDocument_documentName=v1.verified.txt | 52 +++- ...enApiDocument_documentName=v2.verified.txt | 40 +++ .../OpenApiDocumentServiceTests.Parameters.cs | 28 ++ ...OpenApiDocumentServiceTests.RequestBody.cs | 268 +++++++++++++++++- src/OpenApi/test/SharedTypes.cs | 2 +- 11 files changed, 724 insertions(+), 39 deletions(-) create mode 100644 src/OpenApi/src/PACKAGE.md diff --git a/src/OpenApi/sample/Controllers/TestController.cs b/src/OpenApi/sample/Controllers/TestController.cs index b7d9079c8892..7c2ab39efb31 100644 --- a/src/OpenApi/sample/Controllers/TestController.cs +++ b/src/OpenApi/sample/Controllers/TestController.cs @@ -12,6 +12,13 @@ public string GetByIdAndName(RouteParamsContainer paramsContainer) return paramsContainer.Id + "_" + paramsContainer.Name; } + [HttpPost] + [Route("/forms")] + public IActionResult PostForm([FromForm] MvcTodo todo) + { + return Ok(todo); + } + public class RouteParamsContainer { [FromRoute] @@ -21,4 +28,6 @@ public class RouteParamsContainer [MinLength(5)] public string? Name { get; set; } } + + public record MvcTodo(string Title, string Description, bool IsCompleted); } diff --git a/src/OpenApi/sample/Program.cs b/src/OpenApi/sample/Program.cs index 6607ace87e6e..415daaa33947 100644 --- a/src/OpenApi/sample/Program.cs +++ b/src/OpenApi/sample/Program.cs @@ -43,7 +43,9 @@ forms.MapPost("/form-file", (IFormFile resume) => Results.Ok(resume.FileName)); forms.MapPost("/form-files", (IFormFileCollection files) => Results.Ok(files.Count)); +forms.MapPost("/form-file-multiple", (IFormFile resume, IFormFileCollection files) => Results.Ok(files.Count + resume.FileName)); forms.MapPost("/form-todo", ([FromForm] Todo todo) => Results.Ok(todo)); +forms.MapPost("/forms-pocos-and-files", ([FromForm] Todo todo, IFormFile file) => Results.Ok(new { Todo = todo, File = file.FileName })); var v1 = app.MapGroup("v1") .WithGroupName("v1"); diff --git a/src/OpenApi/src/PACKAGE.md b/src/OpenApi/src/PACKAGE.md new file mode 100644 index 000000000000..8484e9f8686e --- /dev/null +++ b/src/OpenApi/src/PACKAGE.md @@ -0,0 +1,54 @@ +## About + +Microsoft.AspNetCore.OpenApi is a NuGet package that provides built-in support for generating OpenAPI documents from minimal or controller-based APIs in ASP.NET. + +## Key Features + +* Supports generating an OpenAPI document at runtime exposed via an endpoint (`/openapi/{documentName}.json`) +* Supports generating an OpenAPI document at build-time +* Supports customizing the generated document via document transformers + +## How to Use + +To start using Microsoft.AspNetCore.OpenApi in your ASP.NET Core application, follow these steps: + +### Installation + +```sh +dotnet add package Microsoft.AspNetCore.OpenApi +``` + +### Configuration + +In your Program.cs file, register the services provided by this package in the DI container and map the provided OpenAPI document endpoint in the application. + +```C# +var builder = WebApplication.CreateBuilder(); + +// Registers the required services +builder.Services.AddOpenApi(); + +var app = builder.Build(); + +// Adds the /openapi/{documentName}.json endpoint to the application +app.MapOpenApi(); + +app.Run(); +``` + +For more information on configuring and using Microsoft.AspNetCore.OpenApi, refer to the [official documentation](https://learn.microsoft.com/en-us/aspnet/core/fundamentals/minimal-apis/openapi). + +## Main Types + + + +The main types provided by this library are: + +* `OpenApiOptions`: Options for configuring OpenAPI document generation. +* `IDocumentTransformer`: Transformer that modifies the OpenAPI document generated by the library. + +## Feedback & Contributing + + + +Microsoft.AspNetCore.OpenApi is released as open-source under the [MIT license](https://licenses.nuget.org/MIT). Bug reports and contributions are welcome at [the GitHub repository](https://github.com/dotnet/aspnetcore). diff --git a/src/OpenApi/src/Services/OpenApiDocumentService.cs b/src/OpenApi/src/Services/OpenApiDocumentService.cs index 5c476df41898..0752ccc3bf07 100644 --- a/src/OpenApi/src/Services/OpenApiDocumentService.cs +++ b/src/OpenApi/src/Services/OpenApiDocumentService.cs @@ -213,7 +213,7 @@ private OpenApiResponse GetResponse(ApiDescription apiDescription, int statusCod .Select(responseFormat => responseFormat.MediaType); foreach (var contentType in apiResponseFormatContentTypes) { - var schema = apiResponseType.Type is {} type ? _componentService.GetOrCreateSchema(type) : new OpenApiSchema(); + var schema = apiResponseType.Type is { } type ? _componentService.GetOrCreateSchema(type) : new OpenApiSchema(); response.Content[contentType] = new OpenApiMediaType { Schema = schema }; } @@ -296,11 +296,78 @@ private OpenApiRequestBody GetFormRequestBody(IList supportedR Content = new Dictionary() }; - // Forms are represented as objects with properties for each form field. var schema = new OpenApiSchema { Type = "object", Properties = new Dictionary() }; - foreach (var parameter in formParameters) + // Group form parameters by their parameter name because MVC explodes form parameters that are bound from the + // same model instance into separate parameters, while minimal APIs does not. + // + // public record Todo(int Id, string Title, bool Completed, DateTime CreatedAt) + // public void PostMvc([FromForm] Todo person) { } + // app.MapGet("/form-todo", ([FromForm] Todo todo) => Results.Ok(todo)); + // + // In the example above, MVC will bind four separate arguments to the Todo model while minimal APIs will + // bind a single Todo model instance to the todo parameter. Grouping by name allows us to handle both cases. + var groupedFormParameters = formParameters.GroupBy(parameter => parameter.ParameterDescriptor.Name).ToList(); + // If there is only one real parameter derive from the form body, then set it directly in the schema. + if (groupedFormParameters is [var groupedParameter]) { - schema.Properties[parameter.Name] = _componentService.GetOrCreateSchema(parameter.Type); + // If the group contains one element, then we're dealing with a minimal API scenario where a + // single form parameter produces a single API description parameter. + if (groupedParameter.Count() == 1) + { + var description = groupedParameter.Single(); + // Form files are keyed by their parameter name so we must capture the parameter name + // as a property in the schema. + if (description.Type == typeof(IFormFile) || description.Type == typeof(IFormFileCollection)) + { + schema.Properties[description.Name] = _componentService.GetOrCreateSchema(description.Type); + } + else + { + // POCOs do not need to be subset under their parameter name. The form-binding implementation + // will capture them implicitly. + schema = _componentService.GetOrCreateSchema(description.Type); + } + } + // If there are multiple API description parameters in the group, then we are dealing + // with the MVC scenario where form parameters are exploded into separate API parameters + // for each property within the complex type. + else + { + foreach (var parameter in groupedParameter) + { + schema.Properties[parameter.Name] = _componentService.GetOrCreateSchema(parameter.Type); + } + } + } + // If there are multiple parameters sourced from the form, then we use `allOf` to capture each parameter. + else + { + // Process the arguments in the same way that we do for the single-parameter case but + // set each sub-schema as an `allOf` item in the schema. + foreach (var parameter in groupedFormParameters) + { + if (parameter.Count() == 1) + { + var description = parameter.Single(); + if (description.Type == typeof(IFormFile) || description.Type == typeof(IFormFileCollection)) + { + schema.AllOf.Add(new OpenApiSchema { Type = "object", Properties = new Dictionary { [description.Name] = _componentService.GetOrCreateSchema(description.Type) } }); + } + else + { + schema.AllOf.Add(_componentService.GetOrCreateSchema(description.Type)); + } + } + else + { + var propertySchema = new OpenApiSchema { Type = "object", Properties = new Dictionary() }; + foreach (var description in parameter) + { + propertySchema.Properties[description.Name] = _componentService.GetOrCreateSchema(description.Type); + } + schema.AllOf.Add(propertySchema); + } + } } foreach (var requestFormat in supportedRequestFormats) diff --git a/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=forms.verified.txt b/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=forms.verified.txt index e4413bbf1eb6..6cb76cd312d0 100644 --- a/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=forms.verified.txt +++ b/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=forms.verified.txt @@ -76,7 +76,7 @@ }, } }, - "/forms/form-todo": { + "/forms/form-file-multiple": { "post": { "tags": [ "Sample" @@ -86,32 +86,78 @@ "multipart/form-data": { "schema": { "type": "object", - "properties": { - "todo": { - "required": [ - "id", - "title", - "completed", - "createdAt" - ], + "allOf": [ + { "type": "object", "properties": { - "id": { - "type": "integer", - "format": "int32" - }, - "title": { - "type": "string" - }, - "completed": { - "type": "boolean" - }, - "createdAt": { + "resume": { "type": "string", - "format": "date-time" + "format": "binary" + } + } + }, + { + "type": "object", + "properties": { + "files": { + "type": "array", + "items": { + "type": "string", + "format": "binary" + } } } } + ] + }, + "encoding": { + "multipart/form-data": { + "style": "form", + "explode": true + } + } + } + }, + "required": true + }, + "responses": { + "200": { + "description": "OK" + } + }, + } + }, + "/forms/form-todo": { + "post": { + "tags": [ + "Sample" + ], + "requestBody": { + "content": { + "multipart/form-data": { + "schema": { + "required": [ + "id", + "title", + "completed", + "createdAt" + ], + "type": "object", + "properties": { + "id": { + "type": "integer", + "format": "int32" + }, + "title": { + "type": "string" + }, + "completed": { + "type": "boolean" + }, + "createdAt": { + "type": "string", + "format": "date-time" + } } }, "encoding": { @@ -123,9 +169,59 @@ }, "application/x-www-form-urlencoded": { "schema": { + "required": [ + "id", + "title", + "completed", + "createdAt" + ], "type": "object", "properties": { - "todo": { + "id": { + "type": "integer", + "format": "int32" + }, + "title": { + "type": "string" + }, + "completed": { + "type": "boolean" + }, + "createdAt": { + "type": "string", + "format": "date-time" + } + } + }, + "encoding": { + "application/x-www-form-urlencoded": { + "style": "form", + "explode": true + } + } + } + }, + "required": true + }, + "responses": { + "200": { + "description": "OK" + } + }, + } + }, + "/forms/forms-pocos-and-files": { + "post": { + "tags": [ + "Sample" + ], + "requestBody": { + "content": { + "multipart/form-data": { + "schema": { + "type": "object", + "allOf": [ + { "required": [ "id", "title", @@ -149,11 +245,20 @@ "format": "date-time" } } + }, + { + "type": "object", + "properties": { + "file": { + "type": "string", + "format": "binary" + } + } } - } + ] }, "encoding": { - "application/x-www-form-urlencoded": { + "multipart/form-data": { "style": "form", "explode": true } @@ -217,6 +322,46 @@ } }, } + }, + "/forms": { + "post": { + "tags": [ + "Test" + ], + "requestBody": { + "content": { + "application/x-www-form-urlencoded": { + "schema": { + "type": "object", + "properties": { + "Title": { + "minLength": 5, + "type": "string" + }, + "Description": { + "minLength": 5, + "type": "string" + }, + "IsCompleted": { + "type": "boolean" + } + } + }, + "encoding": { + "application/x-www-form-urlencoded": { + "style": "form", + "explode": true + } + } + } + } + }, + "responses": { + "200": { + "description": "OK" + } + }, + } } }, "tags": [ diff --git a/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=responses.verified.txt b/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=responses.verified.txt index 6ee441a2fbcd..686eab3fef24 100644 --- a/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=responses.verified.txt +++ b/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=responses.verified.txt @@ -223,6 +223,46 @@ } }, } + }, + "/forms": { + "post": { + "tags": [ + "Test" + ], + "requestBody": { + "content": { + "application/x-www-form-urlencoded": { + "schema": { + "type": "object", + "properties": { + "Title": { + "minLength": 5, + "type": "string" + }, + "Description": { + "minLength": 5, + "type": "string" + }, + "IsCompleted": { + "type": "boolean" + } + } + }, + "encoding": { + "application/x-www-form-urlencoded": { + "style": "form", + "explode": true + } + } + } + } + }, + "responses": { + "200": { + "description": "OK" + } + }, + } } }, "tags": [ diff --git a/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=v1.verified.txt b/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=v1.verified.txt index 7a20d85a2961..167bb851fbe2 100644 --- a/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=v1.verified.txt +++ b/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=v1.verified.txt @@ -227,6 +227,56 @@ } }, } + }, + "/forms": { + "post": { + "tags": [ + "Test" + ], + "parameters": [ + { + "name": "X-Version", + "in": "header", + "schema": { + "type": "string", + "default": "1.0" + } + } + ], + "requestBody": { + "content": { + "application/x-www-form-urlencoded": { + "schema": { + "type": "object", + "properties": { + "Title": { + "minLength": 5, + "type": "string" + }, + "Description": { + "minLength": 5, + "type": "string" + }, + "IsCompleted": { + "type": "boolean" + } + } + }, + "encoding": { + "application/x-www-form-urlencoded": { + "style": "form", + "explode": true + } + } + } + } + }, + "responses": { + "200": { + "description": "OK" + } + }, + } } }, "components": { @@ -246,4 +296,4 @@ "name": "Test" } ] -} +} \ No newline at end of file diff --git a/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=v2.verified.txt b/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=v2.verified.txt index 687f32f7f5bc..83a913e6ea2a 100644 --- a/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=v2.verified.txt +++ b/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=v2.verified.txt @@ -92,6 +92,46 @@ } }, } + }, + "/forms": { + "post": { + "tags": [ + "Test" + ], + "requestBody": { + "content": { + "application/x-www-form-urlencoded": { + "schema": { + "type": "object", + "properties": { + "Title": { + "minLength": 5, + "type": "string" + }, + "Description": { + "minLength": 5, + "type": "string" + }, + "IsCompleted": { + "type": "boolean" + } + } + }, + "encoding": { + "application/x-www-form-urlencoded": { + "style": "form", + "explode": true + } + } + } + } + }, + "responses": { + "200": { + "description": "OK" + } + }, + } } }, "tags": [ diff --git a/src/OpenApi/test/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.Parameters.cs b/src/OpenApi/test/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.Parameters.cs index 3610923576e1..c3ee12c6d074 100644 --- a/src/OpenApi/test/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.Parameters.cs +++ b/src/OpenApi/test/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.Parameters.cs @@ -116,6 +116,34 @@ await VerifyOpenApiDocument(builder, document => } #nullable restore +#nullable disable + // Test coverage for https://github.com/dotnet/aspnetcore/issues/46746 + [Fact] + public async Task GetOpenApiParameters_RouteParametersAreAlwaysRequired_NullabilityDisables() + { + // Arrange + var builder = CreateBuilder(); + + // Act + builder.MapGet("/api/todos/{id}", (int id) => { }); + builder.MapGet("/api/todos/{guid}", (Guid? guid) => { }); + builder.MapGet("/api/todos/{isCompleted}", (bool isCompleted = false) => { }); + + // Assert + await VerifyOpenApiDocument(builder, document => + { + var pathParameter = Assert.Single(document.Paths["/api/todos/{id}"].Operations[OperationType.Get].Parameters); + Assert.Equal("id", pathParameter.Name); + Assert.True(pathParameter.Required); + var guidParameter = Assert.Single(document.Paths["/api/todos/{guid}"].Operations[OperationType.Get].Parameters); + Assert.Equal("guid", guidParameter.Name); + Assert.True(guidParameter.Required); + var isCompletedParameter = Assert.Single(document.Paths["/api/todos/{isCompleted}"].Operations[OperationType.Get].Parameters); + Assert.Equal("isCompleted", isCompletedParameter.Name); + Assert.True(isCompletedParameter.Required); + }); + } +#nullable restore [Fact] public async Task GetOpenApiRequestBody_SkipsRequestBodyParameters() { diff --git a/src/OpenApi/test/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.RequestBody.cs b/src/OpenApi/test/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.RequestBody.cs index 16604a533fe8..a4033e162c64 100644 --- a/src/OpenApi/test/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.RequestBody.cs +++ b/src/OpenApi/test/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.RequestBody.cs @@ -187,15 +187,24 @@ await VerifyOpenApiDocument(builder, document => var content = Assert.Single(operation.RequestBody.Content); Assert.Equal("multipart/form-data", content.Key); Assert.Equal("object", content.Value.Schema.Type); - Assert.NotNull(content.Value.Schema.Properties); - Assert.Contains("formFile1", content.Value.Schema.Properties); - Assert.Contains("formFile2", content.Value.Schema.Properties); - var formFile1Property = content.Value.Schema.Properties["formFile1"]; - Assert.Equal("string", formFile1Property.Type); - Assert.Equal("binary", formFile1Property.Format); - var formFile2Property = content.Value.Schema.Properties["formFile2"]; - Assert.Equal("string", formFile2Property.Type); - Assert.Equal("binary", formFile2Property.Format); + Assert.NotNull(content.Value.Schema.AllOf); + Assert.Collection(content.Value.Schema.AllOf, + allOfItem => + { + Assert.NotNull(allOfItem.Properties); + Assert.Contains("formFile1", allOfItem.Properties); + var formFile1Property = allOfItem.Properties["formFile1"]; + Assert.Equal("string", formFile1Property.Type); + Assert.Equal("binary", formFile1Property.Format); + }, + allOfItem => + { + Assert.NotNull(allOfItem.Properties); + Assert.Contains("formFile2", allOfItem.Properties); + var formFile2Property = allOfItem.Properties["formFile2"]; + Assert.Equal("string", formFile2Property.Type); + Assert.Equal("binary", formFile2Property.Format); + }); }); } @@ -388,4 +397,245 @@ await VerifyOpenApiDocument(builder, document => }); } + // Test coverage for https://github.com/dotnet/aspnetcore/issues/52284 + [Fact] + public async Task GetOpenApiRequestBody_HandlesFromFormWithPoco() + { + // Arrange + var builder = CreateBuilder(); + + // Act + builder.MapPost("/form", ([FromForm] Todo todo) => { }); + + // Assert + await VerifyOpenApiDocument(builder, document => + { + var paths = Assert.Single(document.Paths.Values); + var operation = paths.Operations[OperationType.Post]; + Assert.NotNull(operation.RequestBody); + Assert.NotNull(operation.RequestBody.Content); + var content = operation.RequestBody.Content; + // Forms can be provided in both the URL and via form data + Assert.Contains("application/x-www-form-urlencoded", content.Keys); + Assert.Contains("multipart/form-data", content.Keys); + // Same schema should be produced for both content-types + foreach (var item in content.Values) + { + Assert.NotNull(item.Schema); + Assert.Equal("object", item.Schema.Type); + Assert.NotNull(item.Schema.Properties); + Assert.Collection(item.Schema.Properties, + property => + { + Assert.Equal("id", property.Key); + Assert.Equal("integer", property.Value.Type); + }, + property => + { + Assert.Equal("title", property.Key); + Assert.Equal("string", property.Value.Type); + }, + property => + { + Assert.Equal("completed", property.Key); + Assert.Equal("boolean", property.Value.Type); + }, + property => + { + Assert.Equal("createdAt", property.Key); + Assert.Equal("string", property.Value.Type); + Assert.Equal("date-time", property.Value.Format); + }); + } + }); + } + + [Fact] + public async Task GetOpenApiRequestBody_HandlesFromFormWithPoco_MvcAction() + { + // Arrange + var action = CreateActionDescriptor(nameof(ActionWithFormModel)); + + // Assert + await VerifyOpenApiDocument(action, document => + { + var paths = Assert.Single(document.Paths.Values); + var operation = paths.Operations[OperationType.Get]; + Assert.NotNull(operation.RequestBody); + Assert.NotNull(operation.RequestBody.Content); + var content = operation.RequestBody.Content; + // Forms can be provided in both the URL and via form data + Assert.Contains("application/x-www-form-urlencoded", content.Keys); + // Same schema should be produced for both content-types + foreach (var item in content.Values) + { + Assert.NotNull(item.Schema); + Assert.Equal("object", item.Schema.Type); + Assert.NotNull(item.Schema.Properties); + Assert.Collection(item.Schema.Properties, + property => + { + Assert.Equal("Id", property.Key); + Assert.Equal("integer", property.Value.Type); + }, + property => + { + Assert.Equal("Title", property.Key); + Assert.Equal("string", property.Value.Type); + }, + property => + { + Assert.Equal("Completed", property.Key); + Assert.Equal("boolean", property.Value.Type); + }, + property => + { + Assert.Equal("CreatedAt", property.Key); + Assert.Equal("string", property.Value.Type); + Assert.Equal("date-time", property.Value.Format); + }); + } + }); + } + + [Route("/form-model")] + private void ActionWithFormModel([FromForm] Todo todo) { } + + // Test coverage for https://github.com/dotnet/aspnetcore/issues/53831 + [Fact] + public async Task GetOpenApiRequestBody_HandlesMultipleFormWithPoco() + { + // Arrange + var builder = CreateBuilder(); + + // Act + builder.MapPost("/form", ([FromForm] Todo todo, [FromForm] Error error) => { }); + + // Assert + await VerifyOpenApiDocument(builder, document => + { + var paths = Assert.Single(document.Paths.Values); + var operation = paths.Operations[OperationType.Post]; + Assert.NotNull(operation.RequestBody); + Assert.NotNull(operation.RequestBody.Content); + var content = operation.RequestBody.Content; + // Forms can be provided in both the URL and via form data + Assert.Contains("application/x-www-form-urlencoded", content.Keys); + Assert.Contains("multipart/form-data", content.Keys); + // Same schema should be produced for both content-types + foreach (var item in content.Values) + { + Assert.NotNull(item.Schema); + Assert.Equal("object", item.Schema.Type); + Assert.NotNull(item.Schema.AllOf); + Assert.Collection(item.Schema.AllOf, + allOfItem => + { + Assert.Collection(allOfItem.Properties, property => + { + Assert.Equal("id", property.Key); + Assert.Equal("integer", property.Value.Type); + }, + property => + { + Assert.Equal("title", property.Key); + Assert.Equal("string", property.Value.Type); + }, + property => + { + Assert.Equal("completed", property.Key); + Assert.Equal("boolean", property.Value.Type); + }, + property => + { + Assert.Equal("createdAt", property.Key); + Assert.Equal("string", property.Value.Type); + Assert.Equal("date-time", property.Value.Format); + }); + }, + allOfItem => + { + Assert.Collection(allOfItem.Properties, + property => + { + Assert.Equal("code", property.Key); + Assert.Equal("integer", property.Value.Type); + }, + property => + { + Assert.Equal("message", property.Key); + Assert.Equal("string", property.Value.Type); + }); + }); + } + }); + } + + [Fact] + public async Task GetOpenApiRequestBody_HandlesMultipleFormWithPoco_MvcAction() + { + // Arrange + var action = CreateActionDescriptor(nameof(ActionWithMultipleFormModel)); + + // Assert + await VerifyOpenApiDocument(action, document => + { + var paths = Assert.Single(document.Paths.Values); + var operation = paths.Operations[OperationType.Get]; + Assert.NotNull(operation.RequestBody); + Assert.NotNull(operation.RequestBody.Content); + var content = operation.RequestBody.Content; + // Forms can be provided in both the URL and via form data + Assert.Contains("application/x-www-form-urlencoded", content.Keys); + // Same schema should be produced for both content-types + foreach (var item in content.Values) + { + Assert.NotNull(item.Schema); + Assert.Equal("object", item.Schema.Type); + Assert.NotNull(item.Schema.AllOf); + Assert.Collection(item.Schema.AllOf, + allOfItem => + { + Assert.Collection(allOfItem.Properties, property => + { + Assert.Equal("Id", property.Key); + Assert.Equal("integer", property.Value.Type); + }, + property => + { + Assert.Equal("Title", property.Key); + Assert.Equal("string", property.Value.Type); + }, + property => + { + Assert.Equal("Completed", property.Key); + Assert.Equal("boolean", property.Value.Type); + }, + property => + { + Assert.Equal("CreatedAt", property.Key); + Assert.Equal("string", property.Value.Type); + Assert.Equal("date-time", property.Value.Format); + }); + }, + allOfItem => + { + Assert.Collection(allOfItem.Properties, + property => + { + Assert.Equal("Code", property.Key); + Assert.Equal("integer", property.Value.Type); + }, + property => + { + Assert.Equal("Message", property.Key); + Assert.Equal("string", property.Value.Type); + }); + }); + } + }); + } + + [Route("/form-model")] + private void ActionWithMultipleFormModel([FromForm] Todo todo, [FromForm] Error error) { } } diff --git a/src/OpenApi/test/SharedTypes.cs b/src/OpenApi/test/SharedTypes.cs index d5e7a120bde6..b5e3baf267d5 100644 --- a/src/OpenApi/test/SharedTypes.cs +++ b/src/OpenApi/test/SharedTypes.cs @@ -10,7 +10,7 @@ internal record Todo(int Id, string Title, bool Completed, DateTime CreatedAt); internal record TodoWithDueDate(int Id, string Title, bool Completed, DateTime CreatedAt, DateTime DueDate) : Todo(Id, Title, Completed, CreatedAt); -internal record Error(int code, string Message); +internal record Error(int Code, string Message); internal record Result(bool IsSuccessful, T Value, Error Error); From 74b306324a5a1aafe48b3bcd0de8fb5106f7ed4b Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Wed, 24 Apr 2024 16:43:12 -0700 Subject: [PATCH 2/4] Address feedback and add more tests --- src/OpenApi/src/PACKAGE.md | 6 +- .../src/Services/OpenApiComponentService.cs | 21 +- .../src/Services/OpenApiDocumentService.cs | 96 +++-- .../OpenApiDocumentServiceTests.Parameters.cs | 5 +- ...OpenApiDocumentServiceTests.RequestBody.cs | 345 ++++++++++++++++++ src/OpenApi/test/SharedTypes.cs | 3 + 6 files changed, 432 insertions(+), 44 deletions(-) diff --git a/src/OpenApi/src/PACKAGE.md b/src/OpenApi/src/PACKAGE.md index 8484e9f8686e..fee4a3a4a199 100644 --- a/src/OpenApi/src/PACKAGE.md +++ b/src/OpenApi/src/PACKAGE.md @@ -1,10 +1,10 @@ ## About -Microsoft.AspNetCore.OpenApi is a NuGet package that provides built-in support for generating OpenAPI documents from minimal or controller-based APIs in ASP.NET. +Microsoft.AspNetCore.OpenApi is a NuGet package that provides built-in support for generating OpenAPI documents from minimal or controller-based APIs in ASP.NET Core. ## Key Features -* Supports generating an OpenAPI document at runtime exposed via an endpoint (`/openapi/{documentName}.json`) +* Supports viewing generated OpenAPI documents at runtime via a parameterized endpoint (`/openapi/{documentName}.json`) * Supports generating an OpenAPI document at build-time * Supports customizing the generated document via document transformers @@ -36,7 +36,7 @@ app.MapOpenApi(); app.Run(); ``` -For more information on configuring and using Microsoft.AspNetCore.OpenApi, refer to the [official documentation](https://learn.microsoft.com/en-us/aspnet/core/fundamentals/minimal-apis/openapi). +For more information on configuring and using Microsoft.AspNetCore.OpenApi, refer to the [official documentation](https://learn.microsoft.com/aspnet/core/fundamentals/minimal-apis/openapi). ## Main Types diff --git a/src/OpenApi/src/Services/OpenApiComponentService.cs b/src/OpenApi/src/Services/OpenApiComponentService.cs index 0bd3174e0b9f..0db74964e8ce 100644 --- a/src/OpenApi/src/Services/OpenApiComponentService.cs +++ b/src/OpenApi/src/Services/OpenApiComponentService.cs @@ -42,7 +42,26 @@ internal sealed class OpenApiComponentService(IOptions jsonOptions) { OnSchemaGenerated = (context, schema) => { - schema.ApplyPrimitiveTypesAndFormats(context.TypeInfo.Type); + var type = context.TypeInfo.Type; + // Fix up schemas generated for IFormFile, IFormFileCollection, Stream, and PipeReader + // that appear as properties within complex types. + if (type == typeof(IFormFile) || type == typeof(Stream) || type == typeof(PipeReader)) + { + schema.Clear(); + schema[OpenApiSchemaKeywords.TypeKeyword] = "string"; + schema[OpenApiSchemaKeywords.FormatKeyword] = "binary"; + } + else if (type == typeof(IFormFileCollection)) + { + schema.Clear(); + schema[OpenApiSchemaKeywords.TypeKeyword] = "array"; + schema[OpenApiSchemaKeywords.ItemsKeyword] = new JsonObject + { + [OpenApiSchemaKeywords.TypeKeyword] = "string", + [OpenApiSchemaKeywords.FormatKeyword] = "binary" + }; + } + schema.ApplyPrimitiveTypesAndFormats(type); if (context.GetCustomAttributes(typeof(ValidationAttribute)) is { } validationAttributes) { schema.ApplyValidationAttributes(validationAttributes); diff --git a/src/OpenApi/src/Services/OpenApiDocumentService.cs b/src/OpenApi/src/Services/OpenApiDocumentService.cs index 0752ccc3bf07..7efbfff39ce2 100644 --- a/src/OpenApi/src/Services/OpenApiDocumentService.cs +++ b/src/OpenApi/src/Services/OpenApiDocumentService.cs @@ -306,59 +306,72 @@ private OpenApiRequestBody GetFormRequestBody(IList supportedR // // In the example above, MVC will bind four separate arguments to the Todo model while minimal APIs will // bind a single Todo model instance to the todo parameter. Grouping by name allows us to handle both cases. - var groupedFormParameters = formParameters.GroupBy(parameter => parameter.ParameterDescriptor.Name).ToList(); - // If there is only one real parameter derive from the form body, then set it directly in the schema. - if (groupedFormParameters is [var groupedParameter]) + var groupedFormParameters = formParameters.GroupBy(parameter => parameter.ParameterDescriptor.Name); + // If there is only one real parameter derived from the form body, then set it directly in the schema. + var hasMultipleFormParameters = groupedFormParameters.Count() > 1; + foreach (var parameter in groupedFormParameters) { - // If the group contains one element, then we're dealing with a minimal API scenario where a - // single form parameter produces a single API description parameter. - if (groupedParameter.Count() == 1) + if (parameter.All(parameter => parameter.ModelMetadata.ContainerType is null)) { - var description = groupedParameter.Single(); + var description = parameter.Single(); // Form files are keyed by their parameter name so we must capture the parameter name // as a property in the schema. if (description.Type == typeof(IFormFile) || description.Type == typeof(IFormFileCollection)) { - schema.Properties[description.Name] = _componentService.GetOrCreateSchema(description.Type); + if (hasMultipleFormParameters) + { + schema.AllOf.Add(new OpenApiSchema + { + Type = "object", + Properties = new Dictionary + { + [description.Name] = _componentService.GetOrCreateSchema(description.Type) + } + }); + } + else + { + schema.Properties[description.Name] = _componentService.GetOrCreateSchema(description.Type); + } } else { - // POCOs do not need to be subset under their parameter name. The form-binding implementation - // will capture them implicitly. - schema = _componentService.GetOrCreateSchema(description.Type); - } - } - // If there are multiple API description parameters in the group, then we are dealing - // with the MVC scenario where form parameters are exploded into separate API parameters - // for each property within the complex type. - else - { - foreach (var parameter in groupedParameter) - { - schema.Properties[parameter.Name] = _componentService.GetOrCreateSchema(parameter.Type); - } - } - } - // If there are multiple parameters sourced from the form, then we use `allOf` to capture each parameter. - else - { - // Process the arguments in the same way that we do for the single-parameter case but - // set each sub-schema as an `allOf` item in the schema. - foreach (var parameter in groupedFormParameters) - { - if (parameter.Count() == 1) - { - var description = parameter.Single(); - if (description.Type == typeof(IFormFile) || description.Type == typeof(IFormFileCollection)) + if (hasMultipleFormParameters) { - schema.AllOf.Add(new OpenApiSchema { Type = "object", Properties = new Dictionary { [description.Name] = _componentService.GetOrCreateSchema(description.Type) } }); + if (description.ModelMetadata.IsComplexType) + { + schema.AllOf.Add(_componentService.GetOrCreateSchema(description.Type)); + } + else + { + schema.AllOf.Add(new OpenApiSchema + { + Type = "object", + Properties = new Dictionary + { + [description.Name] = _componentService.GetOrCreateSchema(description.Type) + } + }); + } } else { - schema.AllOf.Add(_componentService.GetOrCreateSchema(description.Type)); + // POCOs do not need to be subset under their parameter name in the properties grouping. + // The form-binding implementation will capture them implicitly. + if (description.ModelMetadata.IsComplexType) + { + schema = _componentService.GetOrCreateSchema(description.Type); + } + else + { + schema.Properties[description.Name] = _componentService.GetOrCreateSchema(description.Type); + } } } - else + } + else + { + if (hasMultipleFormParameters) { var propertySchema = new OpenApiSchema { Type = "object", Properties = new Dictionary() }; foreach (var description in parameter) @@ -367,6 +380,13 @@ private OpenApiRequestBody GetFormRequestBody(IList supportedR } schema.AllOf.Add(propertySchema); } + else + { + foreach (var description in parameter) + { + schema.Properties[description.Name] = _componentService.GetOrCreateSchema(description.Type); + } + } } } diff --git a/src/OpenApi/test/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.Parameters.cs b/src/OpenApi/test/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.Parameters.cs index c3ee12c6d074..5c8431514c5c 100644 --- a/src/OpenApi/test/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.Parameters.cs +++ b/src/OpenApi/test/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.Parameters.cs @@ -116,10 +116,10 @@ await VerifyOpenApiDocument(builder, document => } #nullable restore +// Test coverage for https://github.com/dotnet/aspnetcore/issues/46746 requires disabling nullability #nullable disable - // Test coverage for https://github.com/dotnet/aspnetcore/issues/46746 [Fact] - public async Task GetOpenApiParameters_RouteParametersAreAlwaysRequired_NullabilityDisables() + public async Task GetOpenApiParameters_RouteParametersAreAlwaysRequired_NullabilityDisabled() { // Arrange var builder = CreateBuilder(); @@ -144,6 +144,7 @@ await VerifyOpenApiDocument(builder, document => }); } #nullable restore + [Fact] public async Task GetOpenApiRequestBody_SkipsRequestBodyParameters() { diff --git a/src/OpenApi/test/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.RequestBody.cs b/src/OpenApi/test/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.RequestBody.cs index a4033e162c64..b1584de51627 100644 --- a/src/OpenApi/test/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.RequestBody.cs +++ b/src/OpenApi/test/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.RequestBody.cs @@ -3,6 +3,7 @@ using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.InternalTesting; using Microsoft.AspNetCore.Mvc; using Microsoft.OpenApi.Models; @@ -638,4 +639,348 @@ await VerifyOpenApiDocument(action, document => [Route("/form-model")] private void ActionWithMultipleFormModel([FromForm] Todo todo, [FromForm] Error error) { } + + [Fact] + public async Task GetOpenApiRequestBody_HandlesFromFormWithPocoSingleProp_MvcAction() + { + // Arrange + var action = CreateActionDescriptor(nameof(ActionWithFormModelSingleProp)); + + // Assert + await VerifyOpenApiDocument(action, document => + { + var paths = Assert.Single(document.Paths.Values); + var operation = paths.Operations[OperationType.Get]; + Assert.NotNull(operation.RequestBody); + Assert.NotNull(operation.RequestBody.Content); + var content = operation.RequestBody.Content; + // Forms can be provided in both the URL and via form data + Assert.Contains("application/x-www-form-urlencoded", content.Keys); + // Same schema should be produced for both content-types + foreach (var item in content.Values) + { + Assert.NotNull(item.Schema); + Assert.Equal("object", item.Schema.Type); + Assert.NotNull(item.Schema.Properties); + Assert.Collection(item.Schema.Properties, + property => + { + Assert.Equal("Name", property.Key); + Assert.Equal("string", property.Value.Type); + }); + } + }); + } + + [Route("/form-model-single-prop")] + private void ActionWithFormModelSingleProp([FromForm] ModelWithSingleProperty model) { } + + private class ModelWithSingleProperty + { + public string Name { get; set; } + } + + [Fact] + public async Task GetOpenApiRequestBody_HandlesFormModelWithFile_MvcAction() + { + // Arrange + var action = CreateActionDescriptor(nameof(ActionWithFormModelWithFile)); + + // Assert + await VerifyOpenApiDocument(action, document => + { + var paths = Assert.Single(document.Paths.Values); + var operation = paths.Operations[OperationType.Get]; + Assert.NotNull(operation.RequestBody.Content); + var content = operation.RequestBody.Content; + var item = Assert.Single(content.Values); + Assert.NotNull(item.Schema); + Assert.Equal("object", item.Schema.Type); + Assert.Collection(item.Schema.Properties, + property => + { + Assert.Equal("Name", property.Key); + Assert.Equal("string", property.Value.Type); + }, + property => + { + Assert.Equal("Description", property.Key); + Assert.Equal("string", property.Value.Type); + }, + property => + { + Assert.Equal("Resume", property.Key); + Assert.Equal("string", property.Value.Type); + Assert.Equal("binary", property.Value.Format); + }); + }); + } + + [Route("/resume")] + private void ActionWithFormModelWithFile([FromForm] ResumeUpload model) { } + + [Fact] + public async Task GetOpenApiRequestBody_HandlesFormModelWithFile() + { + // Arrange + var builder = CreateBuilder(); + + // Act + builder.MapGet("/resume", ([FromForm] ResumeUpload model) => { }); + + // Assert + await VerifyOpenApiDocument(builder, document => + { + var paths = Assert.Single(document.Paths.Values); + var operation = paths.Operations[OperationType.Get]; + Assert.NotNull(operation.RequestBody.Content); + var content = operation.RequestBody.Content; + foreach (var item in content.Values) + { + Assert.NotNull(item.Schema); + Assert.Equal("object", item.Schema.Type); + Assert.Collection(item.Schema.Properties, + property => + { + Assert.Equal("name", property.Key); + Assert.Equal("string", property.Value.Type); + }, + property => + { + Assert.Equal("description", property.Key); + Assert.Equal("string", property.Value.Type); + }, + property => + { + Assert.Equal("resume", property.Key); + Assert.Equal("string", property.Value.Type); + Assert.Equal("binary", property.Value.Format); + }); + } + }); + } + + [Theory] + [InlineData(nameof(ActionWithDateTimeForm), "string", "date-time")] + [InlineData(nameof(ActionWithGuidForm), "string", "uuid")] + [InlineData(nameof(ActionWithIntForm), "integer", "int32")] + public async Task GetOpenApiRequestBody_HandlesFormWithPrimitives_MvcAction(string actionMethodName, string type, string format) + { + // Arrange + var action = CreateActionDescriptor(actionMethodName); + + // Assert + await VerifyOpenApiDocument(action, document => + { + var paths = Assert.Single(document.Paths.Values); + var operation = paths.Operations[OperationType.Get]; + Assert.NotNull(operation.RequestBody.Content); + var content = operation.RequestBody.Content; + var item = Assert.Single(content.Values); + Assert.NotNull(item.Schema); + Assert.Equal("object", item.Schema.Type); + Assert.Collection(item.Schema.Properties, + property => + { + Assert.Equal("model", property.Key); + Assert.Equal(type, property.Value.Type); + Assert.Equal(format, property.Value.Format); + }); + }); + } + + [Route("/form-int")] + private void ActionWithIntForm([FromForm] int model) { } + + [Route("/form-guid")] + private void ActionWithGuidForm([FromForm] Guid model) { } + + [Route("/form-datetime")] + private void ActionWithDateTimeForm([FromForm] DateTime model) { } + + public static object[][] FromFormWithPrimitives => + [ + [([FromForm] int id) => {}, "integer", "int32"], + [([FromForm] long id) => {}, "integer", "int64"], + [([FromForm] float id) => {}, "number", "float"], + [([FromForm] double id) => {}, "number", "double"], + [([FromForm] decimal id) => {}, "number", "double"], + [([FromForm] bool id) => {}, "boolean", null], + [([FromForm] string id) => {}, "string", null], + [([FromForm] char id) => {}, "string", null], + [([FromForm] byte id) => {}, "string", "byte"], + [([FromForm] short id) => {}, "integer", "int16"], + [([FromForm] ushort id) => {}, "integer", "uint16"], + [([FromForm] uint id) => {}, "integer", "uint32"], + [([FromForm] ulong id) => {}, "integer", "uint64"], + [([FromForm] Uri id) => {}, "string", "uri"], + [([FromForm] TimeOnly id) => {}, "string", "time"], + [([FromForm] DateOnly id) => {}, "string", "date"] + ]; + + [Theory] + [MemberData(nameof(FromFormWithPrimitives))] + public async Task GetOpenApiRequestBody_HandlesFormWithPrimitives(Delegate requestHandler, string schemaType, string schemaFormat) + { + // Arrange + var builder = CreateBuilder(); + + // Act + builder.MapGet("/api/", requestHandler); + + // Assert + await VerifyOpenApiDocument(builder, document => + { + var paths = Assert.Single(document.Paths.Values); + var operation = paths.Operations[OperationType.Get]; + Assert.NotNull(operation.RequestBody.Content); + var content = operation.RequestBody.Content; + foreach (var item in content.Values) + { + Assert.NotNull(item.Schema); + Assert.Equal("object", item.Schema.Type); + Assert.Collection(item.Schema.Properties, + property => + { + Assert.Equal("id", property.Key); + Assert.Equal(schemaType, property.Value.Type); + Assert.Equal(schemaFormat, property.Value.Format); + }); + } + }); + } + + [Fact] + public async Task GetOpenApiRequestBody_HandlesFormWithMultipleMixedTypes() + { + // Arrange + var builder = CreateBuilder(); + + // Act + builder.MapGet("/api/", ([FromForm] Todo todo, IFormFile formFile, [FromForm] Guid guid) => { }); + + // Assert + await VerifyOpenApiDocument(builder, document => + { + var paths = Assert.Single(document.Paths.Values); + var operation = paths.Operations[OperationType.Get]; + Assert.NotNull(operation.RequestBody.Content); + var content = operation.RequestBody.Content; + foreach (var item in content.Values) + { + Assert.NotNull(item.Schema); + Assert.Equal("object", item.Schema.Type); + Assert.Collection(item.Schema.AllOf, + allOfItem => + { + Assert.Collection(allOfItem.Properties, property => + { + Assert.Equal("id", property.Key); + Assert.Equal("integer", property.Value.Type); + }, + property => + { + Assert.Equal("title", property.Key); + Assert.Equal("string", property.Value.Type); + }, + property => + { + Assert.Equal("completed", property.Key); + Assert.Equal("boolean", property.Value.Type); + }, + property => + { + Assert.Equal("createdAt", property.Key); + Assert.Equal("string", property.Value.Type); + Assert.Equal("date-time", property.Value.Format); + }); + }, + allOfItem => + { + Assert.Collection(allOfItem.Properties, property => + { + Assert.Equal("formFile", property.Key); + Assert.Equal("string", property.Value.Type); + Assert.Equal("binary", property.Value.Format); + }); + }, + allOfItem => + { + Assert.Collection(allOfItem.Properties, property => + { + Assert.Equal("guid", property.Key); + Assert.Equal("string", property.Value.Type); + Assert.Equal("uuid", property.Value.Format); + }); + }); + } + }); + } + + [ConditionalFact(Skip = "https://github.com/dotnet/aspnetcore/issues/55349")] + public async Task GetOpenApiRequestBody_HandlesFormWithMultipleMixedTypes_MvcAction() + { + // Arrange + var action = CreateActionDescriptor(nameof(ActionWithMixedFormTypes)); + + // Assert + await VerifyOpenApiDocument(action, document => + { + var paths = Assert.Single(document.Paths.Values); + var operation = paths.Operations[OperationType.Get]; + Assert.NotNull(operation.RequestBody.Content); + var content = operation.RequestBody.Content; + foreach (var item in content.Values) + { + Assert.NotNull(item.Schema); + Assert.Equal("object", item.Schema.Type); + Assert.Collection(item.Schema.AllOf, + allOfItem => + { + Assert.Collection(allOfItem.Properties, property => + { + Assert.Equal("id", property.Key); + Assert.Equal("integer", property.Value.Type); + }, + property => + { + Assert.Equal("title", property.Key); + Assert.Equal("string", property.Value.Type); + }, + property => + { + Assert.Equal("completed", property.Key); + Assert.Equal("boolean", property.Value.Type); + }, + property => + { + Assert.Equal("createdAt", property.Key); + Assert.Equal("string", property.Value.Type); + Assert.Equal("date-time", property.Value.Format); + }); + }, + allOfItem => + { + Assert.Collection(allOfItem.Properties, property => + { + Assert.Equal("formFile", property.Key); + Assert.Equal("string", property.Value.Type); + Assert.Equal("binary", property.Value.Format); + }); + }, + allOfItem => + { + Assert.Collection(allOfItem.Properties, property => + { + Assert.Equal("guid", property.Key); + Assert.Equal("string", property.Value.Type); + Assert.Equal("uuid", property.Value.Format); + }); + }); + } + }); + } + + [Route("/form-mixed-types")] + private void ActionWithMixedFormTypes([FromForm] Todo todo, IFormFile formFile, [FromForm] Guid guid) { } } diff --git a/src/OpenApi/test/SharedTypes.cs b/src/OpenApi/test/SharedTypes.cs index b5e3baf267d5..7a886682e5e6 100644 --- a/src/OpenApi/test/SharedTypes.cs +++ b/src/OpenApi/test/SharedTypes.cs @@ -5,6 +5,7 @@ // and benchmark apps. using System.Text.Json.Serialization; +using Microsoft.AspNetCore.Http; internal record Todo(int Id, string Title, bool Completed, DateTime CreatedAt); @@ -12,6 +13,8 @@ internal record TodoWithDueDate(int Id, string Title, bool Completed, DateTime C internal record Error(int Code, string Message); +internal record ResumeUpload(string Name, string Description, IFormFile Resume); + internal record Result(bool IsSuccessful, T Value, Error Error); [JsonDerivedType(typeof(Triangle), typeDiscriminator: "triangle")] From 1908c19fa6aadc31ddfffbadb125943d5f48196e Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Thu, 25 Apr 2024 12:19:12 -0700 Subject: [PATCH 3/4] Add tests for Stream and PipeReader --- ...OpenApiDocumentServiceTests.RequestBody.cs | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/src/OpenApi/test/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.RequestBody.cs b/src/OpenApi/test/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.RequestBody.cs index b1584de51627..ad6d01ae8a79 100644 --- a/src/OpenApi/test/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.RequestBody.cs +++ b/src/OpenApi/test/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.RequestBody.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.IO.Pipelines; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.InternalTesting; @@ -983,4 +984,59 @@ await VerifyOpenApiDocument(action, document => [Route("/form-mixed-types")] private void ActionWithMixedFormTypes([FromForm] Todo todo, IFormFile formFile, [FromForm] Guid guid) { } + + [Fact] + public async Task GetOpenApiRequestBody_HandlesStreamAndPipeReader() + { + // Arrange + var builder = CreateBuilder(); + + // Act + builder.MapGet("/stream", (Stream stream) => { }); + builder.MapGet("/pipereader", (PipeReader pipeReader) => { }); + + // Assert + await VerifyOpenApiDocument(builder, document => + { + foreach (var path in document.Paths) + { + var operation = path.Value.Operations[OperationType.Get]; + Assert.NotNull(operation.RequestBody.Content); + var content = Assert.Single(operation.RequestBody.Content); + Assert.Equal("application/octet-stream", content.Key); + Assert.NotNull(content.Value.Schema); + Assert.Equal("string", content.Value.Schema.Type); + Assert.Equal("binary", content.Value.Schema.Format); + } + }); + } + + [ConditionalFact(Skip = "https://github.com/dotnet/aspnetcore/issues/55349")] + public async Task GetOpenApiRequestBody_HandlesStreamAndPipeReader_MvcAction() + { + // Arrange + var streamAction = CreateActionDescriptor(nameof(ActionWithStream)); + var pipeReaderAction = CreateActionDescriptor(nameof(ActionWithPipeReader)); + + // Assert + await VerifyOpenApiDocument(streamAction, VerifyDocument); + await VerifyOpenApiDocument(pipeReaderAction, VerifyDocument); + + static void VerifyDocument(OpenApiDocument document) + { + var path = Assert.Single(document.Paths); + var operation = path.Value.Operations[OperationType.Get]; + Assert.NotNull(operation.RequestBody.Content); + var content = Assert.Single(operation.RequestBody.Content); + Assert.Equal("application/octet-stream", content.Key); + Assert.NotNull(content.Value.Schema); + Assert.Equal("string", content.Value.Schema.Type); + Assert.Equal("binary", content.Value.Schema.Format); + } + } + + [Route("/stream")] + private void ActionWithStream(Stream stream) { } + [Route("/pipereader")] + private void ActionWithPipeReader(PipeReader pipeReader) { } } From 8738bb639c2ef93cd1c0a6fdfd82f00e7b5d6da2 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Mon, 29 Apr 2024 12:37:53 -0700 Subject: [PATCH 4/4] More tests and feedback --- .../src/Services/OpenApiDocumentService.cs | 25 +++++++++-------- ...nApiComponentService.RequestBodySchemas.cs | 28 +++++++++++++++++++ src/OpenApi/test/SharedTypes.cs | 6 ++++ 3 files changed, 48 insertions(+), 11 deletions(-) diff --git a/src/OpenApi/src/Services/OpenApiDocumentService.cs b/src/OpenApi/src/Services/OpenApiDocumentService.cs index 7efbfff39ce2..f2b480e25006 100644 --- a/src/OpenApi/src/Services/OpenApiDocumentService.cs +++ b/src/OpenApi/src/Services/OpenApiDocumentService.cs @@ -297,23 +297,26 @@ private OpenApiRequestBody GetFormRequestBody(IList supportedR }; var schema = new OpenApiSchema { Type = "object", Properties = new Dictionary() }; - // Group form parameters by their parameter name because MVC explodes form parameters that are bound from the - // same model instance into separate parameters, while minimal APIs does not. + // Group form parameters by their name because MVC explodes form parameters that are bound from the + // same model instance into separate ApiParameterDescriptions in ApiExplorer, while minimal APIs does not. // // public record Todo(int Id, string Title, bool Completed, DateTime CreatedAt) // public void PostMvc([FromForm] Todo person) { } // app.MapGet("/form-todo", ([FromForm] Todo todo) => Results.Ok(todo)); // - // In the example above, MVC will bind four separate arguments to the Todo model while minimal APIs will + // In the example above, MVC's ApiExplorer will bind four separate arguments to the Todo model while minimal APIs will // bind a single Todo model instance to the todo parameter. Grouping by name allows us to handle both cases. var groupedFormParameters = formParameters.GroupBy(parameter => parameter.ParameterDescriptor.Name); // If there is only one real parameter derived from the form body, then set it directly in the schema. var hasMultipleFormParameters = groupedFormParameters.Count() > 1; foreach (var parameter in groupedFormParameters) { + // ContainerType is not null when the parameter has been exploded into separate API + // parameters by ApiExplorer as in the MVC model. if (parameter.All(parameter => parameter.ModelMetadata.ContainerType is null)) { var description = parameter.Single(); + var parameterSchema = _componentService.GetOrCreateSchema(description.Type); // Form files are keyed by their parameter name so we must capture the parameter name // as a property in the schema. if (description.Type == typeof(IFormFile) || description.Type == typeof(IFormFileCollection)) @@ -325,22 +328,24 @@ private OpenApiRequestBody GetFormRequestBody(IList supportedR Type = "object", Properties = new Dictionary { - [description.Name] = _componentService.GetOrCreateSchema(description.Type) + [description.Name] = parameterSchema } }); } else { - schema.Properties[description.Name] = _componentService.GetOrCreateSchema(description.Type); + schema.Properties[description.Name] = parameterSchema; } } else { if (hasMultipleFormParameters) { + // Here and below: POCOs do not need to be need under their parameter name in the grouping. + // The form-binding implementation will capture them implicitly. if (description.ModelMetadata.IsComplexType) { - schema.AllOf.Add(_componentService.GetOrCreateSchema(description.Type)); + schema.AllOf.Add(parameterSchema); } else { @@ -349,22 +354,20 @@ private OpenApiRequestBody GetFormRequestBody(IList supportedR Type = "object", Properties = new Dictionary { - [description.Name] = _componentService.GetOrCreateSchema(description.Type) + [description.Name] = parameterSchema } }); } } else { - // POCOs do not need to be subset under their parameter name in the properties grouping. - // The form-binding implementation will capture them implicitly. if (description.ModelMetadata.IsComplexType) { - schema = _componentService.GetOrCreateSchema(description.Type); + schema = parameterSchema; } else { - schema.Properties[description.Name] = _componentService.GetOrCreateSchema(description.Type); + schema.Properties[description.Name] = parameterSchema; } } } diff --git a/src/OpenApi/test/Services/OpenApiComponentService/OpenApiComponentService.RequestBodySchemas.cs b/src/OpenApi/test/Services/OpenApiComponentService/OpenApiComponentService.RequestBodySchemas.cs index 95d3735ad359..a46882ea6c88 100644 --- a/src/OpenApi/test/Services/OpenApiComponentService/OpenApiComponentService.RequestBodySchemas.cs +++ b/src/OpenApi/test/Services/OpenApiComponentService/OpenApiComponentService.RequestBodySchemas.cs @@ -79,6 +79,34 @@ await VerifyOpenApiDocument(builder, document => }); } + [Fact] + public async Task GetOpenApiRequestBody_GeneratesSchemaForFilesInRecursiveType() + { + // Arrange + var builder = CreateBuilder(); + + // Act + builder.MapPost("/proposal", (Proposal stream) => { }); + + // Assert + await VerifyOpenApiDocument(builder, document => + { + var operation = document.Paths[$"/proposal"].Operations[OperationType.Post]; + var requestBody = operation.RequestBody; + var schema = requestBody.Content["application/json"].Schema; + Assert.Collection(schema.Properties, + property => { + Assert.Equal("proposalElement", property.Key); + // Todo: Assert that refs are used correctly. + }, + property => { + Assert.Equal("stream", property.Key); + Assert.Equal("string", property.Value.Type); + Assert.Equal("binary", property.Value.Format); + }); + }); + } + [Fact] public async Task GetOpenApiRequestBody_GeneratesSchemaForListOf() { diff --git a/src/OpenApi/test/SharedTypes.cs b/src/OpenApi/test/SharedTypes.cs index 7a886682e5e6..c691f54acd30 100644 --- a/src/OpenApi/test/SharedTypes.cs +++ b/src/OpenApi/test/SharedTypes.cs @@ -57,3 +57,9 @@ internal enum Status Approved, Rejected } + +internal class Proposal +{ + public required Proposal ProposalElement { get; set; } + public required Stream Stream { get; set; } +}