feat(soft_delete): soft-delete variants alongside the parent blob

Author: LeSim

app/jobs/delayed_purge_job.rb

@@ -52,14 +52,29 @@ def soft_delete
     # We should replicate the same behavior here.
     # https://github.com/rails/rails/blob/ef88965e8a0c72496c210a5a0a48b85ec9a2ed17/activestorage/app/models/active_storage/blob.rb#L53-L55
     return if blob.attachments.exists?
+    return if !expire_file(blob)
+
+    blob.update_column(:soft_delete_at, Time.current)
+    soft_delete_variants if blob.image?
+  end
+
+  def soft_delete_variants
+    variant_blob_ids = ActiveStorage::Attachment
+      .where(record_type: 'ActiveStorage::VariantRecord', record_id: blob.variant_records.ids)
+      .pluck(:blob_id)
+
+    ActiveStorage::Blob.where(id: variant_blob_ids).find_each { expire_file(it) }
+  end
+
+  # Copy the blob's file onto itself with an X-Delete-At header so Swift expires
+  # it after the retention delay. Returns whether Swift accepted the request and
+  # reports to Sentry on failure.
+  def expire_file(blob)
     headers = { "Content-Type" => blob.content_type, 'X-Delete-At' => delay.to_s }
-    excon_response = client.copy_object(container, key, container, key, headers)
-    if excon_response.status != 201
-      Sentry.capture_message("Can't expire blob", extra: { key:, headers: })
-    else
-      blob.update_column(:soft_delete_at, Time.current)
-      service.delete_prefixed("variants/#{key}/") if blob.image?
-    end
+    return true if client.copy_object(container, blob.key, container, blob.key, headers).status == 201
+
+    Sentry.capture_message("Can't expire blob", extra: { key: blob.key, headers: })
+    false
   end
 
   def soft_delete_enabled?

spec/jobs/delayed_purge_job_spec.rb

@@ -65,6 +65,54 @@
     end
   end
 
+  context 'when a soft-deleted image has variants' do
+    let(:container) { "bucket" }
+    let(:client) { double("client") }
+
+    # Blobs are created against the real service, then the OpenStack service is
+    # mocked; the parent copy_object is stubbed to succeed.
+    def setup_image_with_variant
+      image_blob = ActiveStorage::Blob.create_and_upload!(io: StringIO.new("img"), filename: "i.png", content_type: "image/png")
+      variant_blob = ActiveStorage::Blob.create_and_upload!(io: StringIO.new("variant"), filename: "v.png", content_type: "image/png")
+      variant_record = ActiveStorage::VariantRecord.create!(blob: image_blob, variation_digest: "digest")
+      image_attachment = ActiveStorage::Attachment.create!(name: "image", record: variant_record, blob: variant_blob)
+
+      allow_any_instance_of(ActiveStorage::Blob).to receive(:service).and_return(double(name: :openstack, container:))
+      job = described_class.new(image_blob)
+      allow(job).to receive(:client).and_return(client)
+      allow(client).to receive(:copy_object).and_return(double(status: 201))
+
+      { job:, image_blob:, variant_blob:, variant_record:, image_attachment: }
+    end
+
+    it 'marks the variant files for expiration and keeps their rows for the cron' do
+      ctx = setup_image_with_variant
+      expect(client).to receive(:copy_object)
+        .with(container, ctx[:variant_blob].key, container, ctx[:variant_blob].key, hash_including('X-Delete-At'))
+        .and_return(double(status: 201))
+
+      ctx[:job].perform_now
+
+      expect(ActiveStorage::Blob.where(id: ctx[:variant_blob].id)).to exist
+      expect(ActiveStorage::VariantRecord.where(id: ctx[:variant_record].id)).to exist
+      expect(ActiveStorage::Attachment.where(id: ctx[:image_attachment].id)).to exist
+      expect(ActiveStorage::Blob.where(id: ctx[:image_blob].id)).to exist # parent soft-deleted
+    end
+
+    it 'reports to Sentry when a variant copy fails, keeping the rows' do
+      ctx = setup_image_with_variant
+      allow(client).to receive(:copy_object)
+        .with(container, ctx[:variant_blob].key, container, ctx[:variant_blob].key, anything)
+        .and_return(double(status: 500))
+      expect(Sentry).to receive(:capture_message).with("Can't expire blob", extra: hash_including(key: ctx[:variant_blob].key))
+
+      ctx[:job].perform_now
+
+      expect(ActiveStorage::VariantRecord.where(id: ctx[:variant_record].id)).to exist
+      expect(ActiveStorage::Blob.where(id: ctx[:variant_blob].id)).to exist
+    end
+  end
+
   context 'when destroying an instance' do
     it 'uses our custom job' do
       expect { dossier.destroy }.to have_enqueued_job(DelayedPurgeJob)