REST entity resource missing entity & field access cacheability metadata

Some questions:

How does wget / curl respond?
Are you logged in?
Can you share your guzzle code on ie github?

Added a link to 'stale content'

The problems described in the issue so far sound like browser caching, plain & simple. If the response have a Cache-Control: max-age=60 header for example, the browser is going to cache the response for a minute, regardless of cache tags being invalidated. Cache tags can only invalidate responses on the server, not on the client.

But, I'm going to repurpose this issue for something closely related, because in EntityResource there's at least this problem wrt cacheability:

    $response = new ResourceResponse($entity, 200);
    // Make the response use the entity's cacheability metadata.
    // @todo include access cacheability metadata, for the access checks above.
    $response->addCacheableDependency($entity);
    return $response;
  }

So, I'll take a look at improving this.

The fix was trivial, it took me far more time to write test coverage. (Rolled against 8.1 but should apply cleanly to 8.0 too.)

Unrelated test failures due to pre-existing tests asserting the expected cacheability metadata for the field_test_text field, which I changed here to have something to detect:

+++ b/core/modules/system/tests/modules/entity_test/src/EntityTestAccessControlHandler.php
@@ -59,4 +61,16 @@ protected function checkCreateAccess(AccountInterface $account, array $context,
+  protected function checkFieldAccess($operation, FieldDefinitionInterface $field_definition, AccountInterface $account, FieldItemListInterface $items = NULL) {
+    // Custom cacheability metadata for viewing the 'field_test_text' field.
+    if ($operation === 'view' && $field_definition->getName() === 'field_test_text') {
+      return AccessResult::allowed()->addCacheTags(['entity_test_accesss:field_test_text']);
+    }
+
+    return parent::checkFieldAccess($operation, $field_definition, $account, $items);
+  }

I'll work on addressing those.

Oh wow. Going to have to work to wrap my head around that.

Skim:

1. +++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php
   @@ -45,19 +45,25 @@ class EntityResource extends ResourceBase {
   +    $entity_access = $entity->access('view', NULl, TRUE);
   

   NULL

2. +++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php
   @@ -45,19 +45,25 @@ class EntityResource extends ResourceBase {
   +      var_dump($field_name);
   

   :-D

+++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php
@@ -45,19 +45,25 @@ class EntityResource extends ResourceBase {
-    if (!$entity->access('view')) {
+    $entity_access = $entity->access('view', NULl, TRUE);
+    if (!$entity_access->isAllowed()) {

NUL1 ?

lol lol lol

I'm a moron.

First, fix the plain stupid things.

I can't reproduce the failure in Drupal\rest\Tests\ReadTest. :(

Let's hope #17 will only have failures in Drupal\system\Tests\Entity\FieldAccessTest — the failures there I can reproduce.

This fixes the remaining failure.

Interdiff explanation: Rather than modifying EntityTestAccessControlHandler, I'm now just adding one more special value to entity_test_entity_field_access() to test for. In doing so, I have to use addCacheableDependency(). The surrounding code is still using the deprecated cacheUntilEntityChanges(). I think it's in scope here to update them.

+++ b/core/modules/rest/src/Tests/PageCacheTest.php
@@ -33,6 +33,7 @@ public function testConfigChangePageCache() {
+    $entity->set('field_test_text', 'custom cache tag value');

+1 for a more explicit value.

+++ b/core/modules/rest/src/Tests/PageCacheTest.php
@@ -40,6 +41,7 @@ public function testConfigChangePageCache() {
+    $this->assertCacheTag('entity_test_accesss:field_test_text');

Are there three s on purpose? Looks like a typo that got copied everywhere.

Yep, that's a typo. Fixing.

He, good spot.

For a minute I though it was a llama -> snake switch.

Committed/pushed to 8.1.x and cherry-picked to 8.0.x. Thanks!

For a minute I though it was a llama -> snake switch.

:P

LLAMAS FOREVER <3 <3