Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Value-based array operations #18

Open
heruan opened this issue Nov 2, 2016 · 12 comments
Open

Value-based array operations #18

heruan opened this issue Nov 2, 2016 · 12 comments

Comments

@heruan
Copy link

heruan commented Nov 2, 2016

Arrays should have value-based patch operations. Currently, only index-based actions are permitted:

  • insert by index or last;
  • replace by index;
  • remove by index.

The current spec lacks value-based operations on arrays, such as:

  • insert before/after certain existing values;
  • replace existing values;
  • remove by values.

For example, given:

{
    "items": [ "a", "c", "e", "z" ]
}

This patch should be valid:

[
    { "op": "add",     "path": "/items/-", "value": "b", "before": "c" },
    { "op": "replace", "path": "/items/-", "value": "d", "replace": "e" },
    { "op": "remove",  "path": "/items/-", "value": "z" }
]

And produce:

{
    "items": [ "a", "b", "c", "d" ]
}

The special - key, could be the indicator of an array at a specific path to keep compatibility with the current spec.

This was referenced Nov 3, 2016
@serac
Copy link

serac commented Jan 19, 2017

This is an important capability for REST APIs where the backing data model looks nothing like the JSON documents produced and consumed via HTTP. In many cases array ordering is fulfilled by issuing a query, and requiring array items to be addressed by index would require in some cases to re-issue the query in order to know what item in the array is to be updated. Addressing by value provides a much better alternative.

@konrad7d
Copy link

+1 for this. Without being able to operate on values, we also need to watch out for races where an element might have changed the position in the list by the time the delete is executed.

@rnagulapalle
Copy link

any update on this issue

@jdodds
Copy link

jdodds commented May 26, 2018

How would you feel about replacing the use of json-pointer for path with JSONPath ? It would allow for operations on values, but would be a pretty large departure from the current spec for json-patch.

@sebastien-rosset
Copy link

sebastien-rosset commented Sep 3, 2018

+1 for JSONPath support, though admittedly it brings additional complexity. The proposed "value" attribute would work in simple cases when the array contains primitive values, but could become cumbersome when the array elements are JSON objects.

Whether the new specification is based on the "value" attribute or JSONPath, one consideration is that arrays may contain duplicate elements, so add, remove, replace would have to handle the duplicate element scenario. For example, one additional attribute may specify whether the by-value applies to the first match or all elements.
In practice, it may be useful to be able to delete or replace all elements that match a value (or JSONPath expression); or delete/replace the first element that match the value/JSONPath expression.

When the array elements are complex JSON objects, some applications use the JSON array as a set that has identity properties, i.e. the array cannot contain two elements that have the same identifying properties.

@jlindner
Copy link

jlindner commented Oct 2, 2018

+1 for JSONPath, or anything that can make this work using the Value, and not the index in the array. For my use case, any operation that works off index in the array, and not a value, is a show stopper. My list can contain tens of thousands of items in the array(user has to page the data). Making the user get the index in the array, and then call the patch, is dangerous. Another user could have modified the array after they got the array to get the index, removing a different item, changing the index in the array for the item I need to remove, and this would just remove the wrong one.

@gregsdennis
Copy link

gregsdennis commented Feb 27, 2019

👎 on JSON Path replacing JSON Pointers. As I stated in #1:

JSON Path is a query syntax that has the potential to return multiple results. Many use cases for JSON Patch require a single result, for which JSON Pointer is much more well-suited.

The only time I could see wanting JSON Path is for when we explicitly want potentially multiple matches (like a SQL where clause).

They have different purposes, and we shouldn't conflate the two.

(see also #23)


I like this feature, though.

@bow
Copy link

bow commented Jun 18, 2019

-1 on JSON Path replacing JSON Pointers. As I stated in #1:

JSON Path is a query syntax that has the potential to return multiple results. Many use cases for JSON Patch require a single result, for which JSON Pointer is much more well-suited.
The only time I could see wanting JSON Path is for when we explicitly want potentially multiple matches (like a SQL where clause).

They have different purposes, and we shouldn't conflate the two.

(see also #23)

I like this feature, though.

I think using something meant for querying does not invalidate JSONPatch's use case. A query operation is inherent in an update operation: you have to select things you want to change first, before you can actually change them. We can of course prioritize having update operations that targets single items. Though the nature of an array itself, which allows multiple items of the same value, makes this preference not really always applicable.

In fact, just because we do have array types, I would argue that JSONPatch must also support single patch objects that targets multiple values. I feel that whatever this operation will be, it may need to be more complex than just specifying by value.

To illustrate this with OP's proposal, given this object:

{
    "items": [ "a", "c", "e", "z" ]
}

The operation below may make sense:

[{ "op": "add",     "path": "/items/-", "value": "b", "before": "c" }]

But what should we do then, if our original object is like this:

{
    "items": [ "a", "c", "e", "c", "z" ]
}

Should we then insert "b" before both "c"s? What if we just want to insert before the first one? or the second one?

So having some better support for operations with multiple results seems necessary. This could imply that, in addition to being able to target multiple items, there should also be support for limiting the maximum number of items to target.

In other words, I would be fine with something like JSONPath instead of JSONPointer.

That being said, I think we should not use JSONPath per se though, mainly for the reason that it is not formally specified anywhere, but also for the reason that it seems too cumbersome for JSONPatch.

Are there any alternative to JSONPath that is formally specified and more lightweight? Perhaps that's something JSONPatch can use?

@mitar
Copy link

mitar commented Oct 12, 2019

I do not think this is a good approach and I opened #/27 as an alternative to this (and few others).

I think this is a very fragile solution to an underlying problem that the data to which the JSON patch should be applied has changed in meantime. While for some cases of such changes index by value could be a solution, it would still fail for many other cases. Like if values are not unique in the list. Or if you didn't really want to insert at some value, but between two values, and those both got moved differently.

Another issue above mentioned is about queries which change. Again, I think a better approach then trying to figure out in the patch how to be robust against query results changing, is to instead store information about which version of query results (like cursor ID) the patch should use. You generally need support for that for proper pagination anyway. So patching is then a similar thing reusing cursor ID support.

My proposal is just to standardize how optional additional context/metadata can be provided so that algorithms to try to apply those patches when underlying data changed can do their work (better). So patch is then a simple diff, but applying the patch can use different algorithms (given different requirements by different apps) and patch can contain useful information for those algorithms. Which exactly is (should be) out of scope of this standard.

@mitar
Copy link

mitar commented Oct 12, 2019

Oh, and another problem with this proposal is that value based array operations do not work where values are objects. So you open a whole can of worms if you then try to address those cases as well. And shortly you have whole programming language for adaptive patches. I think this should really be at a different layer (or standard) while JSON patch should just be about representing a diff.

@dove-young
Copy link

"op": "remove" does not work in OpenShift 4.x

[root@sre4-inf Argo]# kubectl get scc anyuid -o json | jq .users
[
  "something",
  "else",
  "name"
]
[root@sre4-inf Argo]# kubectl patch scc anyuid    --type=json -p '[
{"op":"remove","path":"/users/-", "value": "something" }
]'
The request is invalid
[root@sre4-inf Argo]#

"op":"add" works well.

[root@sre4-inf Argo]# kubectl patch scc anyuid \
>    --type=json -p '[
> {"op":"add","path":"/users/-", "value": "new" }
> ]'
securitycontextconstraints.security.openshift.io/anyuid patched
[root@sre4-inf Argo]# kubectl get scc anyuid -o json | jq .users
[
  "something",
  "else",
  "name",
  "new"
]

@SCLogo
Copy link

SCLogo commented Jul 28, 2022

any chance to implement this ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests