core/core.services.yml | 2 +- core/includes/menu.inc | 3 +- core/lib/Drupal/Core/Menu/MenuActiveTrail.php | 3 + .../Drupal/Core/Menu/MenuActiveTrailInterface.php | 7 +- core/lib/Drupal/Core/Menu/MenuLinkTree.php | 185 ++++----------------- .../lib/Drupal/Core/Menu/MenuLinkTreeInterface.php | 57 ++++--- .../Drupal/Core/Menu/MenuParentFormSelector.php | 5 +- core/modules/menu_ui/src/MenuForm.php | 3 +- .../system/src/Plugin/Block/SystemMenuBlock.php | 7 +- core/modules/system/src/SystemManager.php | 4 +- .../system/src/Tests/Menu/MenuLinkTreeTest.php | 1 - 11 files changed, 87 insertions(+), 190 deletions(-) diff --git a/core/core.services.yml b/core/core.services.yml index 5c00693..99fef7d 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -266,7 +266,7 @@ services: arguments: ['@menu.tree_storage', '@menu_link.static.overrides', '@module_handler'] menu.link_tree: class: Drupal\Core\Menu\MenuLinkTree - arguments: ['@menu.tree_storage', '@plugin.manager.menu.link', '@request_stack', '@router.route_provider', '@cache.menu', '@access_manager', '@current_user', '@menu.active_trail'] + arguments: ['@menu.tree_storage', '@plugin.manager.menu.link', '@router.route_provider', '@access_manager', '@current_user', '@menu.active_trail'] menu.active_trail: class: Drupal\Core\Menu\MenuActiveTrail arguments: ['@plugin.manager.menu.link', '@request_stack', '@access_manager', '@current_user', '@config.factory'] diff --git a/core/includes/menu.inc b/core/includes/menu.inc index 2f78c37..9d8ee25 100644 --- a/core/includes/menu.inc +++ b/core/includes/menu.inc @@ -301,7 +301,8 @@ function menu_navigation_links($menu_name, $level = 0) { // Get the menu hierarchy for the current page. /** @var \Drupal\Core\Menu\MenuLinkTreeInterface $menu_tree */ $menu_tree = \Drupal::service('menu.link_tree'); - $tree = $menu_tree->buildPageData($menu_name, $level + 1); + $parameters = $menu_tree->buildPageDataTreeParameters($menu_name, $level + 1); + $tree = $menu_tree->buildTree($menu_name, $parameters); // Go down the active trail until the right level is reached. while ($level-- > 0 && $tree) { diff --git a/core/lib/Drupal/Core/Menu/MenuActiveTrail.php b/core/lib/Drupal/Core/Menu/MenuActiveTrail.php index 97c4f6f..25a7284 100644 --- a/core/lib/Drupal/Core/Menu/MenuActiveTrail.php +++ b/core/lib/Drupal/Core/Menu/MenuActiveTrail.php @@ -13,6 +13,9 @@ use Symfony\Cmf\Component\Routing\RouteObjectInterface; use Symfony\Component\HttpFoundation\RequestStack; +/** + * Provides the default implementation of the active menu trail service. + */ class MenuActiveTrail implements MenuActiveTrailInterface { /** diff --git a/core/lib/Drupal/Core/Menu/MenuActiveTrailInterface.php b/core/lib/Drupal/Core/Menu/MenuActiveTrailInterface.php index 3222378..2f09fac 100644 --- a/core/lib/Drupal/Core/Menu/MenuActiveTrailInterface.php +++ b/core/lib/Drupal/Core/Menu/MenuActiveTrailInterface.php @@ -7,7 +7,12 @@ namespace Drupal\Core\Menu; - +/** + * Defines an interface for the active menu trail service. + * + * The active trail of a given menu is the trail from the current page to the + * root of that menu's tree. + */ interface MenuActiveTrailInterface { /** diff --git a/core/lib/Drupal/Core/Menu/MenuLinkTree.php b/core/lib/Drupal/Core/Menu/MenuLinkTree.php index c4e6d57..63e272f 100644 --- a/core/lib/Drupal/Core/Menu/MenuLinkTree.php +++ b/core/lib/Drupal/Core/Menu/MenuLinkTree.php @@ -8,12 +8,8 @@ namespace Drupal\Core\Menu; use Drupal\Core\Access\AccessManager; -use Drupal\Core\Cache\Cache; -use Drupal\Core\Cache\CacheBackendInterface; use Drupal\Core\Routing\RouteProviderInterface; use Drupal\Core\Session\AccountInterface; -use Symfony\Cmf\Component\Routing\RouteObjectInterface; -use Symfony\Component\HttpFoundation\RequestStack; /** * Manages discovery, instantiation, and tree building of menu link plugins. @@ -60,14 +56,6 @@ class MenuLinkTree implements MenuLinkTreeInterface { 'id' => '', ); - - /** - * Cache backend instance for the extracted tree data. - * - * @var \Drupal\Core\Cache\CacheBackendInterface - */ - protected $treeCacheBackend; - /** * The menu link tree storage. * @@ -83,13 +71,6 @@ class MenuLinkTree implements MenuLinkTreeInterface { protected $overrides; /** - * The request stack. - * - * @var \Symfony\Component\HttpFoundation\RequestStack - */ - protected $requestStack; - - /** * The plugin instances. * * @var array @@ -125,79 +106,34 @@ class MenuLinkTree implements MenuLinkTreeInterface { protected $account; /** - * The menu active trail service. + * The active menu trail service. * * @var \Drupal\Core\Menu\MenuActiveTrailInterface */ protected $menuActiveTrail; /** - * Stores the menu tree used by the doBuildTree method, keyed by a cache ID. - * - * This cache ID is built using the $menu_name, the current language and - * some parameters passed into an entity query. - */ - protected $menuTree; - - /** - * Stores the menu tree data on the current page keyed by a cache ID. - * - * This contains less information than a tree built with buildAllData. - * - * @var array - */ - protected $menuPageTrees; - - /** - * Stores the preferred menu link keyed by route_name + parameters. - * - * @var array - */ - protected $preferredLinks = array(); - - /** - * Stores the active menu names. - * - * @var array - */ - protected $activeMenus = array(); - - /** - * Stores the parameters for buildAllData keyed by cached ID. - * - * @var array - */ - protected $buildAllDataParameters = array(); - - /** * Constructs a \Drupal\Core\Menu\MenuLinkTree object. * * @param \Drupal\Core\Menu\MenuTreeStorageInterface $tree_storage * The menu link tree storage. * @param \Drupal\Core\Menu\MenuLinkManagerInterface $menu_link_manager * The menu link plugin manager. - * @param \Symfony\Component\HttpFoundation\RequestStack $request_stack - * A request object for the controller resolver and finding the preferred - * menu and link for the current page. * @param \Drupal\Core\Routing\RouteProviderInterface $route_provider * The route provider to load routes by name. - * @param \Drupal\Core\Cache\CacheBackendInterface $tree_cache_backend - * Cache backend instance for the extracted tree data. * @param \Drupal\Core\Access\AccessManager $access_manager * The access manager. * @param \Drupal\Core\Session\AccountInterface $account * The current user. * @param \Drupal\Core\Menu\MenuActiveTrailInterface $menu_active_trail - * Menu active trail service. + * The active menu trail service. */ - public function __construct(MenuTreeStorageInterface $tree_storage, MenuLinkManagerInterface $menu_link_manager, RequestStack $request_stack, RouteProviderInterface $route_provider, CacheBackendInterface $tree_cache_backend, AccessManager $access_manager, AccountInterface $account, MenuActiveTrailInterface $menu_active_trail) { + public function __construct(MenuTreeStorageInterface $tree_storage, MenuLinkManagerInterface $menu_link_manager, RouteProviderInterface $route_provider, AccessManager $access_manager, AccountInterface $account, MenuActiveTrailInterface $menu_active_trail) { $this->treeStorage = $tree_storage; $this->menuLinkManager = $menu_link_manager; - $this->requestStack = $request_stack; $this->routeProvider = $route_provider; $this->accessManager = $access_manager; $this->account = $account; - $this->treeCacheBackend = $tree_cache_backend; $this->menuActiveTrail = $menu_active_trail; } @@ -274,80 +210,40 @@ public function buildRenderTree($tree) { * {@inheritdoc} */ public function buildPageData($menu_name, $max_depth = NULL) { + $tree_parameters = $this->buildPageDataTreeParameters($menu_name, $max_depth); - // Load the request corresponding to the current page. - $request = $this->requestStack->getCurrentRequest(); - $page_is_403 = FALSE; - $system_path = NULL; - if ($route_name = $request->attributes->get(RouteObjectInterface::ROUTE_NAME)) { - $system_path = $request->attributes->get('_system_path'); - $page_is_403 = $request->attributes->get('_exception_statuscode') == 403; - } - - if (isset($max_depth)) { - $max_depth = min($max_depth, $this->treeStorage->maxDepth()); - } - // Generate a cache ID (cid) specific for this page. - $cid = 'links:' . $menu_name . ':page:' . $system_path . ':' . $page_is_403 . ':' . (int) $max_depth; - - // @todo Decide whether it makes sense to static cache page menu trees. - if (!isset($this->menuPageTrees[$cid])) { - // If the static variable doesn't have the data, check {cache_menu}. - $cache = $this->treeCacheBackend->get($cid); - if ($cache && isset($cache->data)) { - // If the cache entry exists, it contains the parameters for - // menu_build_tree(). - $tree_parameters = $cache->data; - } - else { - $tree_parameters = $this->doBuildPageDataTreeParameters($menu_name, $max_depth, $page_is_403); - - // Cache the tree building parameters using the page-specific cid. - $this->treeCacheBackend->set($cid, $tree_parameters, Cache::PERMANENT, array('menu' => $menu_name)); - } - - // Build the tree using the parameters; the resulting tree will be cached - // by $this->buildTree()). - $this->menuPageTrees[$cid] = $this->buildTree($menu_name, $tree_parameters); - } - return $this->menuPageTrees[$cid]; + // Build the tree using the parameters. + return $this->buildTree($menu_name, $tree_parameters); } /** - * Determines the required tree parameters used for the page menu tree. + * Builds the required tree parameters used for the page menu tree. * * This method takes into account the active trail of the current page. * * @param string $menu_name * The menu name. * @param int $max_depth - * The maximum allowed depth of menus. - * menu tree. - * @param bool $page_is_403 - * Is the current request happening on a 403 subrequest. + * (optional) The maximum depth of links to retrieve. * * @return array * An array of tree parameters. */ - protected function doBuildPageDataTreeParameters($menu_name, $max_depth, $page_is_403) { + public function buildPageDataTreeParameters($menu_name, $max_depth = NULL) { + if (isset($max_depth)) { + $max_depth = min($max_depth, $this->treeStorage->maxDepth()); + } + $tree_parameters = array( 'min_depth' => 1, 'max_depth' => $max_depth, ); - // If this page is accessible to the current user, build the tree - // parameters accordingly. - if (!$page_is_403) { - $active_trail = $this->menuActiveTrail->getActiveTrailIds($menu_name); - // Collect all the links set to be expanded, and then add all of - // their children to the list as well. - $parents = $this->treeStorage->getExpanded($menu_name, $active_trail); - } - else { - // If access is denied, we only show top-level links in menus. - $active_trail = array('' => ''); - $parents = $active_trail; - } + $active_trail = $this->menuActiveTrail->getActiveTrailIds($menu_name); + // Collect all the links set to be expanded, and then add all of + // their children to the list as well. + $parents = $this->treeStorage->getExpanded($menu_name, $active_trail); + $tree_parameters['expanded'] = $parents; $tree_parameters['active_trail'] = $active_trail; return $tree_parameters; @@ -358,32 +254,24 @@ protected function doBuildPageDataTreeParameters($menu_name, $max_depth, $page_i * * @todo should this accept a menu link instance or just the ID? */ - public function buildAllData($menu_name, $id = NULL, $max_depth = NULL) { - + public function buildAllDataTreeParameters($id = NULL, $max_depth = NULL) { // Use ID as a flag for whether the data being loaded is for the whole // tree. $id = isset($id) ? $id : '%'; - // Generate a cache ID (cid) specific for this $menu_name, $link, $language, - // and depth. - $cid = 'links:' . $menu_name . ':all:' . $id . ':' . (int) $max_depth; - if (!isset($this->buildAllDataParameters[$cid])) { - $tree_parameters = array( - 'min_depth' => 1, - 'max_depth' => $max_depth, - ); - if ($id != '%') { - // The tree is for a single item, so we need to match the values in - // of all the IDs on the path to root. - $tree_parameters['active_trail'] = $this->menuLinkManager->getParentIds($id); - $tree_parameters['expanded'] = $tree_parameters['active_trail']; - // Include top-level links. - $tree_parameters['expanded'][''] = ''; - } - $this->buildAllDataParameters[$cid] = $tree_parameters; + + $tree_parameters = array( + 'min_depth' => 1, + 'max_depth' => $max_depth, + ); + if ($id != '%') { + // The tree is for a single item, so we need to match the values in + // of all the IDs on the path to root. + $tree_parameters['active_trail'] = $this->menuLinkManager->getParentIds($id); + $tree_parameters['expanded'] = $tree_parameters['active_trail']; + // Include top-level links. + $tree_parameters['expanded'][''] = ''; } - // Build the tree using the parameters; the resulting tree will be cached - // by buildTree(). - return $this->buildTree($menu_name, $this->buildAllDataParameters[$cid]); + return $tree_parameters; } /** @@ -519,13 +407,4 @@ protected function menuLinkCheckAccess(array $definition) { return NULL; } - /** - * {@inheritdoc} - */ - public function resetStaticCache() { - $this->menuTree = array(); - $this->buildAllDataParameters = array(); - $this->menuPageTrees = array(); - } - } diff --git a/core/lib/Drupal/Core/Menu/MenuLinkTreeInterface.php b/core/lib/Drupal/Core/Menu/MenuLinkTreeInterface.php index 0d7c435..65002ed 100644 --- a/core/lib/Drupal/Core/Menu/MenuLinkTreeInterface.php +++ b/core/lib/Drupal/Core/Menu/MenuLinkTreeInterface.php @@ -49,9 +49,7 @@ public function getSubtreeHeight($id); public function buildRenderTree($tree); /** - * Gets the data structure for a named menu tree, based on the current page. - * - * Only visible links (hidden == 0) are returned in the data. + * Builds the tree parameters to build a menu tree based on the current page. * * @param string $menu_name * The named menu links to return. @@ -59,18 +57,12 @@ public function buildRenderTree($tree); * (optional) The maximum depth of links to retrieve. * * @return array - * An array of menu links, in the order they should be rendered. The array - * is a list of associative arrays -- these have several keys: - * - link: the menu link plugin instance - * - below: the subtree below the link, or empty array. It has the same - * structure as the top level array. - * - depth: - * - has_children: boolean. even if the below value may be empty the link - * may have children in the tree that are not shown. This - * is a hint for adding appropriate classes for theming. - * - in_active_trail: boolean + * An array of tree parameters. + * + * @todo This is now a misnomer; it's no longer based on the current page, but + * on the active trail. */ - public function buildPageData($menu_name, $max_depth = NULL); + public function buildPageDataTreeParameters($menu_name, $max_depth = NULL); /** * Gets the data structure representing a named menu tree. @@ -78,8 +70,6 @@ public function buildPageData($menu_name, $max_depth = NULL); * Since this can be the full tree including hidden items, the data returned * may be used for generating an an admin interface or a select. * - * @param string $menu_name - * The named menu links to return * @param array $id * A menu link ID, or NULL. If a link ID is supplied, only the * path to root will be included in the returned tree - as if this link @@ -90,13 +80,15 @@ public function buildPageData($menu_name, $max_depth = NULL); * $id, in which case $max_depth should be greater than $link['depth']. * * @return array - * An tree of menu links in an array, in the order they should be rendered. + * An array of tree parameters. */ - public function buildAllData($menu_name, $id = NULL, $max_depth = NULL); + public function buildAllDataTreeParameters($id = NULL, $max_depth = NULL); /** * Builds a menu tree, translates links, and checks access. * + * Only visible links (hidden == 0) are returned in the data. + * * @param string $menu_name * The name of the menu. * @param array $parameters @@ -116,20 +108,40 @@ public function buildAllData($menu_name, $id = NULL, $max_depth = NULL); * condition key/value pairs; see _menu_build_tree() for the actual query. * * @return array - * A fully built and access-checked menu tree. + * An array of menu links, in the order they should be rendered. The array + * is a list of associative arrays -- these have several keys: + * - link: the menu link plugin instance + * - below: the subtree below the link, or empty array. It has the same + * structure as the top level array. + * - depth: + * - has_children: boolean. even if the below value may be empty the link + * may have children in the tree that are not shown. This + * is a hint for adding appropriate classes for theming. + * - in_active_trail: boolean */ public function buildTree($menu_name, array $parameters = array()); /** * Returns a subtree starting with the passed in menu link plugin ID. * + * Only visible links (hidden == 0) are returned in the data. + * * @param string $id * The menu link plugin ID. * @param int $max_relative_depth * The maximum depth of child menu links relative to the passed in. * * @return array - * A fully built and access-checked menu subtree. + * An array of menu links, in the order they should be rendered. The array + * is a list of associative arrays -- these have several keys: + * - link: the menu link plugin instance + * - below: the subtree below the link, or empty array. It has the same + * structure as the top level array. + * - depth: + * - has_children: boolean. even if the below value may be empty the link + * may have children in the tree that are not shown. This + * is a hint for adding appropriate classes for theming. + * - in_active_trail: boolean */ public function buildSubtree($id, $max_relative_depth = NULL); @@ -147,9 +159,4 @@ public function buildSubtree($id, $max_relative_depth = NULL); */ public function getChildLinks($id, $max_relative_depth = NULL); - /** - * For test purposes, clear any static data caches. - */ - public function resetStaticCache(); - } diff --git a/core/lib/Drupal/Core/Menu/MenuParentFormSelector.php b/core/lib/Drupal/Core/Menu/MenuParentFormSelector.php index 46bf257..6226c19 100644 --- a/core/lib/Drupal/Core/Menu/MenuParentFormSelector.php +++ b/core/lib/Drupal/Core/Menu/MenuParentFormSelector.php @@ -45,7 +45,8 @@ public function getParentSelectOptions($id = '', array $menus = NULL) { foreach ($menus as $menu_name => $menu_title) { $options[$menu_name . ':'] = '<' . $menu_title . '>'; - $tree = $this->menuLinkTree->buildAllData($menu_name, NULL, $depth_limit); + $parameters = $this->menuLinkTree->buildAllDataTreeParameters(NULL, $depth_limit); + $tree = $this->menuLinkTree->buildTree($menu_name, $parameters); $this->parentSelectOptionsTreeWalk($tree, $menu_name, '--', $options, $id, $depth_limit); } return $options; @@ -148,4 +149,4 @@ protected function getMenuOptions(array $menu_names = NULL) { return $options; } -} \ No newline at end of file +} diff --git a/core/modules/menu_ui/src/MenuForm.php b/core/modules/menu_ui/src/MenuForm.php index 29835a2..c6d1f21 100644 --- a/core/modules/menu_ui/src/MenuForm.php +++ b/core/modules/menu_ui/src/MenuForm.php @@ -217,7 +217,8 @@ protected function buildOverviewForm(array &$form, array &$form_state) { // We indicate that a menu administrator is running the menu access check. $this->getRequest()->attributes->set('_menu_admin', TRUE); - $tree = $this->menuTree->buildAllData($this->entity->id()); + $parameters = $this->menuTree->buildAllDataTreeParameters(); + $tree = $this->menuTree->buildTree($this->entity->id(), $parameters); $count = $this->countElements($tree); $delta = max($count, 50); diff --git a/core/modules/system/src/Plugin/Block/SystemMenuBlock.php b/core/modules/system/src/Plugin/Block/SystemMenuBlock.php index c22cfaa..9e1fc50 100644 --- a/core/modules/system/src/Plugin/Block/SystemMenuBlock.php +++ b/core/modules/system/src/Plugin/Block/SystemMenuBlock.php @@ -35,7 +35,7 @@ class SystemMenuBlock extends BlockBase implements ContainerFactoryPluginInterfa protected $menuTree; /** - * The menu active trail service. + * The active menu trail service. * * @var \Drupal\Core\Menu\MenuActiveTrailInterface */ @@ -53,7 +53,7 @@ class SystemMenuBlock extends BlockBase implements ContainerFactoryPluginInterfa * @param \Drupal\Core\Menu\MenuLinkTreeInterface $menu_tree * The menu tree. * @param \Drupal\Core\Menu\MenuActiveTrailInterface $menu_active_trail - * Menu active trail service. + * The active menu trail service. */ public function __construct(array $configuration, $plugin_id, $plugin_definition, MenuLinkTreeInterface $menu_tree, MenuActiveTrailInterface $menu_active_trail) { parent::__construct($configuration, $plugin_id, $plugin_definition); @@ -79,7 +79,8 @@ public static function create(ContainerInterface $container, array $configuratio */ public function build() { $menu_name = $this->getDerivativeId(); - $tree = $this->menuTree->buildPageData($menu_name); + $parameters = $this->menuTree->buildPageDataTreeParameters($menu_name); + $tree = $this->menuTree->buildTree($menu_name, $parameters); return $this->menuTree->buildRenderTree($tree); } diff --git a/core/modules/system/src/SystemManager.php b/core/modules/system/src/SystemManager.php index 33d8c19..18b6a85 100644 --- a/core/modules/system/src/SystemManager.php +++ b/core/modules/system/src/SystemManager.php @@ -49,7 +49,7 @@ class SystemManager { protected $menuTree; /** - * The menu active trail service. + * The active menu trail service. * * @var \Drupal\Core\Menu\MenuActiveTrailInterface */ @@ -91,7 +91,7 @@ class SystemManager { * @param \Drupal\Core\Menu\MenuLinkTreeInterface $menu_tree * The menu tree manager. * @param \Drupal\Core\Menu\MenuActiveTrailInterface $menu_active_trail - * Menu active trail service. + * The active menu trail service. */ public function __construct(ModuleHandlerInterface $module_handler, Connection $database, EntityManagerInterface $entity_manager, RequestStack $request_stack, MenuLinkTreeInterface $menu_tree, MenuActiveTrailInterface $menu_active_trail) { $this->moduleHandler = $module_handler; diff --git a/core/modules/system/src/Tests/Menu/MenuLinkTreeTest.php b/core/modules/system/src/Tests/Menu/MenuLinkTreeTest.php index caabb22..92f6744 100644 --- a/core/modules/system/src/Tests/Menu/MenuLinkTreeTest.php +++ b/core/modules/system/src/Tests/Menu/MenuLinkTreeTest.php @@ -79,7 +79,6 @@ public function testDeleteLinksInMenu() { $this->assertEqual(count($output), 1); $this->menuLinkManager->deleteLinksInMenu('menu1'); - $this->linkTree->resetStaticCache(); $output = $this->linkTree->buildTree('menu1'); $this->assertEqual(count($output), 0);