From 8da236611b5182a1105111904027ae3e74ed1682 Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@selenight.nl>
Date: Wed, 15 Nov 2017 13:27:37 +0100
Subject: [PATCH] Prefer polymorphism over specific type checks in Import
 service

---
 app/models/project.rb                         |  4 ----
 app/services/projects/import_service.rb       | 22 +++++++++----------
 lib/gitlab/github_import/parallel_importer.rb |  4 ++++
 lib/gitlab/import_export/importer.rb          |  4 ++++
 4 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/app/models/project.rb b/app/models/project.rb
index bae16b6b2af..8da03e23057 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -704,10 +704,6 @@ class Project < ActiveRecord::Base
     import_type == 'gitea'
   end
 
-  def github_import?
-    import_type == 'github'
-  end
-
   def check_limit
     unless creator.can_create_project? || namespace.kind == 'group'
       projects_limit = creator.projects_limit
diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb
index c950da44aba..c3b11341b4d 100644
--- a/app/services/projects/import_service.rb
+++ b/app/services/projects/import_service.rb
@@ -11,13 +11,11 @@ module Projects
     # supported by an importer class (`Gitlab::GithubImport::ParallelImporter`
     # for example).
     def async?
-      return false unless has_importer?
-
-      !!importer_class.try(:async?)
+      has_importer? && !!importer_class.try(:async?)
     end
 
     def execute
-      add_repository_to_project unless project.gitlab_project_import?
+      add_repository_to_project
 
       import_data
 
@@ -29,6 +27,14 @@ module Projects
     private
 
     def add_repository_to_project
+      if project.external_import? && !unknown_url?
+        raise Error, 'Blocked import URL.' if Gitlab::UrlBlocker.blocked_url?(project.import_url)
+      end
+
+      # We should skip the repository for a GitHub import or GitLab project import,
+      # because these importers fetch the project repositories for us.
+      return if has_importer? && importer_class.try(:imports_repository?)
+
       if unknown_url?
         # In this case, we only want to import issues, not a repository.
         create_repository
@@ -44,12 +50,6 @@ module Projects
     end
 
     def import_repository
-      raise Error, 'Blocked import URL.' if Gitlab::UrlBlocker.blocked_url?(project.import_url)
-
-      # We should return early for a GitHub import because the new GitHub
-      # importer fetch the project repositories for us.
-      return if project.github_import?
-
       begin
         if project.gitea_import?
           fetch_repository
@@ -88,7 +88,7 @@ module Projects
     end
 
     def importer_class
-      Gitlab::ImportSources.importer(project.import_type)
+      @importer_class ||= Gitlab::ImportSources.importer(project.import_type)
     end
 
     def has_importer?
diff --git a/lib/gitlab/github_import/parallel_importer.rb b/lib/gitlab/github_import/parallel_importer.rb
index 81739834b41..6da11e6ef08 100644
--- a/lib/gitlab/github_import/parallel_importer.rb
+++ b/lib/gitlab/github_import/parallel_importer.rb
@@ -11,6 +11,10 @@ module Gitlab
         true
       end
 
+      def self.imports_repository?
+        true
+      end
+
       def initialize(project)
         @project = project
       end
diff --git a/lib/gitlab/import_export/importer.rb b/lib/gitlab/import_export/importer.rb
index fbdd74788bc..c14646b0611 100644
--- a/lib/gitlab/import_export/importer.rb
+++ b/lib/gitlab/import_export/importer.rb
@@ -1,6 +1,10 @@
 module Gitlab
   module ImportExport
     class Importer
+      def self.imports_repository?
+        true
+      end
+
       def initialize(project)
         @archive_file = project.import_source
         @current_user = project.creator