From 90f990e8dd7cf5cbce874e03ed3d3974347e7c17 Mon Sep 17 00:00:00 2001
From: danielgrippi <danielgrippi@gmail.com>
Date: Mon, 31 Oct 2011 17:07:14 -0700
Subject: [PATCH] use the cache if cache size is less than limit

---
 lib/diaspora/user/querying.rb     | 17 ++++++++--
 spec/models/user/querying_spec.rb | 53 +++++++++++++++++++++++++++----
 2 files changed, 62 insertions(+), 8 deletions(-)

diff --git a/lib/diaspora/user/querying.rb b/lib/diaspora/user/querying.rb
index d6c3b7274..c9b76bf6e 100644
--- a/lib/diaspora/user/querying.rb
+++ b/lib/diaspora/user/querying.rb
@@ -23,8 +23,9 @@ module Diaspora
 
       def visible_shareable_ids(klass, opts={})
         opts = prep_opts(klass, opts)
+        cache = nil
 
-        if RedisCache.configured? && RedisCache.supported_order?(opts[:order_field]) && opts[:all_aspects?].present?
+        if use_cache?(opts)
           cache = RedisCache.new(self, opts[:order_field])
 
           cache.ensure_populated!(opts)
@@ -32,7 +33,7 @@ module Diaspora
           shareable_ids = cache.send(name+"_ids", opts[:max_time], opts[:limit])
         end
 
-        if shareable_ids.blank? || shareable_ids.length < opts[:limit]
+        if perform_db_query?(shareable_ids, cache, opts)
           visible_ids_from_sql(klass, opts)
         else
           shareable_ids
@@ -91,6 +92,7 @@ module Diaspora
         return nil unless person
         contact_for_person_id(person.id)
       end
+
       def aspects_with_shareable(base_class_name_or_class, shareable_id)
         base_class_name = base_class_name_or_class
         base_class_name = base_class_name_or_class.base_class.to_s if base_class_name_or_class.is_a?(Class)
@@ -156,6 +158,17 @@ module Diaspora
       end
 
       protected
+      # @return [Boolean]
+      def use_cache?(opts)
+        RedisCache.configured? && RedisCache.supported_order?(opts[:order_field]) && opts[:all_aspects?].present?
+      end
+
+      # @return [Boolean]
+      def perform_db_query?(shareable_ids, cache, opts)
+        return true if cache == nil
+        return false if cache.size <= opts[:limit]
+        shareable_ids.blank? || shareable_ids.length < opts[:limit]
+      end
 
       # @return [Hash]
       def prep_opts(klass, opts)
diff --git a/spec/models/user/querying_spec.rb b/spec/models/user/querying_spec.rb
index c028aa01d..94f60bcb1 100644
--- a/spec/models/user/querying_spec.rb
+++ b/spec/models/user/querying_spec.rb
@@ -91,7 +91,7 @@ describe User do
     context "RedisCache" do
       before do
         AppConfig[:redis_cache] = true
-        @opts = {:order => "created_at DESC", :all_aspects? => true}
+        @opts = {:order => "created_at DESC", :order_field => "created_at", :all_aspects? => true}
       end
 
       after do
@@ -99,7 +99,7 @@ describe User do
       end
 
       it "gets populated with latest 100 posts" do
-        cache = mock(:cache_exists? => true, :supported_order? => true, :ensure_populated! => mock, :post_ids => [])
+        cache = mock(:cache_exists? => true, :supported_order? => true, :size => 0, :ensure_populated! => mock, :post_ids => [])
         RedisCache.stub(:new).and_return(cache)
         @opts = alice.send(:prep_opts, Post, @opts)
         cache.should_receive(:ensure_populated!).with(hash_including(@opts))
@@ -118,9 +118,51 @@ describe User do
         it 're-populates the cache with the latest posts (in hashes)'
       end
 
+      describe '#use_cache?' do
+        before do
+          cache = mock(:cache_exists? => true, :supported_order? => true, :ensure_populated! => mock, :post_ids => [])
+          RedisCache.stub(:new).and_return(cache)
+        end
+
+        it 'returns true if redis cache is set' do
+          AppConfig[:redis_cache] = true
+          alice.send(:use_cache?, @opts).should be_true
+        end
+
+        it 'returns false if redis cache is set' do
+          AppConfig[:redis_cache] = nil
+          alice.send(:use_cache?, @opts).should be_false
+        end
+      end
+
+      describe '#perform_db_query?' do
+        before do
+          @opts = {:limit => 15}
+        end
+
+        it 'returns true if cache is nil' do
+          alice.send(:perform_db_query?, [1,2,3], nil, @opts).should be_true
+        end
+
+        it 'returns true if cache shareable_ids is blank' do
+          cache = mock(:size => 100)
+          alice.send(:perform_db_query?, [], cache, @opts).should be_true
+        end
+
+        it 'returns true if cache shareable_ids length is less than opts[:limit]' do
+          cache = mock(:size => 100)
+          alice.send(:perform_db_query?, [1,2,3], cache, @opts).should be_true
+        end
+
+        it 'returns false if cache size is less than opts[:limit]' do
+          cache = mock(:size => 10)
+          alice.send(:perform_db_query?, [1,2,3], cache, @opts).should be_false
+        end
+      end
+
       context 'populated cache' do
         before do
-          @cache = mock(:cache_exists? => true, :ensure_populated! => mock)
+          @cache = mock(:cache_exists? => true, :size => 100, :ensure_populated! => mock)
           RedisCache.stub(:new).and_return(@cache)
         end
 
@@ -137,12 +179,11 @@ describe User do
           alice.visible_shareable_ids(Post, @opts)
         end
 
-        it "does not get repopulated" do
-        end
+        it "does not get repopulated"
       end
     end
   end
-  
+
   describe "#prep_opts" do
     it "defaults the opts" do
       time = Time.now