sorentwo / readthis

:newspaper: Pooled active support compliant caching with redis
MIT License
504 stars 40 forks source link

does not work with rails5 cached collection renderer #49

Closed dreyks closed 8 years ago

dreyks commented 8 years ago

So, Rails5 introduced multi fetching when rendering a collection in partial. It does not use fetch_multi but calls directly read_multi instead, thus treating nil values as a valid cached contents. This leads to such partials being considered as "present" in cache and just being nil

As seen here the hits are counted as a size of result returned by multi_read. Later to check if there's a hit for a particular partial it calls fetch on the result hash.

Abstract ActiveSupport::Cache::Store#read_multi treats nil as cache miss

To make readthis a drop-in replacement you'll sadly have to make a backwards-incompatible change to read_multi

sorentwo commented 8 years ago

Interestingly I've always been in favor of treating nil as a cache miss, so I'm fine with the change.

sorentwo commented 8 years ago

On second thought, or at least, on attempt, this is a little trickier. fetch_multi relies on the fact that read_multi retains nil values, so this will take some more refactoring.

dreyks commented 8 years ago

yes read_multi will have to remove missed keys and this will probably break some things

sorentwo commented 8 years ago

2.0.0 has been released with the breaking change, and an escape hatch to retain the old behavior: c9c5c0a