Améliorations de robustesse à la tâche de migration des PR (#3904)

Améliorations de robustesse à la tâche de migration des PR

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,14 +67,19 @@ 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
@@ -86,13 +96,14 @@ class PieceJustificativeToChampPieceJointeMigrationService
 
     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.
 

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

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

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