From a3d2849d151c6158f9fc876c57567a7b5378cfe6 Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 30 Jan 2025 10:18:46 +0100 Subject: [PATCH] Fix `tootctl feeds build` not building list timelines (#33783) --- app/lib/feed_manager.rb | 32 ++++++++ app/models/list.rb | 1 + app/models/list_account.rb | 2 + app/services/precompute_feed_service.rb | 4 + spec/services/precompute_feed_service_spec.rb | 74 ++++++++++++++----- 5 files changed, 95 insertions(+), 18 deletions(-) diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index 8a0043268b..30ec11c2d9 100644 --- a/app/lib/feed_manager.rb +++ b/app/lib/feed_manager.rb @@ -301,6 +301,38 @@ class FeedManager 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 # @param [Symbol] type # @param [Array] ids diff --git a/app/models/list.rb b/app/models/list.rb index bb7dd4cfc0..cd01774539 100644 --- a/app/models/list.rb +++ b/app/models/list.rb @@ -24,6 +24,7 @@ class List < ApplicationRecord has_many :list_accounts, inverse_of: :list, dependent: :destroy has_many :accounts, through: :list_accounts + has_many :active_accounts, -> { merge(ListAccount.active) }, through: :list_accounts, source: :account validates :title, presence: true diff --git a/app/models/list_account.rb b/app/models/list_account.rb index cfbcdbf0d5..00ecd44c3c 100644 --- a/app/models/list_account.rb +++ b/app/models/list_account.rb @@ -20,6 +20,8 @@ class ListAccount < ApplicationRecord validates :account_id, uniqueness: { scope: :list_id } validate :validate_relationship + scope :active, -> { where.not(follow_id: nil) } + before_validation :set_follow, unless: :list_owner_account_is_account? private diff --git a/app/services/precompute_feed_service.rb b/app/services/precompute_feed_service.rb index f813f06b20..86aad50983 100644 --- a/app/services/precompute_feed_service.rb +++ b/app/services/precompute_feed_service.rb @@ -5,6 +5,10 @@ class PrecomputeFeedService < BaseService def call(account) FeedManager.instance.populate_home(account) + + account.owned_lists.each do |list| + FeedManager.instance.populate_list(list) + end ensure redis.del("account:#{account.id}:regeneration") end diff --git a/spec/services/precompute_feed_service_spec.rb b/spec/services/precompute_feed_service_spec.rb index 9b2c6c280f..8019ffadda 100644 --- a/spec/services/precompute_feed_service_spec.rb +++ b/spec/services/precompute_feed_service_spec.rb @@ -7,31 +7,69 @@ RSpec.describe PrecomputeFeedService do describe 'call' do let(:account) { Fabricate(:account) } + let!(:list) { Fabricate(:list, account: account, exclusive: false) } - it 'fills a user timeline with statuses' do - account = Fabricate(:account) - status = Fabricate(:status, account: account) + context 'when no eligible status exist' do + it 'raises no error and results in an empty timeline' do + expect { subject.call(account) }.to_not raise_error - subject.call(account) - - expect(redis.zscore(FeedManager.instance.key(:home, account.id), status.id)).to be_within(0.1).of(status.id.to_f) + expect(redis.zcard(FeedManager.instance.key(:home, account.id))).to eq(0) + end 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 - end + context 'with eligible statuses' do + let(:muted_account) { Fabricate(:account) } + let!(:followed_account) { Fabricate(:account) } + let!(:requested_account) { Fabricate(:account) } + let!(:own_status) { Fabricate(:status, account: account) } + 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) } - it 'filters statuses' do - account = Fabricate(:account) - muted_account = Fabricate(:account) - Fabricate(:mute, account: account, target_account: muted_account) - reblog = Fabricate(:status, account: muted_account) - Fabricate(:status, account: account, reblog: reblog) + before do + account.follow!(followed_account) + account.request_follow!(requested_account) + account.mute!(muted_account) - subject.call(account) + AddAccountsToListService.new.call(list, [followed_account]) + end - expect(redis.zscore(FeedManager.instance.key(:home, account.id), reblog.id)).to be_nil + it "fills a user's home and list timelines with the expected posts" do + subject.call(account) + + 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