Issue #2217985 by Berdir, Wim Leers: Replace the custom menu caching strategy in Toolbar with Core's standard caching

core/modules/toolbar/js/views/ToolbarVisualView.js

@@ -267,8 +267,7 @@
       //   (3) The orientation of the tray is vertical.
       if (!this.model.get('areSubtreesLoaded') && typeof $activeTab.data('drupal-subtrees') !== 'undefined' && orientation === 'vertical') {
         var subtreesHash = drupalSettings.toolbar.subtreesHash;
-        var langcode = drupalSettings.toolbar.langcode;
+        var endpoint = Drupal.url('toolbar/subtrees/' + subtreesHash);
-        var endpoint = Drupal.url('toolbar/subtrees/' + subtreesHash + '/' + langcode);
         var cachedSubtreesHash = localStorage.getItem('Drupal.toolbar.subtreesHash');
         var cachedSubtrees = JSON.parse(localStorage.getItem('Drupal.toolbar.subtrees'));
         var isVertical = this.model.get('orientation') === 'vertical';

core/modules/toolbar/src/Controller/ToolbarController.php

@@ -46,14 +46,12 @@ class ToolbarController extends ControllerBase {
    *
    * @param string $hash
    *   The hash of the toolbar subtrees.
-   * @param string $langcode
-   *   The langcode of the requested site, NULL if none given.
    *
    * @return \Drupal\Core\Access\AccessResultInterface
    *   The access result.
    */
-  public function checkSubTreeAccess($hash, $langcode) {
+  public function checkSubTreeAccess($hash) {
-    return AccessResult::allowedIf($this->currentUser()->hasPermission('access toolbar') && $hash == _toolbar_get_subtrees_hash($langcode))->cachePerPermissions();
+    return AccessResult::allowedIf($this->currentUser()->hasPermission('access toolbar') && $hash == _toolbar_get_subtrees_hash()[0])->cachePerPermissions();
   }
 
 }

core/modules/toolbar/src/Tests/ToolbarAdminMenuTest.php

@@ -209,98 +209,11 @@ class ToolbarAdminMenuTest extends WebTestBase {
     $this->assertEqual($admin_user_2_hash, $new_subtree_hash, 'The user-specific subtree menu hash has not been updated.');
   }
 
-  /**
-   * Tests that all toolbar cache entries for a user are cleared with a cache
-   * tag for that user, i.e. cache entries for all languages for that user.
-   */
-  function testCacheClearByCacheTag() {
-    // Test that the toolbar admin menu subtrees cache is invalidated for a user
-    // across multiple languages.
-    $this->drupalLogin($this->adminUser);
-    $toolbarCache = $this->container->get('cache.toolbar');
-    $admin_user_id = $this->adminUser->id();
-    $admin_user_2_id = $this->adminUser2->id();
-
-    // Assert that a cache tag in the toolbar cache under the key "user" exists
-    // for adminUser against the language "en".
-    $cache = $toolbarCache->get('toolbar_' . $admin_user_id . ':' . 'en');
-    $this->assertEqual(in_array('user:' . $admin_user_id, $cache->tags), 'A cache tag in the toolbar cache under the key "user" exists for admin_user against the language "en".');
-
-    // Assert that no toolbar cache exists for adminUser against the
-    // language "fr".
-    $cache = $toolbarCache->get('toolbar_' . $admin_user_id . ':' . 'fr');
-    $this->assertFalse($cache, 'No toolbar cache exists for admin_user against the language "fr".');
-
-    // Install a second language.
-    $edit = array(
-      'predefined_langcode' => 'fr',
-    );
-    $this->drupalPostForm('admin/config/regional/language/add', $edit, 'Add language');
-
-    // Request a page in 'fr' to update the cache.
-    $this->drupalGet('fr/test-page');
-    $this->assertResponse(200);
-
-    // Assert that a cache tag in the toolbar cache under the key "user" exists
-    // for adminUser against the language "fr".
-    $cache = $toolbarCache->get('toolbar_' . $admin_user_id . ':' . 'fr');
-    $this->assertEqual(in_array('user:' . $admin_user_id, $cache->tags), 'A cache tag in the toolbar cache under the key "user" exists for admin_user against the language "fr".');
-
-    // Log in the adminUser2 user. We will use this user as a control to
-    // verify that clearing a cache tag for adminUser does not clear the cache
-    // for adminUser2.
-    $this->drupalLogin($this->adminUser2);
-
-    // Request a page in 'en' to create the cache.
-    $this->drupalGet('test-page');
-    $this->assertResponse(200);
-    // Assert that a cache tag in the toolbar cache under the key "user" exists
-    // for adminUser2 against the language "en".
-    $cache = $toolbarCache->get('toolbar_' . $admin_user_2_id . ':' . 'en');
-    $this->assertEqual(in_array('user:' . $admin_user_2_id, $cache->tags), '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');
-    $this->assertResponse(200);
-    // Assert that a cache tag in the toolbar cache under the key "user" exists
-    // for adminUser against the language "fr".
-    $cache = $toolbarCache->get('toolbar_' . $admin_user_2_id . ':' . 'fr');
-    $this->assertEqual(in_array('user:' . $admin_user_2_id, $cache->tags), 'A cache tag in the toolbar cache under the key "user" exists for admin_user_2 against the language "fr".');
-
-    // Log in the admin user and clear the caches for this user using a tag.
-    $this->drupalLogin($this->adminUser);
-    Cache::invalidateTags(array('user:' . $admin_user_id));
-
-    // Assert that no toolbar cache exists for adminUser against the
-    // language "en".
-    $cache = $toolbarCache->get($admin_user_id . ':' . 'en');
-    $this->assertFalse($cache, 'No toolbar cache exists for admin_user against the language "en".');
-
-    // Assert that no toolbar cache exists for adminUser against the
-    // language "fr".
-    $cache = $toolbarCache->get($admin_user_id . ':' . 'fr');
-    $this->assertFalse($cache, 'No toolbar cache exists for admin_user against the language "fr".');
-
-    // Log in adminUser2 and verify that this user's caches still exist.
-    $this->drupalLogin($this->adminUser2);
-
-    // Assert that a cache tag in the toolbar cache under the key "user" exists
-    // for adminUser2 against the language "en".
-    $cache = $toolbarCache->get('toolbar_' . $admin_user_2_id . ':' . 'en');
-    $this->assertEqual(in_array('user:' . $admin_user_2_id, $cache->tags), 'A cache tag in the toolbar cache under the key "user" exists for admin_user_2 against the language "en".');
-
-    // Assert that a cache tag in the toolbar cache under the key "user" exists
-    // for adminUser2 against the language "fr".
-    $cache = $toolbarCache->get('toolbar_' . $admin_user_2_id . ':' . 'fr');
-    $this->assertEqual(in_array('user:' . $admin_user_2_id, $cache->tags), 'A cache tag in the toolbar cache under the key "user" exists for admin_user_2 against the language "fr".');
-  }
-
   /**
    * Tests that changes to a user account by another user clears the changed
    * account's toolbar cached, not the user's who took the action.
    */
   function testNonCurrentUserAccountUpdates() {
-    $toolbarCache = $this->container->get('cache.toolbar');
     $admin_user_id = $this->adminUser->id();
     $admin_user_2_id = $this->adminUser2->id();
     $this->hash = $this->getSubtreesHash();
@@ -337,9 +250,7 @@ class ToolbarAdminMenuTest extends WebTestBase {
    * Tests that toolbar cache is cleared when string translations are made.
    */
   function testLocaleTranslationSubtreesHashCacheClear() {
-    $toolbarCache = $this->container->get('cache.toolbar');
     $admin_user = $this->adminUser;
-    $admin_user_id = $this->adminUser->id();
     // User to translate and delete string.
     $translate_user = $this->drupalCreateUser(array('translate interface', 'access administration pages'));
 
@@ -373,11 +284,6 @@ class ToolbarAdminMenuTest extends WebTestBase {
     $this->drupalGet($langcode . '/test-page');
     $this->assertResponse(200);
 
-    // Assert that a cache tag in the toolbar cache under the key "user" exists
-    // for adminUser against the language "xx".
-    $cache = $toolbarCache->get('toolbar_' . $admin_user_id . ':' . $langcode);
-    $this->assertEqual(in_array('user:' . $admin_user_id, $cache->tags), 'A cache tag in the toolbar cache under the key "user" exists for admin_user against the language "xx".');
-
     // Get a baseline hash for the admin menu subtrees before translating one
     // of the menu link items.
     $original_subtree_hash = $this->getSubtreesHash();
@@ -435,28 +341,6 @@ class ToolbarAdminMenuTest extends WebTestBase {
 
     $this->drupalGetJSON('toolbar/subtrees/' . $subtrees_hash);
     $this->assertResponse('200');
-
-    // Test that the subtrees hash changes with a different language code and
-    // that JSON is returned when a language code is specified.
-    // Create a new language with the langcode 'xx'.
-    $langcode = 'xx';
-    // The English name for the language. This will be translated.
-    $name = $this->randomMachineName(16);
-    $edit = array(
-      'predefined_langcode' => 'custom',
-      'langcode' => $langcode,
-      'label' => $name,
-      'direction' => LanguageInterface::DIRECTION_LTR,
-    );
-    $this->drupalPostForm('admin/config/regional/language/add', $edit, t('Add custom language'));
-
-    // Get a page with the new language langcode in the URL.
-    $this->drupalGet('xx/test-page');
-    // Request a new page to refresh the drupalSettings object.
-    $subtrees_hash = $this->getSubtreesHash();
-
-    $this->drupalGetJSON('toolbar/subtrees/' . $subtrees_hash . '/' . $langcode);
-    $this->assertResponse('200');
   }
 
   /**

core/modules/toolbar/toolbar.module

@@ -6,6 +6,7 @@
  */
 
 use Drupal\Core\Cache\Cache;
+use Drupal\Core\Cache\CacheableMetadata;
 use Drupal\Core\Menu\MenuTreeParameters;
 use Drupal\Core\Render\Element;
 use Drupal\Core\Routing\RouteMatchInterface;
@@ -13,7 +14,6 @@ use Drupal\Core\Template\Attribute;
 use Drupal\Component\Datetime\DateTimePlus;
 use Drupal\Component\Utility\Crypt;
 use Drupal\Component\Utility\SafeMarkup;
-use Drupal\user\Entity\Role;
 use Drupal\Core\Url;
 
 /**
@@ -158,10 +158,9 @@ function toolbar_toolbar() {
   // toolbar_subtrees route. We provide the JavaScript requesting that JSONP
   // script here with the hash parameter that is needed for that route.
   // @see toolbar_subtrees_jsonp()
-  $langcode = \Drupal::languageManager()->getCurrentLanguage()->getId();
+  list($hash, $hash_cacheability) = _toolbar_get_subtrees_hash();
   $subtrees_attached['drupalSettings']['toolbar'] = [
-    'subtreesHash' => _toolbar_get_subtrees_hash($langcode),
+    'subtreesHash' => $hash,
-    'langcode' => $langcode,
   ];
 
   // The administration element has a link that is themed to correspond to
@@ -198,6 +197,7 @@ function toolbar_toolbar() {
     ),
     '#weight' => -15,
   );
+  $hash_cacheability->applyTo($items['administration']);
 
   return $items;
 }
@@ -269,8 +269,30 @@ function toolbar_menu_navigation_links(array $tree) {
 
 /**
  * Returns the rendered subtree of each top-level toolbar link.
+ *
+ * @return array
+ *   An array with the following key-value pairs:
+ *   - 'subtrees': the rendered subtrees
+ *   - 'cacheability: the associated cacheability.
  */
 function toolbar_get_rendered_subtrees() {
+  $data = [
+    '#pre_render' => ['_toolbar_do_get_rendered_subtrees'],
+    '#cache' => [
+      'keys' => [
+        'toolbar_rendered_subtrees',
+      ],
+    ],
+    '#cache_properties' => ['#subtrees'],
+  ];
+  \Drupal::service('renderer')->renderPlain($data);
+  return [$data['#subtrees'], CacheableMetadata::createFromRenderArray($data)];
+}
+
+/**
+ * #pre_render callback for toolbar_get_rendered_subtrees().
+ */
+function _toolbar_do_get_rendered_subtrees(array $data) {
   $menu_tree = \Drupal::service('toolbar.menu_tree');
   $parameters = new MenuTreeParameters();
   $parameters->setRoot('system.admin')->excludeRoot()->setMaxDepth(3)->onlyEnabledLinks();
@@ -282,12 +304,15 @@ function toolbar_get_rendered_subtrees() {
   );
   $tree = $menu_tree->transform($tree, $manipulators);
   $subtrees = array();
+  // Calculated the combined cacheability of all subtrees.
+  $cacheability = new CacheableMetadata();
   foreach ($tree as $element) {
     /** @var \Drupal\Core\Menu\MenuLinkInterface $link */
     $link = $element->link;
     if ($element->subtree) {
       $subtree = $menu_tree->build($element->subtree);
       $output = \Drupal::service('renderer')->renderPlain($subtree);
+      $cacheability = $cacheability->merge(CacheableMetadata::createFromRenderArray($subtree));
     }
     else {
       $output = '';
@@ -298,35 +323,24 @@ function toolbar_get_rendered_subtrees() {
 
     $subtrees[$id] = $output;
   }
-  return $subtrees;
+
+  // Store the subtrees, along with the cacheability metadata.
+  $cacheability->applyTo($data);
+  $data['#subtrees'] = $subtrees;
+
+  return $data;
 }
 
 /**
  * Returns the hash of the per-user rendered toolbar subtrees.
  *
- * @param string $langcode
- *   The langcode of the current request.
- *
  * @return string
  *   The hash of the admin_menu subtrees.
  */
-function _toolbar_get_subtrees_hash($langcode) {
+function _toolbar_get_subtrees_hash() {
-  $uid = \Drupal::currentUser()->id();
+  list($subtrees, $cacheability) = toolbar_get_rendered_subtrees();
-  $cid = _toolbar_get_user_cid($uid, $langcode);
-  if ($cache = \Drupal::cache('toolbar')->get($cid)) {
-    $hash = $cache->data;
-  }
-  else {
-    $subtrees = toolbar_get_rendered_subtrees();
   $hash = Crypt::hashBase64(serialize($subtrees));
-    // Cache using a tag 'user' so that we can invalidate all user-specific
+  return [$hash, $cacheability];
-    // caches later, based on the user's ID regardless of language.
-    // Clear the cache when the 'locale' tag is deleted. This ensures a fresh
-    // subtrees rendering when string translations are made.
-    $role_list_cache_tags = \Drupal::entityManager()->getDefinition('user_role')->getListCacheTags();
-    \Drupal::cache('toolbar')->set($cid, $hash, Cache::PERMANENT, Cache::mergeTags(array('user:' . $uid, 'locale', 'config:system.menu.admin'), $role_list_cache_tags));
-  }
-  return $hash;
 }
 
 /**

core/modules/toolbar/toolbar.routing.yml

@@ -1,5 +1,5 @@
 toolbar.subtrees:
-  path: '/toolbar/subtrees/{hash}/{langcode}'
+  path: '/toolbar/subtrees/{hash}'
   defaults:
     _controller: '\Drupal\toolbar\Controller\ToolbarController::subtreesJsonp'
     langcode: null