Merge pull request #3883 from betagouv/mark-migrated-pjs-as-scanned

Migration des pièces-jointes : évite de faire tourner l'antivirus lors de la migration

app/models/concerns/blob_virus_scanner.rb

@@ -1,3 +1,7 @@
+# TODO: once we're using Rails 6, use the hooks on attachments creation
+# (rather than on blob creation).
+# This will help to avoid cloberring metadata accidentally (as metadata
+# are more stable on attachment creation than on blob creation).
 module BlobVirusScanner
   extend ActiveSupport::Concern
 

app/services/carrierwave_active_storage_migration_service.rb

@@ -75,14 +75,15 @@ class CarrierwaveActiveStorageMigrationService
   # but when the first attachment that references this blob is created.
   def make_blob(uploader, created_at, filename: nil, identify: false)
     content_type = uploader.content_type
+    identified = content_type.present? && !identify
 
     ActiveStorage::Blob.create(
       filename: filename || uploader.filename,
       content_type: uploader.content_type,
-      identified: content_type.present? && !identify,
       byte_size: uploader.size,
       checksum: checksum(uploader),
-      created_at: created_at
+      created_at: created_at,
+      metadata: { identified: identified, virus_scan_result: ActiveStorage::VirusScanner::SAFE }
     )
   end
 

config/initializers/active_storage.rb

@@ -1,3 +1,9 @@
 ActiveStorage::Service.url_expires_in = 1.hour
 
+# In Rails 5.2, we have to hook at `on_load` on the blob themeselves, which is
+# not ideal.
+#
+# Rails 6 adds support for `.on_load(:active_storage_attachment)`, which is
+# cleaner (as it allows to enqueue the virus scan on attachment creation, rather
+# than on blob creation).
 ActiveSupport.on_load(:active_storage_blob) { include BlobVirusScanner }

spec/services/carrierwave_active_storage_migration_service_spec.rb

@@ -1,9 +1,37 @@
 require 'spec_helper'
 
 describe CarrierwaveActiveStorageMigrationService do
-  describe '#hex_to_base64' do
+  let(:service) { CarrierwaveActiveStorageMigrationService.new }
-    let(:service) { CarrierwaveActiveStorageMigrationService.new }
 
+  describe '#hex_to_base64' do
     it { expect(service.hex_to_base64('deadbeef')).to eq('3q2+7w==') }
   end
+
+  describe '.make_blob' do
+    let(:pj) { create(:piece_justificative, :rib) }
+    let(:identify) { false }
+
+    before do
+      allow(service).to receive(:checksum).and_return('cafe')
+    end
+
+    subject(:blob) { service.make_blob(pj.content, pj.updated_at.iso8601, filename: pj.original_filename, identify: identify) }
+
+    it 'marks the blob as already scanned by the antivirus' do
+      expect(blob.metadata[:virus_scan_result]).to eq(ActiveStorage::VirusScanner::SAFE)
+    end
+
+    it 'sets the blob MIME type from the file' do
+      expect(blob.identified).to be true
+      expect(blob.content_type).to eq 'application/pdf'
+    end
+
+    context 'when asking for explicit MIME type identification' do
+      let(:identify) { true }
+
+      it 'marks the file as needing MIME type detection' do
+        expect(blob.identified).to be false
+      end
+    end
+  end
 end