diff --git a/core/lib/Drupal/Core/Menu/DefaultMenuLinkTreeManipulators.php b/core/lib/Drupal/Core/Menu/DefaultMenuLinkTreeManipulators.php index d22f53888e007648584217550a8af85705f07368..9461683c15edd69827781a8135a210471f5c55ae 100644 --- a/core/lib/Drupal/Core/Menu/DefaultMenuLinkTreeManipulators.php +++ b/core/lib/Drupal/Core/Menu/DefaultMenuLinkTreeManipulators.php @@ -8,6 +8,7 @@ namespace Drupal\Core\Menu; use Drupal\Core\Access\AccessManagerInterface; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\Query\QueryFactory; use Drupal\Core\Session\AccountInterface; @@ -15,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 { @@ -63,12 +63,24 @@ public function __construct(AccessManagerInterface $access_manager, AccountInter /** * Performs access checks of a menu tree. * - * Removes menu links from the given menu tree whose links are inaccessible - * for the current user, sets the 'access' property to TRUE on tree elements - * that are accessible for the current user. + * Sets the 'access' property to AccessResultInterface objects on menu link + * tree elements. Descends into subtrees if the root of the subtree is + * accessible. Inaccessible subtrees are deleted, except the top-level + * inaccessible link, to be compatible with render caching. * - * Makes the resulting menu tree impossible to render cache, unless render - * caching per user is acceptable. + * (This means that top-level inaccessible links 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 + * accessible to some users) will bubble just like they do for render arrays. + * This is why inaccessible subtrees are deleted, except at the top-level + * inaccessible link: if we didn't keep the first (depth-wise) inaccessible + * link, we wouldn't be able to know which cache contexts would cause those + * subtrees to become accessible again, thus forcing us to conclude that that + * subtree is unconditionally inaccessible. * * @param \Drupal\Core\Menu\MenuLinkTreeElement[] $tree * The menu link tree to manipulate. @@ -83,13 +95,27 @@ public function checkAccess(array $tree) { if (!isset($element->access)) { $tree[$key]->access = $this->menuLinkCheckAccess($element->link); } - if ($tree[$key]->access) { + if ($tree[$key]->access->isAllowed()) { if ($tree[$key]->subtree) { $tree[$key]->subtree = $this->checkAccess($tree[$key]->subtree); } } else { - unset($tree[$key]); + // Replace the link with an InaccessibleMenuLink object, so that if it + // is accidentally rendered, no sensitive information is divulged. + $tree[$key]->link = new InaccessibleMenuLink($tree[$key]->link); + // Always keep top-level inaccessible links: their cacheability metadata + // that indicates why they're not accessible by the current user must be + // bubbled. Otherwise, those subtrees will not be varied by any cache + // contexts at all, therefore forcing them to remain empty for all users + // unless some other part of the menu link tree accidentally varies by + // the same cache contexts. + // For deeper levels, we *can* remove the subtrees and therefore also + // not perform access checking on the subtree, thanks to bubbling/cache + // redirects. This therefore allows us to still do significantly less + // work in case of inaccessible subtrees, which is the entire reason why + // this deletes subtrees in the first place. + $tree[$key]->subtree = []; } } return $tree; @@ -120,17 +146,19 @@ public function checkNodeAccess(array $tree) { // query rewrite as well as not checking for the node status. The // 'view own unpublished nodes' permission is ignored to not require cache // entries per user. + $access_result = AccessResult::allowed()->cachePerPermissions(); if ($this->account->hasPermission('bypass node access')) { $query->accessCheck(FALSE); } else { + $access_result->addCacheContexts(['user.node_grants:view']); $query->condition('status', NODE_PUBLISHED); } $nids = $query->execute(); foreach ($nids as $nid) { foreach ($node_links[$nid] as $key => $link) { - $node_links[$nid][$key]->access = TRUE; + $node_links[$nid][$key]->access = $access_result; } } } @@ -155,7 +183,7 @@ protected function collectNodeLinks(array &$tree, array &$node_links) { $nid = $element->link->getRouteParameters()['node']; $node_links[$nid][$key] = $element; // Deny access by default. checkNodeAccess() will re-add it. - $element->access = FALSE; + $element->access = AccessResult::neutral(); } if ($element->hasChildren) { $this->collectNodeLinks($element->subtree, $node_links); @@ -169,24 +197,27 @@ protected function collectNodeLinks(array &$tree, array &$node_links) { * @param \Drupal\Core\Menu\MenuLinkInterface $instance * The menu link instance. * - * @return bool - * TRUE if the current user can access the link, FALSE otherwise. + * @return \Drupal\Core\Access\AccessResultInterface + * The access result. */ protected function menuLinkCheckAccess(MenuLinkInterface $instance) { + $access_result = NULL; if ($this->account->hasPermission('link to any page')) { - return TRUE; - } - // Use the definition here since that's a lot faster than creating a Url - // object that we don't need. - $definition = $instance->getPluginDefinition(); - // 'url' should only be populated for external links. - if (!empty($definition['url']) && empty($definition['route_name'])) { - $access = TRUE; + $access_result = AccessResult::allowed(); } else { - $access = $this->accessManager->checkNamedRoute($definition['route_name'], $definition['route_parameters'], $this->account); + // Use the definition here since that's a lot faster than creating a Url + // object that we don't need. + $definition = $instance->getPluginDefinition(); + // 'url' should only be populated for external links. + if (!empty($definition['url']) && empty($definition['route_name'])) { + $access_result = AccessResult::allowed(); + } + else { + $access_result = $this->accessManager->checkNamedRoute($definition['route_name'], $definition['route_parameters'], $this->account, TRUE); + } } - return $access; + return $access_result->cachePerPermissions(); } /** diff --git a/core/lib/Drupal/Core/Menu/InaccessibleMenuLink.php b/core/lib/Drupal/Core/Menu/InaccessibleMenuLink.php new file mode 100644 index 0000000000000000000000000000000000000000..075e3569d1f9b421fdd07d73e5dd8d1b5ff42a97 --- /dev/null +++ b/core/lib/Drupal/Core/Menu/InaccessibleMenuLink.php @@ -0,0 +1,86 @@ +<?php + +/** + * @file + * Contains \Drupal\Core\Menu\InaccessibleMenuLink. + */ + +namespace Drupal\Core\Menu; + +use Drupal\Component\Plugin\Exception\PluginException; +use Drupal\Component\Utility\SafeMarkup; + +/** + * A menu link plugin for wrapping another menu link, in sensitive situations. + * + * @see \Drupal\Core\Menu\DefaultMenuLinkTreeManipulators::checkAccess() + */ +class InaccessibleMenuLink extends MenuLinkBase { + + /** + * The wrapped menu link. + * + * @var \Drupal\Core\Menu\MenuLinkInterface + */ + protected $wrappedLink; + + /** + * Constructs a new InaccessibleMenuLink. + * + * @param \Drupal\Core\Menu\MenuLinkInterface $wrapped_link + * The menu link to wrap. + */ + public function __construct(MenuLinkInterface $wrapped_link) { + $this->wrappedLink = $wrapped_link; + $plugin_definition = [ + 'route_name' => '<front>', + 'route_parameters' => [], + 'url' => NULL, + ] + $this->wrappedLink->getPluginDefinition(); + parent::__construct([], $this->wrappedLink->getPluginId(), $plugin_definition); + } + + /** + * {@inheritdoc} + */ + public function getTitle() { + return $this->t('Inaccessible'); + } + + /** + * {@inheritdoc} + */ + public function getDescription() { + return ''; + } + + /** + * {@inheritdoc} + */ + public function getCacheContexts() { + return $this->wrappedLink->getCacheContexts(); + } + + /** + * {@inheritdoc} + */ + public function getCacheTags() { + return $this->wrappedLink->getCacheTags(); + } + + /** + * {@inheritdoc} + */ + public function getCacheMaxAge() { + return $this->wrappedLink->getCacheMaxAge(); + } + + + /** + * {@inheritdoc} + */ + public function updateLink(array $new_definition_values, $persist) { + throw new PluginException(SafeMarkup::format('Inaccessible menu link plugins do not support updating')); + } + +} diff --git a/core/lib/Drupal/Core/Menu/MenuLinkTree.php b/core/lib/Drupal/Core/Menu/MenuLinkTree.php index 44eed04456c59b127630318b9eb74559cdcd62f8..090293c101593f64a35e3b584415fe4cafe6e886 100644 --- a/core/lib/Drupal/Core/Menu/MenuLinkTree.php +++ b/core/lib/Drupal/Core/Menu/MenuLinkTree.php @@ -8,6 +8,8 @@ namespace Drupal\Core\Menu; use Drupal\Component\Utility\NestedArray; +use Drupal\Core\Access\AccessResultInterface; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Cache\CacheBackendInterface; use Drupal\Core\Controller\ControllerResolverInterface; use Drupal\Core\Routing\RouteMatchInterface; @@ -152,17 +154,97 @@ public function transform(array $tree, array $manipulators) { /** * {@inheritdoc} */ - public function build(array $tree, $level = 0) { + public function build(array $tree) { + $tree_access_cacheability = new CacheableMetadata(); + $tree_link_cacheability = new CacheableMetadata(); + $items = $this->buildItems($tree, $tree_access_cacheability, $tree_link_cacheability); + + $build = []; + + // Apply the tree-wide gathered access cacheability metadata and link + // cacheability metadata to the render array. This ensures that the + // rendered menu is varied by the cache contexts that the access results + // and (dynamic) links depended upon, and invalidated by the cache tags + // that may change the values of the access results and links. + $tree_cacheability = $tree_access_cacheability->merge($tree_link_cacheability); + $tree_cacheability->applyTo($build); + + if ($items) { + // Make sure drupal_render() does not re-order the links. + $build['#sorted'] = TRUE; + // Get the menu name from the last link. + $item = end($items); + $link = $item['original_link']; + $menu_name = $link->getMenuName(); + // Add the theme wrapper for outer markup. + // Allow menu-specific theme overrides. + $build['#theme'] = 'menu__' . strtr($menu_name, '-', '_'); + $build['#items'] = $items; + // Set cache tag. + $build['#cache']['tags'][] = 'config:system.menu.' . $menu_name; + } + + return $build; + } + + /** + * Builds the #items property for a menu tree's renderable array. + * + * Helper function for ::build(). + * + * @param \Drupal\Core\Menu\MenuLinkTreeElement[] $tree + * A data structure representing the tree, as returned from + * MenuLinkTreeInterface::load(). + * @param \Drupal\Core\Cache\CacheableMetadata &$tree_access_cacheability + * Internal use only. The aggregated cacheability metadata for the access + * results across the entire tree. Used when rendering the root level. + * @param \Drupal\Core\Cache\CacheableMetadata &$tree_link_cacheability + * Internal use only. The aggregated cacheability metadata for the menu + * links across the entire tree. Used when rendering the root level. + * + * @return array + * The value to use for the #items property of a renderable menu. + * + * @throws \DomainException + */ + protected function buildItems(array $tree, CacheableMetadata &$tree_access_cacheability, CacheableMetadata &$tree_link_cacheability) { $items = array(); foreach ($tree as $data) { - $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; } + + if ($data->access !== NULL && !$data->access instanceof AccessResultInterface) { + throw new \DomainException('MenuLinkTreeElement::access must be either NULL or an AccessResultInterface object.'); + } + + // 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 vary the rendered menu by the same cache + // contexts that the access results vary by. + // However, if $data->access is not an AccessResultInterface object, this + // will still render the menu link, because this method does not want to + // require access checking to be able to render a menu tree. + if ($data->access instanceof AccessResultInterface) { + $tree_access_cacheability = $tree_access_cacheability->merge(CacheableMetadata::createFromObject($data->access)); + } + + // Gather the cacheability of every item in the menu link tree. Some links + // may be dynamic: they may have a dynamic text (e.g. a "Hi, <user>" link + // text, which would vary by 'user' cache context), or a dynamic route + // name or route parameters. + $tree_link_cacheability = $tree_link_cacheability->merge(CacheableMetadata::createFromObject($data->link)); + + // Only render accessible links. + if ($data->access instanceof AccessResultInterface && !$data->access->isAllowed()) { + continue; + } + + $class = ['menu-item']; // Set a class for the <li>-tag. Only set 'expanded' class if the link // also has visible children within the current tree. if ($data->hasChildren && !empty($data->subtree)) { @@ -176,14 +258,15 @@ public function build(array $tree, $level = 0) { $class[] = 'menu-item--active-trail'; } - // Allow menu-specific theme overrides. + // Note: links are rendered in the menu.html.twig template; and they + // automatically bubble their associated cacheability metadata. $element = array(); $element['attributes'] = new Attribute(); $element['attributes']['class'] = $class; $element['title'] = $link->getTitle(); $element['url'] = $link->getUrlObject(); $element['url']->setOption('set_active_class', TRUE); - $element['below'] = $data->subtree ? $this->build($data->subtree, $level + 1) : array(); + $element['below'] = $data->subtree ? $this->buildItems($data->subtree, $tree_access_cacheability, $tree_link_cacheability) : array(); if (isset($data->options)) { $element['url']->setOptions(NestedArray::mergeDeep($element['url']->getOptions(), $data->options)); } @@ -192,26 +275,7 @@ public function build(array $tree, $level = 0) { $items[$link->getPluginId()] = $element; } - if (!$items) { - return array(); - } - elseif ($level == 0) { - $build = array(); - // Make sure drupal_render() does not re-order the links. - $build['#sorted'] = TRUE; - // Get the menu name from the last link. - $menu_name = $link->getMenuName(); - // Add the theme wrapper for outer markup. - // Allow menu-specific theme overrides. - $build['#theme'] = 'menu__' . strtr($menu_name, '-', '_'); - $build['#items'] = $items; - // Set cache tag. - $build['#cache']['tags'][] = 'config:system.menu.' . $menu_name; - return $build; - } - else { - return $items; - } + return $items; } /** diff --git a/core/lib/Drupal/Core/Menu/MenuLinkTreeElement.php b/core/lib/Drupal/Core/Menu/MenuLinkTreeElement.php index a52b6c2f858b506161cf4293c74329af7842d2b9..7358e4f1da4b3fe9aabb897f5e70720c56d2a3a0 100644 --- a/core/lib/Drupal/Core/Menu/MenuLinkTreeElement.php +++ b/core/lib/Drupal/Core/Menu/MenuLinkTreeElement.php @@ -70,10 +70,10 @@ class MenuLinkTreeElement { /** * Whether this link is accessible by the current user. * - * If the value is NULL the access was not determined yet, if Boolean it was - * determined already. + * If the value is NULL the access was not determined yet, if an access result + * object, it was determined already. * - * @var bool|NULL + * @var \Drupal\Core\Access\AccessResultInterface|NULL */ public $access; diff --git a/core/lib/Drupal/Core/Menu/MenuParentFormSelector.php b/core/lib/Drupal/Core/Menu/MenuParentFormSelector.php index e5a1264e9abbb4b22cf1cdcbaf64441d13fa0b8e..c01411286ca59983243f051c33b36fa94c25d7ea 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 33ada871ca99b384bafa05304d848a8fdf0343b4..8e096e5b09e214d0f3b6ca02cb15685cf680c9d4 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 @@ interface MenuParentFormSelectorInterface { * @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/lib/Drupal/Core/Render/Element.php b/core/lib/Drupal/Core/Render/Element.php index e4b22db7ab369cdb2ab97ce828c7098c6da625b5..c0d3a4a527b7efb640e1bff29f913eb5874ec7a1 100644 --- a/core/lib/Drupal/Core/Render/Element.php +++ b/core/lib/Drupal/Core/Render/Element.php @@ -177,4 +177,20 @@ public static function setAttributes(array &$element, array $map) { } } + /** + * Indicates whether the given element is empty. + * + * An element that only has #cache set is considered empty, because it will + * render to the empty string. + * + * @param array $elements + * The element. + * + * @return bool + * Whether the given element is empty. + */ + public static function isEmpty(array $elements) { + return empty($elements) || (count($elements) === 1 && array_keys($elements) === ['#cache']); + } + } diff --git a/core/modules/block/src/BlockViewBuilder.php b/core/modules/block/src/BlockViewBuilder.php index 216dfa85db2952b22a3bff6451d6d47cc6c1ea87..a68b6eb896ab3d1bd029b5390fa6182c16b82faf 100644 --- a/core/modules/block/src/BlockViewBuilder.php +++ b/core/modules/block/src/BlockViewBuilder.php @@ -9,9 +9,11 @@ use Drupal\Component\Utility\SafeMarkup; use Drupal\Core\Cache\Cache; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Entity\EntityViewBuilder; use Drupal\Core\Entity\EntityViewBuilderInterface; use Drupal\Core\Entity\EntityInterface; +use Drupal\Core\Render\Element; /** * Provides a Block view builder. @@ -103,7 +105,7 @@ public function buildBlock($build) { // Remove the block entity from the render array, to ensure that blocks // can be rendered without the block config entity. unset($build['#block']); - if (!empty($content)) { + if ($content !== NULL && !Element::isEmpty($content)) { // Place the $content returned by the block plugin into a 'content' child // element, as a way to allow the plugin to have complete control of its // properties and rendering (e.g., its own #theme) without conflicting @@ -122,6 +124,8 @@ public function buildBlock($build) { } $build['content'] = $content; } + // Either the block's content is completely empty, or it consists only of + // cacheability metadata. else { // Abort rendering: render as the empty string and ensure this block is // render cached, so we can avoid the work of having to repeatedly @@ -131,6 +135,15 @@ public function buildBlock($build) { '#markup' => '', '#cache' => $build['#cache'], ); + // If $content is not empty, then it contains cacheability metadata, and + // we must merge it with the existing cacheability metadata. This allows + // blocks to be empty, yet still bubble cacheability metadata, to indicate + // why they are empty. + if (!empty($content)) { + CacheableMetadata::createFromRenderArray($build) + ->merge(CacheableMetadata::createFromRenderArray($content)) + ->applyTo($build); + } } return $build; } diff --git a/core/modules/book/src/Plugin/Block/BookNavigationBlock.php b/core/modules/book/src/Plugin/Block/BookNavigationBlock.php index 6a0a15d39a77f30c0c14be9070e36e020413a077..1a686220ae0482affbcbac30f1b674734ec7b6a6 100644 --- a/core/modules/book/src/Plugin/Block/BookNavigationBlock.php +++ b/core/modules/book/src/Plugin/Block/BookNavigationBlock.php @@ -193,7 +193,7 @@ public function getCacheContexts() { /** * {@inheritdoc} * - * @todo Make cacheable as part of https://www.drupal.org/node/1805054. + * @todo Make cacheable in https://www.drupal.org/node/2483181 */ public function getCacheMaxAge() { return 0; diff --git a/core/modules/menu_ui/menu_ui.module b/core/modules/menu_ui/menu_ui.module index 471f0a960e3e89c66650ec45188d3d06649d8950..bad8d9f9fed16ee86b4fc04d442fb82b298f7d89 100644 --- a/core/modules/menu_ui/menu_ui.module +++ b/core/modules/menu_ui/menu_ui.module @@ -8,6 +8,7 @@ * used for navigation. */ +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Block\BlockPluginInterface; use Drupal\Core\Link; @@ -402,7 +403,8 @@ function menu_ui_form_node_type_form_alter(&$form, FormStateInterface $form_stat // To avoid an 'illegal option' error after saving the form we have to load // all available menu parents. Otherwise, it is not possible to dynamically // add options to the list using ajax. - $options = $menu_parent_selector->getParentSelectOptions(''); + $options_cacheability = new CacheableMetadata(); + $options = $menu_parent_selector->getParentSelectOptions('', NULL, $options_cacheability); $form['menu']['menu_parent'] = array( '#type' => 'select', '#title' => t('Default parent item'), @@ -411,6 +413,7 @@ function menu_ui_form_node_type_form_alter(&$form, FormStateInterface $form_stat '#description' => t('Choose the menu item to be the default parent for a new link in the content authoring form.'), '#attributes' => array('class' => array('menu-title-select')), ); + $options_cacheability->applyTo($form['menu']['menu_parent']); $form['actions']['submit']['#validate'][] = 'menu_ui_form_node_type_form_validate'; $form['#entity_builders'][] = 'menu_ui_form_node_type_form_builder'; diff --git a/core/modules/menu_ui/src/Controller/MenuController.php b/core/modules/menu_ui/src/Controller/MenuController.php index 95788bdc4fe518bc080e73e818fde0f56b1ac5b6..72870547fd207aebdb9879a2db7a65ca9afb2f1f 100644 --- a/core/modules/menu_ui/src/Controller/MenuController.php +++ b/core/modules/menu_ui/src/Controller/MenuController.php @@ -60,6 +60,8 @@ public function getParentOptions(Request $request) { $available_menus[$menu] = $menu; } } + // @todo Update this to use the optional $cacheability parameter, so that + // a cacheable JSON response can be sent. $options = $this->menuParentSelector->getParentSelectOptions('', $available_menus); return new JsonResponse($options); diff --git a/core/modules/menu_ui/src/MenuForm.php b/core/modules/menu_ui/src/MenuForm.php index 589409b9610aa13df7ce494fa5fc5c7dcd3fb64b..e008b3b4a2bb7707fcf1c5a090af809299ba1376 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 45a9df7b0fa2524c354d45aff378f5feee811837..adff9a033b5b80ed04232ce1e95130c539f6c6a4 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/MenuNodeTest.php b/core/modules/menu_ui/src/Tests/MenuNodeTest.php index 6b1a9a4bf80bc607a4f08f05b08884f3faf59689..0f0d1689efeeaa876f1e2253ab7db906bba7eebd 100644 --- a/core/modules/menu_ui/src/Tests/MenuNodeTest.php +++ b/core/modules/menu_ui/src/Tests/MenuNodeTest.php @@ -53,6 +53,13 @@ protected function setUp() { * Test creating, editing, deleting menu links via node form widget. */ function testMenuNodeFormWidget() { + // Verify that cacheability metadata is bubbled from the menu link tree + // access checking that is performed when determining the "default parent + // item" options in menu_ui_form_node_type_form_alter(). The "log out" link + // adds the "user.roles:authenticated" cache context. + $this->drupalGet('admin/structure/types/manage/page'); + $this->assertCacheContext('user.roles:authenticated'); + // Disable the default main menu, so that no menus are enabled. $edit = array( 'menu_options[main]' => FALSE, diff --git a/core/modules/menu_ui/src/Tests/MenuTest.php b/core/modules/menu_ui/src/Tests/MenuTest.php index 948e84fcdd6a3cc3aa3bba336f4d8b82b0894d74..59616e75fb0a43cdb18d9462cfbb263688f8a5c0 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. See DefaultMenuLinkTreeManipulators::menuLinkCheckAccess(). + $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 c6af0ac19dc0643912c636f309fb39f098e51f69..95bd739b42f59a116386caaa23c85e172db495af 100644 --- a/core/modules/page_cache/src/Tests/PageCacheTagsIntegrationTest.php +++ b/core/modules/page_cache/src/Tests/PageCacheTagsIntegrationTest.php @@ -78,7 +78,9 @@ function testPageCacheTags() { 'theme', 'timezone', 'user.permissions', - 'user.roles', + // 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 13e2f73660ebbdac8ce6f4c87f362404af283b52..20ea39e409df470c8b5f4fc5830a8fbc580172d2 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/Plugin/Block/SystemBreadcrumbBlock.php b/core/modules/system/src/Plugin/Block/SystemBreadcrumbBlock.php index 1916d4079ec9ad8c6cbcd3d4a19ef9f38f042060..c7629f08ee089dcd0b17e35960976211f18a4cce 100644 --- a/core/modules/system/src/Plugin/Block/SystemBreadcrumbBlock.php +++ b/core/modules/system/src/Plugin/Block/SystemBreadcrumbBlock.php @@ -87,7 +87,7 @@ public function build() { /** * {@inheritdoc} * - * @todo Make cacheable as part of https://www.drupal.org/node/1805054. + * @todo Make cacheable in https://www.drupal.org/node/2483183 */ public function getCacheMaxAge() { return 0; diff --git a/core/modules/system/src/Plugin/Block/SystemMenuBlock.php b/core/modules/system/src/Plugin/Block/SystemMenuBlock.php index 4a6a2eacc71c368739e73efebbded0514b633442..aec97166cbf7beb4da606e12b520c35e53f81f3d 100644 --- a/core/modules/system/src/Plugin/Block/SystemMenuBlock.php +++ b/core/modules/system/src/Plugin/Block/SystemMenuBlock.php @@ -187,10 +187,14 @@ public function getCacheTags() { * {@inheritdoc} */ public function getCacheContexts() { - // Menu blocks must be cached per role and per active trail. + // ::build() uses MenuLinkTreeInterface::getCurrentRouteMenuTreeParameters() + // to generate menu tree parameters, and those take the active menu trail + // into account. Therefore, we must vary the rendered menu by the active + // trail of the rendered menu. + // Additional cache contexts, e.g. those that determine link text or + // accessibility of a menu, will be bubbled automatically. $menu_name = $this->getDerivativeId(); return [ - 'user.roles', 'route.menu_active_trails:' . $menu_name, ]; } diff --git a/core/modules/system/src/SystemManager.php b/core/modules/system/src/SystemManager.php index 11c3efcde831fa50bdadb2011541eb89083dce59..33d6de522134e64705ae275bc317dd455ccb1cc8 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 0dcd3ec676b52c760ea77c9b11eb4162e253dec1..8acaf3305cfeb48cfbae00f81313824f84f3c7c4 100644 --- a/core/modules/system/system.module +++ b/core/modules/system/system.module @@ -1168,6 +1168,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/system/tests/src/Unit/Menu/MenuLinkTreeTest.php b/core/modules/system/tests/src/Unit/Menu/MenuLinkTreeTest.php new file mode 100644 index 0000000000000000000000000000000000000000..d699a8e4e1205694a3d6b4a9be00906780005d76 --- /dev/null +++ b/core/modules/system/tests/src/Unit/Menu/MenuLinkTreeTest.php @@ -0,0 +1,261 @@ +<?php + +/** + * @file + * Contains \Drupal\Tests\system\Unit\Menu\MenuLinkTreeTest. + */ + +namespace Drupal\Tests\system\Unit\Menu; + +use Drupal\Core\Access\AccessResult; +use Drupal\Core\Cache\Cache; +use Drupal\Core\DependencyInjection\ContainerBuilder; +use Drupal\Core\Menu\MenuLinkTree; +use Drupal\Core\Menu\MenuLinkTreeElement; +use Drupal\Core\Template\Attribute; +use Drupal\Core\Url; +use Drupal\Tests\Core\Menu\MenuLinkMock; +use Drupal\Tests\UnitTestCase; + +/** + * @coversDefaultClass \Drupal\Core\Menu\MenuLinkTree + * @group Menu + */ +class MenuLinkTreeTest extends UnitTestCase { + + /** + * The tested menu link tree service. + * + * @var \Drupal\Core\Menu\MenuLinkTree + */ + protected $menuLinkTree; + + /** + * {@inheritdoc} + */ + protected function setUp() { + parent::setUp(); + + $this->menuLinkTree = new MenuLinkTree( + $this->getMock('\Drupal\Core\Menu\MenuTreeStorageInterface'), + $this->getMock('\Drupal\Core\Menu\MenuLinkManagerInterface'), + $this->getMock('\Drupal\Core\Routing\RouteProviderInterface'), + $this->getMock('\Drupal\Core\Menu\MenuActiveTrailInterface'), + $this->getMock('\Drupal\Core\Controller\ControllerResolverInterface') + ); + + $cache_contexts_manager = $this->getMockBuilder('Drupal\Core\Cache\CacheContextsManager') + ->disableOriginalConstructor() + ->getMock(); + $container = new ContainerBuilder(); + $container->set('cache_contexts_manager', $cache_contexts_manager); + \Drupal::setContainer($container); + } + + /** + * @covers ::build + * + * MenuLinkTree::build() gathers both: + * 1. the tree's access cacheability: the cacheability of the access result + * of checking a link in a menu tree's access. Callers can opt out of + * this by MenuLinkTreeElement::access to NULL (the default) value, in + * which case the menu link is always visible. Only when an + * AccessResultInterface object is specified, we gather this cacheability + * metadata. + * This means there are three cases: + * a. no access result (NULL): menu link is visible + * b. AccessResultInterface object that is allowed: menu link is visible + * c. AccessResultInterface object that is not allowed: menu link is + * invisible, but cacheability metadata is still applicable + * 2. the tree's menu links' cacheability: the cacheability of a menu link + * itself, because it may be dynamic. For this reason, MenuLinkInterface + * extends CacheableDependencyInterface. It allows any menu link plugin to + * mark itself as uncacheable (max-age=0) or dynamic (by specifying cache + * tags and/or contexts), to indicate the extent of dynamism. + * This means there are two cases: + * a. permanently cacheable, no cache tags, no cache contexts + * b. anything else: non-permanently cacheable, and/or cache tags, and/or + * cache contexts. + * + * Finally, there are four important shapes of trees, all of which we want to + * test: + * 1. the empty tree + * 2. a single-element tree + * 3. a single-level tree (>1 element; just 1 element is case 2) + * 4. a multi-level tree + * + * The associated data provider aims to test the handling of both of these + * types of cacheability, and for all four tree shapes, for each of the types + * of values for the two types of cacheability. + * + * There is another level of cacheability involved when actually rendering + * built menu trees (i.e. when invoking RendererInterface::render() on the + * return value of MenuLinkTreeInterface::build()): the cacheability of the + * generated URLs. + * Fortunately, that doesn't need additional test coverage here because that + * cacheability is handled at the level of the Renderer (i.e. menu.html.twig + * template's link() function invocation). It also has its own test coverage. + * + * @see \Drupal\menu_link_content\Tests\MenuLinkContentCacheabilityBubblingTest + * + * @dataProvider providerTestBuildCacheability + */ + public function testBuildCacheability($description, $tree, $expected_build) { + $build = $this->menuLinkTree->build($tree); + sort($expected_build['#cache']['contexts']); + $this->assertEquals($expected_build, $build, $description); + } + + /** + * Provides the test cases to test for ::testBuildCacheability(). + * + * As explained in the documentation for ::testBuildCacheability(), this + * generates 1 + (3 * 2 * 3) = 19 test cases. + * + * @see testBuildCacheability + */ + public function providerTestBuildCacheability() { + $base_expected_build_empty = [ + '#cache' => [ + 'contexts' => [], + 'tags' => [], + 'max-age' => Cache::PERMANENT, + ], + ]; + $base_expected_build = [ + '#cache' => [ + 'contexts' => [], + 'tags' => [ + 'config:system.menu.mock', + ], + 'max-age' => Cache::PERMANENT, + ], + '#sorted' => TRUE, + '#theme' => 'menu__mock', + '#items' => [ + // To be filled when generating test cases, using $get_built_element(). + ] + ]; + + $get_built_element = function(MenuLinkTreeElement $element, array $classes) { + return [ + 'attributes' => new Attribute(['class' => array_merge(['menu-item'], $classes)]), + 'title' => $element->link->getTitle(), + 'url' => new Url($element->link->getRouteName(), $element->link->getRouteParameters(), ['set_active_class' => TRUE]), + 'below' => [], + 'original_link' => $element->link, + ]; + }; + + // The three access scenarios described in this method's documentation. + $access_scenarios = [ + [NULL, []], + [AccessResult::allowed(), ['access:allowed']], + [AccessResult::neutral(), ['access:neutral']], + ]; + + // The two links scenarios described in this method's documentation. + $cache_defaults = ['cache_max_age' => Cache::PERMANENT, 'cache_tags' => []]; + $links_scenarios = [ + [ + MenuLinkMock::create(['id' => 'test.example1', 'route_name' => 'example1', 'title' => 'Example 1']), + MenuLinkMock::create(['id' => 'test.example2', 'route_name' => 'example1', 'title' => 'Example 2', 'metadata' => ['cache_contexts' => ['llama']] + $cache_defaults]), + ], + [ + MenuLinkMock::create(['id' => 'test.example1', 'route_name' => 'example1', 'title' => 'Example 1', 'metadata' => ['cache_contexts' => ['foo']] + $cache_defaults]), + MenuLinkMock::create(['id' => 'test.example2', 'route_name' => 'example1', 'title' => 'Example 2', 'metadata' => ['cache_contexts' => ['bar']] + $cache_defaults]), + ], + ]; + + + $data = []; + + // Empty tree. + $data[] = [ + 'description' => 'Empty tree.', + 'tree' => [], + 'expected_build' => $base_expected_build_empty, + ]; + + for ($i = 0; $i < count($access_scenarios); $i++) { + list($access, $access_cache_contexts) = $access_scenarios[$i]; + if ($access !== NULL) { + $access->addCacheContexts($access_cache_contexts); + } + + for ($j = 0; $j < count($links_scenarios); $j++) { + $links = $links_scenarios[$j]; + + // Single-element tree. + $tree = [ + new MenuLinkTreeElement($links[0], FALSE, 0, FALSE, []), + ]; + $tree[0]->access = $access; + if ($access === NULL || $access->isAllowed()) { + $expected_build = $base_expected_build; + $expected_build['#items']['test.example1'] = $get_built_element($tree[0], []); + } + else { + $expected_build = $base_expected_build_empty; + } + $expected_build['#cache']['contexts'] = array_merge($expected_build['#cache']['contexts'], $access_cache_contexts, $links[0]->getCacheContexts()); + $data[] = [ + 'description' => "Single-item tree; access=$i; link=$j.", + 'tree' => $tree, + 'expected_build' => $expected_build, + ]; + + // Single-level tree. + $tree = [ + new MenuLinkTreeElement($links[0], FALSE, 0, FALSE, []), + new MenuLinkTreeElement($links[1], FALSE, 0, FALSE, []), + ]; + $tree[0]->access = $access; + $expected_build = $base_expected_build; + if ($access === NULL || $access->isAllowed()) { + $expected_build['#items']['test.example1'] = $get_built_element($tree[0], []); + } + $expected_build['#items']['test.example2'] = $get_built_element($tree[1], []); + $expected_build['#cache']['contexts'] = array_merge($expected_build['#cache']['contexts'], $access_cache_contexts, $links[0]->getCacheContexts(), $links[1]->getCacheContexts()); + $data[] = [ + 'description' => "Single-level tree; access=$i; link=$j.", + 'tree' => $tree, + 'expected_build' => $expected_build, + ]; + + // Multi-level tree. + $multi_level_root_a = MenuLinkMock::create(['id' => 'test.roota', 'route_name' => 'roota', 'title' => 'Root A']); + $multi_level_root_b = MenuLinkMock::create(['id' => 'test.rootb', 'route_name' => 'rootb', 'title' => 'Root B']); + $multi_level_parent_c = MenuLinkMock::create(['id' => 'test.parentc', 'route_name' => 'parentc', 'title' => 'Parent C']); + $tree = [ + new MenuLinkTreeElement($multi_level_root_a, TRUE, 0, FALSE, [ + new MenuLinkTreeElement($multi_level_parent_c, TRUE, 0, FALSE, [ + new MenuLinkTreeElement($links[0], FALSE, 0, FALSE, []), + ]) + ]), + new MenuLinkTreeElement($multi_level_root_b, TRUE, 0, FALSE, [ + new MenuLinkTreeElement($links[1], FALSE, 1, FALSE, []) + ]), + ]; + $tree[0]->subtree[0]->subtree[0]->access = $access; + $expected_build = $base_expected_build; + $expected_build['#items']['test.roota'] = $get_built_element($tree[0], ['menu-item--expanded']); + $expected_build['#items']['test.roota']['below']['test.parentc'] = $get_built_element($tree[0]->subtree[0], ['menu-item--expanded']); + if ($access === NULL || $access->isAllowed()) { + $expected_build['#items']['test.roota']['below']['test.parentc']['below']['test.example1'] = $get_built_element($tree[0]->subtree[0]->subtree[0], []); + } + $expected_build['#items']['test.rootb'] = $get_built_element($tree[1], ['menu-item--expanded']); + $expected_build['#items']['test.rootb']['below']['test.example2'] = $get_built_element($tree[1]->subtree[0], []); + $expected_build['#cache']['contexts'] = array_merge($expected_build['#cache']['contexts'], $access_cache_contexts, $links[0]->getCacheContexts(), $links[1]->getCacheContexts()); + $data[] = [ + 'description' => "Multi-level tree; access=$i; link=$j.", + 'tree' => $tree, + 'expected_build' => $expected_build, + ]; + } + } + + return $data; + } + +} diff --git a/core/modules/user/src/Tests/UserAccountLinksTest.php b/core/modules/user/src/Tests/UserAccountLinksTest.php index 0e36e3e4f1739be84d1fc35383aff5a3db4bec6d..3d509986b2c95aeb47b8423a81f67d3953556f2a 100644 --- a/core/modules/user/src/Tests/UserAccountLinksTest.php +++ b/core/modules/user/src/Tests/UserAccountLinksTest.php @@ -66,13 +66,10 @@ function testSecondaryMenu() { $this->drupalGet('<front>'); // For a logged-out user, expect no secondary links. - $menu_tree = \Drupal::menuTree(); - $tree = $menu_tree->load('account', new MenuTreeParameters()); - $manipulators = array( - array('callable' => 'menu.default_tree_manipulators:checkAccess'), - ); - $tree = $menu_tree->transform($tree, $manipulators); - $this->assertEqual(count($tree), 0, 'The secondary links menu contains no menu link.'); + $menu = $this->xpath('//ul[@class=:menu_class]', array( + ':menu_class' => 'menu', + )); + $this->assertEqual(count($menu), 0, 'The secondary links menu is not rendered, because none of its menu links are accessible for the anonymous user.'); } /** diff --git a/core/modules/views_ui/src/Tests/DisplayPathTest.php b/core/modules/views_ui/src/Tests/DisplayPathTest.php index 5b2a02284e1c7ef893cf616d181e9e216306afba..cc6826fb2d1332e1a252386cdee13f3bfb316812 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'); } } diff --git a/core/tests/Drupal/Tests/Core/Menu/DefaultMenuLinkTreeManipulatorsTest.php b/core/tests/Drupal/Tests/Core/Menu/DefaultMenuLinkTreeManipulatorsTest.php index bef3b11ab859f104bb0c70a63c932b8c9b89de84..c20a99b74f69ca4889b9b98578f662f8d240c42e 100644 --- a/core/tests/Drupal/Tests/Core/Menu/DefaultMenuLinkTreeManipulatorsTest.php +++ b/core/tests/Drupal/Tests/Core/Menu/DefaultMenuLinkTreeManipulatorsTest.php @@ -7,6 +7,7 @@ namespace Drupal\Tests\Core\Menu; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Menu\DefaultMenuLinkTreeManipulators; use Drupal\Core\Menu\MenuLinkTreeElement; use Drupal\Tests\UnitTestCase; @@ -154,39 +155,61 @@ public function testCheckAccess() { $this->accessManager->expects($this->exactly(4)) ->method('checkNamedRoute') ->will($this->returnValueMap(array( - array('example1', array(), $this->currentUser, FALSE, FALSE), - array('example2', array('foo' => 'bar'), $this->currentUser, FALSE, TRUE), - array('example3', array('baz' => 'qux'), $this->currentUser, FALSE, FALSE), - array('example5', array(), $this->currentUser, FALSE, TRUE), + array('example1', array(), $this->currentUser, TRUE, AccessResult::forbidden()), + array('example2', array('foo' => 'bar'), $this->currentUser, TRUE, AccessResult::allowed()->cachePerPermissions()), + array('example3', array('baz' => 'qux'), $this->currentUser, TRUE, AccessResult::neutral()), + array('example5', array(), $this->currentUser, TRUE, AccessResult::allowed()), ))); $this->mockTree(); - $this->originalTree[5]->subtree[7]->access = TRUE; - $this->originalTree[8]->access = FALSE; + $this->originalTree[5]->subtree[7]->access = AccessResult::neutral(); + $this->originalTree[8]->access = AccessResult::allowed()->cachePerUser(); + // Since \Drupal\Core\Menu\DefaultMenuLinkTreeManipulators::checkAccess() + // allows access to any link if the user has the 'link to any page' + // permission, *every* single access result is varied by permissions. $tree = $this->defaultMenuTreeManipulators->checkAccess($this->originalTree); - // Menu link 1: route without parameters, access forbidden, hence removed. - $this->assertFalse(array_key_exists(1, $tree)); + // Menu link 1: route without parameters, access forbidden, but at level 0, + // hence kept. + $element = $tree[1]; + $this->assertEquals(AccessResult::forbidden()->cachePerPermissions(), $element->access); + $this->assertInstanceOf('\Drupal\Core\Menu\InaccessibleMenuLink', $element->link); // Menu link 2: route with parameters, access granted. $element = $tree[2]; - $this->assertTrue($element->access); - // Menu link 3: route with parameters, access forbidden, hence removed, - // including its children. - $this->assertFalse(array_key_exists(3, $tree[2]->subtree)); - // Menu link 4: child of menu link 3, which already is removed. - $this->assertSame(array(), $tree[2]->subtree); + $this->assertEquals(AccessResult::allowed()->cachePerPermissions(), $element->access); + $this->assertNotInstanceOf('\Drupal\Core\Menu\InaccessibleMenuLink', $element->link); + // Menu link 3: route with parameters, AccessResult::neutral(), top-level + // inaccessible link, hence kept for its cacheability metadata. + // Note that the permissions cache context is added automatically, because + // we always check the "link to any page" permission. + $element = $tree[2]->subtree[3]; + $this->assertEquals(AccessResult::neutral()->cachePerPermissions(), $element->access); + $this->assertInstanceOf('\Drupal\Core\Menu\InaccessibleMenuLink', $element->link); + // Menu link 4: child of menu link 3, which was AccessResult::neutral(), + // hence menu link 3's subtree is removed, of which this menu link is one. + $this->assertFalse(array_key_exists(4, $tree[2]->subtree[3]->subtree)); // Menu link 5: no route name, treated as external, hence access granted. $element = $tree[5]; - $this->assertTrue($element->access); + $this->assertEquals(AccessResult::allowed()->cachePerPermissions(), $element->access); + $this->assertNotInstanceOf('\Drupal\Core\Menu\InaccessibleMenuLink', $element->link); // Menu link 6: external URL, hence access granted. $element = $tree[6]; - $this->assertTrue($element->access); - // Menu link 7: 'access' already set. + $this->assertEquals(AccessResult::allowed()->cachePerPermissions(), $element->access); + $this->assertNotInstanceOf('\Drupal\Core\Menu\InaccessibleMenuLink', $element->link); + // Menu link 7: 'access' already set: AccessResult::neutral(), top-level + // inaccessible link, hence kept for its cacheability metadata. + // Note that unlike for menu link 3, the permission cache context is absent, + // because ::checkAccess() doesn't perform access checking when 'access' is + // already set. $element = $tree[5]->subtree[7]; - $this->assertTrue($element->access); - // Menu link 8: 'access' already set, to FALSE, hence removed. - $this->assertFalse(array_key_exists(8, $tree)); + $this->assertEquals(AccessResult::neutral(), $element->access); + $this->assertInstanceOf('\Drupal\Core\Menu\InaccessibleMenuLink', $element->link); + // Menu link 8: 'access' already set, note that 'per permissions' caching + // is not added. + $element = $tree[8]; + $this->assertEquals(AccessResult::allowed()->cachePerUser(), $element->access); + $this->assertNotInstanceOf('\Drupal\Core\Menu\InaccessibleMenuLink', $element->link); } /** @@ -205,13 +228,14 @@ public function testCheckAccessWithLinkToAnyPagePermission() { $this->mockTree(); $this->defaultMenuTreeManipulators->checkAccess($this->originalTree); - $this->assertTrue($this->originalTree[1]->access); - $this->assertTrue($this->originalTree[2]->access); - $this->assertTrue($this->originalTree[2]->subtree[3]->access); - $this->assertTrue($this->originalTree[2]->subtree[3]->subtree[4]->access); - $this->assertTrue($this->originalTree[5]->subtree[7]->access); - $this->assertTrue($this->originalTree[6]->access); - $this->assertTrue($this->originalTree[8]->access); + $expected_access_result = AccessResult::allowed()->cachePerPermissions(); + $this->assertEquals($expected_access_result, $this->originalTree[1]->access); + $this->assertEquals($expected_access_result, $this->originalTree[2]->access); + $this->assertEquals($expected_access_result, $this->originalTree[2]->subtree[3]->access); + $this->assertEquals($expected_access_result, $this->originalTree[2]->subtree[3]->subtree[4]->access); + $this->assertEquals($expected_access_result, $this->originalTree[5]->subtree[7]->access); + $this->assertEquals($expected_access_result, $this->originalTree[6]->access); + $this->assertEquals($expected_access_result, $this->originalTree[8]->access); } /** @@ -233,7 +257,7 @@ public function testFlatten() { * @covers ::collectNodeLinks * @covers ::checkAccess */ - public function testCheckNodeAccess() { + public function testCheckNodeAccess() { $links = array( 1 => MenuLinkMock::create(array('id' => 'node.1', 'route_name' => 'entity.node.canonical', 'title' => 'foo', 'parent' => '', 'route_parameters' => array('node' => 1))), 2 => MenuLinkMock::create(array('id' => 'node.2', 'route_name' => 'entity.node.canonical', 'title' => 'bar', 'parent' => '', 'route_parameters' => array('node' => 2))), @@ -268,12 +292,14 @@ public function testCheckNodeAccess() { ->with('node') ->willReturn($query); + $node_access_result = AccessResult::allowed()->cachePerPermissions()->addCacheContexts(['user.node_grants:view']); + $tree = $this->defaultMenuTreeManipulators->checkNodeAccess($tree); - $this->assertTrue($tree[1]->access); - $this->assertTrue($tree[2]->access); + $this->assertEquals($node_access_result, $tree[1]->access); + $this->assertEquals($node_access_result, $tree[2]->access); // Ensure that access denied is set. - $this->assertFalse($tree[2]->subtree[3]->access); - $this->assertTrue($tree[2]->subtree[3]->subtree[4]->access); + $this->assertEquals(AccessResult::neutral(), $tree[2]->subtree[3]->access); + $this->assertEquals($node_access_result, $tree[2]->subtree[3]->subtree[4]->access); // Ensure that other routes than entity.node.canonical are set as well. $this->assertNull($tree[5]->access); $this->assertNull($tree[5]->subtree[6]->access); @@ -284,19 +310,19 @@ public function testCheckNodeAccess() { // Ensure that the access manager is just called for the non-node routes. $this->accessManager->expects($this->at(0)) ->method('checkNamedRoute') - ->with('test_route', [], $this->currentUser) - ->willReturn(TRUE); + ->with('test_route', [], $this->currentUser, TRUE) + ->willReturn(AccessResult::allowed()); $this->accessManager->expects($this->at(1)) ->method('checkNamedRoute') - ->with('test_route', [], $this->currentUser) - ->willReturn(FALSE); + ->with('test_route', [], $this->currentUser, TRUE) + ->willReturn(AccessResult::neutral()); $tree = $this->defaultMenuTreeManipulators->checkAccess($tree); - $this->assertTrue($tree[1]->access); - $this->assertTrue($tree[2]->access); - $this->assertFalse(isset($tree[2]->subtree[3])); - $this->assertTrue($tree[5]->access); - $this->assertFalse(isset($tree[5]->subtree[6])); + $this->assertEquals($node_access_result, $tree[1]->access); + $this->assertEquals($node_access_result, $tree[2]->access); + $this->assertEquals(AccessResult::neutral(), $tree[2]->subtree[3]->access); + $this->assertEquals(AccessResult::allowed()->cachePerPermissions(), $tree[5]->access); + $this->assertEquals(AccessResult::neutral()->cachePerPermissions(), $tree[5]->subtree[6]->access); } } diff --git a/core/tests/Drupal/Tests/Core/Menu/MenuLinkMock.php b/core/tests/Drupal/Tests/Core/Menu/MenuLinkMock.php index 9a9e98e4bc2d324f984e946d80650b3840f45cd7..b0b8354cbdcdeda312720aa66e969900424f36fc 100644 --- a/core/tests/Drupal/Tests/Core/Menu/MenuLinkMock.php +++ b/core/tests/Drupal/Tests/Core/Menu/MenuLinkMock.php @@ -7,6 +7,7 @@ namespace Drupal\Tests\Core\Menu; +use Drupal\Core\Cache\Cache; use Drupal\Core\Menu\MenuLinkBase; /** @@ -27,9 +28,13 @@ class MenuLinkMock extends MenuLinkBase { 'weight' => '0', 'options' => array(), 'expanded' => '0', - 'hidden' => '0', + 'enabled' => '1', 'provider' => 'simpletest', - 'metadata' => array(), + 'metadata' => [ + 'cache_contexts' => [], + 'cache_tags' => [], + 'cache_max_age' => Cache::PERMANENT, + ], 'class' => 'Drupal\\Tests\\Core\Menu\\MenuLinkMock', 'form_class' => 'Drupal\\Core\\Menu\\Form\\MenuLinkDefaultForm', 'id' => 'MUST BE PROVIDED', @@ -67,4 +72,25 @@ public function updateLink(array $new_definition_values, $persist) { return $this->pluginDefinition; } + /** + * {@inheritdoc} + */ + public function getCacheContexts() { + return $this->pluginDefinition['metadata']['cache_contexts']; + } + + /** + * {@inheritdoc} + */ + public function getCacheTags() { + return $this->pluginDefinition['metadata']['cache_tags']; + } + + /** + * {@inheritdoc} + */ + public function getCacheMaxAge() { + return $this->pluginDefinition['metadata']['cache_max_age']; + } + } diff --git a/core/tests/Drupal/Tests/Core/Render/ElementTest.php b/core/tests/Drupal/Tests/Core/Render/ElementTest.php index 0d764cd3f72fee2e992a54d411e77515757f340b..75c0645e5c1d5322a18b95b1c7a19c5cda6ec746 100644 --- a/core/tests/Drupal/Tests/Core/Render/ElementTest.php +++ b/core/tests/Drupal/Tests/Core/Render/ElementTest.php @@ -179,4 +179,28 @@ public function providerTestSetAttributes() { ); } + /** + * @covers ::isEmpty + * + * @dataProvider providerTestIsEmpty + */ + public function testIsEmpty(array $element, $expected) { + $this->assertSame(Element::isEmpty($element), $expected); + } + + public function providerTestIsEmpty() { + return [ + [[], TRUE], + [['#cache' => []], TRUE], + [['#cache' => ['tags' => ['foo']]], TRUE], + [['#cache' => ['contexts' => ['bar']]], TRUE], + + [['#cache' => [], '#markup' => 'llamas are awesome'], FALSE], + [['#markup' => 'llamas are the most awesome ever'], FALSE], + + [['#cache' => [], '#any_other_property' => TRUE], FALSE], + [['#any_other_property' => TRUE], FALSE], + ]; + } + }