Fix JsonArray.Add and ReplaceWith regressions. (dotnet#94854)

* Fix JsonArray.Add and ReplaceWith regressions. * Add comment * Address feedback.
jkotas · Nov 16, 2023 · b135980 · b135980
1 parent 7f42ffa
commit b135980
Show file tree

Hide file tree

Showing 3 changed files with 105 additions and 14 deletions.
diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonArray.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonArray.cs
@@ -175,13 +175,7 @@ internal JsonArray(JsonElement element, JsonNodeOptions? options = null) : base(
         [RequiresDynamicCode(JsonValue.CreateDynamicCodeMessage)]
         public void Add<T>(T? value)
         {
-            JsonNode? nodeToAdd = value switch
-            {
-                null => null,
-                JsonNode node => node,
-                _ => JsonValue.Create(value, Options)
-            };
-
+            JsonNode? nodeToAdd = ConvertFromValue(value, Options);
             Add(nodeToAdd);
         }
 

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonNode.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonNode.cs
@@ -1,8 +1,9 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
+using System.Text.Json.Serialization.Converters;
+using System.Text.Json.Serialization.Metadata;
 
 namespace System.Text.Json.Nodes
 {
@@ -311,17 +312,16 @@ public static bool DeepEquals(JsonNode? node1, JsonNode? node2)
         [RequiresDynamicCode(JsonValue.CreateDynamicCodeMessage)]
         public void ReplaceWith<T>(T value)
         {
+            JsonNode? node;
             switch (_parent)
             {
-                case null:
-                    return;
                 case JsonObject jsonObject:
-                    JsonValue? jsonValue = JsonValue.Create(value);
-                    jsonObject.SetItem(GetPropertyName(), jsonValue);
+                    node = ConvertFromValue(value);
+                    jsonObject.SetItem(GetPropertyName(), node);
                     return;
                 case JsonArray jsonArray:
-                    JsonValue? jValue = JsonValue.Create(value);
-                    jsonArray.SetItem(GetElementIndex(), jValue);
+                    node = ConvertFromValue(value);
+                    jsonArray.SetItem(GetElementIndex(), node);
                     return;
             }
         }
@@ -346,5 +346,33 @@ internal void AssignParent(JsonNode parent)
 
             Parent = parent;
         }
+
+        /// <summary>
+        /// Adaptation of the equivalent JsonValue.Create factory method extended
+        /// to support arbitrary <see cref="JsonElement"/> and <see cref="JsonNode"/> values.
+        /// TODO consider making public cf. https://github.com/dotnet/runtime/issues/70427
+        /// </summary>
+        [RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
+        [RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)]
+        internal static JsonNode? ConvertFromValue<T>(T? value, JsonNodeOptions? options = null)
+        {
+            if (value is null)
+            {
+                return null;
+            }
+
+            if (value is JsonNode node)
+            {
+                return node;
+            }
+
+            if (value is JsonElement element)
+            {
+                return JsonNodeConverter.Create(element, options);
+            }
+
+            var jsonTypeInfo = (JsonTypeInfo<T>)JsonSerializerOptions.Default.GetTypeInfo(typeof(T));
+            return new JsonValueCustomized<T>(value, jsonTypeInfo, options);
+        }
     }
 }
diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonArrayTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonArrayTests.cs
@@ -680,5 +680,74 @@ public static void ReplaceWith()
             Assert.Null(jValue.Parent);
             Assert.Equal("[5]", jArray.ToJsonString());
         }
+
+        [Theory]
+        [InlineData("null")]
+        [InlineData("1")]
+        [InlineData("false")]
+        [InlineData("\"str\"")]
+        [InlineData("""{"test":"hello world"}""")]
+        [InlineData("[1,2,3]")]
+        public static void AddJsonElement(string json)
+        {
+            // Regression test for https://github.com/dotnet/runtime/issues/94842
+            using var jdoc = JsonDocument.Parse(json);
+            var array = new JsonArray();
+
+            array.Add(jdoc.RootElement);
+
+            JsonNode arrayElement = Assert.Single(array);
+            switch (jdoc.RootElement.ValueKind)
+            {
+                case JsonValueKind.Object:
+                    Assert.IsAssignableFrom<JsonObject>(arrayElement);
+                    break;
+                case JsonValueKind.Array:
+                    Assert.IsAssignableFrom<JsonArray>(arrayElement);
+                    break;
+                case JsonValueKind.Null:
+                    Assert.Null(arrayElement);
+                    break;
+                default:
+                    Assert.IsAssignableFrom<JsonValue>(arrayElement);
+                    break;
+            }
+            Assert.Equal($"[{json}]", array.ToJsonString());
+        }
+
+        [Theory]
+        [InlineData("null")]
+        [InlineData("1")]
+        [InlineData("false")]
+        [InlineData("\"str\"")]
+        [InlineData("""{"test":"hello world"}""")]
+        [InlineData("[1,2,3]")]
+        public static void ReplaceWithJsonElement(string json)
+        {
+            // Regression test for https://github.com/dotnet/runtime/issues/94842
+            using var jdoc = JsonDocument.Parse(json);
+            var array = new JsonArray { 1 };
+
+            array[0].ReplaceWith(jdoc.RootElement);
+
+            JsonNode arrayElement = Assert.Single(array);
+            switch (jdoc.RootElement.ValueKind)
+            {
+                case JsonValueKind.Object:
+                    Assert.IsAssignableFrom<JsonObject>(arrayElement);
+                    break;
+                case JsonValueKind.Array:
+                    Assert.IsAssignableFrom<JsonArray>(arrayElement);
+                    break;
+                case JsonValueKind.Null:
+                    Assert.Null(arrayElement);
+                    break;
+                default:
+                    Assert.IsAssignableFrom<JsonValue>(arrayElement);
+                    break;
+            }
+
+            Assert.Equal($"[{json}]", array.ToJsonString());
+        }
     }
 }