JsonPatchDocument .ApplyTo(...) raises JsonPatchException for unknown Properties instead of transparently mapping them to JsonExtensionData

[hf-kklein]

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

If a property is annotated with the [System.Text.Json.Serialization.JsonExtensionData] attribute, I expect the JsonPatchDocument.ApplyTo(...) method to map unknown paths to the respective property.
Instead (actual behaviour), an Microsoft.AspNetCore.JsonPatch.Exceptions.JsonPatchException is raised.

Expected Behavior

Unmapped paths in a JsonPatchDocument should be mapped (transparently) to the JsonExtensionData property.
By "transparently" I mean, that the client sending the patch to the server doesn't need to be aware of the ExtensionData property.

Steps To Reproduce

#nullable enable
using System;
using System.Collections.Generic;
using Microsoft.AspNetCore.JsonPatch;
using Microsoft.AspNetCore.JsonPatch.Operations;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace UnitTests;

class MyClass
{
    public int Foo { get; set; }
    public string Bar { get; set; }

    [System.Text.Json.Serialization.JsonExtensionData]
    public IDictionary<string, object>? MyExtensionData { get; set; }
}

[TestClass]
public class JsonPatchDocumentTest
{
    [TestMethod]
    [DataRow(true, false, false)]
    [DataRow(false, false, false)]
    [DataRow(false, true, false)]
    // the following operations with "useStringlyTypedPath=true fail
    [DataRow(true, false, true)]
    [DataRow(false, false, true)]
    [DataRow(false, true, true)]
    public void TestPatchingExtensionData(bool initialExtensionDataAreEmpty, bool overwriteExistingExtensionData, bool useStringlyTypedPath)
    {
        if (initialExtensionDataAreEmpty && overwriteExistingExtensionData)
        {
            throw new Exception("This makes no sense");
        }

        string myEntityAsJson;
        if (initialExtensionDataAreEmpty)
        {
            myEntityAsJson = """
                             {
                                 "Foo": 17,
                                 "Bar": "asd"
                             }   
                             """;
        }
        else
        {
            myEntityAsJson = """
                             {
                                 "Foo": 17,
                                 "Bar": "asd",
                                 "abc": "def"
                             }   
                             """;
        }

        var myEntity = System.Text.Json.JsonSerializer.Deserialize<MyClass>(myEntityAsJson);
        Assert.AreEqual(expected: initialExtensionDataAreEmpty, actual: myEntity.MyExtensionData is null);

        var myPatch = new JsonPatchDocument<MyClass>();
        myPatch.Add(x => x.Foo, 42);
        myPatch.Add(x => x.Bar, "fgh");
        string modifiedKey = overwriteExistingExtensionData ? "abc" : "uvw";

        if (!useStringlyTypedPath)
        {
            // this code path works, but my client isn't aware of the extension data property nor its name
            if (initialExtensionDataAreEmpty)
            {
                myPatch.Add(x => x.MyExtensionData, new Dictionary<string, object> { { modifiedKey, "xyz" } });
            }
            else
            {
                myPatch.Add(x => x.MyExtensionData[modifiedKey], "xyz");
            }
        }
        else
        {
            // this is the behaviour of my client, which doesn't know about the extension data
            myPatch.Operations.Add(new Operation<MyClass> { path = "/" + modifiedKey, op = "add", value = "xyz" });
        }
        
        myPatch.ApplyTo(myEntity);
        // this throws a Microsoft.AspNetCore.JsonPatch.Exceptions.JsonPatchException:
        // "The target location specified by path segment 'abc' was not found."
        // "The target location specified by path segment 'uvw' was not found."

        // Instead of the exception, I'd like to automatically map the path "/xyz" (or "/abc" respectively) to MyExtensionData

        Assert.AreEqual(expected: 42, actual: myEntity.Foo);
        Assert.AreEqual(expected: "fgh", actual: myEntity.Bar);
        Assert.IsNotNull(myEntity.MyExtensionData);
        Assert.IsTrue(myEntity.MyExtensionData.ContainsKey(modifiedKey));
        Assert.AreEqual(expected: "xyz", actual: myEntity.MyExtensionData[modifiedKey]);
        if (!overwriteExistingExtensionData && !initialExtensionDataAreEmpty)
        {
            Assert.IsTrue(myEntity.MyExtensionData.ContainsKey("abc"));
            Assert.AreEqual(expected: "def", actual: myEntity.MyExtensionData["abc"].ToString());
        }
    }
}

Exceptions (if any)

No response

.NET Version

8

Anything else?

No response

[captainsafia]

@hf-kklein Thanks for reporting this issue! The Microsoft.AspNetCore.JsonPatch package doesn't support System.Text.Json yet, so I don't expect the use of STJ's JsonExtensionData attribute to work given we're not yet using STJ's serializer under the hood. See #24333 for more info.

There is currently an open-source version of the JsonPatch support via System.Text.Json that you can use: https://github.com/Havunen/SystemTextJsonPatch

Perhaps try that and see if it resolves your issue?

[hf-kklein]

I created a workaround for anyone else having this issue: https://github.com/Hochfrequenz/JsonPatchDocumentExtensionDataAdapter

It's probably neither as pretty nor as elegant as it could be, but works in my tests.

[captainsafia]

@hf-kklein Does your implementation assume that a user is mixing Newtonsoft.Json and System.Text.Json in their application to support the full end-to-end? I would've expected your implementation to extend on the third-party library mentioned above but it seems to build on the built-in implementation which uses Newtonsoft.Json at the moment...

[hf-kklein]

it [your application] seems to build on the built-in implementation which uses Newtonsoft.Json at the moment...

Yes, it's built on top of the existing implementation - regardless of whether that's based on Newtonsoft or STJ. My intention was to make it as least "invasive" as possible. Users just need to add few lines of code at those places where patches shall be applied to STJ extension data annotated properties. It does not enhance any existing middleware or model binding under the hood. As I said: It's not as elegant or well designed as it could be, but it works.