Skip to content

Commit 2a67ffb

Browse files
captainsafiacaptainsafia
authored
Set requiredness for primitive and files in forms (#57196)
1 parent 71ddc60 commit 2a67ffb

File tree

5 files changed

+111
-7
lines changed

5 files changed

+111
-7
lines changed

‎src/OpenApi/src/Services/OpenApiDocumentService.cs‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,10 @@ private async Task<OpenApiRequestBody> GetFormRequestBody(
381381

382382
var requestBody = new OpenApiRequestBody
383383
{
384-
Required = formParameters.Any(IsRequired),
384+
// Form bodies are always required because the framework doesn't support
385+
// serializing a form collection from an empty body. Instead, requiredness
386+
// must be set on a per-parameter basis. See below.
387+
Required = true,
385388
Content = new Dictionary<string, OpenApiMediaType>()
386389
};
387390

@@ -410,6 +413,10 @@ private async Task<OpenApiRequestBody> GetFormRequestBody(
410413
// as a property in the schema.
411414
if (description.Type == typeof(IFormFile) || description.Type == typeof(IFormFileCollection))
412415
{
416+
if (IsRequired(description))
417+
{
418+
schema.Required.Add(description.Name);
419+
}
413420
if (hasMultipleFormParameters)
414421
{
415422
schema.AllOf.Add(new OpenApiSchema
@@ -444,6 +451,10 @@ private async Task<OpenApiRequestBody> GetFormRequestBody(
444451
}
445452
else
446453
{
454+
if (IsRequired(description))
455+
{
456+
schema.Required.Add(description.Name);
457+
}
447458
schema.AllOf.Add(new OpenApiSchema
448459
{
449460
Type = "object",
@@ -462,6 +473,10 @@ private async Task<OpenApiRequestBody> GetFormRequestBody(
462473
}
463474
else
464475
{
476+
if (IsRequired(description))
477+
{
478+
schema.Required.Add(description.Name);
479+
}
465480
schema.Properties[description.Name] = parameterSchema;
466481
}
467482
}

‎src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=controllers.verified.txt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,8 @@
7979
}
8080
}
8181
}
82-
}
82+
},
83+
"required": true
8384
},
8485
"responses": {
8586
"200": {

‎src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=forms.verified.txt‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@
1414
"content": {
1515
"multipart/form-data": {
1616
"schema": {
17+
"required": [
18+
"resume"
19+
],
1720
"type": "object",
1821
"properties": {
1922
"resume": {
@@ -41,6 +44,9 @@
4144
"content": {
4245
"multipart/form-data": {
4346
"schema": {
47+
"required": [
48+
"files"
49+
],
4450
"type": "object",
4551
"properties": {
4652
"files": {
@@ -68,6 +74,10 @@
6874
"content": {
6975
"multipart/form-data": {
7076
"schema": {
77+
"required": [
78+
"resume",
79+
"files"
80+
],
7181
"type": "object",
7282
"allOf": [
7383
{
@@ -135,6 +145,9 @@
135145
"content": {
136146
"multipart/form-data": {
137147
"schema": {
148+
"required": [
149+
"file"
150+
],
138151
"type": "object",
139152
"allOf": [
140153
{

‎src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.RequestBody.cs‎

Lines changed: 77 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ await VerifyOpenApiDocument(builder, document =>
3434
var paths = Assert.Single(document.Paths.Values);
3535
var operation = paths.Operations[OperationType.Post];
3636
Assert.NotNull(operation.RequestBody);
37-
Assert.False(operation.RequestBody.Required);
37+
Assert.True(operation.RequestBody.Required);
3838
Assert.NotNull(operation.RequestBody.Content);
3939
var content = Assert.Single(operation.RequestBody.Content);
4040
Assert.Equal("multipart/form-data", content.Key);
@@ -72,7 +72,16 @@ await VerifyOpenApiDocument(builder, document =>
7272
var paths = Assert.Single(document.Paths.Values);
7373
var operation = paths.Operations[OperationType.Post];
7474
Assert.NotNull(operation.RequestBody);
75-
Assert.Equal(!isOptional, operation.RequestBody.Required);
75+
Assert.True(operation.RequestBody.Required);
76+
var schema = operation.RequestBody.Content["multipart/form-data"].Schema;
77+
if (!isOptional)
78+
{
79+
Assert.Contains("formFile", schema.Required);
80+
}
81+
else
82+
{
83+
Assert.DoesNotContain("formFile", schema.Required);
84+
}
7685
});
7786
}
7887
#nullable restore
@@ -101,7 +110,7 @@ await VerifyOpenApiDocument(builder, document =>
101110
var paths = Assert.Single(document.Paths.Values);
102111
var operation = paths.Operations[OperationType.Post];
103112
Assert.NotNull(operation.RequestBody);
104-
Assert.False(operation.RequestBody.Required);
113+
Assert.True(operation.RequestBody.Required);
105114
Assert.NotNull(operation.RequestBody.Content);
106115
var content = Assert.Single(operation.RequestBody.Content);
107116
Assert.Equal("multipart/form-data", content.Key);
@@ -140,7 +149,16 @@ await VerifyOpenApiDocument(builder, document =>
140149
var paths = Assert.Single(document.Paths.Values);
141150
var operation = paths.Operations[OperationType.Post];
142151
Assert.NotNull(operation.RequestBody);
143-
Assert.Equal(!isOptional, operation.RequestBody.Required);
152+
Assert.True(operation.RequestBody.Required);
153+
var schema = operation.RequestBody.Content["multipart/form-data"].Schema;
154+
if (!isOptional)
155+
{
156+
Assert.Contains("formFile", schema.Required);
157+
}
158+
else
159+
{
160+
Assert.DoesNotContain("formFile", schema.Required);
161+
}
144162
});
145163
}
146164
#nullable restore
@@ -401,6 +419,10 @@ await VerifyOpenApiDocument(builder, document =>
401419
Assert.NotNull(item.Schema);
402420
Assert.Equal("object", item.Schema.Type);
403421
Assert.NotNull(item.Schema.Properties);
422+
Assert.Contains("id", item.Schema.Required);
423+
Assert.Contains("title", item.Schema.Required);
424+
Assert.Contains("completed", item.Schema.Required);
425+
Assert.Contains("createdAt", item.Schema.Required);
404426
Assert.Collection(item.Schema.Properties,
405427
property =>
406428
{
@@ -427,6 +449,57 @@ await VerifyOpenApiDocument(builder, document =>
427449
});
428450
}
429451

452+
// Test coverage for https://github.com/dotnet/aspnetcore/issues/57112
453+
[Fact]
454+
public async Task GetOpenApiRequestBody_HandlesFromFormWithRequiredPrimitive()
455+
{
456+
// Arrange
457+
var builder = CreateBuilder();
458+
459+
// Act
460+
builder.MapPost("/form", ([FromForm] int id, [FromForm] DateTime date, [FromForm] short? value) => { });
461+
462+
// Assert
463+
await VerifyOpenApiDocument(builder, document =>
464+
{
465+
var paths = Assert.Single(document.Paths.Values);
466+
var operation = paths.Operations[OperationType.Post];
467+
Assert.NotNull(operation.RequestBody);
468+
Assert.NotNull(operation.RequestBody.Content);
469+
var content = operation.RequestBody.Content;
470+
// Forms can be provided in both the URL and via form data
471+
Assert.Contains("application/x-www-form-urlencoded", content.Keys);
472+
// Same schema should be produced for both content-types
473+
foreach (var item in content.Values)
474+
{
475+
Assert.NotNull(item.Schema);
476+
Assert.Equal("object", item.Schema.Type);
477+
Assert.NotNull(item.Schema.Properties);
478+
// Assert that requiredness has been set for primitives
479+
Assert.Contains("id", item.Schema.Required);
480+
Assert.Contains("date", item.Schema.Required);
481+
Assert.DoesNotContain("value", item.Schema.Required);
482+
Assert.Collection(item.Schema.AllOf,
483+
subSchema =>
484+
{
485+
Assert.Contains("id", subSchema.Properties);
486+
Assert.Equal("integer", subSchema.Properties["id"].Type);
487+
},
488+
subSchema =>
489+
{
490+
Assert.Contains("date", subSchema.Properties);
491+
Assert.Equal("string", subSchema.Properties["date"].Type);
492+
Assert.Equal("date-time", subSchema.Properties["date"].Format);
493+
},
494+
subSchema =>
495+
{
496+
Assert.Contains("value", subSchema.Properties);
497+
Assert.Equal("integer", subSchema.Properties["value"].Type);
498+
});
499+
}
500+
});
501+
}
502+
430503
[Fact]
431504
public async Task GetOpenApiRequestBody_HandlesFromFormWithPoco_MvcAction()
432505
{

‎src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.RequestBodySchemas.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,10 @@ await VerifyOpenApiDocument(builder, document =>
135135
{
136136
Assert.True(GetRequestBodyForPath(document, "/required-poco").Required);
137137
Assert.False(GetRequestBodyForPath(document, "/non-required-poco").Required);
138+
// Form bodies are always required for form-based requests Individual elements
139+
// within the form can be optional.
138140
Assert.True(GetRequestBodyForPath(document, "/required-form").Required);
139-
Assert.False(GetRequestBodyForPath(document, "/non-required-form").Required);
141+
Assert.True(GetRequestBodyForPath(document, "/non-required-form").Required);
140142
});
141143

142144
static OpenApiRequestBody GetRequestBodyForPath(OpenApiDocument document, string path)

0 commit comments

Comments
 (0)