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

    case 'args':
      foreach ($args as $arg) {
        $id .= ':' . $arg;
      }
      break;

panels_everywhere/theme/theme.inc line 18

  $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?

CommentFileSizeAuthor
#12 panels-cache.1419710-11.patch3.95 KBtstoeckler
#11 static-cache-eck-classes-1914746-11.patch13.87 KBtstoeckler
#5 panels-cache.1419710.patch2.53 KBkars-t
#4 panels-cache.1419710.patch2.56 KBkars-t
#1 panels-cache.patch2.12 KBkars-t

Comments

kars-t’s picture

kars-t commented
Title: Creation of a cache id for simple caching fails if the args aren't an array » Simple Cache: Change cache keys to md5(serialize()) and use arg() as new caching context
Category: bug » feature
Status: Active » Needs review
StatusFileSize
new panels-cache.patch2.12 KB

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?

merlinofchaos’s picture

merlinofchaos commented

Won't this break the code that tries to clear old entries from the cache when a panel is updated?

kars-t’s picture

kars-t commented

Yes thats true. I write an hook_update() to delete the old entries than.

kars-t’s picture

kars-t commented
StatusFileSize
new panels-cache.1419710.patch2.56 KB

Added new version with wild card deletion of old cache entries in a hook_update.

kars-t’s picture

kars-t commented
StatusFileSize
new panels-cache.1419710.patch2.53 KB

New version with an line break at end of file...

Letharion’s picture

Letharion commented
Assigned: Unassigned » merlinofchaos
merlinofchaos’s picture

merlinofchaos commented

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.

merlinofchaos’s picture

merlinofchaos commented
Status: Needs review » Needs work

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()

kars-t’s picture

kars-t commented

If those prefixes are save to use as they are probably numeric


  // 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?

merlinofchaos’s picture

merlinofchaos commented

Yes I think we could probably do that safely.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Needs work » Needs review
StatusFileSize
new static-cache-eck-classes-1914746-11.patch13.87 KB

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.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
StatusFileSize
new panels-cache.1419710-11.patch3.95 KB

Ahh, sorry ignore #11. That was the entirely wrong patch. Here we go.

R.Hendel’s picture

R.Hendel commented

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.

func0der’s picture

func0der commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

The last submitted patch, 4: panels-cache.1419710.patch, failed testing.

The last submitted patch, 5: panels-cache.1419710.patch, failed testing.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 12: panels-cache.1419710-11.patch, failed testing.

The last submitted patch, 11: static-cache-eck-classes-1914746-11.patch, failed testing.

japerry’s picture

japerry
Location KOMK
commented
Status: Needs work » Closed (outdated)

Drupal 7 is no longer supported, closing.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.