Skip to content

Commit 67ef969

Browse files
Youssef1313Youssef1313
committed
Handle nullability more accurately
1 parent 1591007 commit 67ef969

7 files changed

Lines changed: 170 additions & 106 deletions

File tree

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

Lines changed: 42 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@ namespace Microsoft.OpenApi
2121
/// </summary>
2222
public class OpenApiSchema : IOpenApiExtensible, IOpenApiSchema, IOpenApiSchemaMissingProperties, IOpenApiSchemaWithUnevaluatedProperties, IMetadataContainer
2323
{
24+
private static readonly OpenApiSchema s_enumWithSingleNullElement = new OpenApiSchema()
25+
{
26+
Enum = new List<JsonNode> { JsonNullSentinel.JsonNull },
27+
};
28+
2429
/// <inheritdoc />
2530
public string? Title { get; set; }
2631

@@ -110,12 +115,17 @@ public string? ExclusiveMinimum
110115
public JsonSchemaType? Type { get; set; }
111116

112117
// 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;
118+
internal bool IsNullable
119+
{
120+
get => field || HasNullableExtension || (Type.HasValue && Type.Value.HasFlag(JsonSchemaType.Null));
121+
set => field = value;
122+
}
123+
124+
private bool HasNullableExtension
125+
=> Extensions is not null &&
126+
Extensions.TryGetValue(OpenApiConstants.NullableExtension, out var nullExtRawValue) &&
127+
nullExtRawValue is JsonNodeExtension { Node: JsonNode jsonNode } &&
128+
jsonNode.GetValueKind() is JsonValueKind.True;
119129

120130
/// <inheritdoc />
121131
public string? Const { get; set; }
@@ -518,56 +528,36 @@ private void SerializeInternal(IOpenApiWriter writer, OpenApiSpecVersion version
518528
writer.WriteOptionalCollection(OpenApiConstants.Enum, enumValue, (nodeWriter, s) => nodeWriter.WriteAny(s));
519529

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

527534
if (version == OpenApiSpecVersion.OpenApi3_0)
528535
{
529-
(effectiveOneOf, var inferredOneOf, var nullInOneOf) = ProcessCompositionForNull(OneOf);
536+
(var inferredOneOf, var nullInOneOf) = ProcessCompositionForNull(OneOf);
530537
hasNullInComposition |= nullInOneOf;
531538
inferredType = inferredOneOf ?? inferredType;
532-
(effectiveAnyOf, var inferredAnyOf, var nullInAnyOf) = ProcessCompositionForNull(AnyOf);
539+
(var inferredAnyOf, var nullInAnyOf) = ProcessCompositionForNull(AnyOf);
533540
hasNullInComposition |= nullInAnyOf;
534541
inferredType = inferredAnyOf ?? inferredType;
535542

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

540550
// type
541-
SerializeTypeProperty(writer, version, inferredType);
551+
var serializedTypeProperty = SerializeTypeProperty(writer, version, inferredType);
542552

543553
// allOf
544554
writer.WriteOptionalCollection(OpenApiConstants.AllOf, AllOf, callback);
545555

546556
// anyOf
547-
writer.WriteOptionalCollection(OpenApiConstants.AnyOf, effectiveAnyOf, callback);
557+
writer.WriteOptionalCollection(OpenApiConstants.AnyOf, AnyOf, callback);
548558

549559
// 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-
560+
writer.WriteOptionalCollection(OpenApiConstants.OneOf, OneOf, callback);
571561

572562
// not
573563
writer.WriteOptionalObject(OpenApiConstants.Not, Not, callback);
@@ -603,8 +593,13 @@ private void SerializeInternal(IOpenApiWriter writer, OpenApiSpecVersion version
603593
writer.WriteOptionalObject(OpenApiConstants.Default, Default, (w, d) => w.WriteAny(d));
604594

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

@@ -995,14 +990,14 @@ private void SerializeAsV2(
995990
writer.WriteEndObject();
996991
}
997992

998-
private void SerializeTypeProperty(IOpenApiWriter writer, OpenApiSpecVersion version, JsonSchemaType? inferredType = null)
993+
private bool SerializeTypeProperty(IOpenApiWriter writer, OpenApiSpecVersion version, JsonSchemaType? inferredType = null)
999994
{
1000995
// Use original type or inferred type when the explicit type is not set
1001996
var typeToUse = Type ?? inferredType;
1002997

1003998
if (typeToUse is null)
1004999
{
1005-
return;
1000+
return false;
10061001
}
10071002

10081003
var unifiedType = IsNullable ? typeToUse.Value | JsonSchemaType.Null : typeToUse.Value;
@@ -1014,12 +1009,15 @@ private void SerializeTypeProperty(IOpenApiWriter writer, OpenApiSpecVersion ver
10141009
if (typeWithoutNull != 0 && !HasMultipleTypes(typeWithoutNull))
10151010
{
10161011
writer.WriteProperty(OpenApiConstants.Type, typeWithoutNull.ToFirstIdentifier());
1012+
return true;
10171013
}
10181014
break;
10191015
default:
10201016
WriteUnifiedSchemaType(unifiedType, writer);
1021-
break;
1017+
return true;
10221018
}
1019+
1020+
return false;
10231021
}
10241022

10251023
private static bool IsPowerOfTwo(int x)
@@ -1075,13 +1073,13 @@ private void SerializeNullable(IOpenApiWriter writer, OpenApiSpecVersion version
10751073
/// </summary>
10761074
/// <param name="composition">The list of schemas in the composition.</param>
10771075
/// <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)
1076+
private static (JsonSchemaType? inferredType, bool hasNullInComposition)
10791077
ProcessCompositionForNull(IList<IOpenApiSchema>? composition)
10801078
{
10811079
if (composition is null || !composition.Any(static s => s.Type is JsonSchemaType.Null))
10821080
{
10831081
// Nothing to patch
1084-
return (composition, null, false);
1082+
return (null, false);
10851083
}
10861084

10871085
var nonNullSchemas = composition
@@ -1114,16 +1112,14 @@ private static (IList<IOpenApiSchema>? effective, JsonSchemaType? inferredType,
11141112

11151113
commonType |= currentType;
11161114
}
1117-
1118-
commonType |= JsonSchemaType.String;
11191115
}
11201116
}
11211117

1122-
return (nonNullSchemas, commonType == 0 ? null : commonType, true);
1118+
return (commonType == 0 ? null : commonType, true);
11231119
}
11241120
else
11251121
{
1126-
return (null, null, true);
1122+
return (null, true);
11271123
}
11281124
}
11291125

‎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)