diff --git a/app/services/carrierwave_active_storage_migration_service.rb b/app/services/carrierwave_active_storage_migration_service.rb index 9d80f3be3..1183cf8f9 100644 --- a/app/services/carrierwave_active_storage_migration_service.rb +++ b/app/services/carrierwave_active_storage_migration_service.rb @@ -79,7 +79,7 @@ class CarrierwaveActiveStorageMigrationService ActiveStorage::Blob.create( filename: filename || uploader.filename, - content_type: uploader.content_type, + content_type: content_type, byte_size: uploader.size, checksum: checksum(uploader), created_at: created_at, @@ -87,6 +87,20 @@ class CarrierwaveActiveStorageMigrationService ) end + def make_empty_blob(uploader, created_at, filename: nil) + content_type = uploader.content_type || 'text/plain' + + blob = ActiveStorage::Blob.build_after_upload( + io: StringIO.new('File not found when migrating from CarrierWave.'), + filename: filename || uploader.filename, + content_type: content_type || 'text/plain', + metadata: { virus_scan_result: ActiveStorage::VirusScanner::SAFE } + ) + blob.created_at = created_at + blob.save! + blob + end + def checksum(uploader) hex_to_base64(uploader.file.send(:file).etag) end diff --git a/app/services/piece_justificative_to_champ_piece_jointe_migration_service.rb b/app/services/piece_justificative_to_champ_piece_jointe_migration_service.rb index 30b025201..18d37251e 100644 --- a/app/services/piece_justificative_to_champ_piece_jointe_migration_service.rb +++ b/app/services/piece_justificative_to_champ_piece_jointe_migration_service.rb @@ -1,3 +1,5 @@ +require Rails.root.join("lib", "tasks", "task_helper") + class PieceJustificativeToChampPieceJointeMigrationService def initialize(**params) params.each do |key, value| @@ -37,10 +39,13 @@ class PieceJustificativeToChampPieceJointeMigrationService procedure.types_de_champ += types_de_champ_pj # Unscope to make sure all dossiers are migrated, even the soft-deleted ones - procedure.dossiers.unscope(where: :hidden_at).find_each do |dossier| + procedure.dossiers.unscope(where: :hidden_at).includes(:champs).find_each do |dossier| + # Add the new pieces justificatives champs to the dossier champs_pj = types_de_champ_pj.map(&:build_champ) dossier.champs += champs_pj + # Copy the dossier old pieces jointes to the new champs + # (even if the champs already existed, so that we ensure a clean state) champs_pj.each do |champ| type_pj_id = champ.type_de_champ.old_pj&.fetch('stable_id', nil) pj = dossier.retrieve_last_piece_justificative_by_type(type_pj_id) @@ -62,31 +67,43 @@ class PieceJustificativeToChampPieceJointeMigrationService yield if block_given? end end - rescue + + rescue StandardError, SignalException # If anything goes wrong, we roll back the migration by destroying the newly created # types de champ, champs blobs and attachments. + rake_puts "Error received. Rolling back migration of procedure #{procedure.id}…" + types_de_champ_pj.each do |type_champ| type_champ.champ.each { |c| c.piece_justificative_file.purge } type_champ.destroy end + rake_puts "Migration of procedure #{procedure.id} rolled back." + # Reraise the exception to abort the migration. raise end def convert_pj_to_champ!(pj, champ) - blob = make_blob(pj) + actual_file_exists = pj.content.file.send(:file) + if actual_file_exists + blob = make_blob(pj) - # Upload the file before creating the attachment to make sure MIME type - # identification doesn’t fail. - storage_service.copy_from_carrierwave_to_active_storage!(pj.content.path, blob) - attachment = storage_service.make_attachment(champ, 'piece_justificative_file', blob) + # Upload the file before creating the attachment to make sure MIME type + # identification doesn’t fail. + storage_service.copy_from_carrierwave_to_active_storage!(pj.content.path, blob) + attachment = storage_service.make_attachment(champ, 'piece_justificative_file', blob) + + else + make_empty_blob(pj) + rake_puts "Notice: attached file for champ #{champ.id} not found. An empty blob has been attached instead." + end # By reloading, we force ActiveStorage to look at the attachment again, and see # that one exists now. We do this so that, if we need to roll back and destroy the champ, # the blob, the attachment and the actual file on OpenStack also get deleted. champ.reload - rescue + rescue StandardError, SignalException # Destroy partially attached object that the more general rescue in `populate_champs_pjs!` # might not be able to handle. @@ -112,4 +129,8 @@ class PieceJustificativeToChampPieceJointeMigrationService def make_blob(pj) storage_service.make_blob(pj.content, pj.updated_at.iso8601, filename: pj.original_filename) end + + def make_empty_blob(pj) + storage_service.make_empty_blob(pj.content, pj.updated_at.iso8601, filename: pj.original_filename) + end end diff --git a/app/services/pieces_justificatives_service.rb b/app/services/pieces_justificatives_service.rb index 7ca08d233..411e11124 100644 --- a/app/services/pieces_justificatives_service.rb +++ b/app/services/pieces_justificatives_service.rb @@ -33,12 +33,8 @@ class PiecesJustificativesService end def self.types_pj_as_types_de_champ(procedure) - last_champ = procedure.types_de_champ.last - if last_champ.present? - order_place = last_champ.order_place + 1 - else - order_place = 0 - end + max_order_place = procedure.types_de_champ.pluck(:order_place).compact.max || -1 + order_place = max_order_place + 1 types_de_champ = [ TypeDeChamp.new( diff --git a/lib/tasks/pieces_justificatives.rake b/lib/tasks/pieces_justificatives.rake index 49fe00193..5f11f5fa8 100644 --- a/lib/tasks/pieces_justificatives.rake +++ b/lib/tasks/pieces_justificatives.rake @@ -1,6 +1,9 @@ require Rails.root.join("lib", "tasks", "task_helper") namespace :pieces_justificatives do + desc <<~EOD + Migrate the PJ to champs for a single PROCEDURE_ID. + EOD task migrate_procedure_to_champs: :environment do procedure_id = ENV['PROCEDURE_ID'] procedure = Procedure.find(procedure_id) @@ -17,6 +20,9 @@ namespace :pieces_justificatives do progress.finish end + desc <<~EOD + Migrate the PJ to champs for several procedures ids, from RANGE_START to RANGE_END. + EOD task migrate_procedures_range_to_champs: :environment do if ENV['RANGE_START'].nil? || ENV['RANGE_END'].nil? fail "RANGE_START and RANGE_END must be specified" diff --git a/lib/tasks/task_helper.rb b/lib/tasks/task_helper.rb index 8be2ab6dc..ef6a0bd95 100644 --- a/lib/tasks/task_helper.rb +++ b/lib/tasks/task_helper.rb @@ -37,6 +37,8 @@ class ProgressReport rake_puts end + private + def set_progress(total: nil, count: nil) if total.present? @total = total @@ -60,7 +62,7 @@ class ProgressReport def format_duration(seconds) if seconds.finite? - Time.zone.at(seconds).strftime('%H:%M:%S') + Time.zone.at(seconds).utc.strftime('%H:%M:%S') else '--:--:--' end diff --git a/spec/lib/tasks/task_helper_spec.rb b/spec/lib/tasks/task_helper_spec.rb new file mode 100644 index 000000000..4d02d159c --- /dev/null +++ b/spec/lib/tasks/task_helper_spec.rb @@ -0,0 +1,16 @@ +require 'spec_helper' + +describe ProgressReport, lib: true do + context 'when the count pass above 100%' do + let(:total) { 2 } + + subject(:progress) { ProgressReport.new(total) } + + it 'doesn’t raise errors' do + expect do + (total + 2).times { progress.inc } + progress.finish + end.not_to raise_error + end + end +end diff --git a/spec/services/carrierwave_active_storage_migration_service_spec.rb b/spec/services/carrierwave_active_storage_migration_service_spec.rb index a3bb2b042..fb70b0d6f 100644 --- a/spec/services/carrierwave_active_storage_migration_service_spec.rb +++ b/spec/services/carrierwave_active_storage_migration_service_spec.rb @@ -8,7 +8,7 @@ describe CarrierwaveActiveStorageMigrationService do end describe '.make_blob' do - let(:pj) { create(:piece_justificative, :rib) } + let(:pj) { create(:piece_justificative, :rib, updated_at: Time.zone.local(2019, 01, 01, 12, 00)) } let(:identify) { false } before do @@ -17,6 +17,8 @@ describe CarrierwaveActiveStorageMigrationService do subject(:blob) { service.make_blob(pj.content, pj.updated_at.iso8601, filename: pj.original_filename, identify: identify) } + it { expect(blob.created_at).to eq pj.updated_at } + it 'marks the blob as already scanned by the antivirus' do expect(blob.metadata[:virus_scan_result]).to eq(ActiveStorage::VirusScanner::SAFE) end @@ -34,4 +36,37 @@ describe CarrierwaveActiveStorageMigrationService do end end end + + describe '.make_empty_blob' do + let(:pj) { create(:piece_justificative, :rib, updated_at: Time.zone.local(2019, 01, 01, 12, 00)) } + + before 'set the underlying stored file as missing' do + allow(pj.content.file).to receive(:file).and_return(nil) + end + + subject(:blob) { service.make_empty_blob(pj.content, pj.updated_at.iso8601, filename: pj.original_filename) } + + it { expect(blob.created_at).to eq pj.updated_at } + + 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 the file metadata are also missing' do + before do + allow(pj).to receive(:original_filename).and_return(nil) + allow(pj.content).to receive(:content_type).and_return(nil) + end + + it 'fallbacks on default values' do + expect(blob.filename).to eq pj.content.filename + expect(blob.content_type).to eq 'text/plain' + end + end + end end diff --git a/spec/services/piece_justificative_to_champ_piece_jointe_migration_service_spec.rb b/spec/services/piece_justificative_to_champ_piece_jointe_migration_service_spec.rb index bbd94d8ef..86ea98b7f 100644 --- a/spec/services/piece_justificative_to_champ_piece_jointe_migration_service_spec.rb +++ b/spec/services/piece_justificative_to_champ_piece_jointe_migration_service_spec.rb @@ -182,6 +182,7 @@ describe PieceJustificativeToChampPieceJointeMigrationService do context 'cleanup when conversion fails' do let(:pjs) { make_pjs } + let(:exception) { 'LOL no!' } let!(:failing_dossier) do create( @@ -197,14 +198,14 @@ describe PieceJustificativeToChampPieceJointeMigrationService do expect(storage_service).to receive(:copy_from_carrierwave_to_active_storage!) expect(storage_service).to receive(:copy_from_carrierwave_to_active_storage!) - .and_raise('LOL no!') + .and_raise(exception) expect(storage_service).to receive(:delete_from_active_storage!) end def try_convert(procedure) service.convert_procedure_pjs_to_champ_pjs(procedure) - rescue => e + rescue StandardError, SignalException => e e end @@ -242,5 +243,17 @@ describe PieceJustificativeToChampPieceJointeMigrationService do expect { try_convert(procedure) } .not_to change { ActiveStorage::Attachment.count } end + + context 'when receiving a Signal interruption (like Ctrl+C)' do + let(:exception) { Interrupt } + + it 'handles the exception as well' do + expect { service.convert_procedure_pjs_to_champ_pjs(procedure) }.to raise_error { Interrupt } + end + + it 'does not create champs' do + expect { try_convert(procedure) }.not_to change { dossier.champs.count } + end + end end end diff --git a/spec/services/pieces_justificatives_service_spec.rb b/spec/services/pieces_justificatives_service_spec.rb index 63754ecdd..3d8eb6d09 100644 --- a/spec/services/pieces_justificatives_service_spec.rb +++ b/spec/services/pieces_justificatives_service_spec.rb @@ -112,11 +112,25 @@ describe PiecesJustificativesService do create( :procedure, types_de_piece_justificative: tpjs, - types_de_champ: [build(:type_de_champ, order_place: 0)] + types_de_champ: [build(:type_de_champ, order_place: 0), build(:type_de_champ, order_place: 1)] ) end it 'generates a sequence of incrementing order_places that continues where the last type de champ left off' do + expect(subject.pluck(:order_place)).to contain_exactly(2, 3) + end + end + + context 'with pre-existing champs without an order place' do + let(:procedure) do + create( + :procedure, + types_de_piece_justificative: tpjs, + types_de_champ: [build(:type_de_champ, order_place: 0), build(:type_de_champ, order_place: nil)] + ) + end + + it 'ignores champs without an order place' do expect(subject.pluck(:order_place)).to contain_exactly(1, 2) end end