Subject: Re: [apps-discuss] JSON patch: "test" operation

From: Julian Reschke <[email-masked]>
Date: Thu, 01 Dec 2011 10:10:49 +0100

On 2011-12-01 08:17, TianLinyi wrote:
> Hi, All
>
> I think test would make it complicated. The test could result in <, >,
> <=, >=, = conditions.

"=", not the others. (Which, yes, maybe trick for complex objects)

> If we want something like HTTP If-Match mechanism, I would think it
> should not be in the JSON patch. We should keep JSON patch simple. The
> status code is better to be delivered in the protocol layer who delivers
> the JSON patch document.

It's not a status code, it's context information.

How do you deliver it when patching a JSON document in the local file 
system?

Best regards, Julian

---

From: Carsten Bormann <[email-masked]>
Date: Thu, 1 Dec 2011 13:26:26 +0100

What happens when a test fails?
-- Keep or rewind the changes done so far?
	(or do we stipulate "test" has to come before modifiers?)
-- What is the response code you want to see?

Grüße, Carsten

---

From: Julian Reschke <[email-masked]>
Date: Thu, 01 Dec 2011 13:37:39 +0100

> 	(or do we stipulate "test" has to come before modifiers?)

I think requiring them to come first makes a lot of sense.

> -- What is the response code you want to see?

409 comes to mind.

Best regards, Julian

---

From: Carsten Bormann <[email-masked]>
Date: Thu, 1 Dec 2011 13:47:02 +0100

> I think requiring them to come first makes a lot of sense.

I think so, too.
(Still, something should be said about what is supposed to happen if they don't -- is that a MUST detect then?)

My comment ("what happens") can be generalized to the entire section 5 of draft-pbryan-json-patch-02.txt…  "Fails to complete" doesn't cut it, I think.

>> -- What is the response code you want to see?

Sounds good.  I think that the media type spec should contain text suggesting a specific response code, to rein in the otherwise uncontrollable inventiveness of the implementers.  Could go in section 5, too.

Grüße, Carsten

---

From: Julian Reschke <[email-masked]>
Date: Thu, 01 Dec 2011 14:01:38 +0100

> (Still, something should be said about what is supposed to happen if they don't -- is that a MUST detect then?)

I would support that (MUST occur first).

> Sounds good.  I think that the media type spec should contain text suggesting a specific response code, to rein in the otherwise uncontrollable inventiveness of the implementers.  Could go in secti…

Not sure. I think 
<http://greenbytes.de/tech/webdav/rfc5789.html#rfc.section.2.2> is quite 
clear on that.

Best regards, Julian

---

From: "Paul C. Bryan" <[email-masked]>
Date: Thu, 01 Dec 2011 09:55:19 -0800

I can project cases where multiple operations are performed, and you
can't determine the state until an operation is completed before being
in a position to test for the next operation. Forcing tests to be up
front will needlessly complicate diff implementations, will not reduce
any complexity on patch implementations, for no good reason as far as I
can tell

Paul 

On Thu, 2011-12-01 at 14:01 +0100, Julian Reschke wrote:

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

---

From: Mark Nottingham <[email-masked]>
Date: Fri, 2 Dec 2011 08:51:33 +1100

I thought that at first too, but upon reflection I don't see how they're different from other directives -- other ones can fail too. 

--
Mark Nottingham   http://www.mnot.net/

---

From: Julian Reschke <[email-masked]>
Date: Thu, 01 Dec 2011 22:59:30 +0100

On 2011-12-01 22:51, Mark Nottingham wrote:
> ...

So whether the patch operation is atomic or not is implementation dependent?

Or are we saying the server needs to be able to deal with the operation 
being aborted in process, and roll back?

(just want to see that clarified)

Best regards, Julian

---

From: Mark Nottingham <[email-masked]>
Date: Fri, 2 Dec 2011 09:12:22 +1100

On 02/12/2011, at 8:59 AM, Julian Reschke wrote:

> Or are we saying the server needs to be able to deal with the operation being aborted in process, and roll back?
> 

RFC5789, section 2:

>    The server MUST apply the entire set of changes atomically and never
>    provide (e.g., in response to a GET during this operation) a
>    partially modified representation.  If the entire patch document
>    cannot be successfully applied, then the server MUST NOT apply any of
>    the changes.  The determination of what constitutes a successful
>    PATCH can vary depending on the patch document and the type of
>    resource(s) being modified.  For example, the common 'diff' utility
>    can generate a patch document that applies to multiple files in a
>    directory hierarchy.  The atomicity requirement holds for all
>    directly affected files.

---

From: "Paul C. Bryan" <[email-masked]>
Date: Thu, 01 Dec 2011 14:45:21 -0800

Indeed, RFC 5789 should make it unambiguous for HTTP PATCH. For other
uses of JSON Patch, it would be implementation-specific. This is why the
"fails to complete" verbiage is there...

Paul

---

From: Julian Reschke <[email-masked]>
Date: Thu, 29 Dec 2011 16:40:56 +0100

Hi there,

in discussions in Apache Jackrabbit space, two more features have been 
mentioned as potentially useful:

1) The ability to send additional data along with the actual patch; such 
as a plain text string describing the change (think "commit" message), 
or user information.

2) The ability to *copy* (not *move*) objects around.

...feedback appreciated,

Julian

---

From: Sam Johnston <[email-masked]>
Date: Thu, 29 Dec 2011 18:48:28 +0100

Julian,

1) The "commit" message sounds like a job for a header, but rather than
define one I'd suggest using a generic one for all such applications — I've
been using an "Attribute:" header for my semantic HTTP work (e.g.
Attribute: title="Picture of man biting dog").

2) For MOVE & COPY I've been using WebDAV methods (sans XML).

Thoughts?

Sam

On Thu, Dec 29, 2011 at 4:40 PM, Julian Reschke <[email-masked]>wrote:

> as a plain text string describing the change (think "commit" message), or
> user information.
>
> ______________________________**_________________
> https://www.ietf.org/mailman/**listinfo/apps-discuss<https://www.ietf.org/mailman/listinfo/apps-discuss>
>

---

From: Julian Reschke <[email-masked]>
Date: Thu, 29 Dec 2011 22:15:30 +0100

On 2011-12-29 18:48, Sam Johnston wrote:
> define one I'd suggest using a generic one for all such applications —
> I've been using an "Attribute:" header for my semantic HTTP work (e.g.

This might work; but of course that needs to be defined as well...

> 2) For MOVE & COPY I've been using WebDAV methods (sans XML).

Understood. But may not be the right thing for everybody...

---

From: Nico Williams <[email-masked]>
Date: Thu, 29 Dec 2011 15:31:19 -0600

On Thu, Dec 29, 2011 at 11:48 AM, Sam Johnston <[email-masked]> wrote:
> been using an "Attribute:" header for my semantic HTTP work (e.g. Attribute:
> title="Picture of man biting dog").

You'll want some well-defined headers for "commit" metadata (e.g., to
list merge parents).

Nico
--

---

From: "Paul C. Bryan" <[email-masked]>
Date: Thu, 29 Dec 2011 21:24:48 -0800

> as a plain text string describing the change (think "commit"

I've had a few thoughts here.

First, I think commit messages should probably be out of scope for JSON
Patch specifically. That said, we should allow the ability to build on
the specified format, allowing for extensions, including additional
(more domain-specific) operations.

I'm generally against the idea of using out-of-band metadata such as
header fields in an HTTP request, mostly because it ties metadata to the
transfer protocol rather (what I believe rightly should be tied to the
document).

I'm interested in feedback on the possibility that I add text stating
that if an operation cannot be determined for a given patch object, then
an implementation should ignore it. Thoughts?

> 2) The ability to *copy* (not *move*) objects around.

This is another example of something trivial for the patch
implementation to perform, so I'm inclined to add it to the next draft.

Paul

---

From: Sam Johnston <[email-masked]>
Date: Fri, 30 Dec 2011 11:09:15 +0100

On Fri, Dec 30, 2011 at 6:24 AM, Paul C. Bryan <[email-masked]>wrote:

> **
> Patch specifically. That said, we should allow the ability to build on the
> specified format, allowing for extensions, including additional (more
> domain-specific) operations.
>

Maybe so, but saying what you're doing when manipulating an object is often
a good idea.

> transfer protocol rather (what I believe rightly should be tied to the

The distinction between data and metadata isn't always clear — to me a
document detailing manipulations could just as easily be separated from the
description of it (or it could be redundant — with the headers reflecting
data contained within the document). It's also not always possible to
define what goes in a document, so if you have metadata (like commit
messages) then I'd argue you should use HTTP's existing metadata channel
rather than an "envelope" like SOAP (or JSON).

> I'm interested in feedback on the possibility that I add text stating that
> if an operation cannot be determined for a given patch object, then an
> implementation should ignore it. Thoughts?
>

To me this sounds dangerous — add versioning and if there's sufficient
demand for extensions then just include them in a future version?

> This is another example of something trivial for the patch implementation
> to perform, so I'm inclined to add it to the next draft.
>

This begs the question as to whether PATCH should be creating new
resources; the scope of PATCH ("to modify an existing HTTP resource") is
fairly clear on this IMO. Furthermore, MOVE and COPY verbs are useful for
other applications (e.g. cloud APIs) so promoting them from WebDAV, sans
XML dependency, would likely be useful for other applications.

Sam

---

From: Julian Reschke <[email-masked]>
Date: Fri, 30 Dec 2011 11:41:15 +0100

On 2011-12-30 11:09, Sam Johnston wrote:
> On Fri, Dec 30, 2011 at 6:24 AM, Paul C. Bryan <[email-masked]
> <mailto:[email-masked]>> wrote:
>
>
>     First, I think commit messages should probably be out of scope for
>     JSON Patch specifically. That said, we should allow the ability to
>     build on the specified format, allowing for extensions, including
>     additional (more domain-specific) operations.
>
>
> Maybe so, but saying what you're doing when manipulating an object is
> often a good idea.
>
>     header fields in an HTTP request, mostly because it ties metadata to
>     the transfer protocol rather (what I believe rightly should be tied
>     to the document).
>
>
> document detailing manipulations could just as easily be separated from
> the description of it (or it could be redundant — with the headers
> reflecting data contained within the document). It's also not always
> possible to define what goes in a document, so if you have metadata
> (like commit messages) then I'd argue you should use HTTP's existing
> metadata channel rather than an "envelope" like SOAP (or JSON).

I think both are valid points of view :-)

As a data point: when I submit a hg patch to Mozilla, I specify the 
commit message in-band. I think that's a very useful thing to have.

>     I'm interested in feedback on the possibility that I add text
>     stating that if an operation cannot be determined for a given patch
>     object, then an implementation should ignore it. Thoughts?
>
>

So yes, we need to state whether the format follows the 
"must-understand" or "must-ignore" pattern. Or allow optional parameters 
(such as a commit message) to be marked as such.

> resources; the scope of PATCH ("to modify an existing HTTP resource") is

Of course that depends on how you define the lifetime of a resource. 
Does a resource which currently doesn't have a retrievable 
representation "exist"?

Technically, PATCH can work for creating an initial mapping as well.

> fairly clear on this IMO. Furthermore, MOVE and COPY verbs are useful
> for other applications (e.g. cloud APIs) so promoting them from WebDAV,
> sans XML dependency, would likely be useful for other applications.

I agree that COPY and MOVE are good things if you indeed want to COPY or 
MOVE full resources (or hierarchies). I never understood the resistance 
to COPY and MOVE in the AtomPub community.

That being said: the proposal is for copying JSON object members, not 
HTTP resources; so this is an operation that can't be done with COPY 
unless you have HTTP mappings for each of the JSON object members (and 
even then you wouldn't be able to express them as part of a bigger PATCH 
operation).

Best regards, Julian

---

From: Sam Johnston <[email-masked]>
Date: Fri, 30 Dec 2011 12:36:20 +0100

On Fri, Dec 30, 2011 at 11:41 AM, Julian Reschke <[email-masked]>wrote:

> As a data point: when I submit a hg patch to Mozilla, I specify the commit
> message in-band. I think that's a very useful thing to have.

For a patch format, agreed — so include it (as optional). In saying "it's
also not always
possible to define what goes in a document" I mean that as a user you may
have needs not considered by the document format authors. For example, you
may want to include a link to an issue tracking system — do you extend the
format or use a Link: header? I know what I'd do.

Another more generic example I like to use is a photo hosting site — while
photo formats (e.g. JPEG) now carry metadata channels (e.g. EXIF) I'd
rather be able to glean semantic information (e.g. links, attributes,
categories) from the headers than having to retrieve and parse the resource
for an embedded resource that itself has to be parsed!

Anyway this is getting off-topic — in this case I think there's
justification for native in-band support, possibly with link references too.

> So yes, we need to state whether the format follows the "must-understand"
> or "must-ignore" pattern. Or allow optional parameters (such as a commit
> message) to be marked as such.

IMO a patch format (c.f. diff) should be "must-understand", even if some
parameters are made optional.

> Of course that depends on how you define the lifetime of a resource. Does
> a resource which currently doesn't have a retrievable representation
> "exist"?
>

That's a contrived interpretation — is it something we want to support? I'd
rather a PATCH throw a 4xx error if a resource didn't exist... an empty PUT
would create a resource that could then be PATCHed, no?

> MOVE full resources (or hierarchies). I never understood the resistance to
> COPY and MOVE in the AtomPub community.
>
> That being said: the proposal is for copying JSON object members, not HTTP
> resources; so this is an operation that can't be done with COPY unless you
> have HTTP mappings for each of the JSON object members (and even then you
> wouldn't be able to express them as part of a bigger PATCH operation).
>

Ah, that makes a lot more sense now, though it does sound like a premature
optimisation.

Sam

---

From: Julian Reschke <[email-masked]>
Date: Fri, 30 Dec 2011 13:12:47 +0100

On 2011-12-30 12:36, Sam Johnston wrote:
> That's a contrived interpretation — is it something we want to support?
> I'd rather a PATCH throw a 4xx error if a resource didn't exist... an
> empty PUT would create a resource that could then be PATCHed, no?
> ...

It probably belongs into the description of the patch format.

I don't see how requiring an empty PUT makes things better, though.

Best regards, Julian

---

From: Sam Johnston <[email-masked]>
Date: Fri, 30 Dec 2011 15:05:39 +0100

> It probably belongs into the description of the patch format.

Agreed.

>  I don't see how requiring an empty PUT makes things better, though.

It would prevent misplaced PATCHes from creating new resources, while still
allowing you to build up a resource from scratch with PATCHes. Maybe I'm
being pedantic, but I thought that's what standards groups were for ;)

Sam

---

From: Julian Reschke <[email-masked]>
Date: Fri, 30 Dec 2011 15:14:55 +0100

On 2011-12-30 15:05, Sam Johnston wrote:
> On Fri, Dec 30, 2011 at 1:12 PM, Julian Reschke <[email-masked]
> <mailto:[email-masked]>> wrote:
>
> It would prevent misplaced PATCHes from creating new resources, while

If you want to do that, you can always add "If-None-Match: *".

> still allowing you to build up a resource from scratch with PATCHes.
> Maybe I'm being pedantic, but I thought that's what standards groups
> were for ;)

:-) As long as being too pedantic doesn't make things more complicated 
than they need to be (I can't recall why we have the prose about 
"existing" resources in the PATCH spec; we definitively discussed this 
use case at some point...)

Best regards, Julian