Merge branch 'da-geo-replication-base-transfer' into 'master'

Fix Gitlab::Geo::Replication::BaseTransfer for orphaned registries

See merge request gitlab-org/gitlab!34292

ee/changelogs/unreleased/da-geo-replication-base-transfer.yml

+---
+title: Geo - Fix Gitlab::Geo::Replication::BaseTransfer for orphaned registries
+merge_request: 34292
+author:
+type: fixed

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

@@ -6,15 +6,15 @@ module Gitlab
       class BaseTransfer
         include LogHelpers
 
-        attr_reader :file_type, :file_id, :filename, :uploader, :expected_checksum, :request_data
+        attr_reader :file_type, :file_id, :filename, :expected_checksum, :request_data, :resource
 
         TEMP_PREFIX = 'tmp_'.freeze
 
-        def initialize(file_type:, file_id:, request_data:, expected_checksum: nil, filename: nil, uploader: nil)
+        def initialize(resource:, file_type:, file_id:, request_data:, expected_checksum: nil, filename: nil)
+          @resource = resource
           @file_type = file_type
           @file_id = file_id
           @filename = filename
-          @uploader = uploader
           @expected_checksum = expected_checksum
           @request_data = request_data
         end
@@ -85,6 +85,10 @@ module Gitlab
 
         private
 
+        def uploader
+          raise NotImplementedError, "#{self.class} does not implement #{__method__}"
+        end
+
         def skipped_result
           Result.new(success: false, bytes_downloaded: 0, skipped: true)
         end

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

@@ -9,7 +9,6 @@ module Gitlab
       #   * Returning a detailed Result object
       class FileTransfer < BaseTransfer
         # Initialize a transfer service for a specified Upload
-        #
         # @param [Symbol] file_type
         # @param [Upload] upload
         def initialize(file_type, upload)
@@ -25,12 +24,16 @@ module Gitlab
 
         private
 
+        def uploader
+          resource.retrieve_uploader
+        end
+
         def local_file_attributes(file_type, upload)
           {
+            resource: upload,
             file_type: file_type,
             file_id: upload.id,
             filename: upload.absolute_path,
-            uploader: upload.retrieve_uploader,
             expected_checksum: upload.checksum,
             request_data: build_request_data(file_type, upload)
           }
@@ -38,9 +41,9 @@ module Gitlab
 
         def remote_file_attributes(file_type, upload)
           {
+            resource: upload,
             file_type: file_type,
             file_id: upload.id,
-            uploader: upload.retrieve_uploader,
             request_data: build_request_data(file_type, upload)
           }
         end

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

@@ -21,12 +21,16 @@ module Gitlab
 
         private
 
+        def uploader
+          resource.file
+        end
+
         def local_job_artifact_attributes(job_artifact)
           {
+            resource: job_artifact,
             file_type: :job_artifact,
             file_id: job_artifact.id,
             filename: job_artifact.file.path,
-            uploader: job_artifact.file,
             expected_checksum: job_artifact.file_sha256,
             request_data: job_artifact_request_data(job_artifact)
           }
@@ -34,9 +38,9 @@ module Gitlab
 
         def remote_job_artifact_attributes(job_artifact)
           {
+            resource: job_artifact,
             file_type: :job_artifact,
             file_id: job_artifact.id,
-            uploader: job_artifact.file,
             request_data: job_artifact_request_data(job_artifact)
           }
         end

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

@@ -21,12 +21,16 @@ module Gitlab
 
         private
 
+        def uploader
+          resource.file
+        end
+
         def local_lfs_object_attributes(lfs_object)
           {
+            resource: lfs_object,
             file_type: :lfs,
             file_id: lfs_object.id,
             filename: lfs_object.file.path,
-            uploader: lfs_object.file,
             expected_checksum: lfs_object.oid,
             request_data: lfs_request_data(lfs_object)
           }
@@ -34,9 +38,9 @@ module Gitlab
 
         def remote_lfs_object_attributes(lfs_object)
           {
+            resource: lfs_object,
             file_type: :lfs,
             file_id: lfs_object.id,
-            uploader: lfs_object.file,
             expected_checksum: lfs_object.oid,
             request_data: lfs_request_data(lfs_object)
           }

ee/spec/lib/gitlab/geo/replication/base_transfer_spec.rb

@@ -10,8 +10,9 @@ RSpec.describe Gitlab::Geo::Replication::BaseTransfer do
 
   describe '#resource_url' do
     subject do
-      described_class.new(file_type: 'design_management/design_v432x230', file_id: 1,
-                          filename: Tempfile.new, expected_checksum: nil, request_data: nil)
+      described_class.new(file_type: 'design_management/design_v432x230',
+                          file_id: 1, filename: Tempfile.new, expected_checksum: nil,
+                          request_data: nil, resource: nil)
     end
 
     context 'when file type contains /' do
@@ -25,7 +26,7 @@ RSpec.describe Gitlab::Geo::Replication::BaseTransfer do
   describe '#can_transfer?' do
     subject do
       described_class.new(file_type: :avatar, file_id: 1, filename: Tempfile.new,
-                          expected_checksum: nil, request_data: nil)
+                          expected_checksum: nil, request_data: nil, resource: nil)
     end
 
     before do
@@ -51,7 +52,7 @@ RSpec.describe Gitlab::Geo::Replication::BaseTransfer do
     context 'when destination filename is a directory' do
       it 'returns false' do
         subject = described_class.new(file_type: :avatar, file_id: 1, filename: Dir::Tmpname.tmpdir,
-                                      expected_checksum: nil, request_data: nil)
+                                      expected_checksum: nil, request_data: nil, resource: nil)
 
         expect(subject.can_transfer?).to be_falsey
       end
@@ -59,8 +60,8 @@ RSpec.describe Gitlab::Geo::Replication::BaseTransfer do
 
     context 'when no filename is informed' do
       it 'returns true' do
-        subject = described_class.new(file_type: :avatar, file_id: 1,
-                                      expected_checksum: nil, request_data: nil)
+        subject = described_class.new(file_type: :avatar, file_id: 1, expected_checksum: nil,
+                                      request_data: nil, resource: nil)
 
         expect(subject.can_transfer?).to be_truthy
       end