We currently rely on the panels naming of panel panes for cache clearing. This doesn't really make sense, since it changes when you change the panel order and also when you go between an in-code and an in-database panel.

To fix this situation, I think we should expose the cache key to the users and let them configure them. We can pre-populate it with a value by default so they don't have to do it, but power users can use this. This also opens up interesting advanced use cases, you could for instance prefix all panels in a group with the same cache prefix and then do a wildcard cache clear.

CommentFileSizeAuthor
#10 set-cache-key-1428196-9.patch24.04 KBfabsor
#8 set-cache-key-1428196-8.patch23.7 KBdas-peter
#6 set-cache-key-1428196-6.patch24.07 KBdas-peter
#5 set-cache-key-1428196-5.patch24.14 KBdas-peter
#4 set-cache-key-1428196-4.patch23.27 KBdas-peter
#3 set-cache-key-1428196-3.patch22.05 KBdas-peter
#1 set-cache-key-1428196-1.patch21.23 KBfabsor

Comments

fabsor’s picture

fabsor commented
Status: Active » Needs review
StatusFileSize
new set-cache-key-1428196-1.patch21.23 KB

Here is a quite big patch that addresses the problem.

* The cache plugin will automaticly create a cache id for each pane you set it on.
* There is an advanced fieldset that let's you set the cache key yourself
* I added substitutions that can be used by advanced users. This won't work with the regular pane cache invalidation rule though, so I strip those away from the administration of that action.
* I added some code that will automaticly update the name of the cache key in rules that contains them. It runs after a panel has been saved.
* I added an upgrade path. I don't know if it makes sense to have that or not.

das-peter’s picture

das-peter commented

I'm all for it. :)
To provide a default but allow users to change it is really a nice way to support simple as well as complex scenarios at the same time.

das-peter’s picture

das-peter commented
StatusFileSize
new set-cache-key-1428196-3.patch22.05 KB

I'll give this a shot now.
I've re-rolled the patch to clean up a few coding standards issues.

das-peter’s picture

das-peter commented
StatusFileSize
new set-cache-key-1428196-4.patch23.27 KB

Looks good so far.
I've made some small changes:

  • cache_actions_update_7201():
    Ensure the loop acts only on RulesAction. RulesLoop doesn't have a ->getElementName() and will throw an error.
    Manually trigger cache_actions_panels_display_save() since we can't be sure that the hook is already registered at the time to update runs.
  • Sneaked in: #1425936-4: Store the detected cache bins to avoid invoking hook_flush_caches() multiple times.
  • Replaced deprecated panels_get_pane_title() by ctools_content_admin_title()
  • cache_actions_rules_cache_settings_form_validate: added check to ensure $pane->cache['method'] exists before accessing it.

Tests on developer stage were promising. Tests on test stage will begin shortly.

das-peter’s picture

das-peter commented
StatusFileSize
new set-cache-key-1428196-5.patch24.14 KB

Next update :)

  • 'cache_actions_' . $display->cache_key replaced by cache_actions_updated_panes and usage of sub-array in it. That way the module doesn't create a random variable.
  • $key = 'task:' . $handler->name . ':' . $handler->task; in _cache_actions_get_panels_handlers was on the wrong location - I guess...
  • Added missing $form['display_cache_key'] in the pane cache settings.
das-peter’s picture

das-peter commented
StatusFileSize
new set-cache-key-1428196-6.patch24.07 KB

Ouch, I should commit before I create a patch...

fabsor’s picture

fabsor commented
Status: Needs review » Needs work

This patch needs a reroll:

* It accidently removes stuff in the .install file that I really think should be there.
* #1425936: Store the detected cache bins to avoid invoking hook_flush_caches() multiple times. is fixed, so this breaks the patch right now.

das-peter’s picture

das-peter commented
StatusFileSize
new set-cache-key-1428196-8.patch23.7 KB

Thanks for committing the other patch :)
Re-rolled patch against latest dev.

das-peter’s picture

das-peter commented
Status: Needs work » Needs review

Oh, and I think it could use a review..

fabsor’s picture

fabsor commented
StatusFileSize
new set-cache-key-1428196-9.patch24.04 KB

There were issues with custom pages, some old code appended the task and plugin name. This patch fixes that issue.

das-peter’s picture

das-peter commented

Ah I already thought, something with the $key is weird ;)

fabsor’s picture

fabsor commented
Status: Needs review » Fixed

Alright, this has been here long enough. I'm commiting this, since this is actually not broken as opposed to what we have, and it's a really great improvement.

das-peter’s picture

das-peter commented

*yay* thanks :)

Status: Fixed » Closed (fixed)

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