Problem/Motivation

We want to support partial updates of resources in REST module. Example: a client should be able to update the title of a node without having the full representation of that node. The client should be able to send only the title property in a JSON-LD data structure. The client should also be able to remove properties/fields. Example: a client wants to remove all term references from the field_tags property of a node, so it sends an empty flag for field_tags in JSON-LD.

HTTP PUT is not a good fit for partial updates since it has the semantics of completely replacing a whole resource with the transmitted data. Therefore we should use HTTP PATCH which reflects our needs more closely.

Proposed resolution

Implement a PATCH operation for rest.module.

Remaining tasks

  • HTTP PATCH is not part of the official HTTP standard yet, so we have to research if it is widely supported by most clients (Javascript, cURL, Guzzle, ...)
  • A patch has to be developed and reviewed

User interface changes

none.

API changes

A patch operation is added to entity resource plugins.

Original report by [mitchell]

A PATCH request to "/node/{node}/revision/{vid}" should contain, in the body information, different field values than a source revision or an equivalent patch file.

RFC: 5789 - PATCH Method for HTTP

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.

Related Issues

Files: 
CommentFileSizeAuthor
#16 rest-patch-1826688-16.patch13.92 KBklausi
PASSED: [[SimpleTest]]: [MySQL] 49,354 pass(es).
[ View ]
#16 rest-patch-1826688-16-interdiff.txt5.47 KBklausi
#14 rest-patch-1826688-14.patch13.13 KBklausi
PASSED: [[SimpleTest]]: [MySQL] 49,134 pass(es).
[ View ]
#14 rest-patch-1826688-14-interdiff.txt1.03 KBklausi
#12 rest-patch-1826688-12.patch13.24 KBklausi
PASSED: [[SimpleTest]]: [MySQL] 49,397 pass(es).
[ View ]
#12 rest-patch-1826688-12-interdiff.txt4.45 KBklausi
#10 rest-patch-1826688-10.patch8.36 KBklausi
FAILED: [[SimpleTest]]: [MySQL] 49,387 pass(es), 3 fail(s), and 0 exception(s).
[ View ]

Comments

mitchell’s picture

Title:Add an entity PATCH operation» Support PATCH method for entity updates
Component:entity system» routing system

JS clients can use data from #1811510: Enable JSON-LD entity serialization to generate diff payloads instead of sending back the entire entity. See #120955-73: Integrate Diff into Core and this article, Why PATCH Is Good for Your HTTP API, which describes PATCH with JSON and PATCH with XML.

mitchell’s picture

Title:Support PATCH method for entity updates» REST module: PATCH/update
Component:routing system» rest.module
Issue tags:+revision, +WSCCI
mitchell’s picture

Issue summary:View changes

x

Grayside’s picture

The explicit patch payload formats as discussed in the "Why PATCH..." article have not reached a standard practice AFAICT, so I assume you mean to allow partial-entity-updates, where the absence of a payload element means to leave it untouched.

Here is a use case. Configuration.

  • No PATCH: Every Module Y configuration element has a REST URI.
  • No PATCH: Module Y configuration has a URI. To change one part I need to retrieve and return all it's configuration in a big lump.
  • PATCH: Module Y configuration has a URI. I retrieve all it's configuration, but update one or some of the individual elements.
Grayside’s picture

Issue summary:View changes

x

klausi’s picture

I updated the issue summary.

Damien Tournoud’s picture

If we want to go that route, we really want to implement full support for application/json-patch. There are a couple of implementations floating around, for example [1] and [2]. There are both slightly out of date now.

[1] https://github.com/stefankoegl/python-json-patch
[2] https://github.com/mikemccabe/json-patch-php

effulgentsia’s picture

The issue summary suggests the patch payload to be in JSON-LD and #5 suggests to do it in straight JSON. I don't know if the two are actually compatible: at a minimum, I would expect JSON-LD to require a different mime type than application/json-patch. Which begs the question: while JSON-LD makes a lot of sense for receiving (GET), what's its value in sending (PATCH)? And is it even possible to GET in JSON-LD and send in JSON? For example, if we get entity references as URIs in JSON-LD, but need to send them as IDs in JSON, that could be problematic, right?

Also, #5 doesn't link to the application/json-patch spec itself, but following the other links, I think the current one is http://tools.ietf.org/html/draft-ietf-appsawg-json-patch-08. Please correct if not.

Grayside’s picture

In an ideal world, all output formats are viable input formats, and there is no "type accent" tainting the data in between operations.

linclark’s picture

No, the two would not be compatible.

I think we want to ask the question the other way around. What is the benefit of introducing application/json-patch in the mix? Is it widely used? Are there implementations we want to integrate with that use it? What does it give us?

I think that it makes sense for the use case that they outline in the spec—working with simple JSON and where the document is extremely flexible, without a fixed set of property names which can be used. However, I don't think that it offers us anything, and I think the DX it offers is worse than PATCHing with the output format (JSON-LD for example) since you actually have to specify both operation AND data, rather than just data.

linclark’s picture

Also, just saw this in a discussion of json-patch, which sums up how I see it:

I think that's the biggest caveat I'm seeing with the proposal. It's pitched as a way to handle HTTP PATCH requests, but a format that defines "diffs on JSON documents" is most suited for systems that primarily store JSON documents and diffs on JSON documents - it is much less broadly applicable for "HTTP servers that accept JSON data", since frequently they only work with JSON over the wire, not as the ultimate source of truth. If you don't store JSON documents as the ultimate source of truth or have a domain-model reason to want diffs, I'm unsure of the advantage.

http://news.ycombinator.com/item?id=4479442

klausi’s picture

Status:Active» Needs review
StatusFileSize
new8.36 KB
FAILED: [[SimpleTest]]: [MySQL] 49,387 pass(es), 3 fail(s), and 0 exception(s).
[ View ]

I agree that the json-patch format is just too verbose and requires a lot of effort on the client side to build the structure correctly.

I'm thinking of our usual JSON-LD format where we require @type and @id and all other properties are optional. Just include those properties in the payload that you want to overwrite.

Here is a fist patch that does not work, largely inspired by #1839366: REST module: PUT/update. I'm having trouble to detect empty array values for properties that should be deleted. The entity API fills up all known properties with empty arrays, so I don't know how to copy empty values from the deserialized entity to the stored entity.

Status:Needs review» Needs work

The last submitted patch, rest-patch-1826688-10.patch, failed testing.

klausi’s picture

Status:Needs work» Needs review
StatusFileSize
new4.45 KB
new13.24 KB
PASSED: [[SimpleTest]]: [MySQL] 49,397 pass(es).
[ View ]

So we need the entity API to distinguish between empty() and isset() properties in the magic methods. See #1868274: Improved EntityNG isset() test coverage for that, which is included in this patch.

Now PATCHing actually works as expected, "[]" for a field in JSON-LD triggers its deletion. Happy klausi is happy! :-)

moshe weitzman’s picture

Status:Needs review» Needs work

Nice work.

+++ b/core/modules/rest/lib/Drupal/rest/Tests/UpdateTest.php
@@ -0,0 +1,94 @@
+    // Create a second stub entity where the UUID is different.
+    $patch_entity = entity_create($entity_type, array());
+    $patch_entity->{$entity_info['entity_keys']['id']} = $entity->id();
+    $serialized = $serializer->serialize($patch_entity, 'drupal_jsonld');

Does entity->id() really return a UUID? Is that only the behavior when an entity has not yet been saved? Also, I think it would be clearer if we replaced $patch_entity->{$entity_info['entity_keys']['id']} with uuid

+++ b/core/modules/rest/lib/Drupal/rest/Tests/UpdateTest.php
@@ -0,0 +1,94 @@
+    // Update the entity over the web API.

To be extra clear, I would say 'over the REST API'. Happens a few times in this file, and maybe others.

klausi’s picture

Status:Needs work» Needs review
StatusFileSize
new1.03 KB
new13.13 KB
PASSED: [[SimpleTest]]: [MySQL] 49,134 pass(es).
[ View ]

No, $entity->id() does not return a UUID, it returns the usual numeric integer. I just realized that it is not even necessary to copy over the ID property, so I removed that line and another one.

I'm using the term "web API" consistently throughout REST module, so I would leave it at that for now. If we want to change that we should do it an a separate issue.

moshe weitzman’s picture

Status:Needs review» Needs work

Could we change 'UUID was successfully updated.' to some other field. Its a pretty unrealistic use case to change an entity's uuid.

I opened #1869424: Use the terms 'form api' and 'rest api' instead of 'web api' as a followup.

klausi’s picture

Status:Needs work» Needs review
StatusFileSize
new5.47 KB
new13.92 KB
PASSED: [[SimpleTest]]: [MySQL] 49,354 pass(es).
[ View ]

I wrote those tests with different entity types in mind and thought it might be easier to use a property that all of them share (the UUID), once we expand the test to other entity types. But yes, using a field for testing is more realistic.

* Fixed test case to overwrite a field instead of UUID.
* Switched to 204 success status code.
* Made patching not existing entities more robust and added test case.

moshe weitzman’s picture

Status:Needs review» Reviewed & tested by the community

Thats better - thanks.

Dries’s picture

Status:Reviewed & tested by the community» Fixed

Committed to 8.x. Thanks!

fago’s picture

Looks like this commit contained #1868274: Improved EntityNG isset() test coverage which hasn't received any reviews yet :(

klausi’s picture

Yep, this has gone a bit too fast from RTBC to fixed. I should have mentioned it in the summary that this issue depends on another one, sorry for that.

I'm going to set the other issue to "active", so we can develop a followup there.

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary:View changes

issue summary template.