Fix deletes not reaching every server that interacted with status (#15200)

Extract logic for determining ActivityPub inboxes to send deletes
to to its own class and explicitly include the person the status
replied to (even if not mentioned), people who favourited it, and
people who replied to it (though that one is still not recursive)
This commit is contained in:
Eugen Rochko 2020-11-27 15:48:31 +01:00 committed by GitHub
parent e1a6526c8d
commit e7e099d1a0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 97 additions and 46 deletions

View File

@ -0,0 +1,52 @@
# frozen_string_literal: true
class StatusReachFinder
def initialize(status)
@status = status
end
def inboxes
Account.where(id: reached_account_ids).inboxes
end
private
def reached_account_ids
[
replied_to_account_id,
reblog_of_account_id,
mentioned_account_ids,
reblogs_account_ids,
favourites_account_ids,
replies_account_ids,
].tap do |arr|
arr.flatten!
arr.compact!
arr.uniq!
end
end
def replied_to_account_id
@status.in_reply_to_account_id
end
def reblog_of_account_id
@status.reblog.account_id if @status.reblog?
end
def mentioned_account_ids
@status.mentions.pluck(:account_id)
end
def reblogs_account_ids
@status.reblogs.pluck(:account_id)
end
def favourites_account_ids
@status.favourites.pluck(:account_id)
end
def replies_account_ids
@status.replies.pluck(:account_id)
end
end

View File

@ -14,39 +14,42 @@ class RemoveStatusService < BaseService
@payload = Oj.dump(event: :delete, payload: status.id.to_s) @payload = Oj.dump(event: :delete, payload: status.id.to_s)
@status = status @status = status
@account = status.account @account = status.account
@tags = status.tags.pluck(:name).to_a
@mentions = status.active_mentions.includes(:account).to_a
@reblogs = status.reblogs.includes(:account).to_a
@options = options @options = options
RedisLock.acquire(lock_options) do |lock| RedisLock.acquire(lock_options) do |lock|
if lock.acquired? if lock.acquired?
remove_from_self if status.account.local? remove_from_self if @account.local?
remove_from_followers remove_from_followers
remove_from_lists remove_from_lists
remove_from_affected
remove_reblogs
remove_from_hashtags
remove_from_public
remove_from_media if status.media_attachments.any?
remove_from_spam_check
remove_media
@status.destroy! if @options[:immediate] || !@status.reported?
else
raise Mastodon::RaceConditionError
end
end
# There is no reason to send out Undo activities when the # There is no reason to send out Undo activities when the
# cause is that the original object has been removed, since # cause is that the original object has been removed, since
# original object being removed implicitly removes reblogs # original object being removed implicitly removes reblogs
# of it. The Delete activity of the original is forwarded # of it. The Delete activity of the original is forwarded
# separately. # separately.
return if !@account.local? || @options[:original_removed] if @account.local? && !@options[:original_removed]
remove_from_remote_followers remove_from_remote_followers
remove_from_remote_affected remove_from_remote_reach
end
# Since reblogs don't mention anyone, don't get reblogged,
# favourited and don't contain their own media attachments
# or hashtags, this can be skipped
unless @status.reblog?
remove_from_mentions
remove_reblogs
remove_from_hashtags
remove_from_public
remove_from_media if @status.media_attachments.any?
remove_from_spam_check
remove_media
end
@status.destroy! if @options[:immediate] || !@status.reported?
else
raise Mastodon::RaceConditionError
end
end
end end
private private
@ -67,31 +70,35 @@ class RemoveStatusService < BaseService
end end
end end
def remove_from_affected def remove_from_mentions
@mentions.map(&:account).select(&:local?).each do |account| # For limited visibility statuses, the mentions that determine
redis.publish("timeline:#{account.id}", @payload) # who receives them in their home feed are a subset of followers
# and therefore the delete is already handled by sending it to all
# followers. Here we send a delete to actively mentioned accounts
# that may not follow the account
@status.active_mentions.find_each do |mention|
redis.publish("timeline:#{mention.account_id}", @payload)
end end
end end
def remove_from_remote_affected def remove_from_remote_reach
return if @status.reblog?
# People who got mentioned in the status, or who # People who got mentioned in the status, or who
# reblogged it from someone else might not follow # reblogged it from someone else might not follow
# the author and wouldn't normally receive the # the author and wouldn't normally receive the
# delete notification - so here, we explicitly # delete notification - so here, we explicitly
# send it to them # send it to them
target_accounts = (@mentions.map(&:account).reject(&:local?) + @reblogs.map(&:account).reject(&:local?)) status_reach_finder = StatusReachFinder.new(@status)
target_accounts << @status.reblog.account if @status.reblog? && !@status.reblog.account.local?
target_accounts.uniq!(&:id)
# ActivityPub ActivityPub::DeliveryWorker.push_bulk(status_reach_finder.inboxes) do |inbox_url|
ActivityPub::DeliveryWorker.push_bulk(target_accounts.select(&:activitypub?).uniq(&:preferred_inbox_url)) do |target_account| [signed_activity_json, @account.id, inbox_url]
[signed_activity_json, @account.id, target_account.preferred_inbox_url]
end end
end end
def remove_from_remote_followers def remove_from_remote_followers
# ActivityPub
ActivityPub::DeliveryWorker.push_bulk(@account.followers.inboxes) do |inbox_url| ActivityPub::DeliveryWorker.push_bulk(@account.followers.inboxes) do |inbox_url|
[signed_activity_json, @account.id, inbox_url] [signed_activity_json, @account.id, inbox_url]
end end
@ -118,19 +125,19 @@ class RemoveStatusService < BaseService
# because once original status is gone, reblogs will disappear # because once original status is gone, reblogs will disappear
# without us being able to do all the fancy stuff # without us being able to do all the fancy stuff
@reblogs.each do |reblog| @status.reblogs.includes(:account).find_each do |reblog|
RemoveStatusService.new.call(reblog, original_removed: true) RemoveStatusService.new.call(reblog, original_removed: true)
end end
end end
def remove_from_hashtags def remove_from_hashtags
@account.featured_tags.where(tag_id: @status.tags.pluck(:id)).each do |featured_tag| @account.featured_tags.where(tag_id: @status.tags.map(&:id)).each do |featured_tag|
featured_tag.decrement(@status.id) featured_tag.decrement(@status.id)
end end
return unless @status.public_visibility? return unless @status.public_visibility?
@tags.each do |hashtag| @status.tags.map(&:name).each do |hashtag|
redis.publish("timeline:hashtag:#{hashtag.mb_chars.downcase}", @payload) redis.publish("timeline:hashtag:#{hashtag.mb_chars.downcase}", @payload)
redis.publish("timeline:hashtag:#{hashtag.mb_chars.downcase}:local", @payload) if @status.local? redis.publish("timeline:hashtag:#{hashtag.mb_chars.downcase}:local", @payload) if @status.local?
end end
@ -140,22 +147,14 @@ class RemoveStatusService < BaseService
return unless @status.public_visibility? return unless @status.public_visibility?
redis.publish('timeline:public', @payload) redis.publish('timeline:public', @payload)
if @status.local? redis.publish(@status.local? ? 'timeline:public:local' : 'timeline:public:remote', @payload)
redis.publish('timeline:public:local', @payload)
else
redis.publish('timeline:public:remote', @payload)
end
end end
def remove_from_media def remove_from_media
return unless @status.public_visibility? return unless @status.public_visibility?
redis.publish('timeline:public:media', @payload) redis.publish('timeline:public:media', @payload)
if @status.local? redis.publish(@status.local? ? 'timeline:public:local:media' : 'timeline:public:remote:media', @payload)
redis.publish('timeline:public:local:media', @payload)
else
redis.publish('timeline:public:remote:media', @payload)
end
end end
def remove_media def remove_media