Simplify how Downloader and Uploader are instantiated

Use less meta-programming to make it easy to refactor the codebase.

ee/app/services/geo/file_service.rb → ee/app/services/geo/base_file_service.rb

 # frozen_string_literal: true
 
 module Geo
-  class FileService
+  # Base class for services that handles any type of blob replication
+  #
+  # GitLab handles this types of blobs:
+  #   * user uploads: anything the user can upload on the Web UI (ex: issue attachments, avatars, etc)
+  #   * job artifacts: anything generated by the CI (ex: logs, artifacts, etc)
+  #   * lfs blobs: anything stored in LFS
+  class BaseFileService
     include ExclusiveLeaseGuard
     include ::Gitlab::Geo::LogHelpers
 
     attr_reader :object_type, :object_db_id
 
-    DEFAULT_KLASS_NAME = 'File'.freeze
-
     def initialize(object_type, object_db_id)
       @object_type = object_type.to_sym
       @object_db_id = object_db_id
@@ -20,7 +24,7 @@ module Geo
 
     private
 
-    def upload_object?
+    def user_upload?
       Gitlab::Geo::FileReplication.object_type_from_user_uploads?(object_type)
     end
 
@@ -28,10 +32,8 @@ module Geo
       object_type == :job_artifact
     end
 
-    def service_klass_name
-      return DEFAULT_KLASS_NAME if upload_object?
-
-      object_type.to_s.camelize
+    def lfs?
+      object_type == :lfs
     end
 
     def base_log_data(message)

ee/app/services/geo/file_download_service.rb

@@ -5,7 +5,7 @@ module Geo
   #   * Finding the appropriate Downloader class for a FileRegistry record
   #   * Executing the Downloader
   #   * Marking the FileRegistry record as synced or needing retry
-  class FileDownloadService < FileService
+  class FileDownloadService < BaseFileService
     LEASE_TIMEOUT = 8.hours.freeze
 
     include Delay
@@ -26,14 +26,21 @@ module Geo
       end
     end
 
+    def downloader
+      downloader_klass.new(object_type, object_db_id)
+    end
+
     private
 
-    def downloader
-      klass = "Gitlab::Geo::#{service_klass_name}Downloader".constantize
-      klass.new(object_type, object_db_id)
-    rescue NameError => e
-      log_error('Unknown file type', e)
-      raise
+    def downloader_klass
+      return Gitlab::Geo::FileDownloader if user_upload?
+      return Gitlab::Geo::JobArtifactDownloader if job_artifact?
+      return Gitlab::Geo::LfsDownloader if lfs?
+
+      error_message = "Cannot find a Gitlab::Geo Downloader for object_type = '#{object_type}'"
+
+      log_error(error_message)
+      raise NotImplementedError, error_message
     end
 
     def log_file_download(mark_as_synced, download_result, start_time)

ee/app/services/geo/file_registry_removal_service.rb

 # frozen_string_literal: true
 
 module Geo
-  class FileRegistryRemovalService < FileService
+  class FileRegistryRemovalService < BaseFileService
     include ::Gitlab::Utils::StrongMemoize
 
     LEASE_TIMEOUT = 8.hours.freeze
@@ -62,7 +62,7 @@ module Geo
         next file_uploader.file.path if file_uploader.object_store == ObjectStorage::Store::LOCAL
 
         # For remote storage more juggling is needed to actually get the full path on disk
-        if upload?
+        if user_upload?
           upload = file_uploader.upload
           file_uploader.class.absolute_path(upload)
         else

ee/app/services/geo/file_upload_service.rb

@@ -4,7 +4,7 @@ module Geo
   # This class is responsible for:
   #   * Handling file requests from the secondary over the API
   #   * Returning the necessary response data to send the file back
-  class FileUploadService < FileService
+  class FileUploadService < BaseFileService
     attr_reader :auth_header
     include ::Gitlab::Utils::StrongMemoize
 
@@ -17,7 +17,7 @@ module Geo
     def execute
       return unless decoded_authorization.present? && jwt_scope_valid?
 
-      uploader_klass.new(object_db_id, decoded_authorization).execute
+      uploader.execute
     end
 
     private
@@ -32,11 +32,19 @@ module Geo
       end
     end
 
+    def uploader
+      uploader_klass.new(object_db_id, decoded_authorization)
+    end
+
     def uploader_klass
-      "Gitlab::Geo::#{service_klass_name}Uploader".constantize
-    rescue NameError => e
-      log_error('Unknown file type', e)
-      raise
+      return Gitlab::Geo::FileUploader if user_upload?
+      return Gitlab::Geo::JobArtifactUploader if job_artifact?
+      return Gitlab::Geo::LfsUploader if lfs?
+
+      error_message = "Cannot find a Gitlab::Geo Uploader for object_type = '#{object_type}'"
+
+      log_error(error_message)
+      raise NotImplementedError, error_message
     end
   end
 end

ee/spec/services/geo/base_file_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Geo::BaseFileService do
+  subject { described_class.new('file', 8) }
+
+  describe '#execute' do
+    it 'requires a subclass overrides it' do
+      expect { subject.execute }.to raise_error(NotImplementedError)
+    end
+  end
+end

ee/spec/services/geo/file_download_service_spec.rb

@@ -11,6 +11,16 @@ describe Geo::FileDownloadService do
     stub_current_geo_node(secondary)
   end
 
+  describe '#downloader' do
+    Gitlab::Geo::FileReplication::USER_UPLOADS_OBJECT_TYPES.each do |object_type|
+      subject {described_class.new(object_type, 1) }
+
+      it "returns FileDownloader given object_type is #{object_type}" do
+        expect(subject.downloader).to eq(Gitlab::Geo::FileDownloader)
+      end
+    end
+  end
+
   context 'retry time' do
     before do
       stub_transfer_result(bytes_downloaded: 0, success: false)