From 4d9cb0bb7aea821c7a0258f3c5ba6a18a7c33267 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Wed, 29 May 2019 11:48:21 +0200 Subject: [PATCH 1/7] task_helper: fix duration formatting If we don't convert the duration to utc, it starts at 01:00:00 --- lib/tasks/task_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tasks/task_helper.rb b/lib/tasks/task_helper.rb index 8be2ab6dc..fa60c58c7 100644 --- a/lib/tasks/task_helper.rb +++ b/lib/tasks/task_helper.rb @@ -60,7 +60,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 From 2bb103b8bd51173510f772e76b1bef05231d70cd Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Wed, 29 May 2019 11:48:41 +0200 Subject: [PATCH 2/7] task_helper: mark private methods --- lib/tasks/task_helper.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/tasks/task_helper.rb b/lib/tasks/task_helper.rb index fa60c58c7..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 From 60121a1be6d6bfff3df0ec87a7b2d4fed4341675 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Wed, 29 May 2019 11:49:11 +0200 Subject: [PATCH 3/7] =?UTF-8?q?task=5Fhelper:=20ensure=20that=20incrementi?= =?UTF-8?q?ng=20above=20100%=20doesn=E2=80=99t=20raise=20an=20error?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- spec/lib/tasks/task_helper_spec.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 spec/lib/tasks/task_helper_spec.rb 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 From a168ae3a729df0c9c4a2c3e94a8efee27d6454c8 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Wed, 29 May 2019 12:02:33 +0200 Subject: [PATCH 4/7] pj migration: reduce the number of queries by preloading champs --- ...ece_justificative_to_champ_piece_jointe_migration_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 0ca726b19..a157a1983 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 @@ -37,7 +37,7 @@ 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| champs_pj = types_de_champ_pj.map(&:build_champ) dossier.champs += champs_pj From 1a256b37f89681e847681dd49914603e95c04ded Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Wed, 29 May 2019 12:03:16 +0200 Subject: [PATCH 5/7] pj migration: print a notice when migrating missing files --- ...ce_justificative_to_champ_piece_jointe_migration_service.rb | 3 +++ 1 file changed, 3 insertions(+) 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 a157a1983..8b2f4946e 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| @@ -86,6 +88,7 @@ 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 From 6cb02f292793d08dba7edbf5468daad77fd15082 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Wed, 29 May 2019 13:06:36 +0200 Subject: [PATCH 6/7] pj migration: handle signal interrupts --- ...e_to_champ_piece_jointe_migration_service.rb | 5 +++-- ...champ_piece_jointe_migration_service_spec.rb | 17 +++++++++++++++-- 2 files changed, 18 insertions(+), 4 deletions(-) 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 8b2f4946e..ed6597e94 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 @@ -64,7 +64,8 @@ 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. types_de_champ_pj.each do |type_champ| @@ -95,7 +96,7 @@ class PieceJustificativeToChampPieceJointeMigrationService # 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. 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 From a8847e40ea3585cbc5af3e4aecde7b10aa856a38 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Wed, 29 May 2019 13:06:57 +0200 Subject: [PATCH 7/7] pj migration: add comments and notices --- ...ustificative_to_champ_piece_jointe_migration_service.rb | 7 +++++++ 1 file changed, 7 insertions(+) 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 ed6597e94..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 @@ -40,9 +40,12 @@ class PieceJustificativeToChampPieceJointeMigrationService # Unscope to make sure all dossiers are migrated, even the soft-deleted ones 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) @@ -68,11 +71,15 @@ class PieceJustificativeToChampPieceJointeMigrationService 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