myquay / JsonPatch

Adds JSON Patch support to ASP.NET Web API (http://tools.ietf.org/html/rfc6902)
MIT License
98 stars 30 forks source link

Allow the character "-" to denote a position after the last array element #8

Closed mindingdata closed 8 years ago

mindingdata commented 9 years ago

JsonPatch and more so RFC 6901 (http://tools.ietf.org/html/rfc6901) allows for the use of "-" to denote a position after the last position of an array. This is important as at the moment the library only allows for you to use an integer past the last position, and then it will add onto the end.

The issue is if there is more than one addition to an array, we have to rely on the operations being ran in order. For example.

Existing Array = 0,1,2 New Additions = 3,4

If the operations are ran as 3 then 4. It works fine. If it's run as 4,3. Then the array ends up looking like so 0,1,2,4 (And then it likely errors, I haven't tested this, but I'm unsure what would happen if you called ADD on an array with an index that is within the range already, it may actually end up looking like 0,1,2,3 with no 4 instead).

I'm currently working on implementing a "DiffHelper" where it will output a list of "JsonPatchOperations" that you can then either output via JSON serialization, or move around internally to still use the existing "Patching" operations. But this definitely hinders that capability.

myquay commented 9 years ago

Yep, I can see how this will be useful, I'll take a look into it.

mindingdata commented 9 years ago

I don't mind making a stab, just waiting for my Pull request to be accepted (Or rejected) so I can add it there too.

myquay commented 9 years ago

Ah, #9? I haven't taken a look at the code yet but my chief concern is that it belongs in a separate library as I want JsonPatch to be as lightweight as possible.

How do you feel about making this change as a separate branch as it's more relevant to the core library? We can upgrade the DiffHelper to support '-' also once we've figured out where to put it.

jasongb commented 8 years ago

....this was the nail in the coffin for this library for me.

I'm trying all manner of inserting array values, and it's failing each time. The only thing I can do successfully is overwrite an existing value. I'm using 'add', and given a path of "/Ratings" with one document in the array, I've tried "/Ratings/" [replaces existing], "/Ratings/0" [Immediate HTTP 500], "/Ratings/1" [Immediate HTTP 500]...

I can't get the library to let me add an element to an existing array. I wish this library supported the spec!