Skip to content

Commit b135980

Browse files
Fix JsonArray.Add and ReplaceWith regressions. (#94854)
* Fix JsonArray.Add and ReplaceWith regressions. * Add comment * Address feedback.
1 parent 7f42ffa commit b135980

3 files changed

Lines changed: 105 additions & 14 deletions

File tree

src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonArray.cs

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -175,13 +175,7 @@ internal JsonArray(JsonElement element, JsonNodeOptions? options = null) : base(
175175
[RequiresDynamicCode(JsonValue.CreateDynamicCodeMessage)]
176176
public void Add<T>(T? value)
177177
{
178-
JsonNode? nodeToAdd = value switch
179-
{
180-
null => null,
181-
JsonNode node => node,
182-
_ => JsonValue.Create(value, Options)
183-
};
184-
178+
JsonNode? nodeToAdd = ConvertFromValue(value, Options);
185179
Add(nodeToAdd);
186180
}
187181

src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonNode.cs

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using System.Collections.Generic;
54
using System.Diagnostics.CodeAnalysis;
5+
using System.Text.Json.Serialization.Converters;
6+
using System.Text.Json.Serialization.Metadata;
67

78
namespace System.Text.Json.Nodes
89
{
@@ -311,17 +312,16 @@ public static bool DeepEquals(JsonNode? node1, JsonNode? node2)
311312
[RequiresDynamicCode(JsonValue.CreateDynamicCodeMessage)]
312313
public void ReplaceWith<T>(T value)
313314
{
315+
JsonNode? node;
314316
switch (_parent)
315317
{
316-
case null:
317-
return;
318318
case JsonObject jsonObject:
319-
JsonValue? jsonValue = JsonValue.Create(value);
320-
jsonObject.SetItem(GetPropertyName(), jsonValue);
319+
node = ConvertFromValue(value);
320+
jsonObject.SetItem(GetPropertyName(), node);
321321
return;
322322
case JsonArray jsonArray:
323-
JsonValue? jValue = JsonValue.Create(value);
324-
jsonArray.SetItem(GetElementIndex(), jValue);
323+
node = ConvertFromValue(value);
324+
jsonArray.SetItem(GetElementIndex(), node);
325325
return;
326326
}
327327
}
@@ -346,5 +346,33 @@ internal void AssignParent(JsonNode parent)
346346

347347
Parent = parent;
348348
}
349+
350+
/// <summary>
351+
/// Adaptation of the equivalent JsonValue.Create factory method extended
352+
/// to support arbitrary <see cref="JsonElement"/> and <see cref="JsonNode"/> values.
353+
/// TODO consider making public cf. https://github.com/dotnet/runtime/issues/70427
354+
/// </summary>
355+
[RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
356+
[RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)]
357+
internal static JsonNode? ConvertFromValue<T>(T? value, JsonNodeOptions? options = null)
358+
{
359+
if (value is null)
360+
{
361+
return null;
362+
}
363+
364+
if (value is JsonNode node)
365+
{
366+
return node;
367+
}
368+
369+
if (value is JsonElement element)
370+
{
371+
return JsonNodeConverter.Create(element, options);
372+
}
373+
374+
var jsonTypeInfo = (JsonTypeInfo<T>)JsonSerializerOptions.Default.GetTypeInfo(typeof(T));
375+
return new JsonValueCustomized<T>(value, jsonTypeInfo, options);
376+
}
349377
}
350378
}

src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonArrayTests.cs

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -680,5 +680,74 @@ public static void ReplaceWith()
680680
Assert.Null(jValue.Parent);
681681
Assert.Equal("[5]", jArray.ToJsonString());
682682
}
683+
684+
[Theory]
685+
[InlineData("null")]
686+
[InlineData("1")]
687+
[InlineData("false")]
688+
[InlineData("\"str\"")]
689+
[InlineData("""{"test":"hello world"}""")]
690+
[InlineData("[1,2,3]")]
691+
public static void AddJsonElement(string json)
692+
{
693+
// Regression test for https://github.com/dotnet/runtime/issues/94842
694+
using var jdoc = JsonDocument.Parse(json);
695+
var array = new JsonArray();
696+
697+
array.Add(jdoc.RootElement);
698+
699+
JsonNode arrayElement = Assert.Single(array);
700+
switch (jdoc.RootElement.ValueKind)
701+
{
702+
case JsonValueKind.Object:
703+
Assert.IsAssignableFrom<JsonObject>(arrayElement);
704+
break;
705+
case JsonValueKind.Array:
706+
Assert.IsAssignableFrom<JsonArray>(arrayElement);
707+
break;
708+
case JsonValueKind.Null:
709+
Assert.Null(arrayElement);
710+
break;
711+
default:
712+
Assert.IsAssignableFrom<JsonValue>(arrayElement);
713+
break;
714+
}
715+
Assert.Equal($"[{json}]", array.ToJsonString());
716+
}
717+
718+
[Theory]
719+
[InlineData("null")]
720+
[InlineData("1")]
721+
[InlineData("false")]
722+
[InlineData("\"str\"")]
723+
[InlineData("""{"test":"hello world"}""")]
724+
[InlineData("[1,2,3]")]
725+
public static void ReplaceWithJsonElement(string json)
726+
{
727+
// Regression test for https://github.com/dotnet/runtime/issues/94842
728+
using var jdoc = JsonDocument.Parse(json);
729+
var array = new JsonArray { 1 };
730+
731+
array[0].ReplaceWith(jdoc.RootElement);
732+
733+
JsonNode arrayElement = Assert.Single(array);
734+
switch (jdoc.RootElement.ValueKind)
735+
{
736+
case JsonValueKind.Object:
737+
Assert.IsAssignableFrom<JsonObject>(arrayElement);
738+
break;
739+
case JsonValueKind.Array:
740+
Assert.IsAssignableFrom<JsonArray>(arrayElement);
741+
break;
742+
case JsonValueKind.Null:
743+
Assert.Null(arrayElement);
744+
break;
745+
default:
746+
Assert.IsAssignableFrom<JsonValue>(arrayElement);
747+
break;
748+
}
749+
750+
Assert.Equal($"[{json}]", array.ToJsonString());
751+
}
683752
}
684753
}

0 commit comments

Comments
 (0)