KevinDockx / JsonPatch

JSON Patch (JsonPatchDocument) RFC 6902 implementation for .NET
MIT License
173 stars 29 forks source link

Honor [JsonProperty] attributes and ContractResolver #44

Closed thomaslevesque closed 8 years ago

thomaslevesque commented 8 years ago

I'm looking into using this library for making JSON Patch requests, however I often use the [JsonProperty] attribute (and custom contract resolvers), and this library doesn't take them into account.

It probably works fine if both the client and server use the same library, but it's not the case for me; the back-end is written in Java, and property names are case-sensitive, so if I send /parentid instead of /parentId, it won't work...

I haven't looked at the code yet, but I suspect the change would be fairly simple. A simple solution would be to add a JsonPatchDocument<T> constructor overload that takes a contract resolver, and use that to resolve the property name, instead of just lowercasing the C# property name.

madmox commented 8 years ago

I'd like to +1 this feature request, because:

We have an existing API in which we use snake_case and JsonProperty attributes on all our properties, and we would like to add PATCH capabilities, so it would be very nice to be able to use this library :)

KevinDockx commented 8 years ago

Hi,

you're absolutely right, this is high on the todo-list. That said: the library is already integrated in .NET Core currently (you can find it here: https://github.com/aspnet/JsonPatch, and you can use this with the newly released RC2 version of .NET Core), and we made sure that version already honours JsonProperty attributes.

Also, if you currently want this functionality and you're working on the full .NET framework, you can already get this by using Marvin.JsonPatch.Dynamic (https://github.com/KevinDockx/JsonPatch.Dynamic) instead of this version - .Dynamic already features JsonProperty support.

The reason Marvin.JsonPatch currently doesn't is that it's a bit less straighforward to implement support for all the PCL targets this library supports than it is for the full framework.

Hope this clarifies this a bit :)

amnesia0287 commented 8 years ago

Is the implementation in .net core bidirectional? I've been trying to get it to correctly serialize patch requests, but have thus far been unable to figure out how to get it to create patch requests using the jsonproperty attributes, though it does look like it probably could read them if they were serialized correctly.

KevinDockx commented 8 years ago

That's a good catch. No, it isn't - but it probably should be. Got some work to do I guess :) In any case, to be able to continue now you can use the non-generic version of JsonPatchDocument to create a request (you can still use the generic version at API level).

KevinDockx commented 8 years ago

JsonProperty is now honoured when applying a PatchDoc. I split up JsonPatchDocument & injecting a custom resolver into two separate issues.