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

["Paul C. Bryan" <[e-mail]>]

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

On Tue, 2012-03-20 at 16:19 -0700, Mike Acar wrote:

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