Re: PATCH draft

Lisa Dusseault wrote:
> Julian helped me get another draft of this out that fixes his issues:
> 
> http://tools.ietf.org/html/draft-dusseault-http-patch-12
> ...

OK, here's new feedback, most of which are nits:


INTRODUCTION, paragraph 14:
OLD:

    Several applications extending HTTP require a feature to do partial
    resource modification.  Existing HTTP functionality only allows a
    complete replacement of a document.  This proposal adds a new HTTP
    method, PATCH, to modify an existing HTTP resource.

NEW:

    Several applications extending the Hypertext Transfer Protocol (HTTP)
    require a feature to do partial resource modification.  The existing
    HTTP PUT method only allows a complete replacement of a document.
    This proposal adds a new HTTP method, PATCH, to modify an existing
    HTTP resource.

(expand HTTP acronym on first use; probably should also be done in the 
title)


Section 1., paragraph 1:
OLD:

    This specification defines the new HTTP 1.1 [RFC2616] method PATCH
    that is used to apply partial modifications to a resource.

NEW:

    This specification defines the new HTTP/1.1 [RFC2616] method PATCH
    that is used to apply partial modifications to a resource.

s|HTTP 1.1|HTTP/1.1|


Section 2., paragraph 1:
OLD:

    The PATCH method requests that a set of changes described in the
    request entity be applied to the resource identified by the Request-
    URI.  The set of changes is represented in a format called a "patch
    document" identified by a media type.  PATCH is neither safe or
    idempotent as defined by [RFC2616], Section 9.1.  If the Request-URI
    does not point to an existing resource, and that URI is capable of
    being defined as a new resource by the requesting user agent, the
    origin server can create the resource with that URI.

NEW:

    The PATCH method requests that a set of changes described in the
    request entity be applied to the resource identified by the Request-
    URI.  The set of changes is represented in a format called a "patch
    document" identified by a media type.  If the Request-URI does not
    point to an existing resource, and that URI is capable of being
    defined as a new resource by the requesting user agent, the origin
    server can create the resource with that URI.

    PATCH is neither safe nor idempotent as defined by [RFC2616], Section
    9.1.

(pull statement about safety into separate paragraph)


Section 2., paragraph 4:
OLD:

    If the request passes through a cache and the Request-URI identifies
    one or more currently cached entities, those entries SHOULD be
    treated as stale.  Responses to this method are not cacheable, unless
    the response includes appropriate Cache-Control or Expires header
    fields or the response uses the 209 Content Returned status code as
    defined in Section 4.  The 303 (See Other) response can be used to
    direct the user agent to retrieve a cacheable resource.

NEW:

    If the request passes through a cache and the Request-URI identifies
    one or more currently cached entities, those entries SHOULD be
    treated as stale.  Responses to this method are not cacheable, unless
    the response includes appropriate Cache-Control or Expires header
    fields or the response uses the 209 Content Returned status code as
    defined in Section 4.  The 303 (See Other) response can be used to
    direct the user agent to retrieve a cacheable resource. [[anchor1:
    ???]]

I'm not totally sure why we'd want 303 here. What's wrong with 200; it's 
not like the 303 ever would include a different URI in the Location 
header, right?


Section 2., paragraph 5:
OLD:

    Collisions from multiple requests are more dangerous than PUT
    collisions, because a patch document that is not operating from a
    known base point may corrupt the resource.  Clients wishing to apply
    a patch document to a known entity can first acquire the strong ETag
    of the resource to be modified, and use that Etag in the If-Match
    header on the PATCH request to verify that the resource is still
    unchanged.  If a strong ETag is not available for a given resource,
    the client can use If-Unmodified-Since as a less-reliable safeguard.

NEW:

    Collisions from multiple requests are more dangerous than PUT
    collisions, because a patch document that is not operating from a
    known base point may corrupt the resource.  Clients wishing to apply
    a patch document to a known entity can first acquire the ETag of the
    resource to be modified, and use that Etag in the If-Match header on
    the PATCH request to verify that the resource is still unchanged.  If
    an ETag is not available for a given resource, the client can use If-
    Unmodified-Since as a less-reliable safeguard.

Why exactly do we require a strong etag here in general?


Section 2.2., paragraph 2:
OLD:

    Unsupported patch document:  Can be Specified using a 415
       (Unsupported Media Type) when the client sends a patch document
       format that the server does not support for the resource
       identified by the Request-URI.  Such a response SHOULD include an
       Accept-Patch response header as described in Section 3.1 to notify
       the client what patch document formats are supported.

NEW:
    Unsupported patch document:  Can be specified using a 415
       (Unsupported Media Type) when the client sends a patch document
       format that the server does not support for the resource
       identified by the Request-URI.  Such a response SHOULD include an
       Accept-Patch response header as described in Section 3.1 to notify
       the client what patch document formats are supported.

s/S/s/

Section 2.2., paragraph 3:
OLD:

       *  The client attempted to apply a structural modification and the
          structures assumed to exist did not exist (e.g. a patch which
          specifies changing element 'foo' to element 'bar' but element
          'foo' doesn't exist).
       *  The client attempted to modify a resource in a way that would
          cause the resource to become invalid.  For instance, a
          modification to a well-formed XML document that would cause it
          to no longer be well-formed.
       *  The client attempted to modify a resource that has multiple
          representations but the server was unable to choose which
          representation to modify.
    Conflicting modification:  Specified with a 412 (Precondition Failed)
       when a client uses either the If-Match or If-Unmodified-Since
       request headers and attempts to apply a patch document to a
       resource whose state has changed since the patch was created.  If
       the server detects a possible conflicting modification and neither
       the If-Match or If-Unmodified-Since request headers are used, the
       server can return a 409 (Conflict) response.
    Concurrent modification:  When a server receives multiple concurrent
       requests to modify a resource, those requests SHOULD be queued and
       processed in the order in which they are received.  If a server is
       incapable of queuing concurrent requests, all subsequent requests
       SHOULD be rejected with a 409 (Conflict) until the first
       modification request is complete.

I think in all these cases 422 is not the right choice, because the 
problem is not with the request, but the state of the resource.  Thus it 
seems 409 Conflict would be better.

Section 3.1., paragraph 3:
OLD:

  3.2.  An example OPTIONS request and response

NEW:

  3.2.  An example OPTIONS Request and Response

(title case)

Appendix A., paragraph 1:
OLD:

     PATCH is not a new concept, it first appeared in HTTP in drafts of
     version 1.1 written by Roy Fielding and Henrik Frystyk and also
     appears in RFC 2068.

NEW:

     PATCH is not a new concept, it first appeared in HTTP in drafts of
     version 1.1 written by Roy Fielding and Henrik Frystyk and also
     appears in Section 19.6.1.1 of RFC 2068.

(point to specific section of RFC 2068)


BR, Julian

Received on Friday, 30 January 2009 13:17:49 UTC