Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 2 additions & 39 deletions app/jobs/cron/purge_soft_deleted_blobs_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,51 +3,14 @@
class Cron::PurgeSoftDeletedBlobsJob < Cron::CronJob
self.schedule_expression = "every day at 01:00" # after PurgeUnattachedBlobsJob (00:30)

# Swift bulk delete accepts up to 10_000 objects per request; stay well under.
BULK_DELETE_LIMIT = 1000

def perform
return if ENV['PURGE_LATER_DELAY_IN_DAY'].blank?
return if ActiveStorage::Blob.service.name != :openstack

retention = Integer(ENV['PURGE_LATER_DELAY_IN_DAY']).days

ActiveStorage::Blob
.where(service_name: :openstack, soft_delete_at: ..retention.ago)
.in_batches(of: BULK_DELETE_LIMIT) { purge_batch(it.ids) }
end

private

# For a batch of expired blobs
# - add all the related variant blobs
# - delete all the corresponding files on the bucket
# - delete in db attachment / variant / blob
def purge_batch(parent_ids)
variant_record_ids = ActiveStorage::VariantRecord.where(blob_id: parent_ids).ids
image_attachments = ActiveStorage::Attachment
.where(record_type: 'ActiveStorage::VariantRecord', record_id: variant_record_ids)
blob_ids = image_attachments.pluck(:blob_id) + parent_ids

delete_files(blob_ids)

# delete_all bypasses callbacks, so we drop the rows ourselves, in FK order:
# image attachments -> variant records -> blobs (variants + parents).
image_attachments.delete_all
ActiveStorage::VariantRecord.where(id: variant_record_ids).delete_all
ActiveStorage::Blob.where(id: blob_ids).delete_all
end

def delete_files(blob_ids)
keys = ActiveStorage::Blob.where(id: blob_ids).pluck(:key)

service = ActiveStorage::Blob.service
client = service.send(:client)

keys.each_slice(BULK_DELETE_LIMIT) do |slice|
response = client.delete_multiple_objects(service.container, slice)
errors = response.body['Errors']
Sentry.capture_message("Bulk delete errors", extra: { errors: }) if errors.present?
end
.where(service_name: :openstack, soft_deleted_at: ..retention.ago)
.in_batches(of: BlobService::BULK_DELETE_LIMIT) { BlobService.purge_blobs_with_variants(it.ids) }
end
end
2 changes: 1 addition & 1 deletion app/jobs/cron/purge_unattached_blobs_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def perform
# caveats: the job needs to be run at least once a week to avoid missing blobs
ActiveStorage::Blob
.where(created_at: 1.week.ago..1.day.ago)
.where(soft_delete_at: nil)
.where(soft_deleted_at: nil)
.unattached
.select(:id, :service_name)
.in_batches do |relation|
Expand Down
2 changes: 1 addition & 1 deletion app/jobs/delayed_purge_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def soft_delete
return if blob.attachments.exists?
return if !expire_file(blob)

blob.update_column(:soft_delete_at, Time.current)
blob.update_column(:soft_deleted_at, Time.current)
soft_delete_variants if blob.image?
end

Expand Down
45 changes: 45 additions & 0 deletions app/services/blob_service.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# frozen_string_literal: true

class BlobService
# Swift bulk delete accepts up to 10_000 objects per request; stay well under.
BULK_DELETE_LIMIT = 1000

class << self
# For a batch of expired blobs
# - keep only the openstack ones (never touch rows/files from another service)
# - add all the related variant blobs
# - delete the corresponding files on the openstack bucket
# - delete in db attachment / variant / blob
def purge_blobs_with_variants(parent_ids)
parent_ids = ActiveStorage::Blob.where(id: parent_ids, service_name: :openstack).ids

variant_record_ids = ActiveStorage::VariantRecord.where(blob_id: parent_ids).ids
variant_attachments = ActiveStorage::Attachment
.where(record_type: 'ActiveStorage::VariantRecord', record_id: variant_record_ids)
blob_ids = variant_attachments.pluck(:blob_id) + parent_ids

delete_files(blob_ids)

# delete_all bypasses callbacks, so we drop the rows ourselves, in FK order:
# image attachments -> variant records -> blobs (variants + parents).
variant_attachments.delete_all
ActiveStorage::VariantRecord.where(id: variant_record_ids).delete_all
ActiveStorage::Blob.where(id: blob_ids).delete_all
end

private

def delete_files(blob_ids)
keys = ActiveStorage::Blob.where(id: blob_ids).pluck(:key)

service = ActiveStorage::Blob.service
client = service.send(:client)

keys.each_slice(BULK_DELETE_LIMIT) do |slice|
response = client.delete_multiple_objects(service.container, slice)
errors = response.body['Errors']
Sentry.capture_message("Bulk delete errors", extra: { errors: }) if errors.present?
end
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# frozen_string_literal: true

class RenameSoftDeleteAtToSoftDeletedAtOnActiveStorageBlobs < ActiveRecord::Migration[8.0]
def change
safety_assured do
rename_column :active_storage_blobs, :soft_delete_at, :soft_deleted_at
end
end
end
6 changes: 3 additions & 3 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[8.0].define(version: 2026_07_03_000000) do
ActiveRecord::Schema[8.0].define(version: 2026_07_13_000000) do
# These are extensions that must be enabled in order to support this database
enable_extension "pg_buffercache"
enable_extension "pg_stat_statements"
Expand Down Expand Up @@ -52,12 +52,12 @@
t.text "metadata"
t.jsonb "ocr"
t.string "service_name", null: false
t.datetime "soft_delete_at"
t.datetime "soft_deleted_at"
t.string "virus_scan_result"
t.datetime "virus_scanned_at", precision: nil
t.datetime "watermarked_at", precision: nil
t.index ["key"], name: "index_active_storage_blobs_on_key", unique: true
t.index ["soft_delete_at"], name: "index_active_storage_blobs_on_soft_delete_at", where: "(soft_delete_at IS NOT NULL)"
t.index ["soft_deleted_at"], name: "index_active_storage_blobs_on_soft_deleted_at", where: "(soft_deleted_at IS NOT NULL)"
t.index ["virus_scan_result", "id"], name: "index_active_storage_blobs_on_pending_virus_scan", order: { id: :desc }
end

Expand Down
43 changes: 3 additions & 40 deletions spec/jobs/cron/purge_soft_deleted_blobs_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@
# retention = PURGE_LATER_DELAY_IN_DAY = 1.day
let(:blob_old) do
ActiveStorage::Blob.create_and_upload!(io: StringIO.new("old"), filename: "old.txt", content_type: "text/plain").tap do |blob|
blob.update_columns(soft_delete_at: 2.days.ago, service_name: 'openstack')
blob.update_columns(soft_deleted_at: 2.days.ago, service_name: 'openstack')
end
end

let(:blob_recent) do
ActiveStorage::Blob.create_and_upload!(io: StringIO.new("recent"), filename: "recent.txt", content_type: "text/plain").tap do |blob|
blob.update_column(:soft_delete_at, 1.hour.ago)
blob.update_column(:soft_deleted_at, 1.hour.ago)
end
end

Expand All @@ -55,7 +55,7 @@
it 'keeps blobs stored on another service' do
blob_other_service = ActiveStorage::Blob
.create_and_upload!(io: StringIO.new("other"), filename: "other.txt", content_type: "text/plain")
.tap { it.update_columns(soft_delete_at: 2.days.ago, service_name: 'test') }
.tap { it.update_columns(soft_deleted_at: 2.days.ago, service_name: 'test') }

perform

Expand All @@ -67,42 +67,5 @@

expect { described_class.perform_now }.not_to change(ActiveStorage::Blob, :count)
end

it 'bulk-deletes the parent file (safety net against a missed X-Delete-At)' do
expect(client).to receive(:delete_multiple_objects)
.with('bucket', [blob_old.key])
.and_return(double(body: { 'Errors' => [] }))

perform
end

context 'when a soft-deleted image has variants' do
let!(:variant_blob) do
ActiveStorage::Blob.create_and_upload!(io: StringIO.new("variant"), filename: "v.png", content_type: "image/png")
end
let!(:variant_record) { ActiveStorage::VariantRecord.create!(blob: blob_old, variation_digest: "digest") }
let!(:image_attachment) { ActiveStorage::Attachment.create!(name: "image", record: variant_record, blob: variant_blob) }

it 'bulk-deletes both variant and parent files, then drops every row' do
expect(client).to receive(:delete_multiple_objects)
.with('bucket', match_array([variant_blob.key, blob_old.key]))
.and_return(double(body: { 'Errors' => [] }))

perform

expect(ActiveStorage::Blob.where(id: [blob_old.id, variant_blob.id])).not_to exist
expect(ActiveStorage::VariantRecord.where(id: variant_record.id)).not_to exist
expect(ActiveStorage::Attachment.where(id: image_attachment.id)).not_to exist
end

it 'reports per-object bulk delete errors to Sentry' do
errors = [["bucket/#{variant_blob.key}", "409 Conflict"]]
allow(client).to receive(:delete_multiple_objects).and_return(double(body: { 'Errors' => errors }))

expect(Sentry).to receive(:capture_message).with("Bulk delete errors", extra: { errors: })

perform
end
end
end
end
2 changes: 1 addition & 1 deletion spec/jobs/cron/purge_unattached_blobs_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
end

context 'when the blob has already been soft-deleted' do
before { blob.update_column(:soft_delete_at, 1.hour.ago) }
before { blob.update_column(:soft_deleted_at, 1.hour.ago) }

it 'does not enqueue a purge' do
expect { subject }.not_to have_enqueued_job(DelayedPurgeJob)
Expand Down
6 changes: 3 additions & 3 deletions spec/jobs/delayed_purge_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,16 @@
.and_return(double(status: 201))
subject
perform_enqueued_jobs
expect(blob.reload.soft_delete_at).to be_present
expect(blob.reload.soft_deleted_at).to be_present
end

it 'does not mark soft_delete_at when the copy fails' do
it 'does not mark soft_deleted_at when the copy fails' do
dossier.champ_data.first.piece_justificative_file.first.delete
expect(client).to receive(:copy_object).and_return(double(status: 500))
expect(Sentry).to receive(:capture_message)
subject
perform_enqueued_jobs
expect(blob.reload.soft_delete_at).to be_nil
expect(blob.reload.soft_deleted_at).to be_nil
end

it 'with cloned dossier' do
Expand Down
80 changes: 80 additions & 0 deletions spec/services/blob_service_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
# frozen_string_literal: true

RSpec.describe BlobService do
describe '.purge_blobs_with_variants' do
# Blobs are created against the real (disk) service, then flipped to openstack
# and purged against a mocked openstack service — installed by the subject,
# once every blob exists.
subject(:purge_blobs_with_variants) do
service = double('openstack service', container: 'bucket', name: :openstack)
allow(service).to receive(:client).and_return(client) # reached via service.send(:client)
allow(ActiveStorage::Blob).to receive(:service).and_return(service)
BlobService.purge_blobs_with_variants([blob.id])
end

let(:client) { double('openstack client') }

let!(:blob) do
ActiveStorage::Blob.create_and_upload!(io: StringIO.new("old"), filename: "old.txt", content_type: "text/plain")
.tap { it.update_column(:service_name, 'openstack') }
end

before do
allow(client).to receive(:delete_multiple_objects).and_return(double(body: { 'Errors' => [] }))
end

it 'drops the blob row' do
expect { purge_blobs_with_variants }.to change { ActiveStorage::Blob.exists?(id: blob.id) }.from(true).to(false)
end

it 'bulk-deletes the parent file (safety net against a missed X-Delete-At)' do
expect(client).to receive(:delete_multiple_objects)
.with('bucket', [blob.key])
.and_return(double(body: { 'Errors' => [] }))

purge_blobs_with_variants
end

context 'when the blob is an image with variants' do
let!(:variant_blob) do
ActiveStorage::Blob.create_and_upload!(io: StringIO.new("variant"), filename: "v.png", content_type: "image/png")
.tap { it.update_column(:service_name, 'openstack') }
end
let!(:variant_record) { ActiveStorage::VariantRecord.create!(blob:, variation_digest: "digest") }
let!(:image_attachment) { ActiveStorage::Attachment.create!(name: "image", record: variant_record, blob: variant_blob) }

it 'bulk-deletes both variant and parent files, then drops every row' do
expect(client).to receive(:delete_multiple_objects)
.with('bucket', match_array([variant_blob.key, blob.key]))
.and_return(double(body: { 'Errors' => [] }))

purge_blobs_with_variants

expect(ActiveStorage::Blob.where(id: [blob.id, variant_blob.id])).not_to exist
expect(ActiveStorage::VariantRecord.where(id: variant_record.id)).not_to exist
expect(ActiveStorage::Attachment.where(id: image_attachment.id)).not_to exist
end

it 'reports per-object bulk delete errors to Sentry' do
errors = [["bucket/#{variant_blob.key}", "409 Conflict"]]
allow(client).to receive(:delete_multiple_objects).and_return(double(body: { 'Errors' => errors }))

expect(Sentry).to receive(:capture_message).with("Bulk delete errors", extra: { errors: })

purge_blobs_with_variants
end
end

context 'when the blob is stored on another service' do
let!(:blob) do
ActiveStorage::Blob.create_and_upload!(io: StringIO.new("other"), filename: "other.txt", content_type: "text/plain")
end

it 'leaves its file and row untouched' do
expect(client).not_to receive(:delete_multiple_objects)

expect { purge_blobs_with_variants }.not_to change { ActiveStorage::Blob.exists?(id: blob.id) }
end
end
end
end
Loading