Issue #2477157 by dawehner, Wim Leers, Fabianx, aneek, catch: rest_export...

Issue #2477157 by dawehner, Wim Leers, Fabianx, aneek, catch: rest_export Views display plugin does not set necessary cache metadata

core/modules/rest/src/Plugin/views/display/RestExport.php

@@ -8,12 +8,15 @@
 namespace Drupal\rest\Plugin\views\display;
 
 use Drupal\Component\Utility\SafeMarkup;
+use Drupal\Core\Cache\Cache;
+use Drupal\Core\Cache\CacheableMetadata;
+use Drupal\Core\Cache\CacheableResponse;
+use Drupal\Core\Render\RendererInterface;
 use Drupal\Core\State\StateInterface;
 use Drupal\Core\Routing\RouteProviderInterface;
 use Drupal\views\ViewExecutable;
 use Drupal\views\Plugin\views\display\PathPluginBase;
 use Symfony\Component\DependencyInjection\ContainerInterface;
-use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\Routing\RouteCollection;
 
 /**
@@ -71,6 +74,13 @@ class RestExport extends PathPluginBase {
    */
   protected $mimeType;
 
+  /**
+   * The renderer
+   *
+   * @var \Drupal\Core\Render\RendererInterface
+   */
+  protected $renderer;
+
   /**
    * Constructs a Drupal\rest\Plugin\ResourceBase object.
    *
@@ -84,9 +94,13 @@ class RestExport extends PathPluginBase {
    *   The route provider
    * @param \Drupal\Core\State\StateInterface $state
    *   The state key value store.
+   * @param \Drupal\Core\Render\RendererInterface $renderer
+   *   The renderer.
    */
-  public function __construct(array $configuration, $plugin_id, $plugin_definition, RouteProviderInterface $route_provider, StateInterface $state) {
+  public function __construct(array $configuration, $plugin_id, $plugin_definition, RouteProviderInterface $route_provider, StateInterface $state, RendererInterface $renderer) {
     parent::__construct($configuration, $plugin_id, $plugin_definition, $route_provider, $state);
+
+    $this->renderer = $renderer;
   }
 
   /**
@@ -98,7 +112,8 @@ public static function create(ContainerInterface $container, array $configuratio
       $plugin_id,
       $plugin_definition,
       $container->get('router.route_provider'),
-      $container->get('state')
+      $container->get('state'),
+      $container->get('renderer')
     );
   }
 
@@ -259,7 +274,16 @@ public function execute() {
     parent::execute();
 
     $output = $this->view->render();
-    return new Response(drupal_render_root($output), 200, array('Content-type' => $this->getMimeType()));
+
+    $header = [];
+    $header['Content-type'] = $this->getMimeType();
+
+    $response = new CacheableResponse($this->renderer->renderRoot($output), 200);
+    $cache_metadata = CacheableMetadata::createFromRenderArray($output);
+
+    $response->addCacheableDependency($cache_metadata);
+
+    return $response;
   }
 
   /**
@@ -276,6 +300,16 @@ public function render() {
       $build['#suffix'] = '</pre>';
     }
 
+    // Defaults for bubbleable rendering metadata.
+    $build['#cache']['tags'] = isset($build['#cache']['tags']) ? $build['#cache']['tags'] : array();
+    $build['#cache']['max-age'] = isset($build['#cache']['max-age']) ? $build['#cache']['max-age'] : Cache::PERMANENT;
+
+    /** @var \Drupal\views\Plugin\views\cache\CachePluginBase $cache */
+    $cache = $this->getPlugin('cache');
+
+    $build['#cache']['tags'] = Cache::mergeTags($build['#cache']['tags'], $cache->getCacheTags());
+    $build['#cache']['max-age'] = Cache::mergeMaxAges($build['#cache']['max-age'], $cache->getCacheMaxAge());
+
     return $build;
   }
 

core/modules/rest/src/Tests/Views/StyleSerializerTest.php

@@ -8,6 +8,8 @@
 namespace Drupal\rest\Tests\Views;
 
 use Drupal\Component\Utility\SafeMarkup;
+use Drupal\Core\Cache\Cache;
+use Drupal\system\Tests\Cache\AssertPageCacheContextsAndTagsTrait;
 use Drupal\views\Views;
 use Drupal\views\Tests\Plugin\PluginTestBase;
 use Drupal\views\Tests\ViewTestData;
@@ -24,6 +26,13 @@
  */
 class StyleSerializerTest extends PluginTestBase {
 
+  use AssertPageCacheContextsAndTagsTrait;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected $dumpHeaders = TRUE;
+
   /**
    * Modules to install.
    *
@@ -69,6 +78,9 @@ public function testSerializerResponses() {
 
     $actual_json = $this->drupalGet('test/serialize/field', array(), array('Accept: application/json'));
     $this->assertResponse(200);
+    $this->assertCacheTags($view->getCacheTags());
+    // @todo Due to https://www.drupal.org/node/2352009 we can't yet test the
+    // propagation of cache max-age.
 
     // Test the http Content-type.
     $headers = $this->drupalGetHeaders();
@@ -117,10 +129,18 @@ public function testSerializerResponses() {
     $actual_json = $this->drupalGet('test/serialize/entity', array(), array('Accept: application/json'));
     $this->assertResponse(200);
     $this->assertIdentical($actual_json, $expected, 'The expected JSON output was found.');
+    $expected_cache_tags = $view->getCacheTags();
+    $expected_cache_tags[] = 'entity_test_list';
+    /** @var \Drupal\Core\Entity\EntityInterface $entity */
+    foreach ($entities as $entity) {
+      $expected_cache_tags = Cache::mergeTags($expected_cache_tags, $entity->getCacheTags());
+    }
+    $this->assertCacheTags($expected_cache_tags);
 
     $expected = $serializer->serialize($entities, 'hal_json');
     $actual_json = $this->drupalGet('test/serialize/entity', array(), array('Accept: application/hal+json'));
     $this->assertIdentical($actual_json, $expected, 'The expected HAL output was found.');
+    $this->assertCacheTags($expected_cache_tags);
   }
 
   /**

core/modules/rest/tests/src/Unit/CollectRoutesTest.php

@@ -76,6 +76,7 @@ protected function setUp() {
       ->disableOriginalConstructor()
       ->getMock();
     $container->set('plugin.manager.views.style', $style_manager);
+    $container->set('renderer', $this->getMock('Drupal\Core\Render\RendererInterface'));
 
     \Drupal::setContainer($container);
 

core/modules/system/src/Tests/Cache/AssertPageCacheContextsAndTagsTrait.php

@@ -25,6 +25,25 @@ protected function enablePageCaching() {
     $config->save();
   }
 
+  /**
+   * Gets a specific header value as array.
+   *
+   * @param string $header_name
+   *   The header name.
+   *
+   * @return string[]
+   *   The header value, potentially exploded by spaces.
+   */
+  protected function getCacheHeaderValues($header_name) {
+    $header_value = $this->drupalGetHeader($header_name);
+    if (empty($header_value)) {
+      return [];
+    }
+    else {
+      return explode(' ', $header_value);
+    }
+  }
+
   /**
    * Asserts page cache miss, then hit for the given URL; checks cache headers.
    *
@@ -40,47 +59,16 @@ protected function assertPageCacheContextsAndTags(Url $url, array $expected_cont
     sort($expected_contexts);
     sort($expected_tags);
 
-    $get_cache_header_values = function ($header_name) {
-      $header_value = $this->drupalGetHeader($header_name);
-      if (empty($header_value)) {
-        return [];
-      }
-      else {
-        return explode(' ', $header_value);
-      }
-    };
-
     // Assert cache miss + expected cache contexts + tags.
     $this->drupalGet($absolute_url);
     $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'MISS');
-    $actual_contexts = $get_cache_header_values('X-Drupal-Cache-Contexts');
-    $actual_tags = $get_cache_header_values('X-Drupal-Cache-Tags');
-    $this->assertIdentical($actual_contexts, $expected_contexts);
-    if ($actual_contexts !== $expected_contexts) {
-      debug('Missing cache contexts: ' . implode(',', array_diff($actual_contexts, $expected_contexts)));
-      debug('Unwanted cache contexts: ' . implode(',', array_diff($expected_contexts, $actual_contexts)));
-    }
-    $this->assertIdentical($actual_tags, $expected_tags);
-    if ($actual_tags !== $expected_tags) {
-      debug('Missing cache tags: ' . implode(',', array_diff($actual_tags, $expected_tags)));
-      debug('Unwanted cache tags: ' . implode(',', array_diff($expected_tags, $actual_tags)));
-    }
+    $this->assertCacheTags($expected_tags);
+    $this->assertCacheContexts($expected_contexts);
 
     // Assert cache hit + expected cache contexts + tags.
     $this->drupalGet($absolute_url);
-    $actual_contexts = $get_cache_header_values('X-Drupal-Cache-Contexts');
-    $actual_tags = $get_cache_header_values('X-Drupal-Cache-Tags');
-    $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'HIT');
-    $this->assertIdentical($actual_contexts, $expected_contexts);
-    if ($actual_contexts !== $expected_contexts) {
-      debug('Missing cache contexts: ' . implode(',', array_diff($actual_contexts, $expected_contexts)));
-      debug('Unwanted cache contexts: ' . implode(',', array_diff($expected_contexts, $actual_contexts)));
-    }
-    $this->assertIdentical($actual_tags, $expected_tags);
-    if ($actual_tags !== $expected_tags) {
-      debug('Missing cache tags: ' . implode(',', array_diff($actual_tags, $expected_tags)));
-      debug('Unwanted cache tags: ' . implode(',', array_diff($expected_tags, $actual_tags)));
-    }
+    $this->assertCacheTags($expected_tags);
+    $this->assertCacheContexts($expected_contexts);
 
     // Assert page cache item + expected cache tags.
     $cid_parts = array($url->setAbsolute()->toString(), 'html');
@@ -94,4 +82,47 @@ protected function assertPageCacheContextsAndTags(Url $url, array $expected_cont
     }
   }
 
+  /**
+   * Ensures that some cache tags are present in the current response.
+   *
+   * @param string[] $expected_tags
+   *   The expected tags.
+   */
+  protected function assertCacheTags(array $expected_tags) {
+    $actual_tags = $this->getCacheHeaderValues('X-Drupal-Cache-Tags');
+    $this->assertIdentical($actual_tags, $expected_tags);
+    if ($actual_tags !== $expected_tags) {
+      debug('Missing cache tags: ' . implode(',', array_diff($actual_tags, $expected_tags)));
+      debug('Unwanted cache tags: ' . implode(',', array_diff($expected_tags, $actual_tags)));
+    }
+  }
+
+  /**
+   * Ensures that some cache contexts are present in the current response.
+   *
+   * @param string[] $expected_contexts
+   *   The expected cache contexts.
+   */
+  protected function assertCacheContexts(array $expected_contexts) {
+    $actual_contexts = $this->getCacheHeaderValues('X-Drupal-Cache-Contexts');
+    $this->assertIdentical($actual_contexts, $expected_contexts);
+    if ($actual_contexts !== $expected_contexts) {
+      debug('Missing cache contexts: ' . implode(',', array_diff($actual_contexts, $expected_contexts)));
+      debug('Unwanted cache contexts: ' . implode(',', array_diff($expected_contexts, $actual_contexts)));
+    }
+  }
+
+  /**
+   * Asserts the max age header.
+   *
+   * @param int $max_age
+   */
+  protected function assertCacheMaxAge($max_age) {
+    $cache_control_header = $this->drupalGetHeader('Cache-Control');
+    if (strpos($cache_control_header, 'max-age:' . $max_age) === FALSE) {
+      debug('Expected max-age:' . $max_age . '; Response max-age:' . $cache_control_header);
+    }
+    $this->assertTrue(strpos($cache_control_header, 'max-age:' . $max_age));
+  }
+
 }

core/modules/views/src/Plugin/views/cache/CachePluginBase.php

@@ -372,6 +372,25 @@ public function getCacheTags() {
     return $tags;
   }
 
+  /**
+   * Gets the max age for the current view.
+   *
+   * @return int
+   */
+  public function getCacheMaxAge() {
+    $max_age = $this->getDefaultCacheMaxAge();
+    $max_age = Cache::mergeMaxAges($max_age, $this->view->getQuery()->getCacheMaxAge());
+    return $max_age;
+  }
+
+  /**
+   * Returns the default cache max age.
+   */
+  protected function getDefaultCacheMaxAge() {
+    // The default cache backend is not caching anything.
+    return 0;
+  }
+
   /**
    * Prepares the view result before putting it into cache.
    *

core/modules/views/src/Plugin/views/cache/Tag.php

@@ -7,6 +7,8 @@
 
 namespace Drupal\views\Plugin\views\cache;
 
+use Drupal\Core\Cache\CacheBackendInterface;
+
 /**
  * Simple caching of query results for Views displays.
  *
@@ -34,4 +36,11 @@ protected function cacheExpire($type) {
     return FALSE;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  protected function getDefaultCacheMaxAge() {
+    return CacheBackendInterface::CACHE_PERMANENT;
+  }
+
 }

core/modules/views/src/Plugin/views/cache/Time.php

@@ -13,6 +13,7 @@
 use Drupal\Core\Render\RendererInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Drupal\Core\Form\FormStateInterface;
+use Symfony\Component\HttpFoundation\Request;
 
 /**
  * Simple caching of query results for Views displays.
@@ -39,6 +40,13 @@ class Time extends CachePluginBase {
    */
   protected $dateFormatter;
 
+  /**
+   * The current request.
+   *
+   * @var \Symfony\Component\HttpFoundation\Request
+   */
+  protected $request;
+
   /**
    * Constructs a Time cache plugin object.
    *
@@ -54,9 +62,13 @@ class Time extends CachePluginBase {
    *   The date formatter service.
    * @param \Drupal\Core\Render\RenderCacheInterface $render_cache
    *   The render cache service.
+   * @param \Symfony\Component\HttpFoundation\Request $request
+   *   The current request.
    */
-  public function __construct(array $configuration, $plugin_id, $plugin_definition, RendererInterface $renderer, RenderCacheInterface $render_cache, DateFormatter $date_formatter) {
+  public function __construct(array $configuration, $plugin_id, $plugin_definition, RendererInterface $renderer, RenderCacheInterface $render_cache, DateFormatter $date_formatter, Request $request) {
     $this->dateFormatter = $date_formatter;
+    $this->request = $request;
+
     parent::__construct($configuration, $plugin_id, $plugin_definition, $renderer, $render_cache);
   }
 
@@ -70,7 +82,8 @@ public static function create(ContainerInterface $container, array $configuratio
       $plugin_definition,
       $container->get('renderer'),
       $container->get('render_cache'),
-      $container->get('date.formatter')
+      $container->get('date.formatter'),
+      $container->get('request_stack')->getCurrentRequest()
     );
   }
 
@@ -174,4 +187,13 @@ protected function cacheSetExpire($type) {
     }
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  protected function getDefaultCacheMaxAge() {
+    // The max age, unless overridden by some other piece of the rendered code
+    // is determined by the output time setting.
+    return $this->cacheSetExpire('output');
+  }
+
 }

core/modules/views/src/Plugin/views/query/QueryPluginBase.php

@@ -7,6 +7,7 @@
 
 namespace Drupal\views\Plugin\views\query;
 
+use Drupal\Core\Cache\Cache;
 use Drupal\Core\Form\FormStateInterface;
 use Drupal\views\Plugin\CacheablePluginInterface;
 use Drupal\views\Plugin\views\PluginBase;
@@ -341,6 +342,13 @@ public function getCacheTags() {
     return [];
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function getCacheMaxAge() {
+    return Cache::PERMANENT;
+  }
+
 }
 
 /**

core/modules/views/src/Plugin/views/query/Sql.php

@@ -1552,16 +1552,45 @@ public function getCacheTags() {
     $tags = [];
     // Add cache tags for each row, if there is an entity associated with it.
     if (!$this->hasAggregate) {
-      foreach ($this->view->result as $row)  {
-        if ($row->_entity) {
-          $tags = Cache::mergeTags($row->_entity->getCacheTags(), $tags);
-        }
+      foreach ($this->getAllEntities() as $entity) {
+        $tags = Cache::mergeTags($entity->getCacheTags(), $tags);
       }
     }
 
     return $tags;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function getCacheMaxAge() {
+    $max_age = parent::getCacheMaxAge();
+    foreach ($this->getAllEntities() as $entity) {
+      $max_age = Cache::mergeMaxAges($max_age, $entity->getCacheMaxAge());
+    }
+
+    return $max_age;
+  }
+
+  /**
+   * Gets all the involved entities of the view.
+   *
+   * @return \Drupal\Core\Entity\EntityInterface[]
+   */
+  protected function getAllEntities() {
+    $entities = [];
+    foreach ($this->view->result as $row) {
+      if ($row->_entity) {
+        $entities[] = $row->_entity;
+      }
+      foreach ($row->_relationship_entities as $entity) {
+        $entities[] = $entity;
+      }
+    }
+
+    return $entities;
+  }
+
   public function addSignature(ViewExecutable $view) {
     $view->query->addField(NULL, "'" . $view->storage->id() . ':' . $view->current_display . "'", 'view_name');
   }

core/modules/views/src/Tests/Plugin/CacheWebTest.php

@@ -7,6 +7,7 @@
 
 namespace Drupal\views\Tests\Plugin;
 
+use Drupal\system\Tests\Cache\AssertPageCacheContextsAndTagsTrait;
 use Drupal\views\Views;
 
 /**
@@ -17,6 +18,8 @@
  */
 class CacheWebTest extends PluginTestBase {
 
+  use AssertPageCacheContextsAndTagsTrait;
+
   /**
    * Views used by this test.
    *
@@ -63,10 +66,18 @@ public function testCacheOutputOnPage() {
     $this->drupalGet('test-display');
     $this->assertResponse(200);
     $this->assertTrue(\Drupal::cache('render')->get($output_key));
+    $cache_tags = [
+      'config:user.role.anonymous',
+      'config:views.view.test_display',
+      'node_list',
+      'rendered'
+    ];
+    $this->assertCacheTags($cache_tags);
 
     $this->drupalGet('test-display');
     $this->assertResponse(200);
     $this->assertTrue(\Drupal::cache('render')->get($output_key));
+    $this->assertCacheTags($cache_tags);
   }
 
 }

core/modules/views/tests/src/Unit/Plugin/query/SqlTest.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Tests\views\Unit\Plugin\query\SqlTest.
+ */
+
+namespace Drupal\Tests\views\Unit\Plugin\query;
+
+use Drupal\Tests\UnitTestCase;
+use Drupal\views\Plugin\views\query\Sql;
+use Drupal\views\ResultRow;
+
+/**
+ * @coversDefaultClass \Drupal\views\Plugin\views\query\Sql
+ *
+ * @group views
+ */
+class SqlTest extends UnitTestCase {
+
+  /**
+   * @covers ::getCacheTags
+   * @covers ::getAllEntities
+   */
+  public function testGetCacheTags() {
+    $view = $this->prophesize('Drupal\views\ViewExecutable')->reveal();
+
+    $query = new Sql([], 'sql', []);
+    $query->view = $view;
+
+    $result = [];
+    $view->result = $result;
+
+    // Add a row with an entity.
+    $row = new ResultRow();
+    $prophecy = $this->prophesize('Drupal\Core\Entity\EntityInterface');
+    $prophecy->getCacheTags()->willReturn(['entity_test:123']);
+    $entity = $prophecy->reveal();
+    $row->_entity = $entity;
+
+    $result[] = $row;
+    $view->result = $result;
+
+    // Add a row with an entity and a relationship entity.
+    $row = new ResultRow();
+    $prophecy = $this->prophesize('Drupal\Core\Entity\EntityInterface');
+    $prophecy->getCacheTags()->willReturn(['entity_test:124']);
+    $entity = $prophecy->reveal();
+    $row->_entity = $entity;
+
+    $prophecy = $this->prophesize('Drupal\Core\Entity\EntityInterface');
+    $prophecy->getCacheTags()->willReturn(['entity_test:125']);
+    $entity = $prophecy->reveal();
+    $row->_relationship_entities[] = $entity;
+    $prophecy = $this->prophesize('Drupal\Core\Entity\EntityInterface');
+    $prophecy->getCacheTags()->willReturn(['entity_test:126']);
+    $entity = $prophecy->reveal();
+    $row->_relationship_entities[] = $entity;
+
+    $result[] = $row;
+    $view->result = $result;
+
+    $this->assertEquals(['entity_test:123', 'entity_test:124', 'entity_test:125', 'entity_test:126'], $query->getCacheTags());
+  }
+
+  /**
+   * @covers ::getCacheTags
+   * @covers ::getAllEntities
+   */
+  public function testGetCacheMaxAge() {
+    $view = $this->prophesize('Drupal\views\ViewExecutable')->reveal();
+
+    $query = new Sql([], 'sql', []);
+    $query->view = $view;
+
+    $view->result = [];
+
+    // Add a row with an entity.
+    $row = new ResultRow();
+    $prophecy = $this->prophesize('Drupal\Core\Entity\EntityInterface');
+    $prophecy->getCacheMaxAge()->willReturn(10);
+    $entity = $prophecy->reveal();
+
+    $row->_entity = $entity;
+    $view->result[] = $row;
+
+    // Add a row with an entity and a relationship entity.
+    $row = new ResultRow();
+    $prophecy = $this->prophesize('Drupal\Core\Entity\EntityInterface');
+    $prophecy->getCacheMaxAge()->willReturn(20);
+    $entity = $prophecy->reveal();
+    $row->_entity = $entity;
+
+    $prophecy = $this->prophesize('Drupal\Core\Entity\EntityInterface');
+    $prophecy->getCacheMaxAge()->willReturn(30);
+    $entity = $prophecy->reveal();
+    $row->_relationship_entities[] = $entity;
+    $prophecy = $this->prophesize('Drupal\Core\Entity\EntityInterface');
+    $prophecy->getCacheMaxAge()->willReturn(40);
+    $entity = $prophecy->reveal();
+    $row->_relationship_entities[] = $entity;
+
+    $this->assertEquals(10, $query->getCacheMaxAge());
+  }
+
+}