[wp-trac] [WordPress Trac] #47676: Add support for If-Unmodified-Since header to make a conditional request when updating posts via REST API (for offline and collaborative editing)

[WordPress Trac]

#47676: Add support for If-Unmodified-Since header to make a conditional request
when updating posts via REST API (for offline and collaborative editing)
--------------------------------------+------------------------------
 Reporter:  westonruter               |       Owner:  (none)
     Type:  defect (bug)              |      Status:  new
 Priority:  normal                    |   Milestone:  Awaiting Review
Component:  Editor                    |     Version:  4.7
 Severity:  normal                    |  Resolution:
 Keywords:  has-patch has-unit-tests  |     Focuses:  rest-api
--------------------------------------+------------------------------

Comment (by westonruter):

 Replying to [comment:2 TimothyBlynJacobs]:
 > Why `If-Unmodified-Since` instead of using ETag's? An ETag could be
 built by serializing the item response.

 Good question. I didn't think about that. One issue with using ETag
 generated from serializing the response is that it would require always
 querying data for all fields, even if they are not requested by the user.
 For example, if someone had obtained the entity originally via `/wp-
 json/wp/v2/posts/2553?_fields=id,title,link` and then they modify the
 `title` and `PUT` back the modified post, then the ETag would not have
 been generated to include the `content`. Or perhaps this doesn't matter
 because you only requested the `title` in the first place?

 > That would handle updates to the entity that don't trigger
 `post_modified` like metadata, terms, and any other custom fields.
 > I suppose if we wanted to get similar benefits, we could force a
 `post_modified` update whenever a post entity is updated through the posts
 controller.

 That is a very good point. For custom fields, the only thing to protect
 against that would be to store some additional `last_modified` in postmeta
 and bump it whenever postmeta is changed or term associations are changed,
 such as via the posts controller. But that's not ideal.

 > Or is the fact that `post_modified` would only track changes to the post
 resource a benefit?

 The `post_modified` would probably be an 80% solution.

 > If the `If-Unmodified-Since` is used I think we should definitely send
 `Last-Modified` as a header, and ideally support `If-Modified-Since` as
 well. `Last-Modified` can also be sent as a [https://json-
 schema.org/latest/json-schema-hypermedia.html#rfc.section.6.5.5
 targetHint] so it is accessible in the collection route.

 Another great point. Yes, sending the `Last-Modified` header as derived
 from `post_modified_gmt` is something I missed.  A client should not be
 trying to convert the `post_modified_gmt` into a value to for the `If-
 Unmodified-Since` header.

 > One downside to an ETag is it'd be more intensive to calculate.

 I see it being more intensive in two ways:

 1. The `get_item` method would need to compute the `ETag` from serializing
 the response data.
 2. The `update_item` method would need to call `get_item` if the request
 has an `If-Match` header, and then compare the `get_item` method's
 response `ETag` with the `PUT` request's `If-Match` header to see if there
 is a match.

 In other words:

 {{{#!diff
 --- a/src/wp-includes/rest-api/endpoints/class-wp-rest-posts-
 controller.php
 +++ b/src/wp-includes/rest-api/endpoints/class-wp-rest-posts-
 controller.php
 @@ -477,6 +477,12 @@ class WP_REST_Posts_Controller extends
 WP_REST_Controller {
                         $response->link_header( 'alternate',
 get_permalink( $post->ID ), array( 'type' => 'text/html' ) );
                 }

 +               $response->set_headers(
 +                       array(
 +                               'ETag' => md5( serialize(
 $response->get_data() ) ),
 +                       )
 +               );
 +
                 return $response;
         }

 @@ -685,6 +691,17 @@ class WP_REST_Posts_Controller extends
 WP_REST_Controller {
                         return $valid_check;
                 }

 +               // Handle conditional request.
 +               if ( $request->get_header( 'If-Match' ) ) {
 +                       $get_request = clone $request;
 +                       $get_request->set_method( 'GET' );
 +                       $get_request->set_body( null );
 +                       $etag = md5( serialize( $this->get_item(
 $get_request )->get_data() ) );
 +                       if ( $etag !== trim( $request->get_header( 'If-
 Match' ), '"' ) ) {
 +                               return new WP_Error(
 'rest_precondition_failed', __( 'Post has been changed on server. Please
 resolve conflicts and try again' ), array( 'status' => 412 ) );
 +                       }
 +               }
 +
                 $post = $this->prepare_item_for_database( $request );

                 if ( is_wp_error( $post ) ) {
 }}}

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/47676#comment:3>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform