core/lib/Drupal/Core/Cache/ApcuBackend.php | 3 +- core/lib/Drupal/Core/Cache/Cache.php | 32 +++++++++++-- core/lib/Drupal/Core/Cache/DatabaseBackend.php | 10 +++- core/lib/Drupal/Core/Cache/MemoryBackend.php | 6 ++- core/lib/Drupal/Core/Cache/PhpBackend.php | 3 +- core/lib/Drupal/Core/Menu/MenuTreeStorage.php | 10 +--- .../Cache/GenericCacheBackendUnitTestBase.php | 23 +++++++++ .../toolbar/src/Tests/ToolbarAdminMenuTest.php | 2 +- core/modules/user/src/PermissionsHash.php | 5 +- core/tests/Drupal/Tests/Core/Cache/CacheTest.php | 54 ++++++++++++++++++++++ 10 files changed, 125 insertions(+), 23 deletions(-) diff --git a/core/lib/Drupal/Core/Cache/ApcuBackend.php b/core/lib/Drupal/Core/Cache/ApcuBackend.php index 199a134..50fb0fd 100644 --- a/core/lib/Drupal/Core/Cache/ApcuBackend.php +++ b/core/lib/Drupal/Core/Cache/ApcuBackend.php @@ -189,7 +189,8 @@ protected function prepareItem($cache, $allow_invalid) { * {@inheritdoc} */ public function set($cid, $data, $expire = CacheBackendInterface::CACHE_PERMANENT, array $tags = array()) { - $tags = Cache::mergeTags($tags, []); + Cache::validateTags($tags); + $tags = array_unique($tags); $cache = new \stdClass(); $cache->cid = $cid; $cache->created = round(microtime(TRUE), 3); diff --git a/core/lib/Drupal/Core/Cache/Cache.php b/core/lib/Drupal/Core/Cache/Cache.php index a29ae57..ddf444e 100644 --- a/core/lib/Drupal/Core/Cache/Cache.php +++ b/core/lib/Drupal/Core/Cache/Cache.php @@ -32,10 +32,10 @@ class Cache { * allows items to be invalidated based on all tags attached to the content * they're constituted from. * - * @param array … + * @param string[] … * Arrays of cache tags to merge. * - * @return array + * @return string[] * The merged array of cache tags. */ public static function mergeTags() { @@ -55,7 +55,7 @@ public static function mergeTags() { * * Can be called before using cache tags in operations, to ensure validity. * - * @param array $tags + * @param string[] $tags * An array of cache tags. * * @throws \LogicException @@ -72,6 +72,28 @@ public static function validateTags(array $tags) { } /** + * Build an array of cache tags from a given prefix and an array of suffixes. + * + * Each suffix will be converted to a cache tag by appending it to the prefix, + * with a colon between them. + * + * @param string $prefix + * A prefix string. + * @param array $suffixes + * An array of suffixes. Will be cast to strings. + * + * @return string[] + * An array of cache tags. + */ + public static function buildTags($prefix, array $suffixes) { + $tags = []; + foreach ($suffixes as $suffix) { + $tags[] = $prefix . ':' . $suffix; + } + return $tags; + } + + /** * Deletes items from all bins with any of the specified tags. * * Many sites have more than one active cache backend, and each backend may @@ -81,7 +103,7 @@ public static function validateTags(array $tags) { * When deleting a given list of tags, we iterate over each cache backend, and * and call deleteTags() on each. * - * @param array $tags + * @param string[] $tags * The list of tags to delete cache items for. */ public static function deleteTags(array $tags) { @@ -101,7 +123,7 @@ public static function deleteTags(array $tags) { * When invalidating a given list of tags, we iterate over each cache backend, * and call invalidateTags() on each. * - * @param array $tags + * @param string[] $tags * The list of tags to invalidate cache items for. */ public static function invalidateTags(array $tags) { diff --git a/core/lib/Drupal/Core/Cache/DatabaseBackend.php b/core/lib/Drupal/Core/Cache/DatabaseBackend.php index 2c94498..1e24968 100644 --- a/core/lib/Drupal/Core/Cache/DatabaseBackend.php +++ b/core/lib/Drupal/Core/Cache/DatabaseBackend.php @@ -147,7 +147,10 @@ protected function prepareItem($cache, $allow_invalid) { * Implements Drupal\Core\Cache\CacheBackendInterface::set(). */ public function set($cid, $data, $expire = Cache::PERMANENT, array $tags = array()) { - $tags = Cache::mergeTags($tags, []); + Cache::validateTags($tags); + $tags = array_unique($tags); + // Sort the cache tags so that they are stored consistently in the database. + sort($tags); $try_again = FALSE; try { // The bin might not yet exist. @@ -234,7 +237,10 @@ public function setMultiple(array $items) { 'tags' => array(), ); - $item['tags'] = Cache::mergeTags($item['tags'], []); + Cache::validateTags($item['tags']); + $item['tags'] = array_unique($item['tags']); + // Sort the cache tags so that they are stored consistently in the DB. + sort($item['tags']); // Remove tags that were already deleted or invalidated during this // request from the static caches so that another deletion or diff --git a/core/lib/Drupal/Core/Cache/MemoryBackend.php b/core/lib/Drupal/Core/Cache/MemoryBackend.php index ce74ff4..23fddc5 100644 --- a/core/lib/Drupal/Core/Cache/MemoryBackend.php +++ b/core/lib/Drupal/Core/Cache/MemoryBackend.php @@ -107,12 +107,16 @@ protected function prepareItem($cache, $allow_invalid) { * Implements Drupal\Core\Cache\CacheBackendInterface::set(). */ public function set($cid, $data, $expire = Cache::PERMANENT, array $tags = array()) { + Cache::validateTags($tags); + $tags = array_unique($tags); + // Sort the cache tags so that they are stored consistently in the database. + sort($tags); $this->cache[$cid] = (object) array( 'cid' => $cid, 'data' => serialize($data), 'created' => REQUEST_TIME, 'expire' => $expire, - 'tags' => Cache::mergeTags($tags, []), + 'tags' => $tags, ); } diff --git a/core/lib/Drupal/Core/Cache/PhpBackend.php b/core/lib/Drupal/Core/Cache/PhpBackend.php index e7de3b6..e188ffb 100644 --- a/core/lib/Drupal/Core/Cache/PhpBackend.php +++ b/core/lib/Drupal/Core/Cache/PhpBackend.php @@ -133,12 +133,13 @@ protected function prepareItem($cache, $allow_invalid) { * {@inheritdoc} */ public function set($cid, $data, $expire = Cache::PERMANENT, array $tags = array()) { + Cache::validateTags($tags); $item = (object) array( 'cid' => $cid, 'data' => $data, 'created' => round(microtime(TRUE), 3), 'expire' => $expire, - 'tags' => Cache::mergeTags($tags, []), + 'tags' => array_unique($tags), ); $this->writeItem($this->normalizeCid($cid), $item); } diff --git a/core/lib/Drupal/Core/Menu/MenuTreeStorage.php b/core/lib/Drupal/Core/Menu/MenuTreeStorage.php index b7e525f..fb75390 100644 --- a/core/lib/Drupal/Core/Menu/MenuTreeStorage.php +++ b/core/lib/Drupal/Core/Menu/MenuTreeStorage.php @@ -193,10 +193,7 @@ public function rebuild(array $definitions) { $this->resetDefinitions(); $affected_menus = $this->getMenuNames() + $before_menus; // Invalidate any cache tagged with any menu name. - $cache_tags = []; - foreach ($affected_menus as $affected_menu) { - $cache_tags[] = 'menu:' . $affected_menu; - } + $cache_tags = Cache::buildTags('menu', $affected_menus); Cache::invalidateTags($cache_tags); $this->resetDefinitions(); // Every item in the cache bin should have one of the menu cache tags but it @@ -259,10 +256,7 @@ protected function safeExecuteSelect(SelectInterface $query) { public function save(array $link) { $affected_menus = $this->doSave($link); $this->resetDefinitions(); - $cache_tags = []; - foreach ($affected_menus as $affected_menu) { - $cache_tags[] = 'menu:' . $affected_menu; - } + $cache_tags = Cache::buildTags('menu', $affected_menus); Cache::invalidateTags($cache_tags); return $affected_menus; } diff --git a/core/modules/system/src/Tests/Cache/GenericCacheBackendUnitTestBase.php b/core/modules/system/src/Tests/Cache/GenericCacheBackendUnitTestBase.php index 44e6a94..958acff 100644 --- a/core/modules/system/src/Tests/Cache/GenericCacheBackendUnitTestBase.php +++ b/core/modules/system/src/Tests/Cache/GenericCacheBackendUnitTestBase.php @@ -212,6 +212,15 @@ public function testSetGet() { $cid = str_repeat('a', 300); $backend->set($cid, 'test'); $this->assertEqual('test', $backend->get($cid)->data); + + // Calling ::set() with invalid cache tags. + try { + $backend->set('exception_test', 'value', Cache::PERMANENT, ['node' => [3, 5, 7]]); + $this->fail('::set() was called with invalid cache tags, no exception was thrown.'); + } + catch (\LogicException $e) { + $this->pass('::set() was called with invalid cache tags, an exception was thrown.'); + } } /** @@ -398,6 +407,20 @@ public function testSetMultiple() { $this->assertEqual($cached['cid_5']->data, $items['cid_5']['data'], 'New cache item set correctly.'); $this->assertEqual($cached['cid_5']->tags, array('test:a', 'test:b')); + + // Calling ::setMultiple() with invalid cache tags. + try { + $items = [ + 'exception_test_1' => array('data' => 1, 'tags' => []), + 'exception_test_2' => array('data' => 2, 'tags' => ['valid']), + 'exception_test_3' => array('data' => 3, 'tags' => ['node' => [3, 5, 7]]), + ]; + $backend->setMultiple($items); + $this->fail('::setMultiple() was called with invalid cache tags, no exception was thrown.'); + } + catch (\LogicException $e) { + $this->pass('::setMultiple() was called with invalid cache tags, an exception was thrown.'); + } } /** diff --git a/core/modules/toolbar/src/Tests/ToolbarAdminMenuTest.php b/core/modules/toolbar/src/Tests/ToolbarAdminMenuTest.php index 486dc20..da51771 100644 --- a/core/modules/toolbar/src/Tests/ToolbarAdminMenuTest.php +++ b/core/modules/toolbar/src/Tests/ToolbarAdminMenuTest.php @@ -255,7 +255,7 @@ function testCacheClearByCacheTag() { // Assert that a cache tag in the toolbar cache under the key "user" exists // for admin_user_2 against the language "en". $cache = $toolbarCache->get('toolbar_' . $admin_user_2_id . ':' . 'en'); - $this->assertEqual($cache->tags[2], 'user:' . $admin_user_2_id); //, 'A cache tag in the toolbar cache under the key "user" exists for admin_user_2 against the language "en".'); + $this->assertEqual($cache->tags[2], 'user:' . $admin_user_2_id, 'A cache tag in the toolbar cache under the key "user" exists for admin_user_2 against the language "en".'); // Request a page in 'fr' to create the cache. $this->drupalGet('fr/test-page'); diff --git a/core/modules/user/src/PermissionsHash.php b/core/modules/user/src/PermissionsHash.php index 3d91576..8fffcdd 100644 --- a/core/modules/user/src/PermissionsHash.php +++ b/core/modules/user/src/PermissionsHash.php @@ -59,10 +59,7 @@ public function generate(AccountInterface $account) { } else { $permissions_hash = $this->doGenerate($sorted_roles); - $tags = []; - foreach ($sorted_roles as $role) { - $tags[] = 'user_role:' . $role; - } + $tags = Cache::buildTags('menu', $sorted_roles); $this->cache->set("user_permissions_hash:$role_list", $permissions_hash, Cache::PERMANENT, $tags); } diff --git a/core/tests/Drupal/Tests/Core/Cache/CacheTest.php b/core/tests/Drupal/Tests/Core/Cache/CacheTest.php index 612899d..e5ae63c 100644 --- a/core/tests/Drupal/Tests/Core/Cache/CacheTest.php +++ b/core/tests/Drupal/Tests/Core/Cache/CacheTest.php @@ -82,4 +82,58 @@ public function testMergeTags(array $a, array $b, array $expected) { $this->assertEquals($expected, Cache::mergeTags($a, $b)); } + /** + * Provides a list of pairs of (prefix, suffixes) to build cache tags from. + * + * @return array + */ + public function buildTagsProvider() { + return [ + ['node', [1], ['node:1']], + ['node', [1, 2, 3], ['node:1', 'node:2', 'node:3']], + ['node', [3, 2, 1], ['node:3', 'node:2', 'node:1']], + ['node', [NULL], ['node:']], + ['node', [TRUE, FALSE], ['node:1', 'node:']], + ['node', ['a', 'z', 'b'], ['node:a', 'node:z', 'node:b']], + // No suffixes, no cache tags. + ['', [], []], + ['node', [], []], + // Only suffix values matter, not keys: verify that expectation. + ['node', [5 => 145, 4545 => 3], ['node:145', 'node:3']], + ['node', [5 => TRUE], ['node:1']], + ['node', [5 => NULL], ['node:']], + ['node', ['a' => NULL], ['node:']], + ['node', ['a' => TRUE], ['node:1']], + ]; + } + + /** + * @covers buildTags + * + * @dataProvider buildTagsProvider + */ + public function testBuildTags($prefix, array $suffixes, array $expected) { + $this->assertEquals($expected, Cache::buildTags($prefix, $suffixes)); + } + + /** + * @covers deleteTags + * + * @expectedException \LogicException + * @expectedExceptionMessage Cache tags must be strings, array given. + */ + public function testDeleteTags() { + Cache::deleteTags(['node' => [2, 3, 5, 8, 13]]); + } + + /** + * @covers invalidateTags + * + * @expectedException \LogicException + * @expectedExceptionMessage Cache tags must be strings, array given. + */ + public function testInvalidateTags() { + Cache::deleteTags(['node' => [2, 3, 5, 8, 13]]); + } + }