artsy / garner

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

Mongoid document proxy binding doesn't work with aliased updated_at field #86

Open onomated opened 9 years ago

onomated commented 9 years ago

The current implementation of Garner::Mixins::Mongoid::Document::_latest_by_updated_at currently checks for the existence of the updated_at field. This doesn't cover documents that alias the updated_at field (typically by including include Mongoid::Timestamps::Short). This should consider aliased fields. The latest document query can also be optimized to return a single document. Something along these lines:

def self._latest_by_updated_at
  # Only find the latest if we can order by :updated_at
  return nil unless fields['updated_at'] || aliased_fields['updated_at'] 
  only(:_id, :_type, :updated_at).order_by(updated_at: :desc).limit(1).first
end

May serve to document the proxied binding behavior on model classes in the readme, so devs know to create indexes on the updated_at field if they so choose.

Cheers!

dblock commented 9 years ago

Absolutely, would love a PR!

onomated commented 9 years ago

Sounds good. I've tested locally so I'll make a push request for your review!