#620618: the toolbar module should properly build its menu slice.

From: Damien Tournoud <damien@tournoud.net>


---
 menu.inc               |  275 +++++++++++++++++++++++++++---------------------
 toolbar/toolbar.module |   13 +-
 2 files changed, 161 insertions(+), 127 deletions(-)

diff --git includes/menu.inc includes/menu.inc
index 2951129..f114403 100644
--- includes/menu.inc
+++ includes/menu.inc
@@ -961,77 +961,44 @@ function menu_tree_all_data($menu_name, $link = NULL, $max_depth = NULL) {
     // If the static variable doesn't have the data, check {cache_menu}.
     $cache = cache_get($cid, 'cache_menu');
     if ($cache && isset($cache->data)) {
-      // If the cache entry exists, it will just be the cid for the actual data.
-      // This avoids duplication of large amounts of data.
-      $cache = cache_get($cache->data, 'cache_menu');
-      if ($cache && isset($cache->data)) {
-        $data = $cache->data;
-      }
+      // If the cache entry exists, it will just be the parameters of
+      // menu_build_tree().
+      $tree_parameters = $cache->data;
     }
     // If the tree data was not in the cache, $data will be NULL.
-    if (!isset($data)) {
-      // Build the query using a LEFT JOIN since there is no match in
-      // {menu_router} for an external link.
-      $query = db_select('menu_links', 'ml', array('fetch' => PDO::FETCH_ASSOC));
-      $query->addTag('translatable');
-      $query->leftJoin('menu_router', 'm', 'm.path = ml.router_path');
-      $query->fields('ml');
-      $query->fields('m', array(
-        'load_functions',
-        'to_arg_functions',
-        'access_callback',
-        'access_arguments',
-        'page_callback',
-        'page_arguments',
-        'delivery_callback',
-        'title',
-        'title_callback',
-        'title_arguments',
-        'theme_callback',
-        'theme_arguments',
-        'type',
-        'description',
-      ));
-      for ($i = 1; $i <= MENU_MAX_DEPTH; $i++) {
-        $query->orderBy('p' . $i, 'ASC');
-      }
-      $query->condition('ml.menu_name', $menu_name);
-      if (isset($max_depth)) {
-        $query->condition('ml.depth', $max_depth, '<=');
-      }
+    if (!isset($tree_parameters)) {
       if ($mlid) {
         // The tree is for a single item, so we need to match the values in its
         // p columns and 0 (the top level) with the plid values of other links.
-        $args = array(0);
+        $parents = array(0);
         for ($i = 1; $i < MENU_MAX_DEPTH; $i++) {
-          $args[] = $link["p$i"];
+          if (!empty($link["p$i"])) {
+            $parents[] = $link["p$i"];
+          }
         }
-        $args = array_unique($args);
-        $query->condition('ml.plid', $args, 'IN');
-        $parents = $args;
-        $parents[] = $link['mlid'];
+        $active_link = $parents;
+        $active_link[] = $mlid;
       }
       else {
         // Get all links in this menu.
         $parents = array();
+        $active_link = array();
       }
-      // Select the links from the table, and build an ordered array of links
-      // using the query result object.
-      $links = array();
-      foreach ($query->execute() as $item) {
-        $links[] = $item;
-      }
-      $data['tree'] = menu_tree_data($links, $parents);
-      $data['node_links'] = array();
-      menu_tree_collect_node_links($data['tree'], $data['node_links']);
-      // Cache the data, if it is not already in the cache.
-      $tree_cid = _menu_tree_cid($menu_name, $data);
-      if (!cache_get($tree_cid, 'cache_menu')) {
-        cache_set($tree_cid, $data, 'cache_menu');
-      }
-      // Cache the cid of the (shared) data using the menu and item-specific cid.
-      cache_set($cid, $tree_cid, 'cache_menu');
+      $tree_parameters = array(
+        'parents' => $parents,
+        'active_link' => $active_link,
+        'min_depth' => 1,
+        'max_depth' => $max_depth,
+      );
+
+      // Cache the tree building parameters using the page-specific cid.
+      cache_set($cid, $tree_parameters, 'cache_menu');
     }
+
+    // Build the tree using the parameters (the resulting tree will get
+    // cached by menu_build_tree()).
+    $data = menu_build_tree($menu_name, $tree_parameters);
+
     // Check access for the current user to each item in the tree.
     menu_tree_check_access($data['tree'], $data['node_links']);
     $tree[$cid] = $data['tree'];
@@ -1073,15 +1040,12 @@ function menu_tree_page_data($menu_name, $max_depth = NULL) {
       // If the static variable doesn't have the data, check {cache_menu}.
       $cache = cache_get($cid, 'cache_menu');
       if ($cache && isset($cache->data)) {
-        // If the cache entry exists, it will just be the cid for the actual data.
-        // This avoids duplication of large amounts of data.
-        $cache = cache_get($cache->data, 'cache_menu');
-        if ($cache && isset($cache->data)) {
-          $data = $cache->data;
-        }
+        // If the cache entry exists, it will just be the parameters of
+        // menu_build_tree().
+        $tree_parameters = $cache->data;
       }
       // If the tree data was not in the cache, $data will be NULL.
-      if (!isset($data)) {
+      if (!isset($tree_parameters)) {
         // Build and run the query, and build the tree.
         if ($item['access']) {
           // Check whether a menu link exists that corresponds to the current path.
@@ -1089,7 +1053,7 @@ function menu_tree_page_data($menu_name, $max_depth = NULL) {
           if (drupal_is_front_page()) {
             $args[] = '<front>';
           }
-          $parents = db_select('menu_links')
+          $active_link = db_select('menu_links')
             ->fields('menu_links', array(
               'p1',
               'p2',
@@ -1104,10 +1068,10 @@ function menu_tree_page_data($menu_name, $max_depth = NULL) {
             ->condition('link_path', $args, 'IN')
             ->execute()->fetchAssoc();
 
-          if (empty($parents)) {
+          if (empty($active_link)) {
             // If no link exists, we may be on a local task that's not in the links.
             // TODO: Handle the case like a local task on a specific node in the menu.
-            $parents = db_select('menu_links')
+            $active_link = db_select('menu_links')
               ->fields('menu_links', array(
                 'p1',
                 'p2',
@@ -1122,11 +1086,13 @@ function menu_tree_page_data($menu_name, $max_depth = NULL) {
               ->condition('link_path', $item['tab_root'])
               ->execute()->fetchAssoc();
           }
+
           // We always want all the top-level links with plid == 0.
-          $parents[] = '0';
+          $active_link[] = '0';
 
           // Use array_values() so that the indices are numeric for array_merge().
-          $args = $parents = array_unique(array_values($parents));
+          $parents = $active_link = array_unique(array_values($active_link));
+
           $expanded = variable_get('menu_expanded', array());
           // Check whether the current menu has any links set to be expanded.
           if (in_array($menu_name, $expanded)) {
@@ -1138,69 +1104,41 @@ function menu_tree_page_data($menu_name, $max_depth = NULL) {
                 ->condition('menu_name', $menu_name)
                 ->condition('expanded', 1)
                 ->condition('has_children', 1)
-                ->condition('plid', $args, 'IN')
-                ->condition('mlid', $args, 'NOT IN')
+                ->condition('plid', $parents, 'IN')
+                ->condition('mlid', $parents, 'NOT IN')
                 ->execute();
               $num_rows = FALSE;
               foreach ($result as $item) {
-                $args[] = $item['mlid'];
+                $parents[] = $item['mlid'];
                 $num_rows = TRUE;
               }
             } while ($num_rows);
           }
+          $tree_parameters = array(
+            'parents' => $parents,
+            'active_link' => $active_link,
+            'min_depth' => 1,
+            'max_depth' => $max_depth,
+          );
         }
         else {
           // Show only the top-level menu items when access is denied.
-          $args = array(0);
-          $parents = array();
-        }
-        // Select the links from the table, and recursively build the tree. We
-        // LEFT JOIN since there is no match in {menu_router} for an external
-        // link.
-        $query = db_select('menu_links', 'ml', array('fetch' => PDO::FETCH_ASSOC));
-        $query->addTag('translatable');
-        $query->leftJoin('menu_router', 'm', 'm.path = ml.router_path');
-        $query->fields('ml');
-        $query->fields('m', array(
-          'load_functions',
-          'to_arg_functions',
-          'access_callback',
-          'access_arguments',
-          'page_callback',
-          'page_arguments',
-          'delivery_callback',
-          'title',
-          'title_callback',
-          'title_arguments',
-          'theme_callback',
-          'theme_arguments',
-          'type',
-          'description',
-        ));
-        for ($i = 1; $i <= MENU_MAX_DEPTH; $i++) {
-          $query->orderBy('p' . $i, 'ASC');
-        }
-        $query->condition('ml.menu_name', $menu_name);
-        $query->condition('ml.plid', $args, 'IN');
-        if (isset($max_depth)) {
-          $query->condition('ml.depth', $max_depth, '<=');
+          $tree_parameters = array(
+            'parents' => array(0),
+            'active_link' => array(),
+            'min_depth' => 1,
+            'max_depth' => $max_depth,
+          );
         }
-        // Build an ordered array of links using the query result object.
-        $links = array();
-        foreach ($query->execute() as $item) {
-          $links[] = $item;
-        }
-        $data['tree'] = menu_tree_data($links, $parents);
-        $data['node_links'] = array();
-        menu_tree_collect_node_links($data['tree'], $data['node_links']);
-        // Cache the data, if it is not already in the cache.
-        $tree_cid = _menu_tree_cid($menu_name, $data);
-        if (!cache_get($tree_cid, 'cache_menu')) {
-          cache_set($tree_cid, $data, 'cache_menu');
-        }
-        // Cache the cid of the (shared) data using the page-specific cid.
-        cache_set($cid, $tree_cid, 'cache_menu');
+
+        // Cache the tree building parameters using the page-specific cid.
+        cache_set($cid, $tree_parameters, 'cache_menu');
       }
+
+      // Build the tree using the parameters (the resulting tree will get
+      // cached by menu_build_tree()).
+      $data = menu_build_tree($menu_name, $tree_parameters);
+
       // Check access for the current user to each item in the tree.
       menu_tree_check_access($data['tree'], $data['node_links']);
       $tree[$cid] = $data['tree'];
@@ -1212,6 +1150,101 @@ function menu_tree_page_data($menu_name, $max_depth = NULL) {
 }
 
 /**
+ * Build a menu tree.
+ *
+ * @param $menu_name
+ *   The name of the menu.
+ * @param $parameters
+ *   An associative array of build parameters. Possible keys:
+ *     - 'parents': return only menu links that are children of a link in this
+ *        list. If empty, build the whole menu tree.
+ *     - 'active_link': an array of mlid, representing the coordinates
+ *        of the active link.
+ *     - 'min_depth': the minimum depth of the resulting tree (defaults to 1).
+ *     - 'max_depth': the maximum depth of the resulting tree (defaults to
+ *        infinite).
+ * @return
+ *   A fully built menu tree.
+ */
+function menu_build_tree($menu_name, array $parameters = array()) {
+  // Merge in defaults.
+  $parameters += array(
+    'parents' => array(),
+    'active_link' => array(),
+    'min_depth' => 1,
+    'max_depth' => NULL,
+  );
+
+  // Cache of the built trees.
+  $trees = &drupal_static(__FUNCTION__, array());
+
+  // Build the cache id.
+  sort($parameters['parents']);
+  $tree_cid = 'links:' . $menu_name . ':' . serialize($parameters);
+
+  if (!isset($trees[$tree_cid])) {
+    // If the static variable doesn't have the data, check {cache_menu}.
+    $cache = cache_get($tree_cid, 'cache_menu');
+    if ($cache && isset($cache->data)) {
+      $trees[$tree_cid] = $cache->data;
+    }
+  }
+
+  if (!isset($trees[$tree_cid])) {
+    // Select the links from the table, and recursively build the tree. We
+    // LEFT JOIN since there is no match in {menu_router} for an external
+    // link.
+    $query = db_select('menu_links', 'ml', array('fetch' => PDO::FETCH_ASSOC));
+    $query->addTag('translatable');
+    $query->leftJoin('menu_router', 'm', 'm.path = ml.router_path');
+    $query->fields('ml');
+    $query->fields('m', array(
+      'load_functions',
+      'to_arg_functions',
+      'access_callback',
+      'access_arguments',
+      'page_callback',
+      'page_arguments',
+      'delivery_callback',
+      'title',
+      'title_callback',
+      'title_arguments',
+      'theme_callback',
+      'theme_arguments',
+      'type',
+      'description',
+    ));
+    for ($i = 1; $i <= MENU_MAX_DEPTH; $i++) {
+      $query->orderBy('p' . $i, 'ASC');
+    }
+    $query->condition('ml.menu_name', $menu_name);
+    if (!empty($parameters['parents'])) {
+      $query->condition('ml.plid', $parameters['parents'], 'IN');
+    }
+    if ($parameters['min_depth'] != 1) {
+      $query->condition('ml.depth', $parameters['min_depth'], '>=');
+    }
+    if (isset($parameters['max_depth'])) {
+      $query->condition('ml.depth', $parameters['max_depth'], '<=');
+    }
+
+    // Build an ordered array of links using the query result object.
+    $links = array();
+    foreach ($query->execute() as $item) {
+      $links[] = $item;
+    }
+    $data['tree'] = menu_tree_data($links, $parameters['active_link'], $parameters['min_depth']);
+    $data['node_links'] = array();
+    menu_tree_collect_node_links($data['tree'], $data['node_links']);
+    // Cache the data, if it is not already in the cache.
+    cache_set($tree_cid, $data, 'cache_menu');
+    $trees[$tree_cid] = $data;
+  }
+
+  return $trees[$tree_cid];
+}
+
+/**
  * Helper function - compute the real cache ID for menu tree data.
  */
 function _menu_tree_cid($menu_name, $data) {
diff --git modules/toolbar/toolbar.module modules/toolbar/toolbar.module
index 26a0bc1..743e033 100644
--- modules/toolbar/toolbar.module
+++ modules/toolbar/toolbar.module
@@ -134,12 +134,13 @@ function toolbar_get_menu_tree() {
   $tree = array();
   $admin_link = db_query("SELECT * FROM {menu_links} WHERE menu_name = 'management' AND module = 'system' AND link_path = 'admin'")->fetchAssoc();
   if ($admin_link) {
-    // @todo Use a function like book_menu_subtree_data().
-    $tree = menu_tree_all_data('management', $admin_link, $admin_link['depth'] + 1);
-    // The tree will be a sub-tree with the admin link as a single root item.
-    // @todo It is wrong to assume it's the last.
-    $admin_link = array_pop($tree);
-    $tree = $admin_link['below'] ? $admin_link['below'] : array();
+    $data = menu_build_tree('management', array(
+      'parents' => array($admin_link['mlid']),
+      'min_depth' => 2,
+      'max_depth' => 2,
+    ));
+    menu_tree_check_access($data['tree'], $data['node_links']);
+    $tree = $data['tree'];
   }
   return $tree;
 }