Specify PATCH

[csarven]

No description provided.

[dmitrizagidulin]

Reminder to discuss: What to do about the MS-Author-Via: SPARQL header? (see PR nodeSolidServer/node-solid-server#1313 for example). (I bring it up here because it's PATCH-related.)
Does it need to be added to the spec? As a MUST or a MAY?

[kjetilk]

Just wanting to post this somewhere as a note to self, since we need to specify this behaviour, which is mostly correct:

        # HTTP/1.1 415 Unsupported Media Type
        # Connection: close
        # Date: Fri, 15 Nov 2019 00:12:28 GMT
        # ETag: W/"2b-rKTuKHccg1SjlLhItRjeKZPSFY4"
        # Vary: Accept, Authorization, Origin
        # Allow: OPTIONS, HEAD, GET, PATCH, POST, PUT, DELETE
        # Content-Length: 43
        # Content-Type: text/plain; charset=utf-8
        # Access-Control-Allow-Credentials: true
        # Access-Control-Expose-Headers: Authorization, User, Location, Link, Vary, Last-Modified, ETag, Accept-Patch, Accept-Post, Updates-Via, Allow, WAC-Allow, Content-Length, WWW-Authenticate, MS-Author-Via
        # Link: <alice_share_bob.txt.acl>; rel="acl", <alice_share_bob.txt.meta>; rel="describedBy", <http://www.w3.org/ns/ldp#Resource>; rel="type"
        # MS-Author-Via: SPARQL
        # Set-Cookie: connect.sid=[...] Path=/; Expires=Sat, 16 Nov 2019 00:12:28 GMT; HttpOnly; Secure
        # X-Powered-By: solid-server/5.2.1
        # 
        # Unsupported patch content type: text/plain
        # No expected headers set

[csarven]

@dmitrizagidulin IMO, we shouldn't bother to bring along a propriety header like MS-Author-Via. I don't quite see what it adds meanwhile Allow, Accept-*..

@kjetilk Anything in particular that's incorrect or inadequate? I'm assuming the key things in that example is the method and the content type.

[kjetilk]

@dmitrizagidulin IMO, we shouldn't bother to bring along a propriety header like MS-Author-Via. I don't quite see what it adds meanwhile Allow, Accept-*..

IIRC, @timbl was OK with MS-Author-Via, it is a de facto standard, and even though it hasn't (AFAIK) been through any open standardization process, there aren't any particular strings attached to it, so it can be used. The other headers don't point to SPARQL specifically.

@kjetilk Anything in particular that's incorrect or inadequate? I'm assuming the key things in that example is the method and the content type.

Basically, mainly that this is a LDP-NR, and it can't be edited with SPARQL, so it shouldn't be there in this case even if it is there for LDP-RS. Also, with ref to #118, the POST operation should possibly not be in the Allow header either.

But the observation that 415 should be used when you have no patch algo is a good one. Though for text/* it should be possible to use a diff -u patch, but AFAIK, there exists no authoratitive MIME type for that…

[csarven]

Isn't Allow: PATCH, Accept-Patch: application/sparql-update precise and sufficient?

Wouldn't updating text/* by any means simply left unspecified in the spec? I agree re 415.

[kjetilk]

Isn't Allow: PATCH, Accept-Patch: application/sparql-update precise and sufficient?

Actually... Yes! /me takes note to read RFC5789 in full... :-) Sorry.

Wouldn't updating text/* by any means simply left unspecified in the spec? I agree re 415.

Yes, it was the distinction between LDP-RS and LDP-NR that was the important part of my comment.

[csarven]

A bit more on the HTML and XML family: https://tools.ietf.org/html/rfc7351 and https://tools.ietf.org/html/rfc5261 is one way to have PATCH work for XML compatibles; XHTML and Polyglot markup. Unlikely to work well with HTML serialisations in general eg. those using optional tags. SVG and MathML should be fine. If a server supports XML Patch, application/xml-patch+xml will be listed in Accept-Patch.

Someone can correct me on this but AFAIK application/sparql-update is the only one we have implementation experience with, so that should probably be the only one appearing in the spec - with some requirement level.

I don't think the spec needs to bring any attention to LDP-NR or LDP-RS. Simply, if server allows certain methods cross content type for a resource, it can be known through Allow and Accept-*. That makes it possible for servers to advertise media types they support for PATCH - which is already an LDP requirement.

[kjetilk]

OK, so something around "If the server supports PATCH, it MUST advertize the media types it supports through the Accept-Patch header, and MUST respond with 415 if manipulation is attempted through other media types"? Or does this already follow from RFC5789?

[csarven]

If the server supports PATCH, it MUST advertize the media types it supports through the Accept-Patch header

Like I said, already in LDP: https://www.w3.org/TR/ldp/#ldpr-patch-acceptpatch

MUST respond with 415 if manipulation is attempted through other media types

I think that's just RFC 7231.

[kjetilk]

If the server supports PATCH, it MUST advertize the media types it supports through the Accept-Patch header

Like I said, already in LDP: https://www.w3.org/TR/ldp/#ldpr-patch-acceptpatch

Yeah, but... Perhaps I should defer commenting until I have caught up on RFC5789, but it seems terribly underspecified; It doesn't seem useful at all to know that a server supports a certain patch media type, for it to be useful, it needs to be known for each media type, or at the very least for each interaction model.

[kjetilk]

Caught up on RFC5789, and I think LDP is awkward at that point. I think it makes zero sense to add Accept-Patch to OPTIONS *, but LDP doesn't qualify that case. The RFC makes the intention much clearer than LDP does:

The presence of a specific patch document format in this header indicates that that specific format is allowed on the resource identified by the Request-URI.

I think we need to tighten this up a bit, but also make clear that you don't need Accept-Patch on OPTIONS *.

[csarven]

I don't quiet follow what you're saying or want. LDP inherits RFC 5789.

Why say the opposite with not needing Accept-Patch on OPTIONS? Wouldn't that conflict with LDP and RFC 5789?

Tighten up how for instance? Specify or clarify something (non-normative)? Change the level of requirement somewhere?

By the way, see also #43 .. basically no new semantics is needed for Accept-Post. Accept-Post and Accept-Patch should have a uniform behaviour.

[kjetilk]

I'll try to clarify.

The most important point being that the case of OPTIONS * has to be clarified, and LDP seems to have completely missed this point. According to RFC7231:

An OPTIONS request with an asterisk ("*") as the request-target (Section 5.3 of [RFC7230]) applies to the server in general rather than to a specific resource.

So, when LDP says:

LDP servers that support PATCH MUST include an Accept-Patch HTTP response header [RFC5789] on HTTP OPTIONS requests, listing patch document media type(s) supported by the server.

The implication of an unqualified "supported by the server" is that it applies to OPTIONS *, but IMHO, that's a misunderstanding of Accept-Patch and OPTIONS *. It has zero value (or I'd like to see concrete use cases for it), and it does not conform to the spirit of Accept-Patch. And it leaves too much up to interpretation when it comes to resources.

So, no we shouldn't break RFC5789 (that one is in line with my intuition), but break LDP because LDP is being silly.

Moreover, I think we should respond with Accept-Patch to any request for a resource that could be patched, to avoid round-trips.

So, I'd rather prefer we adopt RFC5789's language, something like

"Accept-Patch MUST appear in the response for any resource that supports the use of the PATCH method."

Alternatively, we could do it just for read requests and 415 responses, as I suppose that if you've just done a successful PUT, that you could have done a PATCH too is of no particular interest :-)

Given LDPs silliness, we might relax the requirement somewhat from LDP, e.g.

"Accept-Patch SHOULD NOT appear in the response for OPTIONS *".

This would allow implementors to stay conformant with LDP if they take it literally and has to, but allow Solid implementors to not spend any time on that. (I would have said "MAY NOT", but that's not in RFC2119).

[csarven]

re 'unqualified "supported by the server"', I think I see what you mean but I'm not sure if there is any severe violation. I agree that one way of interpreting the LDP text is that it missed OPTIONS * but another way is that it was only talking about supported media types for specific resources (as opposed to server wide).

For the solid spec, I would suggest extending @TallTed 's proposed:

https://github.com/solid/solid-spec/pull/103/files#diff-bc6e7d60c7ea3d7165eb78a87a94b626R368-R371

with the information he also gave in nodeSolidServer/node-solid-server#628 (comment) . That seems to cover what you're saying, right?

Moreover, I think we should respond with Accept-Patch to any request for a resource that could be patched, to avoid round-trips.

There are probably similar cases. So, raised #123 .

Alternatively, we could do it just for read requests and 415 responses, as I suppose that if you've just done a successful PUT, that you could have done a PATCH too is of no particular interest :-)

Which is a good enough reason to leave it optional.

"Accept-Patch SHOULD NOT appear in the response for OPTIONS *".

I don't think that's needed. The context is * (server's capability) any way. It just means that any one of the advertised media types could be used but it doesn't suggest or commit to all of them being available for all resources that can be PATCHed.

[kjetilk]

Not sure it addresses all my concerns, we need to work out the exact wording.

There are two important concerns that I'm not confident about: One is that we should make sure the client doesn't need more roundtrips than necessary, and so MUST Accept-Patch is important to have where appropriate. For read requests, it is appropriate, I think if the client as allowed to patch the resource.

OTOH, we certainly don't want to burden implementors with implementing useless stuff. So, for

OPTIONS *

they shouldn't need to respond:

Accept-Patch: application/sparql-update, application/xml-patch+xml, text/x-patch

because that's of very little use. In my reading, LDP's MUST requires them to do that, and we should make sure Solid implementors can safely ignore that and remain Solid compliant (but also, allow LDP implementors who do that be Solid compliant). I know that I would totally ignore that MUST if I wrote a Solid server. :-)

[csarven]

First of all, I don't see LDP's MUST as an issue as you do. It has utility. How is the MUST anymore "useless" than the SHOULD that the RFCs prescribe? I find OPTIONS * listing all potentially available media types in Accept-Patch to be a simple implementation. Certainly a server has an idea about what it is capable of. Am I missing something? Why should any server be prohibited from even advertising that? Is there a security or privacy issue? If so, that'd be strong grounds to recommend against it.

Having said that, I haven't made use OPTIONS * in the past and you may not want to but.. that's only anecdotal evidence based on a small sample :)

[kjetilk]

The MUST makes it mandatory, the SHOULD doesn't, which makes it a compliance issue. Simple, it might be, but it is still an issue. There is still actual time that has be allocated. I am not advocating a MUST NOT, I merely want language to make it so that Solid implementors wouldn't have to to do it to be compliant.

The burden of proof should be on those advocating a MUST, so why do you think MUST is important?

Say, a client wants to patch /foo.ttl. With this MUST, it might go

OPTIONS *

find

Accept-Patch: application/sparql-update, application/xml-patch+xml, text/x-patch

realize it has to then figure out from heuristics which one applies to /foo.ttl, or just try out what might work, possibly getting a 415, or just go GET, HEAD or OPTIONS on /foo.ttl, which it probably has to anyway, since it probably needs the ETag anyway.

Or it would go HEAD /foo.ttl right away, get everything it needs from there.

I would really like to see a justification for MUST in this case, why is it worth the effort. I just can't see any use for it. MUST on Accept-Patch on individual resources, yes, but not on OPTIONS *.

[csarven]

The burden of proof should be on those advocating a MUST, so why do you think MUST is important?

I'll defer to LDP. I think the point is if Solid is generally inheriting LDP but with extensions and changes, we need to provide a reason for the changes which may be in conflict.

What we could perhaps do re https://www.w3.org/TR/ldp/#ldpr-patch-acceptpatch is i) be certain about LDP's intention, or ii) clarify whether it is for all OPTIONS request targets (* and specific target resources) or only for specific targets. If there is any interpretation in there that suggests that LDP's wording is for specific targets, then we have a simpler path to not saying anything and so effectively, OPTIONS * with Accept-Patch is a SHOULD - which is I think the minimum criteria from RFCs - or perhaps just note as an exception. [My interpretation is that LDP only meant it for specific targets.]

Say, a client wants to patch /foo.ttl. With this MUST, it might go
OPTIONS *

Why? A conforming client would know that request ( https://tools.ietf.org/html/rfc7230#section-5.3.4 ) is for server's capabilities as opposed to /foo.ttl. If it wants to know how it can PATCH /foo.ttl, it will simply do OPTIONS /foo.ttl (or more like OPTIONS /foo but that's beside the point here).

[kjetilk]

The burden of proof should be on those advocating a MUST, so why do you think MUST is important?

I'll defer to LDP. I think the point is if Solid is generally inheriting LDP but with extensions and changes, we need to provide a reason for the changes which may be in conflict.

I have a different philosophy: We are making something that should be without obvious overhead, and so, we should question everything that adds overhead without actual value.

What we could perhaps do re https://www.w3.org/TR/ldp/#ldpr-patch-acceptpatch is i) be certain about LDP's intention, or ii) clarify whether it is for all OPTIONS request targets (* and specific target resources) or only for specific targets. If there is any interpretation in there that suggests that LDP's wording is for specific targets, then we have a simpler path to not saying anything and so effectively, OPTIONS * with Accept-Patch is a SHOULD - which is I think the minimum criteria from RFCs - or perhaps just note as an exception. [My interpretation is that LDP only meant it for specific targets.]

If LDP is sufficiently precise, we wouldn't need to put down extra effort, and if LDP is imprecise, like in this case, I suggest that we instead look to use cases.

So, I suppose LDP meant for specific targets, but that it had insufficient understanding of the OPTIONS method, and therefore, it ended up being a problem towards implementators. Thus, we shouldn't defer to LDP when they do stuff like that, implementors need to be assured that they can safely ignore the problem.

Say, a client wants to patch /foo.ttl. With this MUST, it might go
OPTIONS *

Why? A conforming client would know that request ( https://tools.ietf.org/html/rfc7230#section-5.3.4 ) is for server's capabilities as opposed to /foo.ttl. If it wants to know how it can PATCH /foo.ttl, it will simply do OPTIONS /foo.ttl (or more like OPTIONS /foo but that's beside the point here).

Yes, and in which case Accept-Patch in a OPTIONS * is completely redundant, which is exactly my point. We shouldn't spend time on interpreting bad specs, we should instead spend time on writing good ones.

[csarven]

[
This part of the discussion is about general design considerations. I think we should move this thread elsewhere.

I have a different philosophy: We are making something that should be without obvious overhead, and so, we should question everything that adds overhead without actual value.

No objection from my end but I don't believe that was the approach we took or agreed on. If you want to revisit that, we can, perhaps in chat or at the next F2F?
]

Yes, and in which case Accept-Patch in a OPTIONS * is completely redundant, which is exactly my point. We shouldn't spend time on interpreting bad specs, we should instead spend time on writing good ones.

So, why can't we just clarify meanwhile stay compatible with the specs? I don't think that we should introduce a new/different criteria around this.

[kjetilk]

Yeah, lets discuss some differences in philosophy in the editorial call!

[TallTed]

@kjetilk wrote --

Say, a client wants to patch /foo.ttl. With this MUST, it might go

OPTIONS *

find

Accept-Patch: application/sparql-update, application/xml-patch+xml, text/x-patch

realize it has to then figure out from heuristics which one applies to /foo.ttl, or just try out what might work, possibly getting a 415, or just go GET, HEAD or OPTIONS on /foo.ttl, which it probably has to anyway, since it probably needs the ETag anyway.

If the client knows it "wants to patch /foo.ttl", it has no reason to call for OPTIONS * but should (by all logic!) simply call for OPTIONS /foo.ttl -- because the client is not concerned with the server's overall capabilities, but with what the server can/will do with /foo.ttl.

The (potential) inefficiency you're flagging would not be due to the requirements placed on the server by either existing HTTP RFC or LDP Spec SHOULD nor the proposed Solid spec MUST (or SHOULD), but due to poor logic on the part of the client -- which is not told it SHOULD nor MUST do the OPTIONS * call at all. (This chunk of Solid spec is telling the server how it must act in response to requests received from the client, not about what requests the client must make of the server.)

The utility of an Accept-Patch laundry list in response to OPTIONS * is limited, but I think it is not zero. Still, I would not push this on the server as a MUST. A SHOULD seems reasonable, as it always allows for justified omission.

In the LDP WG, we tried very hard to not restate things that were already stated in existing RFC or other spec which LDP inherited. That effort may have meant that we left out details that should have been included. We clearly did not always succeed in clarifying existing ambiguities.

All that said... A SHOULD requirement says "do this unless you have a very good reason not to, and fully understand the ramifications of not doing it" while a MUST says "do this and only this". I believe that SHOULD is the appropriate final choice in most cases where spec development leads to a MUST vs SHOULD disagreement between authors -- because there is clearly a good reason why the thing might not be done.

[kjetilk]

re "use cases", sure, then I suggest we first resolve #9 so that we can systematically examine and document every single desired feature before going further.

Yes, that would be strongly preferable. Do we have time?

2. Accept-Patch can effectively communicate the same functionality as Allow: PATCH.

But it is a superset of the functionality of Allow: Patch.

[JordanShurmer]

I think this discussion is about a non-issue.

The Accept-Patch header explicitly refers to requesting an actual resource.

Accept-Patch SHOULD appear in the OPTIONS response for any resource
that supports the use of the PATCH method rfc5789

The MUST constraint that LDP adds is only for LDP-Resources (it's in section 4.2). Meaning it also applies only to actual resource requests.

On the other hand, the Options * request is for info about the server itself, and is explicitly not about resource requests.

An OPTIONS request with an asterisk ("") as the request-target
(Section 5.3 of [RFC7230]) applies to the server in general rather
than to a specific resource. Since a server's communication options
typically depend on the resource, the "" request is only useful as a
"ping" or "no-op" type of method; it does nothing beyond allowing the
client to test the capabilities of the server. For example, this can
be used to test a proxy for HTTP/1.1 conformance (or lack thereof). rfc7231

When a client wishes to request OPTIONS for the server as a whole, as
opposed to a specific named resource of that server, the client MUST
send only "*" (%x2A) as the request-target. rfc7230

Therefore, the must in LDP does not force servers to add Accept-Patch headers to an OPTIONS * request. Indeed, Accept-Patch for the server in general is not even defined in any spec as far as I can tell

[kjetilk]

@JordanShurmer Yeah, I agree with all this, my problem was the LDP in 4.2.7.1 doesn't mention resource.

[kjetilk]

We're moving towards recording consensus on this, so just to summarize what is needed:

• A resolution to OPTIONS *.
• Whether SPARQL Update is required.
• What SPARQL Update features are required.
• When a SPARQL Update query is an append (What does an append operation imply? #118) or write operation.
• If other patch formats are required, in particular, we should mention LD-PATCH, since it was designed along with LDP.

The rest seems to follow from LDP and other RFCs.

[JordanShurmer]

@kjetilk all of the section 4 sub-sections apply only to resource requests.

From the non-normative introduction, which is not normative but can help resolve ambiguities:

The following sections define the conformance rules for LDP servers when serving LDPRs. - ldp

More importantly, 4.2.7.1 explicitly refers only to ldp resources:

When a LDP server supports this method, this specification imposes the following new requirements for LDPRs. - ldp

[kjetilk]

Hmmm, right, that's a good point.

[kjetilk]

Wrote an overview of what we need to consider for SPARQL Update in #125 , and it got pretty lengthy.

[csarven]

Jotting this down for later:

One criteria to emphasise in the Solid spec is PATCH's atomicity requirement. It is sufficiently clear in RFC 5789:

The atomicity requirement holds for all directly affected files.

and what that entails. The Solid spec will have a global atomicity requirement ( #137 ) on creating nested containers when necessary and error if not possible. So, the requirements are well aligned.

Potential errors should be reported from general (can create nested containers?) to specific (can modify resource?)

[kjetilk]

I think that to advance this, we need to free ourselves from the requirements of SPARQL Update, as the number of issues to be settled there is pretty large, and it is best done in a panel.

So, nevermind my list above, what do we really need to agree on to find rough consensus on this one?

[csarven]

We agree on PATCH being required. It may require one or more specified ways to perform an update. If/when SPARQL Update and other approaches are cleared, the PATCH section can refer to them.

There are other relatively minor things that's in agreement eg. PATCH appears in Allow, Allow-Patch in some places (I think for the sake of uniformity, it should also be in OPTIONS * but this is not a show stopper for rough consensus right now), atomicity expectations, .. There are no particular restrictions on the kind of resources that would allow PATCH eg. containers and regular resources can be PATCH'd. It is only the formats that's of a concern that can be addressed.

[kjetilk]

Right, and since we have freed ourselves somewhat from the shacles of LDP since we had most of the discussion, I think we can go for something like

• OPTIONS * MUST include Allow: PATCH, MAY include Accept-Patch with a list of media types.
• HEAD, GET and OPTIONS on the resource itself, MUST include Allow: PATCH, and MUST include Accept-Patch with a list of media types.
• As you say, any resource can be PATCHed, it is just whether there is a media type that determines it. 415 if it doesn't.

Then, we need to say something around atomicity.

Is that it? Can we move to rough consensus?