Improve account suspension speed and completeness (#9290)

- Some associations were missing from the clean-up
- Some attributes were not reset on suspension
- Skip federation and streaming deletes when purging a dead domain
- Move account association definitions to concern
This commit is contained in:
Eugen Rochko 2018-12-03 01:32:08 +01:00 committed by GitHub
parent 2df5ef18ae
commit 6ddf0432e7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 153 additions and 76 deletions

View file

@ -49,6 +49,7 @@ class Account < ApplicationRecord
USERNAME_RE = /[a-z0-9_]+([a-z0-9_\.-]+[a-z0-9_]+)?/i USERNAME_RE = /[a-z0-9_]+([a-z0-9_\.-]+[a-z0-9_]+)?/i
MENTION_RE = /(?<=^|[^\/[:word:]])@((#{USERNAME_RE})(?:@[a-z0-9\.\-]+[a-z0-9]+)?)/i MENTION_RE = /(?<=^|[^\/[:word:]])@((#{USERNAME_RE})(?:@[a-z0-9\.\-]+[a-z0-9]+)?)/i
include AccountAssociations
include AccountAvatar include AccountAvatar
include AccountFinderConcern include AccountFinderConcern
include AccountHeader include AccountHeader
@ -59,9 +60,6 @@ class Account < ApplicationRecord
enum protocol: [:ostatus, :activitypub] enum protocol: [:ostatus, :activitypub]
# Local users
has_one :user, inverse_of: :account
validates :username, presence: true validates :username, presence: true
# Remote user validations # Remote user validations
@ -76,45 +74,6 @@ class Account < ApplicationRecord
validates :note, length: { maximum: 160 }, if: -> { local? && will_save_change_to_note? } validates :note, length: { maximum: 160 }, if: -> { local? && will_save_change_to_note? }
validates :fields, length: { maximum: 4 }, if: -> { local? && will_save_change_to_fields? } validates :fields, length: { maximum: 4 }, if: -> { local? && will_save_change_to_fields? }
# Timelines
has_many :stream_entries, inverse_of: :account, dependent: :destroy
has_many :statuses, inverse_of: :account, dependent: :destroy
has_many :favourites, inverse_of: :account, dependent: :destroy
has_many :mentions, inverse_of: :account, dependent: :destroy
has_many :notifications, inverse_of: :account, dependent: :destroy
# Pinned statuses
has_many :status_pins, inverse_of: :account, dependent: :destroy
has_many :pinned_statuses, -> { reorder('status_pins.created_at DESC') }, through: :status_pins, class_name: 'Status', source: :status
# Endorsements
has_many :account_pins, inverse_of: :account, dependent: :destroy
has_many :endorsed_accounts, through: :account_pins, class_name: 'Account', source: :target_account
# Media
has_many :media_attachments, dependent: :destroy
# PuSH subscriptions
has_many :subscriptions, dependent: :destroy
# Report relationships
has_many :reports
has_many :targeted_reports, class_name: 'Report', foreign_key: :target_account_id
has_many :report_notes, dependent: :destroy
has_many :custom_filters, inverse_of: :account, dependent: :destroy
# Moderation notes
has_many :account_moderation_notes, dependent: :destroy
has_many :targeted_moderation_notes, class_name: 'AccountModerationNote', foreign_key: :target_account_id, dependent: :destroy
# Lists
has_many :list_accounts, inverse_of: :account, dependent: :destroy
has_many :lists, through: :list_accounts
# Account migrations
belongs_to :moved_to_account, class_name: 'Account', optional: true
scope :remote, -> { where.not(domain: nil) } scope :remote, -> { where.not(domain: nil) }
scope :local, -> { where(domain: nil) } scope :local, -> { where(domain: nil) }
scope :expiring, ->(time) { remote.where.not(subscription_expires_at: nil).where('subscription_expires_at < ?', time) } scope :expiring, ->(time) { remote.where.not(subscription_expires_at: nil).where('subscription_expires_at < ?', time) }
@ -452,6 +411,7 @@ class Account < ApplicationRecord
before_create :generate_keys before_create :generate_keys
before_validation :normalize_domain before_validation :normalize_domain
before_validation :prepare_contents, if: :local? before_validation :prepare_contents, if: :local?
before_destroy :clean_feed_manager
private private
@ -477,4 +437,19 @@ class Account < ApplicationRecord
def emojifiable_text def emojifiable_text
[note, display_name, fields.map(&:value)].join(' ') [note, display_name, fields.map(&:value)].join(' ')
end end
def clean_feed_manager
reblog_key = FeedManager.instance.key(:home, id, 'reblogs')
reblogged_id_set = Redis.current.zrange(reblog_key, 0, -1)
Redis.current.pipelined do
Redis.current.del(FeedManager.instance.key(:home, id))
Redis.current.del(reblog_key)
reblogged_id_set.each do |reblogged_id|
reblog_set_key = FeedManager.instance.key(:home, id, "reblogs:#{reblogged_id}")
Redis.current.del(reblog_set_key)
end
end
end
end end

View file

@ -0,0 +1,53 @@
# frozen_string_literal: true
module AccountAssociations
extend ActiveSupport::Concern
included do
# Local users
has_one :user, inverse_of: :account, dependent: :destroy
# Timelines
has_many :stream_entries, inverse_of: :account, dependent: :destroy
has_many :statuses, inverse_of: :account, dependent: :destroy
has_many :favourites, inverse_of: :account, dependent: :destroy
has_many :mentions, inverse_of: :account, dependent: :destroy
has_many :notifications, inverse_of: :account, dependent: :destroy
has_many :conversations, class_name: 'AccountConversation', dependent: :destroy, inverse_of: :account
# Pinned statuses
has_many :status_pins, inverse_of: :account, dependent: :destroy
has_many :pinned_statuses, -> { reorder('status_pins.created_at DESC') }, through: :status_pins, class_name: 'Status', source: :status
# Endorsements
has_many :account_pins, inverse_of: :account, dependent: :destroy
has_many :endorsed_accounts, through: :account_pins, class_name: 'Account', source: :target_account
# Media
has_many :media_attachments, dependent: :destroy
# PuSH subscriptions
has_many :subscriptions, dependent: :destroy
# Report relationships
has_many :reports, dependent: :destroy, inverse_of: :account
has_many :targeted_reports, class_name: 'Report', foreign_key: :target_account_id, dependent: :destroy, inverse_of: :target_account
has_many :report_notes, dependent: :destroy
has_many :custom_filters, inverse_of: :account, dependent: :destroy
# Moderation notes
has_many :account_moderation_notes, dependent: :destroy, inverse_of: :account
has_many :targeted_moderation_notes, class_name: 'AccountModerationNote', foreign_key: :target_account_id, dependent: :destroy, inverse_of: :target_account
# Lists (that the account is on, not owned by the account)
has_many :list_accounts, inverse_of: :account, dependent: :destroy
has_many :lists, through: :list_accounts
# Lists (owned by the account)
has_many :owned_lists, class_name: 'List', dependent: :destroy, inverse_of: :account
# Account migrations
belongs_to :moved_to_account, class_name: 'Account', optional: true
end
end

View file

@ -9,7 +9,9 @@ class BatchedRemoveStatusService < BaseService
# Remove statuses from home feeds # Remove statuses from home feeds
# Push delete events to streaming API for home feeds and public feeds # Push delete events to streaming API for home feeds and public feeds
# @param [Status] statuses A preferably batched array of statuses # @param [Status] statuses A preferably batched array of statuses
def call(statuses) # @param [Hash] options
# @option [Boolean] :skip_side_effects
def call(statuses, **options)
statuses = Status.where(id: statuses.map(&:id)).includes(:account, :stream_entry).flat_map { |status| [status] + status.reblogs.includes(:account, :stream_entry).to_a } statuses = Status.where(id: statuses.map(&:id)).includes(:account, :stream_entry).flat_map { |status| [status] + status.reblogs.includes(:account, :stream_entry).to_a }
@mentions = statuses.each_with_object({}) { |s, h| h[s.id] = s.active_mentions.includes(:account).to_a } @mentions = statuses.each_with_object({}) { |s, h| h[s.id] = s.active_mentions.includes(:account).to_a }
@ -26,6 +28,8 @@ class BatchedRemoveStatusService < BaseService
status.destroy status.destroy
end end
return if options[:skip_side_effects]
# Batch by source account # Batch by source account
statuses.group_by(&:account_id).each_value do |account_statuses| statuses.group_by(&:account_id).each_value do |account_statuses|
account = account_statuses.first.account account = account_statuses.first.account

View file

@ -1,6 +1,41 @@
# frozen_string_literal: true # frozen_string_literal: true
class SuspendAccountService < BaseService class SuspendAccountService < BaseService
ASSOCIATIONS_ON_SUSPEND = %w(
account_pins
active_relationships
block_relationships
blocked_by_relationships
conversation_mutes
conversations
custom_filters
domain_blocks
favourites
follow_requests
list_accounts
media_attachments
mute_relationships
muted_by_relationships
notifications
owned_lists
passive_relationships
report_notes
status_pins
stream_entries
subscriptions
).freeze
ASSOCIATIONS_ON_DESTROY = %w(
reports
targeted_moderation_notes
targeted_reports
).freeze
# Suspend an account and remove as much of its data as possible
# @param [Account]
# @param [Hash] options
# @option [Boolean] :including_user Remove the user record as well
# @option [Boolean] :destroy Remove the account record instead of suspending
def call(account, **options) def call(account, **options)
@account = account @account = account
@options = options @options = options
@ -8,60 +43,66 @@ class SuspendAccountService < BaseService
purge_user! purge_user!
purge_profile! purge_profile!
purge_content! purge_content!
unsubscribe_push_subscribers!
end end
private private
def purge_user! def purge_user!
if @options[:remove_user] return if !@account.local? || @account.user.nil?
@account.user&.destroy
if @options[:including_user]
@account.user.destroy
else else
@account.user&.disable! @account.user.disable!
end end
end end
def purge_content! def purge_content!
if @account.local? distribute_delete_actor! if @account.local?
ActivityPub::DeliveryWorker.push_bulk(delivery_inboxes) do |inbox_url|
[delete_actor_json, @account.id, inbox_url]
end
end
@account.statuses.reorder(nil).find_in_batches do |statuses| @account.statuses.reorder(nil).find_in_batches do |statuses|
BatchedRemoveStatusService.new.call(statuses) BatchedRemoveStatusService.new.call(statuses, skip_side_effects: @options[:destroy])
end end
[ associations_for_destruction.each do |association_name|
@account.media_attachments, destroy_all(@account.public_send(association_name))
@account.stream_entries,
@account.notifications,
@account.favourites,
@account.active_relationships,
@account.passive_relationships,
].each do |association|
destroy_all(association)
end end
@account.destroy if @options[:destroy]
end end
def purge_profile! def purge_profile!
@account.suspended = true # If the account is going to be destroyed
@account.display_name = '' # there is no point wasting time updating
@account.note = '' # its values first
@account.statuses_count = 0
return if @options[:destroy]
@account.silenced = false
@account.suspended = true
@account.locked = false
@account.display_name = ''
@account.note = ''
@account.fields = {}
@account.statuses_count = 0
@account.followers_count = 0
@account.following_count = 0
@account.moved_to_account = nil
@account.avatar.destroy @account.avatar.destroy
@account.header.destroy @account.header.destroy
@account.save! @account.save!
end end
def unsubscribe_push_subscribers!
destroy_all(@account.subscriptions)
end
def destroy_all(association) def destroy_all(association)
association.in_batches.destroy_all association.in_batches.destroy_all
end end
def distribute_delete_actor!
ActivityPub::DeliveryWorker.push_bulk(delivery_inboxes) do |inbox_url|
[delete_actor_json, @account.id, inbox_url]
end
end
def delete_actor_json def delete_actor_json
return @delete_actor_json if defined?(@delete_actor_json) return @delete_actor_json if defined?(@delete_actor_json)
@ -77,4 +118,12 @@ class SuspendAccountService < BaseService
def delivery_inboxes def delivery_inboxes
Account.inboxes + Relay.enabled.pluck(:inbox_url) Account.inboxes + Relay.enabled.pluck(:inbox_url)
end end
def associations_for_destruction
if @options[:destroy]
ASSOCIATIONS_ON_SUSPEND + ASSOCIATIONS_ON_DESTROY
else
ASSOCIATIONS_ON_SUSPEND
end
end
end end

View file

@ -6,6 +6,6 @@ class Admin::SuspensionWorker
sidekiq_options queue: 'pull' sidekiq_options queue: 'pull'
def perform(account_id, remove_user = false) def perform(account_id, remove_user = false)
SuspendAccountService.new.call(Account.find(account_id), remove_user: remove_user) SuspendAccountService.new.call(Account.find(account_id), including_user: remove_user)
end end
end end

View file

@ -22,11 +22,7 @@ module Mastodon
dry_run = options[:dry_run] ? ' (DRY RUN)' : '' dry_run = options[:dry_run] ? ' (DRY RUN)' : ''
Account.where(domain: domain).find_each do |account| Account.where(domain: domain).find_each do |account|
unless options[:dry_run] SuspendAccountService.new.call(account, destroy: true) unless options[:dry_run]
SuspendAccountService.new.call(account)
account.destroy
end
removed += 1 removed += 1
say('.', :green, false) say('.', :green, false)
end end