rack-cache custom cache key patch

49 views
Skip to first unread message

Ryan Tomayko

unread,
Feb 4, 2009, 6:55:08 AM2/4/09
to patna...@gmail.com, rack-...@googlegroups.com
Hi Pat,

I had a chance to sit down with your cache key changes and merge
everything in tonight:

http://github.com/rtomayko/rack-cache/commits/e8bbc84

I changed a few things up and wanted to give an explanation:

* I added the scheme and port to the default canonized key. Content and
cache policy may vary on these as well.

* The query string normalization bit was using request.params but that's
problematic for a couple of reasons: Request#params reads from POST
bodies as well as the query string (theoretically not an issue since we
only cache GET), and Rack 1.0 will return nested parameters a la Rails
so we can't really use it.

* I removed the URI encoding of the cache key from the core metastore
code. The underlying metastore implementation is responsible for
escaping the cache key in whatever way is necessary. The Memcache
metastore implementation uses a SHA hexdigest of the URL, for instance.
Most other metastore implementations do the same, actually. I wasn't
able to test this, however, because my memcached gem and libmemcached
port are all fucked up. Did you see an issue with the memcached tests
without escaping?

* Minor nit but something that would help me out tremendously in the
future is if you could keep patches free of whitespace errors. Any
whitespace at the end of a line and more than one line at the end of the
file is considered a whitespace error. "git diff" will highlight these
in red blocks if you have coloring turned on. There's also a git
pre-commit hook you can turn on that will refuse to commit code with
whitespace errors ("chmod +x .git/hooks/pre-commit" to enable).
Whitespace errors can wreak havoc when trying to merge patches due to
random/insignificant line changes.

The commit messages have additional detail. I've also been thinking
about rearranging some of this code a bit to make it easier to construct
a custom key when needed, but I'll save that for a separate post.

Thanks
--
Ryan http://tomayko.com/about

Reply all
Reply to author
Forward
0 new messages