[development] The new menu system

[Larry Garfield]

On Tuesday 26 September 2006 06:58, Karoly Negyesi wrote:
> Hi,
>
> Note: this post is very long. But it worths a reading.

Indeed!

> I dare to say that one of the biggest results of DrupalCon is that a new
> menu system is born.

Who was the midwife?

> It all began almost a year ago:
>
> http://lists.drupal.org/archives/development/2005-10/msg00708.html
>
> [quote]
> There are three basic tasks to consider when talking about performance
> improvements:
> - displaying the visible menu (in general, this means all accessible items
> since we may need them for DHTML solutions)
> - displaying the breadcrumb trail
> - finding the active callback
> - checking access rights
> [/quote]
>
> OK that's four. I already have code for the last three in my sandbox and
> Adrian have outlined how the first will happen.
>
> My focus was finding the active callback. I simply followed JonBob's
> directions: "We could simplify menu declaration by allowing some sort of
> wildcards in menu paths. For example, if a module could declare the path
> "node/%d/edit", then we could dispense with the logic that defines the
> function only if we are in a node path. The code would certainly be
> cleaner in this case."
>
> I am using the percent sign as a wildcard. There is no %d or %s just %. So
> you define node/%/edit. Let's say someone navigates to node/12/edit/foo.
> Now, how do we find out that node/%/edit needs to be run? We generate all
> possible callbacks and check which one exists:

So far I really like this concept.

> 1111 node/12/edit/foo
> 1110 node/12/edit/%
> 1101 node/12/%/foo
> 1100 node/12/%/%
> 1011 node/%/edit/foo
> 1010 node/%/edit/%
> 1001 node/%/%/foo
> 1000 node/%/%/%
> 111 node/12/edit
> 110 node/12/%
> 101 node/%/edit
> 100 node/%/%
> 11  node/12
> 10  node/%
> 1   node
>
> The binary numbers are generated naturally: where you see a %, that's a 0,
> where you see a specific part, that's a 1. This will nicely indicate the
> the order in which they need to be checked (this is my idea and I am very
> proud of it -- nothing else in here is my idea).

*gives chx a cookie.

So then, you'd generate all interpolations of an incoming path (above), then 
loop and do an isset() on the cached menu array for the path?  Something like 
that?

> So, we have found node/%/edit. Can we access it? Instead of specifying
> 'access' => node_access('update', arg(1)) now we do 'access' =>
> 'node_access', 'access arguments' => array('update', 1) . So we use
> Adrian's favorite: callbacks. I map 1 to arg(1) on runtime for both access
> and callback arguments. (Side note: I know currently node_access gets a
> node and not a node id. Adding node_load won't stop us.)

How well would this support more complex access rules?  Currently the end 
result is a boolean and that's all that's saved.  That allows for complex 
rules like:

user_access('break things') || $uid=1 || $uid == $nid->uid

(or whatever).  How would we do similar compound access rules with this 
system?  It looks like it would only support a single function call, which is 
not always appropriate.

> Our life gets a bit complicated here, because we do not need to grab one
> menu item but three: one that defines the title, one that defines the
> (page) callback and one that defines access. Instead of running three
> queries against a huge table, I planned to retrieve all possible matches,
> and pick my three items. As a bonus, I will get the breadcrumb trail.
> Someone at the second menu meeting (Steven? Adrian?) pointed out that I
> could move this to the builder phase, and that's done. So we are back to
> retrieving one item which has title, callback and access inherited as
> needed.

I do not recall the original thread in detail, and was (sadly) not at 
DrupalCon.  Why would we need 3 calls?  

> At mentioned in the third meeting, Steven pointed out that my system is
> vulnerable to a simple DoS attack so now menu item definitions are limited
> to 6 parts. I can hardly imagine this being a real limitation, I can
> consider raising it to seven but definitely not more. We are shooting at
> what's useable not all possible cases.

That's probably safe for internal paths.  The vast majority of paths accessed 
are node/# anyway, which doesn't exactly have many permutations.  As long as 
the path alias is unaffected, this should be fine.

> Finally we need to display the navigation block. As said, I shoot at
> what's probable so instead of caching per user I will check every menu
> item's access on the fly. The probable case is that these are user_access
> calls. Maybe user_access needs to be changed to use a multi dimensional
> array if the repeated strpos calls are too slow. With that said, we need a
> mechanism to discover which menu items are possible. We will simply cache
> by parent id and unserialize that (credit goes to Adrian for this). Note
> that these structures are nowhere near the size of the current menu array.

User permissions shouldn't be stored as a comma-delimited string anyway.  They 
should be independent entries, so that we can just to an in_array() or even 
isset() on an array of permissions a user has.

> As a bonus feature, we will have hook_menu_alter. To make that usable, we
> moved the path to the menu item key. (Silly me was doing it in the builder
> but Adrian pointed out that I should simply define the menu items this way)

Sweet, more FAPI-like. :-)  Would that allow us to *remove* menu items via 
_menu_alter()?  

> Aside from Adrian and Steven, big kudos goes to Earl Milles
> (merlinofchaos).
>
> What's done? On the menu level, I need to write the navigation block, all
> else is done. I also need to convert modules -- node and user already
> done. All this took only a dozen hours or so, beginning with the first
> evening/night of DrupalCon first day ending on the train to Charleroi
> converting user and three meetings at DrupalCon / BarCamp.

This sounds like a good move overall, but I am curious what a "typical" 
hook_menu() will look like when this is done?  Can you provide or point to a 
basic example?  (An excerpt from the new and improved node_menu() would be 
fine.)

Spiffiness!

-- 
Larry Garfield			AIM: LOLG42
larry at garfieldtech.com		ICQ: 6817012

"If nature has made any one thing less susceptible than all others of 
exclusive property, it is the action of the thinking power called an idea, 
which an individual may exclusively possess as long as he keeps it to 
himself; but the moment it is divulged, it forces itself into the possession 
of every one, and the receiver cannot dispossess himself of it."  -- Thomas 
Jefferson