From 9aaddd2557488767ca603651316666a4e19373d7 Mon Sep 17 00:00:00 2001 From: Alex Pott <alex.a.pott@googlemail.com> Date: Mon, 10 Aug 2015 11:55:08 +0100 Subject: [PATCH] Issue #2501117 by catch, olli, Wim Leers, dawehner: Add static caching for PermissionsHashGenerator::generate() --- core/core.services.yml | 11 +++- core/lib/Drupal/Core/Cache/MemoryBackend.php | 9 +++ .../Core/Session/PermissionsHashGenerator.php | 31 +++++++--- .../Session/PermissionsHashGeneratorTest.php | 59 +++++++++++++++++-- sites/development.services.yml | 2 - 5 files changed, 98 insertions(+), 14 deletions(-) diff --git a/core/core.services.yml b/core/core.services.yml index bf28df9a17b5..5ba01167a5b8 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -163,6 +163,15 @@ services: cache.backend.php: class: Drupal\Core\Cache\PhpBackendFactory arguments: ['@cache_tags.invalidator.checksum'] + cache.backend.memory: + class: Drupal\Core\Cache\MemoryBackendFactory + # A special cache bin that does not persist beyond the length of the request. + cache.static: + class: Drupal\Core\Cache\CacheBackendInterface + tags: + - { name: cache.bin, default_backend: cache.backend.memory } + factory: cache_factory:get + arguments: [static] cache.bootstrap: class: Drupal\Core\Cache\CacheBackendInterface tags: @@ -1340,7 +1349,7 @@ services: arguments: ['@current_user', '@session_handler.write_safe'] user_permissions_hash_generator: class: Drupal\Core\Session\PermissionsHashGenerator - arguments: ['@private_key', '@cache.default'] + arguments: ['@private_key', '@cache.default', '@cache.static'] current_user: class: Drupal\Core\Session\AccountProxy session_configuration: diff --git a/core/lib/Drupal/Core/Cache/MemoryBackend.php b/core/lib/Drupal/Core/Cache/MemoryBackend.php index d9bad8d6676f..5c7f9288aa4a 100644 --- a/core/lib/Drupal/Core/Cache/MemoryBackend.php +++ b/core/lib/Drupal/Core/Cache/MemoryBackend.php @@ -217,4 +217,13 @@ public function __sleep() { return []; } + /** + * Reset statically cached variables. + * + * This is only used by tests. + */ + public function reset() { + $this->cache = []; + } + } diff --git a/core/lib/Drupal/Core/Session/PermissionsHashGenerator.php b/core/lib/Drupal/Core/Session/PermissionsHashGenerator.php index c4fe7b9b4542..f9da69dd9b49 100644 --- a/core/lib/Drupal/Core/Session/PermissionsHashGenerator.php +++ b/core/lib/Drupal/Core/Session/PermissionsHashGenerator.php @@ -26,23 +26,33 @@ class PermissionsHashGenerator implements PermissionsHashGeneratorInterface { protected $privateKey; /** - * The cache backend interface to use for the permission hash cache. + * The cache backend interface to use for the persistent cache. * * @var \Drupal\Core\Cache\CacheBackendInterface */ protected $cache; + /** + * The cache backend interface to use for the static cache. + * + * @var \Drupal\Core\Cache\CacheBackendInterface + */ + protected $static; + /** * Constructs a PermissionsHashGenerator object. * * @param \Drupal\Core\PrivateKey $private_key * The private key service. * @param \Drupal\Core\Cache\CacheBackendInterface $cache - * The cache backend interface to use for the permission hash cache. + * The cache backend interface to use for the persistent cache. + * @param \Drupal\Core\Cache\CacheBackendInterface + * The cache backend interface to use for the static cache. */ - public function __construct(PrivateKey $private_key, CacheBackendInterface $cache) { + public function __construct(PrivateKey $private_key, CacheBackendInterface $cache, CacheBackendInterface $static) { $this->privateKey = $private_key; $this->cache = $cache; + $this->static = $static; } /** @@ -60,13 +70,20 @@ public function generate(AccountInterface $account) { $sorted_roles = $account->getRoles(); sort($sorted_roles); $role_list = implode(',', $sorted_roles); - if ($cache = $this->cache->get("user_permissions_hash:$role_list")) { - $permissions_hash = $cache->data; + $cid = "user_permissions_hash:$role_list"; + if ($static_cache = $this->static->get($cid)) { + return $static_cache->data; } else { - $permissions_hash = $this->doGenerate($sorted_roles); $tags = Cache::buildTags('config:user.role', $sorted_roles, '.'); - $this->cache->set("user_permissions_hash:$role_list", $permissions_hash, Cache::PERMANENT, $tags); + if ($cache = $this->cache->get($cid)) { + $permissions_hash = $cache->data; + } + else { + $permissions_hash = $this->doGenerate($sorted_roles); + $this->cache->set($cid, $permissions_hash, Cache::PERMANENT, $tags); + } + $this->static->set($cid, $permissions_hash, Cache::PERMANENT, $tags); } return $permissions_hash; diff --git a/core/tests/Drupal/Tests/Core/Session/PermissionsHashGeneratorTest.php b/core/tests/Drupal/Tests/Core/Session/PermissionsHashGeneratorTest.php index 616788d8e395..925c3109bddb 100644 --- a/core/tests/Drupal/Tests/Core/Session/PermissionsHashGeneratorTest.php +++ b/core/tests/Drupal/Tests/Core/Session/PermissionsHashGeneratorTest.php @@ -61,6 +61,13 @@ class PermissionsHashGeneratorTest extends UnitTestCase { */ protected $cache; + /** + * The mocked cache backend. + * + * @var \Drupal\Core\Cache\CacheBackendInterface|\PHPUnit_Framework_MockObject_MockObject + */ + protected $staticCache; + /** * The permission hash class being tested. * @@ -138,12 +145,15 @@ protected function setUp() { $this->cache = $this->getMockBuilder('Drupal\Core\Cache\CacheBackendInterface') ->disableOriginalConstructor() ->getMock(); + $this->staticCache = $this->getMockBuilder('Drupal\Core\Cache\CacheBackendInterface') + ->disableOriginalConstructor() + ->getMock(); - $this->permissionsHash = new PermissionsHashGenerator($this->privateKey, $this->cache); + $this->permissionsHash = new PermissionsHashGenerator($this->privateKey, $this->cache, $this->staticCache); } /** - * Tests the generate() method. + * @covers ::generate */ public function testGenerate() { // Ensure that the super user (user 1) always gets the same hash. @@ -162,15 +172,23 @@ public function testGenerate() { } /** - * Tests the generate method with cache returned. + * @covers ::generate */ - public function testGenerateCache() { + public function testGeneratePersistentCache() { // Set expectations for the mocked cache backend. $expected_cid = 'user_permissions_hash:administrator,authenticated'; $mock_cache = new \stdClass(); $mock_cache->data = 'test_hash_here'; + $this->staticCache->expects($this->once()) + ->method('get') + ->with($expected_cid) + ->will($this->returnValue(FALSE)); + $this->staticCache->expects($this->once()) + ->method('set') + ->with($expected_cid, $this->isType('string')); + $this->cache->expects($this->once()) ->method('get') ->with($expected_cid) @@ -181,6 +199,31 @@ public function testGenerateCache() { $this->permissionsHash->generate($this->account2); } + /** + * @covers ::generate + */ + public function testGenerateStaticCache() { + // Set expectations for the mocked cache backend. + $expected_cid = 'user_permissions_hash:administrator,authenticated'; + + $mock_cache = new \stdClass(); + $mock_cache->data = 'test_hash_here'; + + $this->staticCache->expects($this->once()) + ->method('get') + ->with($expected_cid) + ->will($this->returnValue($mock_cache)); + $this->staticCache->expects($this->never()) + ->method('set'); + + $this->cache->expects($this->never()) + ->method('get'); + $this->cache->expects($this->never()) + ->method('set'); + + $this->permissionsHash->generate($this->account2); + } + /** * Tests the generate method with no cache returned. */ @@ -188,6 +231,14 @@ public function testGenerateNoCache() { // Set expectations for the mocked cache backend. $expected_cid = 'user_permissions_hash:administrator,authenticated'; + $this->staticCache->expects($this->once()) + ->method('get') + ->with($expected_cid) + ->will($this->returnValue(FALSE)); + $this->staticCache->expects($this->once()) + ->method('set') + ->with($expected_cid, $this->isType('string')); + $this->cache->expects($this->once()) ->method('get') ->with($expected_cid) diff --git a/sites/development.services.yml b/sites/development.services.yml index cc212117dc87..73cc998255e0 100644 --- a/sites/development.services.yml +++ b/sites/development.services.yml @@ -3,7 +3,5 @@ # To activate this feature, follow the instructions at the top of the # 'example.settings.local.php' file, which sits next to this file. services: - cache.backend.memory: - class: Drupal\Core\Cache\MemoryBackendFactory cache.backend.null: class: Drupal\Core\Cache\NullBackendFactory -- GitLab