Skip to content

Commit 0e4ccd1

Browse files
captainsafiacaptainsafia
authored
Fix up nullability and form schema handling in OpenAPI (#56957)
* Parameters sourced from query, header, and route are never nullable * Don't set nullable property on form parameters * Remove default form encoding and set schema IDs * Add more tests
1 parent 0fee04e commit 0e4ccd1

8 files changed

+158
-119
lines changed

‎src/OpenApi/src/Extensions/JsonNodeSchemaExtensions.cs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
using System.Text.Json.Serialization.Metadata;
1313
using Microsoft.AspNetCore.Mvc.ApiExplorer;
1414
using Microsoft.AspNetCore.Mvc.Infrastructure;
15+
using Microsoft.AspNetCore.Mvc.ModelBinding;
1516
using Microsoft.AspNetCore.Routing;
1617
using Microsoft.AspNetCore.Routing.Constraints;
1718
using Microsoft.OpenApi.Models;
@@ -317,6 +318,19 @@ internal static void ApplyParameterInfo(this JsonNode schema, ApiParameterDescri
317318
{
318319
schema.ApplyRouteConstraints(constraints);
319320
}
321+
322+
if (parameterDescription.Source is { } bindingSource && SupportsNullableProperty(bindingSource))
323+
{
324+
schema[OpenApiSchemaKeywords.NullableKeyword] = false;
325+
}
326+
327+
// Parameters sourced from the header, query, route, and/or form cannot be nullable based on our binding
328+
// rules but can be optional.
329+
static bool SupportsNullableProperty(BindingSource bindingSource) =>bindingSource == BindingSource.Header
330+
|| bindingSource == BindingSource.Query
331+
|| bindingSource == BindingSource.Path
332+
|| bindingSource == BindingSource.Form
333+
|| bindingSource == BindingSource.FormFile;
320334
}
321335

322336
/// <summary>

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,6 @@ internal sealed class OpenApiDocumentService(
3838
private readonly OpenApiSchemaService _componentService = serviceProvider.GetRequiredKeyedService<OpenApiSchemaService>(documentName);
3939
private readonly OpenApiSchemaReferenceTransformer _schemaReferenceTransformer = new();
4040

41-
private static readonly OpenApiEncoding _defaultFormEncoding = new() { Style = ParameterStyle.Form, Explode = true };
42-
4341
/// <summary>
4442
/// Cache of <see cref="OpenApiOperationTransformerContext"/> instances keyed by the
4543
/// `ApiDescription.ActionDescriptor.Id` of the associated operation. ActionDescriptor IDs
@@ -407,7 +405,7 @@ private async Task<OpenApiRequestBody> GetFormRequestBody(
407405
if (parameter.All(parameter => parameter.ModelMetadata.ContainerType is null))
408406
{
409407
var description = parameter.Single();
410-
var parameterSchema = await _componentService.GetOrCreateSchemaAsync(description.Type, null, cancellationToken: cancellationToken);
408+
var parameterSchema = await _componentService.GetOrCreateSchemaAsync(description.Type, description, cancellationToken: cancellationToken);
411409
// Form files are keyed by their parameter name so we must capture the parameter name
412410
// as a property in the schema.
413411
if (description.Type == typeof(IFormFile) || description.Type == typeof(IFormFileCollection))
@@ -476,15 +474,15 @@ private async Task<OpenApiRequestBody> GetFormRequestBody(
476474
var propertySchema = new OpenApiSchema { Type = "object", Properties = new Dictionary<string, OpenApiSchema>() };
477475
foreach (var description in parameter)
478476
{
479-
propertySchema.Properties[description.Name] = await _componentService.GetOrCreateSchemaAsync(description.Type, null, cancellationToken: cancellationToken);
477+
propertySchema.Properties[description.Name] = await _componentService.GetOrCreateSchemaAsync(description.Type, description, cancellationToken: cancellationToken);
480478
}
481479
schema.AllOf.Add(propertySchema);
482480
}
483481
else
484482
{
485483
foreach (var description in parameter)
486484
{
487-
schema.Properties[description.Name] = await _componentService.GetOrCreateSchemaAsync(description.Type, null, cancellationToken: cancellationToken);
485+
schema.Properties[description.Name] = await _componentService.GetOrCreateSchemaAsync(description.Type, description, cancellationToken: cancellationToken);
488486
}
489487
}
490488
}
@@ -495,8 +493,7 @@ private async Task<OpenApiRequestBody> GetFormRequestBody(
495493
var contentType = requestFormat.MediaType;
496494
requestBody.Content[contentType] = new OpenApiMediaType
497495
{
498-
Schema = schema,
499-
Encoding = new Dictionary<string, OpenApiEncoding>() { [contentType] = _defaultFormEncoding }
496+
Schema = schema
500497
};
501498
}
502499

‎src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,8 @@ internal sealed class OpenApiSchemaService(
6666
schema = new JsonObject
6767
{
6868
[OpenApiSchemaKeywords.TypeKeyword] = "string",
69-
[OpenApiSchemaKeywords.FormatKeyword] = "binary"
69+
[OpenApiSchemaKeywords.FormatKeyword] = "binary",
70+
[OpenApiConstants.SchemaId] = "IFormFile"
7071
};
7172
}
7273
else if (type == typeof(IFormFileCollection))
@@ -77,7 +78,8 @@ internal sealed class OpenApiSchemaService(
7778
[OpenApiSchemaKeywords.ItemsKeyword] = new JsonObject
7879
{
7980
[OpenApiSchemaKeywords.TypeKeyword] = "string",
80-
[OpenApiSchemaKeywords.FormatKeyword] = "binary"
81+
[OpenApiSchemaKeywords.FormatKeyword] = "binary",
82+
[OpenApiConstants.SchemaId] = "IFormFile"
8183
}
8284
};
8385
}

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,6 @@
7777
"type": "boolean"
7878
}
7979
}
80-
},
81-
"encoding": {
82-
"application/x-www-form-urlencoded": {
83-
"style": "form",
84-
"explode": true
85-
}
8680
}
8781
}
8882
}

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

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,6 @@
2020
"$ref": "#/components/schemas/IFormFile"
2121
}
2222
}
23-
},
24-
"encoding": {
25-
"multipart/form-data": {
26-
"style": "form",
27-
"explode": true
28-
}
2923
}
3024
}
3125
},
@@ -53,12 +47,6 @@
5347
"$ref": "#/components/schemas/IFormFileCollection"
5448
}
5549
}
56-
},
57-
"encoding": {
58-
"multipart/form-data": {
59-
"style": "form",
60-
"explode": true
61-
}
6250
}
6351
}
6452
},
@@ -99,12 +87,6 @@
9987
}
10088
}
10189
]
102-
},
103-
"encoding": {
104-
"multipart/form-data": {
105-
"style": "form",
106-
"explode": true
107-
}
10890
}
10991
}
11092
},
@@ -127,23 +109,11 @@
127109
"multipart/form-data": {
128110
"schema": {
129111
"$ref": "#/components/schemas/Todo"
130-
},
131-
"encoding": {
132-
"multipart/form-data": {
133-
"style": "form",
134-
"explode": true
135-
}
136112
}
137113
},
138114
"application/x-www-form-urlencoded": {
139115
"schema": {
140116
"$ref": "#/components/schemas/Todo"
141-
},
142-
"encoding": {
143-
"application/x-www-form-urlencoded": {
144-
"style": "form",
145-
"explode": true
146-
}
147117
}
148118
}
149119
},
@@ -179,12 +149,6 @@
179149
}
180150
}
181151
]
182-
},
183-
"encoding": {
184-
"multipart/form-data": {
185-
"style": "form",
186-
"explode": true
187-
}
188152
}
189153
}
190154
},

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

Lines changed: 42 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -10,31 +10,6 @@
1010

1111
public partial class OpenApiDocumentServiceTests : OpenApiDocumentServiceTestBase
1212
{
13-
[Fact]
14-
public async Task GetRequestBody_VerifyDefaultFormEncoding()
15-
{
16-
// Arrange
17-
var builder = CreateBuilder();
18-
19-
// Act
20-
builder.MapPost("/", (IFormFile formFile) => { });
21-
22-
// Assert -- The defaults for form encoding are Explode = true and Style = Form
23-
// which align with the encoding formats that are used by ASP.NET Core's binding layer.
24-
await VerifyOpenApiDocument(builder, document =>
25-
{
26-
var paths = Assert.Single(document.Paths.Values);
27-
var operation = paths.Operations[OperationType.Post];
28-
Assert.NotNull(operation.RequestBody);
29-
Assert.NotNull(operation.RequestBody.Content);
30-
var content = Assert.Single(operation.RequestBody.Content);
31-
Assert.Equal("multipart/form-data", content.Key);
32-
var encoding = content.Value.Encoding["multipart/form-data"];
33-
Assert.True(encoding.Explode);
34-
Assert.Equal(ParameterStyle.Form, encoding.Style);
35-
});
36-
}
37-
3813
[Theory]
3914
[InlineData(false)]
4015
[InlineData(true)]
@@ -681,6 +656,48 @@ private class ModelWithSingleProperty
681656
public string Name { get; set; }
682657
}
683658

659+
[Fact]
660+
public async Task GetOpenApiRequestBody_HandlesFromFormWithNullableProperties_MvcAction()
661+
{
662+
// Arrange
663+
var action = CreateActionDescriptor(nameof(ActionWithFormModelNullableProps));
664+
665+
// Assert
666+
await VerifyOpenApiDocument(action, document =>
667+
{
668+
var paths = Assert.Single(document.Paths.Values);
669+
var operation = paths.Operations[OperationType.Get];
670+
Assert.NotNull(operation.RequestBody);
671+
Assert.NotNull(operation.RequestBody.Content);
672+
var content = operation.RequestBody.Content;
673+
// Forms can be provided in both the URL and via form data
674+
Assert.Contains("application/x-www-form-urlencoded", content.Keys);
675+
// Assert that all properties within the form schema are not marked as nullable
676+
foreach (var item in content.Values)
677+
{
678+
Assert.NotNull(item.Schema);
679+
Assert.Equal("object", item.Schema.Type);
680+
Assert.NotNull(item.Schema.Properties);
681+
Assert.All(item.Schema.Properties,
682+
property =>
683+
{
684+
Assert.False(property.Value.Nullable);
685+
});
686+
}
687+
});
688+
}
689+
690+
[Route("/form-model-nullable")]
691+
private void ActionWithFormModelNullableProps([FromForm] ModelWithNullableProperties model) { }
692+
693+
#nullable enable
694+
private class ModelWithNullableProperties
695+
{
696+
public string? Name { get; set; }
697+
public int? Age { get; set; }
698+
}
699+
#nullable restore
700+
684701
[Fact]
685702
public async Task GetOpenApiRequestBody_HandlesFormModelWithFile_MvcAction()
686703
{

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

Lines changed: 41 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -18,46 +18,46 @@ public partial class OpenApiSchemaServiceTests : OpenApiDocumentServiceTestBase
1818
#nullable enable
1919
public static object?[][] RouteParametersWithPrimitiveTypes =>
2020
[
21-
[(int id) => {}, "integer", "int32", false],
22-
[(long id) => {}, "integer", "int64", false],
23-
[(float id) => {}, "number", "float", false],
24-
[(double id) => {}, "number", "double", false],
25-
[(decimal id) => {}, "number", "double", false],
26-
[(bool id) => {}, "boolean", null, false],
27-
[(string id) => {}, "string", null, false],
28-
[(char id) => {}, "string", "char", false],
29-
[(byte id) => {}, "integer", "uint8", false],
30-
[(byte[] id) => {}, "string", "byte", false],
31-
[(short id) => {}, "integer", "int16", false],
32-
[(ushort id) => {}, "integer", "uint16", false],
33-
[(uint id) => {}, "integer", "uint32", false],
34-
[(ulong id) => {}, "integer", "uint64", false],
35-
[(Uri id) => {}, "string", "uri", false],
36-
[(TimeOnly id) => {}, "string", "time", false],
37-
[(DateOnly id) => {}, "string", "date", false],
38-
[(int? id) => {}, "integer", "int32", true],
39-
[(long? id) => {}, "integer", "int64", true],
40-
[(float? id) => {}, "number", "float", true],
41-
[(double? id) => {}, "number", "double", true],
42-
[(decimal? id) => {}, "number", "double", true],
43-
[(bool? id) => {}, "boolean", null, true],
44-
[(string? id) => {}, "string", null, true],
45-
[(char? id) => {}, "string", "char", true],
46-
[(byte? id) => {}, "integer", "uint8", true],
47-
[(byte[]? id) => {}, "string", "byte", true],
48-
[(short? id) => {}, "integer", "int16", true],
49-
[(ushort? id) => {}, "integer", "uint16", true],
50-
[(uint? id) => {}, "integer", "uint32", true],
51-
[(ulong? id) => {}, "integer", "uint64", true],
52-
[(Uri? id) => {}, "string", "uri", true],
53-
[(TimeOnly? id) => {}, "string", "time", true],
54-
[(DateOnly? id) => {}, "string", "date", true]
21+
[(int id) => {}, "integer", "int32"],
22+
[(long id) => {}, "integer", "int64"],
23+
[(float id) => {}, "number", "float"],
24+
[(double id) => {}, "number", "double"],
25+
[(decimal id) => {}, "number", "double"],
26+
[(bool id) => {}, "boolean", null],
27+
[(string id) => {}, "string", null],
28+
[(char id) => {}, "string", "char"],
29+
[(byte id) => {}, "integer", "uint8"],
30+
[(byte[] id) => {}, "string", "byte"],
31+
[(short id) => {}, "integer", "int16"],
32+
[(ushort id) => {}, "integer", "uint16"],
33+
[(uint id) => {}, "integer", "uint32"],
34+
[(ulong id) => {}, "integer", "uint64"],
35+
[(Uri id) => {}, "string", "uri"],
36+
[(TimeOnly id) => {}, "string", "time"],
37+
[(DateOnly id) => {}, "string", "date"],
38+
[(int? id) => {}, "integer", "int32"],
39+
[(long? id) => {}, "integer", "int64"],
40+
[(float? id) => {}, "number", "float"],
41+
[(double? id) => {}, "number", "double"],
42+
[(decimal? id) => {}, "number", "double"],
43+
[(bool? id) => {}, "boolean", null],
44+
[(string? id) => {}, "string", null],
45+
[(char? id) => {}, "string", "char"],
46+
[(byte? id) => {}, "integer", "uint8"],
47+
[(byte[]? id) => {}, "string", "byte"],
48+
[(short? id) => {}, "integer", "int16"],
49+
[(ushort? id) => {}, "integer", "uint16"],
50+
[(uint? id) => {}, "integer", "uint32"],
51+
[(ulong? id) => {}, "integer", "uint64"],
52+
[(Uri? id) => {}, "string", "uri"],
53+
[(TimeOnly? id) => {}, "string", "time"],
54+
[(DateOnly? id) => {}, "string", "date"]
5555
];
5656
#nullable restore
5757

5858
[Theory]
5959
[MemberData(nameof(RouteParametersWithPrimitiveTypes))]
60-
public async Task GetOpenApiParameters_HandlesRouteParameterWithPrimitiveType(Delegate requestHandler, string schemaType, string schemaFormat, bool isNullable)
60+
public async Task GetOpenApiParameters_HandlesRouteParameterWithPrimitiveType(Delegate requestHandler, string schemaType, string schemaFormat)
6161
{
6262
// Arrange
6363
var builder = CreateBuilder();
@@ -72,7 +72,7 @@ await VerifyOpenApiDocument(builder, document =>
7272
var parameter = Assert.Single(operation.Parameters);
7373
Assert.Equal(schemaType, parameter.Schema.Type);
7474
Assert.Equal(schemaFormat, parameter.Schema.Format);
75-
Assert.Equal(isNullable, parameter.Schema.Nullable);
75+
Assert.False(parameter.Schema.Nullable);
7676
});
7777
}
7878

@@ -420,6 +420,11 @@ await VerifyOpenApiDocument(builder, document =>
420420
var parameter = Assert.Single(operation.Parameters);
421421
Assert.Equal("array", parameter.Schema.Type);
422422
Assert.Equal(innerSchemaType, parameter.Schema.Items.Type);
423+
// Array items can be serialized to nullable values when the element
424+
// type is nullable. For example, array-of-ints?ints=1&ints=2&ints=&ints=4
425+
// will produce [1, 2, null, 4] when the parameter is int?[] ints.
426+
// When the element type is not nullable (int[] ints), the binding
427+
// will produce [1, 2, 0, 4]
423428
Assert.Equal(isNullable, parameter.Schema.Items.Nullable);
424429
});
425430
}

0 commit comments

Comments
 (0)