[EE] 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

ee/app/controllers/ee/projects/mirrors_controller.rb

@@ -39,7 +39,7 @@ module EE
           project.update_remote_mirrors
           flash[:notice] = "The remote repository is being updated..."
         else
-          project.force_import_job!
+          project.import_state.force_import_job!
           flash[:notice] = "The repository is being updated..."
         end
 

ee/app/helpers/mirror_helper.rb

 module MirrorHelper
   def render_mirror_failed_message(raw_message:)
-    mirror_last_update_at = @project.mirror_last_update_at
+    mirror_last_update_at = @project.import_state.last_update_at
     message = "The repository failed to update #{time_ago_with_tooltip(mirror_last_update_at)}.".html_safe
 
     return message if raw_message

ee/app/models/ee/project.rb

@@ -14,6 +14,10 @@ module EE
       include EE::DeploymentPlatform
       include EachBatch
 
+      ignore_column :mirror_last_update_at,
+        :mirror_last_successful_update_at,
+        :next_execution_timestamp
+
       before_save :set_override_pull_mirror_available, unless: -> { ::Gitlab::CurrentSettings.mirror_available }
       before_save :set_next_execution_timestamp_to_now, if: ->(project) { project.mirror? && project.mirror_changed? && project.import_state }
 
@@ -59,11 +63,9 @@ module EE
 
       scope :mirror, -> { where(mirror: true) }
 
-      scope :inner_joins_import_state, -> { joins("INNER JOIN project_mirror_data import_state ON import_state.project_id = projects.id") }
-
       scope :mirrors_to_sync, ->(freeze_at) do
         mirror
-          .inner_joins_import_state
+          .joins_import_state
           .where.not(import_state: { status: [:scheduled, :started] })
           .where("import_state.next_execution_timestamp <= ?", freeze_at)
           .where("import_state.retry_count <= ?", ::Gitlab::Mirror::MAX_RETRY)
@@ -83,6 +85,10 @@ module EE
       delegate :actual_shared_runners_minutes_limit,
         :shared_runners_minutes_used?, to: :shared_runners_limit_namespace
 
+      delegate :last_update_succeeded?, :last_update_failed?,
+        :ever_updated_successfully?, :hard_failed?,
+        to: :import_state, prefix: :mirror, allow_nil: true
+
       validates :repository_size_limit,
         numericality: { only_integer: true, greater_than_or_equal_to: 0, allow_nil: true }
 
@@ -144,101 +150,10 @@ module EE
     end
     alias_method :mirror?, :mirror
 
-    def mirror_updated?
-      mirror? && self.mirror_last_update_at
-    end
-
-    def mirror_waiting_duration
-      return unless mirror?
-
-      (import_state.last_update_started_at.to_i -
-        import_state.last_update_scheduled_at.to_i).seconds
-    end
-
-    def mirror_update_duration
-      return unless mirror?
-
-      (mirror_last_update_at.to_i -
-        import_state.last_update_started_at.to_i).seconds
-    end
-
     def mirror_with_content?
       mirror? && !empty_repo?
     end
 
-    def import_state_args
-      super.merge(last_update_at: self[:mirror_last_update_at],
-                  last_successful_update_at: self[:mirror_last_successful_update_at])
-    end
-
-    def mirror_last_update_at=(new_value)
-      ensure_import_state
-
-      import_state&.last_update_at = new_value
-    end
-
-    def mirror_last_update_at
-      ensure_import_state
-
-      import_state&.last_update_at
-    end
-
-    def mirror_last_successful_update_at=(new_value)
-      ensure_import_state
-
-      import_state&.last_successful_update_at = new_value
-    end
-
-    def mirror_last_successful_update_at
-      ensure_import_state
-
-      import_state&.last_successful_update_at
-    end
-
-    override :import_in_progress?
-    def import_in_progress?
-      # If we're importing while we do have a repository, we're simply updating the mirror.
-      super && !mirror_with_content?
-    end
-
-    def mirror_about_to_update?
-      return false unless mirror_with_content?
-      return false if mirror_hard_failed?
-      return false if updating_mirror?
-
-      self.import_state.next_execution_timestamp <= Time.now
-    end
-
-    def updating_mirror?
-      (import_scheduled? || import_started?) && mirror_with_content?
-    end
-
-    def mirror_last_update_status
-      return unless mirror_updated?
-
-      if self.mirror_last_update_at == self.mirror_last_successful_update_at
-        :success
-      else
-        :failed
-      end
-    end
-
-    def mirror_last_update_succeeded?
-      mirror_last_update_status == :success
-    end
-
-    def mirror_last_update_failed?
-      mirror_last_update_status == :failed
-    end
-
-    def mirror_ever_updated_successfully?
-      mirror_updated? && self.mirror_last_successful_update_at
-    end
-
-    def mirror_hard_failed?
-      self.import_state.retry_limit_exceeded?
-    end
-
     def fetch_mirror
       return unless mirror?
 
@@ -293,19 +208,6 @@ module EE
       config.address&.gsub(wildcard, full_path)
     end
 
-    def force_import_job!
-      return if mirror_about_to_update? || updating_mirror?
-
-      import_state = self.import_state
-
-      import_state.set_next_execution_to_now
-      import_state.reset_retry_count if import_state.retry_limit_exceeded?
-
-      import_state.save!
-
-      UpdateAllMirrorsWorker.perform_async
-    end
-
     override :add_import_job
     def add_import_job
       return if gitlab_custom_project_template_import?

ee/app/models/ee/project_import_state.rb

@@ -72,6 +72,58 @@ module EE
       end
     end
 
+    override :in_progress?
+    def in_progress?
+      # If we're importing while we do have a repository, we're simply updating the mirror.
+      super && !project.mirror_with_content?
+    end
+
+    def mirror_waiting_duration
+      return unless mirror?
+
+      (last_update_started_at.to_i - last_update_scheduled_at.to_i).seconds
+    end
+
+    def mirror_update_duration
+      return unless mirror?
+
+      (last_update_at.to_i - last_update_started_at.to_i).seconds
+    end
+
+    def updating_mirror?
+      (scheduled? || started?) && project.mirror_with_content?
+    end
+
+    def mirror_update_due?
+      return false unless project.mirror_with_content?
+      return false if hard_failed?
+      return false if updating_mirror?
+
+      next_execution_timestamp <= Time.now
+    end
+
+    def last_update_status
+      return unless state_updated?
+
+      if last_update_at == last_successful_update_at
+        :success
+      else
+        :failed
+      end
+    end
+
+    def last_update_succeeded?
+      last_update_status == :success
+    end
+
+    def last_update_failed?
+      last_update_status == :failed
+    end
+
+    def ever_updated_successfully?
+      state_updated? && last_successful_update_at
+    end
+
     def reset_retry_count
       self.retry_count = 0
     end
@@ -91,8 +143,19 @@ module EE
       self.next_execution_timestamp = timestamp + delay
     end
 
+    def force_import_job!
+      return if mirror_update_due? || updating_mirror?
+
+      set_next_execution_to_now
+      reset_retry_count if hard_failed?
+
+      save!
+
+      UpdateAllMirrorsWorker.perform_async
+    end
+
     def set_next_execution_to_now
-      return unless project.mirror?
+      return unless mirror?
 
       self.next_execution_timestamp = Time.now
     end
@@ -100,9 +163,14 @@ module EE
     def retry_limit_exceeded?
       self.retry_count > ::Gitlab::Mirror::MAX_RETRY
     end
+    alias_method :hard_failed?, :retry_limit_exceeded?
 
     private
 
+    def state_updated?
+      mirror? && last_update_at
+    end
+
     def base_delay(timestamp)
       return 0 unless self.last_update_started_at
 

ee/app/services/ee/projects/update_service.rb

@@ -34,7 +34,7 @@ module EE
           log_audit_events
 
           sync_wiki_on_enable if !wiki_was_enabled && project.wiki_enabled?
-          project.force_import_job! if params[:mirror].present? && project.mirror?
+          project.import_state.force_import_job! if params[:mirror].present? && project.mirror?
         end
 
         result

ee/app/views/notify/mirror_was_hard_failed_email.html.haml

@@ -2,7 +2,7 @@
   Repository mirroring on #{@project.full_path} has been paused due to too many failures. The last failure was:
 
 %pre
-  = @project.import_error
+  = @project.import_state.last_error
 
 %p
   To resume mirroring update your #{link_to("repository mirroring settings", project_settings_repository_url(@project))}.

ee/app/views/notify/mirror_was_hard_failed_email.text.erb

 Repository mirroring on <%= @project.full_path %> has been paused due to too many failures. The last failure was:
 
-<%= @project.import_error %>
+<%= @project.import_state.last_error %>
 
 To resume mirroring update your repository settings at <%= project_settings_repository_url(@project) %>.

ee/app/views/projects/mirrors/_table_pull_row.html.haml

-- if @project.mirror
-  %tr
-    %td= @project.username_only_import_url
-    %td= _('Pull')
-    %td= @project.mirror_last_update_at.present? ? time_ago_with_tooltip(@project.mirror_last_update_at) : _('Never')
-    %td
-      - if @project.import_error.present?
-        .badge.mirror-error-badge{ data: { toggle: 'tooltip', html: 'true' }, title: html_escape(@project.import_error.try(:strip)) }= _('Error')
-    %td.mirror-action-buttons
-      .btn-group.mirror-actions-group.pull-right{ role: 'group' }
-        - if @project.mirror?
-          - if @project.mirror_about_to_update? || @project.updating_mirror?
-            %button.btn.disabled{ type: 'button', data: { container: 'body', toggle: 'tooltip' }, title: _('Updating') }= icon("refresh spin")
-          - else
-            = link_to update_now_project_mirror_path(@project), method: :post, class: 'btn js-force-update-mirror', data: { container: 'body', toggle: 'tooltip' }, title: _('Update now') do
-              = icon("refresh")
-        %button.js-delete-mirror.js-delete-pull-mirror.btn.btn-danger{ type: 'button', data: { toggle: 'tooltip', container: 'body' }, title: _('Remove') }= icon('trash-o')
+- return unless @project.mirror
+
+- import_state = @project.import_state
+
+%tr
+  %td= @project.username_only_import_url
+  %td= _('Pull')
+  %td= import_state.last_update_at.present? ? time_ago_with_tooltip(import_state.last_update_at) : _('Never')
+  %td
+    - if import_state&.last_error.present?
+      .badge.mirror-error-badge{ data: { toggle: 'tooltip', html: 'true' }, title: html_escape(import_state.last_error.try(:strip)) }= _('Error')
+  %td.mirror-action-buttons
+    .btn-group.mirror-actions-group.pull-right{ role: 'group' }
+      - if @project.mirror?
+        - if import_state.mirror_update_due? || import_state.updating_mirror?
+          %button.btn.disabled{ type: 'button', data: { container: 'body', toggle: 'tooltip' }, title: _('Updating') }= icon("refresh spin")
+        - else
+          = link_to update_now_project_mirror_path(@project), method: :post, class: 'btn js-force-update-mirror', data: { container: 'body', toggle: 'tooltip' }, title: _('Update now') do
+            = icon("refresh")
+      %button.js-delete-mirror.js-delete-pull-mirror.btn.btn-danger{ type: 'button', data: { toggle: 'tooltip', container: 'body' }, title: _('Remove') }= icon('trash-o')

ee/app/views/projects/mirrors/pull/_mirror_update_fail.html.haml

@@ -5,4 +5,4 @@
     .card-body
       %pre
         :preserve
-          #{h(@project.import_error.try(:strip))}
+          #{h(@project.import_state.last_error.try(:strip))}

ee/app/views/shared/_mirror_status.html.haml

-- last_successful_update_at = @project.mirror_last_successful_update_at
+- last_successful_update_at = @project.import_state.last_successful_update_at
 - raw_message = local_assigns.fetch(:raw_message, false)
 
-- case @project.mirror_last_update_status
+- case @project.import_state.last_update_status
 - when :success
   Updated #{time_ago_with_tooltip(last_successful_update_at)}.
 - when :failed

ee/app/views/shared/_mirror_update_button.html.haml

 - if @project.mirror? && can?(current_user, :push_code, @project)
   .append-bottom-default
-    - if @project.mirror_about_to_update?
+    - if @project.import_state.mirror_update_due?
       %span.btn.disabled
         = icon("refresh spin")
         Update Scheduled…
-    - elsif @project.updating_mirror?
+    - elsif @project.import_state.updating_mirror?
       %span.btn.disabled
         = icon("refresh spin")
         Updating…
@@ -14,4 +14,4 @@
         Update Now
     - if @project.mirror_last_update_succeeded?
       %p.inline.prepend-left-10
-        Successfully updated #{time_ago_with_tooltip(@project.mirror_last_successful_update_at)}.
+        Successfully updated #{time_ago_with_tooltip(@project.import_state.last_successful_update_at)}.

ee/app/workers/ee/repository_import_worker.rb

@@ -8,7 +8,7 @@ module EE
 
       # Explicitly enqueue mirror for update so
       # that upstream remote is created and fetched
-      project.force_import_job! if project.mirror?
+      project.import_state.force_import_job! if project.mirror?
     end
 
     override :template_import?

ee/app/workers/project_import_schedule_worker.rb

 class ProjectImportScheduleWorker
+  ImportStateNotFound = Class.new(StandardError)
+
   include ApplicationWorker
   prepend WaitableWorker
 
+  sidekiq_options retry: false
+
   # rubocop: disable CodeReuse/ActiveRecord
   def perform(project_id)
-    project = Project.find_by(id: project_id)
-    project&.import_schedule
+    import_state = ProjectImportState.find_by(project_id: project_id)
+    raise ImportStateNotFound unless import_state
+
+    import_state.schedule
   end
   # rubocop: enable CodeReuse/ActiveRecord
 end

ee/app/workers/repository_update_mirror_worker.rb

@@ -45,38 +45,31 @@ class RepositoryUpdateMirrorWorker
   end
 
   def start_mirror(project)
-    if start(project)
-      Rails.logger.info("Mirror update for #{project.full_path} started. Waiting duration: #{project.mirror_waiting_duration}")
-      metric_mirror_waiting_duration_seconds.observe({}, project.mirror_waiting_duration)
+    import_state = project.import_state
 
-      Gitlab::Metrics.add_event_with_values(
-        :mirrors_running,
-        { duration: project.mirror_waiting_duration },
-        { path: project.full_path })
+    if start(import_state)
+      Rails.logger.info("Mirror update for #{project.full_path} started. Waiting duration: #{import_state.mirror_waiting_duration}")
+      metric_mirror_waiting_duration_seconds.observe({}, import_state.mirror_waiting_duration)
 
       true
     else
-      Rails.logger.info("Project #{project.full_path} was in inconsistent state: #{project.import_status}")
+      Rails.logger.info("Project #{project.full_path} was in inconsistent state: #{import_state.status}")
       false
     end
   end
 
   def fail_mirror(project, message)
-    project.mark_import_as_failed(message)
+    project.import_state.mark_as_failed(message)
 
     Rails.logger.error("Mirror update for #{project.full_path} failed with the following message: #{message}")
-    Gitlab::Metrics.add_event(:mirrors_failed)
   end
 
   def finish_mirror(project)
-    project.import_finish
+    import_state = project.import_state
+    import_state.finish
 
-    Rails.logger.info("Mirror update for #{project.full_path} successfully finished. Update duration: #{project.mirror_update_duration}}.")
-    Gitlab::Metrics.add_event_with_values(
-      :mirrors_finished,
-      { duration: project.mirror_update_duration })
-
-    metric_mirror_update_duration_seconds.observe({}, project.mirror_update_duration)
+    Rails.logger.info("Mirror update for #{project.full_path} successfully finished. Update duration: #{import_state.mirror_update_duration}}.")
+    metric_mirror_update_duration_seconds.observe({}, import_state.mirror_update_duration)
   end
 
   def metric_mirror_update_duration_seconds

ee/lib/api/project_mirror.rb

@@ -51,7 +51,7 @@ module API
 
         break render_api_error!('The project is not mirrored', 400) unless project.mirror?
 
-        project.force_import_job!
+        project.import_state.force_import_job!
 
         status 200
       end

ee/spec/controllers/projects/mirrors_controller_spec.rb

@@ -120,7 +120,7 @@ describe Projects::MirrorsController do
       project = create(:project, :mirror)
       sign_in(project.owner)
 
-      expect_any_instance_of(EE::Project).to receive(:force_import_job!)
+      expect_any_instance_of(EE::ProjectImportState).to receive(:force_import_job!)
 
       put :update_now, { namespace_id: project.namespace.to_param, project_id: project.to_param }
     end

ee/spec/controllers/projects_controller_spec.rb

@@ -192,7 +192,7 @@ describe ProjectsController do
         end
 
         it 'updates repository mirror attributes' do
-          expect_any_instance_of(EE::Project).to receive(:force_import_job!).once
+          expect_any_instance_of(EE::ProjectImportState).to receive(:force_import_job!).once
 
           put :update,
             namespace_id: project.namespace,

ee/spec/features/projects/mirror_spec.rb

@@ -24,7 +24,7 @@ describe 'Project mirror', :js do
         it 'forces import' do
           import_state.update(last_update_at: timestamp - 8.minutes)
 
-          expect_any_instance_of(EE::Project).to receive(:force_import_job!)
+          expect_any_instance_of(EE::ProjectImportState).to receive(:force_import_job!)
 
           Timecop.freeze(timestamp) do
             visit project_mirror_path(project)
@@ -38,7 +38,7 @@ describe 'Project mirror', :js do
         it 'does not force import' do
           import_state.update(last_update_at: timestamp - 3.minutes)
 
-          expect_any_instance_of(EE::Project).not_to receive(:force_import_job!)
+          expect_any_instance_of(EE::ProjectImportState).not_to receive(:force_import_job!)
 
           Timecop.freeze(timestamp) do
             visit project_mirror_path(project)

ee/spec/requests/api/project_mirror_spec.rb

@@ -163,7 +163,7 @@ describe API::ProjectMirror do
           end
 
           it 'syncs the mirror' do
-            expect(project_mirrored).to receive(:force_import_job!)
+            expect(project_mirrored.import_state).to receive(:force_import_job!)
 
             do_post
           end
@@ -182,7 +182,7 @@ describe API::ProjectMirror do
           end
 
           it "doesn't sync the mirror" do
-            expect(project_mirrored).not_to receive(:force_import_job!)
+            expect(project_mirrored.import_state).not_to receive(:force_import_job!)
 
             post api("/projects/#{project_mirrored.id}/mirror/pull"), {}, { 'X-Hub-Signature' => 'signature' }
           end

ee/spec/requests/api/projects_spec.rb

@@ -177,7 +177,7 @@ describe API::Projects do
           mirror_params[:mirror_user_id] = admin.id
           project.add_maintainer(admin)
 
-          expect_any_instance_of(EE::Project).to receive(:force_import_job!).once
+          expect_any_instance_of(EE::ProjectImportState).to receive(:force_import_job!).once
 
           put(api("/projects/#{project.id}", admin), mirror_params)
 
@@ -194,7 +194,7 @@ describe API::Projects do
       end
 
       it 'updates mirror related attributes' do
-        expect_any_instance_of(EE::Project).to receive(:force_import_job!).once
+        expect_any_instance_of(EE::ProjectImportState).to receive(:force_import_job!).once
 
         put(api("/projects/#{project.id}", user), mirror_params)
 

ee/spec/services/projects/update_service_spec.rb

@@ -22,7 +22,7 @@ describe Projects::UpdateService, '#execute' do
       }
 
       stub_licensed_features(repository_mirrors: true)
-      expect(project).to receive(:force_import_job!).once
+      expect_any_instance_of(EE::ProjectImportState).to receive(:force_import_job!).once
 
       update_project(project, user, opts)
     end

ee/spec/views/shared/_mirror_status.html.haml_spec.rb

@@ -42,7 +42,7 @@ describe 'shared/_mirror_status.html.haml' do
 
     context 'with a previous successful update' do
       it 'renders failure message' do
-        @project.mirror_last_successful_update_at = Time.now - 1.minute
+        @project.import_state.last_successful_update_at = Time.now - 1.minute
 
         render 'shared/mirror_status', raw_message: true
 

ee/spec/workers/project_import_schedule_worker_spec.rb

+require 'spec_helper'
+
+describe ProjectImportScheduleWorker do
+  describe '#perform' do
+    it 'schedules an import for a project' do
+      import_state = create(:import_state)
+
+      allow_any_instance_of(EE::Project).to receive(:add_import_job).and_return(nil)
+
+      expect do
+        subject.perform(import_state.project_id)
+      end.to change { import_state.reload.status }.from("none").to("scheduled")
+    end
+
+    context 'when project is not found' do
+      it 'raises ImportStateNotFound' do
+        expect { subject.perform(-1) }.to raise_error(described_class::ImportStateNotFound)
+      end
+    end
+
+    context 'when project does not have import state' do
+      it 'raises ImportStateNotFound' do
+        project = create(:project)
+
+        expect { subject.perform(project.id) }.to raise_error(described_class::ImportStateNotFound)
+      end
+    end
+  end
+end

ee/spec/workers/repository_import_worker_spec.rb

@@ -7,14 +7,15 @@ describe RepositoryImportWorker do
     stub_licensed_features(custom_project_templates: true)
     error = %q{remote: Not Found fatal: repository 'https://user:pass@test.com/root/repoC.git/' not found }
 
-    project.update(import_jid: '123', import_type: 'gitlab_custom_project_template')
+    project.update(import_type: 'gitlab_custom_project_template')
+    project.import_state.update(jid: '123')
     expect_any_instance_of(Projects::ImportService).to receive(:execute).and_return({ status: :error, message: error })
 
     expect do
       subject.perform(project.id)
     end.to raise_error(RuntimeError, error)
 
-    expect(project.reload.import_error).not_to be_nil
+    expect(project.import_state.reload.last_error).not_to be_nil
   end
 
   context 'when project is a mirror' do
@@ -24,7 +25,7 @@ describe RepositoryImportWorker do
       expect_any_instance_of(Projects::ImportService).to receive(:execute)
         .and_return({ status: :ok })
 
-      expect_any_instance_of(EE::Project).to receive(:force_import_job!)
+      expect_any_instance_of(EE::ProjectImportState).to receive(:force_import_job!)
 
       subject.perform(project.id)
     end