Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/8.0-staging] Fix JsonArray.Add and ReplaceWith regressions. #94882

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions src/libraries/Directory.Build.targets
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,18 @@
<StrongNameKeyId Condition="'$(IsTestProject)' == 'true' or '$(IsTestSupportProject)' == 'true'">$(TestStrongNameKeyId)</StrongNameKeyId>
</PropertyGroup>

<!-- Need to be defined before packaging.targets is imported. -->
<PropertyGroup>
<!-- The source of truth for these IsNETCoreApp* properties is NetCoreAppLibrary.props. -->
<IsNETCoreAppSrc Condition="'$(IsSourceProject)' == 'true' and
$(NetCoreAppLibrary.Contains('$(AssemblyName);'))">true</IsNETCoreAppSrc>
<IsNETCoreAppRef Condition="('$(IsReferenceAssemblyProject)' == 'true' or '$(IsRuntimeAndReferenceAssembly)' == 'true') and
$(NetCoreAppLibrary.Contains('$(AssemblyName);')) and
'$(IsPrivateAssembly)' != 'true'">true</IsNETCoreAppRef>
<IsNETCoreAppAnalyzer Condition="'$(IsGeneratorProject)' == 'true' and
$(NetCoreAppLibraryGenerator.Contains('$(MSBuildProjectName);'))">true</IsNETCoreAppAnalyzer>
</PropertyGroup>

<!-- resources.targets need to be imported before the Arcade SDK. -->
<Import Project="$(RepositoryEngineeringDir)resources.targets" />
<Import Project="..\..\Directory.Build.targets" />
Expand Down Expand Up @@ -47,14 +59,6 @@
-->
<WarningsAsErrors>$(WarningsAsErrors.Replace('NU1605', ''))</WarningsAsErrors>

<!-- The source of truth for these IsNETCoreApp* properties is NetCoreAppLibrary.props. -->
<IsNETCoreAppSrc Condition="'$(IsSourceProject)' == 'true' and
$(NetCoreAppLibrary.Contains('$(AssemblyName);'))">true</IsNETCoreAppSrc>
<IsNETCoreAppRef Condition="('$(IsReferenceAssemblyProject)' == 'true' or '$(IsRuntimeAndReferenceAssembly)' == 'true') and
$(NetCoreAppLibrary.Contains('$(AssemblyName);')) and
'$(IsPrivateAssembly)' != 'true'">true</IsNETCoreAppRef>
<IsNETCoreAppAnalyzer Condition="'$(IsGeneratorProject)' == 'true' and
$(NetCoreAppLibraryGenerator.Contains('$(MSBuildProjectName);'))">true</IsNETCoreAppAnalyzer>
<!-- Inbox analyzers shouldn't use the live targeting / runtime pack. They better depend on an LKG to avoid layering concerns. -->
<UseLocalTargetingRuntimePack Condition="'$(IsNETCoreAppAnalyzer)' == 'true'">false</UseLocalTargetingRuntimePack>
<!-- By default, disable implicit framework references for NetCoreAppCurrent libraries. -->
Expand Down
2 changes: 2 additions & 0 deletions src/libraries/System.Text.Json/src/System.Text.Json.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
<NoWarn>CS8969</NoWarn>
<IncludeInternalObsoleteAttribute>true</IncludeInternalObsoleteAttribute>
<IsPackable>true</IsPackable>
<GeneratePackageOnBuild>true</GeneratePackageOnBuild>
<ServicingVersion>1</ServicingVersion>
<PackageDescription>Provides high-performance and low-allocating types that serialize objects to JavaScript Object Notation (JSON) text and deserialize JSON text to objects, with UTF-8 support built-in. Also provides types to read and write JSON text encoded as UTF-8, and to create an in-memory document object model (DOM), that is read-only, for random access of the JSON elements within a structured view of the data.

The System.Text.Json library is built-in as part of the shared framework in .NET Runtime. The package can be installed when you need to use it in other target frameworks.</PackageDescription>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,13 +174,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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

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
{
Expand Down Expand Up @@ -316,17 +318,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;
}
}
Expand All @@ -351,5 +352,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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
}
Loading