Skip to content

Commit 6c82aa6

Browse files
baywetbaywet
authored
Merge pull request #2358 from bkoelman/nullable-fixes
fix: nullable and type ordering should be maintain to ease up migration work fix: nullable should not be inserted as an attempt to normalize the document
2 parents 5e456de + 32efa38 commit 6c82aa6

17 files changed

+195
-130
lines changed

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

Lines changed: 57 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,14 @@ public string? ExclusiveMinimum
104104
/// <inheritdoc />
105105
public JsonSchemaType? Type { get; set; }
106106

107+
// x-nullable is filtered out by deserializers, but keep the check here in case it gets added from user code.
108+
private bool IsNullable =>
109+
(Type.HasValue && Type.Value.HasFlag(JsonSchemaType.Null)) ||
110+
Extensions is not null &&
111+
Extensions.TryGetValue(OpenApiConstants.NullableExtension, out var nullExtRawValue) &&
112+
nullExtRawValue is JsonNodeExtension { Node: JsonNode jsonNode } &&
113+
jsonNode.GetValueKind() is JsonValueKind.True;
114+
107115
/// <inheritdoc />
108116
public string? Const { get; set; }
109117

@@ -437,7 +445,7 @@ private void SerializeInternal(IOpenApiWriter writer, OpenApiSpecVersion version
437445
writer.WriteOptionalCollection(OpenApiConstants.Enum, Enum, (nodeWriter, s) => nodeWriter.WriteAny(s));
438446

439447
// type
440-
SerializeTypeProperty(Type, writer, version);
448+
SerializeTypeProperty(writer, version);
441449

442450
// allOf
443451
writer.WriteOptionalCollection(OpenApiConstants.AllOf, AllOf, callback);
@@ -479,6 +487,12 @@ private void SerializeInternal(IOpenApiWriter writer, OpenApiSpecVersion version
479487
// default
480488
writer.WriteOptionalObject(OpenApiConstants.Default, Default, (w, d) => w.WriteAny(d));
481489

490+
// nullable
491+
if (version == OpenApiSpecVersion.OpenApi3_0)
492+
{
493+
SerializeNullable(writer, version);
494+
}
495+
482496
// discriminator
483497
writer.WriteOptionalObject(OpenApiConstants.Discriminator, Discriminator, callback);
484498

@@ -619,7 +633,7 @@ private void SerializeAsV2(
619633
writer.WriteStartObject();
620634

621635
// type
622-
SerializeTypeProperty(Type, writer, OpenApiSpecVersion.OpenApi2_0);
636+
SerializeTypeProperty(writer, OpenApiSpecVersion.OpenApi2_0);
623637

624638
// description
625639
writer.WriteProperty(OpenApiConstants.Description, Description);
@@ -742,68 +756,36 @@ private void SerializeAsV2(
742756
// example
743757
writer.WriteOptionalObject(OpenApiConstants.Example, Example, (w, e) => w.WriteAny(e));
744758

759+
// x-nullable extension
760+
SerializeNullable(writer, OpenApiSpecVersion.OpenApi2_0);
761+
745762
// extensions
746763
writer.WriteExtensions(Extensions, OpenApiSpecVersion.OpenApi2_0);
747764

748765
writer.WriteEndObject();
749766
}
750767

751-
private void SerializeTypeProperty(JsonSchemaType? type, IOpenApiWriter writer, OpenApiSpecVersion version)
768+
private void SerializeTypeProperty(IOpenApiWriter writer, OpenApiSpecVersion version)
752769
{
753-
// check whether nullable is true for upcasting purposes
754-
var isNullable = (Type.HasValue && Type.Value.HasFlag(JsonSchemaType.Null)) ||
755-
Extensions is not null &&
756-
Extensions.TryGetValue(OpenApiConstants.NullableExtension, out var nullExtRawValue) &&
757-
nullExtRawValue is JsonNodeExtension { Node: JsonNode jsonNode } &&
758-
jsonNode.GetValueKind() is JsonValueKind.True;
759-
if (type is null)
770+
if (Type is null)
760771
{
761-
if (version is OpenApiSpecVersion.OpenApi3_0 && isNullable)
762-
{
763-
writer.WriteProperty(OpenApiConstants.Nullable, true);
764-
}
772+
return;
765773
}
766-
else if (!HasMultipleTypes(type.Value))
767-
{
768774

769-
switch (version)
770-
{
771-
case OpenApiSpecVersion.OpenApi3_1 when isNullable:
772-
UpCastSchemaTypeToV31(type.Value, writer);
773-
break;
774-
case OpenApiSpecVersion.OpenApi3_0 when isNullable && type.Value == JsonSchemaType.Null:
775-
writer.WriteProperty(OpenApiConstants.Nullable, true);
776-
writer.WriteProperty(OpenApiConstants.Type, JsonSchemaType.Object.ToFirstIdentifier());
777-
break;
778-
case OpenApiSpecVersion.OpenApi3_0 when isNullable && type.Value != JsonSchemaType.Null:
779-
writer.WriteProperty(OpenApiConstants.Nullable, true);
780-
writer.WriteProperty(OpenApiConstants.Type, type.Value.ToFirstIdentifier());
781-
break;
782-
default:
783-
writer.WriteProperty(OpenApiConstants.Type, type.Value.ToFirstIdentifier());
784-
break;
785-
}
786-
}
787-
else
775+
var unifiedType = IsNullable ? Type.Value | JsonSchemaType.Null : Type.Value;
776+
var typeWithoutNull = unifiedType & ~JsonSchemaType.Null;
777+
778+
switch (version)
788779
{
789-
// type
790-
if (version is OpenApiSpecVersion.OpenApi2_0 || version is OpenApiSpecVersion.OpenApi3_0)
791-
{
792-
DowncastTypeArrayToV2OrV3(type.Value, writer, version);
793-
}
794-
else
795-
{
796-
var list = (from JsonSchemaType flag in jsonSchemaTypeValues
797-
where type.Value.HasFlag(flag)
798-
select flag).ToList();
799-
writer.WriteOptionalCollection(OpenApiConstants.Type, list, (w, s) =>
780+
case OpenApiSpecVersion.OpenApi2_0 or OpenApiSpecVersion.OpenApi3_0:
781+
if (typeWithoutNull != 0 && !HasMultipleTypes(typeWithoutNull))
800782
{
801-
foreach (var item in s.ToIdentifiers())
802-
{
803-
w.WriteValue(item);
804-
}
805-
});
806-
}
783+
writer.WriteProperty(OpenApiConstants.Type, typeWithoutNull.ToFirstIdentifier());
784+
}
785+
break;
786+
default:
787+
WriteUnifiedSchemaType(unifiedType, writer);
788+
break;
807789
}
808790
}
809791

@@ -815,20 +797,17 @@ private static bool IsPowerOfTwo(int x)
815797
private static bool HasMultipleTypes(JsonSchemaType schemaType)
816798
{
817799
var schemaTypeNumeric = (int)schemaType;
818-
return !IsPowerOfTwo(schemaTypeNumeric) && // Boolean, Integer, Number, String, Array, Object
819-
schemaTypeNumeric != (int)JsonSchemaType.Null;
800+
return !IsPowerOfTwo(schemaTypeNumeric);
820801
}
821802

822-
private static void UpCastSchemaTypeToV31(JsonSchemaType type, IOpenApiWriter writer)
803+
private static void WriteUnifiedSchemaType(JsonSchemaType type, IOpenApiWriter writer)
823804
{
824-
// create a new array and insert the type and "null" as values
825-
var temporaryType = type | JsonSchemaType.Null;
826-
var list = (from JsonSchemaType flag in jsonSchemaTypeValues// Check if the flag is set in 'type' using a bitwise AND operation
827-
where temporaryType.HasFlag(flag)
828-
select flag.ToFirstIdentifier()).ToList();
829-
if (list.Count > 1)
805+
var array = (from JsonSchemaType flag in jsonSchemaTypeValues
806+
where type.HasFlag(flag)
807+
select flag.ToFirstIdentifier()).ToArray();
808+
if (array.Length > 1)
830809
{
831-
writer.WriteOptionalCollection(OpenApiConstants.Type, list, (w, s) =>
810+
writer.WriteOptionalCollection(OpenApiConstants.Type, array, (w, s) =>
832811
{
833812
if (!string.IsNullOrEmpty(s) && s is not null)
834813
{
@@ -838,54 +817,32 @@ where temporaryType.HasFlag(flag)
838817
}
839818
else
840819
{
841-
writer.WriteProperty(OpenApiConstants.Type, list[0]);
820+
writer.WriteProperty(OpenApiConstants.Type, array[0]);
842821
}
843822
}
844823

845-
#if NET5_0_OR_GREATER
846-
private static readonly Array jsonSchemaTypeValues = System.Enum.GetValues<JsonSchemaType>();
847-
#else
848-
private static readonly Array jsonSchemaTypeValues = System.Enum.GetValues(typeof(JsonSchemaType));
849-
#endif
850-
851-
private static void DowncastTypeArrayToV2OrV3(JsonSchemaType schemaType, IOpenApiWriter writer, OpenApiSpecVersion version)
824+
private void SerializeNullable(IOpenApiWriter writer, OpenApiSpecVersion version)
852825
{
853-
/* If the array has one non-null value, emit Type as string
854-
* If the array has one null value, emit x-nullable as true
855-
* If the array has two values, one null and one non-null, emit Type as string and x-nullable as true
856-
* If the array has more than two values or two non-null values, do not emit type
857-
* */
858-
859-
var nullableProp = version.Equals(OpenApiSpecVersion.OpenApi2_0)
860-
? OpenApiConstants.NullableExtension
861-
: OpenApiConstants.Nullable;
862-
863-
if (!HasMultipleTypes(schemaType & ~JsonSchemaType.Null) && (schemaType & JsonSchemaType.Null) == JsonSchemaType.Null) // checks for two values and one is null
864-
{
865-
foreach (JsonSchemaType flag in jsonSchemaTypeValues)
866-
{
867-
// Skip if the flag is not set or if it's the Null flag
868-
if (schemaType.HasFlag(flag) && flag != JsonSchemaType.Null)
869-
{
870-
// Write the non-null flag value to the writer
871-
writer.WriteProperty(OpenApiConstants.Type, flag.ToFirstIdentifier());
872-
}
873-
}
874-
writer.WriteProperty(nullableProp, true);
875-
}
876-
else if (!HasMultipleTypes(schemaType))
826+
if (IsNullable)
877827
{
878-
if (schemaType is JsonSchemaType.Null)
879-
{
880-
writer.WriteProperty(nullableProp, true);
881-
}
882-
else
828+
switch (version)
883829
{
884-
writer.WriteProperty(OpenApiConstants.Type, schemaType.ToFirstIdentifier());
830+
case OpenApiSpecVersion.OpenApi2_0:
831+
writer.WriteProperty(OpenApiConstants.NullableExtension, true);
832+
break;
833+
case OpenApiSpecVersion.OpenApi3_0:
834+
writer.WriteProperty(OpenApiConstants.Nullable, true);
835+
break;
885836
}
886837
}
887838
}
888839

840+
#if NET5_0_OR_GREATER
841+
private static readonly Array jsonSchemaTypeValues = System.Enum.GetValues<JsonSchemaType>();
842+
#else
843+
private static readonly Array jsonSchemaTypeValues = System.Enum.GetValues(typeof(JsonSchemaType));
844+
#endif
845+
889846
/// <inheritdoc/>
890847
public IOpenApiSchema CreateShallowCopy()
891848
{

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,16 @@ public static IOpenApiSchema LoadSchema(ParseNode node, OpenApiDocument hostDocu
271271
propertyNode.ParseField(schema, _openApiSchemaFixedFields, _openApiSchemaPatternFields, hostDocument);
272272
}
273273

274+
if (schema.Extensions is not null && schema.Extensions.ContainsKey(OpenApiConstants.NullableExtension))
275+
{
276+
if (schema.Type.HasValue)
277+
schema.Type |= JsonSchemaType.Null;
278+
else
279+
schema.Type = JsonSchemaType.Null;
280+
281+
schema.Extensions.Remove(OpenApiConstants.NullableExtension);
282+
}
283+
274284
return schema;
275285
}
276286
}

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ internal static partial class OpenApiV3Deserializer
159159
"type",
160160
(o, n, _) => {
161161
var type = n.GetScalarValue()?.ToJsonSchemaType();
162-
// so we don't loose the value from nullable
162+
// so we don't lose the value from nullable
163163
if (o.Type.HasValue)
164164
o.Type |= type;
165165
else
@@ -307,6 +307,16 @@ public static IOpenApiSchema LoadSchema(ParseNode node, OpenApiDocument hostDocu
307307
propertyNode.ParseField(schema, _openApiSchemaFixedFields, _openApiSchemaPatternFields, hostDocument);
308308
}
309309

310+
if (schema.Extensions is not null && schema.Extensions.ContainsKey(OpenApiConstants.NullableExtension))
311+
{
312+
if (schema.Type.HasValue)
313+
schema.Type |= JsonSchemaType.Null;
314+
else
315+
schema.Type = JsonSchemaType.Null;
316+
317+
schema.Extensions.Remove(OpenApiConstants.NullableExtension);
318+
}
319+
310320
return schema;
311321
}
312322
}

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,10 @@ internal static partial class OpenApiV31Deserializer
286286
var nullable = bool.Parse(value);
287287
if (nullable) // if nullable, convert type into an array of type(s) and null
288288
{
289-
o.Type |= JsonSchemaType.Null;
289+
if (o.Type.HasValue)
290+
o.Type |= JsonSchemaType.Null;
291+
else
292+
o.Type = JsonSchemaType.Null;
290293
}
291294
}
292295
}
@@ -392,8 +395,11 @@ public static IOpenApiSchema LoadSchema(ParseNode node, OpenApiDocument hostDocu
392395

393396
if (schema.Extensions is not null && schema.Extensions.ContainsKey(OpenApiConstants.NullableExtension))
394397
{
395-
var type = schema.Type;
396-
schema.Type = type | JsonSchemaType.Null;
398+
if (schema.Type.HasValue)
399+
schema.Type |= JsonSchemaType.Null;
400+
else
401+
schema.Type = JsonSchemaType.Null;
402+
397403
schema.Extensions.Remove(OpenApiConstants.NullableExtension);
398404
}
399405

‎test/Microsoft.OpenApi.Readers.Tests/V2Tests/OpenApiSchemaTests.cs‎

Lines changed: 47 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,19 @@
22
// Licensed under the MIT license.
33

44
using System.IO;
5-
using FluentAssertions;
6-
using Microsoft.OpenApi.Reader.V2;
7-
using Xunit;
8-
using Microsoft.OpenApi.Reader.ParseNodes;
9-
using Microsoft.OpenApi.Models;
10-
using Microsoft.OpenApi.Extensions;
115
using System.Text.Json.Nodes;
12-
using System.Collections.Generic;
6+
using System.Threading.Tasks;
7+
using FluentAssertions;
138
using FluentAssertions.Equivalency;
9+
using Microsoft.OpenApi.Extensions;
10+
using Microsoft.OpenApi.Models;
1411
using Microsoft.OpenApi.Models.References;
12+
using Microsoft.OpenApi.Reader;
13+
using Microsoft.OpenApi.Reader.ParseNodes;
14+
using Microsoft.OpenApi.Reader.V2;
15+
using Microsoft.OpenApi.Tests;
1516
using Microsoft.OpenApi.Writers;
16-
using Microsoft.OpenApi.Models.Interfaces;
17+
using Xunit;
1718

1819
namespace Microsoft.OpenApi.Readers.Tests.V2Tests
1920
{
@@ -98,6 +99,7 @@ public void ParseSchemaWithEnumShouldSucceed()
9899
.Excluding((IMemberInfo memberInfo) =>
99100
memberInfo.Path.EndsWith("Parent")));
100101
}
102+
101103
[Fact]
102104
public void PropertiesReferenceShouldWork()
103105
{
@@ -152,5 +154,42 @@ public void PropertiesReferenceShouldWork()
152154
);
153155
Assert.True(JsonNode.DeepEquals(JsonNode.Parse(json), expected));
154156
}
157+
158+
[Fact]
159+
public async Task SerializeSchemaWithNullableShouldSucceed()
160+
{
161+
// Arrange
162+
var expected = @"type: string
163+
x-nullable: true";
164+
165+
var path = Path.Combine(SampleFolderPath, "schemaWithNullableExtension.yaml");
166+
167+
// Act
168+
var schema = await OpenApiModelFactory.LoadAsync<OpenApiSchema>(path, OpenApiSpecVersion.OpenApi2_0, new(), SettingsFixture.ReaderSettings);
169+
170+
var writer = new StringWriter();
171+
schema.SerializeAsV2(new OpenApiYamlWriter(writer));
172+
var schemaString = writer.ToString();
173+
174+
Assert.Equal(expected.MakeLineBreaksEnvironmentNeutral(), schemaString.MakeLineBreaksEnvironmentNeutral());
175+
}
176+
177+
[Fact]
178+
public async Task SerializeSchemaWithOnlyNullableShouldSucceed()
179+
{
180+
// Arrange
181+
var expected = @"x-nullable: true";
182+
183+
var path = Path.Combine(SampleFolderPath, "schemaWithOnlyNullableExtension.yaml");
184+
185+
// Act
186+
var schema = await OpenApiModelFactory.LoadAsync<OpenApiSchema>(path, OpenApiSpecVersion.OpenApi2_0, new(), SettingsFixture.ReaderSettings);
187+
188+
var writer = new StringWriter();
189+
schema.SerializeAsV2(new OpenApiYamlWriter(writer));
190+
var schemaString = writer.ToString();
191+
192+
Assert.Equal(expected.MakeLineBreaksEnvironmentNeutral(), schemaString.MakeLineBreaksEnvironmentNeutral());
193+
}
155194
}
156195
}

‎test/Microsoft.OpenApi.Readers.Tests/V2Tests/Samples/OpenApiSchema/schemaWithNullableExtension.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
type: string
2+
x-nullable: true

‎test/Microsoft.OpenApi.Readers.Tests/V2Tests/Samples/OpenApiSchema/schemaWithOnlyNullableExtension.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
x-nullable: true

0 commit comments

Comments
 (0)