Do not push status to feed if its reblog is already inserted (#6488)

A complemental change for precompute_feed_service_spec.rb also fixes its
random failure which is caused by the Snowlake randomization of the order
of an original status and its reblog.
This commit is contained in:
Akihiko Odaki 2018-02-24 13:40:18 +09:00 committed by Eugen Rochko
parent e668180044
commit f8f0572ee0
4 changed files with 45 additions and 7 deletions

View file

@ -220,6 +220,14 @@ class FeedManager
return false return false
end end
else else
# A reblog may reach earlier than the original status because of the
# 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?
redis.zadd(timeline_key, status.id, status.id) redis.zadd(timeline_key, status.id, status.id)
end end

View file

@ -1,4 +1,4 @@
Fabricator(:list) do Fabricator(:list) do
account nil account
title "MyString" title "MyString"
end end

View file

@ -157,7 +157,7 @@ RSpec.describe FeedManager do
end end
end end
describe '#push' do describe '#push_to_home' do
it 'trims timelines if they will have more than FeedManager::MAX_ITEMS' do it 'trims timelines if they will have more than FeedManager::MAX_ITEMS' do
account = Fabricate(:account) account = Fabricate(:account)
status = Fabricate(:status) status = Fabricate(:status)
@ -248,6 +248,39 @@ RSpec.describe FeedManager do
expect(FeedManager.instance.push_to_home(account, reblogs.last)).to be true expect(FeedManager.instance.push_to_home(account, reblogs.last)).to be true
end end
end end
it "does not push when the given status's reblog is already inserted" do
account = Fabricate(:account)
reblog = Fabricate(:status)
status = Fabricate(:status, reblog: reblog)
FeedManager.instance.push_to_home(account, status)
expect(FeedManager.instance.push_to_home(account, reblog)).to eq false
end
end
describe '#push_to_list' do
it "does not push when the given status's reblog is already inserted" do
list = Fabricate(:list)
reblog = Fabricate(:status)
status = Fabricate(:status, reblog: reblog)
FeedManager.instance.push_to_list(list, status)
expect(FeedManager.instance.push_to_list(list, reblog)).to eq false
end
end
describe '#merge_into_timeline' 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)
expect(Redis.current.zscore("feed:home:0", reblog.id)).to eq nil
end
end end
describe '#trim' do describe '#trim' do

View file

@ -9,14 +9,11 @@ RSpec.describe PrecomputeFeedService do
let(:account) { Fabricate(:account) } let(:account) { Fabricate(:account) }
it 'fills a user timeline with statuses' do it 'fills a user timeline with statuses' do
account = Fabricate(:account) account = Fabricate(:account)
followed_account = Fabricate(:account) status = Fabricate(:status, account: account)
Fabricate(:follow, account: account, target_account: followed_account)
reblog = Fabricate(:status, account: followed_account)
status = Fabricate(:status, account: account, reblog: reblog)
subject.call(account) subject.call(account)
expect(Redis.current.zscore(FeedManager.instance.key(:home, account.id), reblog.id)).to be_within(0.1).of(status.id.to_f) expect(Redis.current.zscore(FeedManager.instance.key(:home, account.id), status.id)).to be_within(0.1).of(status.id.to_f)
end end
it 'does not raise an error even if it could not find any status' do it 'does not raise an error even if it could not find any status' do