artsy / garner

A set of Rack middleware and cache helpers that implement various caching strategies.
MIT License
343 stars 24 forks source link

Inconsistency between Mongoid find and Garner::Mixins::Mongoid::Document.garnered_find #62

Closed mzikherman closed 11 years ago

mzikherman commented 11 years ago

Considering Garner::Mixins::Mongoid::Document.garnered_find I believe the API's for the two 'find' methods should be identical, here is a repro of an example that will cause different results between the two.

Here is the garnered_find method for reference:

def self.garnered_find(handle)
  return nil unless (binding = identify(handle))
  identity = Garner::Cache::Identity.new
  identity.bind(binding).key({ :source => :garnered_find }) do
    find(handle)
  end
end

Assuming an emptied cache:

Artist.garnered_find(['andy-warhol'])  
# returns array of one artist (correct)

Artist.garnered_find('andy-warhol')  
# returns array of one artist, rather than the object itself (incorrect)

Or (assuming an empty cache)

Artist.garnered_find('andy-warhol')  
# returns artist object (correct)

Artist.garnered_find(['andy-warhol'])  
# returns artist object, rather than an array (incorrect)

Contrast with find:

Artist.find(['andy-warhol'])   
# returns array of one artist (correct)

Artist.find('andy-warhol')   
# returns artist object (correct)
fancyremarker commented 11 years ago

Yes, definitely a bug. Thanks for reporting and documenting so clearly. Let's modify garnered_find to take a splat, just as it does in Mongoid.

mzikherman commented 11 years ago

Ok, I'm attempting a fix and will hopefully have a PR soon.

mzikherman commented 11 years ago

Closed in https://github.com/artsy/garner/pull/63