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

[Mike Acar <[e-mail]>]

On 03/13/2012 08:31 AM, Paul C. Bryan wrote:
> On Mon, 2012-03-12 at 14:55 -0700, Mike Acar wrote:
>
>> 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?
>
>
> 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.

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
> language in the JSON Pointer spec is still somewhat awkward and could
> still stand to be improved.

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.

> The suggested alternative of requiring the parent and target member to
> 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.

-- 
Mike Acar -                                 - macar at cloudmark dot com