Fix export size doubling from re-exporting prior export blobs (#2707)

* Exclude export/import blobs and attachments from account exports

Export and import file blobs were being assigned the account's
account_id (via the Current.account default), causing subsequent
exports to include previous export/import ZIP files in the data.
This roughly doubled the export size each time and made imports
fail with an IntegrityError on unrecognized "Account::Export"
record type.

Filter out blobs attached to Export/Import records in
BlobRecordSet, FileRecordSet, and a new AttachmentRecordSet.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Use STI base type for export blob exclusion filter

Rails polymorphic_name returns base_class.name for STI models,
so ActiveStorage stores record_type as "Export" rather than the
concrete subclass names. Filter on the base type accordingly.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Fix export filtering for mixed-use Active Storage blobs

Exclude only internal-only blobs for account export metadata/file sets, scope attachment subqueries by account, and add regression coverage for blobs shared between user records and export attachments.

* Simplify export blob filtering

Exports always create fresh blobs, so a blob can never be legitimately
shared between an Export/Import and a real account record. Replace the
internal_only/external split with a single excluded_blob_ids subquery
and drop the contrived shared-blob test.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Daniel Pinto
2026-03-18 15:41:22 +00:00
committed by GitHub
parent e0d66b2fd0
commit ad1b10368d
6 changed files with 57 additions and 2 deletions
@@ -0,0 +1,11 @@
class Account::DataTransfer::ActiveStorage::AttachmentRecordSet < Account::DataTransfer::RecordSet
def initialize(account)
super(account: account, model: ::ActiveStorage::Attachment)
end
private
def records
::ActiveStorage::Attachment.where(account: account)
.where.not(record_type: INTERNAL_RECORD_TYPES)
end
end
@@ -8,6 +8,14 @@ class Account::DataTransfer::ActiveStorage::BlobRecordSet < Account::DataTransfe
end
private
def records
::ActiveStorage::Blob.where(account: account).where.not(id: excluded_blob_ids)
end
def excluded_blob_ids
::ActiveStorage::Attachment.where(account: account, record_type: INTERNAL_RECORD_TYPES).select(:blob_id)
end
def import_batch(files)
batch_data = files.map do |file|
data = load(file)
@@ -5,7 +5,11 @@ class Account::DataTransfer::ActiveStorage::FileRecordSet < Account::DataTransfe
private
def records
::ActiveStorage::Blob.where(account: account)
::ActiveStorage::Blob.where(account: account).where.not(id: excluded_blob_ids)
end
def excluded_blob_ids
::ActiveStorage::Attachment.where(account: account, record_type: INTERNAL_RECORD_TYPES).select(:blob_id)
end
def export_record(blob)
+1 -1
View File
@@ -58,7 +58,7 @@ class Account::DataTransfer::Manifest
::Webhook::Delivery
),
Account::DataTransfer::ActiveStorage::BlobRecordSet.new(account),
*build_record_sets(::ActiveStorage::Attachment),
Account::DataTransfer::ActiveStorage::AttachmentRecordSet.new(account),
Account::DataTransfer::ActionText::RichTextRecordSet.new(account),
Account::DataTransfer::ActiveStorage::FileRecordSet.new(account)
].then { set_importable_model_names(it) }
@@ -3,6 +3,7 @@ class Account::DataTransfer::RecordSet
class ConflictError < IntegrityError; end
IMPORT_BATCH_SIZE = 100
INTERNAL_RECORD_TYPES = %w[Export Account::Import].freeze
attr_accessor :importable_model_names
attr_reader :account, :model, :attributes
+31
View File
@@ -60,6 +60,37 @@ class Account::ExportTest < ActiveSupport::TestCase
end
end
test "export excludes blobs and attachments from previous exports" do
first_export = Account::Export.create!(account: Current.account, user: users(:david))
first_export.build
assert first_export.completed?
first_export_blob = first_export.file.blob
first_export_attachment = first_export.file.attachment
second_export = Account::Export.create!(account: Current.account, user: users(:david))
second_export.build
assert second_export.completed?
second_export.file.open do |file|
reader = ZipKit::FileReader.read_zip_structure(io: file)
filenames = reader.map(&:filename)
blob_entries = filenames.select { |f| f.start_with?("data/active_storage_blobs/") }
blob_ids = blob_entries.map { |f| File.basename(f, ".json") }
assert_not_includes blob_ids, first_export_blob.id, "Export should not include blob metadata from previous exports"
storage_entries = filenames.select { |f| f.start_with?("storage/") }
assert_not storage_entries.any? { |f| f == "storage/#{first_export_blob.key}" },
"Export should not include blob file from previous exports"
attachment_entries = filenames.select { |f| f.start_with?("data/active_storage_attachments/") }
attachment_ids = attachment_entries.map { |f| File.basename(f, ".json") }
assert_not_includes attachment_ids, first_export_attachment.id,
"Export should not include attachment records from previous exports"
end
end
test "build succeeds when rich text references missing blob" do
blob = ActiveStorage::Blob.create_and_upload!(
io: file_fixture("moon.jpg").open,