This project is archived and is in readonly mode.
Activesupport caching can't fetch cached boolean false, always re-calculates
Activesupport cache doesn't correctly fetch cached boolean false values. It caches boolean 'true' correctly. It's caused by the incorrect usage of the cached value as a test for the value's existence. The exist? method should be used instead to determine whether the value has been cached.
The incorrect code:
def fetch(key, options = {}, &block)
if !options[:force] && value = read(key, options)
value
elsif block_given?
<< execute the block, etc >>
Suggested code:
def fetch(key, options = {}, &block)
if !options[:force] && exist?(key, options)
read(key, options)
elsif block_given?
A patch is supplied with this change, and the test case added for this particular case.
-
Chris KampmeierHey Brian, nice catch. I agree that the current behavior isn't correct.
I noticed a few problems with the patch, though:
- You'd need to add stubs for
#exist?to the rest of thetest_fetch_*tests. I found that the test suite would hang when I didn't have memcached running, otherwise. - I think there's a subtle race condition with adding
#exist?: since you're doing two reads, you could get a cache hit for the#exist?call, but a miss for the#readcall (because it could be expired or LRU'd out of the cache just between the two calls). So I think it's important to only make one reading call to the cache, even though the#exist?version is obviously more expressive. Also, it seems like only reading from the cache once for hits would be preferable for performance reasons.
Here's a new patch that maintains the current code, but explicitly checks for
nilto fix the problem withfalse. - You'd need to add stubs for
-
Rohit Arondekar- State changed from new to stale
- Importance changed from to
Marking ticket as stale. If this is still an issue please leave a comment with suggested changes, creating a patch with tests, rebasing an existing patch or just confirming the issue on a latest release or master/branches.