Merge branch '342691_handle_google_exception' into 'master'

Don't fail DeleteStoredFilesWorker if one of files is missing

See merge request gitlab-org/gitlab!73084

app/models/uploads/fog.rb

@@ -15,13 +15,21 @@ module Uploads
     end
 
     def delete_keys(keys)
-      keys.each do |key|
-        connection.delete_object(bucket_name, key)
-      end
+      keys.each { |key| delete_object(key) }
     end
 
     private
 
+    def delete_object(key)
+      connection.delete_object(bucket_name, key)
+
+    # So far, only GoogleCloudStorage raises an exception when the file is not found.
+    # Other providers support idempotent requests and does not raise an error
+    # when the file is missing.
+    rescue ::Google::Apis::ClientError => e
+      Gitlab::ErrorTracking.log_exception(e)
+    end
+
     def object_store
       Gitlab.config.uploads.object_store
     end

spec/models/uploads/fog_spec.rb

@@ -40,7 +40,9 @@ RSpec.describe Uploads::Fog do
     end
 
     describe '#delete_keys' do
+      let(:connection) { ::Fog::Storage.new(FileUploader.object_store_credentials) }
       let(:keys) { data_store.keys(relation) }
+      let(:paths) { relation.pluck(:path) }
       let!(:uploads) { create_list(:upload, 2, :with_file, :issuable_upload, model: project) }
 
       subject { data_store.delete_keys(keys) }
@@ -50,17 +52,32 @@ RSpec.describe Uploads::Fog do
       end
 
       it 'deletes multiple data' do
-        paths = relation.pluck(:path)
+        paths.each do |path|
+          expect(connection.get_object('uploads', path)[:body]).not_to be_nil
+        end
+
+        subject
+
+        paths.each do |path|
+          expect { connection.get_object('uploads', path)[:body] }.to raise_error(Excon::Error::NotFound)
+        end
+      end
 
-        ::Fog::Storage.new(FileUploader.object_store_credentials).tap do |connection|
+      context 'when one of keys is missing' do
+        let(:keys) { ['unknown'] + super() }
+
+        it 'deletes only existing keys' do
           paths.each do |path|
             expect(connection.get_object('uploads', path)[:body]).not_to be_nil
           end
-        end
 
-        subject
+          expect_next_instance_of(::Fog::Storage) do |storage|
+            allow(storage).to receive(:delete_object).and_call_original
+            expect(storage).to receive(:delete_object).with('uploads', keys.first).and_raise(::Google::Apis::ClientError, 'NotFound')
+          end
+
+          subject
 
-        ::Fog::Storage.new(FileUploader.object_store_credentials).tap do |connection|
           paths.each do |path|
             expect { connection.get_object('uploads', path)[:body] }.to raise_error(Excon::Error::NotFound)
           end