Subject: [apps-discuss] I-D Action: draft-ietf-appsawg-json-merge-patch-00.txt

From: [email-masked]
Date: Mon, 20 May 2013 16:33:53 -0700

A New Internet-Draft is available from the on-line Internet-Drafts directories.
 This draft is a work item of the Applications Area Working Group Working Group of the IETF.

	Title           : The application/json-merge-patch Media Type
	Author(s)       : James M Snell
	Filename        : draft-ietf-appsawg-json-merge-patch-00.txt
	Pages           : 10
	Date            : 2013-05-20

Abstract:
   This specification defines the application/json-merge-patch media
   type and it's intended use with the HTTP PATCH method defined by RFC
   5789.

The IETF datatracker status page for this draft is:
https://datatracker.ietf.org/doc/draft-ietf-appsawg-json-merge-patch

There's also a htmlized version available at:
http://tools.ietf.org/html/draft-ietf-appsawg-json-merge-patch-00

Internet-Drafts are also available by anonymous FTP at:
ftp://ftp.ietf.org/internet-drafts/

---

From: James M Snell <[email-masked]>
Date: Mon, 20 May 2013 16:38:06 -0700

A few minor updates here as well as the initial appsawg draft.
Includes a fix in the example javascript implementation. Thanks to
Steve and James M. for their valuable input.

On Mon, May 20, 2013 at 4:33 PM,  <[email-masked]> wrote:
> _______________________________________________
> apps-discuss mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/apps-discuss

---

From: Steve Klabnik <[email-masked]>
Date: Mon, 20 May 2013 20:44:11 -0700

Unsure if this is the right place for this, but it should be 'its' in the introduction.

Ill give it a closer look later this evening.

---

From: James M Snell <[email-masked]>
Date: Mon, 20 May 2013 20:51:04 -0700

Feel free to use my github repo to report any and all editorial bugs..
pull requests accepted. All technical changes, however, need to be
discussed here first :-)

https://github.com/jasnell/specs/tree/master/mature

---

From: "Manger, James H" <[email-masked]>
Date: Tue, 21 May 2013 18:11:25 +1000

> >    type and it's intended use with the HTTP PATCH method defined by
> >    RFC 5789.
> >

I don't agree with the rules that give this test case result:

Original       Patch            Result
--------------------------------------
{"a":"foo"}    {"b": [         {"a":"foo",	
 	            3,            "b": [3, {}]	
 	            null,        }	
 	            {"x":null}
 	           ]	
 	         }

The rules should give the following result:

  {"a":"foo",
   "b": [
     3,
     null,
     {"x":null}
    ]
  }

It is pointless making the receiver of a patch remove nulls-within-an-array just to get a result that is identical to the sender of the patch simply not including those nulls-within-an-array in the first place.

The spec says "JSON arrays are treated the same as JSON primitives". This should be reflected in the processing rules (and then the test case). Implementations shouldn't need to look inside arrays.

--
James Manger

---

From: James M Snell <[email-masked]>
Date: Tue, 21 May 2013 08:08:06 -0700

On Tue, May 21, 2013 at 1:11 AM, Manger, James H
<[email-masked]> wrote:
[snip]
> The spec says "JSON arrays are treated the same as JSON primitives". This should be reflected in the processing rules (and then the test case). Implementations shouldn't need to look inside arrays.

The spec also states that Null specifically that "The JSON null value
is given a special meaning to indicate the removal of an existing
value". A merge-patch document should not include any null values
unless it is specifically intending to remove something from the
target document. The fact that nulls in arrays were not appropriately
dealt with previously is a spec bug, not a feature ;-) ...

- James

---

From: "Manger, James H" <[email-masked]>
Date: Wed, 22 May 2013 10:43:48 +1000

> > It is pointless making the receiver of a patch remove nulls-within-
> an-array just to get a result that is identical to the sender of the
> patch simply not including those nulls-within-an-array in the first
> place.
> >
> > The spec says "JSON arrays are treated the same as JSON primitives".
> This should be reflected in the processing rules (and then the test
> case). Implementations shouldn't need to look inside arrays.

James Snell wrote:
> unless it is specifically intending to remove something from the target
> document. The fact that nulls in arrays were not appropriately dealt
> with previously is a spec bug, not a feature ;-) ...

Removing nulls in arrays means the patch process can be described in 2 stages:

Stage 1: Walk through objects and arrays in the patch removing any nulls inside arrays, resulting in a modified patch.

Stage 2: Walk through objects in the document and modified patch together -- adding, replacing or deleting object elements.

Stage 1 adds zero functionality. Starting with the modified patch achieves an identical result. Stage 1 does adds extra work (extra code, extra time) for the patch processor. No (legitimate) patches will ever include nulls-in-arrays that they have zero impact on the result. The likely consequence is that the extra code to remove those null will be implemented poorly, if at all.

Stage 1 removes some functionality. It further restricts the JSON values that can be created by a merge-patch.

If the rationale is that "all nulls are special", then silently removing them from patches is not appropriate. It would be more appropriate to change stage 1 to throw an error if a patch includes any nulls in an array.

Dropping stage 1 is the sensible approach.

--
James Manger

---

From: James M Snell <[email-masked]>
Date: Tue, 21 May 2013 18:06:19 -0700

Alternatively, one can simply not send a merge patch with null array
elements.

At this point, however, it's just you and I talking back and forth.  Let's
let others have a chance to weigh in on it.
On May 21, 2013 5:44 PM, "Manger, James H" <[email-masked]>
wrote:

> Stage 1: Walk through objects and arrays in the patch removing any nulls
> inside arrays, resulting in a modified patch.
>
> Stage 2: Walk through objects in the document and modified patch together
> -- adding, replacing or deleting object elements.
>
> Stage 1 adds zero functionality. Starting with the modified patch achieves
> an identical result. Stage 1 does adds extra work (extra code, extra time)
> for the patch processor. No (legitimate) patches will ever include
> nulls-in-arrays that they have zero impact on the result. The likely
> consequence is that the extra code to remove those null will be implemented
> poorly, if at all.
>
> If the rationale is that "all nulls are special", then silently removing
> them from patches is not appropriate. It would be more appropriate to
> change stage 1 to throw an error if a patch includes any nulls in an array.
>