Commit e7cfbaf9 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'ee-50341-cleanup-useless-project-import-attributes' into 'master'

Removes all the irrelevant import related code and columns

See merge request gitlab-org/gitlab-ee!8370
parents 26593ec1 541570bc
...@@ -15,7 +15,7 @@ class Projects::ImportsController < Projects::ApplicationController ...@@ -15,7 +15,7 @@ class Projects::ImportsController < Projects::ApplicationController
def create def create
if @project.update(safe_import_params) if @project.update(safe_import_params)
@project.reload.import_schedule @project.import_state.reload.schedule
end end
redirect_to project_import_path(@project) redirect_to project_import_path(@project)
...@@ -23,7 +23,7 @@ class Projects::ImportsController < Projects::ApplicationController ...@@ -23,7 +23,7 @@ class Projects::ImportsController < Projects::ApplicationController
def show def show
if @project.import_finished? if @project.import_finished?
if continue_params if continue_params&.key?(:to)
redirect_to continue_params[:to], notice: continue_params[:notice] redirect_to continue_params[:to], notice: continue_params[:notice]
else else
redirect_to project_path(@project), notice: finished_notice redirect_to project_path(@project), notice: finished_notice
......
...@@ -30,6 +30,7 @@ class Project < ActiveRecord::Base ...@@ -30,6 +30,7 @@ class Project < ActiveRecord::Base
include FeatureGate include FeatureGate
include OptionallySearch include OptionallySearch
include FromUnion include FromUnion
include IgnorableColumn
extend Gitlab::Cache::RequestCache extend Gitlab::Cache::RequestCache
# EE specific modules # EE specific modules
...@@ -56,6 +57,8 @@ class Project < ActiveRecord::Base ...@@ -56,6 +57,8 @@ class Project < ActiveRecord::Base
VALID_MIRROR_PORTS = [22, 80, 443].freeze VALID_MIRROR_PORTS = [22, 80, 443].freeze
VALID_MIRROR_PROTOCOLS = %w(http https ssh git).freeze VALID_MIRROR_PROTOCOLS = %w(http https ssh git).freeze
ignore_column :import_status, :import_jid, :import_error
cache_markdown_field :description, pipeline: :description cache_markdown_field :description, pipeline: :description
delegate :feature_available?, :builds_enabled?, :wiki_enabled?, delegate :feature_available?, :builds_enabled?, :wiki_enabled?,
...@@ -64,6 +67,12 @@ class Project < ActiveRecord::Base ...@@ -64,6 +67,12 @@ class Project < ActiveRecord::Base
delegate :base_dir, :disk_path, :ensure_storage_path_exists, to: :storage 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 :archived, false
default_value_for :visibility_level, gitlab_config_features.visibility_level default_value_for :visibility_level, gitlab_config_features.visibility_level
default_value_for :resolve_outdated_diff_discussions, false default_value_for :resolve_outdated_diff_discussions, false
...@@ -452,8 +461,8 @@ class Project < ActiveRecord::Base ...@@ -452,8 +461,8 @@ class Project < ActiveRecord::Base
scope :excluding_project, ->(project) { where.not(id: project) } 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") } # We require an alias to the project_mirror_data_table in order to use import_state in our queries
scope :import_started, -> { joins_import_state.where("import_state.status = 'started' OR projects.import_status = 'started'") } 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) } scope :for_group, -> (group) { where(group: group) }
class << self class << self
...@@ -629,6 +638,14 @@ class Project < ActiveRecord::Base ...@@ -629,6 +638,14 @@ class Project < ActiveRecord::Base
id && persisted? id && persisted?
end 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 def add_import_job
job_id = job_id =
if forked? if forked?
...@@ -660,7 +677,7 @@ class Project < ActiveRecord::Base ...@@ -660,7 +677,7 @@ class Project < ActiveRecord::Base
ProjectCacheWorker.perform_async(self.id) ProjectCacheWorker.perform_async(self.id)
end end
update(import_error: nil) import_state.update(last_error: nil)
remove_import_data remove_import_data
end end
...@@ -722,130 +739,6 @@ class Project < ActiveRecord::Base ...@@ -722,130 +739,6 @@ class Project < ActiveRecord::Base
import_url.present? import_url.present?
end 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 def safe_import_url
Gitlab::UrlSanitizer.new(import_url).masked_url Gitlab::UrlSanitizer.new(import_url).masked_url
end end
...@@ -1644,8 +1537,8 @@ class Project < ActiveRecord::Base ...@@ -1644,8 +1537,8 @@ class Project < ActiveRecord::Base
def after_import def after_import
repository.after_import repository.after_import
wiki.repository.after_import wiki.repository.after_import
import_finish import_state.finish
remove_import_jid import_state.remove_jid
update_project_counter_caches update_project_counter_caches
after_create_default_branch after_create_default_branch
refresh_markdown_cache! refresh_markdown_cache!
...@@ -1685,32 +1578,11 @@ class Project < ActiveRecord::Base ...@@ -1685,32 +1578,11 @@ class Project < ActiveRecord::Base
end end
# rubocop: enable CodeReuse/ServiceClass # 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 # Lazy loading of the `pipeline_status` attribute
def pipeline_status def pipeline_status
@pipeline_status ||= Gitlab::Cache::Ci::ProjectPipelineStatus.load_for_project(self) @pipeline_status ||= Gitlab::Cache::Ci::ProjectPipelineStatus.load_for_project(self)
end 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: {}) 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) job_id = ProjectExportWorker.perform_async(current_user.id, self.id, after_export_strategy, params)
...@@ -1987,17 +1859,6 @@ class Project < ActiveRecord::Base ...@@ -1987,17 +1859,6 @@ class Project < ActiveRecord::Base
Gitlab::ReferenceCounter.new(gl_repository(is_wiki: wiki)) Gitlab::ReferenceCounter.new(gl_repository(is_wiki: wiki))
end 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 def badges
return project_badges unless group return project_badges unless group
......
...@@ -69,6 +69,35 @@ class ProjectImportState < ActiveRecord::Base ...@@ -69,6 +69,35 @@ class ProjectImportState < ActiveRecord::Base
ensure ensure
@errors = original_errors @errors = original_errors
end end
alias_method :no_import?, :none?
def in_progress?
scheduled? || started?
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 end
ProjectImportState.prepend(EE::ProjectImportState) ProjectImportState.prepend(EE::ProjectImportState)
...@@ -150,7 +150,7 @@ module Projects ...@@ -150,7 +150,7 @@ module Projects
Rails.logger.error(log_message) Rails.logger.error(log_message)
if @project 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 end
@project @project
...@@ -183,7 +183,7 @@ module Projects ...@@ -183,7 +183,7 @@ module Projects
def import_schedule def import_schedule
if @project.errors.empty? 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 else
fail(error: @project.errors.full_messages.join(', ')) fail(error: @project.errors.full_messages.join(', '))
end end
......
...@@ -37,11 +37,12 @@ ...@@ -37,11 +37,12 @@
%td %td
= link_to project.full_path, [project.namespace.becomes(Namespace), project] = link_to project.full_path, [project.namespace.becomes(Namespace), project]
%td.job-status %td.job-status
- if project.import_status == 'finished' - case project.import_status
- when 'finished'
%span %span
%i.fa.fa-check %i.fa.fa-check
= _('done') = _('done')
- elsif project.import_status == 'started' - when 'started'
%i.fa.fa-spinner.fa-spin %i.fa.fa-spinner.fa-spin
= _('started') = _('started')
- else - else
......
...@@ -38,9 +38,10 @@ ...@@ -38,9 +38,10 @@
%td %td
= link_to project.full_path, [project.namespace.becomes(Namespace), project] = link_to project.full_path, [project.namespace.becomes(Namespace), project]
%td.job-status %td.job-status
- if project.import_status == 'finished' - case project.import_status
- when 'finished'
= icon('check', text: 'Done') = icon('check', text: 'Done')
- elsif project.import_status == 'started' - when 'started'
= icon('spin', text: 'started') = icon('spin', text: 'started')
- else - else
= project.human_import_status_name = project.human_import_status_name
......
...@@ -34,11 +34,12 @@ ...@@ -34,11 +34,12 @@
%td %td
= link_to project.full_path, [project.namespace.becomes(Namespace), project] = link_to project.full_path, [project.namespace.becomes(Namespace), project]
%td.job-status %td.job-status
- if project.import_status == 'finished' - case project.import_status
- when 'finished'
%span %span
%i.fa.fa-check %i.fa.fa-check
= _("done") = _("done")
- elsif project.import_status == 'started' - when 'started'
%i.fa.fa-spinner.fa-spin %i.fa.fa-spinner.fa-spin
= _("started") = _("started")
- else - else
......
...@@ -30,11 +30,12 @@ ...@@ -30,11 +30,12 @@
%td %td
= link_to project.full_path, [project.namespace.becomes(Namespace), project] = link_to project.full_path, [project.namespace.becomes(Namespace), project]
%td.job-status %td.job-status
- if project.import_status == 'finished' - case project.import_status
- when 'finished'
%span %span
%i.fa.fa-check %i.fa.fa-check
= _('done') = _('done')
- elsif project.import_status == 'started' - when 'started'
%i.fa.fa-spinner.fa-spin %i.fa.fa-spinner.fa-spin
= _('started') = _('started')
- else - else
......
...@@ -39,11 +39,12 @@ ...@@ -39,11 +39,12 @@
%td %td
= link_to project.full_path, [project.namespace.becomes(Namespace), project] = link_to project.full_path, [project.namespace.becomes(Namespace), project]
%td.job-status %td.job-status
- if project.import_status == 'finished' - case project.import_status
- when 'finished'
%span %span
%i.fa.fa-check %i.fa.fa-check
= _("done") = _("done")
- elsif project.import_status == 'started' - when 'started'
%i.fa.fa-spinner.fa-spin %i.fa.fa-spinner.fa-spin
= _("started") = _("started")
- else - else
......
...@@ -10,7 +10,7 @@ ...@@ -10,7 +10,7 @@
.card-body .card-body
%pre %pre
:preserve :preserve
#{h(@project.import_error)} #{h(@project.import_state.last_error)}
= form_for @project, url: project_import_path(@project), method: :post do |f| = form_for @project, url: project_import_path(@project), method: :post do |f|
= render "shared/import_form", f: f = render "shared/import_form", f: f
......
...@@ -24,7 +24,7 @@ module Gitlab ...@@ -24,7 +24,7 @@ module Gitlab
def find_project(id) def find_project(id)
# If the project has been marked as failed we want to bail out # If the project has been marked as failed we want to bail out
# automatically. # automatically.
Project.import_started.find_by(id: id) Project.joins_import_state.where(import_state: { status: :started }).find_by(id: id)
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
end end
......
...@@ -18,7 +18,7 @@ module ProjectImportOptions ...@@ -18,7 +18,7 @@ module ProjectImportOptions
"import" "import"
end 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']}" Sidekiq.logger.warn "Failed #{job['class']} with #{job['args']}: #{job['error_message']}"
end end
end end
......
...@@ -2,11 +2,11 @@ ...@@ -2,11 +2,11 @@
# Used in EE by mirroring # Used in EE by mirroring
module ProjectStartImport module ProjectStartImport
def start(project) def start(import_state)
if project.import_started? && project.import_jid == self.jid if import_state.started? && import_state.jid == self.jid
return true return true
end end
project.import_start import_state.start
end end
end end
...@@ -31,7 +31,7 @@ module Gitlab ...@@ -31,7 +31,7 @@ module Gitlab
# next_stage - The name of the next stage to start when all jobs have been # next_stage - The name of the next stage to start when all jobs have been
# completed. # completed.
def perform(project_id, waiters, next_stage) 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) new_waiters = wait_for_jobs(waiters)
...@@ -41,7 +41,7 @@ module Gitlab ...@@ -41,7 +41,7 @@ module Gitlab
# the pressure on Redis. We _only_ do this once all jobs are done so # 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 # we don't get stuck forever if one or more jobs failed to notify the
# JobWaiter. # JobWaiter.
project.refresh_import_jid_expiration import_state.refresh_jid_expiration
STAGES.fetch(next_stage.to_sym).perform_async(project_id) STAGES.fetch(next_stage.to_sym).perform_async(project_id)
else else
...@@ -64,11 +64,8 @@ module Gitlab ...@@ -64,11 +64,8 @@ module Gitlab
end end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def find_project(id) def find_import_state(project_id)
# TODO: Only select the JID ProjectImportState.select(:jid).with_status(:started).find_by(project_id: project_id)
# 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)
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
end end
......
...@@ -16,12 +16,13 @@ module Gitlab ...@@ -16,12 +16,13 @@ module Gitlab
# project_id - The ID of the project that is being imported. # 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. # check_job_id - The ID of the job for which to check the status.
def perform(project_id, check_job_id) 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) if SidekiqStatus.running?(check_job_id)
# As long as the repository is being cloned we want to keep refreshing # As long as the repository is being cloned we want to keep refreshing
# the import JID status. # 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) self.class.perform_in_the_future(project_id, check_job_id)
end end
...@@ -31,11 +32,10 @@ module Gitlab ...@@ -31,11 +32,10 @@ module Gitlab
end end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def find_project(id) def find_import_state(project_id)
# TODO: Only select the JID ProjectImportState.select(:jid)
# This is due to the fact that the JID could be present in either the project record or .with_status(:started)
# its associated import_state record .find_by(project_id: project_id)
Project.import_started.find_by(id: id)
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
end end
......
...@@ -23,7 +23,7 @@ module Gitlab ...@@ -23,7 +23,7 @@ module Gitlab
klass.new(project, client).execute klass.new(project, client).execute
end end
project.refresh_import_jid_expiration project.import_state.refresh_jid_expiration
ImportPullRequestsWorker.perform_async(project.id) ImportPullRequestsWorker.perform_async(project.id)
end end
......
...@@ -15,7 +15,7 @@ module Gitlab ...@@ -15,7 +15,7 @@ module Gitlab
.new(project, client) .new(project, client)
.execute .execute
project.refresh_import_jid_expiration project.import_state.refresh_jid_expiration
AdvanceStageWorker.perform_async( AdvanceStageWorker.perform_async(
project.id, project.id,
......
...@@ -12,7 +12,7 @@ class RepositoryForkWorker ...@@ -12,7 +12,7 @@ class RepositoryForkWorker
source_project = target_project.forked_from_project source_project = target_project.forked_from_project
unless source_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 end
fork_repository(target_project, source_project.repository_storage, source_project.disk_path) fork_repository(target_project, source_project.repository_storage, source_project.disk_path)
...@@ -33,7 +33,7 @@ class RepositoryForkWorker ...@@ -33,7 +33,7 @@ class RepositoryForkWorker
end end
def start_fork(project) 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.") Rails.logger.info("Project #{project.full_path} was in inconsistent state (#{project.import_status}) while forking.")
false false
......
...@@ -33,14 +33,14 @@ class RepositoryImportWorker ...@@ -33,14 +33,14 @@ class RepositoryImportWorker
attr_reader :project attr_reader :project
def start_import 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.") Rails.logger.info("Project #{project.full_path} was in inconsistent state (#{project.import_status}) while importing.")
false false
end end
def fail_import(message) def fail_import(message)
project.mark_import_as_failed(message) project.import_state.mark_as_failed(message)
end end
def template_import? def template_import?
......
...@@ -70,7 +70,7 @@ class StuckImportJobsWorker ...@@ -70,7 +70,7 @@ class StuckImportJobsWorker
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
def error_message 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
def stuck_import_jobs_worker_runs_counter def stuck_import_jobs_worker_runs_counter
......
---
title: Removes all the irrelevant code and columns that were migrated from the Project
table over to the ProjectImportState table
merge_request: 21497
author:
type: performance
...@@ -605,10 +605,7 @@ tables: ...@@ -605,10 +605,7 @@ tables:
- merge_requests_ff_only_enabled - merge_requests_ff_only_enabled
- issues_template - issues_template
- mirror - mirror
- mirror_last_update_at
- mirror_last_successful_update_at
- mirror_user_id - mirror_user_id
- import_error
- ci_id - ci_id
- shared_runners_enabled - shared_runners_enabled
- build_coverage_regex - build_coverage_regex
...@@ -633,7 +630,6 @@ tables: ...@@ -633,7 +630,6 @@ tables:
- printing_merge_request_link_enabled - printing_merge_request_link_enabled
- auto_cancel_pending_pipelines - auto_cancel_pending_pipelines
- service_desk_enabled - service_desk_enabled
- import_jid
- delete_error - delete_error
- last_repository_updated_at - last_repository_updated_at
- disable_overriding_approvers_per_merge_request - disable_overriding_approvers_per_merge_request
...@@ -664,9 +660,7 @@ tables: ...@@ -664,9 +660,7 @@ tables:
- reset_approvals_on_push - reset_approvals_on_push
- issues_template - issues_template
- mirror - mirror
- mirror_last_successful_update_at
- mirror_user_id - mirror_user_id
- import_error
- ci_id - ci_id
- shared_runners_enabled - shared_runners_enabled
- build_coverage_regex - build_coverage_regex
...@@ -683,7 +677,6 @@ tables: ...@@ -683,7 +677,6 @@ tables:
- only_allow_merge_if_all_discussions_are_resolved - only_allow_merge_if_all_discussions_are_resolved
- repository_size_limit - repository_size_limit
- auto_cancel_pending_pipelines - auto_cancel_pending_pipelines
- import_jid
- delete_error - delete_error
- last_repository_updated_at - last_repository_updated_at
- disable_overriding_approvers_per_merge_request - disable_overriding_approvers_per_merge_request
...@@ -693,6 +686,16 @@ tables: ...@@ -693,6 +686,16 @@ tables:
- pull_mirror_available_overridden - pull_mirror_available_overridden
- mirror_overwrites_diverged_branches - mirror_overwrites_diverged_branches
- external_authorization_classification_label - external_authorization_classification_label
project_mirror_data:
whitelist:
- last_update_at
- last_successful_update_at
- last_error
- jid
pseudo:
- last_successful_update_at
- last_error
- jid
subscriptions: subscriptions:
whitelist: whitelist:
- id - id
......
...@@ -131,7 +131,7 @@ our import as failed because of this. ...@@ -131,7 +131,7 @@ our import as failed because of this.
To prevent this from happening we periodically refresh the expiration time of To prevent this from happening we periodically refresh the expiration time of
the import process. This works by storing the JID of the import job in the the import process. This works by storing the JID of the import job in the
database, then refreshing this JID's TTL at various stages throughout the import database, then refreshing this JID's TTL at various stages throughout the import
process. This is done by calling `Project#refresh_import_jid_expiration`. By process. This is done by calling `ProjectImportState#refresh_jid_expiration`. By
refreshing this TTL we can ensure our import does not get marked as failed so refreshing this TTL we can ensure our import does not get marked as failed so
long we're still performing work. long we're still performing work.
......
...@@ -41,7 +41,7 @@ module EE ...@@ -41,7 +41,7 @@ module EE
project.update_remote_mirrors project.update_remote_mirrors
flash[:notice] = "The remote repository is being updated..." flash[:notice] = "The remote repository is being updated..."
else else
project.force_import_job! project.import_state.force_import_job!
flash[:notice] = "The repository is being updated..." flash[:notice] = "The repository is being updated..."
end end
......
module MirrorHelper module MirrorHelper
def render_mirror_failed_message(raw_message:) 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 message = "The repository failed to update #{time_ago_with_tooltip(mirror_last_update_at)}.".html_safe
return message if raw_message return message if raw_message
......
...@@ -14,6 +14,10 @@ module EE ...@@ -14,6 +14,10 @@ module EE
include EE::DeploymentPlatform include EE::DeploymentPlatform
include EachBatch 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_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 } before_save :set_next_execution_timestamp_to_now, if: ->(project) { project.mirror? && project.mirror_changed? && project.import_state }
...@@ -59,11 +63,9 @@ module EE ...@@ -59,11 +63,9 @@ module EE
scope :mirror, -> { where(mirror: true) } 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 scope :mirrors_to_sync, ->(freeze_at) do
mirror mirror
.inner_joins_import_state .joins_import_state
.where.not(import_state: { status: [:scheduled, :started] }) .where.not(import_state: { status: [:scheduled, :started] })
.where("import_state.next_execution_timestamp <= ?", freeze_at) .where("import_state.next_execution_timestamp <= ?", freeze_at)
.where("import_state.retry_count <= ?", ::Gitlab::Mirror::MAX_RETRY) .where("import_state.retry_count <= ?", ::Gitlab::Mirror::MAX_RETRY)
...@@ -83,6 +85,10 @@ module EE ...@@ -83,6 +85,10 @@ module EE
delegate :actual_shared_runners_minutes_limit, delegate :actual_shared_runners_minutes_limit,
:shared_runners_minutes_used?, to: :shared_runners_limit_namespace :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, validates :repository_size_limit,
numericality: { only_integer: true, greater_than_or_equal_to: 0, allow_nil: true } numericality: { only_integer: true, greater_than_or_equal_to: 0, allow_nil: true }
...@@ -144,101 +150,10 @@ module EE ...@@ -144,101 +150,10 @@ module EE
end end
alias_method :mirror?, :mirror 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? def mirror_with_content?
mirror? && !empty_repo? mirror? && !empty_repo?
end 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 def fetch_mirror
return unless mirror? return unless mirror?
...@@ -293,19 +208,6 @@ module EE ...@@ -293,19 +208,6 @@ module EE
config.address&.gsub(wildcard, full_path) config.address&.gsub(wildcard, full_path)
end 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 override :add_import_job
def add_import_job def add_import_job
return if gitlab_custom_project_template_import? return if gitlab_custom_project_template_import?
......
...@@ -72,6 +72,58 @@ module EE ...@@ -72,6 +72,58 @@ module EE
end end
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 def reset_retry_count
self.retry_count = 0 self.retry_count = 0
end end
...@@ -91,8 +143,19 @@ module EE ...@@ -91,8 +143,19 @@ module EE
self.next_execution_timestamp = timestamp + delay self.next_execution_timestamp = timestamp + delay
end 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 def set_next_execution_to_now
return unless project.mirror? return unless mirror?
self.next_execution_timestamp = Time.now self.next_execution_timestamp = Time.now
end end
...@@ -100,9 +163,14 @@ module EE ...@@ -100,9 +163,14 @@ module EE
def retry_limit_exceeded? def retry_limit_exceeded?
self.retry_count > ::Gitlab::Mirror::MAX_RETRY self.retry_count > ::Gitlab::Mirror::MAX_RETRY
end end
alias_method :hard_failed?, :retry_limit_exceeded?
private private
def state_updated?
mirror? && last_update_at
end
def base_delay(timestamp) def base_delay(timestamp)
return 0 unless self.last_update_started_at return 0 unless self.last_update_started_at
......
...@@ -34,7 +34,7 @@ module EE ...@@ -34,7 +34,7 @@ module EE
log_audit_events log_audit_events
sync_wiki_on_enable if !wiki_was_enabled && project.wiki_enabled? 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 end
result result
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
Repository mirroring on #{@project.full_path} has been paused due to too many failures. The last failure was: Repository mirroring on #{@project.full_path} has been paused due to too many failures. The last failure was:
%pre %pre
= @project.import_error = @project.import_state.last_error
%p %p
To resume mirroring update your #{link_to("repository mirroring settings", project_settings_repository_url(@project))}. To resume mirroring update your #{link_to("repository mirroring settings", project_settings_repository_url(@project))}.
Repository mirroring on <%= @project.full_path %> has been paused due to too many failures. The last failure was: 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) %>. To resume mirroring update your repository settings at <%= project_settings_repository_url(@project) %>.
- if @project.mirror - return unless @project.mirror
%tr
%td= @project.username_only_import_url - import_state = @project.import_state
%td= _('Pull')
%td= @project.mirror_last_update_at.present? ? time_ago_with_tooltip(@project.mirror_last_update_at) : _('Never') %tr
%td %td= @project.username_only_import_url
- if @project.import_error.present? %td= _('Pull')
.badge.mirror-error-badge{ data: { toggle: 'tooltip', html: 'true' }, title: html_escape(@project.import_error.try(:strip)) }= _('Error') %td= import_state.last_update_at.present? ? time_ago_with_tooltip(import_state.last_update_at) : _('Never')
%td.mirror-action-buttons %td
.btn-group.mirror-actions-group.pull-right{ role: 'group' } - if import_state&.last_error.present?
- if @project.mirror? .badge.mirror-error-badge{ data: { toggle: 'tooltip', html: 'true' }, title: html_escape(import_state.last_error.try(:strip)) }= _('Error')
- if @project.mirror_about_to_update? || @project.updating_mirror? %td.mirror-action-buttons
%button.btn.disabled{ type: 'button', data: { container: 'body', toggle: 'tooltip' }, title: _('Updating') }= icon("refresh spin") .btn-group.mirror-actions-group.pull-right{ role: 'group' }
- else - if @project.mirror?
= 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 - if import_state.mirror_update_due? || import_state.updating_mirror?
= icon("refresh") %button.btn.disabled{ type: 'button', data: { container: 'body', toggle: 'tooltip' }, title: _('Updating') }= icon("refresh spin")
%button.js-delete-mirror.js-delete-pull-mirror.btn.btn-danger{ type: 'button', data: { toggle: 'tooltip', container: 'body' }, title: _('Remove') }= icon('trash-o') - 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')
...@@ -5,4 +5,4 @@ ...@@ -5,4 +5,4 @@
.card-body .card-body
%pre %pre
:preserve :preserve
#{h(@project.import_error.try(:strip))} #{h(@project.import_state.last_error.try(:strip))}
- 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) - raw_message = local_assigns.fetch(:raw_message, false)
- case @project.mirror_last_update_status - case @project.import_state.last_update_status
- when :success - when :success
Updated #{time_ago_with_tooltip(last_successful_update_at)}. Updated #{time_ago_with_tooltip(last_successful_update_at)}.
- when :failed - when :failed
......
- if @project.mirror? && can?(current_user, :push_code, @project) - if @project.mirror? && can?(current_user, :push_code, @project)
.append-bottom-default .append-bottom-default
- if @project.mirror_about_to_update? - if @project.import_state.mirror_update_due?
%span.btn.disabled %span.btn.disabled
= icon("refresh spin") = icon("refresh spin")
Update Scheduled&hellip; Update Scheduled&hellip;
- elsif @project.updating_mirror? - elsif @project.import_state.updating_mirror?
%span.btn.disabled %span.btn.disabled
= icon("refresh spin") = icon("refresh spin")
Updating&hellip; Updating&hellip;
...@@ -14,4 +14,4 @@ ...@@ -14,4 +14,4 @@
Update Now Update Now
- if @project.mirror_last_update_succeeded? - if @project.mirror_last_update_succeeded?
%p.inline.prepend-left-10 %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)}.
...@@ -8,7 +8,7 @@ module EE ...@@ -8,7 +8,7 @@ module EE
# Explicitly enqueue mirror for update so # Explicitly enqueue mirror for update so
# that upstream remote is created and fetched # that upstream remote is created and fetched
project.force_import_job! if project.mirror? project.import_state.force_import_job! if project.mirror?
end end
override :template_import? override :template_import?
......
class ProjectImportScheduleWorker class ProjectImportScheduleWorker
ImportStateNotFound = Class.new(StandardError)
include ApplicationWorker include ApplicationWorker
prepend WaitableWorker prepend WaitableWorker
sidekiq_options retry: false
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def perform(project_id) def perform(project_id)
project = Project.find_by(id: project_id) import_state = ProjectImportState.find_by(project_id: project_id)
project&.import_schedule raise ImportStateNotFound unless import_state
import_state.schedule
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
end end
...@@ -45,38 +45,31 @@ class RepositoryUpdateMirrorWorker ...@@ -45,38 +45,31 @@ class RepositoryUpdateMirrorWorker
end end
def start_mirror(project) def start_mirror(project)
if start(project) import_state = project.import_state
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)
Gitlab::Metrics.add_event_with_values( if start(import_state)
:mirrors_running, Rails.logger.info("Mirror update for #{project.full_path} started. Waiting duration: #{import_state.mirror_waiting_duration}")
{ duration: project.mirror_waiting_duration }, metric_mirror_waiting_duration_seconds.observe({}, import_state.mirror_waiting_duration)
{ path: project.full_path })
true true
else 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 false
end end
end end
def fail_mirror(project, message) 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}") Rails.logger.error("Mirror update for #{project.full_path} failed with the following message: #{message}")
Gitlab::Metrics.add_event(:mirrors_failed)
end end
def finish_mirror(project) 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}}.") Rails.logger.info("Mirror update for #{project.full_path} successfully finished. Update duration: #{import_state.mirror_update_duration}}.")
Gitlab::Metrics.add_event_with_values( metric_mirror_update_duration_seconds.observe({}, import_state.mirror_update_duration)
:mirrors_finished,
{ duration: project.mirror_update_duration })
metric_mirror_update_duration_seconds.observe({}, project.mirror_update_duration)
end end
def metric_mirror_update_duration_seconds def metric_mirror_update_duration_seconds
......
...@@ -51,7 +51,7 @@ module API ...@@ -51,7 +51,7 @@ module API
break render_api_error!('The project is not mirrored', 400) unless project.mirror? 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 status 200
end end
......
...@@ -120,7 +120,7 @@ describe Projects::MirrorsController do ...@@ -120,7 +120,7 @@ describe Projects::MirrorsController do
project = create(:project, :mirror) project = create(:project, :mirror)
sign_in(project.owner) 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 } put :update_now, { namespace_id: project.namespace.to_param, project_id: project.to_param }
end end
......
...@@ -192,7 +192,7 @@ describe ProjectsController do ...@@ -192,7 +192,7 @@ describe ProjectsController do
end end
it 'updates repository mirror attributes' do 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, put :update,
namespace_id: project.namespace, namespace_id: project.namespace,
......
...@@ -24,7 +24,7 @@ describe 'Project mirror', :js do ...@@ -24,7 +24,7 @@ describe 'Project mirror', :js do
it 'forces import' do it 'forces import' do
import_state.update(last_update_at: timestamp - 8.minutes) 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 Timecop.freeze(timestamp) do
visit project_mirror_path(project) visit project_mirror_path(project)
...@@ -38,7 +38,7 @@ describe 'Project mirror', :js do ...@@ -38,7 +38,7 @@ describe 'Project mirror', :js do
it 'does not force import' do it 'does not force import' do
import_state.update(last_update_at: timestamp - 3.minutes) 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 Timecop.freeze(timestamp) do
visit project_mirror_path(project) visit project_mirror_path(project)
......
This diff is collapsed.
This diff is collapsed.
...@@ -163,7 +163,7 @@ describe API::ProjectMirror do ...@@ -163,7 +163,7 @@ describe API::ProjectMirror do
end end
it 'syncs the mirror' do 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 do_post
end end
...@@ -182,7 +182,7 @@ describe API::ProjectMirror do ...@@ -182,7 +182,7 @@ describe API::ProjectMirror do
end end
it "doesn't sync the mirror" do 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' } post api("/projects/#{project_mirrored.id}/mirror/pull"), {}, { 'X-Hub-Signature' => 'signature' }
end end
......
...@@ -177,7 +177,7 @@ describe API::Projects do ...@@ -177,7 +177,7 @@ describe API::Projects do
mirror_params[:mirror_user_id] = admin.id mirror_params[:mirror_user_id] = admin.id
project.add_maintainer(admin) 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) put(api("/projects/#{project.id}", admin), mirror_params)
...@@ -194,7 +194,7 @@ describe API::Projects do ...@@ -194,7 +194,7 @@ describe API::Projects do
end end
it 'updates mirror related attributes' do 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) put(api("/projects/#{project.id}", user), mirror_params)
......
...@@ -22,7 +22,7 @@ describe Projects::UpdateService, '#execute' do ...@@ -22,7 +22,7 @@ describe Projects::UpdateService, '#execute' do
} }
stub_licensed_features(repository_mirrors: true) 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) update_project(project, user, opts)
end end
......
...@@ -42,7 +42,7 @@ describe 'shared/_mirror_status.html.haml' do ...@@ -42,7 +42,7 @@ describe 'shared/_mirror_status.html.haml' do
context 'with a previous successful update' do context 'with a previous successful update' do
it 'renders failure message' 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 render 'shared/mirror_status', raw_message: true
......
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
...@@ -7,14 +7,15 @@ describe RepositoryImportWorker do ...@@ -7,14 +7,15 @@ describe RepositoryImportWorker do
stub_licensed_features(custom_project_templates: true) stub_licensed_features(custom_project_templates: true)
error = %q{remote: Not Found fatal: repository 'https://user:pass@test.com/root/repoC.git/' not found } 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_any_instance_of(Projects::ImportService).to receive(:execute).and_return({ status: :error, message: error })
expect do expect do
subject.perform(project.id) subject.perform(project.id)
end.to raise_error(RuntimeError, error) 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 end
context 'when project is a mirror' do context 'when project is a mirror' do
...@@ -24,7 +25,7 @@ describe RepositoryImportWorker do ...@@ -24,7 +25,7 @@ describe RepositoryImportWorker do
expect_any_instance_of(Projects::ImportService).to receive(:execute) expect_any_instance_of(Projects::ImportService).to receive(:execute)
.and_return({ status: :ok }) .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) subject.perform(project.id)
end end
......
...@@ -145,7 +145,9 @@ module API ...@@ -145,7 +145,9 @@ module API
expose :import_status expose :import_status
# TODO: Use `expose_nil` once we upgrade the grape-entity gem # TODO: Use `expose_nil` once we upgrade the grape-entity gem
expose :import_error, if: lambda { |status, _ops| status.import_error } expose :import_error, if: lambda { |project, _ops| project.import_state&.last_error } do |project|
project.import_state.last_error
end
end end
class BasicProjectDetails < ProjectIdentity class BasicProjectDetails < ProjectIdentity
...@@ -248,7 +250,10 @@ module API ...@@ -248,7 +250,10 @@ module API
expose :creator_id expose :creator_id
expose :forked_from_project, using: Entities::BasicProjectDetails, if: lambda { |project, options| project.forked? } expose :forked_from_project, using: Entities::BasicProjectDetails, if: lambda { |project, options| project.forked? }
expose :import_status expose :import_status
expose :import_error, if: lambda { |_project, options| options[:user_can_admin_project] }
expose :import_error, if: lambda { |_project, options| options[:user_can_admin_project] } do |project|
project.import_state&.last_error
end
expose :open_issues_count, if: lambda { |project, options| project.feature_available?(:issues, options[:current_user]) } expose :open_issues_count, if: lambda { |project, options| project.feature_available?(:issues, options[:current_user]) }
expose :runners_token, if: lambda { |_project, options| options[:user_can_admin_project] } expose :runners_token, if: lambda { |_project, options| options[:user_can_admin_project] }
......
...@@ -35,7 +35,7 @@ module Gitlab ...@@ -35,7 +35,7 @@ module Gitlab
def handle_errors def handle_errors
return unless errors.any? return unless errors.any?
project.update_column(:import_error, { project.import_state.update_column(:last_error, {
message: 'The remote data could not be fully imported.', message: 'The remote data could not be fully imported.',
errors: errors errors: errors
}.to_json) }.to_json)
......
...@@ -56,7 +56,7 @@ module Gitlab ...@@ -56,7 +56,7 @@ module Gitlab
def handle_errors def handle_errors
return unless errors.any? return unless errors.any?
project.update_column(:import_error, { project.import_state.update_column(:last_error, {
message: 'The remote data could not be fully imported.', message: 'The remote data could not be fully imported.',
errors: errors errors: errors
}.to_json) }.to_json)
......
...@@ -80,7 +80,7 @@ module Gitlab ...@@ -80,7 +80,7 @@ module Gitlab
end end
def fail_import(message) def fail_import(message)
project.mark_import_as_failed(message) project.import_state.mark_as_failed(message)
false false
end end
end end
......
...@@ -43,8 +43,7 @@ module Gitlab ...@@ -43,8 +43,7 @@ module Gitlab
Gitlab::SidekiqStatus Gitlab::SidekiqStatus
.set(jid, StuckImportJobsWorker::IMPORT_JOBS_EXPIRATION) .set(jid, StuckImportJobsWorker::IMPORT_JOBS_EXPIRATION)
project.ensure_import_state project.import_state.update_column(:jid, jid)
project.import_state&.update_column(:jid, jid)
Stage::ImportRepositoryWorker Stage::ImportRepositoryWorker
.perform_async(project.id) .perform_async(project.id)
......
...@@ -101,13 +101,11 @@ excluded_attributes: ...@@ -101,13 +101,11 @@ excluded_attributes:
- :avatar - :avatar
- :import_type - :import_type
- :import_source - :import_source
- :import_error
- :mirror - :mirror
- :runners_token - :runners_token
- :repository_storage - :repository_storage
- :repository_read_only - :repository_read_only
- :lfs_enabled - :lfs_enabled
- :import_jid
- :created_at - :created_at
- :updated_at - :updated_at
- :id - :id
...@@ -115,8 +113,6 @@ excluded_attributes: ...@@ -115,8 +113,6 @@ excluded_attributes:
- :last_activity_at - :last_activity_at
- :last_repository_updated_at - :last_repository_updated_at
- :last_repository_check_at - :last_repository_check_at
- :mirror_last_update_at
- :mirror_last_successful_update_at
- :mirror_user_id - :mirror_user_id
- :mirror_trigger_builds - :mirror_trigger_builds
- :storage_version - :storage_version
...@@ -127,6 +123,13 @@ excluded_attributes: ...@@ -127,6 +123,13 @@ excluded_attributes:
- :description_html - :description_html
- :repository_languages - :repository_languages
- :packages_enabled - :packages_enabled
- :mirror_last_update_at
- :mirror_last_successful_update_at
project_import_state:
- :last_error
- :jid
- :last_update_at
- :last_successful_update_at
prometheus_metrics: prometheus_metrics:
- :common - :common
- :identifier - :identifier
......
...@@ -80,8 +80,7 @@ module Gitlab ...@@ -80,8 +80,7 @@ module Gitlab
def handle_errors def handle_errors
return unless errors.any? return unless errors.any?
project.ensure_import_state project.import_state.update_column(:last_error, {
project.import_state&.update_column(:last_error, {
message: 'The remote data could not be fully imported.', message: 'The remote data could not be fully imported.',
errors: errors errors: errors
}.to_json) }.to_json)
......
...@@ -42,7 +42,7 @@ class GithubImport ...@@ -42,7 +42,7 @@ class GithubImport
end end
def import! def import!
@project.force_import_start @project.import_state.force_start
import_success = false import_success = false
...@@ -57,7 +57,7 @@ class GithubImport ...@@ -57,7 +57,7 @@ class GithubImport
puts "Import finished. Timings: #{timings}".color(:green) puts "Import finished. Timings: #{timings}".color(:green)
else else
puts "Import was not successful. Errors were as follows:" puts "Import was not successful. Errors were as follows:"
puts @project.import_error puts @project.import_state.last_error
end end
end end
......
...@@ -3334,6 +3334,9 @@ msgstr "" ...@@ -3334,6 +3334,9 @@ msgstr ""
msgid "EventFilterBy|Filter by team" msgid "EventFilterBy|Filter by team"
msgstr "" msgstr ""
msgid "Every %{action} attempt has failed: %{job_error_message}. Please try again."
msgstr ""
msgid "Every day (at 4:00am)" msgid "Every day (at 4:00am)"
msgstr "" msgstr ""
...@@ -4518,6 +4521,9 @@ msgstr "" ...@@ -4518,6 +4521,9 @@ msgstr ""
msgid "Import repository" msgid "Import repository"
msgstr "" msgstr ""
msgid "Import timed out. Import took longer than %{import_jobs_expiration} seconds"
msgstr ""
msgid "ImportButtons|Connect repositories from" msgid "ImportButtons|Connect repositories from"
msgstr "" msgstr ""
...@@ -7879,6 +7885,9 @@ msgstr "" ...@@ -7879,6 +7885,9 @@ msgstr ""
msgid "Source is not available" msgid "Source is not available"
msgstr "" msgstr ""
msgid "Source project cannot be found."
msgstr ""
msgid "Spam Logs" msgid "Spam Logs"
msgstr "" msgstr ""
......
...@@ -126,7 +126,7 @@ describe Import::BitbucketServerController do ...@@ -126,7 +126,7 @@ describe Import::BitbucketServerController do
end end
it 'assigns repository categories' do it 'assigns repository categories' do
created_project = create(:project, import_type: 'bitbucket_server', creator_id: user.id, import_status: 'finished', import_source: @created_repo.browse_url) created_project = create(:project, :import_finished, import_type: 'bitbucket_server', creator_id: user.id, import_source: @created_repo.browse_url)
repos = instance_double(BitbucketServer::Collection) repos = instance_double(BitbucketServer::Collection)
expect(repos).to receive(:partition).and_return([[@repo, @created_repo], [@invalid_repo]]) expect(repos).to receive(:partition).and_return([[@repo, @created_repo], [@invalid_repo]])
......
...@@ -26,10 +26,11 @@ describe Projects::ImportsController do ...@@ -26,10 +26,11 @@ describe Projects::ImportsController do
context 'when repository exists' do context 'when repository exists' do
let(:project) { create(:project_empty_repo, import_url: 'https://github.com/vim/vim.git') } let(:project) { create(:project_empty_repo, import_url: 'https://github.com/vim/vim.git') }
let(:import_state) { project.import_state }
context 'when import is in progress' do context 'when import is in progress' do
before do before do
project.update(import_status: :started) import_state.update(status: :started)
end end
it 'renders template' do it 'renders template' do
...@@ -47,7 +48,7 @@ describe Projects::ImportsController do ...@@ -47,7 +48,7 @@ describe Projects::ImportsController do
context 'when import failed' do context 'when import failed' do
before do before do
project.update(import_status: :failed) import_state.update(status: :failed)
end end
it 'redirects to new_namespace_project_import_path' do it 'redirects to new_namespace_project_import_path' do
...@@ -59,7 +60,7 @@ describe Projects::ImportsController do ...@@ -59,7 +60,7 @@ describe Projects::ImportsController do
context 'when import finished' do context 'when import finished' do
before do before do
project.update(import_status: :finished) import_state.update(status: :finished)
end end
context 'when project is a fork' do context 'when project is a fork' do
...@@ -108,7 +109,7 @@ describe Projects::ImportsController do ...@@ -108,7 +109,7 @@ describe Projects::ImportsController do
context 'when import never happened' do context 'when import never happened' do
before do before do
project.update(import_status: :none) import_state.update(status: :none)
end end
it 'redirects to namespace_project_path' do it 'redirects to namespace_project_path' do
......
...@@ -5,6 +5,7 @@ FactoryBot.define do ...@@ -5,6 +5,7 @@ FactoryBot.define do
transient do transient do
import_url { generate(:url) } import_url { generate(:url) }
import_type nil
end end
trait :repository do trait :repository do
...@@ -59,7 +60,11 @@ FactoryBot.define do ...@@ -59,7 +60,11 @@ FactoryBot.define do
end end
after(:create) do |import_state, evaluator| after(:create) do |import_state, evaluator|
import_state.project.update_columns(import_url: evaluator.import_url) columns = {}
columns[:import_url] = evaluator.import_url unless evaluator.import_url.blank?
columns[:import_type] = evaluator.import_type unless evaluator.import_type.blank?
import_state.project.update_columns(columns)
end end
end end
end end
...@@ -30,6 +30,11 @@ FactoryBot.define do ...@@ -30,6 +30,11 @@ FactoryBot.define do
# we can't assign the delegated `#ci_cd_settings` attributes directly, as the # we can't assign the delegated `#ci_cd_settings` attributes directly, as the
# `#ci_cd_settings` relation needs to be created first # `#ci_cd_settings` relation needs to be created first
group_runners_enabled nil group_runners_enabled nil
import_status nil
import_jid nil
last_update_at nil
last_successful_update_at nil
retry_count 0
end end
after(:create) do |project, evaluator| after(:create) do |project, evaluator|
...@@ -64,6 +69,16 @@ FactoryBot.define do ...@@ -64,6 +69,16 @@ FactoryBot.define do
# assign the delegated `#ci_cd_settings` attributes after create # assign the delegated `#ci_cd_settings` attributes after create
project.reload.group_runners_enabled = evaluator.group_runners_enabled unless evaluator.group_runners_enabled.nil? project.reload.group_runners_enabled = evaluator.group_runners_enabled unless evaluator.group_runners_enabled.nil?
if evaluator.import_status
import_state = project.import_state || project.build_import_state
import_state.status = evaluator.import_status
import_state.last_update_at = evaluator.last_update_at
import_state.last_successful_update_at = evaluator.last_successful_update_at
import_state.retry_count = evaluator.retry_count
import_state.jid = evaluator.import_jid
import_state.save
end
end end
trait :public do trait :public do
...@@ -102,22 +117,19 @@ FactoryBot.define do ...@@ -102,22 +117,19 @@ FactoryBot.define do
timestamp = Time.now timestamp = Time.now
import_status :finished import_status :finished
mirror_last_update_at timestamp last_update_at timestamp
mirror_last_successful_update_at timestamp last_successful_update_at timestamp
end end
trait :import_failed do trait :import_failed do
import_status :failed import_status :failed
mirror_last_update_at { Time.now } last_update_at { Time.now }
end end
trait :import_hard_failed do trait :import_hard_failed do
import_status :failed import_status :failed
mirror_last_update_at { Time.now - 1.minute } last_update_at { Time.now - 1.minute }
retry_count { Gitlab::Mirror::MAX_RETRY + 1 }
after(:create) do |project|
project.import_state&.update_attributes(retry_count: Gitlab::Mirror::MAX_RETRY + 1)
end
end end
trait :disabled_mirror do trait :disabled_mirror do
......
...@@ -218,7 +218,7 @@ describe Gitlab::GithubImport::Importer::RepositoryImporter do ...@@ -218,7 +218,7 @@ describe Gitlab::GithubImport::Importer::RepositoryImporter do
describe '#fail_import' do describe '#fail_import' do
it 'marks the import as failed' do it 'marks the import as failed' do
expect(project).to receive(:mark_import_as_failed).with('foo') expect(project.import_state).to receive(:mark_as_failed).with('foo')
expect(importer.fail_import('foo')).to eq(false) expect(importer.fail_import('foo')).to eq(false)
end end
......
...@@ -36,7 +36,7 @@ describe Gitlab::GithubImport::ParallelImporter do ...@@ -36,7 +36,7 @@ describe Gitlab::GithubImport::ParallelImporter do
it 'updates the import JID of the project' do it 'updates the import JID of the project' do
importer.execute importer.execute
expect(project.reload.import_jid).to eq("github-importer/#{project.id}") expect(project.import_state.reload.jid).to eq("github-importer/#{project.id}")
end end
end end
end end
...@@ -174,7 +174,7 @@ describe Gitlab::LegacyGithubImport::Importer do ...@@ -174,7 +174,7 @@ describe Gitlab::LegacyGithubImport::Importer do
described_class.new(project).execute described_class.new(project).execute
expect(project.import_error).to eq error.to_json expect(project.import_state.last_error).to eq error.to_json
end end
end end
......
...@@ -10,4 +10,116 @@ describe ProjectImportState, type: :model do ...@@ -10,4 +10,116 @@ describe ProjectImportState, type: :model do
describe 'validations' do describe 'validations' do
it { is_expected.to validate_presence_of(:project) } it { is_expected.to validate_presence_of(:project) }
end end
describe 'Project import job' do
let(:import_state) { create(:import_state, import_url: generate(:url)) }
let(:project) { import_state.project }
before do
allow_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:import_repository)
.with(project.import_url).and_return(true)
# Works around https://github.com/rspec/rspec-mocks/issues/910
allow(Project).to receive(:find).with(project.id).and_return(project)
expect(project.repository).to receive(:after_import).and_call_original
expect(project.wiki.repository).to receive(:after_import).and_call_original
end
it 'imports a project' do
expect(RepositoryImportWorker).to receive(:perform_async).and_call_original
expect { import_state.schedule }.to change { import_state.jid }
expect(import_state.status).to eq('finished')
end
end
describe '#human_status_name' do
context 'when import_state exists' do
it 'returns the humanized status name' do
import_state = build(:import_state, :started)
expect(import_state.human_status_name).to eq("started")
end
end
end
describe 'import state transitions' do
context 'state transition: [:started] => [:finished]' do
let(:after_import_service) { spy(:after_import_service) }
let(:housekeeping_service) { spy(:housekeeping_service) }
before do
allow(Projects::AfterImportService)
.to receive(:new) { after_import_service }
allow(after_import_service)
.to receive(:execute) { housekeeping_service.execute }
allow(Projects::HousekeepingService)
.to receive(:new) { housekeeping_service }
end
it 'resets last_error' do
error_message = 'Some error'
import_state = create(:import_state, :started, last_error: error_message)
expect { import_state.finish }.to change { import_state.last_error }.from(error_message).to(nil)
end
it 'performs housekeeping when an import of a fresh project is completed' do
project = create(:project_empty_repo, :import_started, import_type: :github)
project.import_state.finish
expect(after_import_service).to have_received(:execute)
expect(housekeeping_service).to have_received(:execute)
end
it 'does not perform housekeeping when project repository does not exist' do
project = create(:project, :import_started, import_type: :github)
project.import_state.finish
expect(housekeeping_service).not_to have_received(:execute)
end
it 'does not perform housekeeping when project does not have a valid import type' do
project = create(:project, :import_started, import_type: nil)
project.import_state.finish
expect(housekeeping_service).not_to have_received(:execute)
end
end
end
describe '#remove_jid', :clean_gitlab_redis_cache do
let(:project) { }
context 'without an JID' do
it 'does nothing' do
import_state = create(:import_state)
expect(Gitlab::SidekiqStatus)
.not_to receive(:unset)
import_state.remove_jid
end
end
context 'with an JID' do
it 'unsets the JID' do
import_state = create(:import_state, jid: '123')
expect(Gitlab::SidekiqStatus)
.to receive(:unset)
.with('123')
.and_call_original
import_state.remove_jid
expect(import_state.jid).to be_nil
end
end
end
end end
...@@ -1819,7 +1819,7 @@ describe Project do ...@@ -1819,7 +1819,7 @@ describe Project do
it 'returns the full URL' do it 'returns the full URL' do
project = create(:project, :mirror, import_url: 'http://user:pass@test.com') project = create(:project, :mirror, import_url: 'http://user:pass@test.com')
project.import_finish project.import_state.finish
expect(project.reload.import_url).to eq('http://user:pass@test.com') expect(project.reload.import_url).to eq('http://user:pass@test.com')
end end
...@@ -1829,7 +1829,7 @@ describe Project do ...@@ -1829,7 +1829,7 @@ describe Project do
it 'returns the sanitized URL' do it 'returns the sanitized URL' do
project = create(:project, :import_started, import_url: 'http://user:pass@test.com') project = create(:project, :import_started, import_url: 'http://user:pass@test.com')
project.import_finish project.import_state.finish
expect(project.reload.import_url).to eq('http://test.com') expect(project.reload.import_url).to eq('http://test.com')
end end
...@@ -1949,106 +1949,6 @@ describe Project do ...@@ -1949,106 +1949,6 @@ describe Project do
end end
end end
describe '#human_import_status_name' do
context 'when import_state exists' do
it 'returns the humanized status name' do
project = create(:project)
create(:import_state, :started, project: project)
expect(project.human_import_status_name).to eq("started")
end
end
context 'when import_state was not created yet' do
let(:project) { create(:project, :import_started) }
it 'ensures import_state is created and returns humanized status name' do
expect do
project.human_import_status_name
end.to change { ProjectImportState.count }.from(0).to(1)
end
it 'returns humanized status name' do
expect(project.human_import_status_name).to eq("started")
end
end
end
describe 'Project import job' do
let(:project) { create(:project, import_url: generate(:url)) }
before do
allow_any_instance_of(Gitlab::Shell).to receive(:import_repository)
.with(project.repository_storage, project.disk_path, project.import_url)
.and_return(true)
# Works around https://github.com/rspec/rspec-mocks/issues/910
allow(described_class).to receive(:find).with(project.id).and_return(project)
expect(project.repository).to receive(:after_import)
.and_call_original
expect(project.wiki.repository).to receive(:after_import)
.and_call_original
end
it 'imports a project' do
expect(RepositoryImportWorker).to receive(:perform_async).and_call_original
expect { project.import_schedule }.to change { project.import_jid }
expect(project.reload.import_status).to eq('finished')
end
end
describe 'project import state transitions' do
context 'state transition: [:started] => [:finished]' do
let(:after_import_service) { spy(:after_import_service) }
let(:housekeeping_service) { spy(:housekeeping_service) }
before do
allow(Projects::AfterImportService)
.to receive(:new) { after_import_service }
allow(after_import_service)
.to receive(:execute) { housekeeping_service.execute }
allow(Projects::HousekeepingService)
.to receive(:new) { housekeeping_service }
end
it 'resets project import_error' do
error_message = 'Some error'
mirror = create(:project_empty_repo, :import_started)
mirror.import_state.update(last_error: error_message)
expect { mirror.import_finish }.to change { mirror.import_error }.from(error_message).to(nil)
end
it 'performs housekeeping when an import of a fresh project is completed' do
project = create(:project_empty_repo, :import_started, import_type: :github)
project.import_finish
expect(after_import_service).to have_received(:execute)
expect(housekeeping_service).to have_received(:execute)
end
it 'does not perform housekeeping when project repository does not exist' do
project = create(:project, :import_started, import_type: :github)
project.import_finish
expect(housekeeping_service).not_to have_received(:execute)
end
it 'does not perform housekeeping when project does not have a valid import type' do
project = create(:project, :import_started, import_type: nil)
project.import_finish
expect(housekeeping_service).not_to have_received(:execute)
end
end
end
describe '#latest_successful_builds_for' do describe '#latest_successful_builds_for' do
def create_pipeline(status = 'success') def create_pipeline(status = 'success')
create(:ci_pipeline, project: project, create(:ci_pipeline, project: project,
...@@ -2128,6 +2028,42 @@ describe Project do ...@@ -2128,6 +2028,42 @@ describe Project do
end end
end end
describe '#import_status' do
context 'with import_state' do
it 'returns the right status' do
project = create(:project, :import_started)
expect(project.import_status).to eq("started")
end
end
context 'without import_state' do
it 'returns none' do
project = create(:project)
expect(project.import_status).to eq('none')
end
end
end
describe '#human_import_status_name' do
context 'with import_state' do
it 'returns the right human import status' do
project = create(:project, :import_started)
expect(project.human_import_status_name).to eq('started')
end
end
context 'without import_state' do
it 'returns none' do
project = create(:project)
expect(project.human_import_status_name).to eq('none')
end
end
end
describe '#add_import_job' do describe '#add_import_job' do
let(:import_jid) { '123' } let(:import_jid) { '123' }
...@@ -3705,13 +3641,14 @@ describe Project do ...@@ -3705,13 +3641,14 @@ describe Project do
describe '#after_import' do describe '#after_import' do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:import_state) { create(:import_state, project: project) }
it 'runs the correct hooks' do it 'runs the correct hooks' do
expect(project.repository).to receive(:after_import) expect(project.repository).to receive(:after_import)
expect(project.wiki.repository).to receive(:after_import) expect(project.wiki.repository).to receive(:after_import)
expect(project).to receive(:import_finish) expect(import_state).to receive(:finish)
expect(project).to receive(:update_project_counter_caches) expect(project).to receive(:update_project_counter_caches)
expect(project).to receive(:remove_import_jid) expect(import_state).to receive(:remove_jid)
expect(project).to receive(:after_create_default_branch) expect(project).to receive(:after_create_default_branch)
expect(project).to receive(:refresh_markdown_cache!) expect(project).to receive(:refresh_markdown_cache!)
...@@ -3719,7 +3656,11 @@ describe Project do ...@@ -3719,7 +3656,11 @@ describe Project do
end end
context 'branch protection' do context 'branch protection' do
let(:project) { create(:project, :repository, :import_started) } let(:project) { create(:project, :repository) }
before do
create(:import_state, :started, project: project)
end
it 'does not protect when branch protection is disabled' do it 'does not protect when branch protection is disabled' do
stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE) stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE)
...@@ -3776,37 +3717,6 @@ describe Project do ...@@ -3776,37 +3717,6 @@ describe Project do
end end
end end
describe '#remove_import_jid', :clean_gitlab_redis_cache do
let(:project) { }
context 'without an import JID' do
it 'does nothing' do
project = create(:project)
expect(Gitlab::SidekiqStatus)
.not_to receive(:unset)
project.remove_import_jid
end
end
context 'with an import JID' do
it 'unsets the import JID' do
project = create(:project)
create(:import_state, project: project, jid: '123')
expect(Gitlab::SidekiqStatus)
.to receive(:unset)
.with('123')
.and_call_original
project.remove_import_jid
expect(project.import_jid).to be_nil
end
end
end
describe '#wiki_repository_exists?' do describe '#wiki_repository_exists?' do
it 'returns true when the wiki repository exists' do it 'returns true when the wiki repository exists' do
project = create(:project, :wiki_repo) project = create(:project, :wiki_repo)
......
...@@ -42,7 +42,7 @@ describe API::ProjectImport do ...@@ -42,7 +42,7 @@ describe API::ProjectImport do
end end
it 'does not schedule an import for a namespace that does not exist' do it 'does not schedule an import for a namespace that does not exist' do
expect_any_instance_of(Project).not_to receive(:import_schedule) expect_any_instance_of(ProjectImportState).not_to receive(:schedule)
expect(::Projects::CreateService).not_to receive(:new) expect(::Projects::CreateService).not_to receive(:new)
post api('/projects/import', user), namespace: 'nonexistent', path: 'test-import2', file: fixture_file_upload(file) post api('/projects/import', user), namespace: 'nonexistent', path: 'test-import2', file: fixture_file_upload(file)
...@@ -52,7 +52,7 @@ describe API::ProjectImport do ...@@ -52,7 +52,7 @@ describe API::ProjectImport do
end end
it 'does not schedule an import if the user has no permission to the namespace' do it 'does not schedule an import if the user has no permission to the namespace' do
expect_any_instance_of(Project).not_to receive(:import_schedule) expect_any_instance_of(ProjectImportState).not_to receive(:schedule)
post(api('/projects/import', create(:user)), post(api('/projects/import', create(:user)),
path: 'test-import3', path: 'test-import3',
...@@ -64,7 +64,7 @@ describe API::ProjectImport do ...@@ -64,7 +64,7 @@ describe API::ProjectImport do
end end
it 'does not schedule an import if the user uploads no valid file' do it 'does not schedule an import if the user uploads no valid file' do
expect_any_instance_of(Project).not_to receive(:import_schedule) expect_any_instance_of(ProjectImportState).not_to receive(:schedule)
post api('/projects/import', user), path: 'test-import3', file: './random/test' post api('/projects/import', user), path: 'test-import3', file: './random/test'
...@@ -119,7 +119,7 @@ describe API::ProjectImport do ...@@ -119,7 +119,7 @@ describe API::ProjectImport do
let(:existing_project) { create(:project, namespace: user.namespace) } let(:existing_project) { create(:project, namespace: user.namespace) }
it 'does not schedule an import' do it 'does not schedule an import' do
expect_any_instance_of(Project).not_to receive(:import_schedule) expect_any_instance_of(ProjectImportState).not_to receive(:schedule)
post api('/projects/import', user), path: existing_project.path, file: fixture_file_upload(file) post api('/projects/import', user), path: existing_project.path, file: fixture_file_upload(file)
...@@ -139,7 +139,7 @@ describe API::ProjectImport do ...@@ -139,7 +139,7 @@ describe API::ProjectImport do
end end
def stub_import(namespace) def stub_import(namespace)
expect_any_instance_of(Project).to receive(:import_schedule) expect_any_instance_of(ProjectImportState).to receive(:schedule)
expect(::Projects::CreateService).to receive(:new).with(user, hash_including(namespace_id: namespace.id)).and_call_original expect(::Projects::CreateService).to receive(:new).with(user, hash_including(namespace_id: namespace.id)).and_call_original
end end
end end
......
...@@ -47,7 +47,7 @@ describe Projects::CreateFromTemplateService do ...@@ -47,7 +47,7 @@ describe Projects::CreateFromTemplateService do
end end
it 'is not scheduled' do it 'is not scheduled' do
expect(project.import_scheduled?).to be(false) expect(project.import_scheduled?).to be_nil
end end
it 'repository is empty' do it 'repository is empty' do
......
...@@ -58,14 +58,16 @@ describe Gitlab::GithubImport::StageMethods do ...@@ -58,14 +58,16 @@ describe Gitlab::GithubImport::StageMethods do
end end
describe '#find_project' do describe '#find_project' do
let(:import_state) { create(:import_state, project: project) }
it 'returns a Project for an existing ID' do it 'returns a Project for an existing ID' do
project.update_column(:import_status, 'started') import_state.update_column(:status, 'started')
expect(worker.find_project(project.id)).to eq(project) expect(worker.find_project(project.id)).to eq(project)
end end
it 'returns nil for a project that failed importing' do it 'returns nil for a project that failed importing' do
project.update_column(:import_status, 'failed') import_state.update_column(:status, 'failed')
expect(worker.find_project(project.id)).to be_nil expect(worker.find_project(project.id)).to be_nil
end end
......
...@@ -28,13 +28,23 @@ describe ProjectImportOptions do ...@@ -28,13 +28,23 @@ describe ProjectImportOptions do
worker_class.sidekiq_retries_exhausted_block.call(job) worker_class.sidekiq_retries_exhausted_block.call(job)
expect(project.reload.import_error).to include("fork") expect(project.import_state.reload.last_error).to include("fork")
end end
it 'logs the appropriate error message for forked projects' do it 'logs the appropriate error message for forked projects' do
worker_class.sidekiq_retries_exhausted_block.call(job) worker_class.sidekiq_retries_exhausted_block.call(job)
expect(project.reload.import_error).to include("import") expect(project.import_state.reload.last_error).to include("import")
end
context 'when project does not have import_state' do
let(:project) { create(:project) }
it 'raises an error' do
expect do
worker_class.sidekiq_retries_exhausted_block.call(job)
end.to raise_error(NoMethodError)
end
end end
end end
end end
...@@ -17,8 +17,8 @@ describe Gitlab::GithubImport::AdvanceStageWorker, :clean_gitlab_redis_shared_st ...@@ -17,8 +17,8 @@ describe Gitlab::GithubImport::AdvanceStageWorker, :clean_gitlab_redis_shared_st
context 'when there are remaining jobs' do context 'when there are remaining jobs' do
before do before do
allow(worker) allow(worker)
.to receive(:find_project) .to receive(:find_import_state)
.and_return(project) .and_return(import_state)
end end
it 'reschedules itself' do it 'reschedules itself' do
...@@ -38,8 +38,8 @@ describe Gitlab::GithubImport::AdvanceStageWorker, :clean_gitlab_redis_shared_st ...@@ -38,8 +38,8 @@ describe Gitlab::GithubImport::AdvanceStageWorker, :clean_gitlab_redis_shared_st
context 'when there are no remaining jobs' do context 'when there are no remaining jobs' do
before do before do
allow(worker) allow(worker)
.to receive(:find_project) .to receive(:find_import_state)
.and_return(project) .and_return(import_state)
allow(worker) allow(worker)
.to receive(:wait_for_jobs) .to receive(:wait_for_jobs)
...@@ -48,8 +48,8 @@ describe Gitlab::GithubImport::AdvanceStageWorker, :clean_gitlab_redis_shared_st ...@@ -48,8 +48,8 @@ describe Gitlab::GithubImport::AdvanceStageWorker, :clean_gitlab_redis_shared_st
end end
it 'schedules the next stage' do it 'schedules the next stage' do
expect(project) expect(import_state)
.to receive(:refresh_import_jid_expiration) .to receive(:refresh_jid_expiration)
expect(Gitlab::GithubImport::Stage::FinishImportWorker) expect(Gitlab::GithubImport::Stage::FinishImportWorker)
.to receive(:perform_async) .to receive(:perform_async)
...@@ -96,22 +96,18 @@ describe Gitlab::GithubImport::AdvanceStageWorker, :clean_gitlab_redis_shared_st ...@@ -96,22 +96,18 @@ describe Gitlab::GithubImport::AdvanceStageWorker, :clean_gitlab_redis_shared_st
end end
end end
describe '#find_project' do describe '#find_import_state' do
it 'returns a Project' do it 'returns a ProjectImportState' do
project.update_column(:import_status, 'started') import_state.update_column(:status, 'started')
found = worker.find_project(project.id) found = worker.find_import_state(project.id)
expect(found).to be_an_instance_of(Project) expect(found).to be_an_instance_of(ProjectImportState)
expect(found.attributes.keys).to match_array(%w(id jid))
# This test is there to make sure we only select the columns we care
# about.
# TODO: enable this assertion back again
# expect(found.attributes).to include({ 'id' => nil, 'import_jid' => '123' })
end end
it 'returns nil if the project import is not running' do it 'returns nil if the project import is not running' do
expect(worker.find_project(project.id)).to be_nil expect(worker.find_import_state(project.id)).to be_nil
end end
end end
end end
...@@ -29,7 +29,7 @@ describe Gitlab::GithubImport::RefreshImportJidWorker do ...@@ -29,7 +29,7 @@ describe Gitlab::GithubImport::RefreshImportJidWorker do
context 'when the job is running' do context 'when the job is running' do
it 'refreshes the import JID and reschedules itself' do it 'refreshes the import JID and reschedules itself' do
allow(worker) allow(worker)
.to receive(:find_project) .to receive(:find_import_state)
.with(project.id) .with(project.id)
.and_return(project) .and_return(project)
...@@ -39,7 +39,7 @@ describe Gitlab::GithubImport::RefreshImportJidWorker do ...@@ -39,7 +39,7 @@ describe Gitlab::GithubImport::RefreshImportJidWorker do
.and_return(true) .and_return(true)
expect(project) expect(project)
.to receive(:refresh_import_jid_expiration) .to receive(:refresh_jid_expiration)
expect(worker.class) expect(worker.class)
.to receive(:perform_in_the_future) .to receive(:perform_in_the_future)
...@@ -52,7 +52,7 @@ describe Gitlab::GithubImport::RefreshImportJidWorker do ...@@ -52,7 +52,7 @@ describe Gitlab::GithubImport::RefreshImportJidWorker do
context 'when the job is no longer running' do context 'when the job is no longer running' do
it 'returns' do it 'returns' do
allow(worker) allow(worker)
.to receive(:find_project) .to receive(:find_import_state)
.with(project.id) .with(project.id)
.and_return(project) .and_return(project)
...@@ -62,18 +62,18 @@ describe Gitlab::GithubImport::RefreshImportJidWorker do ...@@ -62,18 +62,18 @@ describe Gitlab::GithubImport::RefreshImportJidWorker do
.and_return(false) .and_return(false)
expect(project) expect(project)
.not_to receive(:refresh_import_jid_expiration) .not_to receive(:refresh_jid_expiration)
worker.perform(project.id, '123') worker.perform(project.id, '123')
end end
end end
end end
describe '#find_project' do describe '#find_import_state' do
it 'returns a Project' do it 'returns a ProjectImportState' do
project = create(:project, :import_started) project = create(:project, :import_started)
expect(worker.find_project(project.id)).to be_an_instance_of(Project) expect(worker.find_import_state(project.id)).to be_an_instance_of(ProjectImportState)
end end
# it 'only selects the import JID field' do # it 'only selects the import JID field' do
...@@ -84,14 +84,14 @@ describe Gitlab::GithubImport::RefreshImportJidWorker do ...@@ -84,14 +84,14 @@ describe Gitlab::GithubImport::RefreshImportJidWorker do
# .to eq({ 'id' => nil, 'import_jid' => '123abc' }) # .to eq({ 'id' => nil, 'import_jid' => '123abc' })
# end # end
it 'returns nil for a project for which the import process failed' do it 'returns nil for a import state for which the import process failed' do
project = create(:project, :import_failed) project = create(:project, :import_failed)
expect(worker.find_project(project.id)).to be_nil expect(worker.find_import_state(project.id)).to be_nil
end end
it 'returns nil for a non-existing project' do it 'returns nil for a non-existing find_import_state' do
expect(worker.find_project(-1)).to be_nil expect(worker.find_import_state(-1)).to be_nil
end end
end end
end end
...@@ -2,6 +2,7 @@ require 'spec_helper' ...@@ -2,6 +2,7 @@ require 'spec_helper'
describe Gitlab::GithubImport::Stage::ImportBaseDataWorker do describe Gitlab::GithubImport::Stage::ImportBaseDataWorker do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:import_state) { create(:import_state, project: project) }
let(:worker) { described_class.new } let(:worker) { described_class.new }
describe '#import' do describe '#import' do
...@@ -18,7 +19,7 @@ describe Gitlab::GithubImport::Stage::ImportBaseDataWorker do ...@@ -18,7 +19,7 @@ describe Gitlab::GithubImport::Stage::ImportBaseDataWorker do
expect(importer).to receive(:execute) expect(importer).to receive(:execute)
end end
expect(project).to receive(:refresh_import_jid_expiration) expect(import_state).to receive(:refresh_jid_expiration)
expect(Gitlab::GithubImport::Stage::ImportPullRequestsWorker) expect(Gitlab::GithubImport::Stage::ImportPullRequestsWorker)
.to receive(:perform_async) .to receive(:perform_async)
......
...@@ -2,6 +2,7 @@ require 'spec_helper' ...@@ -2,6 +2,7 @@ require 'spec_helper'
describe Gitlab::GithubImport::Stage::ImportPullRequestsWorker do describe Gitlab::GithubImport::Stage::ImportPullRequestsWorker do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:import_state) { create(:import_state, project: project) }
let(:worker) { described_class.new } let(:worker) { described_class.new }
describe '#import' do describe '#import' do
...@@ -19,8 +20,8 @@ describe Gitlab::GithubImport::Stage::ImportPullRequestsWorker do ...@@ -19,8 +20,8 @@ describe Gitlab::GithubImport::Stage::ImportPullRequestsWorker do
.to receive(:execute) .to receive(:execute)
.and_return(waiter) .and_return(waiter)
expect(project) expect(import_state)
.to receive(:refresh_import_jid_expiration) .to receive(:refresh_jid_expiration)
expect(Gitlab::GithubImport::AdvanceStageWorker) expect(Gitlab::GithubImport::AdvanceStageWorker)
.to receive(:perform_async) .to receive(:perform_async)
......
...@@ -9,13 +9,13 @@ describe RepositoryImportWorker do ...@@ -9,13 +9,13 @@ describe RepositoryImportWorker do
describe '#perform' do describe '#perform' do
let(:project) { create(:project, :import_scheduled) } let(:project) { create(:project, :import_scheduled) }
let(:import_state) { project.import_state }
context 'when worker was reset without cleanup' do context 'when worker was reset without cleanup' do
it 'imports the project successfully' do it 'imports the project successfully' do
jid = '12345678' jid = '12345678'
started_project = create(:project) started_project = create(:project)
started_import_state = create(:import_state, :started, project: started_project, jid: jid)
create(:import_state, :started, project: started_project, jid: jid)
allow(subject).to receive(:jid).and_return(jid) allow(subject).to receive(:jid).and_return(jid)
...@@ -23,12 +23,12 @@ describe RepositoryImportWorker do ...@@ -23,12 +23,12 @@ describe RepositoryImportWorker do
.and_return({ status: :ok }) .and_return({ status: :ok })
# Works around https://github.com/rspec/rspec-mocks/issues/910 # Works around https://github.com/rspec/rspec-mocks/issues/910
expect(Project).to receive(:find).with(project.id).and_return(project) expect(Project).to receive(:find).with(started_project.id).and_return(started_project)
expect(project.repository).to receive(:expire_emptiness_caches) expect(started_project.repository).to receive(:expire_emptiness_caches)
expect(project.wiki.repository).to receive(:expire_emptiness_caches) expect(started_project.wiki.repository).to receive(:expire_emptiness_caches)
expect(project).to receive(:import_finish) expect(started_import_state).to receive(:finish)
subject.perform(project.id) subject.perform(started_project.id)
end end
end end
...@@ -41,7 +41,7 @@ describe RepositoryImportWorker do ...@@ -41,7 +41,7 @@ describe RepositoryImportWorker do
expect(Project).to receive(:find).with(project.id).and_return(project) expect(Project).to receive(:find).with(project.id).and_return(project)
expect(project.repository).to receive(:expire_emptiness_caches) expect(project.repository).to receive(:expire_emptiness_caches)
expect(project.wiki.repository).to receive(:expire_emptiness_caches) expect(project.wiki.repository).to receive(:expire_emptiness_caches)
expect(project).to receive(:import_finish) expect(import_state).to receive(:finish)
subject.perform(project.id) subject.perform(project.id)
end end
...@@ -51,26 +51,27 @@ describe RepositoryImportWorker do ...@@ -51,26 +51,27 @@ describe RepositoryImportWorker do
it 'hide the credentials that were used in the import URL' do it 'hide the credentials that were used in the import URL' do
error = %q{remote: Not Found fatal: repository 'https://user:pass@test.com/root/repoC.git/' not found } error = %q{remote: Not Found fatal: repository 'https://user:pass@test.com/root/repoC.git/' not found }
project.update(import_jid: '123') import_state.update(jid: '123')
expect_any_instance_of(Projects::ImportService).to receive(:execute).and_return({ status: :error, message: error }) expect_any_instance_of(Projects::ImportService).to receive(:execute).and_return({ status: :error, message: error })
expect do expect do
subject.perform(project.id) subject.perform(project.id)
end.to raise_error(RuntimeError, error) end.to raise_error(RuntimeError, error)
expect(project.reload.import_jid).not_to be_nil expect(import_state.reload.jid).not_to be_nil
end end
it 'updates the error on Import/Export' do it 'updates the error on Import/Export' do
error = %q{remote: Not Found fatal: repository 'https://user:pass@test.com/root/repoC.git/' not found } 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_project') project.update(import_type: 'gitlab_project')
import_state.update(jid: '123')
expect_any_instance_of(Projects::ImportService).to receive(:execute).and_return({ status: :error, message: error }) expect_any_instance_of(Projects::ImportService).to receive(:execute).and_return({ status: :error, message: error })
expect do expect do
subject.perform(project.id) subject.perform(project.id)
end.to raise_error(RuntimeError, error) end.to raise_error(RuntimeError, error)
expect(project.reload.import_error).not_to be_nil expect(import_state.reload.last_error).not_to be_nil
end end
end end
...@@ -90,8 +91,8 @@ describe RepositoryImportWorker do ...@@ -90,8 +91,8 @@ describe RepositoryImportWorker do
.to receive(:async?) .to receive(:async?)
.and_return(true) .and_return(true)
expect_any_instance_of(Project) expect_any_instance_of(ProjectImportState)
.not_to receive(:import_finish) .not_to receive(:finish)
subject.perform(project.id) subject.perform(project.id)
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment