WIP trim is still needed, also possible weirdness with the mock

lib/diaspora/redis_cache.rb

@@ -4,15 +4,24 @@
 #
 
 class RedisCache
-  def initialize(user_id, order)
-    @user_id = user_id
-    @order = order
+
+  SUPPORTED_CACHES = [:created_at] #['updated_at', 
+  CACHE_LIMIT = 100
+
+  def initialize(user, order_field)
+    @user = user
+    @order_field = order_field.to_s
     self
   end
 
   # @return [Boolean]
   def cache_exists?
-    redis.zcard(set_key) != 0
+    self.size != 0
+  end
+
+  # @return [Integer] the cardinality of the redis set
+  def size
+    redis.zcard(set_key)
   end
 
   def post_ids(time=Time.now, limit=15)
@@ -20,6 +29,52 @@ class RedisCache
     post_ids[0...limit]
   end
 
+  # @return [RedisCache] self
+  def ensure_populated!
+    self.repopulate! unless cache_exists?
+    self
+  end
+
+  # @return [RedisCache] self
+  def repopulate!
+    self.populate! && self.trim!
+    self
+  end
+
+  # @return [RedisCache] self
+  def populate!
+    # user executes query and gets back hashes
+    sql = @user.visible_posts_sql(:limit => CACHE_LIMIT, :order => self.order)
+    hashes = Post.connection.select_all(sql)
+
+    # hashes are inserted into set in a single transaction
+    redis.multi do
+      hashes.each do |h|
+        self.redis.zadd(set_key, h[@order_field], h["id"])
+      end
+    end
+
+    self
+  end
+
+  # @return [RedisCache] self
+  def trim!
+    puts "cache limit #{CACHE_LIMIT}"
+    puts "cache size #{self.size}"
+    self.redis.zremrangebyrank(set_key, 0, -(CACHE_LIMIT+1))
+    self
+  end
+
+  # @param order [Symbol, String]
+  # @return [Boolean]
+  def self.supported_order?(order)
+    SUPPORTED_CACHES.include?(order.to_sym)
+  end
+
+  def order
+    "#{@order_field} DESC"
+  end
+
   protected
   # @return [Redis]
   def redis
@@ -28,6 +83,6 @@ class RedisCache
 
   # @return [String]
   def set_key
-    @set_key ||= "cache_stream_#{@user_id}_#{@order}"
+    @set_key ||= "cache_stream_#{@user.id}_#{@order_field}"
   end
 end

lib/diaspora/user/querying.rb

@@ -24,11 +24,11 @@ module Diaspora
       def visible_post_ids(opts={})
         opts = prep_opts(opts)
 
-        if AppConfig[:redis_cache]
-          cache = RedisCache.new(self.id, opts[:order_field])
-          if cache.cache_exists?
-            post_ids = cache.post_ids(opts[:max_time], opts[:limit])
-          end
+        if AppConfig[:redis_cache] && RedisCache.supported_order?(opts[:order_field])
+          cache = RedisCache.new(self, opts[:order_field])
+
+          cache.ensure_populated!
+          post_ids = cache.post_ids(opts[:max_time], opts[:limit])
         end
 
         if post_ids.blank? || post_ids.length < opts[:limit]
@@ -40,6 +40,11 @@ module Diaspora
 
       # @return [Array<Integer>]
       def visible_ids_from_sql(opts={})
+        opts = prep_opts(opts)
+        Post.connection.select_values(visible_posts_sql(opts))
+      end
+
+      def visible_posts_sql(opts={})
         opts = prep_opts(opts)
         select_clause ='DISTINCT posts.id, posts.updated_at AS updated_at, posts.created_at AS created_at'
 
@@ -55,9 +60,7 @@ module Diaspora
         posts_from_others = posts_from_others.select(select_clause).order(opts[:order_with_table]).where(Post.arel_table[opts[:order_field]].lt(opts[:max_time]))
         posts_from_self = posts_from_self.select(select_clause).order(opts[:order_with_table]).where(Post.arel_table[opts[:order_field]].lt(opts[:max_time]))
 
-        all_posts = "(#{posts_from_others.to_sql} LIMIT #{opts[:limit]}) UNION ALL (#{posts_from_self.to_sql} LIMIT #{opts[:limit]}) ORDER BY #{opts[:order]} LIMIT #{opts[:limit]}"
-
-        Post.connection.select_values(all_posts)
+        "(#{posts_from_others.to_sql} LIMIT #{opts[:limit]}) UNION ALL (#{posts_from_self.to_sql} LIMIT #{opts[:limit]}) ORDER BY #{opts[:order]} LIMIT #{opts[:limit]}"
       end
 
       def contact_for(person)

spec/lib/diaspora/redis_cache_spec.rb

@@ -3,18 +3,20 @@
 #   the COPYRIGHT file.
 
 require 'spec_helper'
-require 'diaspora/redis_cache'
 
 describe RedisCache do
   before do
-    @redis = MockRedis.new
-    @cache = RedisCache.new(bob.id, "created_at")
-    @cache.stub(:redis).and_return(@redis)
+    #@redis = MockRedis.new
+    @redis = Redis.new
+    @redis.keys.each{|p| @redis.del(p)}
+
+    @cache = RedisCache.new(bob, :created_at)
+    #@cache.stub(:redis).and_return(@redis)
   end
 
-  it 'gets initialized with user_id and an order field' do
-    cache = RedisCache.new(bob.id, "updated_at")
-    [:@user_id, :@order].each do |var|
+  it 'gets initialized with user and an created_at order' do
+    cache = RedisCache.new(bob, :created_at)
+    [:@user, :@order_field].each do |var|
       cache.instance_variable_get(var).should_not be_blank
     end
   end
@@ -22,7 +24,7 @@ describe RedisCache do
   describe "#cache_exists?" do
     it 'returns true if the sorted set exists' do
       timestamp = Time.now.to_i
-      @redis.zadd("cache_stream_#{@bob.id}_created_at", timestamp, "post_1")
+      @redis.zadd("cache_stream_#{bob.id}_created_at", timestamp, "post_1")
 
       @cache.cache_exists?.should be_true
     end
@@ -38,7 +40,7 @@ describe RedisCache do
       @timestamp = Time.now.to_i
       30.times do |n|
         created_time = @timestamp - n*1000
-        @redis.zadd("cache_stream_#{@bob.id}_created_at", created_time, n)
+        @redis.zadd("cache_stream_#{bob.id}_created_at", created_time, n)
         @timestamps << created_time
       end
     end
@@ -56,7 +58,84 @@ describe RedisCache do
     end
   end
 
-  describe "#populate"
+  describe "#ensure_populated!" do
+    it 'does nothing if the cache is populated' do
+      @cache.stub(:cache_exists?).and_return(true)
+      @cache.should_not_receive(:repopulate!)
+
+      @cache.ensure_populated!
+    end
+
+    it 'clears and poplulates if the cache is not populated' do
+      @cache.stub(:cache_exists?).and_return(false)
+      @cache.should_receive(:repopulate!)
+
+      @cache.ensure_populated!
+    end
+  end
+
+  describe "#repopulate!" do
+    it 'populates' do
+      @cache.stub(:trim!).and_return(true)
+      @cache.should_receive(:populate!).and_return(true)
+      @cache.repopulate!
+    end
+
+    it 'trims' do
+      @cache.stub(:populate!).and_return(true)
+      @cache.should_receive(:trim!)
+      @cache.repopulate!
+    end
+  end
+
+  describe "#populate!" do
+    it 'queries the db with the visible post sql string' do
+      sql = "long_sql"
+      order = "created_at DESC"
+      @cache.should_receive(:order).and_return(order)
+      bob.should_receive(:visible_posts_sql).with(hash_including(
+                                                    :limit => 100,
+                                                    :order => order)).
+                                             and_return(sql)
+
+      Post.connection.should_receive(:select_all).with(sql).and_return([])
+
+      @cache.populate!
+    end
+
+    it 'adds the post from the hash to the cache'
+  end
+
+  describe "#trim!" do
+    it 'does nothing if the set is smaller than the cache limit' do
+      @timestamps = []
+      @timestamp = Time.now.to_i
+      30.times do |n|
+        created_time = @timestamp - n*1000
+        @redis.zadd("cache_stream_#{bob.id}_created_at", created_time, n)
+        @timestamps << created_time
+      end
+
+      post_ids = @cache.post_ids(Time.now.to_i, @cache.size)
+      @cache.trim!
+      @cache.post_ids(Time.now.to_i, @cache.size).should == post_ids
+    end
+
+    it 'trims the set to the cache limit' do
+      @timestamps = []
+      @timestamp = Time.now.to_i
+      120.times do |n|
+        created_time = @timestamp - n*1000
+        @redis.zadd("cache_stream_#{bob.id}_created_at", created_time, n)
+        @timestamps << created_time
+      end
+
+      post_ids = 100.times.map{|n| n}
+      @cache.trim!
+      @cache.post_ids(Time.now.to_i, @cache.size).should == post_ids[0...100]
+    end
+  end
+
   describe "#add"
   describe "#remove"
 end

spec/models/user/querying_spec.rb

@@ -88,37 +88,43 @@ describe User do
     context "RedisCache" do
       before do
         AppConfig[:redis_cache] = true
+        @opts = {:order => "created_at DESC"}
       end
 
-      context 'empty cache' do
-        it "does not read from the cache" do
-          cache = mock(:cache_exists? => false)
-          RedisCache.stub(:new).and_return(cache)
-          cache.should_not_receive(:post_ids)
+      after do
+        AppConfig[:redis_cache] = nil
+      end
 
-          alice.visible_post_ids
-        end
+      it "gets populated with latest 100 posts" do
+        cache = mock(:cache_exists? => true, :ensure_populated! => mock, :post_ids => [])
+        RedisCache.stub(:new).and_return(cache)
+        cache.should_receive(:ensure_populated!)
 
-        it "is populated" do
-        end
+        alice.visible_post_ids(@opts)
+      end
+
+      describe "#ensure_populated_cache" do
+        it 'does nothing if the cache is already populated'
+        it 're-populates the cache with the latest posts (in hashes)'
       end
 
       context 'populated cache' do
         before do
-          @cache = mock(:cache_exists? => true)
+          @cache = mock(:cache_exists? => true, :ensure_populated! => mock)
           RedisCache.stub(:new).and_return(@cache)
         end
-        it "reads from the cache" do
-          @cache.stub(:post_ids).and_return([1,2,3])
 
-          alice.visible_post_ids(:limit => 3).should == [1,2,3]
+        it "reads from the cache" do
+          @cache.should_receive(:post_ids).and_return([1,2,3])
+
+          alice.visible_post_ids(@opts.merge({:limit => 3})).should == [1,2,3]
         end
 
         it "queries if maxtime is later than the last cached post" do
           @cache.stub(:post_ids).and_return([])
           alice.should_receive(:visible_ids_from_sql)
 
-          alice.visible_post_ids
+          alice.visible_post_ids(@opts)
         end
 
         it "does not get repopulated" do