commit: 34118169ace56b31d6d4b26638fb7375171b7796
parent: 4fd7aebd5e81236a5ec2956486d3228a40ac4978
Author: Eugen Rochko <eugen@zeonfederated.com>
Date: Mon, 16 Oct 2017 20:44:31 +0200
Keep references to all reblogs of a status on home feed (#5419)
* Keep references to all reblogs of a status on home feed
When inserting reblog: Add to set of reblogs of this status on
the feed, if original status was present in the feed, add it to
that set as well.
When removing a reblog: Remove it from that set. Take random
remaining item from the set. If one exists, re-insert it into feed,
otherwise do not re-insert anything.
Fix #4210
* When original is removed, toss out reblog references
Diffstat:
3 files changed, 72 insertions(+), 27 deletions(-)
diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb
@@ -174,13 +174,19 @@ class FeedManager
# either action is appropriate.
def add_to_feed(timeline_type, account, status)
timeline_key = key(timeline_type, account.id)
- reblog_key = key(timeline_type, account.id, 'reblogs')
+ reblog_key = key(timeline_type, account.id, 'reblogs')
if status.reblog?
+ reblog_set_key = key(timeline_type, account.id, "reblogs:#{status.reblog_of_id}")
+
# If the original status or a reblog of it is within
# REBLOG_FALLOFF statuses from the top, do not re-insert it into
# the feed
rank = redis.zrevrank(timeline_key, status.reblog_of_id)
+
+ redis.sadd(reblog_set_key, status.reblog_of_id) unless rank.nil?
+ redis.sadd(reblog_set_key, status.id)
+
return false if !rank.nil? && rank < FeedManager::REBLOG_FALLOFF
reblog_rank = redis.zrevrank(reblog_key, status.reblog_of_id)
@@ -201,7 +207,7 @@ class FeedManager
# do so if appropriate.
def remove_from_feed(timeline_type, account, status)
timeline_key = key(timeline_type, account.id)
- reblog_key = key(timeline_type, account.id, 'reblogs')
+ reblog_key = key(timeline_type, account.id, 'reblogs')
if status.reblog?
# 1. If the reblogging status is not in the feed, stop.
@@ -211,13 +217,21 @@ class FeedManager
# 2. Remove the reblogged status from the `:reblogs` zset.
redis.zrem(reblog_key, status.reblog_of_id)
- # 3. Add the reblogged status to the feed.
- # Note that we can't use old score in here
- # and it must be an ID of corresponding status
- # because we need to filter timeline by status ID.
- redis.zadd(timeline_key, status.reblog_of_id, status.reblog_of_id)
+ # 3. Remove reblog from set of this status's reblogs, and
+ # re-insert another reblog or original into the feed if
+ # one remains in the set
+ reblog_set_key = key(timeline_type, account.id, "reblogs:#{status.reblog_of_id}")
+
+ redis.srem(reblog_set_key, status.id)
+ other_reblog = redis.srandmember(reblog_set_key)
+
+ redis.zadd(timeline_key, other_reblog, other_reblog) if other_reblog
# 4. Remove the reblogging status from the feed (as normal)
+ # (outside conditional)
+ else
+ # If the original is getting deleted, no use for reblog references
+ redis.del(key(timeline_type, account.id, "reblogs:#{status.id}"))
end
redis.zrem(timeline_key, status.id)
diff --git a/app/workers/scheduler/feed_cleanup_scheduler.rb b/app/workers/scheduler/feed_cleanup_scheduler.rb
@@ -6,8 +6,9 @@ class Scheduler::FeedCleanupScheduler
def perform
redis.pipelined do
- inactive_users.pluck(:account_id).each do |account_id|
+ inactive_users.each do |account_id|
redis.del(FeedManager.instance.key(:home, account_id))
+ redis.del(FeedManager.instance.key(:home, account_id, 'reblogs'))
end
end
end
@@ -15,7 +16,7 @@ class Scheduler::FeedCleanupScheduler
private
def inactive_users
- User.confirmed.inactive
+ @inactive_users ||= User.confirmed.inactive.pluck(:account_id)
end
def redis
diff --git a/spec/lib/feed_manager_spec.rb b/spec/lib/feed_manager_spec.rb
@@ -231,36 +231,66 @@ RSpec.describe FeedManager do
end
describe '#unpush' do
- it 'leaves a reblogged status when deleting the reblog' do
- account = Fabricate(:account)
- reblogged = Fabricate(:status, id: Mastodon::Snowflake.id_at(2.day.ago.utc))
- other_status = Fabricate(:status, id: Mastodon::Snowflake.id_at(1.day.ago.utc))
- status = Fabricate(:status, reblog: reblogged)
+ let(:receiver) { Fabricate(:account) }
- FeedManager.instance.push('type', account, other_status)
- FeedManager.instance.push('type', account, status)
+ it 'leaves a reblogged status if original was on feed' do
+ reblogged = Fabricate(:status)
+ status = Fabricate(:status, reblog: reblogged)
+
+ FeedManager.instance.push('type', receiver, reblogged)
+ FeedManager::REBLOG_FALLOFF.times { FeedManager.instance.push('type', receiver, Fabricate(:status)) }
+ FeedManager.instance.push('type', receiver, status)
# The reblogging status should show up under normal conditions.
- expect(Redis.current.zrange("feed:type:#{account.id}", 0, -1)).to eq [other_status.id.to_s, status.id.to_s]
+ expect(Redis.current.zrange("feed:type:#{receiver.id}", 0, -1)).to include(status.id.to_s)
- FeedManager.instance.unpush('type', account, status)
+ FeedManager.instance.unpush('type', receiver, status)
- # Because we couldn't tell if the status showed up any other way,
- # we had to stick the reblogged status in by itself.
- # And it must be ordered by status ids.
- expect(Redis.current.zrange("feed:type:#{account.id}", 0, -1)).to eq [reblogged.id.to_s, other_status.id.to_s]
+ # Restore original status
+ expect(Redis.current.zrange("feed:type:#{receiver.id}", 0, -1)).to_not include(status.id.to_s)
+ expect(Redis.current.zrange("feed:type:#{receiver.id}", 0, -1)).to include(reblogged.id.to_s)
+ end
+
+ it 'removes a reblogged status if it was only reblogged once' do
+ reblogged = Fabricate(:status)
+ status = Fabricate(:status, reblog: reblogged)
+
+ FeedManager.instance.push('type', receiver, status)
+
+ # The reblogging status should show up under normal conditions.
+ expect(Redis.current.zrange("feed:type:#{receiver.id}", 0, -1)).to eq [status.id.to_s]
+
+ FeedManager.instance.unpush('type', receiver, status)
+
+ expect(Redis.current.zrange("feed:type:#{receiver.id}", 0, -1)).to be_empty
+ end
+
+ it 'leaves a reblogged status if another reblog was in feed' do
+ reblogged = Fabricate(:status)
+ status = Fabricate(:status, reblog: reblogged)
+ another_status = Fabricate(:status, reblog: reblogged)
+
+ FeedManager.instance.push('type', receiver, status)
+ FeedManager.instance.push('type', receiver, another_status)
+
+ # The reblogging status should show up under normal conditions.
+ expect(Redis.current.zrange("feed:type:#{receiver.id}", 0, -1)).to eq [status.id.to_s]
+
+ FeedManager.instance.unpush('type', receiver, status)
+
+ expect(Redis.current.zrange("feed:type:#{receiver.id}", 0, -1)).to eq [another_status.id.to_s]
end
it 'sends push updates' do
- account = Fabricate(:account)
- status = Fabricate(:status)
- FeedManager.instance.push('type', account, status)
+ status = Fabricate(:status)
+
+ FeedManager.instance.push('type', receiver, status)
allow(Redis.current).to receive_messages(publish: nil)
- FeedManager.instance.unpush('type', account, status)
+ FeedManager.instance.unpush('type', receiver, status)
deletion = Oj.dump(event: :delete, payload: status.id.to_s)
- expect(Redis.current).to have_received(:publish).with("timeline:#{account.id}", deletion)
+ expect(Redis.current).to have_received(:publish).with("timeline:#{receiver.id}", deletion)
end
end
end
