Let views result cache use cache contexts

Looks like a good start :)

Per #2381277-20: Make Views use render caching and remove Views' own "output caching".

Patch looks good to me. Almost ready?

1. +++ b/core/modules/views/src/Plugin/views/cache/CachePluginBase.php
   @@ -290,25 +290,18 @@ public function generateResultsKey() {
   +      $key_data = [
   +        'build_info' => $build_info,
   +      ];
   

   What exactly is in $build_info? Would be great to either document this or add an @see, so that the reason why we have this here will be clear to future readers.

2. +++ b/core/modules/views/src/Tests/Plugin/CacheTest.php
   @@ -239,32 +266,36 @@ public function testCacheData() {
   +    // Now change the cache context, a different query should be executed.
   

   This doesn't change the cache context (e.g. 'user' -> 'url'), it keeps the same cache context, but sets a different value for the same cache context.

   Suggested change: s/cache context/s/cache context value/

3. +++ b/core/modules/views/src/Tests/Plugin/CacheWebTest.php
   @@ -0,0 +1,74 @@
   + * Definition of Drupal\views\Tests\Plugin\CacheTest.
   

   Contains…

Before RTBC'ing, I'd like @Fabianx or @effulgentsia to take a look at it.

Wow! I'm pleasantly surprised that the fix is this simple! Seems pretty sound to me.

+++ b/core/modules/views/src/Plugin/views/display/DisplayPluginInterface.php
@@ -447,6 +447,16 @@ public function preExecute();
   /**
+   * Gets the cache metadata.
+   *
+   * @return array
+   *   Returns an array:
+   *   - first value: (boolean) Whether the display is cacheable.
+   *   - second value: (string[]) The cache contexts the display varies by.
+   */
+  public function getCacheMetadata();

What about changing this to just getCacheContexts()? For that matter, what about making DisplayPluginInterface extend CacheableDependencyInterface, and thereby also add implementations of getCacheTags() and getCacheMaxAge() to DisplayPluginBase?

I agree with effulgentsia to re-use the CacheableDependencyInterface.

I also would like to see the cache key being generated from a standard cacheable render array.

Maybe we should make this public?

https://api.drupal.org/api/drupal/core!lib!Drupal!Core!Render!Renderer.p...

Currently the views thing also does not seem to handle max-age, yet?

1. +1 for CacheableDependencyInterface. Note that when CacheablePluginInterface was added, we still only had CacheableInterface. That was a bad fit, for the reasons that we did #2444231: Fix CacheableInterface so it makes sense (was: "Make Config objects & Entities implement CacheableInterface + add BubbleableMetadata::createFromCacheableObject()") in the first place. The new interface is a good fit.
2. And indeed, max-age is not yet supported, but that's basically for the same reasons as the point above. I don't see any reason why we shouldn't use max-age, since it's just a more precise description of the boolean "is cacheable"; it captures both "is cacheable" and "for how long".

I think that probably belongs in a separate issue though.

It being critical, lets get this in, but please open the follow-up for the re-factoring.

Follow-up filed: #2464431: Remove CacheablePluginInterface in favor of CacheableDependencyInterface.

Yep, I'm ok with the followup. I called it out, because I thought why add an interface method in this issue that we'll then either need to remove or deprecate but retain for BC. But, looking more into this, I see that cache_metadata already exists as a schema-defined property, so adding a getter method for it to the interface seems fine, because if we keep the property, we can keep the getter method, and if we change the property in that followup, then we're breaking BC anyway and additionally removing the getter wouldn't make that worse.

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

diff --git a/core/modules/views/src/Tests/Plugin/CacheWebTest.php b/core/modules/views/src/Tests/Plugin/CacheWebTest.php
index 2a35070..3c825a7 100644
--- a/core/modules/views/src/Tests/Plugin/CacheWebTest.php
+++ b/core/modules/views/src/Tests/Plugin/CacheWebTest.php
@@ -7,8 +7,6 @@
 
 namespace Drupal\views\Tests\Plugin;
 
-use Drupal\node\Entity\Node;
-use Drupal\views\Tests\ViewUnitTestBase;
 use Drupal\views\Views;
 
 /**

Fixed the unused uses in the new file.

Related: #2442769: Views result cache ignores query arguments..