diff --git a/Gemfile b/Gemfile index 89d4d823b..ad45991dc 100644 --- a/Gemfile +++ b/Gemfile @@ -126,6 +126,7 @@ group :test do gem "selenium-webdriver", "~> 2.7.0" gem 'webmock', :require => false gem 'sqlite3' + gem 'mock_redis' end group :development do diff --git a/Gemfile.lock b/Gemfile.lock index c048854c2..b09c94341 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -256,6 +256,7 @@ GEM mobile-fu (0.2.1) rack-mobile-detect rails + mock_redis (0.2.0) moneta (0.6.0) mongrel (1.1.5) cgi_multipart_eof_fix (>= 2.4) @@ -506,6 +507,7 @@ DEPENDENCIES linecache (= 0.43) mini_magick (= 3.2) mobile-fu + mock_redis mongrel mysql2 (= 0.2.13) newrelic_rpm diff --git a/lib/diaspora/redis_cache.rb b/lib/diaspora/redis_cache.rb new file mode 100644 index 000000000..b5b8d7353 --- /dev/null +++ b/lib/diaspora/redis_cache.rb @@ -0,0 +1,33 @@ +# Copyright (c) 2010-2011, Diaspora Inc. This file is +# licensed under the Affero General Public License version 3 or later. See +# the COPYRIGHT file. +# + +class RedisCache + def initialize(user_id, order) + @user_id = user_id + @order = order + self + end + + # @return [Boolean] + def cache_exists? + redis.zcard(set_key) != 0 + end + + def post_ids(time=Time.now, limit=15) + post_ids = redis.zrevrangebyscore(set_key, time.to_i, "-inf") + post_ids[0...limit] + end + + protected + # @return [Redis] + def redis + @redis ||= Redis.new + end + + # @return [String] + def set_key + @set_key ||= "cache_stream_#{@user_id}_#{@order}" + end +end diff --git a/lib/diaspora/user/querying.rb b/lib/diaspora/user/querying.rb index 0cc63f481..437bc718d 100644 --- a/lib/diaspora/user/querying.rb +++ b/lib/diaspora/user/querying.rb @@ -2,6 +2,8 @@ # licensed under the Affero General Public License version 3 or later. See # the COPYRIGHT file. +require 'lib/diaspora/redis_cache' + module Diaspora module UserModules module Querying @@ -13,21 +15,32 @@ module Diaspora post ||= Post.where(key => id, :public => true).where(opts).first end - def visible_posts(opts = {}) - defaults = { - :type => ['StatusMessage', 'Photo'], - :order => 'updated_at DESC', - :limit => 15, - :hidden => false - } - opts = defaults.merge(opts) + def visible_posts(opts={}) + opts = prep_opts(opts) + post_ids = visible_post_ids(opts) + Post.where(:id => post_ids).select('DISTINCT posts.*').limit(opts[:limit]).order(opts[:order_with_table]) + end - order_field = opts[:order].split.first.to_sym - order_with_table = 'posts.' + opts[:order] + def visible_post_ids(opts={}) + opts = prep_opts(opts) - opts[:max_time] = Time.at(opts[:max_time]) if opts[:max_time].is_a?(Integer) - opts[:max_time] ||= Time.now + 1 + 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 + end + if post_ids.blank? || post_ids.length < opts[:limit] + visible_ids_from_sql(opts) + else + post_ids + end + end + + # @return [Array] + def visible_ids_from_sql(opts={}) + opts = prep_opts(opts) select_clause ='DISTINCT posts.id, posts.updated_at AS updated_at, posts.created_at AS created_at' posts_from_others = Post.joins(:contacts).where( :pending => false, :type => opts[:type], :post_visibilities => {:hidden => opts[:hidden]}, :contacts => {:user_id => self.id}) @@ -39,20 +52,12 @@ module Diaspora posts_from_self = posts_from_self.joins(:aspect_visibilities).where(:aspect_visibilities => {:aspect_id => opts[:by_members_of]}) end - unless sqlite? - posts_from_others = posts_from_others.select(select_clause).order(order_with_table).where(Post.arel_table[order_field].lt(opts[:max_time])) - posts_from_self = posts_from_self.select(select_clause).order(order_with_table).where(Post.arel_table[order_field].lt(opts[:max_time])) + 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]}" - else - posts_from_others = posts_from_others.select(select_clause) - posts_from_self = posts_from_self.select(select_clause) - all_posts = "#{posts_from_others.to_sql} UNION ALL #{posts_from_self.to_sql} ORDER BY #{opts[:order]} LIMIT #{opts[:limit]}" - end + 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_ids = Post.connection.select_values(all_posts) - - Post.where(:id => post_ids).select('DISTINCT posts.*').limit(opts[:limit]).order(order_with_table) + Post.connection.select_values(all_posts) end def contact_for(person) @@ -112,6 +117,26 @@ module Diaspora Post.where(:id => post_ids, :pending => false).select('DISTINCT posts.*').order("posts.created_at DESC") end + + protected + + # @return [Hash] + def prep_opts(opts) + defaults = { + :type => ['StatusMessage', 'Photo'], + :order => 'updated_at DESC', + :limit => 15, + :hidden => false + } + opts = defaults.merge(opts) + + opts[:order_field] = opts[:order].split.first.to_sym + opts[:order_with_table] = 'posts.' + opts[:order] + + opts[:max_time] = Time.at(opts[:max_time]) if opts[:max_time].is_a?(Integer) + opts[:max_time] ||= Time.now + 1 + opts + end end end end diff --git a/spec/lib/diaspora/redis_cache_spec.rb b/spec/lib/diaspora/redis_cache_spec.rb new file mode 100644 index 000000000..6e5ed758d --- /dev/null +++ b/spec/lib/diaspora/redis_cache_spec.rb @@ -0,0 +1,62 @@ +# Copyright (c) 2010-2011, Diaspora Inc. This file is +# licensed under the Affero General Public License version 3 or later. See +# 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) + 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| + cache.instance_variable_get(var).should_not be_blank + end + end + + 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") + + @cache.cache_exists?.should be_true + end + + it 'returns false if there is nothing in the set' do + @cache.cache_exists?.should be_false + end + end + + describe "#post_ids" do + before 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 + end + + it 'returns the most recent post ids (default created at, limit 15)' do + @cache.post_ids.should =~ 15.times.map {|n| n} + end + + it 'returns posts ids after the specified time' do + @cache.post_ids(@timestamps[15]).should =~ (15...30).map {|n| n} + end + + it 'returns post ids with a non-default limit' do + @cache.post_ids(@timestamp, 20).should =~ 20.times.map {|n| n} + end + end + + describe "#populate" + describe "#add" + describe "#remove" +end diff --git a/spec/models/user/querying_spec.rb b/spec/models/user/querying_spec.rb index e62110531..7ad9edd60 100644 --- a/spec/models/user/querying_spec.rb +++ b/spec/models/user/querying_spec.rb @@ -12,49 +12,49 @@ describe User do @bobs_aspect = bob.aspects.where(:name => "generic").first end - describe "#visible_posts" do + describe "#visible_post_ids" do it "contains your public posts" do public_post = alice.post(:status_message, :text => "hi", :to => @alices_aspect.id, :public => true) - alice.visible_posts.should include(public_post) + alice.visible_post_ids.should include(public_post.id) end it "contains your non-public posts" do private_post = alice.post(:status_message, :text => "hi", :to => @alices_aspect.id, :public => false) - alice.visible_posts.should include(private_post) + alice.visible_post_ids.should include(private_post.id) end it "contains public posts from people you're following" do dogs = bob.aspects.create(:name => "dogs") bobs_public_post = bob.post(:status_message, :text => "hello", :public => true, :to => dogs.id) - alice.visible_posts.should include(bobs_public_post) + alice.visible_post_ids.should include(bobs_public_post.id) end it "contains non-public posts from people who are following you" do bobs_post = bob.post(:status_message, :text => "hello", :to => @bobs_aspect.id) - alice.visible_posts.should include(bobs_post) + alice.visible_post_ids.should include(bobs_post.id) end it "does not contain non-public posts from aspects you're not in" do dogs = bob.aspects.create(:name => "dogs") invisible_post = bob.post(:status_message, :text => "foobar", :to => dogs.id) - alice.visible_posts.should_not include(invisible_post) + alice.visible_post_ids.should_not include(invisible_post.id) end it "does not contain pending posts" do pending_post = bob.post(:status_message, :text => "hey", :public => true, :to => @bobs_aspect.id, :pending => true) pending_post.should be_pending - alice.visible_posts.should_not include pending_post + alice.visible_post_ids.should_not include pending_post.id end it "does not contain pending photos" do pending_photo = bob.post(:photo, :pending => true, :user_file=> File.open(photo_fixture_name), :to => @bobs_aspect) - alice.visible_posts.should_not include pending_photo + alice.visible_post_ids.should_not include pending_photo.id end it "respects the :type option" do photo = bob.post(:photo, :pending => false, :user_file=> File.open(photo_fixture_name), :to => @bobs_aspect) - alice.visible_posts.should include(photo) - alice.visible_posts(:type => 'StatusMessage').should_not include(photo) + alice.visible_post_ids.should include(photo.id) + alice.visible_post_ids(:type => 'StatusMessage').should_not include(photo.id) end it "does not contain duplicate posts" do @@ -64,8 +64,8 @@ describe User do bobs_post = bob.post(:status_message, :text => "hai to all my people", :to => [@bobs_aspect.id, bobs_other_aspect.id]) - alice.visible_posts.length.should == 1 - alice.visible_posts.should include(bobs_post) + alice.visible_post_ids.length.should == 1 + alice.visible_post_ids.should include(bobs_post.id) end describe 'hidden posts' do @@ -76,15 +76,58 @@ describe User do end it "pulls back non hidden posts" do - alice.visible_posts.include?(@status).should be_true + alice.visible_post_ids.include?(@status.id).should be_true end it "does not pull back hidden posts" do @vis.update_attributes(:hidden => true) - alice.visible_posts.include?(@status).should be_false + alice.visible_post_ids.include?(@status.id).should be_false end end + context "RedisCache" do + before do + AppConfig[:redis_cache] = true + 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) + + alice.visible_post_ids + end + + it "is populated" do + end + end + + context 'populated cache' do + before do + @cache = mock(:cache_exists? => true) + 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] + 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 + end + + it "does not get repopulated" do + end + end + end + end + + describe "#visible_posts" do context 'with many posts' do before do bob.move_contact(eve.person, @bobs_aspect, bob.aspects.create(:name => 'new aspect')) @@ -101,6 +144,7 @@ describe User do end end end + it 'works' do # The set up takes a looong time, so to save time we do several tests in one bob.visible_posts.length.should == 15 #it returns 15 by default bob.visible_posts.should == bob.visible_posts(:by_members_of => bob.aspects.map { |a| a.id }) # it is the same when joining through aspects