Improved specs, error messages and documentation

app/uploaders/gitlab_uploader.rb

@@ -99,10 +99,10 @@ class GitlabUploader < CarrierWave::Uploader::Base
     end
   end
 
-  # Used to replace an existing upload with the informed file without modifying stored metadata
+  # Used to replace an existing upload with another +file+ without modifying stored metadata
   # Use this method only to repair/replace an existing upload, or to upload to a Geo secondary node
   #
-  # @param [CarrierWave::SanitizedFile] file
+  # @param [CarrierWave::SanitizedFile] file that will replace existing upload
   # @return CarrierWave::SanitizedFile
   def replace_file_without_saving!(file)
     raise ArgumentError, 'should be a CarrierWave::SanitizedFile' unless file.is_a? CarrierWave::SanitizedFile

ee/lib/gitlab/geo/replication/base_transfer.rb

@@ -199,13 +199,10 @@ module Gitlab
             # Upload file to Object Storage
             uploader.replace_file_without_saving!(CarrierWave::SanitizedFile.new(temp_file))
 
-            log_info("Successful downloaded", filename: filename, file_size_bytes: file_size)
+            log_info("Successfully transferred", file_type: file_type, file_id: file_id,
+                     file_size_bytes: file_size)
           rescue => e
-            log_error("Error downloading file", error: e, filename: filename, url: url)
-
-            return failure_result
-          rescue Errno::EEXIST => e
-            log_error("Destination file is a directory", error: e, filename: filename)
+            log_error("Error transferring file", error: e, file_type: file_type, file_id: file_id, url: url)
 
             return failure_result
           ensure

ee/spec/models/geo/upload_registry_spec.rb

@@ -42,7 +42,7 @@ describe Geo::UploadRegistry, :geo, :geo_fdw do
       upload = create(:upload, :with_file)
       registry = create(:geo_upload_registry, :file, file_id: upload.id)
 
-      expect(registry.file).to eq("uploads/-/system/project/avatar/#{upload.id}/avatar.jpg")
+      expect(registry.file).to eq(upload.path)
     end
 
     it 'return "removed" message when the upload no longer exists' do

spec/models/upload_spec.rb

@@ -114,10 +114,22 @@ describe Upload do
 
       expect(uploader.upload).to eq(subject)
       expect(uploader.mounted_as).to eq(subject.send(:mount_point))
+      expect(uploader.file).to be_nil
     end
   end
 
-  describe '#needs_checksum??' do
+  describe '#retrieve_uploader' do
+    it 'returns a uploader object with current uploader associated with and cache retrieved' do
+      subject = build(:upload)
+      uploader = subject.retrieve_uploader
+
+      expect(uploader.upload).to eq(subject)
+      expect(uploader.mounted_as).to eq(subject.send(:mount_point))
+      expect(uploader.file).not_to be_nil
+    end
+  end
+
+  describe '#needs_checksum?' do
     context 'with local storage' do
       it 'returns true when no checksum exists' do
         subject = create(:upload, :with_file, checksum: nil)