As explained in Proposing an alternative to application/vnd.drupal.ld+json, the new plan is to use HAL as the primary format for REST.

Files: 
CommentFileSizeAuthor
#19 1935538-19-rest-update.patch23.33 KBeffulgentsia
PASSED: [[SimpleTest]]: [MySQL] 53,933 pass(es).
[ View ]
#18 1935538-18-rest-update.patch20.54 KBeffulgentsia
PASSED: [[SimpleTest]]: [MySQL] 53,909 pass(es).
[ View ]
#17 1935538-17-rest-update.patch22.78 KBeffulgentsia
PASSED: [[SimpleTest]]: [MySQL] 53,873 pass(es).
[ View ]
#17 interdiff.txt2.32 KBeffulgentsia
#16 1935538-16-rest-update.patch22.78 KBlinclark
FAILED: [[SimpleTest]]: [MySQL] 53,914 pass(es), 3 fail(s), and 0 exception(s).
[ View ]
#13 1935538-13-rest-update.patch23.34 KBlinclark
FAILED: [[SimpleTest]]: [MySQL] 53,688 pass(es), 3 fail(s), and 0 exception(s).
[ View ]
#5 1935538-05-rest-test-update.patch45.72 KBlinclark
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1935538-05-rest-test-update.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#5 1935538-05-rest-test-update__for-review.txt21.35 KBlinclark
#2 1935538-02-rest-test-update.patch66.85 KBlinclark
PASSED: [[SimpleTest]]: [MySQL] 52,558 pass(es).
[ View ]
#1 1935538-01-rest-test-update.patch66.88 KBlinclark
PASSED: [[SimpleTest]]: [MySQL] 52,680 pass(es).
[ View ]
#1 1935538-01-rest-test-update__for-review.txt21.41 KBlinclark

Comments

Component:base system» rest.module
Status:Active» Needs review
Issue tags:+WSCCI
StatusFileSize
new21.41 KB
new66.88 KB
PASSED: [[SimpleTest]]: [MySQL] 52,680 pass(es).
[ View ]

This patch depends on #1924220: Support serialization in hal+json and #1931976: Support deserialization for hal+json (needs more language handling tests).

It also required a few small changes in the REST module. Specifically, on PATCH it ensures that the id and uuid properties from the new entity do not overwrite those of the original.

StatusFileSize
new66.85 KB
PASSED: [[SimpleTest]]: [MySQL] 52,558 pass(es).
[ View ]

Reroll since info.yml got in.

+++ b/core/modules/rest/lib/Drupal/rest/Plugin/rest/resource/EntityResource.php
@@ -155,6 +155,11 @@ public function patch($id, EntityInterface $entity) {
+      // Requests cannot overwrite id or uuid, so skip them.
+      if (in_array($name, array('id', 'uuid'))) {
+        continue;
+      }

why is the UUID protected from writing? I can see that most use cases will never do that, but why artificially restricting it?

Otherwise this looks good!

If we can't rely on uuid not changing, we can't build any functionality based off of uuid being, er, universally unique. If you want an object to have a different uuid... make a new object. (If we allow it, someone somewhere is going to change it and then we'll never be able to rely on the uuid again.)

StatusFileSize
new21.35 KB
new45.72 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1935538-05-rest-test-update.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Reroll due to changes made in the REST test and HAL serialization getting in.

The .patch file includes the latest patch from #1931976: Support deserialization for hal+json (needs more language handling tests).

The for-review patch is dead simple and seems fine. If anything, I'd ask if we should be centralizing our definition of default so that we don't have to restate it in 50 places.

The dependent issue is still CNW, though, so we can't RTBC this one.

Issue tags:-WSCCI

#5: 1935538-05-rest-test-update.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+WSCCI

The last submitted patch, 1935538-05-rest-test-update.patch, failed testing.

Status:Needs work» Needs review

Probably just need to upload the .txt file in #5 with a .patch extension.

Status:Needs review» Needs work

The last submitted patch, 1935538-05-rest-test-update.patch, failed testing.

xpost.

Changes in the REST tests since I posted this, have to reroll.

Status:Needs work» Needs review
StatusFileSize
new23.34 KB
FAILED: [[SimpleTest]]: [MySQL] 53,688 pass(es), 3 fail(s), and 0 exception(s).
[ View ]

Here's a partial reroll, it still fails one of the tests. I won't have a chance to come back to it until this evening.

Status:Needs review» Needs work

The last submitted patch, 1935538-13-rest-update.patch, failed testing.

I think I might need Klaus to take a look at these fails. I don't see a clear reason why in these three instances, switching to HAL would give a 500 instead of a 403, but it might be more obvious to him.

StatusFileSize
new22.78 KB
FAILED: [[SimpleTest]]: [MySQL] 53,914 pass(es), 3 fail(s), and 0 exception(s).
[ View ]

The last patch reverted one of eff's fixes from the deserialization patch, so this just reintroduces that change. It doesn't fix the test fails, eff said he would take a look.

Status:Needs work» Needs review
StatusFileSize
new2.32 KB
new22.78 KB
PASSED: [[SimpleTest]]: [MySQL] 53,873 pass(es).
[ View ]

Just a silly typo fix.

Status:Needs review» Needs work
StatusFileSize
new20.54 KB
PASSED: [[SimpleTest]]: [MySQL] 53,909 pass(es).
[ View ]

This just removes hunks unrelated to this issue. I moved them into #1931976-28: Support deserialization for hal+json (needs more language handling tests). I'll follow up with one more reroll for #6.

Status:Needs work» Needs review
StatusFileSize
new23.33 KB
PASSED: [[SimpleTest]]: [MySQL] 53,933 pass(es).
[ View ]

If anything, I'd ask if we should be centralizing our definition of default so that we don't have to restate it in 50 places.

Agreed. Done here. This changes nearly every line so no point in an interdiff.

Status:Needs review» Reviewed & tested by the community

Thanks for fixing tests

Status:Reviewed & tested by the community» Fixed

Oh, JSON-LD. We blow a trumpet for thee. Do we need another patch to remove the module as a whole?

In the meantime, this looks like it cleans up an awful lot of hard-coding as well as switching to a more recognized(?) format, so woohoo.

Committed and pushed to 8.x. Thanks!

Title:Switch REST to default to HALChange notice: Switch REST to default to HAL
Priority:Normal» Critical
Status:Fixed» Active
Issue tags:+Needs change record

Actually. I imagine this will require a change to a change notice somewhere, else a new one to be made if it doesn't already exist.

Shouldn't the change notice be for #1935548: Remove JSON-LD module?

Title:Change notice: Switch REST to default to HALSwitch REST to default to HAL
Priority:Critical» Normal
Status:Active» Fixed
Issue tags:-Needs change record

Moved the change notice requirement to #1816354-43: Add a REST module, starting with DELETE.

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