core/lib/Drupal/Core/Cache/Cache.php | 3 +- core/lib/Drupal/Core/Cache/CacheContexts.php | 50 ++++++++++++++++++++++ .../Views/EntityReferenceRelationshipTest.php | 4 +- .../views.view.test_entity_reference_view.yml | 2 +- .../node/src/Plugin/views/filter/Access.php | 3 +- .../Cache/AssertPageCacheContextsAndTagsTrait.php | 15 ++++--- .../modules/entity_test/entity_test.services.yml | 4 ++ .../src/Cache/EntityTestViewGrantsCacheContext.php | 33 ++++++++++++++ .../src/Plugin/views/filter/TaxonomyIndexTid.php | 2 +- .../Plugin/views/argument_default/CurrentUser.php | 2 +- .../user/src/Plugin/views/filter/Current.php | 2 +- .../views/src/Entity/Render/RendererBase.php | 18 +++++++- .../views/src/Plugin/CacheablePluginInterface.php | 4 +- .../modules/views/src/Plugin/views/field/Field.php | 5 +-- .../views/src/Tests/Entity/FieldEntityTest.php | 15 ++++++- .../Tests/Entity/ViewEntityDependenciesTest.php | 18 +++++--- core/modules/views/src/Tests/GlossaryTest.php | 2 +- .../views/src/Tests/RenderCacheIntegrationTest.php | 2 +- core/modules/views/src/Tests/ViewUnitTestBase.php | 18 ++++++-- .../Drupal/Tests/Core/Cache/CacheContextsTest.php | 48 +++++++++++++++++++++ core/tests/Drupal/Tests/Core/Cache/CacheTest.php | 2 + .../Drupal/Tests/Core/Render/RendererTestBase.php | 1 + 22 files changed, 218 insertions(+), 35 deletions(-) diff --git a/core/lib/Drupal/Core/Cache/Cache.php b/core/lib/Drupal/Core/Cache/Cache.php index ff24324..5541a68 100644 --- a/core/lib/Drupal/Core/Cache/Cache.php +++ b/core/lib/Drupal/Core/Cache/Cache.php @@ -37,6 +37,7 @@ public static function mergeContexts() { $cache_contexts = array_merge($cache_contexts, $contexts); } $cache_contexts = array_unique($cache_contexts); + \Drupal::service('cache_contexts')->validateTokens($cache_contexts); sort($cache_contexts); return $cache_contexts; } @@ -62,10 +63,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/Cache/CacheContexts.php b/core/lib/Drupal/Core/Cache/CacheContexts.php index 781f24a..5f8202f 100644 --- a/core/lib/Drupal/Core/Cache/CacheContexts.php +++ b/core/lib/Drupal/Core/Cache/CacheContexts.php @@ -221,4 +221,54 @@ public static function parseTokens(array $context_tokens) { return $contexts_with_parameters; } + /** + * Validates an array of cache context tokens. + * + * Can be called before using cache contexts in operations, to check validity. + * + * @param string[] $context_tokens + * An array of cache context tokens. + * + * @throws \LogicException + * + * @see \Drupal\Core\Cache\CacheContexts::parseTokens() + */ + public function validateTokens(array $context_tokens = []) { + if (empty($context_tokens)) { + return; + } + + // Initialize the set of valid context tokens with the container's contexts. + if (!isset($this->validContextTokens)) { + $this->validContextTokens = array_flip($this->contexts); + } + + foreach ($context_tokens as $context_token) { + if (!is_string($context_token)) { + throw new \LogicException(sprintf('Cache contexts must be strings, %s given.', gettype($context_token))); + } + + if (isset($this->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; + $colon_pos = strpos($context_id, ':'); + if ($colon_pos !== FALSE) { + $context_id = substr($context_id, 0, $colon_pos); + } + if (isset($this->validContextTokens[$context_id])) { + $this->validContextTokens[$context_token] = TRUE; + } + else { + throw new \LogicException(sprintf('"%s" is not a valid cache context ID.', $context_id)); + } + } + } + } diff --git a/core/modules/entity_reference/src/Tests/Views/EntityReferenceRelationshipTest.php b/core/modules/entity_reference/src/Tests/Views/EntityReferenceRelationshipTest.php index 9ae79a7..f2a803d 100644 --- a/core/modules/entity_reference/src/Tests/Views/EntityReferenceRelationshipTest.php +++ b/core/modules/entity_reference/src/Tests/Views/EntityReferenceRelationshipTest.php @@ -52,8 +52,6 @@ protected function setUp() { $this->installEntitySchema('user'); $this->installEntitySchema('entity_test'); - ViewTestData::createTestViews(get_class($this), array('entity_reference_test_views')); - $field_storage = FieldStorageConfig::create(array( 'entity_type' => 'entity_test', 'field_name' => 'field_test', @@ -76,6 +74,8 @@ protected function setUp() { )); $field->save(); + ViewTestData::createTestViews(get_class($this), array('entity_reference_test_views')); + // Create some test entities which link each other. $entity_storage= \Drupal::entityManager()->getStorage('entity_test'); $referenced_entity = $entity_storage->create(array()); diff --git a/core/modules/entity_reference/tests/modules/entity_reference_test_views/test_views/views.view.test_entity_reference_view.yml b/core/modules/entity_reference/tests/modules/entity_reference_test_views/test_views/views.view.test_entity_reference_view.yml index c60ede7..90e4d31 100644 --- a/core/modules/entity_reference/tests/modules/entity_reference_test_views/test_views/views.view.test_entity_reference_view.yml +++ b/core/modules/entity_reference/tests/modules/entity_reference_test_views/test_views/views.view.test_entity_reference_view.yml @@ -67,7 +67,7 @@ display: test_relationship: id: reverse_field_test table: entity_test - field: reverse_field_test + field: reverse__entity_test__field_test relationship: none plugin_id: standard display_plugin: embed diff --git a/core/modules/node/src/Plugin/views/filter/Access.php b/core/modules/node/src/Plugin/views/filter/Access.php index 5fd6924..5497a91 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[] = 'user.node_grants:view'; return $contexts; } 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 @@ +getEntityTranslationRenderer()->getCacheContexts(); return $contexts; } diff --git a/core/modules/views/src/Tests/Entity/FieldEntityTest.php b/core/modules/views/src/Tests/Entity/FieldEntityTest.php index 37e27c0..770f7f0 100644 --- a/core/modules/views/src/Tests/Entity/FieldEntityTest.php +++ b/core/modules/views/src/Tests/Entity/FieldEntityTest.php @@ -9,6 +9,7 @@ use Drupal\comment\Tests\CommentTestTrait; use Drupal\views\Tests\ViewTestBase; +use Drupal\views\Tests\ViewTestData; use Drupal\views\Views; /** @@ -35,6 +36,18 @@ class FieldEntityTest extends ViewTestBase { public static $modules = array('node', 'comment'); /** + * {@inheritdoc} + */ + protected function setUp($import_test_views = TRUE) { + parent::setUp(FALSE); + + $this->drupalCreateContentType(array('type' => 'page')); + $this->addDefaultCommentField('node', 'page'); + + ViewTestData::createTestViews(get_class($this), array('views_test_config')); + } + + /** * Tests the getEntity method. */ public function testGetEntity() { @@ -43,8 +56,6 @@ public function testGetEntity() { $account = entity_create('user', array('name' => $this->randomMachineName(), 'bundle' => 'user')); $account->save(); - $this->drupalCreateContentType(array('type' => 'page')); - $this->addDefaultCommentField('node', 'page'); $node = entity_create('node', array('uid' => $account->id(), 'type' => 'page')); $node->save(); diff --git a/core/modules/views/src/Tests/Entity/ViewEntityDependenciesTest.php b/core/modules/views/src/Tests/Entity/ViewEntityDependenciesTest.php index d96f666..8805214 100644 --- a/core/modules/views/src/Tests/Entity/ViewEntityDependenciesTest.php +++ b/core/modules/views/src/Tests/Entity/ViewEntityDependenciesTest.php @@ -9,6 +9,7 @@ use Drupal\Component\Utility\Unicode; use Drupal\field\Entity\FieldStorageConfig; +use Drupal\views\Tests\ViewTestData; use Drupal\views\Tests\ViewUnitTestBase; use Drupal\views\Views; @@ -37,16 +38,11 @@ class ViewEntityDependenciesTest extends ViewUnitTestBase { * {@inheritdoc} */ protected function setUp() { - parent::setUp(); + parent::setUp(FALSE); + // Install the necessary dependencies for node type creation to work. $this->installEntitySchema('node'); $this->installConfig(array('field', 'node')); - } - - /** - * Tests the calculateDependencies method. - */ - public function testCalculateDependencies() { $comment_type = entity_create('comment_type', array( 'id' => 'comment', @@ -55,6 +51,7 @@ public function testCalculateDependencies() { 'target_entity_type_id' => 'node', )); $comment_type->save(); + $content_type = entity_create('node_type', array( 'type' => $this->randomMachineName(), 'name' => $this->randomString(), @@ -82,6 +79,13 @@ public function testCalculateDependencies() { 'settings' => array('display_summary' => TRUE), ))->save(); + ViewTestData::createTestViews(get_class($this), array('views_test_config')); + } + + /** + * Tests the calculateDependencies method. + */ + public function testCalculateDependencies() { $expected = []; $expected['test_field_get_entity'] = [ 'module' => [ diff --git a/core/modules/views/src/Tests/GlossaryTest.php b/core/modules/views/src/Tests/GlossaryTest.php index 1df4701..954f7ed 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'), ['languages', 'url', 'user'], [ + $this->assertPageCacheContextsAndTags(Url::fromRoute('view.glossary.page_1'), ['languages', 'url', 'user.node_grants:view'], [ '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 4a9276f..ad2621a 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(['languages', 'user', 'user.node_grants:view'], $view->getDisplay('default')['cache_metadata']['contexts']); + $this->assertEqual(['languages', 'user.node_grants:view'], $view->getDisplay('default')['cache_metadata']['contexts']); } } diff --git a/core/modules/views/src/Tests/ViewUnitTestBase.php b/core/modules/views/src/Tests/ViewUnitTestBase.php index 2de7f2b..eaa2694 100644 --- a/core/modules/views/src/Tests/ViewUnitTestBase.php +++ b/core/modules/views/src/Tests/ViewUnitTestBase.php @@ -33,11 +33,23 @@ */ public static $modules = array('system', 'views', 'views_test_config', 'views_test_data'); - protected function setUp() { + /** + * {@inheritdoc} + * + * @param bool $import_test_views + * Should the views specififed on the test class be imported. If you need + * to setup some additional stuff, like fields, you need to call false and + * then call createTestViews for your own. + */ + protected function setUp($import_test_views = TRUE) { parent::setUp(); $this->installSchema('system', array('router', 'sequences')); $this->setUpFixtures(); + + if ($import_test_views) { + ViewTestData::createTestViews(get_class($this), array('views_test_config')); + } } /** @@ -46,7 +58,7 @@ protected function setUp() { * Because the schema of views_test_data.module is dependent on the test * using it, it cannot be enabled normally. */ - protected function setUpFixtures() { + protected function setUpFixtures($import_test_views = TRUE) { // First install the system module. Many Views have Page displays have menu // links, and for those to work, the system menus must already be present. $this->installConfig(array('system')); @@ -70,8 +82,6 @@ protected function setUpFixtures() { $query->values($record); } $query->execute(); - - ViewTestData::createTestViews(get_class($this), array('views_test_config')); } /** diff --git a/core/tests/Drupal/Tests/Core/Cache/CacheContextsTest.php b/core/tests/Drupal/Tests/Core/Cache/CacheContextsTest.php index a81f52d..76251a1 100644 --- a/core/tests/Drupal/Tests/Core/Cache/CacheContextsTest.php +++ b/core/tests/Drupal/Tests/Core/Cache/CacheContextsTest.php @@ -10,6 +10,7 @@ use Drupal\Core\Cache\CacheContexts; use Drupal\Core\Cache\CacheContextInterface; use Drupal\Core\Cache\CalculatedCacheContextInterface; +use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\Tests\UnitTestCase; use Symfony\Component\DependencyInjection\Container; @@ -165,6 +166,53 @@ protected function getMockContainer() { return $container; } + /** + * Provides a list of cache context token arrays. + * + * @return array + */ + public function validateTokensProvider() { + 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 ::validateTokens + * + * @dataProvider validateTokensProvider + */ + public function testValidateContexts(array $contexts, $expected_exception_message) { + $container = new ContainerBuilder(); + $cache_contexts = new CacheContexts($container, ['foo', 'foo.bar', 'baz']); + if ($expected_exception_message !== FALSE) { + $this->setExpectedException('LogicException', $expected_exception_message); + } + // If it doesn't throw an exception, validateTokens() returns NULL. + $this->assertNull($cache_contexts->validateTokens($contexts)); + } + } /** diff --git a/core/tests/Drupal/Tests/Core/Cache/CacheTest.php b/core/tests/Drupal/Tests/Core/Cache/CacheTest.php index 261aa80..c2e61a1 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.'], diff --git a/core/tests/Drupal/Tests/Core/Render/RendererTestBase.php b/core/tests/Drupal/Tests/Core/Render/RendererTestBase.php index b4320c9..15a3b05 100644 --- a/core/tests/Drupal/Tests/Core/Render/RendererTestBase.php +++ b/core/tests/Drupal/Tests/Core/Render/RendererTestBase.php @@ -86,6 +86,7 @@ protected function setUp() { $this->renderer = new Renderer($this->controllerResolver, $this->themeManager, $this->elementInfo, $this->requestStack, $this->cacheFactory, $this->cacheContexts); $container = new ContainerBuilder(); + $container->set('cache_contexts', $this->cacheContexts); $container->set('renderer', $this->renderer); \Drupal::setContainer($container); }