Use `Replication` namespace on downloaders, transfers and uploaders

Git doesn't track file moves correctly so we have to split the operation
 in two different commits.

ee/app/services/geo/file_download_service.rb

@@ -33,9 +33,9 @@ module Geo
     private
 
     def downloader_klass
-      return Gitlab::Geo::FileDownloader if user_upload?
-      return Gitlab::Geo::JobArtifactDownloader if job_artifact?
-      return Gitlab::Geo::LfsDownloader if lfs?
+      return Gitlab::Geo::Replication::FileDownloader if user_upload?
+      return Gitlab::Geo::Replication::JobArtifactDownloader if job_artifact?
+      return Gitlab::Geo::Replication::LfsDownloader if lfs?
 
       error_message = "Cannot find a Gitlab::Geo Downloader for object_type = '#{object_type}'"
 

ee/app/services/geo/file_upload_service.rb

@@ -20,6 +20,10 @@ module Geo
       uploader.execute
     end
 
+    def uploader
+      uploader_klass.new(object_db_id, decoded_authorization)
+    end
+
     private
 
     def jwt_scope_valid?
@@ -32,14 +36,10 @@ module Geo
       end
     end
 
-    def uploader
-      uploader_klass.new(object_db_id, decoded_authorization)
-    end
-
     def uploader_klass
-      return Gitlab::Geo::FileUploader if user_upload?
-      return Gitlab::Geo::JobArtifactUploader if job_artifact?
-      return Gitlab::Geo::LfsUploader if lfs?
+      return Gitlab::Geo::Replication::FileUploader if user_upload?
+      return Gitlab::Geo::Replication::JobArtifactUploader if job_artifact?
+      return Gitlab::Geo::Replication::LfsUploader if lfs?
 
       error_message = "Cannot find a Gitlab::Geo Uploader for object_type = '#{object_type}'"
 

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

@@ -2,62 +2,64 @@
 
 module Gitlab
   module Geo
-    # This class is responsible for:
-    #   * Finding an Upload record
-    #   * Requesting and downloading the Upload's file from the primary
-    #   * Returning a detailed Result
-    #
-    # TODO: Rearrange things so this class not inherited by JobArtifactDownloader and LfsDownloader
-    # Maybe rename it so it doesn't seem generic. It only works with Upload records.
-    class FileDownloader
-      attr_reader :object_type, :object_db_id
+    module Replication
+      # This class is responsible for:
+      #   * Finding an Upload record
+      #   * Requesting and downloading the Upload's file from the primary
+      #   * Returning a detailed Result
+      #
+      # TODO: Rearrange things so this class not inherited by JobArtifactDownloader and LfsDownloader
+      # Maybe rename it so it doesn't seem generic. It only works with Upload records.
+      class FileDownloader
+        attr_reader :object_type, :object_db_id
 
-      def initialize(object_type, object_db_id)
-        @object_type = object_type
-        @object_db_id = object_db_id
-      end
+        def initialize(object_type, object_db_id)
+          @object_type = object_type
+          @object_db_id = object_db_id
+        end
 
-      # Executes the actual file download
-      #
-      # Subclasses should return the number of bytes downloaded,
-      # or nil or -1 if a failure occurred.
-      # rubocop: disable CodeReuse/ActiveRecord
-      def execute
-        upload = Upload.find_by(id: object_db_id)
-        return fail_before_transfer unless upload.present?
-        return missing_on_primary if upload.model.nil?
+        # Executes the actual file download
+        #
+        # Subclasses should return the number of bytes downloaded,
+        # or nil or -1 if a failure occurred.
+        # rubocop: disable CodeReuse/ActiveRecord
+        def execute
+          upload = Upload.find_by(id: object_db_id)
+          return fail_before_transfer unless upload.present?
+          return missing_on_primary if upload.model.nil?
 
-        transfer = ::Gitlab::Geo::Replication::FileTransfer.new(object_type.to_sym, upload)
-        Result.from_transfer_result(transfer.download_from_primary)
-      end
+          transfer = ::Gitlab::Geo::Replication::FileTransfer.new(object_type.to_sym, upload)
+          Result.from_transfer_result(transfer.download_from_primary)
+        end
 
-      # rubocop: enable CodeReuse/ActiveRecord
+        # rubocop: enable CodeReuse/ActiveRecord
 
-      class Result
-        attr_reader :success, :bytes_downloaded, :primary_missing_file, :failed_before_transfer
+        class Result
+          attr_reader :success, :bytes_downloaded, :primary_missing_file, :failed_before_transfer
 
-        def self.from_transfer_result(transfer_result)
-          Result.new(success: transfer_result.success,
-                     primary_missing_file: transfer_result.primary_missing_file,
-                     bytes_downloaded: transfer_result.bytes_downloaded)
-        end
+          def self.from_transfer_result(transfer_result)
+            Result.new(success: transfer_result.success,
+                       primary_missing_file: transfer_result.primary_missing_file,
+                       bytes_downloaded: transfer_result.bytes_downloaded)
+          end
 
-        def initialize(success:, bytes_downloaded:, primary_missing_file: false, failed_before_transfer: false)
-          @success = success
-          @bytes_downloaded = bytes_downloaded
-          @primary_missing_file = primary_missing_file
-          @failed_before_transfer = failed_before_transfer
+          def initialize(success:, bytes_downloaded:, primary_missing_file: false, failed_before_transfer: false)
+            @success = success
+            @bytes_downloaded = bytes_downloaded
+            @primary_missing_file = primary_missing_file
+            @failed_before_transfer = failed_before_transfer
+          end
         end
-      end
 
-      private
+        private
 
-      def fail_before_transfer
-        Result.new(success: false, bytes_downloaded: 0, failed_before_transfer: true)
-      end
+        def fail_before_transfer
+          Result.new(success: false, bytes_downloaded: 0, failed_before_transfer: true)
+        end
 
-      def missing_on_primary
-        Result.new(success: true, bytes_downloaded: 0, primary_missing_file: true)
+        def missing_on_primary
+          Result.new(success: true, bytes_downloaded: 0, primary_missing_file: true)
+        end
       end
     end
   end

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

@@ -2,33 +2,35 @@
 
 module Gitlab
   module Geo
-    # This class is responsible for:
-    #   * Requesting an Upload file from the primary
-    #   * Saving it in the right place on successful download
-    #   * Returning a detailed Result object
-    class FileTransfer < Transfer
-      def initialize(file_type, upload)
-        super(
-          file_type,
-          upload.id,
-          upload.absolute_path,
-          upload.checksum,
-          build_request_data(file_type, upload)
-        )
-      rescue ObjectStorage::RemoteStoreError
-        ::Gitlab::Geo::Logger.warn "Cannot transfer a remote object."
-      end
+    module Replication
+      # This class is responsible for:
+      #   * Requesting an Upload file from the primary
+      #   * Saving it in the right place on successful download
+      #   * Returning a detailed Result object
+      class FileTransfer < Transfer
+        def initialize(file_type, upload)
+          super(
+            file_type,
+            upload.id,
+            upload.absolute_path,
+            upload.checksum,
+            build_request_data(file_type, upload)
+          )
+        rescue ObjectStorage::RemoteStoreError
+          ::Gitlab::Geo::Logger.warn "Cannot transfer a remote object."
+        end
 
-      private
+        private
 
-      def build_request_data(file_type, upload)
-        {
-          id: upload.model_id,
-          type: upload.model_type,
-          checksum: upload.checksum,
-          file_type: file_type,
-          file_id: upload.id
-        }
+        def build_request_data(file_type, upload)
+          {
+            id: upload.model_id,
+            type: upload.model_type,
+            checksum: upload.checksum,
+            file_type: file_type,
+            file_id: upload.id
+          }
+        end
       end
     end
   end

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

@@ -2,70 +2,72 @@
 
 module Gitlab
   module Geo
-    # This class is responsible for:
-    #   * Finding an Upload record
-    #   * Returning the necessary response data to send the file back
-    #
-    # TODO: Rearrange things so this class not inherited by JobArtifactUploader and LfsUploader
-    # Maybe rename it so it doesn't seem generic. It only works with Upload records.
-    class FileUploader
-      include LogHelpers
+    module Replication
+      # This class is responsible for:
+      #   * Finding an Upload record
+      #   * Returning the necessary response data to send the file back
+      #
+      # TODO: Rearrange things so this class not inherited by JobArtifactUploader and LfsUploader
+      # Maybe rename it so it doesn't seem generic. It only works with Upload records.
+      class FileUploader
+        include LogHelpers
 
-      FILE_NOT_FOUND_GEO_CODE = 'FILE_NOT_FOUND'.freeze
+        FILE_NOT_FOUND_GEO_CODE = 'FILE_NOT_FOUND'.freeze
 
-      attr_reader :object_db_id, :message
+        attr_reader :object_db_id, :message
 
-      def initialize(object_db_id, message)
-        @object_db_id = object_db_id
-        @message = message
-      end
+        def initialize(object_db_id, message)
+          @object_db_id = object_db_id
+          @message = message
+        end
 
-      # rubocop: disable CodeReuse/ActiveRecord
-      def execute
-        recorded_file = Upload.find_by(id: object_db_id)
+        # rubocop: disable CodeReuse/ActiveRecord
+        def execute
+          recorded_file = Upload.find_by(id: object_db_id)
 
-        return error('Upload not found') unless recorded_file
-        return file_not_found(recorded_file) unless recorded_file.exist?
-        return error('Upload not found') unless valid?(recorded_file)
+          return error('Upload not found') unless recorded_file
+          return file_not_found(recorded_file) unless recorded_file.exist?
+          return error('Upload not found') unless valid?(recorded_file)
 
-        success(CarrierWave::SanitizedFile.new(recorded_file.absolute_path))
-      end
+          success(CarrierWave::SanitizedFile.new(recorded_file.absolute_path))
+        end
 
-      # rubocop: enable CodeReuse/ActiveRecord
+        # rubocop: enable CodeReuse/ActiveRecord
 
-      private
+        private
 
-      def valid?(recorded_file)
-        matches_requested_model?(recorded_file) &&
-          matches_checksum?(recorded_file)
-      end
+        def valid?(recorded_file)
+          matches_requested_model?(recorded_file) &&
+            matches_checksum?(recorded_file)
+        end
 
-      def matches_requested_model?(recorded_file)
-        message[:id] == recorded_file.model_id &&
-          message[:type] == recorded_file.model_type
-      end
+        def matches_requested_model?(recorded_file)
+          message[:id] == recorded_file.model_id &&
+            message[:type] == recorded_file.model_type
+        end
 
-      def matches_checksum?(recorded_file)
-        message[:checksum] == Upload.hexdigest(recorded_file.absolute_path)
-      end
+        def matches_checksum?(recorded_file)
+          message[:checksum] == Upload.hexdigest(recorded_file.absolute_path)
+        end
 
-      def success(file)
-        { code: :ok, message: 'Success', file: file }
-      end
+        def success(file)
+          { code: :ok, message: 'Success', file: file }
+        end
 
-      def error(message)
-        { code: :not_found, message: message }
-      end
+        def error(message)
+          { code: :not_found, message: message }
+        end
 
-      # A 404 implies the client made a mistake requesting that resource.
-      # In this case, we know that the resource should exist, so it is a 500 server error.
-      # We send a special "geo_code" so the secondary can mark the file as synced.
-      def file_not_found(resource)
-        {
-          code: :not_found,
-          geo_code: FILE_NOT_FOUND_GEO_CODE,
-          message: "#{resource.class.name} ##{resource.id} file not found"
-        }
+        # A 404 implies the client made a mistake requesting that resource.
+        # In this case, we know that the resource should exist, so it is a 500 server error.
+        # We send a special "geo_code" so the secondary can mark the file as synced.
+        def file_not_found(resource)
+          {
+            code: :not_found,
+            geo_code: FILE_NOT_FOUND_GEO_CODE,
+            message: "#{resource.class.name} ##{resource.id} file not found"
+          }
+        end
       end
     end
   end

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

@@ -2,22 +2,24 @@
 
 module Gitlab
   module Geo
-    # This class is responsible for:
-    #   * Finding a ::Ci::JobArtifact record
-    #   * Requesting and downloading the JobArtifact's file from the primary
-    #   * Returning a detailed Result
-    #
-    # TODO: Rearrange things so this class does not inherit FileDownloader
-    class JobArtifactDownloader < FileDownloader
-      # rubocop: disable CodeReuse/ActiveRecord
-      def execute
-        job_artifact = ::Ci::JobArtifact.find_by(id: object_db_id)
-        return fail_before_transfer unless job_artifact.present?
+    module Replication
+      # This class is responsible for:
+      #   * Finding a ::Ci::JobArtifact record
+      #   * Requesting and downloading the JobArtifact's file from the primary
+      #   * Returning a detailed Result
+      #
+      # TODO: Rearrange things so this class does not inherit FileDownloader
+      class JobArtifactDownloader < FileDownloader
+        # rubocop: disable CodeReuse/ActiveRecord
+        def execute
+          job_artifact = ::Ci::JobArtifact.find_by(id: object_db_id)
+          return fail_before_transfer unless job_artifact.present?
 
-        transfer = ::Gitlab::Geo::JobArtifactTransfer.new(job_artifact)
-        Result.from_transfer_result(transfer.download_from_primary)
+          transfer = ::Gitlab::Geo::Replication::JobArtifactTransfer.new(job_artifact)
+          Result.from_transfer_result(transfer.download_from_primary)
+        end
+        # rubocop: enable CodeReuse/ActiveRecord
       end
-      # rubocop: enable CodeReuse/ActiveRecord
     end
   end
 end

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

@@ -2,29 +2,31 @@
 
 module Gitlab
   module Geo
-    # This class is responsible for:
-    #   * Requesting an ::Ci::JobArtifact file from the primary
-    #   * Saving it in the right place on successful download
-    #   * Returning a detailed Result object
-    class JobArtifactTransfer < Transfer
-      def initialize(job_artifact)
-        super(
-          :job_artifact,
-          job_artifact.id,
-          job_artifact.file.path,
-          job_artifact.file_sha256,
-          job_artifact_request_data(job_artifact)
-        )
-      end
+    module Replication
+      # This class is responsible for:
+      #   * Requesting an ::Ci::JobArtifact file from the primary
+      #   * Saving it in the right place on successful download
+      #   * Returning a detailed Result object
+      class JobArtifactTransfer < Transfer
+        def initialize(job_artifact)
+          super(
+            :job_artifact,
+            job_artifact.id,
+            job_artifact.file.path,
+            job_artifact.file_sha256,
+            job_artifact_request_data(job_artifact)
+          )
+        end
 
-      private
+        private
 
-      def job_artifact_request_data(job_artifact)
-        {
-          id: job_artifact.id,
-          file_type: :job_artifact,
-          file_id: job_artifact.id
-        }
+        def job_artifact_request_data(job_artifact)
+          {
+            id: job_artifact.id,
+            file_type: :job_artifact,
+            file_id: job_artifact.id
+          }
+        end
       end
     end
   end

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

@@ -2,29 +2,31 @@
 
 module Gitlab
   module Geo
-    # This class is responsible for:
-    #   * Finding an ::Ci::JobArtifact record
-    #   * Returning the necessary response data to send the file back
-    #
-    # TODO: Rearrange things so this class does not inherit from FileUploader
-    class JobArtifactUploader < FileUploader
-      # rubocop: disable CodeReuse/ActiveRecord
-      def execute
-        job_artifact = ::Ci::JobArtifact.find_by(id: object_db_id)
+    module Replication
+      # This class is responsible for:
+      #   * Finding an ::Ci::JobArtifact record
+      #   * Returning the necessary response data to send the file back
+      #
+      # TODO: Rearrange things so this class does not inherit from FileUploader
+      class JobArtifactUploader < FileUploader
+        # rubocop: disable CodeReuse/ActiveRecord
+        def execute
+          job_artifact = ::Ci::JobArtifact.find_by(id: object_db_id)
 
-        unless job_artifact.present?
-          return error('Job artifact not found')
-        end
+          unless job_artifact.present?
+            return error('Job artifact not found')
+          end
 
-        unless job_artifact.file.present? && job_artifact.file.exists?
-          log_error("Could not upload job artifact because it does not have a file", id: job_artifact.id)
+          unless job_artifact.file.present? && job_artifact.file.exists?
+            log_error("Could not upload job artifact because it does not have a file", id: job_artifact.id)
 
-          return file_not_found(job_artifact)
-        end
+            return file_not_found(job_artifact)
+          end
 
-        success(job_artifact.file)
+          success(job_artifact.file)
+        end
+        # rubocop: enable CodeReuse/ActiveRecord
       end
-      # rubocop: enable CodeReuse/ActiveRecord
     end
   end
 end

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

@@ -2,22 +2,24 @@
 
 module Gitlab
   module Geo
-    # This class is responsible for:
-    #   * Finding a LfsObject record
-    #   * Requesting and downloading the LfsObject's file from the primary
-    #   * Returning a detailed Result
-    #
-    # TODO: Rearrange things so this class does not inherit FileDownloader
-    class LfsDownloader < FileDownloader
-      # rubocop: disable CodeReuse/ActiveRecord
-      def execute
-        lfs_object = LfsObject.find_by(id: object_db_id)
-        return fail_before_transfer unless lfs_object.present?
+    module Replication
+      # This class is responsible for:
+      #   * Finding a LfsObject record
+      #   * Requesting and downloading the LfsObject's file from the primary
+      #   * Returning a detailed Result
+      #
+      # TODO: Rearrange things so this class does not inherit FileDownloader
+      class LfsDownloader < FileDownloader
+        # rubocop: disable CodeReuse/ActiveRecord
+        def execute
+          lfs_object = LfsObject.find_by(id: object_db_id)
+          return fail_before_transfer unless lfs_object.present?
 
-        transfer = ::Gitlab::Geo::Replication::LfsTransfer.new(lfs_object)
-        Result.from_transfer_result(transfer.download_from_primary)
+          transfer = ::Gitlab::Geo::Replication::LfsTransfer.new(lfs_object)
+          Result.from_transfer_result(transfer.download_from_primary)
+        end
+        # rubocop: enable CodeReuse/ActiveRecord
       end
-      # rubocop: enable CodeReuse/ActiveRecord
     end
   end
 end

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

@@ -2,29 +2,31 @@
 
 module Gitlab
   module Geo
-    # This class is responsible for:
-    #   * Requesting an LfsObject file from the primary
-    #   * Saving it in the right place on successful download
-    #   * Returning a detailed Result object
-    class LfsTransfer < Transfer
-      def initialize(lfs_object)
-        super(
-          :lfs,
-          lfs_object.id,
-          lfs_object.file.path,
-          lfs_object.oid,
-          lfs_request_data(lfs_object)
-        )
-      end
+    module Replication
+      # This class is responsible for:
+      #   * Requesting an LfsObject file from the primary
+      #   * Saving it in the right place on successful download
+      #   * Returning a detailed Result object
+      class LfsTransfer < Transfer
+        def initialize(lfs_object)
+          super(
+            :lfs,
+            lfs_object.id,
+            lfs_object.file.path,
+            lfs_object.oid,
+            lfs_request_data(lfs_object)
+          )
+        end
 
-      private
+        private
 
-      def lfs_request_data(lfs_object)
-        {
-          checksum: lfs_object.oid,
-          file_type: :lfs,
-          file_id: lfs_object.id
-        }
+        def lfs_request_data(lfs_object)
+          {
+            checksum: lfs_object.oid,
+            file_type: :lfs,
+            file_id: lfs_object.id
+          }
+        end
       end
     end
   end

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

@@ -2,28 +2,30 @@
 
 module Gitlab
   module Geo
-    # This class is responsible for:
-    #   * Finding an LfsObject record
-    #   * Returning the necessary response data to send the file back
-    #
-    # TODO: Rearrange things so this class does not inherit from FileUploader
-    class LfsUploader < FileUploader
-      # rubocop: disable CodeReuse/ActiveRecord
-      def execute
-        lfs_object = LfsObject.find_by(id: object_db_id)
+    module Replication
+      # This class is responsible for:
+      #   * Finding an LfsObject record
+      #   * Returning the necessary response data to send the file back
+      #
+      # TODO: Rearrange things so this class does not inherit from FileUploader
+      class LfsUploader < FileUploader
+        # rubocop: disable CodeReuse/ActiveRecord
+        def execute
+          lfs_object = LfsObject.find_by(id: object_db_id)
 
-        return error('LFS object not found') unless lfs_object
-        return error('LFS object not found') if message[:checksum] != lfs_object.oid
+          return error('LFS object not found') unless lfs_object
+          return error('LFS object not found') if message[:checksum] != lfs_object.oid
 
-        unless lfs_object.file.present? && lfs_object.file.exists?
-          log_error("Could not upload LFS object because it does not have a file", id: lfs_object.id)
+          unless lfs_object.file.present? && lfs_object.file.exists?
+            log_error("Could not upload LFS object because it does not have a file", id: lfs_object.id)
 
-          return file_not_found(lfs_object)
-        end
+            return file_not_found(lfs_object)
+          end
 
-        success(lfs_object.file)
+          success(lfs_object.file)
+        end
+        # rubocop: enable CodeReuse/ActiveRecord
       end
-      # rubocop: enable CodeReuse/ActiveRecord
     end
   end
 end

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

@@ -9,7 +9,7 @@ describe Gitlab::Geo::Replication::JobArtifactDownloader, :geo do
         downloader = described_class.new(:job_artifact, job_artifact.id)
         result = Gitlab::Geo::Replication::Transfer::Result.new(success: true, bytes_downloaded: 1)
 
-        allow_any_instance_of(Gitlab::Geo::JobArtifactTransfer)
+        allow_any_instance_of(Gitlab::Geo::Replication::JobArtifactTransfer)
           .to receive(:download_from_primary).and_return(result)
 
         expect(downloader.execute).to be_a(Gitlab::Geo::Replication::FileDownloader::Result)

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

 require 'spec_helper'
 
-describe Gitlab::Geo::JobArtifactTransfer, :geo do
+describe Gitlab::Geo::Replication::JobArtifactTransfer, :geo do
   include ::EE::GeoHelpers
 
   set(:primary_node) { create(:geo_node, :primary) }

ee/spec/services/geo/file_download_service_spec.rb

@@ -435,7 +435,7 @@ describe Geo::FileDownloadService do
 
     context 'bad object type' do
       it 'raises an error' do
-        expect { described_class.new(:bad, 1).execute }.to raise_error(NameError)
+        expect { described_class.new(:bad, 1).execute }.to raise_error(NotImplementedError)
       end
     end
   end
@@ -445,7 +445,7 @@ describe Geo::FileDownloadService do
                     bytes_downloaded: bytes_downloaded,
                     success: success,
                     primary_missing_file: primary_missing_file)
-    instance = double("(instance of Gitlab::Geo::Transfer)", download_from_primary: result)
+    instance = double("(instance of Gitlab::Geo::Replication::Transfer)", download_from_primary: result)
     allow(Gitlab::Geo::Replication::Transfer).to receive(:new).and_return(instance)
   end
 end

ee/spec/services/geo/file_upload_service_spec.rb

@@ -200,7 +200,7 @@ describe Geo::FileUploadService do
     context 'job artifact' do
       let(:job_artifact) { create(:ci_job_artifact, :with_file) }
       let(:params) { { id: job_artifact.id, type: 'job_artifact' } }
-      let(:request_data) { Gitlab::Geo::JobArtifactTransfer.new(job_artifact).request_data }
+      let(:request_data) { Gitlab::Geo::Replication::JobArtifactTransfer.new(job_artifact).request_data }
 
       it 'sends job artifact file' do
         service = described_class.new(params, req_header)