From 80efe27ff255ad46041327e090cac8f87211e6bd Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Tue, 16 Jul 2019 17:51:29 +0200
Subject: [PATCH] pj migration: retry when changing the content type fails

---
 ...erwave_active_storage_migration_service.rb | 12 ++++++++-
 ...e_active_storage_migration_service_spec.rb | 27 +++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/app/services/carrierwave_active_storage_migration_service.rb b/app/services/carrierwave_active_storage_migration_service.rb
index 1183cf8f9..d5a4c59a5 100644
--- a/app/services/carrierwave_active_storage_migration_service.rb
+++ b/app/services/carrierwave_active_storage_migration_service.rb
@@ -154,10 +154,20 @@ class CarrierwaveActiveStorageMigrationService
     attachment
   end
 
-  def fix_content_type(blob)
+  def fix_content_type(blob, retry_delay: 5)
+    retries ||= 0
     # In OpenStack, ActiveStorage cannot inject the MIME type on the fly during direct
     # download. Instead, the MIME type needs to be stored statically on the file object
     # in OpenStack. This is what this call does.
     blob.service.change_content_type(blob.key, blob.content_type)
+  rescue
+    # When we quickly create a new attachment, and then change its content type,
+    # the Object Storage may not be synchronized yet. It this cas, it will return a
+    # "409 Conflict" error.
+    #
+    # Wait for a while, then try again twice (before giving up).
+    sleep(retry_delay)
+    retry if (retries += 1) < 3
+    raise
   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 fb70b0d6f..abf7ff1ca 100644
--- a/spec/services/carrierwave_active_storage_migration_service_spec.rb
+++ b/spec/services/carrierwave_active_storage_migration_service_spec.rb
@@ -69,4 +69,31 @@ describe CarrierwaveActiveStorageMigrationService do
       end
     end
   end
+
+  describe '.fix_content_type' do
+    let(:pj) { create(:piece_justificative, :rib, updated_at: Time.zone.local(2019, 01, 01, 12, 00)) }
+    let(:blob) { service.make_empty_blob(pj.content, pj.updated_at.iso8601, filename: pj.original_filename) }
+
+    context 'when the request is ok' do
+      it 'succeeds' do
+        expect(blob.service).to receive(:change_content_type).and_return(true)
+        expect { service.fix_content_type(blob) }.not_to raise_error
+      end
+    end
+
+    context 'when the request fails initially' do
+      it 'retries the request' do
+        expect(blob.service).to receive(:change_content_type).and_raise(StandardError).ordered
+        expect(blob.service).to receive(:change_content_type).and_return(true).ordered
+        expect { service.fix_content_type(blob, retry_delay: 0.01) }.not_to raise_error
+      end
+    end
+
+    context 'when the request fails too many times' do
+      it 'gives up' do
+        expect(blob.service).to receive(:change_content_type).and_raise(StandardError).thrice
+        expect { service.fix_content_type(blob, retry_delay: 0.01) }.to raise_error(StandardError)
+      end
+    end
+  end
 end