JsonPatch.SystemTextJson: When serializing a `JsonPatchDocument`, "value":null is incorrectly emitted for "remove", "move" and "copy" operations.

[dbc2]

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

This is a follow-on issue related to JsonPatch.SystemTextJson: When serializing a JsonPatchDocument, "from":null is incorrectly emitted for "add", "remove", "replace" and "test" operations. #63862. Since there's already a pull opened for that issue, I'm opening a second issue.

In .NET 10.0.0-rc.2.25502.107, I am trying to manually create a JsonPatchDocument (rather than receive one created by a client). I can do that quite easily, but when I serialize it using System.Text.Json, "value": null properties will be shown for operations that should not have a "value" property:

• "remove"
• "move"
• "copy"

For instance, taking some examples from JSON Patch RFC 6902, if I try to create the following patch:

[
    { "op": "add", "path": "/a/b/c", "value": [ "foo", "bar" ] },
    { "op": "remove", "path": "/a/b/c" },
    { "op": "replace", "path": "/a/b/c", "value": 42 },
    { "op": "move", "from": "/a/b/c", "path": "/a/b/d" },
    { "op": "copy", "from": "/a/b/c", "path": "/a/b/e" },
    { "op": "test", "path": "/a/b/c", "value": "foo" },
    { "op": "replace", "path": "/a/b/nullableString", "value": null }
]

Using the following code:

JsonPatchDocument patchDocument = new ();
patchDocument.Add("/a/b/c", new [] {  "foo", "bar" });
patchDocument.Remove("/a/b/c");
patchDocument.Replace("/a/b/c", 42);
patchDocument.Move(from: "/a/b/c", path: "/a/b/d");
patchDocument.Copy(from: "/a/b/c", path: "/a/b/e");
patchDocument.Test("/a/b/c", "foo");
//Validate that we can still patch in a null value
patchDocument.Replace("/a/b/nullableString", null);

var json = JsonSerializer.Serialize(patchDocument);
Console.WriteLine(json); // Prints a patch that includes several invalid "value":null properties

I will get the following invalid patch:

[
	{"value":["foo","bar"],"path":"/a/b/c","op":"add","from":null},
	{"value":null,"path":"/a/b/c","op":"remove","from":null},
	{"value":42,"path":"/a/b/c","op":"replace","from":null},
	{"value":null,"path":"/a/b/d","op":"move","from":"/a/b/c"},
	{"value":null,"path":"/a/b/e","op":"copy","from":"/a/b/c"},
	{"value":"foo","path":"/a/b/c","op":"test","from":null},
	{"value":null,"path":"/a/b/nullableString","op":"replace","from":null}
]

All but the last of those "value":null properties definitely should not be present as can be seen from the RFC. (The invalid "from":null properties are covered by the previous issue.)

As with Issue #63862, once again the problem is that the Newtonsoft code suppresses the unwanted "value" properties via conditional serialization, specifically the ShouldSerializevalue() method. System.Text.Json, on the other hand, does not support conditional serialization automatically, so the "value":null properties get serialized unconditionally. (However, unlike Issue #63862. no useless ShouldSerializeValue() method was introduced.). Some equivalent conditional serialization mechanism will be required by System.Text.Json.

Also, you might want to mark Operation.value as nullable since it will be null for certain operations -- and can be null for any operation:

[JsonPropertyName(nameof(value))]
public object? value { get; set; }

Expected Behavior

"value" properties should only be emitted for "add", "replace" and "test" operations. Note however that a null value is a valid patch value for these operations, so you can't just unconditionally skip null values, you need to emulate the logic of ShouldSerializevalue() from the old Newtonsoft implementation. You could have done that by making Operation polymorphic, however it might be too late to do that for .NET 10 -- and it might be too much of a breaking change for .NET 11+.

Operation.value should be marked as nullable.

Steps To Reproduce

//dotnet new console
//dotnet add package Microsoft.AspNetCore.JsonPatch.SystemTextJson --prerelease
//dotnet add package NUnit
using System;
using System.Text.Json;
using System.Text.Json.Nodes;

using Microsoft.AspNetCore.JsonPatch.SystemTextJson;

using NUnit.Framework; // Throws NUnit.Framework.AssertionException

Console.WriteLine("\nExamples from https://www.rfc-editor.org/rfc/rfc6902#section-4");

JsonPatchDocument patchDocument = new ();
patchDocument.Add("/a/b/c", new [] {  "foo", "bar" });
patchDocument.Remove("/a/b/c");
patchDocument.Replace("/a/b/c", 42);
patchDocument.Move(from: "/a/b/c", path: "/a/b/d");
patchDocument.Copy(from: "/a/b/c", path: "/a/b/e");
patchDocument.Test("/a/b/c", "foo");
//Validate that we can still patch in a null value
patchDocument.Replace("/a/b/nullableString", null);

var json = JsonSerializer.Serialize(patchDocument);
Console.WriteLine(json); // Prints a patch that includes several invalid "value":null properties

var expectedJson = 
"""
[
    { "op": "add", "path": "/a/b/c", "value": [ "foo", "bar" ] },
    { "op": "remove", "path": "/a/b/c" },
    { "op": "replace", "path": "/a/b/c", "value": 42 },
    { "op": "move", "from": "/a/b/c", "path": "/a/b/d" },
    { "op": "copy", "from": "/a/b/c", "path": "/a/b/e" },
    { "op": "test", "path": "/a/b/c", "value": "foo" },
    { "op": "replace", "path": "/a/b/nullableString", "value": null }
]
"""; // Taken from  https://www.rfc-editor.org/rfc/rfc6902#section-4

Assert.That(JsonNode.DeepEquals(JsonNode.Parse(expectedJson), JsonNode.Parse(json)));  // Fails due to multiple "value":null properties

Exceptions (if any)

None (incorrect JSON is generated without an exception thrown.)

.NET Version

10.0.100-rc.2.25502.107

Anything else?

I reproduced this using the command line only. Output of dotnet --info:

.NET SDK:
 Version:           10.0.100-rc.2.25502.107
 Commit:            89c8f6a112
 Workload version:  10.0.100-manifests.18c66609
 MSBuild version:   18.0.0-preview-25502-107+89c8f6a11

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.19045
 OS Platform: Windows
 RID:         win-x64
 Base Path:   C:\Program Files\dotnet\sdk\10.0.100-rc.2.25502.107\

.NET workloads installed:
There are no installed workloads to display.
Configured to use workload sets when installing new manifests.
No workload sets are installed. Run "dotnet workload restore" to install a workload set.

Host:
  Version:      10.0.0-rc.2.25502.107
  Architecture: x64
  Commit:       89c8f6a112

.NET SDKs installed:
  6.0.428 [C:\Program Files\dotnet\sdk]
  7.0.410 [C:\Program Files\dotnet\sdk]
  8.0.203 [C:\Program Files\dotnet\sdk]
  10.0.100-rc.2.25502.107 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 6.0.36 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 7.0.20 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 8.0.3 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 8.0.15 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 9.0.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 10.0.0-rc.2.25502.107 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 6.0.36 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 7.0.20 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.3 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.15 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 9.0.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 10.0.0-rc.2.25502.107 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 6.0.36 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 7.0.20 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 8.0.3 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 8.0.15 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 10.0.0-rc.2.25502.107 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Other architectures found:
  x86   [C:\Program Files (x86)\dotnet]
    registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\x86\InstallLocation]

Environment variables:
  DOTNET_CLI_TELEMETRY_OPTOUT              [1]

global.json file:
  Not found

Learn more:
  https://aka.ms/dotnet/info

Download .NET:
  https://aka.ms/dotnet/download