Fix account destroy cascade gaps
Account incineration could leave orphaned records due to missing cascade declarations and async jobs failing when the account was already gone. Cascade fixes: - Add Account::Searchable concern to clean up Search::Query (delete_all) and Search::Record (destroy_all, respects SQLite FTS dependent: :destroy) - Add before_destroy in Account::Storage to delete storage entries - Suppress storage entry recording during incineration so attachment purge callbacks don't create entries or enqueue materialize jobs - Guard Access#clean_inaccessible_data_later with unless user.destroyed? to avoid enqueuing pointless jobs during user cascade Job tenancy: - Extract AccountTenanted concern from the global ActiveJob initializer into ApplicationJob (include) with targeted prepends for ActionMailer::MailDeliveryJob and Turbo broadcast jobs - Defer account resolution from deserialize to perform so that missing accounts raise DeserializationError inside the execution path where discard_on can handle it Tests: - Comprehensive incineration test covering 35 model types with before/after assertions and full enqueued job processing - Mailer deliver_later test verifying account context survives job serialization for multi-account users - Turbo broadcast test verifying account-scoped URLs in rendered partials
This commit is contained in:
@@ -1,4 +1,6 @@
|
||||
class ApplicationJob < ActiveJob::Base
|
||||
prepend AccountTenanted
|
||||
|
||||
# Automatically retry jobs that encountered a deadlock
|
||||
# retry_on ActiveRecord::Deadlocked
|
||||
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
# Serializes the current account into job data so that jobs run
|
||||
# within the correct account context via Current.with_account.
|
||||
#
|
||||
# Account resolution is deferred to an around_perform callback so
|
||||
# that missing accounts raise DeserializationError inside the
|
||||
# execution path where discard_on can handle it.
|
||||
module AccountTenanted
|
||||
extend ActiveSupport::Concern
|
||||
|
||||
prepended do
|
||||
attr_reader :account
|
||||
around_perform :with_account_context
|
||||
end
|
||||
|
||||
def initialize(...)
|
||||
super
|
||||
@account = Current.account
|
||||
end
|
||||
|
||||
def serialize
|
||||
super.merge({ "account" => @account&.to_gid })
|
||||
end
|
||||
|
||||
def deserialize(job_data)
|
||||
super
|
||||
@account_gid = job_data["account"]
|
||||
end
|
||||
|
||||
private
|
||||
def with_account_context(&block)
|
||||
resolve_account!
|
||||
|
||||
if account.present?
|
||||
Current.with_account(account, &block)
|
||||
else
|
||||
yield
|
||||
end
|
||||
end
|
||||
|
||||
def resolve_account!
|
||||
if @account_gid
|
||||
@account = GlobalID::Locator.locate(@account_gid)
|
||||
end
|
||||
rescue ActiveRecord::RecordNotFound
|
||||
raise ActiveJob::DeserializationError
|
||||
end
|
||||
end
|
||||
@@ -19,6 +19,6 @@ class Access < ApplicationRecord
|
||||
end
|
||||
|
||||
def clean_inaccessible_data_later
|
||||
Board::CleanInaccessibleDataJob.perform_later(user, board)
|
||||
Board::CleanInaccessibleDataJob.perform_later(user, board) unless user.destroyed?
|
||||
end
|
||||
end
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
class Account < ApplicationRecord
|
||||
include Account::Storage, Cancellable, Entropic, Incineratable, MultiTenantable, Seedeable
|
||||
include Account::Storage, Cancellable, Entropic, Incineratable, MultiTenantable, Searchable, Seedeable
|
||||
|
||||
has_one :join_code, dependent: :destroy
|
||||
has_many :users, dependent: :destroy
|
||||
|
||||
@@ -4,7 +4,7 @@ class Account::Import < ApplicationRecord
|
||||
belongs_to :account
|
||||
belongs_to :identity
|
||||
|
||||
has_one_attached :file
|
||||
has_one_attached :file, dependent: :purge_later
|
||||
|
||||
enum :status, %w[ pending processing completed failed ].index_by(&:itself), default: :pending
|
||||
enum :failure_reason, %w[ conflict invalid_export ].index_by(&:itself), prefix: :failed_due_to, scopes: false
|
||||
|
||||
@@ -11,7 +11,7 @@ module Account::Incineratable
|
||||
|
||||
def incinerate
|
||||
run_callbacks :incinerate do
|
||||
account.destroy
|
||||
Storage::Entry.suppressing_recording { account.destroy }
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -0,0 +1,14 @@
|
||||
module Account::Searchable
|
||||
extend ActiveSupport::Concern
|
||||
|
||||
included do
|
||||
has_many :search_queries, class_name: "Search::Query", dependent: :delete_all
|
||||
|
||||
before_destroy :clear_search_records
|
||||
end
|
||||
|
||||
private
|
||||
def clear_search_records
|
||||
Search::Record.for(id).destroy_all
|
||||
end
|
||||
end
|
||||
@@ -2,7 +2,15 @@ module Account::Storage
|
||||
extend ActiveSupport::Concern
|
||||
include Storage::Totaled
|
||||
|
||||
included do
|
||||
before_destroy :clear_storage_entries
|
||||
end
|
||||
|
||||
private
|
||||
def clear_storage_entries
|
||||
Storage::Entry.where(account_id: id).delete_all
|
||||
end
|
||||
|
||||
def calculate_real_storage_bytes
|
||||
boards.sum { |board| board.send(:calculate_real_storage_bytes) }
|
||||
end
|
||||
|
||||
@@ -2,7 +2,7 @@ class Export < ApplicationRecord
|
||||
belongs_to :account
|
||||
belongs_to :user
|
||||
|
||||
has_one_attached :file
|
||||
has_one_attached :file, dependent: :purge_later
|
||||
|
||||
enum :status, %w[ pending processing completed failed ].index_by(&:itself), default: :pending
|
||||
|
||||
|
||||
@@ -9,7 +9,7 @@ class Identity < ApplicationRecord
|
||||
has_many :users, dependent: :nullify
|
||||
has_many :accounts, through: :users
|
||||
|
||||
has_one_attached :avatar
|
||||
has_one_attached :avatar, dependent: :purge_later
|
||||
|
||||
before_destroy :deactivate_users, prepend: true
|
||||
|
||||
|
||||
@@ -5,12 +5,22 @@ class Storage::Entry < ApplicationRecord
|
||||
|
||||
scope :pending, ->(last_entry_id) { where.not(id: ..last_entry_id) if last_entry_id }
|
||||
|
||||
thread_mattr_accessor :recording, default: true
|
||||
|
||||
def self.suppressing_recording(&block)
|
||||
original, self.recording = self.recording, false
|
||||
yield
|
||||
ensure
|
||||
self.recording = original
|
||||
end
|
||||
|
||||
# Records may be destroyed (during cascading deletes) but .id still works.
|
||||
# Skip entirely if account is destroyed - no need to track storage for deleted accounts.
|
||||
# Skip materialize jobs for destroyed boards since there's nothing to update.
|
||||
def self.record(delta:, operation:, account:, board: nil, recordable: nil, blob: nil)
|
||||
return if delta.zero?
|
||||
return if account.destroyed?
|
||||
return unless recording
|
||||
|
||||
entry = create! \
|
||||
account_id: account.id,
|
||||
|
||||
@@ -11,7 +11,7 @@ module User::Avatar
|
||||
].freeze
|
||||
|
||||
included do
|
||||
has_one_attached :avatar do |attachable|
|
||||
has_one_attached :avatar, dependent: :purge_later do |attachable|
|
||||
attachable.variant :thumb, resize_to_fill: [ 256, 256 ], process: :immediately
|
||||
end
|
||||
|
||||
|
||||
Reference in New Issue
Block a user