Yeah, I messed up :)

In the getMultiple() fix patch, I incorrectly ignored non-object cache entries. Don't ask me why, it was late and I was tired, just wanted to avoid notices I didn't understand and then forgot about it...

Anyway, the attached patch implements this in a better way, by storing the $cid in the $full_keys array and then, flipping the array and then basically using it as a $cid lookup hashmap.

CommentFileSizeAuthor
#3 fixed_wildcards_for_real.patch6.32 KBberdir
#2 diff.diff4.33 KBcatch
fix_wildcards.patch925 bytesberdir

Comments

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Priority: Normal » Critical
catch’s picture

catch
he/him
Primary language English
commented
StatusFileSize
new diff.diff4.33 KB

I managed to make the tests fail to catch this (6 fails), but applying this patch only fixed 4. Have not looked into why yet.

Here's what I'm testing with.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
StatusFileSize
new fixed_wildcards_for_real.patch6.32 KB

Awesome. At least it's not only me who does stupid mistakes :)

dmemcache_get_multi() was clearly broken because it returned results keyed with the internal memcache key. But instead of fixing this, wildcards() worked around that bug by also converting the $cid in it's own $wildcards static cache to $full_keys, so that they can be matched..

The fix is simple, remove all that $key/$wildcard conversion stuff and just use $key in wildcards().

Tests pass for me how.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Fixed

Looks great, committed!

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Updated issue summary.