Fix unnecessary queries when batch-removing statuses, 100x faster (#15387)

1 files changed, 100 insertions, 26 deletions

diff --git a/app/services/delete_account_service.rb b/app/services/delete_account_service.rb
index fa834e775..5123a4697 100644
--- a/app/services/delete_account_service.rb
+++ b/app/services/delete_account_service.rb
@@ -6,15 +6,21 @@ class DeleteAccountService < BaseService
   ASSOCIATIONS_ON_SUSPEND = %w(
     account_pins
     active_relationships
+    aliases
     block_relationships
     blocked_by_relationships
+    bookmarks
     conversation_mutes
     conversations
     custom_filters
+    devices
     domain_blocks
     favourites
+    featured_tags
     follow_requests
+    identity_proofs
     list_accounts
+    migrations
     mute_relationships
     muted_by_relationships
     notifications
@@ -25,6 +31,29 @@ class DeleteAccountService < BaseService
     status_pins
   ).freeze
 
+  # The following associations have no important side-effects
+  # in callbacks and all of their own associations are secured
+  # by foreign keys, making them safe to delete without loading
+  # into memory
+  ASSOCIATIONS_WITHOUT_SIDE_EFFECTS = %w(
+    account_pins
+    aliases
+    conversation_mutes
+    conversations
+    custom_filters
+    devices
+    domain_blocks
+    featured_tags
+    follow_requests
+    identity_proofs
+    migrations
+    mute_relationships
+    muted_by_relationships
+    notifications
+    scheduled_statuses
+    status_pins
+  )
+
   ASSOCIATIONS_ON_DESTROY = %w(
     reports
     targeted_moderation_notes
@@ -55,19 +84,25 @@ class DeleteAccountService < BaseService
 
     @options[:skip_activitypub] = true if @options[:skip_side_effects]
 
-    reject_follows!
-    undo_follows!
-    purge_user!
-    purge_profile!
+    distribute_activities!
     purge_content!
     fulfill_deletion_request!
   end
 
   private
 
-  def reject_follows!
-    return if @account.local? || !@account.activitypub? || @options[:skip_activitypub]
+  def distribute_activities!
+    return if skip_activitypub?
 
+    if @account.local?
+      delete_actor!
+    elsif @account.activitypub?
+      reject_follows!
+      undo_follows!
+    end
+  end
+
+  def reject_follows!
     # When deleting a remote account, the account obviously doesn't
     # actually become deleted on its origin server, i.e. unlike a
     # locally deleted account it continues to have access to its home
@@ -81,8 +116,6 @@ class DeleteAccountService < BaseService
   end
 
   def undo_follows!
-    return if @account.local? || !@account.activitypub? || @options[:skip_activitypub]
-
     # When deleting a remote account, the account obviously doesn't
     # actually become deleted on its origin server, but following relationships
     # are severed on our end. Therefore, make the remote server aware that the
@@ -97,7 +130,7 @@ class DeleteAccountService < BaseService
   def purge_user!
     return if !@account.local? || @account.user.nil?
 
-    if @options[:reserve_email]
+    if keep_user_record?
       @account.user.disable!
       @account.user.invites.where(uses: 0).destroy_all
     else
@@ -106,34 +139,52 @@ class DeleteAccountService < BaseService
   end
 
   def purge_content!
-    distribute_delete_actor! if @account.local? && !@options[:skip_side_effects]
+    purge_user!
+    purge_profile!
+    purge_statuses!
+    purge_media_attachments!
+    purge_polls!
+    purge_generated_notifications!
+    purge_other_associations!
 
+    @account.destroy unless keep_account_record?
+  end
+
+  def purge_statuses!
     @account.statuses.reorder(nil).find_in_batches do |statuses|
-      statuses.reject! { |status| reported_status_ids.include?(status.id) } if @options[:reserve_username]
-      BatchedRemoveStatusService.new.call(statuses, skip_side_effects: @options[:skip_side_effects])
+      statuses.reject! { |status| reported_status_ids.include?(status.id) } if keep_account_record?
+
+      BatchedRemoveStatusService.new.call(statuses, skip_side_effects: skip_side_effects?)
     end
+  end
 
+  def purge_media_attachments!
     @account.media_attachments.reorder(nil).find_each do |media_attachment|
-      next if @options[:reserve_username] && reported_status_ids.include?(media_attachment.status_id)
+      next if keep_account_record? && reported_status_ids.include?(media_attachment.status_id)
 
       media_attachment.destroy
     end
+  end
 
+  def purge_polls!
     @account.polls.reorder(nil).find_each do |poll|
-      next if @options[:reserve_username] && reported_status_ids.include?(poll.status_id)
+      next if keep_account_record? && reported_status_ids.include?(poll.status_id)
 
-      # We can safely delete the poll rather than destroy it, as any non-reported
-      # status should have been deleted already, as long as we take care of
-      # notifications.
-      Notification.where(poll: poll).delete_all
       poll.delete
     end
+  end
 
+  def purge_generated_notifications!
+    # By deleting polls and statuses without callbacks, we've left behind
+    # polymorphically associated notifications generated by this account
+
+    Notification.where(from_account: @account).in_batches.delete_all
+  end
+
+  def purge_other_associations!
     associations_for_destruction.each do |association_name|
-      destroy_all(@account.public_send(association_name))
+      purge_association(association_name)
     end
-
-    @account.destroy unless @options[:reserve_username]
   end
 
   def purge_profile!
@@ -141,7 +192,7 @@ class DeleteAccountService < BaseService
     # there is no point wasting time updating
     # its values first
 
-    return unless @options[:reserve_username]
+    return unless keep_account_record?
 
     @account.silenced_at       = nil
     @account.suspended_at      = @options[:suspended_at] || Time.now.utc
@@ -156,6 +207,7 @@ class DeleteAccountService < BaseService
     @account.followers_count   = 0
     @account.following_count   = 0
     @account.moved_to_account  = nil
+    @account.also_known_as     = []
     @account.trust_level       = :untrusted
     @account.avatar.destroy
     @account.header.destroy
@@ -166,11 +218,17 @@ class DeleteAccountService < BaseService
     @account.deletion_request&.destroy
   end
 
-  def destroy_all(association)
-    association.in_batches.destroy_all
+  def purge_association(association_name)
+    association = @account.public_send(association_name)
+
+    if ASSOCIATIONS_WITHOUT_SIDE_EFFECTS.include?(association_name)
+      association.in_batches.delete_all
+    else
+      association.in_batches.destroy_all
+    end
   end
 
-  def distribute_delete_actor!
+  def delete_actor!
     ActivityPub::DeliveryWorker.push_bulk(delivery_inboxes) do |inbox_url|
       [delete_actor_json, @account.id, inbox_url]
     end
@@ -197,10 +255,26 @@ class DeleteAccountService < BaseService
   end
 
   def associations_for_destruction
-    if @options[:reserve_username]
+    if keep_account_record?
       ASSOCIATIONS_ON_SUSPEND
     else
       ASSOCIATIONS_ON_SUSPEND + ASSOCIATIONS_ON_DESTROY
     end
   end
+
+  def keep_user_record?
+    @options[:reserve_email]
+  end
+
+  def keep_account_record?
+    @options[:reserve_username]
+  end
+
+  def skip_side_effects?
+    @options[:skip_side_effects]
+  end
+
+  def skip_activitypub?
+    @options[:skip_activitypub]
+  end
 end