menu_link_maintain doesn't clear cache properly [#278458]

When using menu_link_maintain to edit a link ($op = 'update'), the menu cache is not cleared unless the menu item you're editing is the the 'navigation' menu. The reason is that menu_cache_clear isn't passed the correct menu_name as a parameter so uses 'navigation' by default.

A quick solution would be to patch menu_link_maintain to call menu_cache_clear_all instead.

Current code (menu.inc line 2040)

function menu_link_maintain($module, $op, $link_path, $link_title) {
  switch ($op) {
...............
    case 'update':
      db_query("UPDATE {menu_links} SET link_title = '%s' WHERE link_path = '%s' AND customized = 0 AND module = '%s'", $link_title, $link_path, $module);
      menu_cache_clear(); // clears cache for navigation menu when no parameter is passed in
      break;
...............
  }
}

A solution (maybe not a good one)

function menu_link_maintain($module, $op, $link_path, $link_title) {
  switch ($op) {
...............
    case 'update':
      db_query("UPDATE {menu_links} SET link_title = '%s' WHERE link_path = '%s' AND customized = 0 AND module = '%s'", $link_title, $link_path, $module);
      menu_cache_clear_all(); // clears cache for all menus
      break;
...............
  }
}

Comment	File	Size	Author
#8	maintain-cache-278458-6.patch	907 bytes	pwolanin
#8
#4	maintain-cache-278458-4.patch	920 bytes	pwolanin
#4

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

pwolanin CreditAttribution: pwolanin commented 5 July 2008 at 16:51

Version:

6.2

» 7.x-dev

Yeah, I think I noticed this at one point recently but didn't open an issue.

This code is problematic since we don't know which menus we are saving to. The solution you suggest would work, but we would invalidate less of the cache maybe if we did something like:

    case 'update':
      db_query("UPDATE {menu_links} SET link_title = '%s' WHERE link_path = '%s' AND customized = 0 AND module = '%s'", $link_title, $link_path, $module);
      $result = db_query("SELECT menu_name FROM {menu_links} WHERE link_path = '%s' AND customized = 0 AND module = '%s'", $link_title, $link_path, $module);
      while ($item = db_fetch_array($result)) {
        menu_cache_clear($item['menu_name']);
      }
      break;

Comment #2

pwolanin CreditAttribution: pwolanin commented 6 July 2008 at 20:44

so, chx agrees in IRC that this is the sensible approach

Comment #3

j0hn-smith CreditAttribution: j0hn-smith commented 7 July 2008 at 03:17

A much better solution.

Comment #4

pwolanin CreditAttribution: pwolanin commented 7 July 2008 at 12:46

Status:

Active

» Needs review

File	Size
maintain-cache-278458-4.patch	920 bytes

just rolled the above into a patch. not really tested yet.

Comment #5

catch

he/him

English

CreditAttribution: catch commented 8 July 2008 at 21:33

Patch looks completely fine to me. I ran all tests, no regressions, and manually edited a menu item. Didn't yet look inside the cache_menu table to ensure it's actually deleting the correct records though.

Comment #6

catch

he/him

English

CreditAttribution: catch commented 8 July 2008 at 22:20

Status:

Needs review

» Reviewed & tested by the community

Spoke to pwolanin in irc, doesn't seem to be a way to reproduce this in core. Since it's pretty minor, going to call it RTBC.

Comment #7

catch

he/him

English

CreditAttribution: catch commented 8 July 2008 at 22:25

Status:

Reviewed & tested by the community

» Needs work

Nope, it doesn't work. Regression in aggregator module when changing a category name - menu items should change with it, but they don't with the patch applied.