diff --git a/app/controllers/api/v1/timelines/home_controller.rb b/app/controllers/api/v1/timelines/home_controller.rb index a07faae7208..6b6fd6a02fe 100644 --- a/app/controllers/api/v1/timelines/home_controller.rb +++ b/app/controllers/api/v1/timelines/home_controller.rb @@ -42,7 +42,7 @@ class Api::V1::Timelines::HomeController < Api::V1::Timelines::BaseController end def account_home_feed - HomeFeed.new(current_account) + HomeFeed.new(current_account, params.permit(:exclude_direct, :exclude_reblogs, :exclude_quotes, :exclude_replies)) end def next_path diff --git a/app/models/feed.rb b/app/models/feed.rb index 27e12809947..16d1b104fb5 100644 --- a/app/models/feed.rb +++ b/app/models/feed.rb @@ -3,9 +3,10 @@ class Feed include Redisable - def initialize(type, id) + def initialize(type, id, options = {}) @type = type - @id = id + @id = id + @options = options end def get(limit, max_id = nil, since_id = nil, min_id = nil) @@ -20,15 +21,32 @@ class Feed protected def from_redis(limit, max_id, since_id, min_id) + scope = Status.all + + # Apply specified filters + scope.merge!(Status.where.not(visibility: :direct)) if @options[:exclude_direct] + scope.merge!(Status.where(reblog_of_id: nil)) if @options[:exclude_reblogs] + scope.merge!(Status.where.missing(:quote)) if @options[:exclude_quotes] + scope.merge!(Status.where(in_reply_to_id: nil).or(Status.where(in_reply_to_id: @id))) if @options[:exclude_replies] # TODO: beware + + # If we have no filter, rely on Redis to apply the limit, otherwise we will have to do a posteriori filtering + filters_present = @options[:exclude_direct] || @options[:exclude_reblogs] || @options[:exclude_quotes] || @options[:exclude_replies] + limit_clause = [0, limit] unless filters_present + max_id = '+inf' if max_id.blank? if min_id.blank? - since_id = '-inf' if since_id.blank? - unhydrated = redis.zrevrangebyscore(key, "(#{max_id}", "(#{since_id}", limit: [0, limit], with_scores: true).map { |id| id.first.to_i } + since_id = '-inf' if since_id.blank? + ids = redis.zrevrangebyscore(key, "(#{max_id}", "(#{since_id}", limit: limit_clause, with_scores: true).map { |id| id.first.to_i } else - unhydrated = redis.zrangebyscore(key, "(#{min_id}", "(#{max_id}", limit: [0, limit], with_scores: true).map { |id| id.first.to_i } + ids = redis.zrangebyscore(key, "(#{min_id}", "(#{max_id}", limit: limit_clause, with_scores: true).map { |id| id.first.to_i } end - Status.where(id: unhydrated) + if min_id.blank? || !filters_present + scope.where(id: ids).limit(limit) + else + # We need to do some filtering *and* do it in the correct order + Status.where(id: scope.reorder(id: :asc).where(id: ids).limit(limit)) + end end def key diff --git a/app/models/home_feed.rb b/app/models/home_feed.rb index 8962a99e32a..9e7e24951e4 100644 --- a/app/models/home_feed.rb +++ b/app/models/home_feed.rb @@ -1,9 +1,9 @@ # frozen_string_literal: true class HomeFeed < Feed - def initialize(account) + def initialize(account, options = {}) @account = account - super(:home, account.id) + super(:home, account.id, options) end def async_refresh diff --git a/spec/models/home_feed_spec.rb b/spec/models/home_feed_spec.rb index 882f84c8a70..18973135ab7 100644 --- a/spec/models/home_feed_spec.rb +++ b/spec/models/home_feed_spec.rb @@ -10,8 +10,9 @@ RSpec.describe HomeFeed do describe '#get' do before do Fabricate(:status, account: account, id: 1) - Fabricate(:status, account: account, id: 2) - Fabricate(:status, account: account, id: 3) + Fabricate(:status, account: account, id: 2, reblog: Fabricate(:status)) + Fabricate(:status, account: account, id: 3, visibility: :direct) + Fabricate(:status, account: account, id: 4) Fabricate(:status, account: account, id: 10) end @@ -23,10 +24,17 @@ RSpec.describe HomeFeed do ) end - it 'gets statuses with ids in the range from redis' do - results = subject.get(3) + it 'gets statuses with ids in the range from redis according to the given parameters' do + expect(described_class.new(account).get(3).map(&:id)).to eq [4, 3, 2] + expect(described_class.new(account, { exclude_direct: true }).get(3).map(&:id)).to eq [4, 2, 1] + expect(described_class.new(account, { exclude_reblogs: true }).get(3).map(&:id)).to eq [4, 3, 1] + expect(described_class.new(account, { exclude_direct: true, exclude_reblogs: true }).get(3).map(&:id)).to eq [4, 1] - expect(results.map(&:id)).to eq [3, 2] + expect(described_class.new(account).get(2, nil, nil, 0).map(&:id)).to eq [2, 1] + expect(described_class.new(account, { exclude_direct: true }).get(2, nil, nil, 0).map(&:id)).to eq [2, 1] + expect(described_class.new(account, { exclude_direct: true }).get(2, nil, nil, 1).map(&:id)).to eq [4, 2] + expect(described_class.new(account, { exclude_reblogs: true }).get(2, nil, nil, 0).map(&:id)).to eq [3, 1] + expect(described_class.new(account, { exclude_direct: true, exclude_reblogs: true }).get(2, nil, nil, 0).map(&:id)).to eq [4, 1] end end