Skip to content

Commit 890af3c

Browse files
authored
Merge pull request #13478 from demarche-numerique/rename_soft_deleted
Tech: renomme `soft_deleted_at` et bouge la purge des blobs et variants dans un service
2 parents f0db321 + e3187f1 commit 890af3c

10 files changed

Lines changed: 148 additions & 88 deletions

app/jobs/cron/purge_soft_deleted_blobs_job.rb

Lines changed: 2 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -3,51 +3,14 @@
33
class Cron::PurgeSoftDeletedBlobsJob < Cron::CronJob
44
self.schedule_expression = "every day at 01:00" # after PurgeUnattachedBlobsJob (00:30)
55

6-
# Swift bulk delete accepts up to 10_000 objects per request; stay well under.
7-
BULK_DELETE_LIMIT = 1000
8-
96
def perform
107
return if ENV['PURGE_LATER_DELAY_IN_DAY'].blank?
118
return if ActiveStorage::Blob.service.name != :openstack
129

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

1512
ActiveStorage::Blob
16-
.where(service_name: :openstack, soft_delete_at: ..retention.ago)
17-
.in_batches(of: BULK_DELETE_LIMIT) { purge_batch(it.ids) }
18-
end
19-
20-
private
21-
22-
# For a batch of expired blobs
23-
# - add all the related variant blobs
24-
# - delete all the corresponding files on the bucket
25-
# - delete in db attachment / variant / blob
26-
def purge_batch(parent_ids)
27-
variant_record_ids = ActiveStorage::VariantRecord.where(blob_id: parent_ids).ids
28-
image_attachments = ActiveStorage::Attachment
29-
.where(record_type: 'ActiveStorage::VariantRecord', record_id: variant_record_ids)
30-
blob_ids = image_attachments.pluck(:blob_id) + parent_ids
31-
32-
delete_files(blob_ids)
33-
34-
# delete_all bypasses callbacks, so we drop the rows ourselves, in FK order:
35-
# image attachments -> variant records -> blobs (variants + parents).
36-
image_attachments.delete_all
37-
ActiveStorage::VariantRecord.where(id: variant_record_ids).delete_all
38-
ActiveStorage::Blob.where(id: blob_ids).delete_all
39-
end
40-
41-
def delete_files(blob_ids)
42-
keys = ActiveStorage::Blob.where(id: blob_ids).pluck(:key)
43-
44-
service = ActiveStorage::Blob.service
45-
client = service.send(:client)
46-
47-
keys.each_slice(BULK_DELETE_LIMIT) do |slice|
48-
response = client.delete_multiple_objects(service.container, slice)
49-
errors = response.body['Errors']
50-
Sentry.capture_message("Bulk delete errors", extra: { errors: }) if errors.present?
51-
end
13+
.where(service_name: :openstack, soft_deleted_at: ..retention.ago)
14+
.in_batches(of: BlobService::BULK_DELETE_LIMIT) { BlobService.purge_blobs_with_variants(it.ids) }
5215
end
5316
end

app/jobs/cron/purge_unattached_blobs_job.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ def perform
1616
# caveats: the job needs to be run at least once a week to avoid missing blobs
1717
ActiveStorage::Blob
1818
.where(created_at: 1.week.ago..1.day.ago)
19-
.where(soft_delete_at: nil)
19+
.where(soft_deleted_at: nil)
2020
.unattached
2121
.select(:id, :service_name)
2222
.in_batches do |relation|

app/jobs/delayed_purge_job.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ def soft_delete
5454
return if blob.attachments.exists?
5555
return if !expire_file(blob)
5656

57-
blob.update_column(:soft_delete_at, Time.current)
57+
blob.update_column(:soft_deleted_at, Time.current)
5858
soft_delete_variants if blob.image?
5959
end
6060

app/services/blob_service.rb

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
# frozen_string_literal: true
2+
3+
class BlobService
4+
# Swift bulk delete accepts up to 10_000 objects per request; stay well under.
5+
BULK_DELETE_LIMIT = 1000
6+
7+
class << self
8+
# For a batch of expired blobs
9+
# - keep only the openstack ones (never touch rows/files from another service)
10+
# - add all the related variant blobs
11+
# - delete the corresponding files on the openstack bucket
12+
# - delete in db attachment / variant / blob
13+
def purge_blobs_with_variants(parent_ids)
14+
parent_ids = ActiveStorage::Blob.where(id: parent_ids, service_name: :openstack).ids
15+
16+
variant_record_ids = ActiveStorage::VariantRecord.where(blob_id: parent_ids).ids
17+
variant_attachments = ActiveStorage::Attachment
18+
.where(record_type: 'ActiveStorage::VariantRecord', record_id: variant_record_ids)
19+
blob_ids = variant_attachments.pluck(:blob_id) + parent_ids
20+
21+
delete_files(blob_ids)
22+
23+
# delete_all bypasses callbacks, so we drop the rows ourselves, in FK order:
24+
# image attachments -> variant records -> blobs (variants + parents).
25+
variant_attachments.delete_all
26+
ActiveStorage::VariantRecord.where(id: variant_record_ids).delete_all
27+
ActiveStorage::Blob.where(id: blob_ids).delete_all
28+
end
29+
30+
private
31+
32+
def delete_files(blob_ids)
33+
keys = ActiveStorage::Blob.where(id: blob_ids).pluck(:key)
34+
35+
service = ActiveStorage::Blob.service
36+
client = service.send(:client)
37+
38+
keys.each_slice(BULK_DELETE_LIMIT) do |slice|
39+
response = client.delete_multiple_objects(service.container, slice)
40+
errors = response.body['Errors']
41+
Sentry.capture_message("Bulk delete errors", extra: { errors: }) if errors.present?
42+
end
43+
end
44+
end
45+
end
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
# frozen_string_literal: true
2+
3+
class RenameSoftDeleteAtToSoftDeletedAtOnActiveStorageBlobs < ActiveRecord::Migration[8.0]
4+
def change
5+
safety_assured do
6+
rename_column :active_storage_blobs, :soft_delete_at, :soft_deleted_at
7+
end
8+
end
9+
end

db/schema.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
#
1212
# It's strongly recommended that you check this file into your version control system.
1313

14-
ActiveRecord::Schema[8.0].define(version: 2026_07_03_000000) do
14+
ActiveRecord::Schema[8.0].define(version: 2026_07_13_000000) do
1515
# These are extensions that must be enabled in order to support this database
1616
enable_extension "pg_buffercache"
1717
enable_extension "pg_stat_statements"
@@ -52,12 +52,12 @@
5252
t.text "metadata"
5353
t.jsonb "ocr"
5454
t.string "service_name", null: false
55-
t.datetime "soft_delete_at"
55+
t.datetime "soft_deleted_at"
5656
t.string "virus_scan_result"
5757
t.datetime "virus_scanned_at", precision: nil
5858
t.datetime "watermarked_at", precision: nil
5959
t.index ["key"], name: "index_active_storage_blobs_on_key", unique: true
60-
t.index ["soft_delete_at"], name: "index_active_storage_blobs_on_soft_delete_at", where: "(soft_delete_at IS NOT NULL)"
60+
t.index ["soft_deleted_at"], name: "index_active_storage_blobs_on_soft_deleted_at", where: "(soft_deleted_at IS NOT NULL)"
6161
t.index ["virus_scan_result", "id"], name: "index_active_storage_blobs_on_pending_virus_scan", order: { id: :desc }
6262
end
6363

spec/jobs/cron/purge_soft_deleted_blobs_job_spec.rb

Lines changed: 3 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,13 @@
2222
# retention = PURGE_LATER_DELAY_IN_DAY = 1.day
2323
let(:blob_old) do
2424
ActiveStorage::Blob.create_and_upload!(io: StringIO.new("old"), filename: "old.txt", content_type: "text/plain").tap do |blob|
25-
blob.update_columns(soft_delete_at: 2.days.ago, service_name: 'openstack')
25+
blob.update_columns(soft_deleted_at: 2.days.ago, service_name: 'openstack')
2626
end
2727
end
2828

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

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

6060
perform
6161

@@ -67,42 +67,5 @@
6767

6868
expect { described_class.perform_now }.not_to change(ActiveStorage::Blob, :count)
6969
end
70-
71-
it 'bulk-deletes the parent file (safety net against a missed X-Delete-At)' do
72-
expect(client).to receive(:delete_multiple_objects)
73-
.with('bucket', [blob_old.key])
74-
.and_return(double(body: { 'Errors' => [] }))
75-
76-
perform
77-
end
78-
79-
context 'when a soft-deleted image has variants' do
80-
let!(:variant_blob) do
81-
ActiveStorage::Blob.create_and_upload!(io: StringIO.new("variant"), filename: "v.png", content_type: "image/png")
82-
end
83-
let!(:variant_record) { ActiveStorage::VariantRecord.create!(blob: blob_old, variation_digest: "digest") }
84-
let!(:image_attachment) { ActiveStorage::Attachment.create!(name: "image", record: variant_record, blob: variant_blob) }
85-
86-
it 'bulk-deletes both variant and parent files, then drops every row' do
87-
expect(client).to receive(:delete_multiple_objects)
88-
.with('bucket', match_array([variant_blob.key, blob_old.key]))
89-
.and_return(double(body: { 'Errors' => [] }))
90-
91-
perform
92-
93-
expect(ActiveStorage::Blob.where(id: [blob_old.id, variant_blob.id])).not_to exist
94-
expect(ActiveStorage::VariantRecord.where(id: variant_record.id)).not_to exist
95-
expect(ActiveStorage::Attachment.where(id: image_attachment.id)).not_to exist
96-
end
97-
98-
it 'reports per-object bulk delete errors to Sentry' do
99-
errors = [["bucket/#{variant_blob.key}", "409 Conflict"]]
100-
allow(client).to receive(:delete_multiple_objects).and_return(double(body: { 'Errors' => errors }))
101-
102-
expect(Sentry).to receive(:capture_message).with("Bulk delete errors", extra: { errors: })
103-
104-
perform
105-
end
106-
end
10770
end
10871
end

spec/jobs/cron/purge_unattached_blobs_job_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
end
2121

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

2525
it 'does not enqueue a purge' do
2626
expect { subject }.not_to have_enqueued_job(DelayedPurgeJob)

spec/jobs/delayed_purge_job_spec.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,16 +40,16 @@
4040
.and_return(double(status: 201))
4141
subject
4242
perform_enqueued_jobs
43-
expect(blob.reload.soft_delete_at).to be_present
43+
expect(blob.reload.soft_deleted_at).to be_present
4444
end
4545

46-
it 'does not mark soft_delete_at when the copy fails' do
46+
it 'does not mark soft_deleted_at when the copy fails' do
4747
dossier.champ_data.first.piece_justificative_file.first.delete
4848
expect(client).to receive(:copy_object).and_return(double(status: 500))
4949
expect(Sentry).to receive(:capture_message)
5050
subject
5151
perform_enqueued_jobs
52-
expect(blob.reload.soft_delete_at).to be_nil
52+
expect(blob.reload.soft_deleted_at).to be_nil
5353
end
5454

5555
it 'with cloned dossier' do

spec/services/blob_service_spec.rb

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
# frozen_string_literal: true
2+
3+
RSpec.describe BlobService do
4+
describe '.purge_blobs_with_variants' do
5+
# Blobs are created against the real (disk) service, then flipped to openstack
6+
# and purged against a mocked openstack service — installed by the subject,
7+
# once every blob exists.
8+
subject(:purge_blobs_with_variants) do
9+
service = double('openstack service', container: 'bucket', name: :openstack)
10+
allow(service).to receive(:client).and_return(client) # reached via service.send(:client)
11+
allow(ActiveStorage::Blob).to receive(:service).and_return(service)
12+
BlobService.purge_blobs_with_variants([blob.id])
13+
end
14+
15+
let(:client) { double('openstack client') }
16+
17+
let!(:blob) do
18+
ActiveStorage::Blob.create_and_upload!(io: StringIO.new("old"), filename: "old.txt", content_type: "text/plain")
19+
.tap { it.update_column(:service_name, 'openstack') }
20+
end
21+
22+
before do
23+
allow(client).to receive(:delete_multiple_objects).and_return(double(body: { 'Errors' => [] }))
24+
end
25+
26+
it 'drops the blob row' do
27+
expect { purge_blobs_with_variants }.to change { ActiveStorage::Blob.exists?(id: blob.id) }.from(true).to(false)
28+
end
29+
30+
it 'bulk-deletes the parent file (safety net against a missed X-Delete-At)' do
31+
expect(client).to receive(:delete_multiple_objects)
32+
.with('bucket', [blob.key])
33+
.and_return(double(body: { 'Errors' => [] }))
34+
35+
purge_blobs_with_variants
36+
end
37+
38+
context 'when the blob is an image with variants' do
39+
let!(:variant_blob) do
40+
ActiveStorage::Blob.create_and_upload!(io: StringIO.new("variant"), filename: "v.png", content_type: "image/png")
41+
.tap { it.update_column(:service_name, 'openstack') }
42+
end
43+
let!(:variant_record) { ActiveStorage::VariantRecord.create!(blob:, variation_digest: "digest") }
44+
let!(:image_attachment) { ActiveStorage::Attachment.create!(name: "image", record: variant_record, blob: variant_blob) }
45+
46+
it 'bulk-deletes both variant and parent files, then drops every row' do
47+
expect(client).to receive(:delete_multiple_objects)
48+
.with('bucket', match_array([variant_blob.key, blob.key]))
49+
.and_return(double(body: { 'Errors' => [] }))
50+
51+
purge_blobs_with_variants
52+
53+
expect(ActiveStorage::Blob.where(id: [blob.id, variant_blob.id])).not_to exist
54+
expect(ActiveStorage::VariantRecord.where(id: variant_record.id)).not_to exist
55+
expect(ActiveStorage::Attachment.where(id: image_attachment.id)).not_to exist
56+
end
57+
58+
it 'reports per-object bulk delete errors to Sentry' do
59+
errors = [["bucket/#{variant_blob.key}", "409 Conflict"]]
60+
allow(client).to receive(:delete_multiple_objects).and_return(double(body: { 'Errors' => errors }))
61+
62+
expect(Sentry).to receive(:capture_message).with("Bulk delete errors", extra: { errors: })
63+
64+
purge_blobs_with_variants
65+
end
66+
end
67+
68+
context 'when the blob is stored on another service' do
69+
let!(:blob) do
70+
ActiveStorage::Blob.create_and_upload!(io: StringIO.new("other"), filename: "other.txt", content_type: "text/plain")
71+
end
72+
73+
it 'leaves its file and row untouched' do
74+
expect(client).not_to receive(:delete_multiple_objects)
75+
76+
expect { purge_blobs_with_variants }.not_to change { ActiveStorage::Blob.exists?(id: blob.id) }
77+
end
78+
end
79+
end
80+
end

0 commit comments

Comments
 (0)