JamesNK / Newtonsoft.Json

Json.NET is a popular high-performance JSON framework for .NET
https://www.newtonsoft.com/json
MIT License
10.8k stars 3.26k forks source link

Deserialise not respecting type passed in for JsonPatchDocument<T> #2682

Closed NonyLoberts closed 2 years ago

NonyLoberts commented 2 years ago

Source/destination types

    public class MyPatchModel
    {
        public string Name { get; set; }
        public string Notes { get; set; }
    }

Source/destination JSON

"[\r\n    {\r\n      \"op\": \"replace\",\r\n      \"path\": \"/NoneExistentProperty\",\r\n      \"value\": \"Some Value\"\r\n    }\r\n]"

Expected behavior

I Expected either a deserialisation error telling me it couldnt deserialise, or a model with no operations

Actual behavior

Instead i got a model with an operation that is not valid for the type passed in image

Steps to reproduce

JsonConvert.DeserializeObject<JsonPatchDocument<MyPatchModel>>(myJson)
elgonzo commented 2 years ago

I don't understand your expectations. There is no issue/bug here.

You are deserializing a JsonPatchDocument<TModel>. You are not deserializing a MyPatchModel.

As such, the JsonPatchDocument<MyPatchModel> instance got all the data (i.e., operation) from the json data, as should be expected.

Note that the value property of Operation\<TModel> is of type object (it is inherited from the non-generic Operation type). Thus, from the point of view of Newtonsoft.Json, having a "Some Value" string (or anything else unrelated to MyPatchModel for that matter) for the operation value is totally a-okay.

If that is a problem for you that you cannot work around, you will have to take this up with the ASP.NET Core team...

NonyLoberts commented 2 years ago

I don't understand your expectations. There is no issue/bug here.

You are deserializing a JsonPatchDocument<TModel>. You are not deserializing a MyPatchModel.

As such, the JsonPatchDocument<MyPatchModel> instance got all the data (i.e., operation) from the json data, as should be expected.

Note that the value property of Operation is of type object (it is inherited from the non-generic Operation type). Thus, from the point of view of Newtonsoft.Json, having a "Some Value" string (or anything else unrelated to MyPatchModel for that matter) for the operation value is totally a-okay.

If that is a problem for you that you cannot work around, you will have to take this up with the ASP.NET Core team...

I guess I assumed that the deserialising would honour the object being used, either through reflection of properties compared against the paths of some other logic. I feel like how it is now is misleading as I could pass any object into the deserialisation and get a list of operations that have no bearing on that type

elgonzo commented 2 years ago

I feel like how it is now is misleading as I could pass any object into the deserialisation and get a list of operations that have no bearing on that type

Yeah, it kinda is. But it is not related to Newtonsoft.Json functionality, bugs or limiations. Your problem is chiefly with ASP.NET's Operation\<TModel> and ASP.NET's TypedJsonPatchDocumentConverter.

I unfortunately don't know what would be considered best practices with regard to handling of inapplicable operators in ASP.NET's JsonPatchDocuments. It would probably be a good question for stackoverflow.com...

NonyLoberts commented 2 years ago

I feel like how it is now is misleading as I could pass any object into the deserialisation and get a list of operations that have no bearing on that type

Yeah, it kinda is. But it is not related to Newtonsoft.Json functionality, bugs or limiations. Your problem is chiefly with ASP.NET's Operation and ASP.NET's TypedJsonPatchDocumentConverter.

I unfortunately don't know what would be considered best practices with regard to handling of inapplicable operators in ASP.NET's JsonPatchDocuments. It would probably be a good question for stackoverflow.com...

No problem. I appreciate your time and patience! I'll try to find an alternative solution.