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

From: [email-masked]
Date: Fri, 09 Mar 2012 13:22:31 -0800

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 Patch
	Author(s)       : Paul C. Bryan
	Filename        : draft-ietf-appsawg-json-patch-01.txt
	Pages           : 12
	Date            : 2012-03-09

   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.

A URL for this Internet-Draft is:
http://www.ietf.org/internet-drafts/draft-ietf-appsawg-json-patch-01.txt

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

This Internet-Draft can be retrieved at:
ftp://ftp.ietf.org/internet-drafts/draft-ietf-appsawg-json-patch-01.txt

---

From: Vadim Zaliva <[email-masked]>
Date: Sat, 10 Mar 2012 22:33:03 -0800

I would like to suggest some extensions to this draft. I was trying to use proposed syntax in one of my project
and found it lacking a couple of operations. Please find below my proposed extensions:

Operation “add-unique”

The new value appended the array only if the it presently does not contain an element with such value. If an element with such value already exists, the duplicate will not be added. 

If an element referenced by this command does not exists, it will be created, as an array with single value. If an element referenced by this command exists, but is not an array such situation will be treated as an error. 

An example target JSON document:

{
       "foo": [ "bar", "qux", "baz" ]
}

A JSON Patch document:

[
       { "add-unique": "/foo", "value": "tic" }
]

The resulting JSON document:

{
       "foo": [ "bar", "qux", "baz", "tic" ]
}

Operation “remove-all”

This operations removes from array all elements with given value.

An example target JSON document:
{
       "foo": [ "bar", "qux", "baz", "tic" ]
}

A JSON Patch document:

[
       { "remove-all": "/foo", "value": "tic" }
]

The resulting JSON document:

{
       "foo": [ "bar", "qux", "baz" ]
}

Operation “replace-all”

This operations replaces in an array all elements with given value with a new value.

An example target JSON document:
{
       "foo": [ "bar", "qux", "baz", "tic" ]
}

A JSON Patch document:

[
       { "replace-all": "/foo", "old-value": "qux", "new-value": "zap" }
]

The resulting JSON document:

{
       "foo": [ "bar", "zap", "baz", "tic" ]
}

P.S. I am not sure "add-unique" is the best name. I guess 'array-add-if-not-present' is more appropriate bit it is a bit too long.

P.P.S. I am tempted to extend semantics of proposed 'remove-all' and 'replace-all' operations also to object properties, if they are applied to JSON object, not an array. They would either remove or replace values of all properties with matching value. It would make things nice and symmetric, but I am not sure if there is enough realistic use-cases for such use.

Sincerely,
Vadim

---

From: "Murray S. Kucherawy" <[email-masked]>
Date: Mon, 12 Mar 2012 20:55:04 +0000

> -----Original Message-----
> From: [email-masked] [mailto:[email-masked]] On Behalf Of Vadim Zaliva
> Sent: Saturday, March 10, 2012 10:33 PM
> To: [email-masked]
> Subject: Re: [apps-discuss] I-D Action: draft-ietf-appsawg-json-patch-01.txt
> 
> I would like to suggest some extensions to this draft. I was trying to
> use proposed syntax in one of my project and found it lacking a couple
> of operations. Please find below my proposed extensions:
> 
> Operation "add-unique"
> 
> The new value appended the array only if the it presently does not
> contain an element with such value. If an element with such value
> already exists, the duplicate will not be added.

This feels like feature creep to me.  A patch to me is a sequence of steps for moving some content from one known state to another.  This proposed capability allows one to move from an uncertain state to a known state.  That might be a useful thing to do, but it's not something I think of as a "patch".

> Operation "remove-all"

I have the same concern here.

> Operation "replace-all"

And here.

-MSK

---

From: Mike Acar <[email-masked]>
Date: Mon, 12 Mar 2012 14:55:10 -0700

On 03/09/2012 01:22 PM, [email-masked] wrote:
>
> A New Internet-Draft is available from the on-line Internet-Drafts
> directories.

> 	Filename        : draft-ietf-appsawg-json-patch-01.txt

This version of the draft doesn't address the bad interaction with the 
JSON Pointer spec, namely that "add" takes a JSON Pointer to a value 
which does not yet exist, and the Pointer spec says that in that case, 
an implementation MAY abort with an error. Is that going to be addressed 
in -02?

-- 
Mike Acar -                                 - macar at cloudmark dot com

---

From: "Paul C. Bryan" <[email-masked]>
Date: Tue, 13 Mar 2012 08:18:51 -0700

I agree with Murray's points.

Paul

On Mon, 2012-03-12 at 20:55 +0000, Murray S. Kucherawy wrote:

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

---

From: "Paul C. Bryan" <[email-masked]>
Date: Tue, 13 Mar 2012 08:31:50 -0700

On Mon, 2012-03-12 at 14:55 -0700, Mike Acar wrote:

> an implementation MAY abort with an error. Is that going to be
> addressed in -02?

I'm of the opinion that expressing a not-yet-existing target as a JSON
Pointer is the most simple and intuitive method so far. I've had no
problems implementing the specs as a result of this. I agree that the
language in the JSON Pointer spec is still somewhat awkward and could
still stand to be improved.

The suggested alternative of requiring the parent and target member to
be expressed separately is more verbose and complex. Over-verbosity of
JSON Patch is already an objection that has been raised more than once
on this list, and I'm not inclined to exacerbate it further unless there
is consensus from the members of appsawg that it should be adopted.

Paul

---

From: Julian Reschke <[email-masked]>
Date: Tue, 20 Mar 2012 15:37:26 +0100

On 2012-03-09 22:22, [email-masked] wrote:
> ...

I'd like to repeat my concern I posted on the -00 version:

> On 2012-01-05 02:42, "Martin J. Dürst" wrote:
>
>     ...
>
>         Appendix A:
>
>         The semantics of fragment identifiers depends on the media type. You
>         need to be clear whether you're trying to define the fragid semantics
>         for application/json (which as far as I recall doesn't define any), or
>         something else.
>
>     Yes. I'd propose that you just say you do, and then add a line at the
>     top saying that your RFC-to-be will update the JSON definition. You'll
>     also have to note that in an IANA section.
>
>> ...
>
> I have to say that I'm getting nervous about this.
>
>
> 1) When this WG adopted this as a work item, was it clear that JSON pointer isn't "a pointer syntax" but "*the* pointer syntax"?
>
> 2) Also, declaring this the one and only fragment identifier syntax for application/json means that we're closing the door for any future syntax that offers more expressiveness.
>
> In XML, people tried to handle this problem with a whole framework (XPointer), which doesn't seem to work too well.
>
> Maybe in this case the solution would be to reserve more delimiter characters that we'll likely need in the future (such as "[", "]", "(", ")", double quotes, single quotes etc).
>
> Best regards, Julian

(see 
<http://www.ietf.org/mail-archive/web/apps-discuss/current/msg04035.html>)

Best regards, Julian

---

From: "Paul C. Bryan" <[email-masked]>
Date: Tue, 20 Mar 2012 09:32:26 -0700

I happen to think JSON Pointer should be the pointer syntax for fragment
identifiers used by JSON Patch, not application/json per se. I don't
currently see why one would need to amend the JSON specification to
support this; any JSON Patch implementation should be fully capable of
resolving fragment identifiers itself.

Paul

---

From: Julian Reschke <[email-masked]>
Date: Tue, 20 Mar 2012 17:42:38 +0100

> resolving fragment identifiers itself.

+1 on that, but in that case Section 6 and the examples in appendix A 
(using pointers in fragment identifiers) should be removed.

We can't say that we aren't defining the fragment syntax for 
application/json, but then have examples doing the opposite.

Best regards, Julian

---

From: "Paul C. Bryan" <[email-masked]>
Date: Tue, 20 Mar 2012 09:54:42 -0700

Why remove Section 6? Where are fragment identifiers being used in
Appendix A?

Paul

---

From: Julian Reschke <[email-masked]>
Date: Tue, 20 Mar 2012 18:26:15 +0100

> Why remove Section 6? Where are fragment identifiers being used in

Section 6 says:

6. URI Fragment Identifier Representation

    A JSON Pointer MAY be represented in a URI fragment identifier.  The
    JSON pointer MUST be UTF-8 [RFC3629] encoded as octets; octets not in
    the URI "unreserved" set SHOULD be percent-encoded, per [RFC3986],
    section 2.5.

What's the point of making this statement? What media type is this about?

Appendix A contains:

    http://example.com/example.json#/foo/inner%20object
       Resolves to the object value of the "inner object" member in the
       "foo" object value in the root object.

    http://example.com/example.json#/foo/inner%20object/baz
       Resolves to the string value "qux", which is the value of the
       "baz" member in the "inner object" member in the "foo" member in
       the root object.

    http://example.com/example.json#/foo/bar/0
       Resolves to the string value "element0", which is the first value
       in the "bar" array in the "foo" member in the root object.

...which to me reads like using JSON pointers as fragment identifiers 
for payloads of type application/json. If that's not the case, it should 
be clarified somehow.

Going back two mails you said:

> I happen to think JSON Pointer should be the pointer syntax for fragment identifiers used by JSON Patch, not application/json per se.

In that case however the JSON Patch specification should state in the 
media type registration what the fragment syntax is (see 
<http://trac.tools.ietf.org/html/draft-ietf-appsawg-media-type-regs-03#appendix-C> 
fourth item), and the JSON Pointer spec should clarify that both Section 
6 and the examples in the appendix are about media types that have 
chosen to use JSON pointer as fragment identifier, which is not the case 
for application/json.

Right now it's way too easy for people to jump to the conclusion that 
JSON Pointer *is* the fragment identifier syntax for application/json.

Best regards, Julian

---

From: "Paul C. Bryan" <[email-masked]>
Date: Tue, 20 Mar 2012 11:44:34 -0700

> What's the point of making this statement? What media type is this

Okay, sorry, didn't know you were referring to the JSON Pointer spec
when referencing these sections.

The intent of JSON Pointer is to be expressed in a URI fragment
identifier for resources that have JSON representations. To that end, it
makes sense to establish the rules for encoding pointers in URIs.

> > I happen to think JSON Pointer should be the pointer syntax for
> fragment identifiers used by JSON Patch, not application/json per se.

I'll now retract this statement. I don't think fragments should have
anything to do with JSON Patch, and there's nothing in the JSON Patch
spec where fragments are in use.

> chosen to use JSON pointer as fragment identifier, which is not the

I don't think it makes a lot of sense for JSON Pointer spec to reference
every spec that happens to reference it.

> JSON Pointer *is* the fragment identifier syntax for application/json.

Simply based on the JSON Pointer spec text itself?

Paul

---

From: Julian Reschke <[email-masked]>
Date: Tue, 20 Mar 2012 19:55:17 +0100

> makes sense to establish the rules for encoding pointers in URIs.

So is the intent to define the fragment identifier syntax for 
application/json or not?

If it is, we need to normatively update the JSON RFC, and need to make 
sure there's really consensus on this.

If it is not, the way the Pointer spec currently is written is very 
confusing.

> every spec that happens to reference it.

What needs to be clear is to what media types the fragment syntax 
applies. All JSON-shaped? application/json? Future json-types, on an 
opt-in basis?

> Simply based on the JSON Pointer spec text itself?

Yes.

Best regards, Julian

---

From: "Paul C. Bryan" <[email-masked]>
Date: Tue, 20 Mar 2012 14:01:34 -0700

> So is the intent to define the fragment identifier syntax for 

It wasn't my intent originally, though your concerns have me think that
perhaps we should consider it. It was to define a fragment identifier
syntax, which other specifications (e.g. JSON Schema, JSON Reference)
can reference.

> sure there's really consensus on this.

Question: Do you think it should?

> If it is not, the way the Pointer spec currently is written is very 

Any text suggestions to resolve this?

Paul

---

From: Julian Reschke <[email-masked]>
Date: Tue, 20 Mar 2012 22:13:28 +0100

> Question: Do you think it should?

I'm not sure.

> Any text suggestions to resolve this?

Section 6 should clarify that the fragment identifier syntax applies to 
Internet Media Types that explicitly choose it, and that 
application/json hasn't (yet).

The examples in the appendix should get an explanation that they apply 
to a JSON-shaped example media type that indeed has choosen the fragid 
syntax...

Best regards, Julian

---

From: Mike Acar <[email-masked]>
Date: Tue, 20 Mar 2012 14:15:28 -0700

On 03/20/2012 09:32 AM, Paul C. Bryan wrote:

> any JSON Patch implementation should be fully capable of resolving
> fragment identifiers itself.

And it will have to, given that the Patch spec depends on undefined 
behavior in Pointer.

No. I haven't given that issue up yet :) I'll respond to the other 
thread a bit later today.

---

From: "Murray S. Kucherawy" <[email-masked]>
Date: Tue, 20 Mar 2012 21:40:13 +0000

> From: [email-masked] [mailto:[email-masked]] On Behalf Of Julian Reschke
> Sent: Tuesday, March 20, 2012 2:13 PM
> To: Paul C. Bryan
> Cc: [email-masked]
> Subject: Re: [apps-discuss] feedback on draft-ietf-appsawg-json-patch-01.txt
> 
> > It wasn't my intent originally, though your concerns have me think
> > that perhaps we should consider it. It was to define a fragment
> > identifier syntax, which other specifications (e.g. JSON Schema, JSON
> > Reference) can reference.
> >
> >> If it is, we need to normatively update the JSON RFC, and need to
> >> make sure there's really consensus on this.
> >

Does there need to be one-and-only-one?  Or I guess more accurately, do we need to say this is the one and only way to do this?  Seems like that closes the door to someone coming up with a better idea.

It seems to me that you can just say that this is a way to do pointers to objects, and that patch is built on top of it, and stop there.

---

From: Julian Reschke <[email-masked]>
Date: Tue, 20 Mar 2012 23:02:54 +0100

> Does there need to be one-and-only-one?  Or I guess more accurately, do we need to say this is the one and only way to do this?  Seems like that closes the door to someone coming up with a better i…

There could be multiple fragment identifier schemes, but then you'd need 
a way to disambiguate (for instance, xml:id doesn't allow things like 
"()", so there's an extension point to hook XMLPointer to).

> It seems to me that you can just say that this is a way to do pointers to objects, and that patch is built on top of it, and stop there.

Well, the patch format doesn't need fragment identifiers to start with...

---

From: Mike Acar <[email-masked]>
Date: Tue, 20 Mar 2012 16:19:28 -0700

On 03/13/2012 08:31 AM, Paul C. Bryan wrote:
> Pointer is the most simple and intuitive method so far.

If the pointer is to the parent, and there is a "name" element, the code 
will look something like (perlish syntax ahead):

# We already figured out we're adding an element
my $new_name = $patch->[0]->{name};
my $new_val = $patch->[0]->{value};

# Resolve the pointer.
my $parent = follow_pointer($patch->[0]->{add});

# "Add" logic:
if (exists($parent->{$new_name}) {
	die "$new_name already exists in $parent!";
}
$parent->{$new_name} = $new_val;

Get a reference to the parent and manipulate it. Nice and simple.

Compare if the location is to the new name to be added:

# We already figured out we're adding an element
my $new_val = $patch->[0]->{val};
my $pointer = $patch->[0]->{add};

# Resolve the pointer.
my @pointer_elements = split_pointer($pointer);
my $new_name = pop(@pointer_elements);
my $parent_pointer = join_pointer(@pointer_elements)
my $parent = follow_pointer($parent_pointer);

# "Add" logic:
if (exists($parent->{$new_name}) {
	die "$new_name already exists in $parent!";
}
$parent->{$new_name} = $new_val;

The first looks simpler to me.

> I've had no problems implementing the specs as a result of this.

 > I agree that the

I think that no matter how it's expressed, this is a flaw in the 
relationship between these specs.

The goal of specification is to improve interoperability. As these 
drafts stand, you can write a conforming Pointer implementation that is 
useless for Patch.

> be expressed separately is more verbose

It is, but I think the extra verbosity can be made very very small (a 
few bytes at most). Balancing that against conforming but 
not-interoperable implementations.... I think verbosity is better.

> and complex.

I disagree here, too. I think pointer-to-parent/name-to-add is simpler 
to specify, to understand, and to use in implementation.

> Over-verbosity of JSON Patch is already an objection that has been
> raised more than once on this list, and I'm not inclined to
> exacerbate it further unless there is consensus from the members of
> appsawg that it should be adopted.

Even very early design discussions noted this problem; see Dean 
Landolt's comments from Apr 27 2011:

     I'm wondering if it makes more sense to use a path for add similar
     to remove -- the last path portion would be the "element". One
     drawback is that the actual submitted pointer path wouldn't point to
     a valid node until after the patch is applied (which, I assume, is
     why you used the "element" approach).

http://groups.google.com/group/json-patch/browse_thread/thread/6e1cc65501eda324#

Your original design was

     3. You're right, the reason I kept "element" was so that the pointer
     always resolves to a node. This idea is getting pushback by those
     who want ultra-tight syntax; any further feedback on this point
     would be appreciated.

Consider this (belated) feedback in favor of pointers always referring 
to existing nodes :)

Dean went on to write

     Given well-defined path semantics this shouldn't be as important --
     we should be able to count on an implementation correctly splitting
     a path into parent and element targets.

I'm not sure why that assumption would be true; somebody reading just 
the Pointer spec wouldn't realize this need, which Patch imposes.

---

From: "Paul C. Bryan" <[email-masked]>
Date: Tue, 20 Mar 2012 22:35:16 -0700

As I stated previously, if there's consensus on a more verbose patch
operation object format, I will defer. Any opinions from other members
of the working group?

Paul

---

From: "Murray S. Kucherawy" <[email-masked]>
Date: Wed, 21 Mar 2012 06:07:50 +0000

I’m inclined to agree that something that’s more likely to produce interoperable implementations at the cost of slightly more bytes on the wire is a desirable tradeoff.

-MSK

From: [email-masked] [mailto:[email-masked]] On Behalf Of Paul C. Bryan
Sent: Tuesday, March 20, 2012 10:35 PM
To: [email-masked]
Subject: Re: [apps-discuss] I-D Action: draft-ietf-appsawg-json-patch-01.txt

As I stated previously, if there's consensus on a more verbose patch operation object format, I will defer. Any opinions from other members of the working group?

Paul

# Resolve the pointer.

# "Add" logic:

}

# Resolve the pointer.

# "Add" logic:

}

useless for Patch.

http://groups.google.com/group/json-patch/browse_thread/thread/6e1cc65501eda324#<http://groups.google.com/group/json-patch/browse_thread/thread/6e1cc65501eda324>

Your original design was

     would be appreciated.

to existing nodes :)

Dean went on to write

---

From: Julian Reschke <[email-masked]>
Date: Wed, 21 Mar 2012 07:45:43 +0100

> operation object format, I will defer. Any opinions from other members

Please leave it the way it is.

Best regards, Julian

---

From: "Martin J. Dürst" <[email-masked]>
Date: Wed, 21 Mar 2012 15:55:04 +0900

On 2012/03/21 6:40, Murray S. Kucherawy wrote:

> Does there need to be one-and-only-one?

In theory, not necessarily. In practice, not having a lot of variation 
usually helps.

> Or I guess more accurately, do we need to say this is the one and only way to do this?  Seems like that closes the door to someone coming up with a better idea.

Fragment pointers into datastructures such as JSON isn't exactly a brand 
new field with lots of new possibilities. And XPointer has shown that 
making it too complicated cuts out most implementations. My proposal 
would be to somehow include an extensibility hatch, but then say this is 
the one and only one.

Regards,    Martin.

---

From: Julian Reschke <[email-masked]>
Date: Wed, 21 Mar 2012 10:11:37 +0100

On 2012-03-21 07:55, "Martin J. Dürst" wrote:
>> Or I guess more accurately, do we need to say this is the one and only
>> way to do this? Seems like that closes the door to someone coming up
>> with a better idea.
>

In XML, as far as I understand, the escape hatch is that identifiers do 
not allow brackets, and thus these are free for defining new addressing 
schemes. So in JSON Pointer, we'd need to escape a few more characters 
when using in fragment identifiers.

Also, it seems this needs to be coordinated with a potential spec 
defining a "+json" media type suffix...

Best regards, Julian

---

From: Julian Reschke <[email-masked]>
Date: Wed, 21 Mar 2012 13:59:57 +0100

> Please leave it the way it is.

...by which I mean the syntax. I *do* agree that we need to tune the 
language so it defines what needs to happen when the pointer identifies 
a non-existing piece of the referenced document (but we can do this by 
tuning the language in the Patch spec).

Best regards, Julian