From 517af45e32535efe1494c0e1e59304a5a7771dba Mon Sep 17 00:00:00 2001 From: ThibG Date: Mon, 7 Sep 2020 18:00:15 +0200 Subject: Fix multiple boosts of a same toot erroneously appearing in TL (#14759) * Check for and record reblog info atomically Instead of using ZREVRANK to determine whether a reblog is a new reblog or not, use ZADD's NX option to perform the check/addition option atomically. * Replace ZREVRANK call with ZSCORE key which is more efficient * Make tests a bit stricter * Fix off-by-one --- app/lib/feed_manager.rb | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) (limited to 'app/lib/feed_manager.rb') diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index 9ab7b53be..785009b52 100644 --- a/app/lib/feed_manager.rb +++ b/app/lib/feed_manager.rb @@ -77,9 +77,11 @@ class FeedManager # Get the score of the REBLOG_FALLOFF'th item in our feed, and stop # tracking anything after it for deduplication purposes. - falloff_rank = FeedManager::REBLOG_FALLOFF - 1 + falloff_rank = FeedManager::REBLOG_FALLOFF falloff_range = redis.zrevrange(timeline_key, falloff_rank, falloff_rank, with_scores: true) - falloff_score = falloff_range&.first&.last&.to_i || 0 + falloff_score = falloff_range&.first&.last&.to_i + + return if falloff_score.nil? # Get any reblogs we might have to clean up after. redis.zrangebyscore(reblog_key, 0, falloff_score).each do |reblogged_id| @@ -279,14 +281,12 @@ class FeedManager return false if !rank.nil? && rank < FeedManager::REBLOG_FALLOFF - reblog_rank = redis.zrevrank(reblog_key, status.reblog_of_id) - - if reblog_rank.nil? + # The ordered set at `reblog_key` holds statuses which have a reblog + # in the top `REBLOG_FALLOFF` statuses of the timeline + if redis.zadd(reblog_key, status.id, status.reblog_of_id, nx: true) # This is not something we've already seen reblogged, so we - # can just add it to the feed (and note that we're - # reblogging it). + # can just add it to the feed (and note that we're reblogging it). redis.zadd(timeline_key, status.id, status.id) - redis.zadd(reblog_key, status.id, status.reblog_of_id) else # Another reblog of the same status was already in the # REBLOG_FALLOFF most recent statuses, so we note that this @@ -300,9 +300,7 @@ class FeedManager # delay of the worker deliverying the original status, the late addition # by merging timelines, and other reasons. # If such a reblog already exists, just do not re-insert it into the feed. - rank = redis.zrevrank(reblog_key, status.id) - - return false unless rank.nil? + return false unless redis.zscore(reblog_key, status.id).nil? redis.zadd(timeline_key, status.id, status.id) end -- cgit From 65760f59df46e388919a9f7ccba1958d967b2695 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 8 Sep 2020 03:41:16 +0200 Subject: Refactor feed manager (#14761) --- app/lib/feed_manager.rb | 236 +++++++++++++++++++++++++------- app/services/after_block_service.rb | 2 +- app/services/notify_service.rb | 6 +- app/services/precompute_feed_service.rb | 2 +- app/workers/feed_insert_worker.rb | 9 +- app/workers/merge_worker.rb | 4 +- app/workers/mute_worker.rb | 7 +- app/workers/unmerge_worker.rb | 4 +- spec/lib/feed_manager_spec.rb | 88 ++++-------- 9 files changed, 235 insertions(+), 123 deletions(-) (limited to 'app/lib/feed_manager.rb') diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index 785009b52..0876d107b 100644 --- a/app/lib/feed_manager.rb +++ b/app/lib/feed_manager.rb @@ -6,31 +6,54 @@ class FeedManager include Singleton include Redisable + # Maximum number of items stored in a single feed MAX_ITEMS = 400 - # Must be <= MAX_ITEMS or the tracking sets will grow forever + # Number of items in the feed since last reblog of status + # before the new reblog will be inserted. Must be <= MAX_ITEMS + # or the tracking sets will grow forever REBLOG_FALLOFF = 40 + # Execute block for every active account + # @yield [Account] + # @return [void] def with_active_accounts(&block) Account.joins(:user).where('users.current_sign_in_at > ?', User::ACTIVE_DURATION.ago).find_each(&block) end + # Redis key of a feed + # @param [Symbol] type + # @param [Integer] id + # @param [Symbol] subtype + # @return [String] def key(type, id, subtype = nil) return "feed:#{type}:#{id}" unless subtype "feed:#{type}:#{id}:#{subtype}" end - def filter?(timeline_type, status, receiver_id) - if timeline_type == :home - filter_from_home?(status, receiver_id, build_crutches(receiver_id, [status])) - elsif timeline_type == :mentions - filter_from_mentions?(status, receiver_id) + # Check if the status should not be added to a feed + # @param [Symbol] timeline_type + # @param [Status] status + # @param [Account|List] receiver + # @return [Boolean] + def filter?(timeline_type, status, receiver) + case timeline_type + when :home + filter_from_home?(status, receiver.id, build_crutches(receiver.id, [status])) + when :list + filter_from_list?(status, receiver) || filter_from_home?(status, receiver.account_id, build_crutches(receiver.account_id, [status])) + when :mentions + filter_from_mentions?(status, receiver.id) else false end end + # Add a status to a home feed and send a streaming API update + # @param [Account] account + # @param [Status] status + # @return [Boolean] def push_to_home(account, status) return false unless add_to_feed(:home, account.id, status, account.user&.aggregates_reblogs?) @@ -39,6 +62,10 @@ class FeedManager true end + # Remove a status from a home feed and send a streaming API update + # @param [Account] account + # @param [Status] status + # @return [Boolean] def unpush_from_home(account, status) return false unless remove_from_feed(:home, account.id, status, account.user&.aggregates_reblogs?) @@ -46,21 +73,22 @@ class FeedManager true end + # Add a status to a list feed and send a streaming API update + # @param [List] list + # @param [Status] status + # @return [Boolean] def push_to_list(list, status) - if status.reply? && status.in_reply_to_account_id != status.account_id - should_filter = status.in_reply_to_account_id != list.account_id - should_filter &&= !list.show_all_replies? - should_filter &&= !(list.show_list_replies? && ListAccount.where(list_id: list.id, account_id: status.in_reply_to_account_id).exists?) - return false if should_filter - end - - return false unless add_to_feed(:list, list.id, status, list.account.user&.aggregates_reblogs?) + return false if filter_from_list?(status, list) || !add_to_feed(:list, list.id, status, list.account.user&.aggregates_reblogs?) trim(:list, list.id) PushUpdateWorker.perform_async(list.account_id, status.id, "timeline:list:#{list.id}") if push_update_required?("timeline:list:#{list.id}") true end + # Remove a status from a list feed and send a streaming API update + # @param [List] list + # @param [Status] status + # @return [Boolean] def unpush_from_list(list, status) return false unless remove_from_feed(:list, list.id, status, list.account.user&.aggregates_reblogs?) @@ -68,36 +96,39 @@ class FeedManager true end - def trim(type, account_id) - timeline_key = key(type, account_id) - reblog_key = key(type, account_id, 'reblogs') + # Fill a home feed with an account's statuses + # @param [Account] from_account + # @param [Account] into_account + # @return [void] + def merge_into_home(from_account, into_account) + timeline_key = key(:home, into_account.id) + aggregate = into_account.user&.aggregates_reblogs? + query = from_account.statuses.where(visibility: [:public, :unlisted, :private]).includes(:preloadable_poll, reblog: :account).limit(FeedManager::MAX_ITEMS / 4) - # Remove any items past the MAX_ITEMS'th entry in our feed - redis.zremrangebyrank(timeline_key, 0, -(FeedManager::MAX_ITEMS + 1)) + if redis.zcard(timeline_key) >= FeedManager::MAX_ITEMS / 4 + oldest_home_score = redis.zrange(timeline_key, 0, 0, with_scores: true).first.last.to_i + query = query.where('id > ?', oldest_home_score) + end - # Get the score of the REBLOG_FALLOFF'th item in our feed, and stop - # tracking anything after it for deduplication purposes. - falloff_rank = FeedManager::REBLOG_FALLOFF - falloff_range = redis.zrevrange(timeline_key, falloff_rank, falloff_rank, with_scores: true) - falloff_score = falloff_range&.first&.last&.to_i + statuses = query.to_a + crutches = build_crutches(into_account.id, statuses) - return if falloff_score.nil? + statuses.each do |status| + next if filter_from_home?(status, into_account.id, crutches) - # Get any reblogs we might have to clean up after. - redis.zrangebyscore(reblog_key, 0, falloff_score).each do |reblogged_id| - # Remove it from the set of reblogs we're tracking *first* to avoid races. - redis.zrem(reblog_key, reblogged_id) - # Just drop any set we might have created to track additional reblogs. - # This means that if this reblog is deleted, we won't automatically insert - # another reblog, but also that any new reblog can be inserted into the - # feed. - redis.del(key(type, account_id, "reblogs:#{reblogged_id}")) + add_to_feed(:home, into_account.id, status, aggregate) end + + trim(:home, into_account.id) end - def merge_into_timeline(from_account, into_account) - timeline_key = key(:home, into_account.id) - aggregate = into_account.user&.aggregates_reblogs? + # Fill a list feed with an account's statuses + # @param [Account] from_account + # @param [List] list + # @return [void] + def merge_into_list(from_account, list) + timeline_key = key(:list, list.id) + aggregate = list.account.user&.aggregates_reblogs? query = from_account.statuses.where(visibility: [:public, :unlisted, :private]).includes(:preloadable_poll, reblog: :account).limit(FeedManager::MAX_ITEMS / 4) if redis.zcard(timeline_key) >= FeedManager::MAX_ITEMS / 4 @@ -106,18 +137,22 @@ class FeedManager end statuses = query.to_a - crutches = build_crutches(into_account.id, statuses) + crutches = build_crutches(list.account_id, statuses) statuses.each do |status| - next if filter_from_home?(status, into_account.id, crutches) + next if filter_from_home?(status, list.account_id, crutches) || filter_from_list?(status, list) - add_to_feed(:home, into_account.id, status, aggregate) + add_to_feed(:list, list.id, status, aggregate) end - trim(:home, into_account.id) + trim(:list, list.id) end - def unmerge_from_timeline(from_account, into_account) + # Remove an account's statuses from a home feed + # @param [Account] from_account + # @param [Account] into_account + # @return [void] + def unmerge_from_home(from_account, into_account) timeline_key = key(:home, into_account.id) oldest_home_score = redis.zrange(timeline_key, 0, 0, with_scores: true)&.first&.last&.to_i || 0 @@ -126,14 +161,31 @@ class FeedManager end end - def clear_from_timeline(account, target_account) - # Clear from timeline all statuses from or mentionning target_account + # Remove an account's statuses from a list feed + # @param [Account] from_account + # @param [List] list + # @return [void] + def unmerge_from_list(from_account, list) + timeline_key = key(:list, list.id) + oldest_list_score = redis.zrange(timeline_key, 0, 0, with_scores: true)&.first&.last&.to_i || 0 + + from_account.statuses.select('id, reblog_of_id').where('id > ?', oldest_list_score).reorder(nil).find_each do |status| + remove_from_feed(:list, list.id, status, list.account.user&.aggregates_reblogs?) + end + end + + # Clear all statuses from or mentioning target_account from a home feed + # @param [Account] account + # @param [Account] target_account + # @return [void] + def clear_from_home(account, target_account) timeline_key = key(:home, account.id) timeline_status_ids = redis.zrange(timeline_key, 0, -1) statuses = Status.where(id: timeline_status_ids).select(:id, :reblog_of_id, :account_id).to_a reblogged_ids = Status.where(id: statuses.map(&:reblog_of_id).compact, account: target_account).pluck(:id) with_mentions_ids = Mention.active.where(status_id: statuses.flat_map { |s| [s.id, s.reblog_of_id] }.compact, account: target_account).pluck(:status_id) - target_statuses = statuses.filter do |status| + + target_statuses = statuses.select do |status| status.account_id == target_account.id || reblogged_ids.include?(status.reblog_of_id) || with_mentions_ids.include?(status.id) || with_mentions_ids.include?(status.reblog_of_id) end @@ -142,7 +194,10 @@ class FeedManager end end - def populate_feed(account) + # Populate home feed of account from scratch + # @param [Account] account + # @return [void] + def populate_home(account) limit = FeedManager::MAX_ITEMS / 2 aggregate = account.user&.aggregates_reblogs? timeline_key = key(:home, account.id) @@ -177,15 +232,59 @@ class FeedManager private - def push_update_required?(timeline_id) - redis.exists?("subscribed:#{timeline_id}") + # Trim a feed to maximum size by removing older items + # @param [Symbol] type + # @param [Integer] timeline_id + # @return [void] + def trim(type, timeline_id) + timeline_key = key(type, timeline_id) + reblog_key = key(type, timeline_id, 'reblogs') + + # Remove any items past the MAX_ITEMS'th entry in our feed + redis.zremrangebyrank(timeline_key, 0, -(FeedManager::MAX_ITEMS + 1)) + + # Get the score of the REBLOG_FALLOFF'th item in our feed, and stop + # tracking anything after it for deduplication purposes. + falloff_rank = FeedManager::REBLOG_FALLOFF + falloff_range = redis.zrevrange(timeline_key, falloff_rank, falloff_rank, with_scores: true) + falloff_score = falloff_range&.first&.last&.to_i + + return if falloff_score.nil? + + # Get any reblogs we might have to clean up after. + redis.zrangebyscore(reblog_key, 0, falloff_score).each do |reblogged_id| + # Remove it from the set of reblogs we're tracking *first* to avoid races. + redis.zrem(reblog_key, reblogged_id) + # Just drop any set we might have created to track additional reblogs. + # This means that if this reblog is deleted, we won't automatically insert + # another reblog, but also that any new reblog can be inserted into the + # feed. + redis.del(key(type, timeline_id, "reblogs:#{reblogged_id}")) + end end + # Check if there is a streaming API client connected + # for the given feed + # @param [String] timeline_key + # @return [Boolean] + def push_update_required?(timeline_key) + redis.exists?("subscribed:#{timeline_key}") + end + + # Check if the account is blocking or muting any of the given accounts + # @param [Integer] receiver_id + # @param [Array] account_ids + # @param [Symbol] context def blocks_or_mutes?(receiver_id, account_ids, context) Block.where(account_id: receiver_id, target_account_id: account_ids).any? || (context == :home ? Mute.where(account_id: receiver_id, target_account_id: account_ids).any? : Mute.where(account_id: receiver_id, target_account_id: account_ids, hide_notifications: true).any?) end + # Check if status should not be added to the home feed + # @param [Status] status + # @param [Integer] receiver_id + # @param [Hash] crutches + # @return [Boolean] def filter_from_home?(status, receiver_id, crutches) return false if receiver_id == status.account_id return true if status.reply? && (status.in_reply_to_id.nil? || status.in_reply_to_account_id.nil?) @@ -218,6 +317,11 @@ class FeedManager false end + # Check if status should not be added to the mentions feed + # @see NotifyService + # @param [Status] status + # @param [Integer] receiver_id + # @return [Boolean] def filter_from_mentions?(status, receiver_id) return true if receiver_id == status.account_id return true if phrase_filtered?(status, receiver_id, :notifications) @@ -234,6 +338,27 @@ class FeedManager should_filter end + # Check if status should not be added to the list feed + # @param [Status] status + # @param [List] list + # @return [Boolean] + def filter_from_list?(status, list) + if status.reply? && status.in_reply_to_account_id != status.account_id + should_filter = status.in_reply_to_account_id != list.account_id + should_filter &&= !list.show_all_replies? + should_filter &&= !(list.show_list_replies? && ListAccount.where(list_id: list.id, account_id: status.in_reply_to_account_id).exists?) + + return !!should_filter + end + + false + end + + # Check if the status hits a phrase filter + # @param [Status] status + # @param [Integer] receiver_id + # @param [Symbol] context + # @return [Boolean] def phrase_filtered?(status, receiver_id, context) active_filters = Rails.cache.fetch("filters:#{receiver_id}") { CustomFilter.where(account_id: receiver_id).active_irreversible.to_a }.to_a @@ -269,6 +394,11 @@ class FeedManager # added, and false if it was not added to the feed. Note that this is # an internal helper: callers must call trim or push updates if # either action is appropriate. + # @param [Symbol] timeline_type + # @param [Integer] account_id + # @param [Status] status + # @param [Boolean] aggregate_reblogs + # @return [Boolean] def add_to_feed(timeline_type, account_id, status, aggregate_reblogs = true) timeline_key = key(timeline_type, account_id) reblog_key = key(timeline_type, account_id, 'reblogs') @@ -312,6 +442,11 @@ class FeedManager # with reblogs, and returning true if a status was removed. As with # `add_to_feed`, this does not trigger push updates, so callers must # do so if appropriate. + # @param [Symbol] timeline_type + # @param [Integer] account_id + # @param [Status] status + # @param [Boolean] aggregate_reblogs + # @return [Boolean] def remove_from_feed(timeline_type, account_id, status, aggregate_reblogs = true) timeline_key = key(timeline_type, account_id) reblog_key = key(timeline_type, account_id, 'reblogs') @@ -346,6 +481,11 @@ class FeedManager redis.zrem(timeline_key, status.id) end + # Pre-fetch various objects and relationships for given statuses that + # are going to be checked by the filtering methods + # @param [Integer] receiver_id + # @param [Array] statuses + # @return [Hash] def build_crutches(receiver_id, statuses) crutches = {} diff --git a/app/services/after_block_service.rb b/app/services/after_block_service.rb index 2a0e10a79..314919df8 100644 --- a/app/services/after_block_service.rb +++ b/app/services/after_block_service.rb @@ -13,7 +13,7 @@ class AfterBlockService < BaseService private def clear_home_feed! - FeedManager.instance.clear_from_timeline(@account, @target_account) + FeedManager.instance.clear_from_home(@account, @target_account) end def clear_conversations! diff --git a/app/services/notify_service.rb b/app/services/notify_service.rb index abd676494..e4ca10eb1 100644 --- a/app/services/notify_service.rb +++ b/app/services/notify_service.rb @@ -13,15 +13,13 @@ class NotifyService < BaseService push_to_conversation! if direct_message? send_email! if email_enabled? rescue ActiveRecord::RecordInvalid - # rubocop:disable Style/RedundantReturn - return - # rubocop:enable Style/RedundantReturn + nil end private def blocked_mention? - FeedManager.instance.filter?(:mentions, @notification.mention.status, @recipient.id) + FeedManager.instance.filter?(:mentions, @notification.mention.status, @recipient) end def blocked_favourite? diff --git a/app/services/precompute_feed_service.rb b/app/services/precompute_feed_service.rb index 076dedaca..61f573534 100644 --- a/app/services/precompute_feed_service.rb +++ b/app/services/precompute_feed_service.rb @@ -2,7 +2,7 @@ class PrecomputeFeedService < BaseService def call(account) - FeedManager.instance.populate_feed(account) + FeedManager.instance.populate_home(account) ensure Redis.current.del("account:#{account.id}:regeneration") end diff --git a/app/workers/feed_insert_worker.rb b/app/workers/feed_insert_worker.rb index 1ae3c877b..633ec91bd 100644 --- a/app/workers/feed_insert_worker.rb +++ b/app/workers/feed_insert_worker.rb @@ -27,9 +27,12 @@ class FeedInsertWorker end def feed_filtered? - # Note: Lists are a variation of home, so the filtering rules - # of home apply to both - FeedManager.instance.filter?(:home, @status, @follower.id) + case @type + when :home + FeedManager.instance.filter?(:home, @status, @follower) + when :list + FeedManager.instance.filter?(:list, @status, @list) + end end def perform_push diff --git a/app/workers/merge_worker.rb b/app/workers/merge_worker.rb index d745cb99c..74ef7d4da 100644 --- a/app/workers/merge_worker.rb +++ b/app/workers/merge_worker.rb @@ -6,6 +6,8 @@ class MergeWorker sidekiq_options queue: 'pull' def perform(from_account_id, into_account_id) - FeedManager.instance.merge_into_timeline(Account.find(from_account_id), Account.find(into_account_id)) + FeedManager.instance.merge_into_home(Account.find(from_account_id), Account.find(into_account_id)) + rescue ActiveRecord::RecordNotFound + true end end diff --git a/app/workers/mute_worker.rb b/app/workers/mute_worker.rb index 7bf0923a5..c74f657cb 100644 --- a/app/workers/mute_worker.rb +++ b/app/workers/mute_worker.rb @@ -4,9 +4,8 @@ class MuteWorker include Sidekiq::Worker def perform(account_id, target_account_id) - FeedManager.instance.clear_from_timeline( - Account.find(account_id), - Account.find(target_account_id) - ) + FeedManager.instance.clear_from_home(Account.find(account_id), Account.find(target_account_id)) + rescue ActiveRecord::RecordNotFound + true end end diff --git a/app/workers/unmerge_worker.rb b/app/workers/unmerge_worker.rb index ea6aacebf..1a23faae5 100644 --- a/app/workers/unmerge_worker.rb +++ b/app/workers/unmerge_worker.rb @@ -6,6 +6,8 @@ class UnmergeWorker sidekiq_options queue: 'pull' def perform(from_account_id, into_account_id) - FeedManager.instance.unmerge_from_timeline(Account.find(from_account_id), Account.find(into_account_id)) + FeedManager.instance.unmerge_from_home(Account.find(from_account_id), Account.find(into_account_id)) + rescue ActiveRecord::RecordNotFound + true end end diff --git a/spec/lib/feed_manager_spec.rb b/spec/lib/feed_manager_spec.rb index d86dd7993..d9c17470f 100644 --- a/spec/lib/feed_manager_spec.rb +++ b/spec/lib/feed_manager_spec.rb @@ -29,14 +29,14 @@ RSpec.describe FeedManager do it 'returns false for followee\'s status' do status = Fabricate(:status, text: 'Hello world', account: alice) bob.follow!(alice) - expect(FeedManager.instance.filter?(:home, status, bob.id)).to be false + expect(FeedManager.instance.filter?(:home, status, bob)).to be false end it 'returns false for reblog by followee' do status = Fabricate(:status, text: 'Hello world', account: jeff) reblog = Fabricate(:status, reblog: status, account: alice) bob.follow!(alice) - expect(FeedManager.instance.filter?(:home, reblog, bob.id)).to be false + expect(FeedManager.instance.filter?(:home, reblog, bob)).to be false end it 'returns true for reblog by followee of blocked account' do @@ -44,7 +44,7 @@ RSpec.describe FeedManager do reblog = Fabricate(:status, reblog: status, account: alice) bob.follow!(alice) bob.block!(jeff) - expect(FeedManager.instance.filter?(:home, reblog, bob.id)).to be true + expect(FeedManager.instance.filter?(:home, reblog, bob)).to be true end it 'returns true for reblog by followee of muted account' do @@ -52,7 +52,7 @@ RSpec.describe FeedManager do reblog = Fabricate(:status, reblog: status, account: alice) bob.follow!(alice) bob.mute!(jeff) - expect(FeedManager.instance.filter?(:home, reblog, bob.id)).to be true + expect(FeedManager.instance.filter?(:home, reblog, bob)).to be true end it 'returns true for reblog by followee of someone who is blocking recipient' do @@ -60,14 +60,14 @@ RSpec.describe FeedManager do reblog = Fabricate(:status, reblog: status, account: alice) bob.follow!(alice) jeff.block!(bob) - expect(FeedManager.instance.filter?(:home, reblog, bob.id)).to be true + expect(FeedManager.instance.filter?(:home, reblog, bob)).to be true end it 'returns true for reblog from account with reblogs disabled' do status = Fabricate(:status, text: 'Hello world', account: jeff) reblog = Fabricate(:status, reblog: status, account: alice) bob.follow!(alice, reblogs: false) - expect(FeedManager.instance.filter?(:home, reblog, bob.id)).to be true + expect(FeedManager.instance.filter?(:home, reblog, bob)).to be true end it 'returns false for reply by followee to another followee' do @@ -75,48 +75,48 @@ RSpec.describe FeedManager do reply = Fabricate(:status, text: 'Nay', thread: status, account: alice) bob.follow!(alice) bob.follow!(jeff) - expect(FeedManager.instance.filter?(:home, reply, bob.id)).to be false + expect(FeedManager.instance.filter?(:home, reply, bob)).to be false end it 'returns false for reply by followee to recipient' do status = Fabricate(:status, text: 'Hello world', account: bob) reply = Fabricate(:status, text: 'Nay', thread: status, account: alice) bob.follow!(alice) - expect(FeedManager.instance.filter?(:home, reply, bob.id)).to be false + expect(FeedManager.instance.filter?(:home, reply, bob)).to be false end it 'returns false for reply by followee to self' do status = Fabricate(:status, text: 'Hello world', account: alice) reply = Fabricate(:status, text: 'Nay', thread: status, account: alice) bob.follow!(alice) - expect(FeedManager.instance.filter?(:home, reply, bob.id)).to be false + expect(FeedManager.instance.filter?(:home, reply, bob)).to be false end it 'returns true for reply by followee to non-followed account' do status = Fabricate(:status, text: 'Hello world', account: jeff) reply = Fabricate(:status, text: 'Nay', thread: status, account: alice) bob.follow!(alice) - expect(FeedManager.instance.filter?(:home, reply, bob.id)).to be true + expect(FeedManager.instance.filter?(:home, reply, bob)).to be true end it 'returns true for the second reply by followee to a non-federated status' do reply = Fabricate(:status, text: 'Reply 1', reply: true, account: alice) second_reply = Fabricate(:status, text: 'Reply 2', thread: reply, account: alice) bob.follow!(alice) - expect(FeedManager.instance.filter?(:home, second_reply, bob.id)).to be true + expect(FeedManager.instance.filter?(:home, second_reply, bob)).to be true end it 'returns false for status by followee mentioning another account' do bob.follow!(alice) status = PostStatusService.new.call(alice, text: 'Hey @jeff') - expect(FeedManager.instance.filter?(:home, status, bob.id)).to be false + expect(FeedManager.instance.filter?(:home, status, bob)).to be false end it 'returns true for status by followee mentioning blocked account' do bob.block!(jeff) bob.follow!(alice) status = PostStatusService.new.call(alice, text: 'Hey @jeff') - expect(FeedManager.instance.filter?(:home, status, bob.id)).to be true + expect(FeedManager.instance.filter?(:home, status, bob)).to be true end it 'returns true for reblog of a personally blocked domain' do @@ -124,7 +124,7 @@ RSpec.describe FeedManager do alice.follow!(jeff) status = Fabricate(:status, text: 'Hello world', account: bob) reblog = Fabricate(:status, reblog: status, account: jeff) - expect(FeedManager.instance.filter?(:home, reblog, alice.id)).to be true + expect(FeedManager.instance.filter?(:home, reblog, alice)).to be true end context 'for irreversibly muted phrases' do @@ -132,7 +132,7 @@ RSpec.describe FeedManager do alice.custom_filters.create!(phrase: 'bob', context: %w(home), irreversible: true) alice.follow!(jeff) status = Fabricate(:status, text: 'bobcats', account: jeff) - expect(FeedManager.instance.filter?(:home, status, alice.id)).to be_falsy + expect(FeedManager.instance.filter?(:home, status, alice)).to be_falsy end it 'returns true if phrase is contained' do @@ -140,14 +140,14 @@ RSpec.describe FeedManager do alice.custom_filters.create!(phrase: 'pop tarts', context: %w(home), irreversible: true) alice.follow!(jeff) status = Fabricate(:status, text: 'i sure like POP TARts', account: jeff) - expect(FeedManager.instance.filter?(:home, status, alice.id)).to be true + expect(FeedManager.instance.filter?(:home, status, alice)).to be true end it 'matches substrings if whole_word is false' do alice.custom_filters.create!(phrase: 'take', context: %w(home), whole_word: false, irreversible: true) alice.follow!(jeff) status = Fabricate(:status, text: 'shiitake', account: jeff) - expect(FeedManager.instance.filter?(:home, status, alice.id)).to be true + expect(FeedManager.instance.filter?(:home, status, alice)).to be true end it 'returns true if phrase is contained in a poll option' do @@ -155,7 +155,7 @@ RSpec.describe FeedManager do alice.custom_filters.create!(phrase: 'pop tarts', context: %w(home), irreversible: true) alice.follow!(jeff) status = Fabricate(:status, text: 'what do you prefer', poll: Fabricate(:poll, options: %w(farts POP TARts)), account: jeff) - expect(FeedManager.instance.filter?(:home, status, alice.id)).to be true + expect(FeedManager.instance.filter?(:home, status, alice)).to be true end end end @@ -164,27 +164,27 @@ RSpec.describe FeedManager do it 'returns true for status that mentions blocked account' do bob.block!(jeff) status = PostStatusService.new.call(alice, text: 'Hey @jeff') - expect(FeedManager.instance.filter?(:mentions, status, bob.id)).to be true + expect(FeedManager.instance.filter?(:mentions, status, bob)).to be true end it 'returns true for status that replies to a blocked account' do status = Fabricate(:status, text: 'Hello world', account: jeff) reply = Fabricate(:status, text: 'Nay', thread: status, account: alice) bob.block!(jeff) - expect(FeedManager.instance.filter?(:mentions, reply, bob.id)).to be true + expect(FeedManager.instance.filter?(:mentions, reply, bob)).to be true end it 'returns true for status by silenced account who recipient is not following' do status = Fabricate(:status, text: 'Hello world', account: alice) alice.silence! - expect(FeedManager.instance.filter?(:mentions, status, bob.id)).to be true + expect(FeedManager.instance.filter?(:mentions, status, bob)).to be true end it 'returns false for status by followed silenced account' do status = Fabricate(:status, text: 'Hello world', account: alice) alice.silence! bob.follow!(alice) - expect(FeedManager.instance.filter?(:mentions, status, bob.id)).to be false + expect(FeedManager.instance.filter?(:mentions, status, bob)).to be false end end end @@ -414,52 +414,20 @@ RSpec.describe FeedManager do end end - describe '#merge_into_timeline' do + describe '#merge_into_home' do it "does not push source account's statuses whose reblogs are already inserted" do account = Fabricate(:account, id: 0) reblog = Fabricate(:status) status = Fabricate(:status, reblog: reblog) FeedManager.instance.push_to_home(account, status) - FeedManager.instance.merge_into_timeline(account, reblog.account) + FeedManager.instance.merge_into_home(account, reblog.account) expect(Redis.current.zscore("feed:home:0", reblog.id)).to eq nil end end - describe '#trim' do - let(:receiver) { Fabricate(:account) } - - it 'cleans up reblog tracking keys' do - reblogged = Fabricate(:status) - status = Fabricate(:status, reblog: reblogged) - another_status = Fabricate(:status, reblog: reblogged) - reblogs_key = FeedManager.instance.key('home', receiver.id, 'reblogs') - reblog_set_key = FeedManager.instance.key('home', receiver.id, "reblogs:#{reblogged.id}") - - FeedManager.instance.push_to_home(receiver, status) - FeedManager.instance.push_to_home(receiver, another_status) - - # We should have a tracking set and an entry in reblogs. - expect(Redis.current.exists?(reblog_set_key)).to be true - expect(Redis.current.zrange(reblogs_key, 0, -1)).to eq [reblogged.id.to_s] - - # Push everything past the reblog falloff. - FeedManager::REBLOG_FALLOFF.times do - FeedManager.instance.push_to_home(receiver, Fabricate(:status)) - end - - # `trim` should be called automatically, but do it anyway, as - # we're testing `trim`, not side effects of `push`. - FeedManager.instance.trim('home', receiver.id) - - # We should not have any reblog tracking data. - expect(Redis.current.exists?(reblog_set_key)).to be false - expect(Redis.current.zrange(reblogs_key, 0, -1)).to be_empty - end - end - - describe '#unpush' do + describe '#unpush_from_home' do let(:receiver) { Fabricate(:account) } it 'leaves a reblogged status if original was on feed' do @@ -525,7 +493,7 @@ RSpec.describe FeedManager do end end - describe '#clear_from_timeline' do + describe '#clear_from_home' do let(:account) { Fabricate(:account) } let(:followed_account) { Fabricate(:account) } let(:target_account) { Fabricate(:account) } @@ -543,8 +511,8 @@ RSpec.describe FeedManager do end end - it 'correctly cleans the timeline' do - FeedManager.instance.clear_from_timeline(account, target_account) + it 'correctly cleans the home timeline' do + FeedManager.instance.clear_from_home(account, target_account) expect(Redis.current.zrange("feed:home:#{account.id}", 0, -1)).to eq [status_1.id.to_s, status_7.id.to_s] end -- cgit