core/lib/Drupal/Core/Cache/Cache.php | 68 +++++++++++++++++++++- core/lib/Drupal/Core/Extension/ModuleInstaller.php | 4 ++ .../src/Plugin/views/argument_default/Node.php | 2 +- .../node/src/Plugin/views/filter/Access.php | 3 +- core/modules/simpletest/src/KernelTestBase.php | 4 ++ .../Cache/AssertPageCacheContextsAndTagsTrait.php | 15 +++-- .../modules/entity_test/entity_test.services.yml | 4 ++ .../src/Cache/EntityTestViewGrantsCacheContext.php | 33 +++++++++++ .../src/Plugin/views/argument_default/Tid.php | 2 +- .../src/Plugin/views/filter/TaxonomyIndexTid.php | 2 +- .../Plugin/views/argument_default/CurrentUser.php | 2 +- .../src/Plugin/views/argument_default/User.php | 2 +- .../user/src/Plugin/views/filter/Current.php | 2 +- .../views/src/Entity/Render/RendererBase.php | 17 +++++- core/modules/views/src/Entity/View.php | 4 +- .../views/src/Plugin/CacheablePluginInterface.php | 5 +- .../Plugin/views/argument/ArgumentPluginBase.php | 2 +- .../views/argument_default/QueryParameter.php | 2 +- .../src/Plugin/views/argument_default/Raw.php | 2 +- .../modules/views/src/Plugin/views/field/Field.php | 5 +- .../src/Plugin/views/filter/FilterPluginBase.php | 2 +- .../views/src/Plugin/views/sort/SortPluginBase.php | 2 +- core/modules/views/src/Tests/GlossaryTest.php | 2 +- .../views/src/Tests/RenderCacheIntegrationTest.php | 2 +- core/tests/Drupal/Tests/Core/Cache/CacheTest.php | 54 +++++++++++++++++ .../Tests/Core/Render/RendererBubblingTest.php | 5 ++ 26 files changed, 218 insertions(+), 29 deletions(-) diff --git a/core/lib/Drupal/Core/Cache/Cache.php b/core/lib/Drupal/Core/Cache/Cache.php index ff24324..d12d538 100644 --- a/core/lib/Drupal/Core/Cache/Cache.php +++ b/core/lib/Drupal/Core/Cache/Cache.php @@ -22,6 +22,13 @@ class Cache { const PERMANENT = CacheBackendInterface::CACHE_PERMANENT; /** + * The set of valid cache context tokens. + * + * @var string[] + */ + protected static $validContextTokens; + + /** * Merges arrays of cache contexts and removes duplicates. * * @param string[] … @@ -37,11 +44,70 @@ public static function mergeContexts() { $cache_contexts = array_merge($cache_contexts, $contexts); } $cache_contexts = array_unique($cache_contexts); + static::validateContexts($cache_contexts); sort($cache_contexts); return $cache_contexts; } /** + * Validates an array of cache contexts. + * + * Can be called before using cache tags in operations, to ensure validity. + * + * @param string[] $context_tokens + * An array of cache context tokens. + * + * @throws \LogicException + * + * @see \Drupal\Core\Cache\CacheContexts::parseTokens() + */ + public static function validateContexts(array $context_tokens) { + if (empty($context_tokens)) { + return; + } + + // The set of existing (thus valid) cache contexts is stored on the + // container; it's safe to statically cache this because it cannot change + // during the request. Initialize the set of valid context tokens with this. + if (!isset(static::$validContextTokens)) { + static::$validContextTokens = array_flip(\Drupal::getContainer()->getParameter('cache_contexts')); + } + + foreach ($context_tokens as $context_token) { + if (!is_string($context_token)) { + throw new \LogicException('Cache contexts must be strings, ' . gettype($context_token) . ' given.'); + } + + if (isset(static::$validContextTokens[$context_token])) { + continue; + } + + // If it's a valid context token, then the ID must be stored in the set + // of valid context tokens (since we initialized it with the list of cache + // context IDs using the container). In case of an invalid context token, + // throw an exception, otherwise cache it, including the parameter, to + // minimize the amount of work in future ::validateContexts() calls. + $context_id = $context_token; + if (strpos($context_id, ':') !== FALSE) { + $context_id = substr($context_id, 0, strpos($context_id, ':')); + } + if (isset(static::$validContextTokens[$context_id])) { + static::$validContextTokens[$context_token] = TRUE; + } + else { + throw new \LogicException('"' . $context_id . '" is not a valid cache context ID.'); + } + } + } + + /** + * Refreshes any static caches. + */ + public static function refresh() { + static::$validContextTokens = NULL; + } + + /** * Merges arrays of cache tags and removes duplicates. * * The cache tags array is returned in a format that is valid for @@ -62,10 +128,10 @@ public static function mergeTags() { $cache_tag_arrays = func_get_args(); $cache_tags = []; foreach ($cache_tag_arrays as $tags) { - static::validateTags($tags); $cache_tags = array_merge($cache_tags, $tags); } $cache_tags = array_unique($cache_tags); + static::validateTags($cache_tags); sort($cache_tags); return $cache_tags; } diff --git a/core/lib/Drupal/Core/Extension/ModuleInstaller.php b/core/lib/Drupal/Core/Extension/ModuleInstaller.php index 412a7b3..b4937c0 100644 --- a/core/lib/Drupal/Core/Extension/ModuleInstaller.php +++ b/core/lib/Drupal/Core/Extension/ModuleInstaller.php @@ -288,6 +288,10 @@ public function install(array $module_list, $enable_dependencies = TRUE) { // Allow the module to perform install tasks. $this->moduleHandler->invoke($module, 'install'); + // Refresh any static caches in the Cache class (e.g. to ensure it has + // the correct set of cache contexts). + Cache::refresh(); + // Record the fact that it was installed. \Drupal::logger('system')->info('%module module installed.', array('%module' => $module)); } diff --git a/core/modules/node/src/Plugin/views/argument_default/Node.php b/core/modules/node/src/Plugin/views/argument_default/Node.php index 4a61f03..e6d8343 100644 --- a/core/modules/node/src/Plugin/views/argument_default/Node.php +++ b/core/modules/node/src/Plugin/views/argument_default/Node.php @@ -83,7 +83,7 @@ public function isCacheable() { * {@inheritdoc} */ public function getCacheContexts() { - return ['cache.context.url']; + return ['url']; } } diff --git a/core/modules/node/src/Plugin/views/filter/Access.php b/core/modules/node/src/Plugin/views/filter/Access.php index 5fd6924..d56edcd 100644 --- a/core/modules/node/src/Plugin/views/filter/Access.php +++ b/core/modules/node/src/Plugin/views/filter/Access.php @@ -53,8 +53,7 @@ public function query() { public function getCacheContexts() { $contexts = parent::getCacheContexts(); - // Node access is potentially cacheable per user. - $contexts[] = 'cache.context.user'; + $contexts[] = 'node_view_grants'; return $contexts; } diff --git a/core/modules/simpletest/src/KernelTestBase.php b/core/modules/simpletest/src/KernelTestBase.php index 14b839b..8b806a6 100644 --- a/core/modules/simpletest/src/KernelTestBase.php +++ b/core/modules/simpletest/src/KernelTestBase.php @@ -9,6 +9,7 @@ use Drupal\Component\Utility\String; use Drupal\Component\Utility\Variable; +use Drupal\Core\Cache\Cache; use Drupal\Core\Database\Database; use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\Core\DrupalKernel; @@ -488,6 +489,9 @@ protected function enableModules(array $modules) { } $active_storage->write('core.extension', $extensions); + // Clear some static caches. + Cache::refresh(); + // Update the kernel to make their services available. $module_filenames = $module_handler->getModuleList(); $this->kernel->updateModules($module_filenames, $module_filenames); diff --git a/core/modules/system/src/Tests/Cache/AssertPageCacheContextsAndTagsTrait.php b/core/modules/system/src/Tests/Cache/AssertPageCacheContextsAndTagsTrait.php index 1523421..a9330ba 100644 --- a/core/modules/system/src/Tests/Cache/AssertPageCacheContextsAndTagsTrait.php +++ b/core/modules/system/src/Tests/Cache/AssertPageCacheContextsAndTagsTrait.php @@ -58,11 +58,13 @@ protected function assertPageCacheContextsAndTags(Url $url, array $expected_cont $actual_tags = $get_cache_header_values('X-Drupal-Cache-Tags'); $this->assertIdentical($actual_contexts, $expected_contexts); if ($actual_contexts !== $expected_contexts) { - debug(array_diff($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(array_diff($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))); } // Assert cache hit + expected cache contexts + tags. @@ -72,11 +74,13 @@ protected function assertPageCacheContextsAndTags(Url $url, array $expected_cont $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'HIT'); $this->assertIdentical($actual_contexts, $expected_contexts); if ($actual_contexts !== $expected_contexts) { - debug(array_diff($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(array_diff($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))); } // Assert page cache item + expected cache tags. @@ -86,7 +90,8 @@ protected function assertPageCacheContextsAndTags(Url $url, array $expected_cont sort($cache_entry->tags); $this->assertEqual($cache_entry->tags, $expected_tags); if ($cache_entry->tags !== $expected_tags) { - debug(array_diff($cache_entry->tags, $expected_tags)); + debug('Missing cache tags: ' . implode(',', array_diff($cache_entry->tags, $expected_tags))); + debug('Unwanted cache tags: ' . implode(',', array_diff($expected_tags, $cache_entry->tags))); } } diff --git a/core/modules/system/tests/modules/entity_test/entity_test.services.yml b/core/modules/system/tests/modules/entity_test/entity_test.services.yml index 8769fbc..75e1bf3 100644 --- a/core/modules/system/tests/modules/entity_test/entity_test.services.yml +++ b/core/modules/system/tests/modules/entity_test/entity_test.services.yml @@ -4,3 +4,7 @@ services: arguments: ['@state'] tags: - { name: event_subscriber } + cache_context.entity_test_view_grants: + class: Drupal\entity_test\Cache\EntityTestViewGrantsCacheContext + tags: + - { name: cache.context } diff --git a/core/modules/system/tests/modules/entity_test/src/Cache/EntityTestViewGrantsCacheContext.php b/core/modules/system/tests/modules/entity_test/src/Cache/EntityTestViewGrantsCacheContext.php new file mode 100644 index 0000000..b93f404 --- /dev/null +++ b/core/modules/system/tests/modules/entity_test/src/Cache/EntityTestViewGrantsCacheContext.php @@ -0,0 +1,33 @@ +getDisplay()->calculateCacheMetadata(); // Always include at least the language context as there will be most // probable translatable strings in the view output. - $display['cache_metadata']['contexts'][] = 'language'; - $display['cache_metadata']['contexts'] = array_unique($display['cache_metadata']['contexts']); + $display['cache_metadata']['contexts'] = Cache::mergeContexts($display['cache_metadata']['contexts'], ['language']); } // Restore the previous active display. $executable->setDisplay($current_display); diff --git a/core/modules/views/src/Plugin/CacheablePluginInterface.php b/core/modules/views/src/Plugin/CacheablePluginInterface.php index a8a72a9..cca8c08 100644 --- a/core/modules/views/src/Plugin/CacheablePluginInterface.php +++ b/core/modules/views/src/Plugin/CacheablePluginInterface.php @@ -8,7 +8,10 @@ namespace Drupal\views\Plugin; /** - * Provides information whether and how the specific Views plugin is cacheable. + * Provides caching information about the result cacheability of views plugins. + * + * For caaching on the render level we rely on cache bubbling of the cache + * contexts. */ interface CacheablePluginInterface { diff --git a/core/modules/views/src/Plugin/views/argument/ArgumentPluginBase.php b/core/modules/views/src/Plugin/views/argument/ArgumentPluginBase.php index 069276b..4e5469b 100644 --- a/core/modules/views/src/Plugin/views/argument/ArgumentPluginBase.php +++ b/core/modules/views/src/Plugin/views/argument/ArgumentPluginBase.php @@ -1208,7 +1208,7 @@ public function getCacheContexts() { // By definition arguments depends on the URL. // @todo Once contexts are properly injected into block views we could pull // the information from there. - $contexts[] = 'cache.context.url'; + $contexts[] = 'url'; // Asks all subplugins (argument defaults, argument validator and styles). if (($plugin = $this->getPlugin('argument_default')) && $plugin instanceof CacheablePluginInterface) { diff --git a/core/modules/views/src/Plugin/views/argument_default/QueryParameter.php b/core/modules/views/src/Plugin/views/argument_default/QueryParameter.php index 337b722..feccd21 100644 --- a/core/modules/views/src/Plugin/views/argument_default/QueryParameter.php +++ b/core/modules/views/src/Plugin/views/argument_default/QueryParameter.php @@ -95,7 +95,7 @@ public function isCacheable() { * {@inheritdoc} */ public function getCacheContexts() { - return ['cache.context.url']; + return ['url']; } } diff --git a/core/modules/views/src/Plugin/views/argument_default/Raw.php b/core/modules/views/src/Plugin/views/argument_default/Raw.php index 4239c50..40bddf3 100644 --- a/core/modules/views/src/Plugin/views/argument_default/Raw.php +++ b/core/modules/views/src/Plugin/views/argument_default/Raw.php @@ -124,7 +124,7 @@ public function isCacheable() { * {@inheritdoc} */ public function getCacheContexts() { - return ['cache.context.url']; + return ['url']; } } diff --git a/core/modules/views/src/Plugin/views/field/Field.php b/core/modules/views/src/Plugin/views/field/Field.php index 46d3f5f..e34d4a1 100644 --- a/core/modules/views/src/Plugin/views/field/Field.php +++ b/core/modules/views/src/Plugin/views/field/Field.php @@ -961,10 +961,7 @@ public function isCacheable() { * {@inheritdoc} */ public function getCacheContexts() { - // @todo what to do about field access? - $contexts = []; - - $contexts[] = 'user'; + $contexts = $this->getEntityTranslationRenderer()->getCacheContexts(); return $contexts; } diff --git a/core/modules/views/src/Plugin/views/filter/FilterPluginBase.php b/core/modules/views/src/Plugin/views/filter/FilterPluginBase.php index b5f8bf8..9abf63e 100644 --- a/core/modules/views/src/Plugin/views/filter/FilterPluginBase.php +++ b/core/modules/views/src/Plugin/views/filter/FilterPluginBase.php @@ -1477,7 +1477,7 @@ public function getCacheContexts() { // input from GET parameters, which are part of the URL. Hence a view with // an exposed filter is cacheable per URL. if ($this->isExposed()) { - $cache_contexts[] = 'cache.context.url'; + $cache_contexts[] = 'url'; } return $cache_contexts; } diff --git a/core/modules/views/src/Plugin/views/sort/SortPluginBase.php b/core/modules/views/src/Plugin/views/sort/SortPluginBase.php index 45b86be..8fe2b43 100644 --- a/core/modules/views/src/Plugin/views/sort/SortPluginBase.php +++ b/core/modules/views/src/Plugin/views/sort/SortPluginBase.php @@ -241,7 +241,7 @@ public function getCacheContexts() { $cache_contexts = []; // Exposed sorts use GET parameters, so it depends on the current URL. if ($this->isExposed()) { - $cache_contexts[] = 'cache.context.url'; + $cache_contexts[] = 'url'; } return $cache_contexts; } diff --git a/core/modules/views/src/Tests/GlossaryTest.php b/core/modules/views/src/Tests/GlossaryTest.php index fd97978..65f7bd6 100644 --- a/core/modules/views/src/Tests/GlossaryTest.php +++ b/core/modules/views/src/Tests/GlossaryTest.php @@ -87,7 +87,7 @@ public function testGlossaryView() { // Verify cache tags. $this->enablePageCaching(); - $this->assertPageCacheContextsAndTags(Url::fromRoute('view.glossary.page_1'), ['cache.context.url', 'node_view_grants', 'language', 'user'], [ + $this->assertPageCacheContextsAndTags(Url::fromRoute('view.glossary.page_1'), ['node_view_grants', 'language', 'url'], [ 'config:views.view.glossary', 'node:' . $nodes_by_char['a'][0]->id(), 'node:' . $nodes_by_char['a'][1]->id(), 'node:' . $nodes_by_char['a'][2]->id(), 'node_list', diff --git a/core/modules/views/src/Tests/RenderCacheIntegrationTest.php b/core/modules/views/src/Tests/RenderCacheIntegrationTest.php index da94ab4..851d736 100644 --- a/core/modules/views/src/Tests/RenderCacheIntegrationTest.php +++ b/core/modules/views/src/Tests/RenderCacheIntegrationTest.php @@ -221,7 +221,7 @@ public function testViewAddCacheMetadata() { $view = View::load('test_display'); $view->save(); - $this->assertEqual(['node_view_grants', 'user', 'language'], $view->getDisplay('default')['cache_metadata']['contexts']); + $this->assertEqual(['language', 'node_view_grants'], $view->getDisplay('default')['cache_metadata']['contexts']); } } diff --git a/core/tests/Drupal/Tests/Core/Cache/CacheTest.php b/core/tests/Drupal/Tests/Core/Cache/CacheTest.php index 261aa80..b601a7d 100644 --- a/core/tests/Drupal/Tests/Core/Cache/CacheTest.php +++ b/core/tests/Drupal/Tests/Core/Cache/CacheTest.php @@ -9,6 +9,7 @@ use Drupal\Core\Cache\Cache; use Drupal\Tests\UnitTestCase; +use Symfony\Component\DependencyInjection\ContainerBuilder; /** * @coversDefaultClass \Drupal\Core\Cache\Cache @@ -27,6 +28,7 @@ public function validateTagsProvider() { [['foo'], FALSE], [['foo', 'bar'], FALSE], [['foo', 'bar', 'llama:2001988', 'baz', 'llama:14031991'], FALSE], + // Invalid. [[FALSE], 'Cache tags must be strings, boolean given.'], [[TRUE], 'Cache tags must be strings, boolean given.'], [['foo', FALSE], 'Cache tags must be strings, boolean given.'], @@ -151,4 +153,56 @@ public function testBuildTags($prefix, array $suffixes, array $expected, $glue = $this->assertEquals($expected, Cache::buildTags($prefix, $suffixes, $glue)); } + /** + * Provides a list of cache context token arrays. + * + * @return array + */ + public function validateContextsProvider() { + return [ + [[], FALSE], + [['foo'], FALSE], + [['foo', 'foo.bar'], FALSE], + [['foo', 'baz:llama'], FALSE], + // Invalid. + [[FALSE], 'Cache contexts must be strings, boolean given.'], + [[TRUE], 'Cache contexts must be strings, boolean given.'], + [['foo', FALSE], 'Cache contexts must be strings, boolean given.'], + [[NULL], 'Cache contexts must be strings, NULL given.'], + [['foo', NULL], 'Cache contexts must be strings, NULL given.'], + [[1337], 'Cache contexts must be strings, integer given.'], + [['foo', 1337], 'Cache contexts must be strings, integer given.'], + [[3.14], 'Cache contexts must be strings, double given.'], + [['foo', 3.14], 'Cache contexts must be strings, double given.'], + [[[]], 'Cache contexts must be strings, array given.'], + [['foo', []], 'Cache contexts must be strings, array given.'], + [['foo', ['bar']], 'Cache contexts must be strings, array given.'], + [[new \stdClass()], 'Cache contexts must be strings, object given.'], + [['foo', new \stdClass()], 'Cache contexts must be strings, object given.'], + // Non-existing. + [['foo.bar', 'qux'], '"qux" is not a valid cache context ID.'], + [['qux', 'baz'], '"qux" is not a valid cache context ID.'], + ]; + } + + /** + * @covers ::validateContexts + * + * @dataProvider validateContextsProvider + */ + public function testValidateContexts(array $contexts, $expected_exception_message) { + $container = new ContainerBuilder(); + $container->setParameter('cache_contexts', [ + 'foo', + 'foo.bar', + 'baz', + ]); + \Drupal::setContainer($container); + if ($expected_exception_message !== FALSE) { + $this->setExpectedException('LogicException', $expected_exception_message); + } + // If it doesn't throw an exception, validateContexts() returns NULL. + $this->assertNull(Cache::validateContexts($contexts)); + } + } diff --git a/core/tests/Drupal/Tests/Core/Render/RendererBubblingTest.php b/core/tests/Drupal/Tests/Core/Render/RendererBubblingTest.php index 929a307..d0108ce 100644 --- a/core/tests/Drupal/Tests/Core/Render/RendererBubblingTest.php +++ b/core/tests/Drupal/Tests/Core/Render/RendererBubblingTest.php @@ -23,6 +23,11 @@ class RendererBubblingTest extends RendererTestBase { protected function setUp() { parent::setUp(); + // Ensure the container lists the valid cache contexts. + $container = \Drupal::getContainer(); + $container->setParameter('cache_contexts', ['foo', 'bar', 'baz', 'user.roles', 'child.cache_context']); + \Drupal::setContainer($container); + $this->setUpRequest(); $this->setupMemoryCache(); }