Merge pull request #13303 from demarche-numerique/fix_race_condition_on_ocr

Tech: résoud un problème d'accès concurrents pour l'analyse OCR

Author: LeSim

app/controllers/champs/piece_justificative_controller.rb

@@ -34,8 +34,6 @@ def attach_piece_justificative
     save_succeed = Attachment::PieceJustificativeService.attach_champ_pj(@champ, params[:blob_signed_id])
 
     if save_succeed
-      @champ.fetch_later! if @champ.has_async_external_data? && @champ.may_fetch_later?
-
       @champ.update_timestamps
 
       dossier = DossierPreloader.load_one(@champ.dossier, pj_template: true)

app/services/attachment/piece_justificative_service.rb

@@ -14,6 +14,11 @@ def self.attach_champ_pj(champ, blob_signed_id)
       champ.reload(lock: true) # SELECT FOR UPDATE + clear association cache
       champ.updated_by = updated_by # keep record dirty so attach defers save to us
       champ.piece_justificative_file.attach(blob_signed_id)
+
+      # fetch_later should be called inside the transaction to avoid
+      # race condition with processor_job
+      champ.fetch_later if champ.has_async_external_data? && champ.may_fetch_later?
+
       context = champ.public? ? :champs_public_value : :champs_private_value
       champ.save(context:)
     end

spec/controllers/champs/piece_justificative_controller_spec.rb

@@ -74,7 +74,6 @@
 
       before do
         allow_any_instance_of(Champs::PieceJustificativeChamp).to receive(:has_async_external_data?).and_return(true)
-        expect_any_instance_of(Champs::PieceJustificativeChamp).to receive(:fetch_later!)
       end
 
       it 'attach the file' do
@@ -84,6 +83,11 @@
         expect(champ.piece_justificative_file[0].filename).to eq('piece_justificative_0.pdf')
       end
 
+      it 'marks the champ waiting_for_job' do
+        subject
+        expect(champ.reload).to be_waiting_for_job
+      end
+
       it 'renders the attachment template as Javascript' do
         subject
         expect(response.status).to eq(200)
@@ -103,10 +107,10 @@
         champ.update_column(:external_state, :fetched)
       end
 
-      it 'does not call fetch_later!' do
-        expect_any_instance_of(Champs::PieceJustificativeChamp).not_to receive(:fetch_later!)
+      it 'does not transition the champ out of fetched' do
         subject
         expect(response.status).to eq(200)
+        expect(champ.reload).to be_fetched
       end
     end
 

spec/services/attachment/piece_justificative_service_spec.rb

@@ -34,5 +34,23 @@
         expect(champ.reload.piece_justificative_file.count).to eq(2)
       end
     end
+
+    context 'with an OCR-compatible PJ (justificatif de domicile)' do
+      let(:procedure) { create(:procedure, :published, types_de_champ_public: [{ type: :piece_justificative, nature: 'justificatif_domicile' }]) }
+
+      it 'transitions the champ to waiting_for_job in the same transaction' do
+        described_class.attach_champ_pj(champ, blob_1.signed_id)
+
+        expect(champ.reload).to be_waiting_for_job
+      end
+    end
+
+    context 'with a non-OCR PJ' do
+      it 'leaves the champ idle' do
+        described_class.attach_champ_pj(champ, blob_1.signed_id)
+
+        expect(champ.reload).to be_idle
+      end
+    end
   end
 end