Removes all the irrelevant import related code and columns

Clears the import related columns and code from the Project
model over to the ProjectImportState model

20 files changed, 103 insertions, 205 deletions

diff --git a/app/controllers/projects/imports_controller.rb b/app/controllers/projects/imports_controller.rb
index e55065c5817..641f45b3b56 100644
--- a/app/controllers/projects/imports_controller.rb
+++ b/app/controllers/projects/imports_controller.rb
@@ -13,10 +13,8 @@ class Projects::ImportsController < Projects::ApplicationController
   end
 
   def create
-    @project.import_url = params[:project][:import_url]
-
-    if @project.save
-      @project.reload.import_schedule
+    if @project.update(safe_import_params)
+      @project.import_state.reload.schedule
     end
 
     redirect_to project_import_path(@project)
@@ -24,7 +22,7 @@ class Projects::ImportsController < Projects::ApplicationController
 
   def show
     if @project.import_finished?
-      if continue_params
+      if continue_params && continue_params[:to]
         redirect_to continue_params[:to], notice: continue_params[:notice]
       else
         redirect_to project_path(@project), notice: finished_notice
@@ -67,4 +65,12 @@ class Projects::ImportsController < Projects::ApplicationController
       redirect_to project_path(@project)
     end
   end
+
+  def import_params
+    params.require(:project).permit(:import_url)
+  end
+
+  def safe_import_params
+    import_params
+  end
 end
diff --git a/app/models/project.rb b/app/models/project.rb
index b85ec90f3ca..c305644c900 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -30,6 +30,7 @@ class Project < ActiveRecord::Base
   include FeatureGate
   include OptionallySearch
   include FromUnion
+  include IgnorableColumn
   extend Gitlab::Cache::RequestCache
 
   extend Gitlab::ConfigHelper
@@ -55,6 +56,8 @@ class Project < ActiveRecord::Base
   VALID_MIRROR_PORTS = [22, 80, 443].freeze
   VALID_MIRROR_PROTOCOLS = %w(http https ssh git).freeze
 
+  ignore_column :import_status, :import_jid, :import_error
+
   cache_markdown_field :description, pipeline: :description
 
   delegate :feature_available?, :builds_enabled?, :wiki_enabled?,
@@ -63,6 +66,12 @@ class Project < ActiveRecord::Base
 
   delegate :base_dir, :disk_path, :ensure_storage_path_exists, to: :storage
 
+  delegate :scheduled?, :started?, :in_progress?,
+    :failed?, :finished?,
+    prefix: :import, to: :import_state, allow_nil: true
+
+  delegate :no_import?, to: :import_state, allow_nil: true
+
   default_value_for :archived, false
   default_value_for :visibility_level, gitlab_config_features.visibility_level
   default_value_for :resolve_outdated_diff_discussions, false
@@ -454,8 +463,8 @@ class Project < ActiveRecord::Base
 
   scope :excluding_project, ->(project) { where.not(id: project) }
 
-  scope :joins_import_state, -> { joins("LEFT JOIN project_mirror_data import_state ON import_state.project_id = projects.id") }
-  scope :import_started, -> { joins_import_state.where("import_state.status = 'started' OR projects.import_status = 'started'") }
+  # We require an alias to the project_mirror_data_table in order to use import_state in our queries
+  scope :joins_import_state, -> { joins("INNER JOIN project_mirror_data import_state ON import_state.project_id = projects.id") }
   scope :for_group, -> (group) { where(group: group) }
 
   class << self
@@ -631,6 +640,14 @@ class Project < ActiveRecord::Base
     id && persisted?
   end
 
+  def import_status
+    import_state&.status || 'none'
+  end
+
+  def human_import_status_name
+    import_state&.human_status_name || 'none'
+  end
+
   def add_import_job
     job_id =
       if forked?
@@ -662,7 +679,7 @@ class Project < ActiveRecord::Base
       ProjectCacheWorker.perform_async(self.id)
     end
 
-    update(import_error: nil)
+    import_state.update(last_error: nil)
     remove_import_data
   end
 
@@ -724,130 +741,6 @@ class Project < ActiveRecord::Base
     import_url.present?
   end
 
-  def imported?
-    import_finished?
-  end
-
-  def import_in_progress?
-    import_started? || import_scheduled?
-  end
-
-  def import_state_args
-    {
-      status: self[:import_status],
-      jid: self[:import_jid],
-      last_error: self[:import_error]
-    }
-  end
-
-  def ensure_import_state(force: false)
-    return if !force && (self[:import_status] == 'none' || self[:import_status].nil?)
-    return unless import_state.nil?
-
-    if persisted?
-      create_import_state(import_state_args)
-
-      update_column(:import_status, 'none')
-    else
-      build_import_state(import_state_args)
-
-      self[:import_status] = 'none'
-    end
-  end
-
-  def human_import_status_name
-    ensure_import_state
-
-    import_state.human_status_name
-  end
-
-  def import_schedule
-    ensure_import_state(force: true)
-
-    import_state.schedule
-  end
-
-  def force_import_start
-    ensure_import_state(force: true)
-
-    import_state.force_start
-  end
-
-  def import_start
-    ensure_import_state(force: true)
-
-    import_state.start
-  end
-
-  def import_fail
-    ensure_import_state(force: true)
-
-    import_state.fail_op
-  end
-
-  def import_finish
-    ensure_import_state(force: true)
-
-    import_state.finish
-  end
-
-  def import_jid=(new_jid)
-    ensure_import_state(force: true)
-
-    import_state.jid = new_jid
-  end
-
-  def import_jid
-    ensure_import_state
-
-    import_state&.jid
-  end
-
-  def import_error=(new_error)
-    ensure_import_state(force: true)
-
-    import_state.last_error = new_error
-  end
-
-  def import_error
-    ensure_import_state
-
-    import_state&.last_error
-  end
-
-  def import_status=(new_status)
-    ensure_import_state(force: true)
-
-    import_state.status = new_status
-  end
-
-  def import_status
-    ensure_import_state
-
-    import_state&.status || 'none'
-  end
-
-  def no_import?
-    import_status == 'none'
-  end
-
-  def import_started?
-    # import? does SQL work so only run it if it looks like there's an import running
-    import_status == 'started' && import?
-  end
-
-  def import_scheduled?
-    import_status == 'scheduled'
-  end
-
-  def import_failed?
-    import_status == 'failed'
-  end
-
-  def import_finished?
-    import_status == 'finished'
-  end
-
   def safe_import_url
     Gitlab::UrlSanitizer.new(import_url).masked_url
   end
@@ -1646,8 +1539,8 @@ class Project < ActiveRecord::Base
   def after_import
     repository.after_import
     wiki.repository.after_import
-    import_finish
-    remove_import_jid
+    import_state.finish
+    import_state.remove_jid
     update_project_counter_caches
     after_create_default_branch
     refresh_markdown_cache!
@@ -1687,32 +1580,11 @@ class Project < ActiveRecord::Base
   end
   # rubocop: enable CodeReuse/ServiceClass
 
-  def remove_import_jid
-    return unless import_jid
-
-    Gitlab::SidekiqStatus.unset(import_jid)
-
-    import_state.update_column(:jid, nil)
-  end
-
   # Lazy loading of the `pipeline_status` attribute
   def pipeline_status
     @pipeline_status ||= Gitlab::Cache::Ci::ProjectPipelineStatus.load_for_project(self)
   end
 
-  def mark_import_as_failed(error_message)
-    original_errors = errors.dup
-    sanitized_message = Gitlab::UrlSanitizer.sanitize(error_message)
-
-    import_fail
-
-    import_state.update_column(:last_error, sanitized_message)
-  rescue ActiveRecord::ActiveRecordError => e
-    Rails.logger.error("Error setting import status to failed: #{e.message}. Original error: #{sanitized_message}")
-  ensure
-    @errors = original_errors
-  end
-
   def add_export_job(current_user:, after_export_strategy: nil, params: {})
     job_id = ProjectExportWorker.perform_async(current_user.id, self.id, after_export_strategy, params)
 
@@ -1989,17 +1861,6 @@ class Project < ActiveRecord::Base
     Gitlab::ReferenceCounter.new(gl_repository(is_wiki: wiki))
   end
 
-  # Refreshes the expiration time of the associated import job ID.
-  #
-  # This method can be used by asynchronous importers to refresh the status,
-  # preventing the StuckImportJobsWorker from marking the import as failed.
-  def refresh_import_jid_expiration
-    return unless import_jid
-
-    Gitlab::SidekiqStatus
-      .set(import_jid, StuckImportJobsWorker::IMPORT_JOBS_EXPIRATION)
-  end
-
   def badges
     return project_badges unless group
 
diff --git a/app/models/project_import_state.rb b/app/models/project_import_state.rb
index 7126bb66d80..c80e612053c 100644
--- a/app/models/project_import_state.rb
+++ b/app/models/project_import_state.rb
@@ -69,4 +69,33 @@ class ProjectImportState < ActiveRecord::Base
   ensure
     @errors = original_errors
   end
+
+  alias_method :no_import?, :none?
+
+  def in_progress?
+    started? || scheduled?
+  end
+
+  def started?
+    # import? does SQL work so only run it if it looks like there's an import running
+    status == 'started' && project.import?
+  end
+
+  def remove_jid
+    return unless jid
+
+    Gitlab::SidekiqStatus.unset(jid)
+
+    update_column(:jid, nil)
+  end
+
+  # Refreshes the expiration time of the associated import job ID.
+  #
+  # This method can be used by asynchronous importers to refresh the status,
+  # preventing the StuckImportJobsWorker from marking the import as failed.
+  def refresh_jid_expiration
+    return unless jid
+
+    Gitlab::SidekiqStatus.set(jid, StuckImportJobsWorker::IMPORT_JOBS_EXPIRATION)
+  end
 end
diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb
index 20bfe5af7a1..481de34b977 100644
--- a/app/services/projects/create_service.rb
+++ b/app/services/projects/create_service.rb
@@ -148,7 +148,7 @@ module Projects
       Rails.logger.error(log_message)
 
       if @project
-        @project.mark_import_as_failed(message) if @project.persisted? && @project.import?
+        @project.import_state.mark_as_failed(message) if @project.persisted? && @project.import?
       end
 
       @project
@@ -181,7 +181,7 @@ module Projects
 
     def import_schedule
       if @project.errors.empty?
-        @project.import_schedule if @project.import? && !@project.bare_repository_import?
+        @project.import_state.schedule if @project.import? && !@project.bare_repository_import?
       else
         fail(error: @project.errors.full_messages.join(', '))
       end
diff --git a/app/views/import/bitbucket/status.html.haml b/app/views/import/bitbucket/status.html.haml
index 3b1b5e55302..2336e1e83f9 100644
--- a/app/views/import/bitbucket/status.html.haml
+++ b/app/views/import/bitbucket/status.html.haml
@@ -37,11 +37,12 @@
           %td
             = link_to project.full_path, [project.namespace.becomes(Namespace), project]
           %td.job-status
-            - if project.import_status == 'finished'
+            - case project.import_status
+            - when 'finished'
               %span
                 %i.fa.fa-check
                 = _('done')
-            - elsif project.import_status == 'started'
+            - when 'started'
               %i.fa.fa-spinner.fa-spin
               = _('started')
             - else
diff --git a/app/views/import/bitbucket_server/status.html.haml b/app/views/import/bitbucket_server/status.html.haml
index 56d4f2ba881..ef69197e453 100644
--- a/app/views/import/bitbucket_server/status.html.haml
+++ b/app/views/import/bitbucket_server/status.html.haml
@@ -38,9 +38,10 @@
           %td
             = link_to project.full_path, [project.namespace.becomes(Namespace), project]
           %td.job-status
-            - if project.import_status == 'finished'
+            - case project.import_status
+            - when 'finished'
               = icon('check', text: 'Done')
-            - elsif project.import_status == 'started'
+            - when 'started'
               = icon('spin', text: 'started')
             - else
               = project.human_import_status_name
diff --git a/app/views/import/fogbugz/status.html.haml b/app/views/import/fogbugz/status.html.haml
index 830d141ebea..eca67582d6f 100644
--- a/app/views/import/fogbugz/status.html.haml
+++ b/app/views/import/fogbugz/status.html.haml
@@ -34,11 +34,12 @@
           %td
             = link_to project.full_path, [project.namespace.becomes(Namespace), project]
           %td.job-status
-            - if project.import_status == 'finished'
+            - case project.import_status
+            - when 'finished'
               %span
                 %i.fa.fa-check
                 = _("done")
-            - elsif project.import_status == 'started'
+            - when 'started'
               %i.fa.fa-spinner.fa-spin
               = _("started")
             - else
diff --git a/app/views/import/gitlab/status.html.haml b/app/views/import/gitlab/status.html.haml
index b7bfbae5edf..a5fa12fe7df 100644
--- a/app/views/import/gitlab/status.html.haml
+++ b/app/views/import/gitlab/status.html.haml
@@ -30,11 +30,12 @@
           %td
             = link_to project.full_path, [project.namespace.becomes(Namespace), project]
           %td.job-status
-            - if project.import_status == 'finished'
+            - case project.import_status
+            - when 'finished'
               %span
                 %i.fa.fa-check
                 = _('done')
-            - elsif project.import_status == 'started'
+            - when 'started'
               %i.fa.fa-spinner.fa-spin
               = _('started')
             - else
diff --git a/app/views/import/google_code/status.html.haml b/app/views/import/google_code/status.html.haml
index 347e2820f94..f322b7a956a 100644
--- a/app/views/import/google_code/status.html.haml
+++ b/app/views/import/google_code/status.html.haml
@@ -39,11 +39,12 @@
           %td
             = link_to project.full_path, [project.namespace.becomes(Namespace), project]
           %td.job-status
-            - if project.import_status == 'finished'
+            - case project.import_status
+            - when 'finished'
               %span
                 %i.fa.fa-check
                 = _("done")
-            - elsif project.import_status == 'started'
+            - when 'started'
               %i.fa.fa-spinner.fa-spin
               = _("started")
             - else
diff --git a/app/views/projects/imports/new.html.haml b/app/views/projects/imports/new.html.haml
index 1c50cfbde85..bd0ab2c19f2 100644
--- a/app/views/projects/imports/new.html.haml
+++ b/app/views/projects/imports/new.html.haml
@@ -10,7 +10,7 @@
     .card-body
       %pre
         :preserve
-          #{h(@project.import_error)}
+          #{h(@project.import_state.last_error)}
 
 = form_for @project, url: project_import_path(@project), method: :post do |f|
   = render "shared/import_form", f: f
diff --git a/app/workers/concerns/gitlab/github_import/stage_methods.rb b/app/workers/concerns/gitlab/github_import/stage_methods.rb
index 59e6bc2c97d..e2dee315cde 100644
--- a/app/workers/concerns/gitlab/github_import/stage_methods.rb
+++ b/app/workers/concerns/gitlab/github_import/stage_methods.rb
@@ -24,7 +24,7 @@ module Gitlab
       def find_project(id)
         # If the project has been marked as failed we want to bail out
         # automatically.
-        Project.import_started.find_by(id: id)
+        Project.joins_import_state.where(import_state: { status: :started }).find_by(id: id)
       end
       # rubocop: enable CodeReuse/ActiveRecord
     end
diff --git a/app/workers/concerns/project_import_options.rb b/app/workers/concerns/project_import_options.rb
index 22bdf441d6b..2baf768bfd1 100644
--- a/app/workers/concerns/project_import_options.rb
+++ b/app/workers/concerns/project_import_options.rb
@@ -18,7 +18,7 @@ module ProjectImportOptions
                  "import"
                end
 
-      project.mark_import_as_failed("Every #{action} attempt has failed: #{job['error_message']}. Please try again.")
+      project.import_state.mark_as_failed(_("Every %{action} attempt has failed: %{job_error_message}. Please try again.") % { action: action, job_error_message: job['error_message'] })
       Sidekiq.logger.warn "Failed #{job['class']} with #{job['args']}: #{job['error_message']}"
     end
   end
diff --git a/app/workers/concerns/project_start_import.rb b/app/workers/concerns/project_start_import.rb
index 46a133db2a1..4462bc51a24 100644
--- a/app/workers/concerns/project_start_import.rb
+++ b/app/workers/concerns/project_start_import.rb
@@ -2,11 +2,11 @@
 
 # Used in EE by mirroring
 module ProjectStartImport
-  def start(project)
-    if project.import_started? && project.import_jid == self.jid
+  def start(import_state)
+    if import_state.started? && import_state.jid == self.jid
       return true
     end
 
-    project.import_start
+    import_state.start
   end
 end
diff --git a/app/workers/gitlab/github_import/advance_stage_worker.rb b/app/workers/gitlab/github_import/advance_stage_worker.rb
index 2b49860025a..0b3437a8a33 100644
--- a/app/workers/gitlab/github_import/advance_stage_worker.rb
+++ b/app/workers/gitlab/github_import/advance_stage_worker.rb
@@ -31,7 +31,7 @@ module Gitlab
       # next_stage - The name of the next stage to start when all jobs have been
       #              completed.
       def perform(project_id, waiters, next_stage)
-        return unless (project = find_project(project_id))
+        return unless import_state = find_import_state(project_id)
 
         new_waiters = wait_for_jobs(waiters)
 
@@ -41,7 +41,7 @@ module Gitlab
           # the pressure on Redis. We _only_ do this once all jobs are done so
           # we don't get stuck forever if one or more jobs failed to notify the
           # JobWaiter.
-          project.refresh_import_jid_expiration
+          import_state.refresh_jid_expiration
 
           STAGES.fetch(next_stage.to_sym).perform_async(project_id)
         else
@@ -64,11 +64,8 @@ module Gitlab
       end
 
       # rubocop: disable CodeReuse/ActiveRecord
-      def find_project(id)
-        # TODO: Only select the JID
-        # This is due to the fact that the JID could be present in either the project record or
-        # its associated import_state record
-        Project.import_started.find_by(id: id)
+      def find_import_state(project_id)
+        ProjectImportState.select(:jid).with_status(:started).find_by(project_id: project_id)
       end
       # rubocop: enable CodeReuse/ActiveRecord
     end
diff --git a/app/workers/gitlab/github_import/refresh_import_jid_worker.rb b/app/workers/gitlab/github_import/refresh_import_jid_worker.rb
index 65473026b4c..76723e4a61f 100644
--- a/app/workers/gitlab/github_import/refresh_import_jid_worker.rb
+++ b/app/workers/gitlab/github_import/refresh_import_jid_worker.rb
@@ -16,12 +16,13 @@ module Gitlab
       # project_id - The ID of the project that is being imported.
       # check_job_id - The ID of the job for which to check the status.
       def perform(project_id, check_job_id)
-        return unless (project = find_project(project_id))
+        import_state = find_import_state(project_id)
+        return unless import_state
 
         if SidekiqStatus.running?(check_job_id)
           # As long as the repository is being cloned we want to keep refreshing
           # the import JID status.
-          project.refresh_import_jid_expiration
+          import_state.refresh_jid_expiration
           self.class.perform_in_the_future(project_id, check_job_id)
         end
 
@@ -31,11 +32,10 @@ module Gitlab
       end
 
       # rubocop: disable CodeReuse/ActiveRecord
-      def find_project(id)
-        # TODO: Only select the JID
-        # This is due to the fact that the JID could be present in either the project record or
-        # its associated import_state record
-        Project.import_started.find_by(id: id)
+      def find_import_state(project_id)
+        ProjectImportState.select(:jid)
+          .with_status(:started)
+          .find_by(project_id: project_id)
       end
       # rubocop: enable CodeReuse/ActiveRecord
     end
diff --git a/app/workers/gitlab/github_import/stage/import_base_data_worker.rb b/app/workers/gitlab/github_import/stage/import_base_data_worker.rb
index 5726fbb573d..ccfed2ae187 100644
--- a/app/workers/gitlab/github_import/stage/import_base_data_worker.rb
+++ b/app/workers/gitlab/github_import/stage/import_base_data_worker.rb
@@ -23,7 +23,7 @@ module Gitlab
             klass.new(project, client).execute
           end
 
-          project.refresh_import_jid_expiration
+          project.import_state.refresh_jid_expiration
 
           ImportPullRequestsWorker.perform_async(project.id)
         end
diff --git a/app/workers/gitlab/github_import/stage/import_pull_requests_worker.rb b/app/workers/gitlab/github_import/stage/import_pull_requests_worker.rb
index 1c5a7139802..37a7a7f4ba0 100644
--- a/app/workers/gitlab/github_import/stage/import_pull_requests_worker.rb
+++ b/app/workers/gitlab/github_import/stage/import_pull_requests_worker.rb
@@ -15,7 +15,7 @@ module Gitlab
             .new(project, client)
             .execute
 
-          project.refresh_import_jid_expiration
+          project.import_state.refresh_jid_expiration
 
           AdvanceStageWorker.perform_async(
             project.id,
diff --git a/app/workers/repository_fork_worker.rb b/app/workers/repository_fork_worker.rb
index 68ec66e8499..7eae07d3f6b 100644
--- a/app/workers/repository_fork_worker.rb
+++ b/app/workers/repository_fork_worker.rb
@@ -12,7 +12,7 @@ class RepositoryForkWorker
 
     source_project = target_project.forked_from_project
     unless source_project
-      return target_project.mark_import_as_failed('Source project cannot be found.')
+      return target_project.import_state.mark_as_failed(_('Source project cannot be found.'))
     end
 
     fork_repository(target_project, source_project.repository_storage, source_project.disk_path)
@@ -33,7 +33,7 @@ class RepositoryForkWorker
   end
 
   def start_fork(project)
-    return true if start(project)
+    return true if start(project.import_state)
 
     Rails.logger.info("Project #{project.full_path} was in inconsistent state (#{project.import_status}) while forking.")
     false
diff --git a/app/workers/repository_import_worker.rb b/app/workers/repository_import_worker.rb
index 82189a3c9f5..59691f48a39 100644
--- a/app/workers/repository_import_worker.rb
+++ b/app/workers/repository_import_worker.rb
@@ -34,14 +34,14 @@ class RepositoryImportWorker
   attr_reader :project
 
   def start_import
-    return true if start(project)
+    return true if start(project.import_state)
 
     Rails.logger.info("Project #{project.full_path} was in inconsistent state (#{project.import_status}) while importing.")
     false
   end
 
   def fail_import(message)
-    project.mark_import_as_failed(message)
+    project.import_state.mark_as_failed(message)
   end
 
   def template_import?
diff --git a/app/workers/stuck_import_jobs_worker.rb b/app/workers/stuck_import_jobs_worker.rb
index 667a4121131..c8a186ba4ce 100644
--- a/app/workers/stuck_import_jobs_worker.rb
+++ b/app/workers/stuck_import_jobs_worker.rb
@@ -63,6 +63,6 @@ class StuckImportJobsWorker
   # rubocop: enable CodeReuse/ActiveRecord
 
   def error_message
-    "Import timed out. Import took longer than #{IMPORT_JOBS_EXPIRATION} seconds"
+    _("Import timed out. Import took longer than %{import_jobs_expiration} seconds") % { import_jobs_expiration: IMPORT_JOBS_EXPIRATION }
   end
 end