Commit f7324499 authored by catch's avatar catch

Issue #2451679 by Wim Leers, dawehner: Validate cache contexts (+ cache...

Issue #2451679 by Wim Leers, dawehner: Validate cache contexts (+ cache contexts in some views plugins wrong)
parent c34478f5
......@@ -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;
}
......
......@@ -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));
}
}
}
}
......@@ -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());
......
......@@ -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
......
......@@ -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;
}
......
......@@ -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)));
}
}
......
......@@ -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 }
<?php
/**
* @file
* Contains \Drupal\entity_test\Cache\EntityTestViewViewGrantsCacheContext.
*/
namespace Drupal\entity_test\Cache;
use Drupal\Core\Cache\CacheContextInterface;
/**
* Defines the entity_test view grants cache context service.
*
* @see \Drupal\node\Cache\NodeAccessViewGrantsCacheContext
*/
class EntityTestViewViewGrantsCacheContext implements CacheContextInterface {
/**
* {@inheritdoc}
*/
public static function getLabel() {
return t("Entity test view grants");
}
/**
* {@inheritdoc}
*/
public function getContext() {
return hash('sha256', REQUEST_TIME);
}
}
......@@ -360,7 +360,7 @@ public function getCacheContexts() {
// The result potentially depends on term access and so is just cacheable
// per user.
// @todo https://www.drupal.org/node/2352175
$contexts[] = 'cache.context.user';
$contexts[] = 'user';
return $contexts;
}
......
......@@ -37,7 +37,7 @@ public function isCacheable() {
* {@inheritdoc}
*/
public function getCacheContexts() {
return ['cache.context.user'];
return ['user'];
}
}
......@@ -54,7 +54,7 @@ public function getCacheContexts() {
$contexts = parent::getCacheContexts();
// This filter depends on the current user.
$contexts[] = 'cache.context.user';
$contexts[] = 'user';
return $contexts;
}
......
......@@ -8,7 +8,9 @@
namespace Drupal\views\Entity\Render;
use Drupal\Core\Entity\EntityTypeInterface;
use Drupal\Core\Language\LanguageInterface;
use Drupal\Core\Language\LanguageManagerInterface;
use Drupal\views\Plugin\CacheablePluginInterface;
use Drupal\views\Plugin\views\query\QueryPluginBase;
use Drupal\views\ResultRow;
use Drupal\views\ViewExecutable;
......@@ -16,7 +18,7 @@
/**
* Defines a base class for entity row renderers.
*/
abstract class RendererBase {
abstract class RendererBase implements CacheablePluginInterface {
/**
* The view executable wrapping the view storage entity.
......@@ -62,6 +64,20 @@ public function __construct(ViewExecutable $view, LanguageManagerInterface $lang
$this->entityType = $entity_type;
}
/**
* {@inheritdoc}
*/
public function isCacheable() {
return TRUE;
}
/**
* {@inheritdoc}
*/
public function getCacheContexts() {
return ['languages'];
}
/**
* Returns the language code associated to the given row.
*
......
......@@ -8,7 +8,9 @@
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 caching on the render level, we rely on bubbling of the cache contexts.
*/
interface CacheablePluginInterface {
......
......@@ -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;
}
......
......@@ -9,6 +9,7 @@
use Drupal\comment\Tests\CommentTestTrait;
use Drupal\views\Tests\ViewTestBase;
use Drupal\views\Tests\ViewTestData;
use Drupal\views\Views;
/**
......@@ -34,6 +35,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.
*/
......@@ -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();
......
......@@ -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' => [
......
......@@ -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',
......
......@@ -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']);
}
}
......@@ -33,11 +33,23 @@ abstract class ViewUnitTestBase extends KernelTestBase {
*/
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'));
}
/**
......
......@@ -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));
}
}
/**
......
......@@ -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.'],
......
......@@ -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);
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment