Subject: Re: Last Call: <draft-ietf-appsawg-json-patch-08.txt> (JSON Patch) to Proposed Standard

From: Barry Leiba <[email-masked]>
Date: Tue, 11 Dec 2012 10:25:57 -0500

> Abstract
>    JSON Patch defines the media type "application/json-patch", a JSON
>    document structure for expressing a sequence of operations to apply
>    to a JSON document, suitable for use with the HTTP PATCH method.
...
> http://datatracker.ietf.org/doc/draft-ietf-appsawg-json-patch/

I've reviewed JSON Patch and JSON Pointer as responsible AD, and am
very happy with the documents -- this is good work, well written.  I
came up with one issue that I want to discuss as part of last call:

   4.1.  add

   The "add" operation adds a new value at the target location.  The
   operation object MUST contain a "value" member that specifies the
   value to be added.

   For example:

   { "op": "add", "path": "/a/b/c", "value": [ "foo", "bar" ] }

   When the operation is applied, the target location MUST reference one
   of:

   o  The root of the target document - whereupon the specified value
      becomes the entire content of the target document.

Now, what this means is that if we start with this:

{ "a": { "num": 1 } }

and we apply this:

{ "op": "add", "path": "", "value": [ "foo", "bar" ] }

we end up with this:

[ "foo", "bar" ]

This doesn't strike me as having any sense of an "add" operation -- it
appears to be a special case that doesn't fit.  In any other
situation, using any other path, the operation either adds something
to what's already there, or it fails.  But when the path is "", it's
anomalous.

So, three questions:

1. Do I have this right, or am I mistaken about the result of that operation?

2. Assuming I have it right, can someone explain why it's this way?

3. Can someone explain why this is the right way to specify it, rather
than using "replace" for this?

Barry

---

From: Pier-Luc Thériault <[email-masked]>
Date: Tue, 11 Dec 2012 18:12:28 -0500

> http://datatracker.ietf.org/doc/draft-ietf-appsawg-json-patch/
I think that at paragraph A.14 the first occurence of

A JSON Patch document:

should be

An example target JSON document:

---

From: Pier-Luc Thériault <[email-masked]>
Date: Tue, 11 Dec 2012 18:32:59 -0500

> 1. Do I have this right, or am I mistaken about the result of that operation?
Yes, it's the good result
> 2. Assuming I have it right, can someone explain why it's this way?
It's the same as adding a value at any existing path
> than using "replace" for this?
It's not the right way, it's a right way. You can use replace to be sure 
that there's a root but there's always a root. Personally I would use 
replace

---

From: Mark Nottingham <[email-masked]>
Date: Wed, 12 Dec 2012 10:41:26 +1100

In SVN, thanks.

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

---

From: Mark Nottingham <[email-masked]>
Date: Wed, 12 Dec 2012 10:43:25 +1100

> 1. Do I have this right, or am I mistaken about the result of that operation?

The former.

> 2. Assuming I have it right, can someone explain why it's this way?

"add" has the semantics of "make the value *this*".

"replace" has the semantics of "make the existing value *this*"; if there isn't an existing value, it's an error.

> than using "replace" for this?

As has already been mentioned, there are multiple tools in the box.

Cheers,

---

From: Barry Leiba <[email-masked]>
Date: Tue, 11 Dec 2012 19:24:27 -0500

> "add" has the semantics of "make the value *this*".

Yeh, I guess the problem I have with that is that that's not the
semantics most of us attach to the concept of "add a thing to a set".
And it bothers me that "add to an object" and "add to an array" have
different semantics.

That difference is unsettling.  It's OK to have "add" on an array have
the semantics of "insert", especially because there's no "insert"
action.  But having "add" on an object have the semantics of "replace"
seems bad, partly because it's inconsistent with the semantics it has
on an array and because there's already a "replace" action.

If I have { "baz": "qux", "foo": "bar" }, these two do exactly the same thing:

case 1a: [{ "op": "add", "path": "/baz", "value": "blarg" }]

case 1b: [{ "op": "replace", "path": "/baz", "value": "blarg" }]

Both result in { "baz": "blarg", "foo": "bar" }

While these are different:

case 2a: [{ "op": "add", "path": "/hah", "value": "blarg" }]

case 2b: [{ "op": "replace", "path": "/hah", "value": "blarg" }]

Case 2a results in { "baz": "blarg", "foo": "bar", "hah": "blarg" },
and 2b is an error.

Case 1 just seems wrong.  The "add" in 1a should be an error, and then
life would make sense.

> As has already been mentioned, there are multiple tools in the box.

Stretching the analogy, having the wrench act as a hammer in some
cases does not make for a particularly well defined, robust tool box.

Barry

---

From: Mark Nottingham <[email-masked]>
Date: Wed, 12 Dec 2012 11:31:39 +1100

So, do you have a suggestion?

---

From: Barry Leiba <[email-masked]>
Date: Tue, 11 Dec 2012 20:01:19 -0500

>> Case 1 just seems wrong.  The "add" in 1a should be an error, and then

Turning that into a text suggestion, it would be this (which
represents a change to the protocol, so the working group would have
to accept it):

OLD

   o  A member to add to an existing object - whereupon the supplied
      value is added to that object at the indicated location.  If the
      member already exists, it is replaced by the specified value.
NEW
   When the operation is applied, the target location MUST reference one
   of:

   o  A member to add to an existing object - whereupon the supplied
      value is added to that object at the indicated location.  It is an error
      for the specified member to already exist.
END

OLD
   For example, "add"ing to the path "/a/b" to this document:

   { "a": { "foo": 1 } }

   is not an error, because "a" exists, and "b" will be added to its
   value.  It is an error in this document:

   { "q": { "bar": 2 } }

   because "a" does not exist.
NEW
   For example, given the following starting document:

   { "a": { "foo": 1 } }

   o  "add"ing to the path "/a/b" is not an error, because "/a" exists,
      and "b" will be added to its value.

   o  "add"ing to the path "/q/b" is an error, because "/q" does not
      exist.  But "/q" can be added first, followed by "add"ing "/q/b".

   o  "add"ing to the path "/a" or "/a/foo" is an error, because "/a"
      and "/a/foo" both exist already, and cannot be added.
END

I understand that this will change implementations -- patches that
used to use "add" will now have to use "replace", and there's now no
way to do "add this if it's not already there, and replace it if it is
already there".  Perhaps there's a need to add something with those
semantics.  On the other hand, as the text stands now, there's no way
to do "add this only if it's not already there", because "test" can't
test for existence.

Barry

---

From: Mark Nottingham <[email-masked]>
Date: Wed, 12 Dec 2012 12:14:49 +1100

On 12/12/2012, at 12:01 PM, Barry Leiba <[email-masked]> wrote:
> already there".

That was a very desirable feature for many people in the WG.

Personally -- to me, it seems like you're getting hung up on the word "add." We've had a few bits of feedback, where people try to map a particular meaning of one of the operation names to a programming language or other system. In this format, "add" means what the format definition says it means, because otherwise we have to rationalise all of the different systems people might use it with to make sense.

That said, if changing the operation name would make things easier, I'd be OK with that; e.g., "set." However, I suspect doing so would just raise issues from other people who are used to having "set" mean something slightly different.

> Perhaps there's a need to add something with those

We discussed having a test for existence in the WG, but there was agreement that it wasn't important enough to justify the added complexity. YMMV, of course.

Cheers,

---

From: Barry Leiba <[email-masked]>
Date: Tue, 11 Dec 2012 23:25:15 -0500

> Personally -- to me, it seems like you're getting hung up on the word "add."
...
> "add" means what the format definition says it means, because otherwise
> we have to rationalise all of the different systems people might use it with
> to make sense.

OK, I'll buy that.  Then let's take a different approach, and make it
clearer that it's Humpty Dumpty's version of "add", so maybe neither I
nor Alice will get hung up on it:

OLD
   The "add" operation adds a new value at the target location. The
   operation object MUST contain a "value" member that specifies the
   value to be added.
NEW
   The "add" operation performs the following function, depending upon
   what the target location references (see details below):

   o  If the target location specifies an array index, a new value is
      inserted into the array at the specified index.

   o  If the target location specifies an object member that does not
      already exist, a new member is added to the object.

   o  If the target location specifies an object member that does
      exist, that member's value is replaced.
END

That may be wordier than we need, but I think it makes the point...
feel free to wordsmith.  With something like that, I think we can say
that we had to pick a name, "add" was picked, and it means exactly
what we choose it to mean - neither more nor less.  And that's glory
for you.

Barry

---

From: James M Snell <[email-masked]>
Date: Tue, 11 Dec 2012 09:22:14 -0800

You are reading it correctly. Note also that the very next bullet point
says:

  A member to add to an existing object - whereupon the supplied
  value is added to that object at the indicated location.  If the
  member already exists, it is replaced by the specified value.

So an "add" is really a "replace" that does not have the "target location
MUST exist" restriction. Personally, I'm not too fond of that decision. For
anything other than an array, "add" should fail if the target already
exists. For arrays, "add" should insert but never replace an existing
value.

- James

On Tue, Dec 11, 2012 at 7:25 AM, Barry Leiba <[email-masked]>wrote:

> _______________________________________________
> apps-discuss mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/apps-discuss
>

---

From: "Paul C. Bryan" <[email-masked]>
Date: Tue, 11 Dec 2012 09:41:40 -0800

I'm not fond of that decision either.

Paul

On Tue, 2012-12-11 at 09:22 -0800, James M Snell wrote:

> You are reading it correctly. Note also that the very next bullet
> point says: 
> 
> 
> 
> So an "add" is really a "replace" that does not have the "target
> location MUST exist" restriction. Personally, I'm not too fond of that
> decision. For anything other than an array, "add" should fail if the
> target already exists. For arrays, "add" should insert but never
> replace an existing value. 
> 
> 
> - James
> 
> 
>

---

From: Dick Franks <[email-masked]>
Date: Wed, 12 Dec 2012 01:53:45 +0000

On 12 December 2012 01:14, Mark Nottingham <[email-masked]> wrote:
[snip]

> Personally -- to me, it seems like you're getting hung up on the word "add." We've had a few bits of feedback, where people try to map a particular meaning of one of the operation names to a progra…

"When I use a word", Humpty Dumpty said, in rather a scornful tone,
"it means what I choose it to mean - neither more nor less."

Lewis Carroll. Through the Looking Glass. 1872.

---

From: Barry Leiba <[email-masked]>
Date: Sun, 16 Dec 2012 18:12:55 -0500

Anyone have any comments on what I suggested below?

Barry

---

From: John C Klensin <[email-masked]>
Date: Sun, 16 Dec 2012 18:53:25 -0500

> Anyone have any comments on what I suggested below?

How about "+1" --complete agreement.  The terminology, if
interpreted in a "normal English" context, lies somewhere
between confusing and misleading and a precise local definition
is the more efficient fix.

   john

>>> "add" means what the format definition says it means,
>>> because otherwise we have to rationalise all of the
>>> different systems people might use it with to make sense.
>> 
>> OK, I'll buy that.  Then let's take a different approach, and
>> make it clearer that it's Humpty Dumpty's version of "add",
>> so maybe neither I nor Alice will get hung up on it:
>...

---

From: Mark Nottingham <[email-masked]>
Date: Mon, 17 Dec 2012 11:31:29 +1100

Works for me.

---

From: Mark Nottingham <[email-masked]>
Date: Mon, 17 Dec 2012 14:30:21 +1100

Now in SVN.