Merge pull request #2559 from microsoft/fix/null-default

fix: fixes a bug where yaml null values would end up as a string "null" during roundtrip serialization

Author: baywet

src/Microsoft.OpenApi.YamlReader/OpenApiYamlReader.cs

@@ -131,9 +131,9 @@ static JsonNode LoadJsonNodesFromYamlDocument(TextReader input)
         {
             var yamlStream = new YamlStream();
             yamlStream.Load(input);
-            if (yamlStream.Documents.Any())
+            if (yamlStream.Documents.Any() && yamlStream.Documents[0].ToJsonNode() is { } jsonNode)
             {
-                return yamlStream.Documents[0].ToJsonNode();
+                return jsonNode;
             }
 
             throw new InvalidOperationException("No documents found in the YAML stream.");

src/Microsoft.OpenApi.YamlReader/YamlConverter.cs

@@ -18,7 +18,7 @@ public static class YamlConverter
         /// </summary>
         /// <param name="yaml">The YAML stream.</param>
         /// <returns>A collection of nodes representing the YAML documents in the stream.</returns>
-        public static IEnumerable<JsonNode> ToJsonNode(this YamlStream yaml)
+        public static IEnumerable<JsonNode?> ToJsonNode(this YamlStream yaml)
         {
             return yaml.Documents.Select(x => x.ToJsonNode());
         }
@@ -28,7 +28,7 @@ public static IEnumerable<JsonNode> ToJsonNode(this YamlStream yaml)
         /// </summary>
         /// <param name="yaml">The YAML document.</param>
         /// <returns>A `JsonNode` representative of the YAML document.</returns>
-        public static JsonNode ToJsonNode(this YamlDocument yaml)
+        public static JsonNode? ToJsonNode(this YamlDocument yaml)
         {
             return yaml.RootNode.ToJsonNode();
         }
@@ -39,7 +39,7 @@ public static JsonNode ToJsonNode(this YamlDocument yaml)
         /// <param name="yaml">The YAML node.</param>
         /// <returns>A `JsonNode` representative of the YAML node.</returns>
         /// <exception cref="NotSupportedException">Thrown for YAML that is not compatible with JSON.</exception>
-        public static JsonNode ToJsonNode(this YamlNode yaml)
+        public static JsonNode? ToJsonNode(this YamlNode yaml)
         {
             return yaml switch
             {
@@ -110,25 +110,25 @@ private static YamlSequenceNode ToYamlSequence(this JsonArray arr)
             return new YamlSequenceNode(arr.Select(x => x!.ToYamlNode()));
         }
 
-        private static JsonValue ToJsonValue(this YamlScalarNode yaml)
+        private static readonly HashSet<string> YamlNullRepresentations = new(StringComparer.Ordinal)
         {
-            switch (yaml.Style)
+            "~",
+            "null",
+            "Null",
+            "NULL"
+        };
+
+        private static JsonValue? ToJsonValue(this YamlScalarNode yaml)
+        {
+            return yaml.Style switch
             {
-                case ScalarStyle.Plain:
-                    return decimal.TryParse(yaml.Value, NumberStyles.Float, CultureInfo.InvariantCulture, out var d)
-                        ? JsonValue.Create(d)
-                        : bool.TryParse(yaml.Value, out var b)
-                            ? JsonValue.Create(b)
-                            : JsonValue.Create(yaml.Value)!;
-                case ScalarStyle.SingleQuoted:
-                case ScalarStyle.DoubleQuoted:
-                case ScalarStyle.Literal:
-                case ScalarStyle.Folded:
-                case ScalarStyle.Any:
-                    return JsonValue.Create(yaml.Value)!;
-                default:
-                    throw new ArgumentOutOfRangeException();
-            }
+                ScalarStyle.Plain when decimal.TryParse(yaml.Value, NumberStyles.Float, CultureInfo.InvariantCulture, out var d) => JsonValue.Create(d),
+                ScalarStyle.Plain when bool.TryParse(yaml.Value, out var b) => JsonValue.Create(b),
+                ScalarStyle.Plain when YamlNullRepresentations.Contains(yaml.Value) => null,
+                ScalarStyle.Plain => JsonValue.Create(yaml.Value),
+                ScalarStyle.SingleQuoted or ScalarStyle.DoubleQuoted or ScalarStyle.Literal or ScalarStyle.Folded or ScalarStyle.Any => JsonValue.Create(yaml.Value),
+                _ => throw new ArgumentOutOfRangeException(nameof(yaml)),
+            };
         }
 
         private static YamlScalarNode ToYamlScalar(this JsonValue val)

test/Microsoft.OpenApi.Readers.Tests/V31Tests/OpenApiSchemaTests.cs

@@ -349,6 +349,68 @@ public void DefaultEmptyCollectionShouldRoundTrip()
             Assert.Empty(resultingArray);
         }
 
+        [Fact]
+        public void DefaultNullIsLossyDuringRoundTripJson()
+        {
+            // Given
+            var serializedSchema =
+            """
+            {
+                "type": ["string", "null"],
+                "default": null
+            }
+            """;
+            using var textWriter = new StringWriter();
+            var writer = new OpenApiJsonWriter(textWriter);
+
+            // When
+            var schema = OpenApiModelFactory.Parse<OpenApiSchema>(serializedSchema, OpenApiSpecVersion.OpenApi3_1, new(), out _, "json", SettingsFixture.ReaderSettings);
+
+            Assert.Null(schema.Default);
+
+            schema.SerializeAsV31(writer);
+            var roundTrippedSchema = textWriter.ToString();
+
+            // Then
+            var parsedResult = JsonNode.Parse(roundTrippedSchema);
+            var parsedExpected = JsonNode.Parse(serializedSchema);
+            Assert.False(JsonNode.DeepEquals(parsedExpected, parsedResult));
+            var resultingDefault = parsedResult["default"];
+            Assert.Null(resultingDefault);
+        }
+
+        [Fact]
+        public void DefaultNullIsLossyDuringRoundTripYaml()
+        {
+            // Given
+            var serializedSchema =
+            """
+            type:
+              - string
+              - 'null'
+            default: null
+            """;
+            using var textWriter = new StringWriter();
+            var writer = new OpenApiYamlWriter(textWriter);
+
+            // When
+            var schema = OpenApiModelFactory.Parse<OpenApiSchema>(serializedSchema, OpenApiSpecVersion.OpenApi3_1, new(), out _, "yaml", SettingsFixture.ReaderSettings);
+
+            Assert.Null(schema.Default);
+
+            schema.SerializeAsV31(writer);
+            var roundTrippedSchema = textWriter.ToString();
+
+            // Then
+            Assert.Equal(
+            """
+            type:
+              - 'null'
+              - string
+            """.MakeLineBreaksEnvironmentNeutral(),
+            roundTrippedSchema.MakeLineBreaksEnvironmentNeutral());
+        }
+
         [Fact]
         public async Task SerializeV31SchemaWithMultipleTypesAsV3Works()
         {

test/Microsoft.OpenApi.Readers.Tests/YamlConverterTests.cs

@@ -0,0 +1,29 @@
+using SharpYaml;
+using SharpYaml.Serialization;
+using Xunit;
+using Microsoft.OpenApi.YamlReader;
+
+namespace Microsoft.OpenApi.Readers.Tests;
+
+public class YamlConverterTests
+{
+    [Theory]
+    [InlineData("~")]
+    [InlineData("null")]
+    [InlineData("Null")]
+    [InlineData("NULL")]
+    public void YamlNullValuesReturnNullJsonNode(string value)
+    {
+        // Given
+        var yamlNull = new YamlScalarNode(value)
+        {
+            Style = ScalarStyle.Plain
+        };
+
+        // When
+        var jsonNode = yamlNull.ToJsonNode();
+
+        // Then
+        Assert.Null(jsonNode);
+    }
+}