Adding element to an array should not end with "last index + 1"

[ghost]

Example

var object = {someValues : ["one", "two"]};
var object2 = {someValues : ["one", "two", "new value"]};
var result = jsonpatch.compare(object, object2 );
result is [{"op":"add", "path": "/someValues/2", "value": "new value"}];

If you add an element to an array at the end of it - it should end with "path/-" and not the index of the new element

[tomalec]

RFC says:

The specified index MUST NOT be greater than the
number of elements in the array.

So it seems, even if it worked nice for our use cases, it's not a valid JSON-Patch :/

Thanks for reporting @danieldonev !

[ghost]

You can make check if the element indedx is equal to the array.length-1, then finish with "/-" otherwise stick to the current implementation (in case the element HAS to be inserted in specific position)

var element = "test4";
var array = ["test0", "test1", "test2", "test3"]
array.push(element )

// the array.indexOf(element) will be 4;
//array.length will be 5

If(array.indexOf(element) === array.length-1){
// use "/-" instead of an index
}else{
//use the current implementation
}

[tomalec]

err.. my bad 2 is equal to "the number of elements in array", so this patch seems to be valid as well.

@danieldonev, Is there any specific reason, why it does not work for you? As we aim to be super fast, every if needs to be considered as performance threat.

[ghost]

Well, let`s imagine that you have JAVA RESTful API, you will be trying to add an element to an index which does not exist. In the case where you add an element to the JS array of 5 element the new one will be 6-th and the patch path will end with "/5" where the java array has also 5 elements but the last element has index 4 and you are trying to add it to index 5. it will throw an exception

[tomalec]

But I would say that's what standards are for. If Java implementation of JSON-Patch cannot process valid JSON-Patch, then this should be fixed on Java end.

[ghost]

True. BUT - "If the "-" character is used to index the end of the array (see [RFC6901]), this has the effect of appending the value to the array." Since I am adding the new element at the end of the array it does not make sence to specify the index since i am not moving any of the existing elements.

[warpech]

The vendor neutral JSON-Patch test suite uses both .../<number> and ../- to add elements at the end of the array. They seem to be equal choices.

I guess that a fix for that would be just a change in line https://github.com/Starcounter-Jack/JSON-Patch/blob/master/src/json-patch-duplex.ts#L189

For me switching to .../- sounds like a potential performance improvement.

@tomalec would there be some potential implications for https://github.com/PuppetJs/JSON-Patch-OT?

[ghost]

Actually it should support both. If the new element is added in specific position (is not at the end) is must use the /number. If it is at the end just use /- version of it

[tomalec]

Well, I'll check if it affect OT, as switching to more efficient solution is always a good idea.

However I cannot find a statement in spec that says it MUST be like that. I can see only implication it opposite direction. Maybe It can be discussed at json-patch spec issue tracker?

[FrittenKeeZ]

When is this getting merged/fixed? It's not working correctly with .NET Core's JSON Patch implementation, failing with this error: For operation 'add', the target location specified by path '/roles/1' was not found.

[miyconst]

cc @Starcounter-Jack