Allow views base tables and entity types to define additional cache contexts

#2099137-66: Entity/field access and node grants not taken into account with core cache contexts contains an initial EntityTypeInterface::getListCacheContexts() implementation. I prefer getting just that part in first, since I expect us to get stuck on tricky Views aspects here, as usual :) If you know how to make this happen, feel free to take the interdiff in that patch and reuse it here when rolling a patch for Views!

(I sadly don't know Views well enough to roll this patch, so I'm choosing to go for the other issue first because I do know how to make progress there.)

The current patch doesn't apply cleanly anymore (but it does apply with git apply -3), so straight reroll. Please don't credit me.

This patch works perfectly! I'd RTBC, but I have one important question and a minor one.

1. +++ b/core/modules/views/config/schema/views.data_types.schema.yml
   @@ -250,16 +250,6 @@ views_display:
   -    cache_metadata:
   
   +++ b/core/modules/views/config/schema/views.schema.yml
   @@ -114,6 +114,18 @@ views.view.*:
   +            cache_metadata:
   

   Why did this have to be moved? It's still at the display level though. Basically: I don't understand these config schemas and therefore don't understand why it's moved.

2. +++ b/core/modules/views/src/Entity/View.php
   @@ -310,13 +310,14 @@ protected function addCacheMetadata() {
   -    foreach ($displays as $display_id => $display) {
   +    foreach (array_keys($displays) as $display_id) {
   +      $display =& $this->getDisplay($display_id);
   

   Oh heh… I guess this means that it never actually worked as expected?

Oh, the IS also mentions as well as in list builder — which is indeed necessary. Do you want to do that in this issue, or in another?

Wonderful — thank you!

RTBC + 1

#2414539: Simplify schema definition for sequence broke this. Straight reroll. Please don't credit me.

+++ b/core/modules/system/src/Tests/Entity/EntityCacheTagsTestBase.php
@@ -378,6 +378,8 @@ public function testReferencedEntity() {
+    // @todo verify <EntityType>::getListCacheTags() are present, blocked on
+    //    https://www.drupal.org/node/2445761

@@ -385,6 +387,8 @@ public function testReferencedEntity() {
+    // @todo verify <EntityType>::getListCacheTags() are present, blocked on
+    //    https://www.drupal.org/node/2445761

The issue has landed.

Right, yay! I'm happy to do this unless dawehner wants to roll it.

Fixed those @todos.

Back to RTBC + 2, looks great!

Oh, how nice to see your cache contexts, views :).

Back to RTBC!

:) This is why I already added cache contexts in #2381217: Views should set cache tags on its render arrays, and bubble the output's cache tags to the cache items written to the Views output cache — because that issue was chiefly about cache tags, but in spirit it was about bubbling in general. So I used cache context merging already there. That's why #21 + #24 are both single-line test fixes. Great to see it come together like that :)

diff --git a/core/modules/node/src/Tests/NodeListBuilderTest.php b/core/modules/node/src/Tests/NodeListBuilderTest.php
index beadf0e..d2e5218 100644
--- a/core/modules/node/src/Tests/NodeListBuilderTest.php
+++ b/core/modules/node/src/Tests/NodeListBuilderTest.php
@@ -6,6 +6,7 @@
  */
 
 namespace Drupal\node\Tests;
+
 use Drupal\simpletest\KernelTestBase;
 
 /**
@@ -26,7 +27,9 @@ protected function setUp() {
     $this->installEntitySchema('node');
   }
 
-
+  /**
+   * Tests that the correct cache contexts are set.
+   */
   public function testCacheContexts() {
     /** @var \Drupal\Core\Entity\EntityListBuilderInterface $list_builder */
     $list_builder = $this->container->get('entity.manager')->getListBuilder('node');
diff --git a/core/modules/system/src/Tests/Entity/EntityListBuilderTest.php b/core/modules/system/src/Tests/Entity/EntityListBuilderTest.php
index 851fed8..58da00f 100644
--- a/core/modules/system/src/Tests/Entity/EntityListBuilderTest.php
+++ b/core/modules/system/src/Tests/Entity/EntityListBuilderTest.php
@@ -56,6 +56,9 @@ public function testPager() {
     $this->assertRaw('Test entity 51', 'Test entity 51 is shown.');
   }
 
+  /**
+   * Tests that the correct cache contexts are set.
+   */
   public function testCacheContexts() {
     /** @var \Drupal\Core\Entity\EntityListBuilderInterface $list_builder */
     $list_builder = $this->container->get('entity.manager')->getListBuilder('entity_test');

Added missing code docblocks.

This issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed a87aff8 and pushed to 8.0.x. Thanks!

Back to RTBC, ouch on the conflict. Do we have a tag for potentially conflicting things?

Let's try again. Committed 0fe1e7d and pushed to 8.0.x. Thanks!

This type of commit conflict is kind of avoidable unless patch author's decide to postpone one of the issues on the other or the rtbc retest gets in first. Which is one of the problems with a long rtbc queue.

This unblocked #2381277: Make Views use render caching and remove Views' own "output caching" and #2099137: Entity/field access and node grants not taken into account with core cache contexts, thanks!

#34: we have the "Avoid commit conflicts" tag, but we generally use it only for patches that are very painful to reroll.