From d04b472386a0882808e2629ace92248aecb7df97 Mon Sep 17 00:00:00 2001 From: Alex Pott Date: Wed, 6 May 2015 16:42:12 -0700 Subject: [PATCH] Issue #2477157 by dawehner, Wim Leers, Fabianx, aneek, catch: rest_export Views display plugin does not set necessary cache metadata --- .../src/Plugin/views/display/RestExport.php | 42 ++++++- .../src/Tests/Views/StyleSerializerTest.php | 20 ++++ .../rest/tests/src/Unit/CollectRoutesTest.php | 1 + .../AssertPageCacheContextsAndTagsTrait.php | 101 +++++++++++------ .../Plugin/views/cache/CachePluginBase.php | 19 ++++ .../views/src/Plugin/views/cache/Tag.php | 9 ++ .../views/src/Plugin/views/cache/Time.php | 26 ++++- .../Plugin/views/query/QueryPluginBase.php | 8 ++ .../views/src/Plugin/views/query/Sql.php | 37 +++++- .../views/src/Tests/Plugin/CacheWebTest.php | 11 ++ .../tests/src/Unit/Plugin/query/SqlTest.php | 106 ++++++++++++++++++ 11 files changed, 335 insertions(+), 45 deletions(-) create mode 100644 core/modules/views/tests/src/Unit/Plugin/query/SqlTest.php diff --git a/core/modules/rest/src/Plugin/views/display/RestExport.php b/core/modules/rest/src/Plugin/views/display/RestExport.php index 1eba5a9909..8af1a75592 100644 --- a/core/modules/rest/src/Plugin/views/display/RestExport.php +++ b/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'] = ''; } + // 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; } diff --git a/core/modules/rest/src/Tests/Views/StyleSerializerTest.php b/core/modules/rest/src/Tests/Views/StyleSerializerTest.php index 12eb653763..137e8f0079 100644 --- a/core/modules/rest/src/Tests/Views/StyleSerializerTest.php +++ b/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); } /** diff --git a/core/modules/rest/tests/src/Unit/CollectRoutesTest.php b/core/modules/rest/tests/src/Unit/CollectRoutesTest.php index 7a2cbae2d9..c49ff8734e 100644 --- a/core/modules/rest/tests/src/Unit/CollectRoutesTest.php +++ b/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); diff --git a/core/modules/system/src/Tests/Cache/AssertPageCacheContextsAndTagsTrait.php b/core/modules/system/src/Tests/Cache/AssertPageCacheContextsAndTagsTrait.php index 43e24ec46a..250e43aece 100644 --- a/core/modules/system/src/Tests/Cache/AssertPageCacheContextsAndTagsTrait.php +++ b/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)); + } + } diff --git a/core/modules/views/src/Plugin/views/cache/CachePluginBase.php b/core/modules/views/src/Plugin/views/cache/CachePluginBase.php index 224a964fe4..a92955789f 100644 --- a/core/modules/views/src/Plugin/views/cache/CachePluginBase.php +++ b/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. * diff --git a/core/modules/views/src/Plugin/views/cache/Tag.php b/core/modules/views/src/Plugin/views/cache/Tag.php index 144709dfa7..87ac8bae29 100644 --- a/core/modules/views/src/Plugin/views/cache/Tag.php +++ b/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; + } + } diff --git a/core/modules/views/src/Plugin/views/cache/Time.php b/core/modules/views/src/Plugin/views/cache/Time.php index 1aff4f0ba9..37c6cddc2c 100644 --- a/core/modules/views/src/Plugin/views/cache/Time.php +++ b/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'); + } + } diff --git a/core/modules/views/src/Plugin/views/query/QueryPluginBase.php b/core/modules/views/src/Plugin/views/query/QueryPluginBase.php index 2b2b4a2d1c..857c0c7cb4 100644 --- a/core/modules/views/src/Plugin/views/query/QueryPluginBase.php +++ b/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; + } + } /** diff --git a/core/modules/views/src/Plugin/views/query/Sql.php b/core/modules/views/src/Plugin/views/query/Sql.php index 660caf12f5..e31297052c 100644 --- a/core/modules/views/src/Plugin/views/query/Sql.php +++ b/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'); } diff --git a/core/modules/views/src/Tests/Plugin/CacheWebTest.php b/core/modules/views/src/Tests/Plugin/CacheWebTest.php index 3c825a71bf..676faa3524 100644 --- a/core/modules/views/src/Tests/Plugin/CacheWebTest.php +++ b/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); } } diff --git a/core/modules/views/tests/src/Unit/Plugin/query/SqlTest.php b/core/modules/views/tests/src/Unit/Plugin/query/SqlTest.php new file mode 100644 index 0000000000..feb3ce2332 --- /dev/null +++ b/core/modules/views/tests/src/Unit/Plugin/query/SqlTest.php @@ -0,0 +1,106 @@ +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()); + } + +} -- GitLab