Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MemcacheStore: deserialize the entry reading from local_cache when using #1608

Merged
merged 1 commit into from
Jul 11, 2011

Conversation

sishen
Copy link
Contributor

@sishen sishen commented Jun 9, 2011

MemcacheStore: deserialize the entry reading from local_cache when using raw.

When with :raw => true and storing the Marshal.dump(obj) into the memcached, Rails.cache.read(key) will return the the serialization data. MemCacheStore should try to deserialize the entry before returning the result.

@sikachu
Copy link
Member

sikachu commented Jul 11, 2011

I like this idea. How do you think @josevalim?

@josevalim
Copy link
Contributor

But isn't :raw exactly about storing a raw object (without dumping)? If your code is dumping it before caching it, you are expected to Marshal.load it as well.

@josevalim
Copy link
Contributor

In other words, what is your use case? :)

@sishen
Copy link
Contributor Author

sishen commented Jul 11, 2011

The problem is not that leave Marshal.load to app when fetching, instead, the solution here is not consistent.

When the entry doesn't exist in LocalCache, the MemcacheStore will try to Marshal.load the raw value before returning the value to app. However, if the entry exists in LocalCache, it will directly return the raw value without Marshal.load.

So we should always leave the dump/load to app users or deserialize both.

@josevalim
Copy link
Contributor

Ah, now I see. The original issue was not clear in the issue message. Thanks.

josevalim added a commit that referenced this pull request Jul 11, 2011
MemcacheStore: deserialize the entry reading from local_cache when using
@josevalim josevalim merged commit 8f2e321 into rails:master Jul 11, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants