Subject: [apps-discuss] json-patch: draft 05 comments

From: "Manger, James H" <[email-masked]>
Date: Mon, 1 Oct 2012 17:00:18 +1000

Feedback on draft-ietf-appsawg-json-patch-05

3. Document Structure: the example fails as the "copy" operation refers to a path that no longer exists due to the previous "move"
OLD: { "op": "move", "path": "/a/b/c", "to": "/a/b/d" },
     { "op": "copy", "path": "/a/b/c", "to": "/a/b/e" }
NEW: { "op": "move", "path": "/a/b/c", "to": "/a/b/d" },
     { "op": "copy", "path": "/a/b/d", "to": "/a/b/e" }

3. Document Structure: avoid "object" when it doesn't mean a JSON object
OLD: "a JSON document whose root object is an array"
NEW: "a JSON document that is an array"

4.1. add: a member MUST *NOT* already exist
OLD: " the specified location MUST already exist for the operation to be successful"
NEW: " the specified location MUST NOT already exist for the operation to be successful"
P.S. I don't like needing to know a member does not exists before adding it, but that is a separate issue.

4.1. add: the note about "pointer’s error handling" is awkward.

4.2. remove: it needn't be an error to remove a value that does not exist (eg has already been removed).
OLD: "The value at the specified location MUST exist for the operation to be successful."
NEW: "If there is no value as the specified location the operation has no affect."

4.4. move and 4.5. copy: defining an exception for the pointless path=to case is unnecessary. "move" should be "path" as well.
REMOVE: "unless "move"(sic) and "to" specify the same object, which has no effect"

4.5. copy: can you copy a value into a component of itself? That is, can "path" be a prefix of "to"?
  {"op":"copy", "path":"/a", "to":"/a/b/c"}

4.4. move: moving a value to an object element should replace any existing value at that element if there is one, instead of being an error. Similarly with copy.

5. Error Handling. "a RFC2119 requirement" is a very awkward way to refer to "MUST" statements in this draft.
OLD: "If a RFC2119 [RFC2119] requirement is violated by a JSON Patch document, or if an operation is not successful"
NEW: "If any operation is not successful"

Appendix A. Examples: add examples with unrecognized elements and invalid operations
NEW:  A.11. Ignore unrecognized elements
        An example target JSON document:
         {"foo":"bar"}
        A JSON Patch document:
         [{"op":"add", "path":"/baz", "value":"qux", "xyz":123}]
        The resulting JSON document:
         {"foo":"bar", "baz":"qux"}

NEW:  A.12. Invalid JSON Patch document
        A JSON Patch document:
         [{"op":"add", "path":"/baz", "value":"qux", "op":"remove"}]
        This JSON Patch document MUST NOT be treated as an "add"
        operation since there is a later "op":"remove" element.
        It SHOULD be an error. A JSON parser that hides such duplicate
        element names MUST NOT be used unless it always exposes only
        the last element with a given name (eg "op":"remove" in this example).

--
James Manger

---

From: Mark Nottingham <[email-masked]>
Date: Mon, 1 Oct 2012 12:45:10 +0100

Hi James,

Thanks for the feedback. Responses below.

>     { "op": "copy", "path": "/a/b/d", "to": "/a/b/e" }

Fixed in SVN

> NEW: "a JSON document that is an array"

Changed to "whose root is an array"

> P.S. I don't like needing to know a member does not exists before adding it, but that is a separate issue.

Fixed in SVN

> 4.1. add: the note about "pointer’s error handling" is awkward.

Agreed, but I didn't have a better way to phrase it. Suggestions?

> NEW: "If there is no value as the specified location the operation has no affect."

Needs discussion.

> REMOVE: "unless "move"(sic) and "to" specify the same object, which has no effect"

"move" changed to "path". Removing it needs discussion; this was an explicit decision earlier, discussed as issue #9 <http://trac.tools.ietf.org/wg/appsawg/trac/ticket/9>.

>  {"op":"copy", "path":"/a", "to":"/a/b/c"}

Needs discussion, but good point; I suspect we need a constraint here.

> 4.4. move: moving a value to an object element should replace any existing value at that element if there is one, instead of being an error. Similarly with copy.

Needs discussion.

> NEW: "If any operation is not successful"

I agree that it's a bit awkward, but there are two things happening here; one is a violation by the doc, another is the operation failing. If you have a suggestion that's smoother yet captures both, I'd be glad to incorporate it.

>        the last element with a given name (eg "op":"remove" in this example).

Added in SVN, although I removed the RFC2119 language, as it's not good to have in examples, or to repeat requirements.

Cheers and thanks again,

--
Mark Nottingham
http://www.mnot.net/

---

From: James M Snell <[email-masked]>
Date: Mon, 1 Oct 2012 08:31:43 -0700

On Mon, Oct 1, 2012 at 4:45 AM, Mark Nottingham <[email-masked]> wrote:

> [snip]
In general this is probably ok, however, if patch is asking to remove a
value that does not exist, then it's a possible indication that the
resource has been modified in other ways since the patch document was
generated, or possibly that some other previous operation or error
condition has left the document in an inconsistent state.

If we allow this one, what should we do if someone attempts to use
"replace" with a value that doesn't exist?

>  > 4.4. move and 4.5. copy: defining an exception for the pointless
> path=to case is unnecessary. "move" should be "path" as well.
> "move" changed to "path". Removing it needs discussion; this was an
> explicit decision earlier, discussed as issue #9 <
> http://trac.tools.ietf.org/wg/appsawg/trac/ticket/9>.
>
>
Not exactly sure what you mean by "move" changed to "path".

> Needs discussion, but good point; I suspect we need a constraint here.
likewise with "move" really...

{"a": {"b": {}}}

It would be quite silly to do {"op":"move","path":"/a","to":"/a/b"}

For copy, it's not as much of an obvious error tho (even if it is
completely non-sensical). Perhaps use MUST NOT with "move" and SHOULD NOT
with "copy" ?

>  > 4.4. move: moving a value to an object element should replace any
> existing value at that element if there is one, instead of being an error.
> Similarly with copy.
>
> Needs discussion.
>
>
-1 ... with stuff like this it becomes too easy to make accidental changes.
I would much rather require an explicit delete of the existing value prior
to issuing the "move".

> > 5. Error Handling. "a RFC2119 requirement" is a very awkward way to
> refer to "MUST" statements in this draft.
> > OLD: "If a RFC2119 [RFC2119] requirement is violated by a JSON Patch
> document, or if an operation is not successful"
> I agree that it's a bit awkward, but there are two things happening here;
> one is a violation by the doc, another is the operation failing. If you
> have a suggestion that's smoother yet captures both, I'd be glad to
> incorporate it.
>

"If a JSON Patch document violates any of the normative requirements of
this specification,
or if an operation is not successful, evaluation of the JSON Patch document
SHOULD
terminate and application of the entire patch document SHALL NOT be deemed
successful."

> Added in SVN, although I removed the RFC2119 language, as it's not good to
> have in examples, or to repeat requirements.
>
> Cheers and thanks again,
>
> --
> Mark Nottingham
> http://www.mnot.net/
>
>
>
>
> _______________________________________________
> apps-discuss mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/apps-discuss
>

---

From: James M Snell <[email-masked]>
Date: Mon, 1 Oct 2012 08:38:44 -0700

Definitely agree that this would be useful. In the JSON-TEST draft that I'm
updating and will be publishing later today, I have a "base" predicate that
essentially does the same thing. Having this in the base JSON-PATCH spec
could be useful but it does introduce a potentially complex new structure
(not necessarily a bad thing, but definitely something to keep in mind).
Rather than calling it "tree", I'd prefer a name such as "base" ... as
you're establishing a base path for the contained operations.

[
  {"op": "base", "path": "/a/b", ops: [
    {"op": "add", "path":"/c", "value": "This is added to /a/b/c"},
    {"op": "base", "path": "/d/e", ops: [
      {"op": "add", "path":"/f", "value": "This is added to /a/b/d/e/f"}
    ]
  ]},
  {"op": "add", "path":"/g", "value": "This is added to /g"}
]

Definite +1 to this.

- James

On Sun, Sep 30, 2012 at 10:41 PM, Manger, James H <
[email-masked]> wrote:

> RE: draft-ietf-appsawg-json-patch-05
>
> Suggestion: define a "tree" operation for grouping operations that apply
> to part of the JSON doc being changed. It avoids repeating a path prefix in
> lots of operations; it simplifies writing a patch that can be applied at
> different points; its easier to separate the patch for changing a value
> from where that value is in a larger JSO structure. New text:
>
>   4.7. tree
>
>     The "tree" operation applies other operations to a subset of
>     the target. The operation object MUST include "path" and "ops"
>     members. "path" holds a JSON pointer that identifies the value
>     to which the other operations apply. "ops" holds an array of
>     operation objects. JSON pointers within "ops" are relative
>     to the value identified by "path".
>
>     Example:
>     Old:   {"a":{"b":{"c":{"d":{"e":{"name":"Alice"}}}}}}
>     Patch: [{"op":"tree", "path":"/a/b/c/d/e", "ops":[
>               {"op":"add", "path":"/age", "value":20},
>               {"op":"add", "path":"/email", "value":"[email-masked]"}]}]
>     New:   {"a":{"b":{"c":{"d":{"e":{
>             "name":"Alice", "age":20, "email":"[email-masked]"}}}}}}
>
> --
> James Manger

---

From: Mark Nottingham <[email-masked]>
Date: Mon, 1 Oct 2012 16:45:51 +0100

Personally --

I'm concerned about the complexity, but open to it (again, as long as we don't trip on it).

What about "with"?

Cheers,

On 01/10/2012, at 4:38 PM, James M Snell <[email-masked]> wrote:

> Definitely agree that this would be useful. In the JSON-TEST draft that I'm updating and will be publishing later today, I have a "base" predicate that essentially does the same thing. Having this in the base JSON-PATCH spec could be useful but it does introduce a potentially complex new structure (not necessarily a bad thing, but definitely something to keep in mind). Rather than calling it "tree", I'd prefer a name such as "base" ... as you're establishing a base path for the contained operations.
> 
> [
> On Sun, Sep 30, 2012 at 10:41 PM, Manger, James H <[email-masked]> wrote:
> Suggestion: define a "tree" operation for grouping operations that apply to part of the JSON doc being changed. It avoids repeating a path prefix in lots of operations; it simplifies writing a patch that can be applied at different points; its easier to separate the patch for changing a value from where that value is in a larger JSO structure. New text:
> 
>   4.7. tree
> 

--
Mark Nottingham
http://www.mnot.net/

---

From: James M Snell <[email-masked]>
Date: Mon, 1 Oct 2012 08:55:15 -0700

Yes, there is definitely a risk of increased complexity here but I think
it's manageable in this case. Yes, a developer could get quite carried away
with too many layers of nesting but that's more an implementation detail
and we can have some simple language warning people away from that if
necessary. The important thing is that having this does not significantly
impact the basic processing of the patch document, and a patch that
contains these can easily be translated into a flattened set of patch
operations with explicit paths.

I'm not overly concerned with what we call it so long as the name is
descriptive of it's purpose. "tree" is a bit unclear. "with" is ok, I
suppose.

On Mon, Oct 1, 2012 at 8:45 AM, Mark Nottingham <[email-masked]> wrote:

> > Definitely agree that this would be useful. In the JSON-TEST draft that
> I'm updating and will be publishing later today, I have a "base" predicate
> that essentially does the same thing. Having this in the base JSON-PATCH
> spec could be useful but it does introduce a potentially complex new
> structure (not necessarily a bad thing, but definitely something to keep in
> mind). Rather than calling it "tree", I'd prefer a name such as "base" ...
> as you're establishing a base path for the contained operations.
> >
> > [

---

From: Mark Nottingham <[email-masked]>
Date: Mon, 1 Oct 2012 17:00:07 +0100

{"op": "with", "base": "/foo/bar/baz", do: [
	…
]}

On 01/10/2012, at 4:55 PM, James M Snell <[email-masked]> wrote:

> Yes, there is definitely a risk of increased complexity here but I think it's manageable in this case. Yes, a developer could get quite carried away with too many layers of nesting but that's more an implementation detail and we can have some simple language warning people away from that if necessary. The important thing is that having this does not significantly impact the basic processing of the patch document, and a patch that contains these can easily be translated into a flattened set of patch operations with explicit paths. 
> 
> I'm not overly concerned with what we call it so long as the name is descriptive of it's purpose. "tree" is a bit unclear. "with" is ok, I suppose.
> 

--
Mark Nottingham
http://www.mnot.net/

---

From: James M Snell <[email-masked]>
Date: Mon, 1 Oct 2012 09:46:27 -0700

Hmm.. I'd rather keep the path named "path" for consistency with the other
operations.

{"op": "with", "path": "/a/b/c", "do": [...]}

This allows me to simply introduce a new operation without special casing.

- James

On Mon, Oct 1, 2012 at 9:00 AM, Mark Nottingham <[email-masked]> wrote:

> > > Definitely agree that this would be useful. In the JSON-TEST draft
> that I'm updating and will be publishing later today, I have a "base"
> predicate that essentially does the same thing. Having this in the base
> JSON-PATCH spec could be useful but it does introduce a potentially complex
> new structure (not necessarily a bad thing, but definitely something to
> keep in mind). Rather than calling it "tree", I'd prefer a name such as
> "base" ... as you're establishing a base path for the contained operations.
> > >
> > > [
> > > Suggestion: define a "tree" operation for grouping operations that
> apply to part of the JSON doc being changed. It avoids repeating a path
> prefix in lots of operations; it simplifies writing a patch that can be
> applied at different points; its easier to separate the patch for changing
> a value from where that value is in a larger JSO structure. New text:
> > >
> > >   4.7. tree
> > >

---

From: Mark Nottingham <[email-masked]>
Date: Sun, 14 Oct 2012 06:55:00 +1100

There hasn't been a lot of discussion of this on-list, but based on a few private exchanges and discussion elsewhere (e.g., <http://www.ietf.org/mail-archive/web/scim/current/msg00744.html>), I think there is at least one problem worth solving here -- being able to add items to arrays without knowing their current length.

The suggestion is to add an 'index' attribute. I think that adds a lot of complexity and corner cases, and wonder whether we couldn't achieve the same thing by allowing negative indices on JSON Pointer; e.g.,

/foo/-1

to indicate the last member of the array (delta a discussion about whether we base on -0 or -1).

If folks are uncomfortable with that amount of complexity, I think we could even meet the use case by doing something like

/foo/-

to indicate the last member of the array.

Thoughts?

Beyond that, there's a notion that we should relax certain requirements so as to avoid making the client know the exact state of the resource in every instance.

I *think* we have agreement that this should be done for "add"  (by removing the "MUST exist" requirement), but not elsewhere. 

Specifically:

>    If the target location references the root of the target document or
>    a member of an existing object, the specified location MUST already
>    exist for the operation to be successful.

... and I note that our examples already seem to violate this requirement, depending on how you read it, so *something* needs to change.

At the same time, it might be good to change the name; I'd suggest "set".

Make sense?

Cheers,

On 01/10/2012, at 3:22 PM, "Manger, James H" <[email-masked]> wrote:

> Suggestion: replace the "add" and "replace" operations with "insert" and "put" operations that work without needing to know (ie be in sync with) the old state of the JSON doc being changed. New text:
> 
>  4.1. insert
> 
>    The "insert" operation adds a new value to an array. The operation
>    object MUST include "path" and "value" members, and MAY include an
>    "i" member. "path" holds a JSON pointer that identifies an array.
>    "value" holds the value to add, which can be any JSON value.
>    "i" holds a number that is rounded to the nearest integer to give
>    the array index of the new value when inserted. If "i" is absent the
>    value is appended to the array. Any items in the array at an index
>    equal to or greater than "i" are moved to the next higher index to
>    make room for the new value.
> 
>    It is an error if "i" (rounded to the nearest integer) is less than
>    0 or greater than the length of the array.
> 
>    Example:
>    Old:   {"n":[1,2,3]}
>    Patch: [{"op":"insert", "path":"/n", "value":4},
>            {"op":"insert", "path":"/n", "value":5, "i":5},
>            {"op":"insert", "path":"/n", "value":1.5, "i":1}]
>    New:   {"n":[1,1.5,2,3,4,5]}
> 
> 
>  4.2. put
> 
>    The "put" operation puts a new value at the target location,
>    replacing any existing value if there is one. The operation object
>    MUST include "path" and "value" members. "path" holds a JSON pointer
>    that identifies the target location, which can be an object member
>    (that is or isn't present), an array index (that must exist), or
>    the whole target. "value" holds the value to add, which can be any
>    JSON value.
> 
>    Example:
>    Old:   {"n":[1,2,3]}
>    Patch: [{"op":"put", "path":"", "value":[true, {"a":1}]},
>            {"op":"put", "path":"/0", "value":false},
>            {"op":"put", "path":"/1/b", "value":2},
>            {"op":"put", "path":"/1/a", "value":3}]
>    New:   [false, {"a":3, "b":2}]
> 
> --
> James Manger

--
Mark Nottingham   http://www.mnot.net/

---

From: "Manger, James H" <[email-masked]>
Date: Mon, 15 Oct 2012 11:37:21 +1100

Java uses:
* "add" to insert a new item into a list (array),
* "set" to replace a current item in a list (array), and
* "put" to put a key/value pair in a map (object), regardless of whether or not the key already exists.

Consequently, I don't think "set" is a good name for a json-patch operation if it provides the functionality of Java’s "add", but not Java’s "set".

It would simplify spec & code to define 3 operations for json-patch, instead of trying to squeeze the functionality into two of these.

> /foo/-1

I like the ability to indicate position in an array (or between array elements) from then end. I’m not sure that extending JSON pointer is the right approach. Currently there is a unique pointer for every value. It would be a shame to lose that property.

--
James Manger

> -----Original Message-----
> From: Mark Nottingham [mailto:[email-masked]]
> Sent: Sunday, 14 October 2012 6:55 AM
> To: Manger, James H
> Cc: [email-masked]
> Subject: Re: [apps-discuss] json-patch: "op":"insert" and "op":"put"
> 
> There hasn't been a lot of discussion of this on-list, but based on a
> few private exchanges and discussion elsewhere (e.g.,
> <http://www.ietf.org/mail-archive/web/scim/current/msg00744.html>), I
> think there is at least one problem worth solving here -- being able to
> add items to arrays without knowing their current length.
> 
> The suggestion is to add an 'index' attribute. I think that adds a lot
> of complexity and corner cases, and wonder whether we couldn't achieve
> the same thing by allowing negative indices on JSON Pointer; e.g.,
> 
> /foo/-1
> 
> If folks are uncomfortable with that amount of complexity, I think we
> could even meet the use case by doing something like
> 
> /foo/-
> 
> Beyond that, there's a notion that we should relax certain requirements
> so as to avoid making the client know the exact state of the resource
> in every instance.
> 
> I *think* we have agreement that this should be done for "add"  (by
> removing the "MUST exist" requirement), but not elsewhere.
> 
> Specifically:
> 
> ... and I note that our examples already seem to violate this
> requirement, depending on how you read it, so *something* needs to
> change.
> 
> > Suggestion: replace the "add" and "replace" operations with "insert"
> and "put" operations that work without needing to know (ie be in sync
> with) the old state of the JSON doc being changed. New text:
> >
> >  4.1. insert
> >

---

From: Mark Nottingham <[email-masked]>
Date: Mon, 15 Oct 2012 11:39:43 +1100

> It would simplify spec & code to define 3 operations for json-patch, instead of trying to squeeze the functionality into two of these.

I'm *not* patterning anything I write after Java, thanks. Besides which, we're not talking about arrays here (although I did flirt with the idea of type-specific operators for a little while).

> I like the ability to indicate position in an array (or between array elements) from then end. I’m not sure that extending JSON pointer is the right approach. Currently there is a unique pointer fo…

What does keeping it give us?

---

From: "Manger, James H" <[email-masked]>
Date: Mon, 15 Oct 2012 12:30:10 +1100

> > Consequently, I don't think "set" is a good name for a json-patch
> operation if it provides the functionality of Java’s "add", but not
> Java’s "set".
> >
> > It would simplify spec & code to define 3 operations for json-patch,
> instead of trying to squeeze the functionality into two of these.

> I'm *not* patterning anything I write after Java, thanks. Besides
> which, we're not talking about arrays here (although I did flirt with
> the idea of type-specific operators for a little while).

Aren’t we talking about arrays?

The object case seems clear: set a key/value pair, regardless of whether the key was already present.

The choice I see for objects is whether an operation:
* Sets 1 key/value pair ("path" ids the key, "value" is separate); or
* Sets any number of pairs ("path" ids the object, "value" is an object of pairs to set).
[The 2nd choice looks better]

The array case is where it is a bit more complex. Adding and replacing are quite separate. Adding needs a gap, from the start or end; replacing needs an index, from the start or end. Gaps and indices can overflow. Ranges add more complexity.

> > /foo/-1
> >
> > I like the ability to indicate position in an array (or between array
> elements) from then end. I’m not sure that extending JSON pointer is
> the right approach. Currently there is a unique pointer for every
> value. It would be a shame to lose that property.

> What does keeping it give us?

You can compare 2 pointers to see if they point to the same thing (without having to resolve them first against the JSON doc they point at).

--
James Manger

---

From: "Paul C. Bryan" <[email-masked]>
Date: Mon, 15 Oct 2012 09:31:54 -0700

On Sun, 2012-10-14 at 06:55 +1100, Mark Nottingham wrote:

> think there is at least one problem worth solving here -- being able
> to add items to arrays without knowing their current length.
> 

I'm fine with this. I would prefer /foo/-1 over /foo/-.

> Beyond that, there's a notion that we should relax certain
> requirements so as to avoid making the client know the exact state of
> the resource in every instance.
> 

+1

> At the same time, it might be good to change the name; I'd suggest

+1

Paul

---

From: "Paul C. Bryan" <[email-masked]>
Date: Mon, 15 Oct 2012 09:37:10 -0700

> Aren’t we talking about arrays?

No, at this point, we're talking about values, which can exist within
arrays or objects.

> The array case is where it is a bit more complex. Adding and replacing
> are quite separate. Adding needs a gap, from the start or end;
> replacing needs an index, from the start or end. Gaps and indices can
> overflow. Ranges add more complexity.

This does raise the issue of attempting to set an array element outside
the current range of values (i.e. sparse allocation).

> You can compare 2 pointers to see if they point to the same thing
> (without having to resolve them first against the JSON doc they point
> at).

And what does that give us?

Paul

---

From: Mike McCabe <[email-masked]>
Date: Mon, 15 Oct 2012 13:09:49 -0700

All -

I'm glad to see this subject discussed.

In discussions about json-patch at my organization, both of these 
requests - unconditional set, and list append -  have come up.

Some users would like to be able to make changes to data without prior 
knowledge of it's exact state ahead of time.

(It's worth mentioning that another use of json-patch in our 
organization is to replace a system with strictly versioned updates, in 
which race conditions and inconsistent data aren't tolerated.  Both 
needs exist, and I think both can be served by json-patch.)

In practice, the lack of safe unconditional sets and appends has led me 
to recommend that internal users generate their patches by getting the 
old document, making changes to a copy, and using an automated diff() 
function to compare old and new, rather than free-coding checks.  This 
will probably always be the most robust way to specify arbitrary 
changes, but there are many uses where an unconditional set or append 
would be simpler.

On to opinions...

* I think there's room for a new op alongside the current 'add' op 
semantics.  I propose 'set' - exactly like 'add', but not throwing an 
error if the value already exists.  For lists, it could work like 
'replace', but could still append.  The existing 'add' semantics remain 
useful for 'transactional' uses of json-patch.

* I also like negative index notation.  It's use is well established in 
javascript (as an array slice() argument), in python and in other 
languages.  There it means 'add the length of the list to the negative 
index to get the offset' - thus -1 indicates the last element, not the 
slot after.  For this reason, if we add negative indices, I think we 
should use -0 to append.  (In fact, a co-worker came up with exactly 
this notation some weeks ago.)

* I'm skeptical of proposals that call for 'opening up' value arguments, 
to merge them into the target value or otherwise.  What if it's an 
array?  What if it's something else?  I think this would lose the 
current generality of allowing arbitrary json in the value field, and 
would make json-patch harder to reason about - and harder to specify!

I'm otherwise very happy with json-patch - I was casting around for a 
json language to describe data deltas when I found it, and it exactly
fits the bill.  We're still in internal testing of json-patch, but it 
will soon be part of the preferred public API for modifying Internet 
Archive item metadata.

Best regards,
Mike McCabe

On 10/15/12 9:37 AM, Paul C. Bryan wrote:
>> (without having to resolve them first against the JSON doc they point at).
>

---

From: "Manger, James H" <[email-masked]>
Date: Tue, 16 Oct 2012 14:48:34 +1100

> useful for 'transactional' uses of json-patch.

Using the dedicated "test" operation is better than duplicating an op.

> this notation some weeks ago.)

"-0" just feels a bit strange for a common case — appending to the end —
but that is a minor complaint.

> * I'm skeptical of proposals that call for 'opening up' value
> arguments, to merge them into the target value or otherwise.  What if
> it's an array?  What if it's something else?  I think this would lose
> the current generality of allowing arbitrary json in the value field,
> and would make json-patch harder to reason about - and harder to
> specify!

Which is easier to reason about:
 {"op":"add", "path":"/a/b/c/d/e/name", "value":"fred"},
 {"op":"add", "path":"/a/b/c/d/e/age", "value":42},
 {"op":"add", "path":"/a/b/c/d/e/foo", "value":{"x":1, "y":-2}},
Or
 {"op":"put", "path":"/a/b/c/d/e", "value":{
    "name":"fred",
    "age":42,
    "foo":{"x":1, "y":-2}
 }},
I think the latter is easier to read, write, and understand.
[change "value" to "obj" and/or "put" to "merge" if it helps
 clarify this object-specific operation]
It also mitigates the annoyance of having to repeat a potentially long
pointer in the common case of adding a bunch of elements to an object.

It might also reduce a likely common typo of forgetting the leading "/".
Saying       {"op":"add", "path":"name", "value":"fred"}
Instead of   {"op":"add", "path":"/name", "value":"fred"}

> And what does that give us?

Perhaps not much in a patch format, but JSON pointer will be used
more widely. For example a unique pointer may be suitable for an access
control list indicating which parts of a JSON doc are private.

--
James Manger

---

From: "Paul C. Bryan" <[email-masked]>
Date: Tue, 16 Oct 2012 09:00:07 -0700

> Using the dedicated "test" operation is better than duplicating an op.

+1. If you want to guarantee the state of (some part of) the document,
test should be the way to do it.

> control list indicating which parts of a JSON doc are private.

I wouldn't want to bake -1 into json-pointer. I think it should remain
in json-patch.

Paul

---

From: James M Snell <[email-masked]>
Date: Tue, 16 Oct 2012 09:57:36 -0700

Hmm... that would mean having to special case json-pointer expressions
specifically for json-patch, which isn't great either. Why not simply bake
it into json-pointer and be done with it? I'm sure json-patch is not the
only place where it could be useful.

On Tue, Oct 16, 2012 at 9:00 AM, Paul C. Bryan <[email-masked]> wrote:

> **
>
>
> [snip]
>
>
> I wouldn't want to bake -1 into json-pointer. I think it should remain in
> json-patch.
>
> Paul
>

---

From: "Paul C. Bryan" <[email-masked]>
Date: Tue, 16 Oct 2012 10:00:32 -0700

We already have special semantics in json-patch for how to handle the
case where the value doesn't exist (when it's being added). This seems
like another case of handling a non-existing referenced value.

Paul

On Tue, 2012-10-16 at 09:57 -0700, James M Snell wrote:

> specifically for json-patch, which isn't great either. Why not simply
> bake it into json-pointer and be done with it? I'm sure json-patch is
> not the only place where it could be useful.
> 
> 
>         I wouldn't want to bake -1 into json-pointer. I think it
>         should remain in json-patch.
>         
>         Paul
>         
>

---

From: James M Snell <[email-masked]>
Date: Tue, 16 Oct 2012 10:02:25 -0700

On Mon, Oct 15, 2012 at 1:09 PM, Mike McCabe <[email-masked]> wrote:

> In discussions about json-patch at my organization, both of these requests
> - unconditional set, and list append -  have come up.
>
> (It's worth mentioning that another use of json-patch in our organization
> is to replace a system with strictly versioned updates, in which race
> conditions and inconsistent data aren't tolerated.  Both needs exist, and I
> think both can be served by json-patch.)
>
> In practice, the lack of safe unconditional sets and appends has led me to
> recommend that internal users generate their patches by getting the old
> document, making changes to a copy, and using an automated diff() function
> to compare old and new, rather than free-coding checks.  This will probably
> always be the most robust way to specify arbitrary changes, but there are
> many uses where an unconditional set or append would be simpler.
>
>
> On to opinions...
>
> semantics.  I propose 'set' - exactly like 'add', but not throwing an error
> if the value already exists.  For lists, it could work like 'replace', but
> could still append.  The existing 'add' semantics remain useful for
> 'transactional' uses of json-patch.
>
>
I imagine that introducing a "set" when we already have "replace" is going
to cause confusion. Perhaps "replace" can be modified such that it acts
like "add" if the path does not currently exist. Or perhaps we rename
"replace" to "set" and keep "add" as it is currently defined.

- James

> index to get the offset' - thus -1 indicates the last element, not the slot
> after.  For this reason, if we add negative indices, I think we should use
> -0 to append.  (In fact, a co-worker came up with exactly this notation
> some weeks ago.)
>
> to merge them into the target value or otherwise.  What if it's an array?
>  What if it's something else?  I think this would lose the current
> generality of allowing arbitrary json in the value field, and would make
> json-patch harder to reason about - and harder to specify!
>
>
> I'm otherwise very happy with json-patch - I was casting around for a json
> language to describe data deltas when I found it, and it exactly
> fits the bill.  We're still in internal testing of json-patch, but it will
> soon be part of the preferred public API for modifying Internet Archive
> item metadata.
>
>
> Best regards,
> Mike McCabe
>
>
>
>
>> ______________________________**_________________
>> https://www.ietf.org/mailman/**listinfo/apps-discuss<https://www.ietf.org/mailman/listinfo/apps-discuss>
>>
>>  ______________________________**_________________
> https://www.ietf.org/mailman/**listinfo/apps-discuss<https://www.ietf.org/mailman/listinfo/apps-discuss>
>

---

From: "Paul C. Bryan" <[email-masked]>
Date: Tue, 16 Oct 2012 10:08:01 -0700

On Tue, 2012-10-16 at 10:02 -0700, James M Snell wrote:

> I imagine that introducing a "set" when we already have "replace" is
> going to cause confusion. Perhaps "replace" can be modified such that
> it acts like "add" if the path does not currently exist. Or perhaps we
> rename "replace" to "set" and keep "add" as it is currently defined.

I think the plan would be to drop "add" and "replace" in favour of
"insert" and "set".

Paul

---

From: James M Snell <[email-masked]>
Date: Tue, 16 Oct 2012 17:04:14 -0700

Special handling when the path refers to a non-existent object is different
than allowing for a json-patch specific variation in the pointer syntax
itself. If we allow for the use of negative indices then that change should
be baked in to json pointer itself. I see little harm in doing so.

- James

On Tue, Oct 16, 2012 at 10:00 AM, Paul C. Bryan <[email-masked]> wrote:

> **
> We already have special semantics in json-patch for how to handle the case
> where the value doesn't exist (when it's being added). This seems like
> another case of handling a non-existing referenced value.
>
> Paul
>
>

---

From: Mark Nottingham <[email-masked]>
Date: Wed, 17 Oct 2012 11:07:08 +1100

I tend to agree that if we're going to do it, it should be in pointer.

If it were left to me, I think I'd just have *one* special value whose semantics is "the end of the array." Simpler.

Cheers,

On 17/10/2012, at 11:04 AM, James M Snell <[email-masked]> wrote:

> Special handling when the path refers to a non-existent object is different than allowing for a json-patch specific variation in the pointer syntax itself. If we allow for the use of negative indices then that change should be baked in to json pointer itself. I see little harm in doing so.
> 
> - James
> 
> We already have special semantics in json-patch for how to handle the case where the value doesn't exist (when it's being added). This seems like another case of handling a non-existing referenced value.
> 
> Paul
> 
> 
>> Hmm... that would mean having to special case json-pointer expressions specifically for json-patch, which isn't great either. Why not simply bake it into json-pointer and be done with it? I'm sure json-patch is not the only place where it could be useful.
>> 
>> I wouldn't want to bake -1 into json-pointer. I think it should remain in json-patch.
>> 
>> Paul
>>

---

From: "Paul C. Bryan" <[email-masked]>
Date: Tue, 16 Oct 2012 17:12:33 -0700

Okay, if we're opening-up json-pointer to change, I think the other
thing I'm seeing in other threads is a desire to identify an object that
contains a specific member-value. Would you be open to discussing this?
I know this pushes final call out even further, but we're discussing
enhancements so I thought I'd put it out there.

Paul

---

From: James M Snell <[email-masked]>
Date: Tue, 16 Oct 2012 17:27:04 -0700

Hmm... that's a slippery slope, indeed. I think mechanisms such as the
"test" operation and my proposals in the JSON Predicates draft give us
plenty of options for testing values once they have been selected with a
JSON Pointer. I don't think we need to go down the path of building
predicate operations into the pointer itself.

As for Mark's comment, I'm good with either negative indices in general or
a single special case like "-" to indicate the end of the array. I agree
the latter would be simpler (and therefore likely marginally better)

Btw... If we do go with negative indices, the reference for the last
element in the array really should be "-0" since we're using zero based
arrays...

  {"foo": ["a","b","c"]}

  /foo/0  => "a"
  /foo/1  => "b"
  /foo/2  => "c"

  /foo/-0 => "c"
  /foo/-1 => "b"
  /foo/-2 => "a"

  /foo/-  => (the end of the array past c)

  [{"op": "set", "path": "/foo/-", "value": "d"},
  {"op": "set", "path": "/foo/-0", "value": "e"},
  {"op": "insert", "path": "/foo/1", "value": "f"}]

  Would result in:

  {"foo": ["a","f","b","e","d"]}

- James

On Tue, Oct 16, 2012 at 5:12 PM, Paul C. Bryan <[email-masked]> wrote:

> **
> Okay, if we're opening-up json-pointer to change, I think the other thing
> I'm seeing in other threads is a desire to identify an object that contains
> a specific member-value. Would you be open to discussing this? I know this
> pushes final call out even further, but we're discussing enhancements so I
> thought I'd put it out there.
>
> Paul
>
>

---

From: Mark Nottingham <[email-masked]>
Date: Wed, 17 Oct 2012 11:29:53 +1100

OK, unless anyone yells (or speaks in a not-so-soft voice), I'll add a single "-" as "end of array" in pointer.

Cheers,

On 17/10/2012, at 11:27 AM, James M Snell <[email-masked]> wrote:

> Hmm... that's a slippery slope, indeed. I think mechanisms such as the "test" operation and my proposals in the JSON Predicates draft give us plenty of options for testing values once they have been selected with a JSON Pointer. I don't think we need to go down the path of building predicate operations into the pointer itself. 
> 
> As for Mark's comment, I'm good with either negative indices in general or a single special case like "-" to indicate the end of the array. I agree the latter would be simpler (and therefore likely marginally better)
> 
> Btw... If we do go with negative indices, the reference for the last element in the array really should be "-0" since we're using zero based arrays...
> 
>   {"foo": ["a","b","c"]}
> 
>   /foo/0  => "a"
>   /foo/1  => "b"
>   /foo/2  => "c"
> 
>   /foo/-0 => "c"
>   /foo/-1 => "b"
>   /foo/-2 => "a"
> 
> Okay, if we're opening-up json-pointer to change, I think the other thing I'm seeing in other threads is a desire to identify an object that contains a specific member-value. Would you be open to discussing this? I know this pushes final call out even further, but we're discussing enhancements so I thought I'd put it out there.
> 
> Paul
> 
> 
>> On 17/10/2012, at 11:04 AM, James M Snell <
>> [email-masked]
>> > wrote:
>> 
>> > On Tue, Oct 16, 2012 at 10:00 AM, Paul C. Bryan <
>> [email-masked]
>> > wrote:
>> >> On Tue, Oct 16, 2012 at 9:00 AM, Paul C. Bryan <
>> [email-masked]
>> > wrote:
>> >>> [snip]
>> >>> 
>> >> 
>> >>

---

From: "Manger, James H" <[email-masked]>
Date: Wed, 17 Oct 2012 11:33:48 +1100

*One* special value might work well.
Define it as a pointer to the index just *beyond* the end of an array (not to the last item in the array).
That way it never identifies a value in a JSON doc. Any value still has a unique pointer.
It is only useful for identifying a point before which to insert a value.

--
James Manger

> Sent: Wednesday, 17 October 2012 11:07 AM
> To: James M Snell
> Cc: Paul C. Bryan; Manger, James H; [email-masked]
> If it were left to me, I think I'd just have *one* special value whose
> semantics is "the end of the array." Simpler.
> 
> Cheers,
> 
> 
> > Special handling when the path refers to a non-existent object is
> different than allowing for a json-patch specific variation in the
> pointer syntax itself. If we allow for the use of negative indices then
> that change should be baked in to json pointer itself. I see little
> harm in doing so.
> >
> > - James
> >
> >> Hmm... that would mean having to special case json-pointer
> expressions specifically for json-patch, which isn't great either. Why
> not simply bake it into json-pointer and be done with it? I'm sure
> json-patch is not the only place where it could be useful.
> >>

---

From: Mark Nottingham <[email-masked]>
Date: Wed, 17 Oct 2012 11:35:07 +1100

That might make the language tricker (since we talk about moving members when there's a reference to a non-existant member), but I'll give it a go.

Cheers,

---

From: "Paul C. Bryan" <[email-masked]>
Date: Tue, 16 Oct 2012 20:39:18 -0700

Grumble mumble...

---

From: Mark Nottingham <[email-masked]>
Date: Mon, 22 Oct 2012 15:34:51 +1100

>> {"op":"copy", "path":"/a", "to":"/a/b/c"}

Addressing with:

                <t>The location in the "to" member MUST NOT be part of the
                location defined by "path"; i.e., a location cannot be [copied | moved]
                into one of its children.</t>

---

From: Mark Nottingham <[email-masked]>
Date: Wed, 24 Oct 2012 17:49:39 +1100

This didn't make it into the last draft, and I'd like to get us to WGLC soon.

Personally, I'm going to push back on it; it's a syntactic convenience at most, and I think that most JSON Patch documents will be generated by code. This format really needs to be as simple as possible, and IME a convenience function like this doesn't make the cut.

However, that's just me.

Do people adamantly-want-this, or is it just nice-to-have?

What do the people actually implementing JSON Patch think?

Cheers,

---

From: James M Snell <[email-masked]>
Date: Tue, 23 Oct 2012 23:54:25 -0700

On Oct 23, 2012 11:49 PM, "Mark Nottingham" <[email-masked]> wrote:
> Personally, I'm going to push back on it; it's a syntactic convenience at
most, and I think that most JSON Patch documents will be generated by code.
This format really needs to be as simple as possible, and IME a convenience
function like this doesn't make the cut.

+1

---

From: Mike McCabe <[email-masked]>
Date: Wed, 24 Oct 2012 08:52:29 -0700

I also agree.  I don't think that this adds much utility - it would also 
mean a considerable departure from the existing flat 'op' structure.

Regards,
Mike