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

From: [email-masked]
Date: Mon, 30 Jun 2014 10:07:57 -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           : JSON Merge Patch
        Authors         : Paul Hoffman
                          James M Snell
	Filename        : draft-ietf-appsawg-json-merge-patch-03.txt
	Pages           : 11
	Date            : 2014-06-30

Abstract:
   This specification defines the JSON merge patch format and processing
   rules.  The merge patch format is primarily intended for use with the
   HTTP PATCH method as a means of describing a set of modifications to
   a subset of target resource's content.

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-03

A diff from the previous version is available at:
http://www.ietf.org/rfcdiff?url2=draft-ietf-appsawg-json-merge-patch-03

Please note that it may take a couple of minutes from the time of submission
until the htmlized version and diff are available at tools.ietf.org.

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

---

From: Paul Hoffman <[email-masked]>
Date: Mon, 30 Jun 2014 10:22:54 -0700

Greetings. James has added me as co-editor on appsawg-json-merge-patch, and I incorporated the changes from WG LC. The most prominent change is clarifying the use of nulls in the patch document, but there were others as well.

If there are no more changes, the chairs can move this on to the ADs for review.

--Paul Hoffman

---

From: "Murray S. Kucherawy" <[email-masked]>
Date: Mon, 30 Jun 2014 11:08:59 -0700

Thanks, Paul.  I'll give this a couple of days for people to reply and then
send it along.

If anyone would like to shepherd this document, so as to give someone the
experience of doing so, please let us know ([email-masked]).
If not, I'll do the writeup myself.

-MSK

On Mon, Jun 30, 2014 at 10:22 AM, Paul Hoffman <[email-masked]>
wrote:

> Greetings. James has added me as co-editor on appsawg-json-merge-patch,
> and I incorporated the changes from WG LC. The most prominent change is
> clarifying the use of nulls in the patch document, but there were others as
> well.
>
> _______________________________________________
> apps-discuss mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/apps-discuss
>

---

From: "Manger, James" <[email-masked]>
Date: Tue, 1 Jul 2014 10:31:06 +1000

Paul,

>  draft-ietf-appsawg-json-merge-patch-03

Thanks for stepping up to edit this document.

> If there are no more changes, the chairs can move this on to the ADs for review.

More changes are required.

1.
Consider a patch such as {"name":"John", "age":null}.
When applied to (ie merged with) a target the result should always be (1) an object, (2) with a "name":"John" member, and (3) no "age" member.

Unfortunately the draft-03 rules don't guarantee this.
If the target was an array, for instance, the result is {"name":"John", "age":null}!
Section 2 rules 1 and 2.D are the problems.

One way to describe the correct rule would be to say: when the patch value is an object, but the target value is not (including when the target value is undefined), then merge the patch value with an empty object {} to get the result.

2.
Appendix A has dropped an important case: when a nested null causes an empty object to be added to the target.
Target {} merged with patch {"a":{"bb":{"ccc":null}}} gives {"a":{"bb":{}}}.

3.
The merge behaviour in draft-03 is now deliberately undefined if the patch is not an object or array. However, it is also undefined if the target is not an object or array. This undefined behaviour can only do harm. Many implementations will involve a merge(target, patch) function that accepts any JSON for its arguments so we may as well define all the results. It is also strange to mention arrays explicitly as merge-patch treats arrays just like a primitive.

--
James Manger

---

From: Carsten Bormann <[email-masked]>
Date: Tue, 1 Jul 2014 08:31:43 +0200

Good points.

Here is my implementation:

def merge_patch(orig, patch)
  if Hash === patch
    orig = {} unless Hash === orig
    patch.each do |k, v|
      if v.nil?
        orig.delete(k)
      else
        orig[k] = merge_patch(orig[k], v)
      end
    end
    orig
  else
    patch
  end
end                             # yep, that's 15 lines.

This could serve to replace the Appendix B.

> The merge behaviour in draft-03 is now deliberately undefined if the patch is not an object or array. However, it is also undefined if the target is not an object or array. This undefined behaviour…

Therefore, it does not check against the examples:

Mismatch: 
orig:     {"a":"foo"}
patch:    null
draft:    "Invalid Example"
actual:   null
Mismatch: 
orig:     {"a":"foo"}
patch:    "bar"
draft:    "Invalid Example"
actual:   “bar"

It does check against your example:

           {}              {"a":{"bb":     {"a":{"bb":{}}}
                           {"ccc":null}}}

I believe the above implementation is exactly what merge-patch should do, and the text should be updated accordingly.

Grüße, Carsten