JsonPatch: test operations fail when contract resolver is used to serialize data

[kdrvn]

Functional impact

When controlling data serialization using a contract resolver (and the contract resolver is not registered for the JsonConvert's default settings), test operations in patch requests fail.

Minimal repro steps

Issue noticed in Microsoft.AspNetCore.JsonPatch 2.1.1.

Example code:

class Person
{
    public string FirstName { get; set; }
    public string MiddleName { get; set; }
    public string LastName { get; set; }
}

class NoMiddleNameContractResolver : DefaultContractResolver {
    protected override JsonProperty CreateProperty(MemberInfo member, MemberSerialization memberSerialization)
    {
        JsonProperty property = base.CreateProperty(member, memberSerialization);

        if (property.DeclaringType == typeof(Person) && property.PropertyName == nameof(Person.MiddleName))
            property.ShouldSerialize = obj => false;

        return property;
    }
}

class Program
{
    static void Main(string[] args)
    {
        var contractResolver = new NoMiddleNameContractResolver();
        var serializerSettings = new JsonSerializerSettings { ContractResolver = contractResolver };

        var person = new Person { FirstName = "John", MiddleName = "Alexander", LastName = "Doe" };
        var serializedValue = JsonConvert.SerializeObject(person, serializerSettings);

        var jsonTestValue = $"[{{ op: \"test\", path: \"/0\", value: {serializedValue} }}]";
        var operations = JsonConvert.DeserializeObject<Operation<List<Person>>[]>(jsonTestValue, serializerSettings).ToList();

        var patchDoc = new JsonPatchDocument<List<Person>>(operations, contractResolver);

        var people = new List<Person> { person };

        try
        {
            patchDoc.ApplyTo(people);
            Console.WriteLine("patch test passed");
        }
        catch (Exception ex)
        {
            Console.WriteLine($"patch test failed: {ex}");
        }
    }
}

Expected result

ApplyTo method does not throw an error, test operation was successful.

Actual result

ApplyTo method throw an error, due to failed test operation.

Further technical details

Each implementation of the IAdapter interface uses the static JsonConvert class with the default serializer settings. The provided ContractResolver is not being used when serializing values being compared.

DictionaryAdapterOfU DynamicObjectAdapter ListAdapter PocoAdapter

[kdrvn]

As I am looking into writing a fix for this issue, I am wondering if the JsonPatchDocument constructor should have an override with a JsonSerializerSettings parameter to be used for comparing the test operation's value and the tested value. As these settings can influence how the objects are serialized for comparison and influence the outcome, it would be useful to be able to change them without setting the JsonConvert.DefaultSettings.

[mkArtakMSFT]

Thanks for contacting us, @kdrvn. Will you be interested in submitting a PR for the proposal?

[kdrvn]

I will give it a try.

[mkArtakMSFT]

Hi. Thanks for contacting us. We're closing this issue as there was not much community interest in this ask for quite a while now.