Because update_flush_caches doesn't include it's special update cache tables unless the MAINTENANCE_MODE constant is defined, none of the cache flush options in admin_menu allow us to flush the update caches.

Comments

dave reid’s picture

Status: Active » Needs review
StatusFileSize
new2 KB
new1.88 KB

Patches for HEAD and DRUPAL-6--3 attached to add an update cache flush menu if update.module is enabled.

dave reid’s picture

I think sun mentioned to me on IRC that modules have a way to alter these cache links, but I couldn't see how seeing as the callback switch code is hard-coded. Are modules able to alter this?

j0rd’s picture

Title: Add a path to clear update cache » Allow contrib modules to hook into admin_menu's "Flush All Caches"
StatusFileSize
new1.25 KB

I was told my issue is a dupe of this issue.

I would like the ability to add a "Clear APC Cache" child item into the "Flush All Caches" menu and have it called when "Flush All Caches" is pressed.

Now I can add the child menu item with hook_admin_menu and hook_menu , but I'm not able to get my stuff into the "hardcoded switch" as Dave as mentioned.

There is a way to "hack" it in using hook_flush_caches though. It's no ideal as hook_flush_caches() get's called too often by other modules, which means my APC cache is getting cleared much much more than I want. Ideally my hook would only get called manually after I update the files on my server.

Provided is my example hackish module which works to clear APC cache when it's menu item is pressed and when "Flush All Caches" is pressed.

sun’s picture

In light of #420816: On-demand loading of dynamic paths and local tasks #1, we want to do this a bit differently.

1) admin_menu should register the path admin-menu/flush-caches (which it currently does already)

2) In addition to that, it should register the path admin-menu/flush-caches/%

3) Implementations then use the new expand_map hook to populate the menu with further dynamic links.

Or is this bullshit? Yes, it is. Sorry.

It doesn't work, because we want to execute something different for each sub-item.

Well, then in light of #631550: Stale + improper logic for MENU_VISIBLE_IN_TREE and MENU_VISIBLE_IN_BREADCRUMB, we may want to register local tasks instead, and make them execute the actual cache clearing function directly?

meh. That wouldn't execute the final redirect in admin_menu_flush_cache().

So perhaps back to a semi-dynamic, but local tasks approach? I.e. make the page callback execute admin_menu_flush_cache(), but assign a certain callback function as first page argument?

Whatever we do here, we want to (partially or not) revamp the existing implementation to remove most of the hard-coded logic.

mstrelan’s picture

Is this still on the cards? It's been almost 2.5 years since the last update and it would make #1471312: Add "Flush LESS files" to admin_menu's "flush all caches" submenu a lot nicer.

Status: Needs review » Needs work

The last submitted patch, 588518-D6.patch, failed testing.

sun’s picture

Assigned: dave reid » Unassigned
Status: Needs work » Needs review
StatusFileSize
new3.63 KB

Yes, this feature request still makes a lot of sense.

We merely need to think through the problem space a bit more in-depth to find the proper solution.

  1. Right now, all cache flush items are hard-coded into admin_menu's hook_menu() and the corresponding page callback admin_menu_flush_cache(), which is invoked for all.
  2. We want to add further cache flush items (such as for Update module, but also for contrib modules) that only exist conditionally.
  3. In the special case of Update module, admin_menu needs to implement the functionality on behalf of Update module in core.
  4. For other contrib modules, the respective contrib module needs to at least provide an cache item name + label, as well as a callback to invoke.
  5. The most simple solution would be to register additional router items in hook_menu() with the static paths of the additional cache flush items.
    However, that would not only pollute the router system; it would also require each module to define the full router item, possibly re-using admin_menu's access definition (or not). Each module would have to copy the execution logic of admin_menu_flush_cache(), which involves proper CSRF token handling as well as the final redirect...
  6. So an alternate approach would be a hook_admin_menu_cache_info() (or similar) to register the different cache flush operations, automatically expand them via hook_admin_menu_map() (done by admin_menu itself), and invoke the registered callback from admin_menu_flush_cache().

What do you think of attached patch?

sun’s picture

StatusFileSize
new3.32 KB
+++ b/admin_menu.inc
@@ -761,6 +754,48 @@ function _admin_menu_developer_modules() {
+    'title' => t('Update data'),
+    'callback' => '_admin_menu_flush_cache',

@@ -809,6 +863,10 @@ function admin_menu_flush_cache($name = NULL) {
+    case 'update':
+      _update_cache_clear();
+      break;

Apparently, 'update' and 'admin_menu' only invoke one specific callback, so we can register that callback directly, which is a nice clean-up and simplifies things.

Thus, the _admin_menu_flush_cache() helper is only used for core caches. yay :)

mstrelan’s picture

StatusFileSize
new3.57 KB

Love it! But you missed the case for "Flush all caches". Updated patch attached.

mstrelan’s picture

Another thought, should admin_menu provide an optional callback message and put that through drupal_set_message()? For the LESS module I am writing a patch and would prefer to just call less_flush_caches(), which intentionally doesn't set a message, but at the moment if I want to set a message I have to create a new callback function which calls less_flush_caches() and drupal_set_message().

Alternatively admin_menu could just set a default message which includes the name of the cache that was flushed. That would probably be sufficient.

sun’s picture

StatusFileSize
new23.25 KB

admin_menu.caches.png

sun’s picture

StatusFileSize
new4.54 KB

Attached patch incorporates #10, which required to change some of the labels to make them work in the generic dsm().

I'm happy with this patch.

sun’s picture

Status: Needs review » Fixed
StatusFileSize
new5.81 KB

Thanks for reporting, reviewing, and testing! Committed attached patch to all branches.

A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.