From c049f3f1d6b7fbf11541fe0139643e38f9ca368c Mon Sep 17 00:00:00 2001 From: catch Date: Sun, 22 Dec 2013 21:58:10 +0000 Subject: [PATCH] Issue #2005644 by Wim Leers, damiankloip, beejeebus, amateescu: Use client-side cache tags & caching to eliminate 1 HTTP requests/page for in-place editing metadata, introduce drupalSettings.user.permissionsHash. --- .../Tests/Menu/ActionLocalTasksTest.php | 2 +- .../Tests/Menu/AggregatorLocalTasksTest.php | 2 +- .../Tests/Menu/CustomBlockLocalTasksTest.php | 6 +- .../block/Tests/Menu/BlockLocalTasksTest.php | 2 +- .../book/Tests/Menu/BookLocalTasksTest.php | 6 +- .../Tests/Menu/ConfigLocalTasksTest.php | 2 +- .../Menu/ContentTranslationLocalTasksTest.php | 25 ++- core/modules/edit/js/edit.js | 65 +++++- .../Tests/Menu/LanguageLocalTasks.php | 4 +- .../Tests/Menu/LocaleLocalTasksTest.php | 4 +- .../Tests/Menu/ShortcutLocalTasksTest.php | 6 +- .../Tests/Menu/TaxonomyLocalTasksTest.php | 2 +- .../user/lib/Drupal/user/Entity/Role.php | 14 +- .../user/lib/Drupal/user/PermissionsHash.php | 86 +++++++ .../Drupal/user/PermissionsHashInterface.php | 28 +++ .../Drupal/user/Tests/UserPermissionsTest.php | 19 ++ .../user/Tests/Menu/UserLocalTasksTest.php | 2 +- .../Drupal/user/Tests/PermissionsHashTest.php | 212 ++++++++++++++++++ core/modules/user/user.module | 6 +- core/modules/user/user.services.yml | 3 + .../Core/Menu/LocalTaskIntegrationTest.php | 30 ++- 21 files changed, 483 insertions(+), 43 deletions(-) create mode 100644 core/modules/user/lib/Drupal/user/PermissionsHash.php create mode 100644 core/modules/user/lib/Drupal/user/PermissionsHashInterface.php create mode 100644 core/modules/user/tests/Drupal/user/Tests/PermissionsHashTest.php diff --git a/core/modules/action/tests/Drupal/action/Tests/Menu/ActionLocalTasksTest.php b/core/modules/action/tests/Drupal/action/Tests/Menu/ActionLocalTasksTest.php index 729d3141e4a..61844d29635 100644 --- a/core/modules/action/tests/Drupal/action/Tests/Menu/ActionLocalTasksTest.php +++ b/core/modules/action/tests/Drupal/action/Tests/Menu/ActionLocalTasksTest.php @@ -26,7 +26,7 @@ class ActionLocalTasksTest extends LocalTaskIntegrationTest { } public function setUp() { - $this->moduleList = array('action' => 'core/modules/action/action.module'); + $this->directoryList = array('action' => 'core/modules/action'); parent::setUp(); } diff --git a/core/modules/aggregator/tests/Drupal/aggregator/Tests/Menu/AggregatorLocalTasksTest.php b/core/modules/aggregator/tests/Drupal/aggregator/Tests/Menu/AggregatorLocalTasksTest.php index 3d0bca99d0e..63caa74cee5 100644 --- a/core/modules/aggregator/tests/Drupal/aggregator/Tests/Menu/AggregatorLocalTasksTest.php +++ b/core/modules/aggregator/tests/Drupal/aggregator/Tests/Menu/AggregatorLocalTasksTest.php @@ -26,7 +26,7 @@ class AggregatorLocalTasksTest extends LocalTaskIntegrationTest { } public function setUp() { - $this->moduleList = array('aggregator' => 'core/modules/aggregator/aggregator.module'); + $this->directoryList = array('aggregator' => 'core/modules/aggregator'); parent::setUp(); } diff --git a/core/modules/block/custom_block/tests/Drupal/custom_blocks/Tests/Menu/CustomBlockLocalTasksTest.php b/core/modules/block/custom_block/tests/Drupal/custom_blocks/Tests/Menu/CustomBlockLocalTasksTest.php index 39ddc5479b7..d04ad152e25 100644 --- a/core/modules/block/custom_block/tests/Drupal/custom_blocks/Tests/Menu/CustomBlockLocalTasksTest.php +++ b/core/modules/block/custom_block/tests/Drupal/custom_blocks/Tests/Menu/CustomBlockLocalTasksTest.php @@ -26,9 +26,9 @@ class CustomBlockLocalTasksTest extends LocalTaskIntegrationTest { } public function setUp() { - $this->moduleList = array( - 'block' => 'core/modules/block/block.module', - 'custom_block' => 'core/modules/block/custom_block/custom_block.module', + $this->directoryList = array( + 'block' => 'core/modules/block', + 'custom_block' => 'core/modules/block/custom_block', ); parent::setUp(); } diff --git a/core/modules/block/tests/Drupal/block/Tests/Menu/BlockLocalTasksTest.php b/core/modules/block/tests/Drupal/block/Tests/Menu/BlockLocalTasksTest.php index 989a7ecd3dc..96dee917dac 100644 --- a/core/modules/block/tests/Drupal/block/Tests/Menu/BlockLocalTasksTest.php +++ b/core/modules/block/tests/Drupal/block/Tests/Menu/BlockLocalTasksTest.php @@ -26,7 +26,7 @@ class BlockLocalTasksTest extends LocalTaskIntegrationTest { } public function setUp() { - $this->moduleList = array('block' => 'core/modules/block/block.module'); + $this->directoryList = array('block' => 'core/modules/block'); parent::setUp(); $config_factory = $this->getConfigFactoryStub(array('system.theme' => array( diff --git a/core/modules/book/tests/Drupal/book/Tests/Menu/BookLocalTasksTest.php b/core/modules/book/tests/Drupal/book/Tests/Menu/BookLocalTasksTest.php index c1f14343131..86492c4fa54 100644 --- a/core/modules/book/tests/Drupal/book/Tests/Menu/BookLocalTasksTest.php +++ b/core/modules/book/tests/Drupal/book/Tests/Menu/BookLocalTasksTest.php @@ -26,9 +26,9 @@ class BookLocalTasksTest extends LocalTaskIntegrationTest { } public function setUp() { - $this->moduleList = array( - 'book' => 'core/modules/book/book.module', - 'node' => 'core/modules/node/node.module', + $this->directoryList = array( + 'book' => 'core/modules/book', + 'node' => 'core/modules/node', ); parent::setUp(); } diff --git a/core/modules/config/tests/Drupal/config/Tests/Menu/ConfigLocalTasksTest.php b/core/modules/config/tests/Drupal/config/Tests/Menu/ConfigLocalTasksTest.php index 7d1bcd182ea..627f61e3751 100644 --- a/core/modules/config/tests/Drupal/config/Tests/Menu/ConfigLocalTasksTest.php +++ b/core/modules/config/tests/Drupal/config/Tests/Menu/ConfigLocalTasksTest.php @@ -26,7 +26,7 @@ class ConfigLocalTasksTest extends LocalTaskIntegrationTest { } public function setUp() { - $this->moduleList = array('config' => 'core/modules/config/config.module'); + $this->directoryList = array('config' => 'core/modules/config'); parent::setUp(); } diff --git a/core/modules/content_translation/tests/Drupal/content_translation/Tests/Menu/ContentTranslationLocalTasksTest.php b/core/modules/content_translation/tests/Drupal/content_translation/Tests/Menu/ContentTranslationLocalTasksTest.php index 48fe2a51e30..e0b164bbc54 100644 --- a/core/modules/content_translation/tests/Drupal/content_translation/Tests/Menu/ContentTranslationLocalTasksTest.php +++ b/core/modules/content_translation/tests/Drupal/content_translation/Tests/Menu/ContentTranslationLocalTasksTest.php @@ -8,6 +8,7 @@ namespace Drupal\content_translation\Tests\Menu; use Drupal\Tests\Core\Menu\LocalTaskIntegrationTest; +use Drupal\content_translation\Plugin\Derivative\ContentTranslationLocalTasks;; /** * Tests existence of block local tasks. @@ -26,9 +27,9 @@ class ContentTranslationLocalTasksTest extends LocalTaskIntegrationTest { } public function setUp() { - $this->moduleList = array( - 'content_translation' => 'core/modules/content_translation/content_translation.module', - 'node' => 'core/modules/node/node.module', + $this->directoryList = array( + 'content_translation' => 'core/modules/content_translation', + 'node' => 'core/modules/node', ); parent::setUp(); @@ -47,6 +48,24 @@ class ContentTranslationLocalTasksTest extends LocalTaskIntegrationTest { \Drupal::getContainer()->set('content_translation.manager', $content_translation_manager); } + /** + * {@inheritdoc} + */ + protected function getLocalTaskManager($modules, $route_name, $route_params) { + $manager = parent::getLocalTaskManager($modules, $route_name, $route_params); + + // Duplicate content_translation_local_tasks_alter()'s code here to avoid + // having to load the .module file. + $this->moduleHandler->expects($this->once()) + ->method('alter') + ->will($this->returnCallback(function ($hook, &$local_tasks) { + // Alters in tab_root_id onto the content translation local task. + $derivative = ContentTranslationLocalTasks::create(\Drupal::getContainer(), 'content_translation.local_tasks'); + $derivative->alterLocalTasks($local_tasks); + })); + return $manager; + } + /** * Tests the block admin display local tasks. * diff --git a/core/modules/edit/js/edit.js b/core/modules/edit/js/edit.js index cdc9d9892dd..429e7b2272a 100644 --- a/core/modules/edit/js/edit.js +++ b/core/modules/edit/js/edit.js @@ -17,7 +17,7 @@ * is not yet known whether the user has permission to edit at >=1 of them. */ -(function ($, _, Backbone, Drupal, drupalSettings) { +(function ($, _, Backbone, Drupal, drupalSettings, JSON, storage) { "use strict"; @@ -68,6 +68,12 @@ Drupal.behaviors.edit = { // Initialize the Edit app once per page load. $('body').once('edit-init', initEdit); + // Find all in-place editable fields, if any. + var $fields = $(context).find('[data-edit-field-id]').once('edit'); + if ($fields.length === 0) { + return; + } + // Process each entity element: identical entities that appear multiple // times will get a numeric identifier, starting at 0. $(context).find('[data-edit-entity-id]').once('edit').each(function (index, entityElement) { @@ -89,10 +95,17 @@ Drupal.behaviors.edit = { // immediately. New fields will be unable to be processed immediately, but // will instead be queued to have their metadata fetched, which occurs below // in fetchMissingMetaData(). - $(context).find('[data-edit-field-id]').once('edit').each(function (index, fieldElement) { + $fields.each(function (index, fieldElement) { processField(fieldElement); }); + // Entities and fields on the page have been detected, try to set up the + // contextual links for those entities that already have the necessary meta- + // data in the client-side cache. + contextualLinksQueue = _.filter(contextualLinksQueue, function (contextualLink) { + return !initializeEntityContextualLink(contextualLink); + }); + // Fetch metadata for any fields that are queued to retrieve it. fetchMissingMetadata(function (fieldElementsWithFreshMetadata) { // Metadata has been fetched, reprocess fields whose metadata was missing. @@ -128,17 +141,47 @@ Drupal.edit = { // Per-field metadata that indicates whether in-place editing is allowed, // which in-place editor should be used, etc. metadata: { - has: function (fieldID) { return _.has(this.data, fieldID); }, - add: function (fieldID, metadata) { this.data[fieldID] = metadata; }, - get: function (fieldID, key) { - return (key === undefined) ? this.data[fieldID] : this.data[fieldID][key]; + has: function (fieldID) { + return storage.getItem(this._prefixFieldID(fieldID)) !== null; }, - intersection: function (fieldIDs) { return _.intersection(fieldIDs, _.keys(this.data)); }, - // Contains the actual metadata, keyed by field ID. - data: {} + add: function (fieldID, metadata) { + storage.setItem(this._prefixFieldID(fieldID), JSON.stringify(metadata)); + }, + get: function (fieldID, key) { + var metadata = JSON.parse(storage.getItem(this._prefixFieldID(fieldID))); + return (key === undefined) ? metadata : metadata[key]; + }, + _prefixFieldID: function (fieldID) { + return 'Drupal.edit.metadata.' + fieldID; + }, + _unprefixFieldID: function (fieldID) { + // Strip "Drupal.edit.metadata.", which is 21 characters long. + return fieldID.substring(21); + }, + intersection: function (fieldIDs) { + var prefixedFieldIDs = _.map(fieldIDs, this._prefixFieldID); + var intersection = _.intersection(prefixedFieldIDs, _.keys(sessionStorage)); + return _.map(intersection, this._unprefixFieldID); + } } }; +// Clear the Edit metadata cache whenever the current user's set of permissions +// changes. +var permissionsHashKey = Drupal.edit.metadata._prefixFieldID('permissionsHash'); +var permissionsHashValue = storage.getItem(permissionsHashKey); +var permissionsHash = drupalSettings.user.permissionsHash; +if (permissionsHashValue !== permissionsHash) { + if (typeof permissionsHash === 'string') { + _.chain(storage).keys().each(function (key) { + if (key.substring(0, 21) === 'Drupal.edit.metadata.') { + storage.removeItem(key); + } + }); + } + storage.setItem(permissionsHashKey, permissionsHash); +} + /** * Detect contextual links on entities annotated by Edit; queue these to be * processed. @@ -291,7 +334,7 @@ function fetchMissingMetadata (callback) { var fieldElementsWithoutMetadata = _.pluck(fieldsMetadataQueue, 'el'); var entityIDs = _.uniq(_.pluck(fieldsMetadataQueue, 'entityID'), true); // Ensure we only request entityIDs for which we don't have metadata yet. - entityIDs = _.difference(entityIDs, _.keys(Drupal.edit.metadata.data)); + entityIDs = _.difference(entityIDs, Drupal.edit.metadata.intersection(entityIDs)); fieldsMetadataQueue = []; $.ajax({ @@ -520,4 +563,4 @@ function deleteContainedModelsAndQueues($context) { }); } -})(jQuery, _, Backbone, Drupal, drupalSettings); +})(jQuery, _, Backbone, Drupal, drupalSettings, window.JSON, window.sessionStorage); diff --git a/core/modules/language/tests/Drupal/language/Tests/Menu/LanguageLocalTasks.php b/core/modules/language/tests/Drupal/language/Tests/Menu/LanguageLocalTasks.php index 204b8cdffc2..8ec2313c77c 100644 --- a/core/modules/language/tests/Drupal/language/Tests/Menu/LanguageLocalTasks.php +++ b/core/modules/language/tests/Drupal/language/Tests/Menu/LanguageLocalTasks.php @@ -26,8 +26,8 @@ class LanguageLocalTasks extends LocalTaskIntegrationTest { } public function setUp() { - $this->moduleList = array( - 'language' => 'core/modules/language/language.module', + $this->directoryList = array( + 'language' => 'core/modules/language', ); parent::setUp(); } diff --git a/core/modules/locale/tests/Drupal/locale/Tests/Menu/LocaleLocalTasksTest.php b/core/modules/locale/tests/Drupal/locale/Tests/Menu/LocaleLocalTasksTest.php index f8db54fcb1f..16edc7e89cd 100644 --- a/core/modules/locale/tests/Drupal/locale/Tests/Menu/LocaleLocalTasksTest.php +++ b/core/modules/locale/tests/Drupal/locale/Tests/Menu/LocaleLocalTasksTest.php @@ -26,8 +26,8 @@ class LocaleLocalTasksTest extends LocalTaskIntegrationTest { } public function setUp() { - $this->moduleList = array( - 'locale' => 'core/modules/locale/locale.module', + $this->directoryList = array( + 'locale' => 'core/modules/locale', ); parent::setUp(); } diff --git a/core/modules/shortcut/tests/Drupal/shortcut/Tests/Menu/ShortcutLocalTasksTest.php b/core/modules/shortcut/tests/Drupal/shortcut/Tests/Menu/ShortcutLocalTasksTest.php index edc5e8b6e45..c4b45645c95 100644 --- a/core/modules/shortcut/tests/Drupal/shortcut/Tests/Menu/ShortcutLocalTasksTest.php +++ b/core/modules/shortcut/tests/Drupal/shortcut/Tests/Menu/ShortcutLocalTasksTest.php @@ -26,9 +26,9 @@ class ShortcutLocalTasksTest extends LocalTaskIntegrationTest { } public function setUp() { - $this->moduleList = array( - 'shortcut' => 'core/modules/shortcut/shortcut.module', - 'user' => 'core/modules/user/user.module', + $this->directoryList = array( + 'shortcut' => 'core/modules/shortcut', + 'user' => 'core/modules/user', ); parent::setUp(); } diff --git a/core/modules/taxonomy/tests/Drupal/taxonomy/Tests/Menu/TaxonomyLocalTasksTest.php b/core/modules/taxonomy/tests/Drupal/taxonomy/Tests/Menu/TaxonomyLocalTasksTest.php index 7a88a73aeaf..d51ff7ac56b 100644 --- a/core/modules/taxonomy/tests/Drupal/taxonomy/Tests/Menu/TaxonomyLocalTasksTest.php +++ b/core/modules/taxonomy/tests/Drupal/taxonomy/Tests/Menu/TaxonomyLocalTasksTest.php @@ -26,7 +26,7 @@ class TaxonomyLocalTasksTest extends LocalTaskIntegrationTest { } public function setUp() { - $this->moduleList = array('taxonomy' => 'core/modules/taxonomy/taxonomy.module'); + $this->directoryList = array('taxonomy' => 'core/modules/taxonomy'); parent::setUp(); } diff --git a/core/modules/user/lib/Drupal/user/Entity/Role.php b/core/modules/user/lib/Drupal/user/Entity/Role.php index f17257d6fa9..2fd26ace830 100644 --- a/core/modules/user/lib/Drupal/user/Entity/Role.php +++ b/core/modules/user/lib/Drupal/user/Entity/Role.php @@ -7,6 +7,7 @@ namespace Drupal\user\Entity; +use Drupal\Core\Cache\Cache; use Drupal\Core\Config\Entity\ConfigEntityBase; use Drupal\Core\Entity\EntityStorageControllerInterface; use Drupal\user\RoleInterface; @@ -133,13 +134,24 @@ class Role extends ConfigEntityBase implements RoleInterface { } } + /** + * {@inheritdoc} + */ + public function postSave(EntityStorageControllerInterface $storage_controller, $update = TRUE) { + parent::postSave($storage_controller, $update); + + Cache::invalidateTags(array('role' => $this->id())); + } + /** * {@inheritdoc} */ public static function postDelete(EntityStorageControllerInterface $storage_controller, array $entities) { parent::postDelete($storage_controller, $entities); - $storage_controller->deleteRoleReferences(array_keys($entities)); + $ids = array_keys($entities); + $storage_controller->deleteRoleReferences($ids); + Cache::invalidateTags(array('role' => $ids)); } } diff --git a/core/modules/user/lib/Drupal/user/PermissionsHash.php b/core/modules/user/lib/Drupal/user/PermissionsHash.php new file mode 100644 index 00000000000..59fea9c4540 --- /dev/null +++ b/core/modules/user/lib/Drupal/user/PermissionsHash.php @@ -0,0 +1,86 @@ +privateKey = $private_key; + $this->cache = $cache; + } + + /** + * {@inheritdoc} + * + * Cached by role, invalidated whenever permissions change. + */ + 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; + } + else { + $permissions_hash = $this->doGenerate($sorted_roles); + $this->cache->set("user_permissions_hash:$role_list", $permissions_hash, CacheBackendInterface::CACHE_PERMANENT, array('role' => $sorted_roles)); + } + + return $permissions_hash; + } + + /** + * Generates a hash that uniquely identifies the user's permissions. + * + * @param \Drupal\user\Entity\Role[] $roles + * The user's roles. + * + * @return string + * The permissions hash. + */ + protected function doGenerate(array $roles) { + // @todo Once Drupal gets rid of user_role_permissions(), we should be able + // to inject the user role controller and call a method on that instead. + $permissions_by_role = user_role_permissions($roles); + foreach ($permissions_by_role as $role => $permissions) { + sort($permissions); + $permissions_by_role[$role] = $permissions; + } + return hash('sha256', $this->privateKey->get() . drupal_get_hash_salt() . serialize($permissions_by_role)); + } + +} diff --git a/core/modules/user/lib/Drupal/user/PermissionsHashInterface.php b/core/modules/user/lib/Drupal/user/PermissionsHashInterface.php new file mode 100644 index 00000000000..33566459856 --- /dev/null +++ b/core/modules/user/lib/Drupal/user/PermissionsHashInterface.php @@ -0,0 +1,28 @@ +container->get('user.permissions_hash'); + $this->drupalLogin($this->admin_user); $rid = $this->rid; $account = $this->admin_user; + $previous_permissions_hash = $permissions_hash_generator->generate($account); + $this->assertIdentical($previous_permissions_hash, $permissions_hash_generator->generate($this->loggedInUser)); // Add a permission. $this->assertFalse(user_access('administer nodes', $account), 'User does not have "administer nodes" permission.'); @@ -50,6 +54,10 @@ class UserPermissionsTest extends WebTestBase { $storage_controller = $this->container->get('entity.manager')->getStorageController('user_role'); $storage_controller->resetCache(); $this->assertTrue(user_access('administer nodes', $account), 'User now has "administer nodes" permission.'); + $current_permissions_hash = $permissions_hash_generator->generate($account); + $this->assertIdentical($current_permissions_hash, $permissions_hash_generator->generate($this->loggedInUser)); + $this->assertNotEqual($previous_permissions_hash, $current_permissions_hash, 'Permissions hash has changed.'); + $previous_permissions_hash = $current_permissions_hash; // Remove a permission. $this->assertTrue(user_access('access user profiles', $account), 'User has "access user profiles" permission.'); @@ -59,6 +67,9 @@ class UserPermissionsTest extends WebTestBase { $this->assertText(t('The changes have been saved.'), 'Successful save message displayed.'); $storage_controller->resetCache(); $this->assertFalse(user_access('access user profiles', $account), 'User no longer has "access user profiles" permission.'); + $current_permissions_hash = $permissions_hash_generator->generate($account); + $this->assertIdentical($current_permissions_hash, $permissions_hash_generator->generate($this->loggedInUser)); + $this->assertNotEqual($previous_permissions_hash, $current_permissions_hash, 'Permissions hash has changed.'); } /** @@ -87,8 +98,11 @@ class UserPermissionsTest extends WebTestBase { * Verify proper permission changes by user_role_change_permissions(). */ function testUserRoleChangePermissions() { + $permissions_hash_generator = $this->container->get('user.permissions_hash'); + $rid = $this->rid; $account = $this->admin_user; + $previous_permissions_hash = $permissions_hash_generator->generate($account); // Verify current permissions. $this->assertFalse(user_access('administer nodes', $account), 'User does not have "administer nodes" permission.'); @@ -106,5 +120,10 @@ class UserPermissionsTest extends WebTestBase { $this->assertTrue(user_access('administer nodes', $account), 'User now has "administer nodes" permission.'); $this->assertFalse(user_access('access user profiles', $account), 'User no longer has "access user profiles" permission.'); $this->assertTrue(user_access('administer site configuration', $account), 'User still has "administer site configuration" permission.'); + + // Verify the permissions hash has changed. + $current_permissions_hash = $permissions_hash_generator->generate($account); + $this->assertNotEqual($previous_permissions_hash, $current_permissions_hash, 'Permissions hash has changed.'); } + } diff --git a/core/modules/user/tests/Drupal/user/Tests/Menu/UserLocalTasksTest.php b/core/modules/user/tests/Drupal/user/Tests/Menu/UserLocalTasksTest.php index ac624fc7d4e..b8897ce2b2a 100644 --- a/core/modules/user/tests/Drupal/user/Tests/Menu/UserLocalTasksTest.php +++ b/core/modules/user/tests/Drupal/user/Tests/Menu/UserLocalTasksTest.php @@ -26,7 +26,7 @@ class UserLocalTasksTest extends LocalTaskIntegrationTest { } public function setUp() { - $this->moduleList = array('user' => 'core/modules/user/user.module'); + $this->directoryList = array('user' => 'core/modules/user'); parent::setUp(); } diff --git a/core/modules/user/tests/Drupal/user/Tests/PermissionsHashTest.php b/core/modules/user/tests/Drupal/user/Tests/PermissionsHashTest.php new file mode 100644 index 00000000000..422abd1d96b --- /dev/null +++ b/core/modules/user/tests/Drupal/user/Tests/PermissionsHashTest.php @@ -0,0 +1,212 @@ + 'Permission hash generator service', + 'description' => 'Tests the user permission hash generator service', + 'group' => 'User', + ); + } + + /** + * {@inheritdoc} + */ + protected function setUp() { + parent::setUp(); + + // Account 1: 'administrator' and 'authenticated' roles. + $roles_1 = array('administrator', 'authenticated'); + $this->account_1 = $this->getMockBuilder('Drupal\user\Entity\User') + ->disableOriginalConstructor() + ->setMethods(array('getRoles')) + ->getMock(); + $this->account_1->expects($this->any()) + ->method('getRoles') + ->will($this->returnValue($roles_1)); + + // Account 2: 'authenticated' and 'administrator' roles (different order). + $roles_2 = array('authenticated', 'administrator'); + $this->account_2 = $this->getMockBuilder('Drupal\user\Entity\User') + ->disableOriginalConstructor() + ->setMethods(array('getRoles')) + ->getMock(); + $this->account_2->expects($this->any()) + ->method('getRoles') + ->will($this->returnValue($roles_2)); + + // Updated account 1: now also 'editor' role. + $roles_1_updated = array('editor', 'administrator', 'authenticated'); + $this->account_1_updated = $this->getMockBuilder('Drupal\user\Entity\User') + ->disableOriginalConstructor() + ->setMethods(array('getRoles')) + ->getMock(); + $this->account_1_updated->expects($this->any()) + ->method('getRoles') + ->will($this->returnValue($roles_1_updated)); + + // Mocked private key + cache services. + $random = Crypt::randomStringHashed(55); + $this->private_key = $this->getMockBuilder('Drupal\Core\PrivateKey') + ->disableOriginalConstructor() + ->setMethods(array('get')) + ->getMock(); + $this->private_key->expects($this->any()) + ->method('get') + ->will($this->returnValue($random)); + $this->cache = $this->getMockBuilder('Drupal\Core\Cache\CacheBackendInterface') + ->disableOriginalConstructor() + ->getMock(); + + $this->permissionsHash = new PermissionsHash($this->private_key, $this->cache); + } + + /** + * Tests the generate() method. + */ + public function testGenerate() { + // Ensure that two user accounts with the same roles generate the same hash. + $hash_1 = $this->permissionsHash->generate($this->account_1); + $hash_2 = $this->permissionsHash->generate($this->account_2); + $this->assertSame($hash_1, $hash_2, 'Different users with the same roles generate the same permissions hash.'); + + // Compare with hash for user account 1 with an additional role. + $updated_hash_1 = $this->permissionsHash->generate($this->account_1_updated); + $this->assertNotSame($hash_1, $updated_hash_1, 'Same user with updated roles generates different permissions hash.'); + } + + /** + * Tests the generate method with cache returned. + */ + public function testGenerateCache() { + // 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->cache->expects($this->once()) + ->method('get') + ->with($expected_cid) + ->will($this->returnValue($mock_cache)); + $this->cache->expects($this->never()) + ->method('set'); + + $this->permissionsHash->generate($this->account_1); + } + + /** + * Tests the generate method with no cache returned. + */ + public function testGenerateNoCache() { + // Set expectations for the mocked cache backend. + $expected_cid = 'user_permissions_hash:administrator,authenticated'; + + $this->cache->expects($this->once()) + ->method('get') + ->with($expected_cid) + ->will($this->returnValue(FALSE)); + $this->cache->expects($this->once()) + ->method('set') + ->with($expected_cid, $this->isType('string')); + + $this->permissionsHash->generate($this->account_1); + } + +} + +} + +namespace { + + // @todo remove once user_role_permissions() can be injected. + if (!function_exists('user_role_permissions')) { + function user_role_permissions(array $roles) { + $role_permissions = array(); + foreach ($roles as $rid) { + $role_permissions[$rid] = array(); + } + return $role_permissions; + } + } + + // @todo remove once drupal_get_hash_salt() can be injected. + if (!function_exists('drupal_get_hash_salt')) { + function drupal_get_hash_salt() { + static $salt; + + if (!isset($salt)) { + $salt = Drupal\Component\Utility\Crypt::randomStringHashed(55); + } + + return $salt; + } + } + +} diff --git a/core/modules/user/user.module b/core/modules/user/user.module index 6744e9ecae3..31a651e8da9 100644 --- a/core/modules/user/user.module +++ b/core/modules/user/user.module @@ -132,9 +132,13 @@ function user_js_alter(&$javascript) { // Provide the user ID in drupalSettings to allow JavaScript code to customize // the experience for the end user, rather than the server side, which would // break the render cache. + // Similarly, provide a permissions hash, so that permission-dependent data + // can be reliably cached on the client side. + $user = \Drupal::currentUser(); $javascript['settings']['data'][] = array( 'user' => array( - 'uid' => \Drupal::currentUser()->id(), + 'uid' => $user->id(), + 'permissionsHash' => \Drupal::service('user.permissions_hash')->generate($user), ), ); } diff --git a/core/modules/user/user.services.yml b/core/modules/user/user.services.yml index f35e710d4bf..05382aff7aa 100644 --- a/core/modules/user/user.services.yml +++ b/core/modules/user/user.services.yml @@ -35,3 +35,6 @@ services: arguments: ['@current_user', '@config.factory', '@entity.manager'] tags: - { name: theme_negotiator, priority: -40 } + user.permissions_hash: + class: Drupal\user\PermissionsHash + arguments: ['@private_key', '@cache.cache'] diff --git a/core/tests/Drupal/Tests/Core/Menu/LocalTaskIntegrationTest.php b/core/tests/Drupal/Tests/Core/Menu/LocalTaskIntegrationTest.php index 61ee6013d8d..47384aca2d5 100644 --- a/core/tests/Drupal/Tests/Core/Menu/LocalTaskIntegrationTest.php +++ b/core/tests/Drupal/Tests/Core/Menu/LocalTaskIntegrationTest.php @@ -7,7 +7,6 @@ namespace Drupal\Tests\Core\Menu; -use Drupal\Core\Extension\ModuleHandler; use Drupal\Core\Language\Language; use Drupal\Core\Plugin\Discovery\ContainerDerivativeDiscoveryDecorator; use Drupal\Core\Plugin\Discovery\YamlDiscovery; @@ -27,11 +26,18 @@ if (!defined('DRUPAL_ROOT')) { abstract class LocalTaskIntegrationTest extends UnitTestCase { /** - * A list of modules used for yaml searching. + * A list of module directories used for YAML searching. * * @var array */ - protected $moduleList; + protected $directoryList; + + /** + * The module handler. + * + * @var \Drupal\Core\Extension\ModuleHandlerInterface|\PHPUnit_Framework_MockObject_MockObject + */ + protected $moduleHandler; protected function setUp() { $container = new ContainerBuilder(); @@ -49,7 +55,7 @@ abstract class LocalTaskIntegrationTest extends UnitTestCase { /** * Sets up the local task manager for the test. */ - protected function getLocalTaskManager($modules, $route_name, $route_params) { + protected function getLocalTaskManager($module_dirs, $route_name, $route_params) { $manager = $this ->getMockBuilder('Drupal\Core\Menu\LocalTaskManager') ->disableOriginalConstructor() @@ -73,8 +79,11 @@ abstract class LocalTaskIntegrationTest extends UnitTestCase { $property->setAccessible(TRUE); $property->setValue($manager, $accessManager); - $module_handler = new ModuleHandler($modules); - $pluginDiscovery = new YamlDiscovery('local_tasks', $module_handler->getModuleDirectories()); + $this->moduleHandler = $this->getMockBuilder('Drupal\Core\Extension\ModuleHandlerInterface') + ->disableOriginalConstructor() + ->getMock(); + + $pluginDiscovery = new YamlDiscovery('local_tasks', $module_dirs); $pluginDiscovery = new ContainerDerivativeDiscoveryDecorator($pluginDiscovery); $property = new \ReflectionProperty('Drupal\Core\Menu\LocalTaskManager', 'discovery'); $property->setAccessible(TRUE); @@ -82,7 +91,7 @@ abstract class LocalTaskIntegrationTest extends UnitTestCase { $method = new \ReflectionMethod('Drupal\Core\Menu\LocalTaskManager', 'alterInfo'); $method->setAccessible(TRUE); - $method->invoke($manager, $module_handler, 'local_tasks'); + $method->invoke($manager, $this->moduleHandler, 'local_tasks'); $plugin_stub = $this->getMock('Drupal\Core\Menu\LocalTaskInterface'); $factory = $this->getMock('Drupal\Component\Plugin\Factory\FactoryInterface'); @@ -118,7 +127,12 @@ abstract class LocalTaskIntegrationTest extends UnitTestCase { */ protected function assertLocalTasks($route_name, $expected_tasks, $route_params = array()) { - $manager = $this->getLocalTaskManager($this->moduleList, $route_name, $route_params); + $directory_list = array(); + foreach ($this->directoryList as $key => $value) { + $directory_list[$key] = DRUPAL_ROOT . '/' . $value; + } + + $manager = $this->getLocalTaskManager($directory_list, $route_name, $route_params); $tmp_tasks = $manager->getLocalTasksForRoute($route_name);