Cashier.expire should clear keys stored in that tag

[amoslanka]

As it currently seems to be working, if I expire a tag using cashier, after expiring all the keys associated with the tag, those keys still exist in that tag's list of keys. (I'm using Redis)

For example:

I am action caching a search page tagged with "locations" where the search string is part of the cache key. So the following would be a list of the cache keys created over a period of time:

views/localhost:3000/locations?q=1
views/localhost:3000/locations?q=2
views/localhost:3000/locations?q=3

Those three keys would be in the list under the key based on the tag "locations".

If I run Cashier.expire "locations", those three view caches are correctly expired. However, if I then visit views/localhost:3000/locations?q=4, and then run Cashier.keys, I get the following output:

views/localhost:3000/locations?q=1
views/localhost:3000/locations?q=2
views/localhost:3000/locations?q=3
views/localhost:3000/locations?q=4

This is because when the tag was expired, the "locations" entry in Redis was left as is.

Is this intended? My opinion is that any key expired should be removed from the tag's list of keys. In a production environment, if there is an infinite number of queries my users would search for, then the "locations" tag's list of keys would grow infinitely as well, and it doesn't seem reasonable to expire the tag myself after running the expire method, as this is a jump into the inner workings of Cashier that my app shouldn't expect to understand.

[ahawkins]

Good catch! I'd be happy to accept a PR for this fix :)

On Thu, Feb 28, 2013 at 11:33 PM, Amos Lanka notifications@github.comwrote:

As it currently seems to be working, if I expire a tag using cashier, after expiring all the keys associated with the tag, those keys still exist in that tag's list of keys. (I'm using Redis)

For example:

I am action caching a search page tagged with "locations" where the search string is part of the cache key. So the following would be a list of the cache keys created over a period of time:

views/localhost:3000/locations?q=1 views/localhost:3000/locations?q=2 views/localhost:3000/locations?q=3

Those three keys would be in the list under the key based on the tag "locations".

If I run Cashier.expire "locations", those three view caches are correctly expired. However, if I then visit views/localhost:3000/locations?q=4, and then run Cashier.keys, I get the following output:

views/localhost:3000/locations?q=1 views/localhost:3000/locations?q=2 views/localhost:3000/locations?q=3 views/localhost:3000/locations?q=4

This is because when the tag was expired, the "locations" entry in Redis was left as is.

Is this intended? My opinion is that any key expired should be removed from the tag's list of keys. In a production environment, if there is an infinite number of queries my users would search for, then the "locations" tag's list of keys would grow infinitely as well, and it doesn't seem reasonable to expire the tag myself after running the expire method, as this is a jump into the inner workings of Cashier that my app shouldn't expect to understand.

— Reply to this email directly or view it on GitHubhttps://github.com/twinturbo/cashier/issues/19 .

[amoslanka]

I've been digging through what I can, and am realizing that perhaps this case is already been thought of and executed, though there may somehow be some gaps. After deleting the keys that are associated with a tag, the Cashier.expire method runs adapter.delete_tag(tag), which in turn does just what it says, according to the code. Strange that this doesn't appear to actually be happening on my app. I'll be digging deeper, and will report back what I find.

[ahawkins]

ok. Please keep me informed. This should be working correctly.

On Fri, Mar 1, 2013 at 8:12 AM, Amos Lanka notifications@github.com wrote:

I've been digging through what I can, and am realizing that perhaps this case is already been thought of and executed, though there may somehow be some gaps. After deleting the keys that are associated with a tag, the Cashier.expire method runs adapter.delete_tag(tag), which in turn does just what it says, according to the code. Strange that this doesn't appear to actually be happening on my app. I'll be digging deeper, and will report back what I find.

— Reply to this email directly or view it on GitHubhttps://github.com/twinturbo/cashier/issues/19#issuecomment-14275868 .

[amoslanka]

Ok what I found that I was having trouble with was not as clear as I thought.

First off, the ActionSupport::Notifications system seems to tank when using Webrick on dev. I've been working with Cashier on two seperate apps, and was able to isolate the issue in the end because I was using Thin on one app and not the other. There may need to be some messaging in the Readme about using Thin (or another server) since Cashier is dependent upon the notifications.

Second, I used the method name "expire" in describing this issue, but should have used the method "clear". Expire works as expected, #clear only removed the "cashier-tags" key. I would expect Cashier.clear to work similarly to Rails.cache.clear, being more broad in what it clears, so in my upcoming pull request I've updated #clear to delete all tagged fragments, all tags, and the "cashier-tags" key.

[ahawkins]

Seems this PR and the other share commits. Is there any dependence here?

[amoslanka]

I accidentally committed these to master and they ended up getting pulled into the namespaces branch that i used for that feature. That was unintentional, and I don't believe namespace stuff is dependent upon these commits.

Also, I threw in the tag count return on simply to make Cashier.clear return something similar to what Rails.cache.clear returns, which is the number of fragments cleared. No real world application yet.

[amoslanka]

Any further changes necessary in order to accept this PR? Would like to close the issue.