diff --git a/app/jobs/cron/fix_missing_antivirus_analysis_job.rb b/app/jobs/cron/fix_missing_antivirus_analysis_job.rb index b12a0445c..f9f4a9dc9 100644 --- a/app/jobs/cron/fix_missing_antivirus_analysis_job.rb +++ b/app/jobs/cron/fix_missing_antivirus_analysis_job.rb @@ -2,9 +2,9 @@ class Cron::FixMissingAntivirusAnalysisJob < Cron::CronJob self.schedule_expression = "every day at 2 am" def perform - ActiveStorage::Blob.where("metadata like '%\"virus_scan_result\":\"pending%'").each do |b| + ActiveStorage::Blob.where(virus_scan_result: ActiveStorage::VirusScanner::PENDING).find_each do |blob| begin - VirusScannerJob.perform_now(b) + VirusScannerJob.perform_now(blob) rescue ActiveStorage::IntegrityError end end diff --git a/app/jobs/migrations/.keep b/app/jobs/migrations/.keep new file mode 100644 index 000000000..e69de29bb diff --git a/app/jobs/migrations/backfill_virus_scan_blobs_job.rb b/app/jobs/migrations/backfill_virus_scan_blobs_job.rb new file mode 100644 index 000000000..f9ba1b002 --- /dev/null +++ b/app/jobs/migrations/backfill_virus_scan_blobs_job.rb @@ -0,0 +1,6 @@ +class Migrations::BackfillVirusScanBlobsJob < ApplicationJob + def perform(batch) + ActiveStorage::Blob.where(id: batch) + .update_all(virus_scan_result: ActiveStorage::VirusScanner::SAFE) + end +end diff --git a/app/jobs/virus_scanner_job.rb b/app/jobs/virus_scanner_job.rb index 0d5f0ba24..3d2ea0944 100644 --- a/app/jobs/virus_scanner_job.rb +++ b/app/jobs/virus_scanner_job.rb @@ -6,30 +6,12 @@ class VirusScannerJob < ApplicationJob # If for some reason the file appears invalid, retry for a while retry_on(ActiveStorage::IntegrityError, attempts: 5, wait: 5.seconds) do |job, _error| blob = job.arguments.first - - metadata = { - virus_scan_result: ActiveStorage::VirusScanner::INTEGRITY_ERROR, - scanned_at: Time.zone.now - } - - merge_and_update_metadata(blob, metadata) + blob.update_columns(virus_scan_result: ActiveStorage::VirusScanner::INTEGRITY_ERROR, virus_scanned_at: Time.zone.now) end def perform(blob) - if blob.virus_scanner.done? then return end + return if blob.virus_scanner.done? - metadata = extract_metadata_via_virus_scanner(blob) - - VirusScannerJob.merge_and_update_metadata(blob, metadata) - end - - def extract_metadata_via_virus_scanner(blob) - ActiveStorage::VirusScanner.new(blob).metadata - end - - private - - def self.merge_and_update_metadata(blob, metadata) - blob.update!(metadata: blob.metadata.merge(metadata)) + blob.update_columns(ActiveStorage::VirusScanner.new(blob).attributes) end end diff --git a/app/lib/active_storage/virus_scanner.rb b/app/lib/active_storage/virus_scanner.rb index 90cd0a528..6e5d26d01 100644 --- a/app/lib/active_storage/virus_scanner.rb +++ b/app/lib/active_storage/virus_scanner.rb @@ -11,36 +11,42 @@ class ActiveStorage::VirusScanner INTEGRITY_ERROR = 'integrity_error' def pending? - blob.metadata[:virus_scan_result] == PENDING + virus_scan_result == PENDING end def infected? - blob.metadata[:virus_scan_result] == INFECTED + virus_scan_result == INFECTED end def safe? - blob.metadata[:virus_scan_result] == SAFE + virus_scan_result == SAFE end def corrupt? - blob.metadata[:virus_scan_result] == INTEGRITY_ERROR + virus_scan_result == INTEGRITY_ERROR end def done? - started? && blob.metadata[:virus_scan_result] != PENDING + started? && virus_scan_result != PENDING end def started? - blob.metadata[:virus_scan_result].present? + virus_scan_result.present? end - def metadata + def attributes blob.open do |file| if ClamavService.safe_file?(file.path) - { virus_scan_result: SAFE, scanned_at: Time.zone.now } + { virus_scan_result: SAFE, virus_scanned_at: Time.zone.now } else - { virus_scan_result: INFECTED, scanned_at: Time.zone.now } + { virus_scan_result: INFECTED, virus_scanned_at: Time.zone.now } end end end + + private + + def virus_scan_result + blob.virus_scan_result || blob.metadata[:virus_scan_result] + end end diff --git a/app/models/concerns/blob_virus_scanner_concern.rb b/app/models/concerns/blob_virus_scanner_concern.rb index 685c0f73e..b77a55d3b 100644 --- a/app/models/concerns/blob_virus_scanner_concern.rb +++ b/app/models/concerns/blob_virus_scanner_concern.rb @@ -23,6 +23,6 @@ module BlobVirusScannerConcern private def set_pending - metadata[:virus_scan_result] ||= ActiveStorage::VirusScanner::PENDING + self.virus_scan_result = metadata[:virus_scan_result] || ActiveStorage::VirusScanner::PENDING end end diff --git a/db/migrate/20221222170319_add_virus_scanned_at_active_storage_blobs.rb b/db/migrate/20221222170319_add_virus_scanned_at_active_storage_blobs.rb new file mode 100644 index 000000000..36b536fcf --- /dev/null +++ b/db/migrate/20221222170319_add_virus_scanned_at_active_storage_blobs.rb @@ -0,0 +1,6 @@ +class AddVirusScannedAtActiveStorageBlobs < ActiveRecord::Migration[6.1] + def change + add_column :active_storage_blobs, :virus_scan_result, :string + add_column :active_storage_blobs, :virus_scanned_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index 037c0a890..059e989e7 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2022_12_13_084442) do +ActiveRecord::Schema.define(version: 2022_12_27_084442) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" @@ -47,6 +47,8 @@ ActiveRecord::Schema.define(version: 2022_12_13_084442) do t.integer "lock_version" t.text "metadata" t.string "service_name", null: false + t.string "virus_scan_result" + t.datetime "virus_scanned_at" t.datetime "watermarked_at" t.index ["key"], name: "index_active_storage_blobs_on_key", unique: true end diff --git a/lib/tasks/deployment/20221222173733_backfill_virus_scan_blobs.rake b/lib/tasks/deployment/20221222173733_backfill_virus_scan_blobs.rake new file mode 100644 index 000000000..7b42af031 --- /dev/null +++ b/lib/tasks/deployment/20221222173733_backfill_virus_scan_blobs.rake @@ -0,0 +1,32 @@ +namespace :after_party do + desc 'Deployment task: backfill_virus_scan_blobs' + task backfill_virus_scan_blobs: :environment do + puts "Running deploy task 'backfill_virus_scan_blobs'" + + pending_blobs = ActiveStorage::Blob.where("metadata like '%\"virus_scan_result\":\"#{ActiveStorage::VirusScanner::PENDING}%'").where(virus_scan_result: nil) + infected_blobs = ActiveStorage::Blob.where("metadata like '%\"virus_scan_result\":\"#{ActiveStorage::VirusScanner::INFECTED}%'").where(virus_scan_result: nil) + integrity_error_blobs = ActiveStorage::Blob.where("metadata like '%\"virus_scan_result\":\"#{ActiveStorage::VirusScanner::INTEGRITY_ERROR}%'").where(virus_scan_result: nil) + safe_blobs = ActiveStorage::Blob.where("metadata like '%\"virus_scan_result\":\"#{ActiveStorage::VirusScanner::SAFE}%'").where(virus_scan_result: nil) + + pp "pending blobs: #{pending_blobs.count}" + pp "infected blobs: #{infected_blobs.count}" + pp "with integrity error blobs: #{integrity_error_blobs.count}" + + pending_blobs.in_batches.update_all(virus_scan_result: ActiveStorage::VirusScanner::PENDING) + infected_blobs.in_batches.update_all(virus_scan_result: ActiveStorage::VirusScanner::INFECTED) + integrity_error_blobs.in_batches.update_all(virus_scan_result: ActiveStorage::VirusScanner::INTEGRITY_ERROR) + + safe_blobs_ids = safe_blobs.pluck(:id) + progress = ProgressReport.new(safe_blobs_ids.size) + safe_blobs_ids.in_groups_of(10_000) do |batch| + Migrations::BackfillVirusScanBlobsJob.perform_later(batch.compact) + progress.inc(batch.compact.size) + end + progress.finish + + # Update task as completed. If you remove the line below, the task will + # run with every deploy (or every time you call after_party:run). + AfterParty::TaskRecord + .create version: AfterParty::TaskRecorder.new(__FILE__).timestamp + end +end diff --git a/spec/components/attachment/edit_component_spec.rb b/spec/components/attachment/edit_component_spec.rb index bcae3b6ec..40851b3aa 100644 --- a/spec/components/attachment/edit_component_spec.rb +++ b/spec/components/attachment/edit_component_spec.rb @@ -127,7 +127,7 @@ RSpec.describe Attachment::EditComponent, type: :component do context 'with non nominal or final antivirus status' do before do - champ.piece_justificative_file[0].blob.update(metadata: attachment.blob.metadata.merge(virus_scan_result: virus_scan_result)) + champ.piece_justificative_file[0].blob.update(virus_scan_result:) end context 'when the anti-virus scan is pending' do diff --git a/spec/components/attachment/multiple_component_spec.rb b/spec/components/attachment/multiple_component_spec.rb index 2ac7396be..54b8782b4 100644 --- a/spec/components/attachment/multiple_component_spec.rb +++ b/spec/components/attachment/multiple_component_spec.rb @@ -84,7 +84,7 @@ RSpec.describe Attachment::MultipleComponent, type: :component do let(:created_at) { 1.second.ago } before do - attached_file.attachments[0].blob.update(metadata: { virus_scan_result: ActiveStorage::VirusScanner::PENDING }) + attached_file.attachments[0].blob.update(virus_scan_result: ActiveStorage::VirusScanner::PENDING) attached_file.attachments[0].update!(created_at:) end diff --git a/spec/components/attachment/pending_poll_component_spec.rb b/spec/components/attachment/pending_poll_component_spec.rb index 1bd9f9f5f..0ca07eddf 100644 --- a/spec/components/attachment/pending_poll_component_spec.rb +++ b/spec/components/attachment/pending_poll_component_spec.rb @@ -48,7 +48,7 @@ RSpec.describe Attachment::PendingPollComponent, type: :component do context "when antivirus is in progress" do before do - attachment.blob[:metadata] = { virus_scan_result: ActiveStorage::VirusScanner::PENDING } + attachment.blob.virus_scan_result = ActiveStorage::VirusScanner::PENDING end it "renders" do diff --git a/spec/components/attachment/show_component_spec.rb b/spec/components/attachment/show_component_spec.rb index 658aa5308..adcf36ff3 100644 --- a/spec/components/attachment/show_component_spec.rb +++ b/spec/components/attachment/show_component_spec.rb @@ -15,7 +15,7 @@ RSpec.describe Attachment::ShowComponent, type: :component do subject { render_inline(component).to_html } before do - champ.piece_justificative_file[0].blob.update(metadata: champ.piece_justificative_file[0].blob.metadata.merge(virus_scan_result: virus_scan_result)) + attachment.blob.update(virus_scan_result:, metadata: attachment.blob.metadata.merge(virus_scan_result:)) end context 'when there is no anti-virus scan' do diff --git a/spec/jobs/virus_scanner_job_spec.rb b/spec/jobs/virus_scanner_job_spec.rb index 2ea97bff6..9de830373 100644 --- a/spec/jobs/virus_scanner_job_spec.rb +++ b/spec/jobs/virus_scanner_job_spec.rb @@ -21,8 +21,8 @@ describe VirusScannerJob, type: :job do context "should raise ActiveRecord::StaleObjectError" do let(:blob_2) { ActiveStorage::Blob.find(blob.id) } before do - blob_2.metadata[:virus_scan_result] = "infected" - blob.metadata[:virus_scan_result] = "safe" + blob_2.virus_scan_result = ActiveStorage::VirusScanner::INFECTED + blob.virus_scan_result = ActiveStorage::VirusScanner::SAFE blob.save end diff --git a/spec/models/champs/piece_justificative_champ_spec.rb b/spec/models/champs/piece_justificative_champ_spec.rb index 0073793db..d3462235d 100644 --- a/spec/models/champs/piece_justificative_champ_spec.rb +++ b/spec/models/champs/piece_justificative_champ_spec.rb @@ -53,24 +53,23 @@ describe Champs::PieceJustificativeChamp do describe '#for_api' do let(:champ_pj) { create(:champ_piece_justificative) } - let(:metadata) { champ_pj.piece_justificative_file.first.blob.metadata } - before { champ_pj.piece_justificative_file.first.blob.update(metadata: metadata.merge(virus_scan_result: status)) } + before { champ_pj.piece_justificative_file.first.blob.update(virus_scan_result:) } subject { champ_pj.for_api } context 'when file is safe' do - let(:status) { ActiveStorage::VirusScanner::SAFE } + let(:virus_scan_result) { ActiveStorage::VirusScanner::SAFE } it { is_expected.to include("/rails/active_storage/disk/") } end context 'when file is not scanned' do - let(:status) { ActiveStorage::VirusScanner::PENDING } + let(:virus_scan_result) { ActiveStorage::VirusScanner::PENDING } it { is_expected.to include("/rails/active_storage/disk/") } end context 'when file is infected' do - let(:status) { ActiveStorage::VirusScanner::INFECTED } + let(:virus_scan_result) { ActiveStorage::VirusScanner::INFECTED } it { is_expected.to be_nil } end end diff --git a/spec/system/users/brouillon_spec.rb b/spec/system/users/brouillon_spec.rb index cc9dc92a8..b36abd2f3 100644 --- a/spec/system/users/brouillon_spec.rb +++ b/spec/system/users/brouillon_spec.rb @@ -280,7 +280,7 @@ describe 'The user' do end attachments.each { - _1.blob.metadata = { virus_scan_result: ActiveStorage::VirusScanner::SAFE } + _1.blob.virus_scan_result = ActiveStorage::VirusScanner::SAFE _1.save! } expect(page).not_to have_text('Analyse antivirus en cours', wait: 10) diff --git a/spec/views/instructeur/dossiers/_state_button.html.haml_spec.rb b/spec/views/instructeur/dossiers/_state_button.html.haml_spec.rb index d44baf449..5c71fc460 100644 --- a/spec/views/instructeur/dossiers/_state_button.html.haml_spec.rb +++ b/spec/views/instructeur/dossiers/_state_button.html.haml_spec.rb @@ -95,7 +95,7 @@ describe 'instructeurs/dossiers/state_button.html.haml', type: :view do context 'with a justificatif' do let(:dossier) do dossier = create(:dossier, state, :with_justificatif) - dossier.justificatif_motivation.blob.update(metadata: dossier.justificatif_motivation.blob.metadata.merge(virus_scan_result: ActiveStorage::VirusScanner::SAFE)) + dossier.justificatif_motivation.blob.update(virus_scan_result: ActiveStorage::VirusScanner::SAFE) dossier end