Posted by Kars-T on January 28, 2012 at 2:59pm
6 followers
| Project: | Panels |
| Version: | 7.x-3.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | merlinofchaos |
| Status: | needs review |
Issue Summary
If I activate panels simple cache with arguments my site will display an exception.
I checked this with a debugger and in this case Panels Everywhere delivers an object as args but the simple caches foreach() and needs an array.
panels/plugins/cache/simple.inc line 98
<?php
case 'args':
foreach ($args as $arg) {
$id .= ':' . $arg;
}
break;
?>panels_everywhere/theme/theme.inc line 18
<?php
$content = new stdClass();
$content->title = drupal_get_title();
if (!empty($page['content'])) {
$content->content = $page['content'];
}
else {
$content->content = " ";
}
$args = array($content);
$contexts = ctools_context_handler_get_task_contexts($task, '', $args);
?>My debugger showed that args contains various things. Most times it is some id probably the nid but sometimes it does contain objects.
I have no clue what panels is doing with the args as I don't know the API that far. And it seems panels has no problems with args being an object. But the caching will fail.
I could do a patch that could get us a crc32 or something key from the object so we can use that for caching. Would that make sense?
Comments
#1
Okay I made a patch. I use serialize() and md5() to create the cache id. This should never fail but maybe we need an empty check before as I am not sure how serialize() will behave.
And I added arg() to be used as caching context as well. In my special case we use the nice menu module. It normally shows the active path but with context or arguments cache it won't change and always behave like we are currently seeing the homepage. If I use arg() for the cid it will react normally but will reside in cache. I extended the comment but think my english sounds stupid.
So what do you think about it?
Can we use md5(serialize()) as cid base?
Is it good to add arg() as cache context as well?
#2
Won't this break the code that tries to clear old entries from the cache when a panel is updated?
#3
Yes thats true. I write an hook_update() to delete the old entries than.
#4
Added new version with wild card deletion of old cache entries in a hook_update.
#5
New version with an line break at end of file...
#6
#7
A hook_update isn't really what I meant. :(
The issue is that right now, the code tries to delete old entries on an as-needed basis. When using MD5s, there isn't a way to identify these old entries. Though I suppose cron can probably do the job.
#8
Actually, cron isn't enough, because you have to be able to delete stale entries when something in the underlying panel is updated. So this will actually totally break that. In order to fix that, you're going to need cache tags or something similar.
And this patch doesn't address that code at all, and it will have to, as it will be running useless queries I think.
See panels_simple_cache_clear_cache()
#9
If those prefixes are save to use as they are probably numeric
<?php
// This is used in case this is an in-code display, which means did will be something like 'new-1'.
if (isset($display->owner) && isset($display->owner->id)) {
$cid .= ':' . $display->owner->id;
}
$cid .= ':' . $display->did;
cache_clear_all($cid, 'cache', TRUE);
?>
maybe we should use them always as prefix and hash the rest?
#10
Yes I think we could probably do that safely.
#11
Because panels_simple_cache_get_id() currently doesn't care for the owner ID at all, I'm not entirely sure how the cache clear currently works. I think this is what you guys meant #9 and #10 but not entirely sure. Feedback appreciated.
#12
Ahh, sorry ignore #11. That was the entirely wrong patch. Here we go.
#13
Thanks, this patch worked fine for me :-)
To be more precise: After patching module I found an option "cache by url". With that option I could cache breadcrumbs pane in site-template in panels everywhere. Caching of that pane works as expected and I couldn't detect any other erratic behaviour in panels elsewhere on that site. So I think, it is alright.