Fix `tootctl feeds build` not building list timelines (#33783)
This commit is contained in:
		
							parent
							
								
									3f4f6317d4
								
							
						
					
					
						commit
						a3d2849d15
					
				| 
						 | 
					@ -301,6 +301,38 @@ class FeedManager
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  # Populate list feed of account from scratch
 | 
				
			||||||
 | 
					  # @param [List] list
 | 
				
			||||||
 | 
					  # @return [void]
 | 
				
			||||||
 | 
					  def populate_list(list)
 | 
				
			||||||
 | 
					    limit        = FeedManager::MAX_ITEMS / 2
 | 
				
			||||||
 | 
					    aggregate    = list.account.user&.aggregates_reblogs?
 | 
				
			||||||
 | 
					    timeline_key = key(:list, list.id)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    list.active_accounts.includes(:account_stat).reorder(nil).find_each do |target_account|
 | 
				
			||||||
 | 
					      if redis.zcard(timeline_key) >= limit
 | 
				
			||||||
 | 
					        oldest_home_score = redis.zrange(timeline_key, 0, 0, with_scores: true).first.last.to_i
 | 
				
			||||||
 | 
					        last_status_score = Mastodon::Snowflake.id_at(target_account.last_status_at)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        # If the feed is full and this account has not posted more recently
 | 
				
			||||||
 | 
					        # than the last item on the feed, then we can skip the whole account
 | 
				
			||||||
 | 
					        # because none of its statuses would stay on the feed anyway
 | 
				
			||||||
 | 
					        next if last_status_score < oldest_home_score
 | 
				
			||||||
 | 
					      end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      statuses = target_account.statuses.list_eligible_visibility.includes(:preloadable_poll, :media_attachments, :account, reblog: :account).limit(limit)
 | 
				
			||||||
 | 
					      crutches = build_crutches(list.account_id, statuses)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      statuses.each do |status|
 | 
				
			||||||
 | 
					        next if filter_from_home(status, list.account_id, crutches) || filter_from_list?(status, list)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        add_to_feed(:list, list.id, status, aggregate_reblogs: aggregate)
 | 
				
			||||||
 | 
					      end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      trim(:list, list.id)
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  # Completely clear multiple feeds at once
 | 
					  # Completely clear multiple feeds at once
 | 
				
			||||||
  # @param [Symbol] type
 | 
					  # @param [Symbol] type
 | 
				
			||||||
  # @param [Array<Integer>] ids
 | 
					  # @param [Array<Integer>] ids
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -24,6 +24,7 @@ class List < ApplicationRecord
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  has_many :list_accounts, inverse_of: :list, dependent: :destroy
 | 
					  has_many :list_accounts, inverse_of: :list, dependent: :destroy
 | 
				
			||||||
  has_many :accounts, through: :list_accounts
 | 
					  has_many :accounts, through: :list_accounts
 | 
				
			||||||
 | 
					  has_many :active_accounts, -> { merge(ListAccount.active) }, through: :list_accounts, source: :account
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  validates :title, presence: true
 | 
					  validates :title, presence: true
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -20,6 +20,8 @@ class ListAccount < ApplicationRecord
 | 
				
			||||||
  validates :account_id, uniqueness: { scope: :list_id }
 | 
					  validates :account_id, uniqueness: { scope: :list_id }
 | 
				
			||||||
  validate :validate_relationship
 | 
					  validate :validate_relationship
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  scope :active, -> { where.not(follow_id: nil) }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  before_validation :set_follow, unless: :list_owner_account_is_account?
 | 
					  before_validation :set_follow, unless: :list_owner_account_is_account?
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  private
 | 
					  private
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -5,6 +5,10 @@ class PrecomputeFeedService < BaseService
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def call(account)
 | 
					  def call(account)
 | 
				
			||||||
    FeedManager.instance.populate_home(account)
 | 
					    FeedManager.instance.populate_home(account)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    account.owned_lists.each do |list|
 | 
				
			||||||
 | 
					      FeedManager.instance.populate_list(list)
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
  ensure
 | 
					  ensure
 | 
				
			||||||
    redis.del("account:#{account.id}:regeneration")
 | 
					    redis.del("account:#{account.id}:regeneration")
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -7,31 +7,69 @@ RSpec.describe PrecomputeFeedService do
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  describe 'call' do
 | 
					  describe 'call' do
 | 
				
			||||||
    let(:account) { Fabricate(:account) }
 | 
					    let(:account) { Fabricate(:account) }
 | 
				
			||||||
 | 
					    let!(:list) { Fabricate(:list, account: account, exclusive: false) }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    it 'fills a user timeline with statuses' do
 | 
					    context 'when no eligible status exist' do
 | 
				
			||||||
      account = Fabricate(:account)
 | 
					      it 'raises no error and results in an empty timeline' do
 | 
				
			||||||
      status = Fabricate(:status, account: account)
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
      subject.call(account)
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
      expect(redis.zscore(FeedManager.instance.key(:home, account.id), status.id)).to be_within(0.1).of(status.id.to_f)
 | 
					 | 
				
			||||||
    end
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
    it 'does not raise an error even if it could not find any status' do
 | 
					 | 
				
			||||||
      account = Fabricate(:account)
 | 
					 | 
				
			||||||
        expect { subject.call(account) }.to_not raise_error
 | 
					        expect { subject.call(account) }.to_not raise_error
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        expect(redis.zcard(FeedManager.instance.key(:home, account.id))).to eq(0)
 | 
				
			||||||
 | 
					      end
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    it 'filters statuses' do
 | 
					    context 'with eligible statuses' do
 | 
				
			||||||
      account = Fabricate(:account)
 | 
					      let(:muted_account) { Fabricate(:account) }
 | 
				
			||||||
      muted_account = Fabricate(:account)
 | 
					      let!(:followed_account) { Fabricate(:account) }
 | 
				
			||||||
      Fabricate(:mute, account: account, target_account: muted_account)
 | 
					      let!(:requested_account) { Fabricate(:account) }
 | 
				
			||||||
      reblog = Fabricate(:status, account: muted_account)
 | 
					      let!(:own_status) { Fabricate(:status, account: account) }
 | 
				
			||||||
      Fabricate(:status, account: account, reblog: reblog)
 | 
					      let!(:followed_status) { Fabricate(:status, account: followed_account) }
 | 
				
			||||||
 | 
					      let!(:unreadable_dm_from_followed) { Fabricate(:status, account: followed_account, visibility: :direct) }
 | 
				
			||||||
 | 
					      let!(:requested_status) { Fabricate(:status, account: requested_account) }
 | 
				
			||||||
 | 
					      let!(:muted_status) { Fabricate(:status, account: muted_account) }
 | 
				
			||||||
 | 
					      let!(:muted_reblog) { Fabricate(:status, account: followed_account, reblog: muted_status) }
 | 
				
			||||||
 | 
					      let!(:known_reply) { Fabricate(:status, account: followed_account, in_reply_to_id: own_status.id) }
 | 
				
			||||||
 | 
					      let!(:unknown_reply) { Fabricate(:status, account: followed_account, in_reply_to_id: requested_status.id) }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      before do
 | 
				
			||||||
 | 
					        account.follow!(followed_account)
 | 
				
			||||||
 | 
					        account.request_follow!(requested_account)
 | 
				
			||||||
 | 
					        account.mute!(muted_account)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        AddAccountsToListService.new.call(list, [followed_account])
 | 
				
			||||||
 | 
					      end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      it "fills a user's home and list timelines with the expected posts" do
 | 
				
			||||||
        subject.call(account)
 | 
					        subject.call(account)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
      expect(redis.zscore(FeedManager.instance.key(:home, account.id), reblog.id)).to be_nil
 | 
					        home_timeline_ids = redis.zrevrangebyscore(FeedManager.instance.key(:home, account.id), '(+inf', '(-inf', limit: [0, 30], with_scores: true).map { |id| id.first.to_i }
 | 
				
			||||||
 | 
					        list_timeline_ids = redis.zrevrangebyscore(FeedManager.instance.key(:list, list.id), '(+inf', '(-inf', limit: [0, 30], with_scores: true).map { |id| id.first.to_i }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        expect(home_timeline_ids).to include(
 | 
				
			||||||
 | 
					          own_status.id,
 | 
				
			||||||
 | 
					          followed_status.id,
 | 
				
			||||||
 | 
					          known_reply.id
 | 
				
			||||||
 | 
					        )
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        expect(list_timeline_ids).to include(
 | 
				
			||||||
 | 
					          followed_status.id
 | 
				
			||||||
 | 
					        )
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        expect(home_timeline_ids).to_not include(
 | 
				
			||||||
 | 
					          requested_status.id,
 | 
				
			||||||
 | 
					          unknown_reply.id,
 | 
				
			||||||
 | 
					          unreadable_dm_from_followed.id,
 | 
				
			||||||
 | 
					          muted_status.id,
 | 
				
			||||||
 | 
					          muted_reblog.id
 | 
				
			||||||
 | 
					        )
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        expect(list_timeline_ids).to_not include(
 | 
				
			||||||
 | 
					          requested_status.id,
 | 
				
			||||||
 | 
					          unknown_reply.id,
 | 
				
			||||||
 | 
					          unreadable_dm_from_followed.id,
 | 
				
			||||||
 | 
					          muted_status.id,
 | 
				
			||||||
 | 
					          muted_reblog.id
 | 
				
			||||||
 | 
					        )
 | 
				
			||||||
 | 
					      end
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
end
 | 
					end
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
		Reference in New Issue