Skip to content

Commit 937b8eb

Browse files
Youssef1313Youssef1313
committed
Handle nullability more accurately
1 parent 1591007 commit 937b8eb

7 files changed

Lines changed: 167 additions & 106 deletions

File tree

‎src/Microsoft.OpenApi/Models/OpenApiSchema.cs‎

Lines changed: 39 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ namespace Microsoft.OpenApi
2121
/// </summary>
2222
public class OpenApiSchema : IOpenApiExtensible, IOpenApiSchema, IOpenApiSchemaMissingProperties, IOpenApiSchemaWithUnevaluatedProperties, IMetadataContainer
2323
{
24+
private static readonly List<JsonNode> s_singleNullElementList = [ JsonNullSentinel.JsonNull ];
25+
2426
/// <inheritdoc />
2527
public string? Title { get; set; }
2628

@@ -110,12 +112,17 @@ public string? ExclusiveMinimum
110112
public JsonSchemaType? Type { get; set; }
111113

112114
// x-nullable is filtered out by deserializers, but keep the check here in case it gets added from user code.
113-
private bool IsNullable =>
114-
(Type.HasValue && Type.Value.HasFlag(JsonSchemaType.Null)) ||
115-
Extensions is not null &&
116-
Extensions.TryGetValue(OpenApiConstants.NullableExtension, out var nullExtRawValue) &&
117-
nullExtRawValue is JsonNodeExtension { Node: JsonNode jsonNode } &&
118-
jsonNode.GetValueKind() is JsonValueKind.True;
115+
internal bool IsNullable
116+
{
117+
get => field || HasNullableExtension || (Type.HasValue && Type.Value.HasFlag(JsonSchemaType.Null));
118+
set => field = value;
119+
}
120+
121+
private bool HasNullableExtension
122+
=> Extensions is not null &&
123+
Extensions.TryGetValue(OpenApiConstants.NullableExtension, out var nullExtRawValue) &&
124+
nullExtRawValue is JsonNodeExtension { Node: JsonNode jsonNode } &&
125+
jsonNode.GetValueKind() is JsonValueKind.True;
119126

120127
/// <inheritdoc />
121128
public string? Const { get; set; }
@@ -518,56 +525,36 @@ private void SerializeInternal(IOpenApiWriter writer, OpenApiSpecVersion version
518525
writer.WriteOptionalCollection(OpenApiConstants.Enum, enumValue, (nodeWriter, s) => nodeWriter.WriteAny(s));
519526

520527
// Handle oneOf/anyOf with null type for v3.0 downcast
521-
IList<IOpenApiSchema>? effectiveOneOf = OneOf;
522-
IList<IOpenApiSchema>? effectiveAnyOf = AnyOf;
523528
bool hasNullInComposition = false;
524-
bool hasOneOfNullAndSingleEnumWith3_0 = false;
525529
JsonSchemaType? inferredType = null;
526530

527531
if (version == OpenApiSpecVersion.OpenApi3_0)
528532
{
529-
(effectiveOneOf, var inferredOneOf, var nullInOneOf) = ProcessCompositionForNull(OneOf);
533+
(var inferredOneOf, var nullInOneOf) = ProcessCompositionForNull(OneOf);
530534
hasNullInComposition |= nullInOneOf;
531535
inferredType = inferredOneOf ?? inferredType;
532-
(effectiveAnyOf, var inferredAnyOf, var nullInAnyOf) = ProcessCompositionForNull(AnyOf);
536+
(var inferredAnyOf, var nullInAnyOf) = ProcessCompositionForNull(AnyOf);
533537
hasNullInComposition |= nullInAnyOf;
534538
inferredType = inferredAnyOf ?? inferredType;
535539

536-
hasOneOfNullAndSingleEnumWith3_0 = nullInOneOf && effectiveOneOf is { Count: 1 } &&
537-
effectiveOneOf[0].Enum is { Count: > 0 };
540+
// If we have a schema that's only just { "type": "null" }, we serialize it as enum with null value.
541+
if (Type == JsonSchemaType.Null && OneOf is not { Count: > 0 } && AnyOf is not { Count: > 0 })
542+
{
543+
writer.WriteOptionalCollection(OpenApiConstants.Enum, s_singleNullElementList, (nodeWriter, s) => nodeWriter.WriteAny(s));
544+
}
538545
}
539546

540547
// type
541-
SerializeTypeProperty(writer, version, inferredType);
548+
var serializedTypeProperty = SerializeTypeProperty(writer, version, inferredType);
542549

543550
// allOf
544551
writer.WriteOptionalCollection(OpenApiConstants.AllOf, AllOf, callback);
545552

546553
// anyOf
547-
writer.WriteOptionalCollection(OpenApiConstants.AnyOf, effectiveAnyOf, callback);
554+
writer.WriteOptionalCollection(OpenApiConstants.AnyOf, AnyOf, callback);
548555

549556
// oneOf
550-
if (hasOneOfNullAndSingleEnumWith3_0)
551-
{
552-
writer.WriteRequiredCollection(OpenApiConstants.OneOf, effectiveOneOf!, (writer, element) =>
553-
{
554-
var clonedToMutateEnum = element.CreateShallowCopy();
555-
if (clonedToMutateEnum is OpenApiSchema { Enum: { } existingEnum } concreteCloned)
556-
{
557-
concreteCloned.Enum = [.. existingEnum, JsonNullSentinel.JsonNull];
558-
callback(writer, clonedToMutateEnum);
559-
}
560-
else
561-
{
562-
callback(writer, element);
563-
}
564-
});
565-
}
566-
else
567-
{
568-
writer.WriteOptionalCollection(OpenApiConstants.OneOf, effectiveOneOf, callback);
569-
}
570-
557+
writer.WriteOptionalCollection(OpenApiConstants.OneOf, OneOf, callback);
571558

572559
// not
573560
writer.WriteOptionalObject(OpenApiConstants.Not, Not, callback);
@@ -603,8 +590,13 @@ private void SerializeInternal(IOpenApiWriter writer, OpenApiSpecVersion version
603590
writer.WriteOptionalObject(OpenApiConstants.Default, Default, (w, d) => w.WriteAny(d));
604591

605592
// nullable
606-
if (version == OpenApiSpecVersion.OpenApi3_0)
593+
if (version == OpenApiSpecVersion.OpenApi3_0 && serializedTypeProperty)
607594
{
595+
// https://spec.openapis.org/oas/v3.0.4.html#fixed-fields-20
596+
// This keyword only takes effect if type is explicitly defined within the same Schema Object.
597+
//
598+
// If the user explicitly set IsNullable to true, we serialize it even if redundant.
599+
// But if **we** are inferring it (from oneOf/anyOf), we don't serialize it when it's redundant.
608600
SerializeNullable(writer, version, hasNullInComposition);
609601
}
610602

@@ -995,14 +987,14 @@ private void SerializeAsV2(
995987
writer.WriteEndObject();
996988
}
997989

998-
private void SerializeTypeProperty(IOpenApiWriter writer, OpenApiSpecVersion version, JsonSchemaType? inferredType = null)
990+
private bool SerializeTypeProperty(IOpenApiWriter writer, OpenApiSpecVersion version, JsonSchemaType? inferredType = null)
999991
{
1000992
// Use original type or inferred type when the explicit type is not set
1001993
var typeToUse = Type ?? inferredType;
1002994

1003995
if (typeToUse is null)
1004996
{
1005-
return;
997+
return false;
1006998
}
1007999

10081000
var unifiedType = IsNullable ? typeToUse.Value | JsonSchemaType.Null : typeToUse.Value;
@@ -1014,12 +1006,15 @@ private void SerializeTypeProperty(IOpenApiWriter writer, OpenApiSpecVersion ver
10141006
if (typeWithoutNull != 0 && !HasMultipleTypes(typeWithoutNull))
10151007
{
10161008
writer.WriteProperty(OpenApiConstants.Type, typeWithoutNull.ToFirstIdentifier());
1009+
return true;
10171010
}
10181011
break;
10191012
default:
10201013
WriteUnifiedSchemaType(unifiedType, writer);
1021-
break;
1014+
return true;
10221015
}
1016+
1017+
return false;
10231018
}
10241019

10251020
private static bool IsPowerOfTwo(int x)
@@ -1075,13 +1070,13 @@ private void SerializeNullable(IOpenApiWriter writer, OpenApiSpecVersion version
10751070
/// </summary>
10761071
/// <param name="composition">The list of schemas in the composition.</param>
10771072
/// <returns>A tuple with the effective list, inferred type, and whether null is present in composition.</returns>
1078-
private static (IList<IOpenApiSchema>? effective, JsonSchemaType? inferredType, bool hasNullInComposition)
1073+
private static (JsonSchemaType? inferredType, bool hasNullInComposition)
10791074
ProcessCompositionForNull(IList<IOpenApiSchema>? composition)
10801075
{
10811076
if (composition is null || !composition.Any(static s => s.Type is JsonSchemaType.Null))
10821077
{
10831078
// Nothing to patch
1084-
return (composition, null, false);
1079+
return (null, false);
10851080
}
10861081

10871082
var nonNullSchemas = composition
@@ -1114,16 +1109,14 @@ private static (IList<IOpenApiSchema>? effective, JsonSchemaType? inferredType,
11141109

11151110
commonType |= currentType;
11161111
}
1117-
1118-
commonType |= JsonSchemaType.String;
11191112
}
11201113
}
11211114

1122-
return (nonNullSchemas, commonType == 0 ? null : commonType, true);
1115+
return (commonType == 0 ? null : commonType, true);
11231116
}
11241117
else
11251118
{
1126-
return (null, null, true);
1119+
return (null, true);
11271120
}
11281121
}
11291122

‎src/Microsoft.OpenApi/Reader/V2/OpenApiSchemaDeserializer.cs‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
// Copyright (c) Microsoft Corporation. All rights reserved.
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT license.
33

4-
using System.Text.Json.Nodes;
5-
4+
using System;
65
using System.Collections.Generic;
76
using System.Globalization;
8-
using System;
97
using System.Linq;
8+
using System.Text.Json;
9+
using System.Text.Json.Nodes;
1010

1111
namespace Microsoft.OpenApi.Reader.V2
1212
{
@@ -268,16 +268,18 @@ public static IOpenApiSchema LoadSchema(JsonNode node, OpenApiDocument hostDocum
268268

269269
ParseMap(jsonObject, schema, _openApiSchemaFixedFields, _openApiSchemaPatternFields, hostDocument, context);
270270

271-
if (schema.Extensions is not null && schema.Extensions.ContainsKey(OpenApiConstants.NullableExtension))
271+
if (schema.Extensions is not null && schema.Extensions.TryGetValue(OpenApiConstants.NullableExtension, out var nullableExtension))
272272
{
273-
if (schema.Type.HasValue)
274-
schema.Type |= JsonSchemaType.Null;
275-
else
276-
schema.Type = JsonSchemaType.Null;
277-
273+
var isNullable = nullableExtension is JsonNodeExtension { Node: JsonNode jsonNode } && jsonNode.GetValueKind() is JsonValueKind.True;
274+
schema.IsNullable = isNullable;
278275
schema.Extensions.Remove(OpenApiConstants.NullableExtension);
279276
}
280277

278+
if (schema.IsNullable && schema.Type is not null && schema.Type != 0)
279+
{
280+
schema.Type |= JsonSchemaType.Null;
281+
}
282+
281283
return schema;
282284
}
283285
}

‎src/Microsoft.OpenApi/Reader/V3/OpenApiSchemaDeserializer.cs‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
1-
// Copyright (c) Microsoft Corporation. All rights reserved.
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT license.
33

4-
using System.Text.Json.Nodes;
5-
64
using System;
75
using System.Collections.Generic;
6+
using System.Data.SqlTypes;
87
using System.Globalization;
98
using System.Linq;
9+
using System.Text.Json;
10+
using System.Text.Json.Nodes;
1011

1112
namespace Microsoft.OpenApi.Reader.V3
1213
{
@@ -16,6 +17,8 @@ namespace Microsoft.OpenApi.Reader.V3
1617
/// </summary>
1718
internal static partial class OpenApiV3Deserializer
1819
{
20+
private static readonly IOpenApiExtension _nullableTrueExtension = new JsonNodeExtension(JsonValue.Create(true));
21+
1922
private static readonly FixedFieldMap<OpenApiSchema> _openApiSchemaFixedFields = new()
2023
{
2124
{
@@ -222,10 +225,7 @@ internal static partial class OpenApiV3Deserializer
222225
{
223226
if (bool.TryParse(n.GetScalarValue(), out var parsed) && parsed)
224227
{
225-
if (o.Type.HasValue)
226-
o.Type |= JsonSchemaType.Null;
227-
else
228-
o.Type = JsonSchemaType.Null;
228+
o.IsNullable = true;
229229
}
230230
}
231231
},
@@ -385,16 +385,18 @@ public static IOpenApiSchema LoadSchema(JsonNode node, OpenApiDocument hostDocum
385385

386386
ParseMap(jsonObject, schema, _openApiSchemaFixedFields, _openApiSchemaPatternFields, hostDocument, context);
387387

388-
if (schema.Extensions is not null && schema.Extensions.ContainsKey(OpenApiConstants.NullableExtension))
388+
if (schema.Extensions is not null && schema.Extensions.TryGetValue(OpenApiConstants.NullableExtension, out var nullableExtension))
389389
{
390-
if (schema.Type.HasValue)
391-
schema.Type |= JsonSchemaType.Null;
392-
else
393-
schema.Type = JsonSchemaType.Null;
394-
390+
var isNullable = nullableExtension is JsonNodeExtension { Node: JsonNode jsonNode } && jsonNode.GetValueKind() is JsonValueKind.True;
391+
schema.IsNullable = isNullable;
395392
schema.Extensions.Remove(OpenApiConstants.NullableExtension);
396393
}
397394

395+
if (schema.IsNullable && schema.Type is not null && schema.Type != 0)
396+
{
397+
schema.Type |= JsonSchemaType.Null;
398+
}
399+
398400
return schema;
399401
}
400402
}

‎src/Microsoft.OpenApi/Reader/V31/OpenApiSchemaDeserializer.cs‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Collections.Generic;
66
using System.Globalization;
77
using System.Linq;
8+
using System.Text.Json;
89
using System.Text.Json.Nodes;
910

1011
namespace Microsoft.OpenApi.Reader.V31;
@@ -332,14 +333,7 @@ internal static partial class OpenApiV31Deserializer
332333
var value = n.GetScalarValue();
333334
if (value is not null)
334335
{
335-
var nullable = bool.Parse(value);
336-
if (nullable) // if nullable, convert type into an array of type(s) and null
337-
{
338-
if (o.Type.HasValue)
339-
o.Type |= JsonSchemaType.Null;
340-
else
341-
o.Type = JsonSchemaType.Null;
342-
}
336+
o.IsNullable = bool.Parse(value);
343337
}
344338
}
345339
},
@@ -476,16 +470,18 @@ public static IOpenApiSchema LoadSchema(JsonNode node, OpenApiDocument hostDocum
476470
schema.UnrecognizedKeywords[name] = value;
477471
});
478472

479-
if (schema.Extensions is not null && schema.Extensions.ContainsKey(OpenApiConstants.NullableExtension))
473+
if (schema.Extensions is not null && schema.Extensions.TryGetValue(OpenApiConstants.NullableExtension, out var nullableExtension))
480474
{
481-
if (schema.Type.HasValue)
482-
schema.Type |= JsonSchemaType.Null;
483-
else
484-
schema.Type = JsonSchemaType.Null;
485-
475+
var isNullable = nullableExtension is JsonNodeExtension { Node: JsonNode jsonNode } && jsonNode.GetValueKind() is JsonValueKind.True;
476+
schema.IsNullable = isNullable;
486477
schema.Extensions.Remove(OpenApiConstants.NullableExtension);
487478
}
488479

480+
if (schema.IsNullable && schema.Type is not null && schema.Type != 0)
481+
{
482+
schema.Type |= JsonSchemaType.Null;
483+
}
484+
489485
if (!string.IsNullOrEmpty(identifier) && hostDocument.Workspace is not null)
490486
{
491487
// register the schema in our registry using the identifier's URL

‎src/Microsoft.OpenApi/Reader/V32/OpenApiSchemaDeserializer.cs‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Collections.Generic;
66
using System.Globalization;
77
using System.Linq;
8+
using System.Text.Json;
89
using System.Text.Json.Nodes;
910

1011
namespace Microsoft.OpenApi.Reader.V32;
@@ -332,14 +333,7 @@ internal static partial class OpenApiV32Deserializer
332333
var value = n.GetScalarValue();
333334
if (value is not null)
334335
{
335-
var nullable = bool.Parse(value);
336-
if (nullable) // if nullable, convert type into an array of type(s) and null
337-
{
338-
if (o.Type.HasValue)
339-
o.Type |= JsonSchemaType.Null;
340-
else
341-
o.Type = JsonSchemaType.Null;
342-
}
336+
o.IsNullable = bool.Parse(value);
343337
}
344338
}
345339
},
@@ -476,16 +470,18 @@ public static IOpenApiSchema LoadSchema(JsonNode node, OpenApiDocument hostDocum
476470
schema.UnrecognizedKeywords[name] = value;
477471
});
478472

479-
if (schema.Extensions is not null && schema.Extensions.ContainsKey(OpenApiConstants.NullableExtension))
473+
if (schema.Extensions is not null && schema.Extensions.TryGetValue(OpenApiConstants.NullableExtension, out var nullableExtension))
480474
{
481-
if (schema.Type.HasValue)
482-
schema.Type |= JsonSchemaType.Null;
483-
else
484-
schema.Type = JsonSchemaType.Null;
485-
475+
var isNullable = nullableExtension is JsonNodeExtension { Node: JsonNode jsonNode } && jsonNode.GetValueKind() is JsonValueKind.True;
476+
schema.IsNullable = isNullable;
486477
schema.Extensions.Remove(OpenApiConstants.NullableExtension);
487478
}
488479

480+
if (schema.IsNullable && schema.Type is not null && schema.Type != 0)
481+
{
482+
schema.Type |= JsonSchemaType.Null;
483+
}
484+
489485
if (!string.IsNullOrEmpty(identifier) && hostDocument.Workspace is not null)
490486
{
491487
// register the schema in our registry using the identifier's URL

0 commit comments

Comments
 (0)