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

Already on GitHub? Sign in to your account

AS::Cache.expand_cache_key generates wrong key in certain situations #3737

Closed
exviva opened this Issue Nov 23, 2011 · 2 comments

Comments

Projects
None yet
2 participants
Contributor

exviva commented Nov 23, 2011

I've recently found the following problems with ActiveSupport::Cache.expand_cache_key ( https://github.com/rails/rails/blob/master/activesupport/lib/active_support/cache.rb#L75 ):

  1. cache_key method is never called when the argument is a 1-element array with something that responds to cache_key
  2. nil and false both expand to "" (empty string), while true expands to "true"; I believe false should expand to "false"
  3. prefix is added recursively when handling arrays
  4. expanding an instance of ActiveRecord::Relation produces "#<ActiveRecord::Relation:0x000000082a6450>"

I'd say first two points are real bugs, while points 3 and 4 are just...surprising (I'm mentioning them to provoke a discussion).

I'm working on a patch that will expose and fix bugs 1 and 2.

@exviva exviva added a commit to exviva/rails that referenced this issue Nov 23, 2011

@exviva exviva Fix #3737 AS::expand_cache_key generates wrong key in certain situati…
…ons (part 2)

`nil` and `false` both expand to `""` (empty string), while `true` expands to
`"true"`; `false` should expand to `"false"`
a650dd0
Contributor

exviva commented Nov 23, 2011

Wow, that was fast :).

Should it be backported to older branches?

I saw @josevalim fixed the third problem in 3ee0116, now what about expanding ActiveRecord::Relation?

@josevalim josevalim closed this in d8e6dc9 Nov 23, 2011

Contributor

exviva commented Nov 24, 2011

@josevalim, should this be backported somewhere?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment