.../Core/Menu/DefaultMenuLinkTreeManipulators.php | 14 ++++++++--- core/lib/Drupal/Core/Menu/MenuLinkTree.php | 13 +++++----- .../Drupal/Core/Menu/MenuParentFormSelector.php | 29 ++++++++++++++++++---- .../Core/Menu/MenuParentFormSelectorInterface.php | 7 +++++- core/modules/menu_ui/src/MenuForm.php | 14 +++++++++++ .../menu_ui/src/Tests/MenuCacheTagsTest.php | 3 +++ core/modules/menu_ui/src/Tests/MenuTest.php | 3 +++ .../src/Tests/PageCacheTagsIntegrationTest.php | 3 +++ .../system/src/Controller/SystemController.php | 21 +++++++++++++--- core/modules/system/src/SystemManager.php | 8 ++++++ core/modules/system/system.module | 6 +++++ .../modules/views_ui/src/Tests/DisplayPathTest.php | 4 +++ 12 files changed, 106 insertions(+), 19 deletions(-) diff --git a/core/lib/Drupal/Core/Menu/DefaultMenuLinkTreeManipulators.php b/core/lib/Drupal/Core/Menu/DefaultMenuLinkTreeManipulators.php index 686b4b7..05726dd 100644 --- a/core/lib/Drupal/Core/Menu/DefaultMenuLinkTreeManipulators.php +++ b/core/lib/Drupal/Core/Menu/DefaultMenuLinkTreeManipulators.php @@ -16,11 +16,10 @@ * Provides a couple of menu link tree manipulators. * * This class provides menu link tree manipulators to: - * - perform access checking + * - perform render cached menu-optimized access checking * - optimized node access checking * - generate a unique index for the elements in a tree and sorting by it * - flatten a tree (i.e. a 1-dimensional tree) - * - extract a subtree of the given tree according to the active trail */ class DefaultMenuLinkTreeManipulators { @@ -66,7 +65,13 @@ public function __construct(AccessManagerInterface $access_manager, AccountInter * * Sets the 'access' property to AccessResultInterface objects on menu link * tree elements. Descends into subtrees if the root of the subtree is - * accessible. + * accessible. Inaccessible subtrees are deleted, except at the root level, to + * be compatible with render caching. + * + * (This means that inaccessible links at the root level are *not* removed; + * it is up to the code doing something with the tree to exclude inaccessible + * links, just like MenuLinkTree::build() does. This allows those things to + * specify the necessary cacheability metadata.) * * This is compatible with render caching, because of cache context bubbling: * conditionally defined cache contexts (i.e. subtrees that are only @@ -105,6 +110,9 @@ public function checkAccess(array $tree, $level = 0) { if ($level > 0) { unset($tree[$key]); } + else { + $tree[$key]->subtree = []; + } } } return $tree; diff --git a/core/lib/Drupal/Core/Menu/MenuLinkTree.php b/core/lib/Drupal/Core/Menu/MenuLinkTree.php index afd0de5..9d95501 100644 --- a/core/lib/Drupal/Core/Menu/MenuLinkTree.php +++ b/core/lib/Drupal/Core/Menu/MenuLinkTree.php @@ -159,6 +159,13 @@ public function build(array $tree, $level = 0) { $tree_link_cacheability = new CacheableMetadata(); foreach ($tree as $data) { + /** @var \Drupal\Core\Menu\MenuLinkInterface $link */ + $link = $data->link; + // Generally we only deal with visible links, but just in case. + if (!$link->isEnabled()) { + continue; + } + // Gather the access cacheability of every item in the menu link tree, // including inaccessible items. This allows us to render cache the menu // tree, yet still automatically the rendered menu by the same cache @@ -177,12 +184,6 @@ public function build(array $tree, $level = 0) { } $class = ['menu-item']; - /** @var \Drupal\Core\Menu\MenuLinkInterface $link */ - $link = $data->link; - // Generally we only deal with visible links, but just in case. - if (!$link->isEnabled()) { - continue; - } // Set a class for the
  • -tag. Only set 'expanded' class if the link // also has visible children within the current tree. if ($data->hasChildren && !empty($data->subtree)) { diff --git a/core/lib/Drupal/Core/Menu/MenuParentFormSelector.php b/core/lib/Drupal/Core/Menu/MenuParentFormSelector.php index e5a1264..c014112 100644 --- a/core/lib/Drupal/Core/Menu/MenuParentFormSelector.php +++ b/core/lib/Drupal/Core/Menu/MenuParentFormSelector.php @@ -7,6 +7,7 @@ namespace Drupal\Core\Menu; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Component\Utility\Unicode; use Drupal\Core\StringTranslation\StringTranslationTrait; @@ -53,7 +54,7 @@ public function __construct(MenuLinkTreeInterface $menu_link_tree, EntityManager /** * {@inheritdoc} */ - public function getParentSelectOptions($id = '', array $menus = NULL) { + public function getParentSelectOptions($id = '', array $menus = NULL, CacheableMetadata &$cacheability = NULL) { if (!isset($menus)) { $menus = $this->getMenuOptions(); } @@ -72,7 +73,7 @@ public function getParentSelectOptions($id = '', array $menus = NULL) { array('callable' => 'menu.default_tree_manipulators:generateIndexAndSort'), ); $tree = $this->menuLinkTree->transform($tree, $manipulators); - $this->parentSelectOptionsTreeWalk($tree, $menu_name, '--', $options, $id, $depth_limit); + $this->parentSelectOptionsTreeWalk($tree, $menu_name, '--', $options, $id, $depth_limit, $cacheability); } return $options; } @@ -81,7 +82,8 @@ public function getParentSelectOptions($id = '', array $menus = NULL) { * {@inheritdoc} */ public function parentSelectElement($menu_parent, $id = '', array $menus = NULL) { - $options = $this->getParentSelectOptions($id, $menus); + $options_cacheability = new CacheableMetadata(); + $options = $this->getParentSelectOptions($id, $menus, $options_cacheability); // If no options were found, there is nothing to select. if ($options) { $element = array( @@ -98,6 +100,7 @@ public function parentSelectElement($menu_parent, $id = '', array $menus = NULL) // Only provide the default value if it is valid among the options. $element += array('#default_value' => $menu_parent); } + $options_cacheability->applyTo($element); return $element; } return array(); @@ -137,13 +140,29 @@ protected function getParentDepthLimit($id) { * An excluded menu link. * @param int $depth_limit * The maximum depth of menu links considered for the select options. + * @param \Drupal\Core\Cache\CacheableMetadata|NULL &$cacheability + * The object to add cacheability metadata to, if not NULL. */ - protected function parentSelectOptionsTreeWalk(array $tree, $menu_name, $indent, array &$options, $exclude, $depth_limit) { + protected function parentSelectOptionsTreeWalk(array $tree, $menu_name, $indent, array &$options, $exclude, $depth_limit, CacheableMetadata &$cacheability = NULL) { foreach ($tree as $element) { if ($element->depth > $depth_limit) { // Don't iterate through any links on this level. break; } + + // Collect the cacheability metadata of the access result, as well as the + // link. + if ($cacheability) { + $cacheability = $cacheability + ->merge(CacheableMetadata::createFromObject($element->access)) + ->merge(CacheableMetadata::createFromObject($element->link)); + } + + // Only show accessible links. + if (!$element->access->isAllowed()) { + continue; + } + $link = $element->link; if ($link->getPluginId() != $exclude) { $title = $indent . ' ' . Unicode::truncate($link->getTitle(), 30, TRUE, FALSE); @@ -152,7 +171,7 @@ protected function parentSelectOptionsTreeWalk(array $tree, $menu_name, $indent, } $options[$menu_name . ':' . $link->getPluginId()] = $title; if (!empty($element->subtree)) { - $this->parentSelectOptionsTreeWalk($element->subtree, $menu_name, $indent . '--', $options, $exclude, $depth_limit); + $this->parentSelectOptionsTreeWalk($element->subtree, $menu_name, $indent . '--', $options, $exclude, $depth_limit, $cacheability); } } } diff --git a/core/lib/Drupal/Core/Menu/MenuParentFormSelectorInterface.php b/core/lib/Drupal/Core/Menu/MenuParentFormSelectorInterface.php index 33ada87..8e096e5 100644 --- a/core/lib/Drupal/Core/Menu/MenuParentFormSelectorInterface.php +++ b/core/lib/Drupal/Core/Menu/MenuParentFormSelectorInterface.php @@ -7,6 +7,8 @@ namespace Drupal\Core\Menu; +use Drupal\Core\Cache\CacheableMetadata; + /** * Defines an interface for menu selector form elements and menu link options. */ @@ -21,12 +23,15 @@ * @param array $menus * Optional array of menu names as keys and titles as values to limit * the select options. If NULL, all menus will be included. + * @param \Drupal\Core\Cache\CacheableMetadata|NULL &$cacheability + * Optional cacheability metadata object, which will be populated based on + * the accessibility of the links and the cacheability of the links. * * @return array * Keyed array where the keys are contain a menu name and parent ID and * the values are a menu name or link title indented by depth. */ - public function getParentSelectOptions($id = '', array $menus = NULL); + public function getParentSelectOptions($id = '', array $menus = NULL, CacheableMetadata &$cacheability = NULL); /** * Gets a form element to choose a menu and parent. diff --git a/core/modules/menu_ui/src/MenuForm.php b/core/modules/menu_ui/src/MenuForm.php index c1b68d6..09d0c0a 100644 --- a/core/modules/menu_ui/src/MenuForm.php +++ b/core/modules/menu_ui/src/MenuForm.php @@ -8,6 +8,7 @@ namespace Drupal\menu_ui; use Drupal\Component\Utility\NestedArray; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Entity\EntityForm; use Drupal\Core\Entity\Query\QueryFactory; use Drupal\Core\Form\FormStateInterface; @@ -337,7 +338,15 @@ protected function buildOverviewForm(array &$form, FormStateInterface $form_stat */ protected function buildOverviewTreeForm($tree, $delta) { $form = &$this->overviewTreeForm; + $tree_access_cacheability = new CacheableMetadata(); foreach ($tree as $element) { + $tree_access_cacheability = $tree_access_cacheability->merge(CacheableMetadata::createFromObject($element->access)); + + // Only render accessible links. + if (!$element->access->isAllowed()) { + continue; + } + /** @var \Drupal\Core\Menu\MenuLinkInterface $link */ $link = $element->link; if ($link) { @@ -419,6 +428,11 @@ protected function buildOverviewTreeForm($tree, $delta) { $this->buildOverviewTreeForm($element->subtree, $delta); } } + + $tree_access_cacheability + ->merge(CacheableMetadata::createFromRenderArray($form)) + ->applyTo($form); + return $form; } diff --git a/core/modules/menu_ui/src/Tests/MenuCacheTagsTest.php b/core/modules/menu_ui/src/Tests/MenuCacheTagsTest.php index 45a9df7..adff9a0 100644 --- a/core/modules/menu_ui/src/Tests/MenuCacheTagsTest.php +++ b/core/modules/menu_ui/src/Tests/MenuCacheTagsTest.php @@ -54,6 +54,9 @@ public function testMenuBlock() { 'config:block_list', 'config:block.block.' . $block->id(), 'config:system.menu.llama', + // The cache contexts associated with the (in)accessible menu links are + // bubbled. + 'config:user.role.anonymous', ); $this->verifyPageCache($url, 'HIT', $expected_tags); diff --git a/core/modules/menu_ui/src/Tests/MenuTest.php b/core/modules/menu_ui/src/Tests/MenuTest.php index 722548f..593ba5d 100644 --- a/core/modules/menu_ui/src/Tests/MenuTest.php +++ b/core/modules/menu_ui/src/Tests/MenuTest.php @@ -555,6 +555,9 @@ function testUnpublishedNodeMenuItem() { $this->drupalLogin($this->adminUser); $this->drupalGet('admin/structure/menu/manage/' . $item->getMenuName()); $this->assertNoText($item->getTitle(), "Menu link pointing to unpublished node is only visible to users with 'bypass node access' permission"); + // The cache contexts associated with the (in)accessible menu links are + // bubbled. + $this->assertCacheContext('user.permissions'); } /** diff --git a/core/modules/page_cache/src/Tests/PageCacheTagsIntegrationTest.php b/core/modules/page_cache/src/Tests/PageCacheTagsIntegrationTest.php index dff09fe..95bd739 100644 --- a/core/modules/page_cache/src/Tests/PageCacheTagsIntegrationTest.php +++ b/core/modules/page_cache/src/Tests/PageCacheTagsIntegrationTest.php @@ -78,6 +78,9 @@ function testPageCacheTags() { 'theme', 'timezone', 'user.permissions', + // The cache contexts associated with the (in)accessible menu links are + // bubbled. + 'user.roles:authenticated', ]; // Full node page 1. diff --git a/core/modules/system/src/Controller/SystemController.php b/core/modules/system/src/Controller/SystemController.php index 13e2f73..20ea39e 100644 --- a/core/modules/system/src/Controller/SystemController.php +++ b/core/modules/system/src/Controller/SystemController.php @@ -8,6 +8,7 @@ namespace Drupal\system\Controller; use Drupal\Component\Serialization\Json; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Controller\ControllerBase; use Drupal\Core\Entity\Query\QueryFactory; use Drupal\Core\Extension\ThemeHandlerInterface; @@ -128,8 +129,16 @@ public function overview($link_id) { array('callable' => 'menu.default_tree_manipulators:generateIndexAndSort'), ); $tree = $this->menuLinkTree->transform($tree, $manipulators); + $tree_access_cacheability = new CacheableMetadata(); $blocks = array(); foreach ($tree as $key => $element) { + $tree_access_cacheability = $tree_access_cacheability->merge(CacheableMetadata::createFromObject($element->access)); + + // Only render accessible links. + if (!$element->access->isAllowed()) { + continue; + } + $link = $element->link; $block['title'] = $link->getTitle(); $block['description'] = $link->getDescription(); @@ -145,15 +154,19 @@ public function overview($link_id) { if ($blocks) { ksort($blocks); - return array( + $build = [ '#theme' => 'admin_page', '#blocks' => $blocks, - ); + ]; + $tree_access_cacheability->applyTo($build); + return $build; } else { - return array( + $build = [ '#markup' => $this->t('You do not have any administrative items.'), - ); + ]; + $tree_access_cacheability->applyTo($build); + return $build; } } diff --git a/core/modules/system/src/SystemManager.php b/core/modules/system/src/SystemManager.php index 11c3efc..33d6de5 100644 --- a/core/modules/system/src/SystemManager.php +++ b/core/modules/system/src/SystemManager.php @@ -200,6 +200,14 @@ public function getAdminBlock(MenuLinkInterface $instance) { ); $tree = $this->menuTree->transform($tree, $manipulators); foreach ($tree as $key => $element) { + // Only render accessible links. + if (!$element->access->isAllowed()) { + // @todo Bubble cacheability metadata of both accessible and + // inaccessible links. Currently made impossible by the way admin + // blocks are rendered. + continue; + } + /** @var $link \Drupal\Core\Menu\MenuLinkInterface */ $link = $element->link; $content[$key]['title'] = $link->getTitle(); diff --git a/core/modules/system/system.module b/core/modules/system/system.module index f2dcc7c..95a8c19 100644 --- a/core/modules/system/system.module +++ b/core/modules/system/system.module @@ -1158,6 +1158,12 @@ function system_get_module_admin_tasks($module, array $info) { $admin_tasks = array(); foreach ($tree as $element) { + if (!$element->access->isAllowed()) { + // @todo Bubble cacheability metadata of both accessible and inaccessible + // links. Currently made impossible by the way admin tasks are rendered. + continue; + } + $link = $element->link; if ($link->getProvider() != $module) { continue; diff --git a/core/modules/views_ui/src/Tests/DisplayPathTest.php b/core/modules/views_ui/src/Tests/DisplayPathTest.php index 5b2a022..cc6826f 100644 --- a/core/modules/views_ui/src/Tests/DisplayPathTest.php +++ b/core/modules/views_ui/src/Tests/DisplayPathTest.php @@ -143,6 +143,10 @@ public function testMenuOptions() { '-- Compose tips (disabled)', '-- Test menu link', ], $menu_options); + + // The cache contexts associated with the (in)accessible menu links are + // bubbled. + $this->assertCacheContext('user.permissions'); } }