[apps-discuss] AppsDir early review for draft-ietf-appsawg-json-merge-patch-02
Carsten Bormann <cabo@tzi.org> Mon, 09 December 2013 22:15 UTC
Return-Path: <cabo@tzi.org>
X-Original-To: apps-discuss@ietfa.amsl.com
Delivered-To: apps-discuss@ietfa.amsl.com
Received: from localhost (ietfa.amsl.com [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 78F4A1AE5F2 for <apps-discuss@ietfa.amsl.com>; Mon, 9 Dec 2013 14:15:09 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.251
X-Spam-Level:
X-Spam-Status: No, score=-1.251 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, HELO_EQ_DE=0.35, MIME_8BIT_HEADER=0.3, SPF_HELO_PASS=-0.001] autolearn=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id VK8I0zrdP648 for <apps-discuss@ietfa.amsl.com>; Mon, 9 Dec 2013 14:15:07 -0800 (PST)
Received: from informatik.uni-bremen.de (mailhost.informatik.uni-bremen.de [IPv6:2001:638:708:30c9::12]) by ietfa.amsl.com (Postfix) with ESMTP id 3A4B11AE5BE for <apps-discuss@ietf.org>; Mon, 9 Dec 2013 14:15:07 -0800 (PST)
X-Virus-Scanned: amavisd-new at informatik.uni-bremen.de
Received: from smtp-fb3.informatik.uni-bremen.de (smtp-fb3.informatik.uni-bremen.de [134.102.224.120]) by informatik.uni-bremen.de (8.14.5/8.14.5) with ESMTP id rB9MExMr013359; Mon, 9 Dec 2013 23:14:59 +0100 (CET)
Received: from [192.168.217.144] (p54890261.dip0.t-ipconnect.de [84.137.2.97]) (using TLSv1 with cipher AES128-SHA (128/128 bits)) (No client certificate requested) by smtp-fb3.informatik.uni-bremen.de (Postfix) with ESMTPSA id 9B25529; Mon, 9 Dec 2013 23:14:58 +0100 (CET)
From: Carsten Bormann <cabo@tzi.org>
Content-Type: text/plain; charset="windows-1252"
Content-Transfer-Encoding: quoted-printable
Date: Mon, 09 Dec 2013 23:14:56 +0100
To: "apps-discuss@ietf.org Discuss" <apps-discuss@ietf.org>, draft-ietf-appsawg-json-merge-patch.all@tools.ietf.org
Message-Id: <B3789B30-2698-4525-96ED-BBA9D8351941@tzi.org>
Mime-Version: 1.0 (Mac OS X Mail 7.0 \(1822\))
X-Mailer: Apple Mail (2.1822)
Subject: [apps-discuss] AppsDir early review for draft-ietf-appsawg-json-merge-patch-02
X-BeenThere: apps-discuss@ietf.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: General discussion of application-layer protocols <apps-discuss.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/apps-discuss>, <mailto:apps-discuss-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/apps-discuss/>
List-Post: <mailto:apps-discuss@ietf.org>
List-Help: <mailto:apps-discuss-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/apps-discuss>, <mailto:apps-discuss-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 09 Dec 2013 22:15:09 -0000
I have been selected as the Applications Area Directorate reviewer for this draft (for background on appsdir, please see http://trac.tools.ietf.org/area/app/trac/wiki/ApplicationsAreaDirectorate). Please resolve these comments along with any other Last Call comments you may receive. Please wait for direction from your WG chair(s) before posting a new version of the draft. Document: draft-ietf-appsawg-json-merge-patch-02 Title: JSON Merge Patch Reviewer: Carsten Bormann Review Date: 2013-12-09 WG Last Call Date: WGLC ended October 4, 2013 Summary: This draft is NOT ready to progress now as a Proposed Standard and should be revised before an IETF Last Call is issued. MAJOR: 1) The language defining the operation of the patch mechanism is unnecessarily confusing. Several phrases are simply not accessible to this reader. E.g., "Any null member contained in the merge patch MUST be ignored and treated as if those members are undefined." (Members in JSON are name/value pairs. Only the values could be the null value. There is no undefined value in JSON. A member can't be both ignored and treated in a specific way.) 2) As the algorithm appears to be defined, there is never a reason for a server to send arrays with null elements. (In JSON, arrays have elements, not members; I'm not sure I'm reading some of the text right.) The purge_nulls mechanism is therefore completely unneeded. Why would a sender send nulls in arrays only to have them removed by the receiver? By removing this unnecessary processing, the description could be simplified considerably and implementations could be more performant. https://developers.google.com/blogger/docs/3.0/performance?hl=en#patch which was cited as a real-world early inspiration for merge-patch does not do any of this superfluous processing either. Rationalizing the existing algorithm means that an implementation only needs to descend recursively through objects, never through other JSON values. Only this descent is justified, as it is the only productive function of the merge algorithm. (The fix also would allow to have nulls in replacement arrays.) The value »null« should only ever be special in the value position of a name/value pair of an object. The text then would only need to distingish object and non-object values, considerably simplifying it and increasing the chance of interoperability. Fixing this will also make the example code much shorter and thus more accessible. 3) The text needs to be modified to take the recent changes to JSON into account. While 4627bis is not yet approved, it is likely that the change to allow non-containers as JSON texts will stand. Merge-patch would then immediately become obsolete once 4627bis is approved. 4) The definition of a charset media type parameter is unacceptable for a +json media type. (Even if it were acceptable, its semantics would need to be defined.) MINOR: 5) Several of the examples contain weird instances of whitespace that are inconsistent between the pre-patch and the post-patch state. This is not just about appearance: It might make the reader wonder whether the patch algorithm is also about changing/updating whitespace. (Clearly, the algorithm is operating at the JSON data model level.) 6) The SHOULD NOTs at the end of section 2 are confusing, as they already seem to be implied by processing required (at a MUST level) further above. NITS: Section 2, item 2: s/or/and/ Grüße, Carsten PS.: The appsdir review template doesn’t have a space for comments. Let me just add at this point that I like what the draft is trying to do and I strongly believe it will be a useful specification to have available.
- [apps-discuss] AppsDir early review for draft-iet… Carsten Bormann
- Re: [apps-discuss] AppsDir early review for draft… Julian Reschke