diff options
48 files changed, 385 insertions, 416 deletions
diff --git a/app/controllers/projects/imports_controller.rb b/app/controllers/projects/imports_controller.rb index e55065c5817..641f45b3b56 100644 --- a/app/controllers/projects/imports_controller.rb +++ b/app/controllers/projects/imports_controller.rb @@ -13,10 +13,8 @@ class Projects::ImportsController < Projects::ApplicationController end def create - @project.import_url = params[:project][:import_url] - - if @project.save - @project.reload.import_schedule + if @project.update(safe_import_params) + @project.import_state.reload.schedule end redirect_to project_import_path(@project) @@ -24,7 +22,7 @@ class Projects::ImportsController < Projects::ApplicationController def show if @project.import_finished? - if continue_params + if continue_params && continue_params[:to] redirect_to continue_params[:to], notice: continue_params[:notice] else redirect_to project_path(@project), notice: finished_notice @@ -67,4 +65,12 @@ class Projects::ImportsController < Projects::ApplicationController redirect_to project_path(@project) end end + + def import_params + params.require(:project).permit(:import_url) + end + + def safe_import_params + import_params + end end diff --git a/app/models/project.rb b/app/models/project.rb index b85ec90f3ca..c305644c900 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -30,6 +30,7 @@ class Project < ActiveRecord::Base include FeatureGate include OptionallySearch include FromUnion + include IgnorableColumn extend Gitlab::Cache::RequestCache extend Gitlab::ConfigHelper @@ -55,6 +56,8 @@ class Project < ActiveRecord::Base VALID_MIRROR_PORTS = [22, 80, 443].freeze VALID_MIRROR_PROTOCOLS = %w(http https ssh git).freeze + ignore_column :import_status, :import_jid, :import_error + cache_markdown_field :description, pipeline: :description delegate :feature_available?, :builds_enabled?, :wiki_enabled?, @@ -63,6 +66,12 @@ class Project < ActiveRecord::Base delegate :base_dir, :disk_path, :ensure_storage_path_exists, to: :storage + delegate :scheduled?, :started?, :in_progress?, + :failed?, :finished?, + prefix: :import, to: :import_state, allow_nil: true + + delegate :no_import?, to: :import_state, allow_nil: true + default_value_for :archived, false default_value_for :visibility_level, gitlab_config_features.visibility_level default_value_for :resolve_outdated_diff_discussions, false @@ -454,8 +463,8 @@ class Project < ActiveRecord::Base scope :excluding_project, ->(project) { where.not(id: project) } - scope :joins_import_state, -> { joins("LEFT JOIN project_mirror_data import_state ON import_state.project_id = projects.id") } - scope :import_started, -> { joins_import_state.where("import_state.status = 'started' OR projects.import_status = 'started'") } + # We require an alias to the project_mirror_data_table in order to use import_state in our queries + scope :joins_import_state, -> { joins("INNER JOIN project_mirror_data import_state ON import_state.project_id = projects.id") } scope :for_group, -> (group) { where(group: group) } class << self @@ -631,6 +640,14 @@ class Project < ActiveRecord::Base id && persisted? end + def import_status + import_state&.status || 'none' + end + + def human_import_status_name + import_state&.human_status_name || 'none' + end + def add_import_job job_id = if forked? @@ -662,7 +679,7 @@ class Project < ActiveRecord::Base ProjectCacheWorker.perform_async(self.id) end - update(import_error: nil) + import_state.update(last_error: nil) remove_import_data end @@ -724,130 +741,6 @@ class Project < ActiveRecord::Base import_url.present? end - def imported? - import_finished? - end - - def import_in_progress? - import_started? || import_scheduled? - end - - def import_state_args - { - status: self[:import_status], - jid: self[:import_jid], - last_error: self[:import_error] - } - end - - def ensure_import_state(force: false) - return if !force && (self[:import_status] == 'none' || self[:import_status].nil?) - return unless import_state.nil? - - if persisted? - create_import_state(import_state_args) - - update_column(:import_status, 'none') - else - build_import_state(import_state_args) - - self[:import_status] = 'none' - end - end - - def human_import_status_name - ensure_import_state - - import_state.human_status_name - end - - def import_schedule - ensure_import_state(force: true) - - import_state.schedule - end - - def force_import_start - ensure_import_state(force: true) - - import_state.force_start - end - - def import_start - ensure_import_state(force: true) - - import_state.start - end - - def import_fail - ensure_import_state(force: true) - - import_state.fail_op - end - - def import_finish - ensure_import_state(force: true) - - import_state.finish - end - - def import_jid=(new_jid) - ensure_import_state(force: true) - - import_state.jid = new_jid - end - - def import_jid - ensure_import_state - - import_state&.jid - end - - def import_error=(new_error) - ensure_import_state(force: true) - - import_state.last_error = new_error - end - - def import_error - ensure_import_state - - import_state&.last_error - end - - def import_status=(new_status) - ensure_import_state(force: true) - - import_state.status = new_status - end - - def import_status - ensure_import_state - - import_state&.status || 'none' - end - - def no_import? - import_status == 'none' - end - - def import_started? - # import? does SQL work so only run it if it looks like there's an import running - import_status == 'started' && import? - end - - def import_scheduled? - import_status == 'scheduled' - end - - def import_failed? - import_status == 'failed' - end - - def import_finished? - import_status == 'finished' - end - def safe_import_url Gitlab::UrlSanitizer.new(import_url).masked_url end @@ -1646,8 +1539,8 @@ class Project < ActiveRecord::Base def after_import repository.after_import wiki.repository.after_import - import_finish - remove_import_jid + import_state.finish + import_state.remove_jid update_project_counter_caches after_create_default_branch refresh_markdown_cache! @@ -1687,32 +1580,11 @@ class Project < ActiveRecord::Base end # rubocop: enable CodeReuse/ServiceClass - def remove_import_jid - return unless import_jid - - Gitlab::SidekiqStatus.unset(import_jid) - - import_state.update_column(:jid, nil) - end - # Lazy loading of the `pipeline_status` attribute def pipeline_status @pipeline_status ||= Gitlab::Cache::Ci::ProjectPipelineStatus.load_for_project(self) end - def mark_import_as_failed(error_message) - original_errors = errors.dup - sanitized_message = Gitlab::UrlSanitizer.sanitize(error_message) - - import_fail - - import_state.update_column(:last_error, sanitized_message) - rescue ActiveRecord::ActiveRecordError => e - Rails.logger.error("Error setting import status to failed: #{e.message}. Original error: #{sanitized_message}") - ensure - @errors = original_errors - end - def add_export_job(current_user:, after_export_strategy: nil, params: {}) job_id = ProjectExportWorker.perform_async(current_user.id, self.id, after_export_strategy, params) @@ -1989,17 +1861,6 @@ class Project < ActiveRecord::Base Gitlab::ReferenceCounter.new(gl_repository(is_wiki: wiki)) end - # Refreshes the expiration time of the associated import job ID. - # - # This method can be used by asynchronous importers to refresh the status, - # preventing the StuckImportJobsWorker from marking the import as failed. - def refresh_import_jid_expiration - return unless import_jid - - Gitlab::SidekiqStatus - .set(import_jid, StuckImportJobsWorker::IMPORT_JOBS_EXPIRATION) - end - def badges return project_badges unless group diff --git a/app/models/project_import_state.rb b/app/models/project_import_state.rb index 7126bb66d80..c80e612053c 100644 --- a/app/models/project_import_state.rb +++ b/app/models/project_import_state.rb @@ -69,4 +69,33 @@ class ProjectImportState < ActiveRecord::Base ensure @errors = original_errors end + + alias_method :no_import?, :none? + + def in_progress? + started? || scheduled? + end + + def started? + # import? does SQL work so only run it if it looks like there's an import running + status == 'started' && project.import? + end + + def remove_jid + return unless jid + + Gitlab::SidekiqStatus.unset(jid) + + update_column(:jid, nil) + end + + # Refreshes the expiration time of the associated import job ID. + # + # This method can be used by asynchronous importers to refresh the status, + # preventing the StuckImportJobsWorker from marking the import as failed. + def refresh_jid_expiration + return unless jid + + Gitlab::SidekiqStatus.set(jid, StuckImportJobsWorker::IMPORT_JOBS_EXPIRATION) + end end diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index 20bfe5af7a1..481de34b977 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -148,7 +148,7 @@ module Projects Rails.logger.error(log_message) if @project - @project.mark_import_as_failed(message) if @project.persisted? && @project.import? + @project.import_state.mark_as_failed(message) if @project.persisted? && @project.import? end @project @@ -181,7 +181,7 @@ module Projects def import_schedule if @project.errors.empty? - @project.import_schedule if @project.import? && !@project.bare_repository_import? + @project.import_state.schedule if @project.import? && !@project.bare_repository_import? else fail(error: @project.errors.full_messages.join(', ')) end diff --git a/app/views/import/bitbucket/status.html.haml b/app/views/import/bitbucket/status.html.haml index 3b1b5e55302..2336e1e83f9 100644 --- a/app/views/import/bitbucket/status.html.haml +++ b/app/views/import/bitbucket/status.html.haml @@ -37,11 +37,12 @@ %td = link_to project.full_path, [project.namespace.becomes(Namespace), project] %td.job-status - - if project.import_status == 'finished' + - case project.import_status + - when 'finished' %span %i.fa.fa-check = _('done') - - elsif project.import_status == 'started' + - when 'started' %i.fa.fa-spinner.fa-spin = _('started') - else diff --git a/app/views/import/bitbucket_server/status.html.haml b/app/views/import/bitbucket_server/status.html.haml index 56d4f2ba881..ef69197e453 100644 --- a/app/views/import/bitbucket_server/status.html.haml +++ b/app/views/import/bitbucket_server/status.html.haml @@ -38,9 +38,10 @@ %td = link_to project.full_path, [project.namespace.becomes(Namespace), project] %td.job-status - - if project.import_status == 'finished' + - case project.import_status + - when 'finished' = icon('check', text: 'Done') - - elsif project.import_status == 'started' + - when 'started' = icon('spin', text: 'started') - else = project.human_import_status_name diff --git a/app/views/import/fogbugz/status.html.haml b/app/views/import/fogbugz/status.html.haml index 830d141ebea..eca67582d6f 100644 --- a/app/views/import/fogbugz/status.html.haml +++ b/app/views/import/fogbugz/status.html.haml @@ -34,11 +34,12 @@ %td = link_to project.full_path, [project.namespace.becomes(Namespace), project] %td.job-status - - if project.import_status == 'finished' + - case project.import_status + - when 'finished' %span %i.fa.fa-check = _("done") - - elsif project.import_status == 'started' + - when 'started' %i.fa.fa-spinner.fa-spin = _("started") - else diff --git a/app/views/import/gitlab/status.html.haml b/app/views/import/gitlab/status.html.haml index b7bfbae5edf..a5fa12fe7df 100644 --- a/app/views/import/gitlab/status.html.haml +++ b/app/views/import/gitlab/status.html.haml @@ -30,11 +30,12 @@ %td = link_to project.full_path, [project.namespace.becomes(Namespace), project] %td.job-status - - if project.import_status == 'finished' + - case project.import_status + - when 'finished' %span %i.fa.fa-check = _('done') - - elsif project.import_status == 'started' + - when 'started' %i.fa.fa-spinner.fa-spin = _('started') - else diff --git a/app/views/import/google_code/status.html.haml b/app/views/import/google_code/status.html.haml index 347e2820f94..f322b7a956a 100644 --- a/app/views/import/google_code/status.html.haml +++ b/app/views/import/google_code/status.html.haml @@ -39,11 +39,12 @@ %td = link_to project.full_path, [project.namespace.becomes(Namespace), project] %td.job-status - - if project.import_status == 'finished' + - case project.import_status + - when 'finished' %span %i.fa.fa-check = _("done") - - elsif project.import_status == 'started' + - when 'started' %i.fa.fa-spinner.fa-spin = _("started") - else diff --git a/app/views/projects/imports/new.html.haml b/app/views/projects/imports/new.html.haml index 1c50cfbde85..bd0ab2c19f2 100644 --- a/app/views/projects/imports/new.html.haml +++ b/app/views/projects/imports/new.html.haml @@ -10,7 +10,7 @@ .card-body %pre :preserve - #{h(@project.import_error)} + #{h(@project.import_state.last_error)} = form_for @project, url: project_import_path(@project), method: :post do |f| = render "shared/import_form", f: f diff --git a/app/workers/concerns/gitlab/github_import/stage_methods.rb b/app/workers/concerns/gitlab/github_import/stage_methods.rb index 59e6bc2c97d..e2dee315cde 100644 --- a/app/workers/concerns/gitlab/github_import/stage_methods.rb +++ b/app/workers/concerns/gitlab/github_import/stage_methods.rb @@ -24,7 +24,7 @@ module Gitlab def find_project(id) # If the project has been marked as failed we want to bail out # automatically. - Project.import_started.find_by(id: id) + Project.joins_import_state.where(import_state: { status: :started }).find_by(id: id) end # rubocop: enable CodeReuse/ActiveRecord end diff --git a/app/workers/concerns/project_import_options.rb b/app/workers/concerns/project_import_options.rb index 22bdf441d6b..2baf768bfd1 100644 --- a/app/workers/concerns/project_import_options.rb +++ b/app/workers/concerns/project_import_options.rb @@ -18,7 +18,7 @@ module ProjectImportOptions "import" end - project.mark_import_as_failed("Every #{action} attempt has failed: #{job['error_message']}. Please try again.") + project.import_state.mark_as_failed(_("Every %{action} attempt has failed: %{job_error_message}. Please try again.") % { action: action, job_error_message: job['error_message'] }) Sidekiq.logger.warn "Failed #{job['class']} with #{job['args']}: #{job['error_message']}" end end diff --git a/app/workers/concerns/project_start_import.rb b/app/workers/concerns/project_start_import.rb index 46a133db2a1..4462bc51a24 100644 --- a/app/workers/concerns/project_start_import.rb +++ b/app/workers/concerns/project_start_import.rb @@ -2,11 +2,11 @@ # Used in EE by mirroring module ProjectStartImport - def start(project) - if project.import_started? && project.import_jid == self.jid + def start(import_state) + if import_state.started? && import_state.jid == self.jid return true end - project.import_start + import_state.start end end diff --git a/app/workers/gitlab/github_import/advance_stage_worker.rb b/app/workers/gitlab/github_import/advance_stage_worker.rb index 2b49860025a..0b3437a8a33 100644 --- a/app/workers/gitlab/github_import/advance_stage_worker.rb +++ b/app/workers/gitlab/github_import/advance_stage_worker.rb @@ -31,7 +31,7 @@ module Gitlab # next_stage - The name of the next stage to start when all jobs have been # completed. def perform(project_id, waiters, next_stage) - return unless (project = find_project(project_id)) + return unless import_state = find_import_state(project_id) new_waiters = wait_for_jobs(waiters) @@ -41,7 +41,7 @@ module Gitlab # the pressure on Redis. We _only_ do this once all jobs are done so # we don't get stuck forever if one or more jobs failed to notify the # JobWaiter. - project.refresh_import_jid_expiration + import_state.refresh_jid_expiration STAGES.fetch(next_stage.to_sym).perform_async(project_id) else @@ -64,11 +64,8 @@ module Gitlab end # rubocop: disable CodeReuse/ActiveRecord - def find_project(id) - # TODO: Only select the JID - # This is due to the fact that the JID could be present in either the project record or - # its associated import_state record - Project.import_started.find_by(id: id) + def find_import_state(project_id) + ProjectImportState.select(:jid).with_status(:started).find_by(project_id: project_id) end # rubocop: enable CodeReuse/ActiveRecord end diff --git a/app/workers/gitlab/github_import/refresh_import_jid_worker.rb b/app/workers/gitlab/github_import/refresh_import_jid_worker.rb index 65473026b4c..76723e4a61f 100644 --- a/app/workers/gitlab/github_import/refresh_import_jid_worker.rb +++ b/app/workers/gitlab/github_import/refresh_import_jid_worker.rb @@ -16,12 +16,13 @@ module Gitlab # project_id - The ID of the project that is being imported. # check_job_id - The ID of the job for which to check the status. def perform(project_id, check_job_id) - return unless (project = find_project(project_id)) + import_state = find_import_state(project_id) + return unless import_state if SidekiqStatus.running?(check_job_id) # As long as the repository is being cloned we want to keep refreshing # the import JID status. - project.refresh_import_jid_expiration + import_state.refresh_jid_expiration self.class.perform_in_the_future(project_id, check_job_id) end @@ -31,11 +32,10 @@ module Gitlab end # rubocop: disable CodeReuse/ActiveRecord - def find_project(id) - # TODO: Only select the JID - # This is due to the fact that the JID could be present in either the project record or - # its associated import_state record - Project.import_started.find_by(id: id) + def find_import_state(project_id) + ProjectImportState.select(:jid) + .with_status(:started) + .find_by(project_id: project_id) end # rubocop: enable CodeReuse/ActiveRecord end diff --git a/app/workers/gitlab/github_import/stage/import_base_data_worker.rb b/app/workers/gitlab/github_import/stage/import_base_data_worker.rb index 5726fbb573d..ccfed2ae187 100644 --- a/app/workers/gitlab/github_import/stage/import_base_data_worker.rb +++ b/app/workers/gitlab/github_import/stage/import_base_data_worker.rb @@ -23,7 +23,7 @@ module Gitlab klass.new(project, client).execute end - project.refresh_import_jid_expiration + project.import_state.refresh_jid_expiration ImportPullRequestsWorker.perform_async(project.id) end diff --git a/app/workers/gitlab/github_import/stage/import_pull_requests_worker.rb b/app/workers/gitlab/github_import/stage/import_pull_requests_worker.rb index 1c5a7139802..37a7a7f4ba0 100644 --- a/app/workers/gitlab/github_import/stage/import_pull_requests_worker.rb +++ b/app/workers/gitlab/github_import/stage/import_pull_requests_worker.rb @@ -15,7 +15,7 @@ module Gitlab .new(project, client) .execute - project.refresh_import_jid_expiration + project.import_state.refresh_jid_expiration AdvanceStageWorker.perform_async( project.id, diff --git a/app/workers/repository_fork_worker.rb b/app/workers/repository_fork_worker.rb index 68ec66e8499..7eae07d3f6b 100644 --- a/app/workers/repository_fork_worker.rb +++ b/app/workers/repository_fork_worker.rb @@ -12,7 +12,7 @@ class RepositoryForkWorker source_project = target_project.forked_from_project unless source_project - return target_project.mark_import_as_failed('Source project cannot be found.') + return target_project.import_state.mark_as_failed(_('Source project cannot be found.')) end fork_repository(target_project, source_project.repository_storage, source_project.disk_path) @@ -33,7 +33,7 @@ class RepositoryForkWorker end def start_fork(project) - return true if start(project) + return true if start(project.import_state) Rails.logger.info("Project #{project.full_path} was in inconsistent state (#{project.import_status}) while forking.") false diff --git a/app/workers/repository_import_worker.rb b/app/workers/repository_import_worker.rb index 82189a3c9f5..59691f48a39 100644 --- a/app/workers/repository_import_worker.rb +++ b/app/workers/repository_import_worker.rb @@ -34,14 +34,14 @@ class RepositoryImportWorker attr_reader :project def start_import - return true if start(project) + return true if start(project.import_state) Rails.logger.info("Project #{project.full_path} was in inconsistent state (#{project.import_status}) while importing.") false end def fail_import(message) - project.mark_import_as_failed(message) + project.import_state.mark_as_failed(message) end def template_import? diff --git a/app/workers/stuck_import_jobs_worker.rb b/app/workers/stuck_import_jobs_worker.rb index 667a4121131..c8a186ba4ce 100644 --- a/app/workers/stuck_import_jobs_worker.rb +++ b/app/workers/stuck_import_jobs_worker.rb @@ -63,6 +63,6 @@ class StuckImportJobsWorker # rubocop: enable CodeReuse/ActiveRecord def error_message - "Import timed out. Import took longer than #{IMPORT_JOBS_EXPIRATION} seconds" + _("Import timed out. Import took longer than %{import_jobs_expiration} seconds") % { import_jobs_expiration: IMPORT_JOBS_EXPIRATION } end end diff --git a/doc/development/github_importer.md b/doc/development/github_importer.md index e860bde48dc..863ac049db6 100644 --- a/doc/development/github_importer.md +++ b/doc/development/github_importer.md @@ -131,7 +131,7 @@ our import as failed because of this. 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 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 long we're still performing work. diff --git a/lib/api/entities.rb b/lib/api/entities.rb index cff05643f3b..4788b0e16a1 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -145,7 +145,9 @@ module API expose :import_status # 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 class BasicProjectDetails < ProjectIdentity @@ -248,7 +250,10 @@ module API expose :creator_id expose :forked_from_project, using: Entities::BasicProjectDetails, if: lambda { |project, options| project.forked? } 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 :runners_token, if: lambda { |_project, options| options[:user_can_admin_project] } diff --git a/lib/gitlab/bitbucket_import/importer.rb b/lib/gitlab/bitbucket_import/importer.rb index 45e550b3450..eaead41a720 100644 --- a/lib/gitlab/bitbucket_import/importer.rb +++ b/lib/gitlab/bitbucket_import/importer.rb @@ -35,7 +35,7 @@ module Gitlab def handle_errors 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.', errors: errors }.to_json) diff --git a/lib/gitlab/bitbucket_server_import/importer.rb b/lib/gitlab/bitbucket_server_import/importer.rb index 15aa4739ee9..d4080536d81 100644 --- a/lib/gitlab/bitbucket_server_import/importer.rb +++ b/lib/gitlab/bitbucket_server_import/importer.rb @@ -56,7 +56,7 @@ module Gitlab def handle_errors 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.', errors: errors }.to_json) diff --git a/lib/gitlab/github_import/importer/repository_importer.rb b/lib/gitlab/github_import/importer/repository_importer.rb index 374dc9d3c00..bc3ea9e9226 100644 --- a/lib/gitlab/github_import/importer/repository_importer.rb +++ b/lib/gitlab/github_import/importer/repository_importer.rb @@ -80,7 +80,7 @@ module Gitlab end def fail_import(message) - project.mark_import_as_failed(message) + project.import_state.mark_as_failed(message) false end end diff --git a/lib/gitlab/github_import/parallel_importer.rb b/lib/gitlab/github_import/parallel_importer.rb index a77ac1e4fa6..9d81441d96e 100644 --- a/lib/gitlab/github_import/parallel_importer.rb +++ b/lib/gitlab/github_import/parallel_importer.rb @@ -41,8 +41,7 @@ module Gitlab Gitlab::SidekiqStatus .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 .perform_async(project.id) diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index b40eac3de9a..8380e86c128 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -98,13 +98,11 @@ excluded_attributes: - :avatar - :import_type - :import_source - - :import_error - :mirror - :runners_token - :repository_storage - :repository_read_only - :lfs_enabled - - :import_jid - :created_at - :updated_at - :id @@ -116,6 +114,9 @@ excluded_attributes: - :remote_mirror_available_overridden - :description_html - :repository_languages + project_import_state: + - :last_error + - :jid prometheus_metrics: - :common - :identifier diff --git a/lib/gitlab/legacy_github_import/importer.rb b/lib/gitlab/legacy_github_import/importer.rb index 43695451b87..c526d31a591 100644 --- a/lib/gitlab/legacy_github_import/importer.rb +++ b/lib/gitlab/legacy_github_import/importer.rb @@ -80,8 +80,7 @@ module Gitlab def handle_errors 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.', errors: errors }.to_json) diff --git a/lib/tasks/import.rake b/lib/tasks/import.rake index a16d4c47273..f912f521dfb 100644 --- a/lib/tasks/import.rake +++ b/lib/tasks/import.rake @@ -42,7 +42,7 @@ class GithubImport end def import! - @project.force_import_start + @project.import_state.force_start import_success = false @@ -57,7 +57,7 @@ class GithubImport puts "Import finished. Timings: #{timings}".color(:green) else puts "Import was not successful. Errors were as follows:" - puts @project.import_error + puts @project.import_state.last_error end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index f76b61a2258..835dc8f916b 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2822,6 +2822,9 @@ msgstr "" msgid "EventFilterBy|Filter by team" msgstr "" +msgid "Every %{action} attempt has failed: %{job_error_message}. Please try again." +msgstr "" + msgid "Every day (at 4:00am)" msgstr "" @@ -3475,6 +3478,9 @@ msgstr "" msgid "Import repository" msgstr "" +msgid "Import timed out. Import took longer than %{import_jobs_expiration} seconds" +msgstr "" + msgid "In order to enable instance-level analytics, please ask an admin to enable %{usage_ping_link_start}usage ping%{usage_ping_link_end}." msgstr "" @@ -6018,6 +6024,9 @@ msgstr "" msgid "Source is not available" msgstr "" +msgid "Source project cannot be found." +msgstr "" + msgid "Spam Logs" msgstr "" diff --git a/spec/controllers/import/bitbucket_server_controller_spec.rb b/spec/controllers/import/bitbucket_server_controller_spec.rb index 77060fdc3be..db912641894 100644 --- a/spec/controllers/import/bitbucket_server_controller_spec.rb +++ b/spec/controllers/import/bitbucket_server_controller_spec.rb @@ -126,7 +126,7 @@ describe Import::BitbucketServerController do end 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) expect(repos).to receive(:partition).and_return([[@repo, @created_repo], [@invalid_repo]]) diff --git a/spec/controllers/projects/imports_controller_spec.rb b/spec/controllers/projects/imports_controller_spec.rb index adf3c78ae51..cdc63f5aab3 100644 --- a/spec/controllers/projects/imports_controller_spec.rb +++ b/spec/controllers/projects/imports_controller_spec.rb @@ -26,10 +26,11 @@ describe Projects::ImportsController do context 'when repository exists' do 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 before do - project.update(import_status: :started) + import_state.update(status: :started) end it 'renders template' do @@ -47,7 +48,7 @@ describe Projects::ImportsController do context 'when import failed' do before do - project.update(import_status: :failed) + import_state.update(status: :failed) end it 'redirects to new_namespace_project_import_path' do @@ -59,7 +60,7 @@ describe Projects::ImportsController do context 'when import finished' do before do - project.update(import_status: :finished) + import_state.update(status: :finished) end context 'when project is a fork' do @@ -108,7 +109,7 @@ describe Projects::ImportsController do context 'when import never happened' do before do - project.update(import_status: :none) + import_state.update(status: :none) end it 'redirects to namespace_project_path' do diff --git a/spec/factories/import_state.rb b/spec/factories/import_state.rb index 15d0a9d466a..d6de26dccbc 100644 --- a/spec/factories/import_state.rb +++ b/spec/factories/import_state.rb @@ -5,6 +5,7 @@ FactoryBot.define do transient do import_url { generate(:url) } + import_type nil end trait :repository do @@ -32,7 +33,11 @@ FactoryBot.define do end 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 diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index e4823a5adf1..1906c06a211 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -30,6 +30,8 @@ FactoryBot.define do # we can't assign the delegated `#ci_cd_settings` attributes directly, as the # `#ci_cd_settings` relation needs to be created first group_runners_enabled nil + import_status nil + import_jid nil end after(:create) do |project, evaluator| @@ -64,6 +66,13 @@ FactoryBot.define do # assign the delegated `#ci_cd_settings` attributes after create 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.jid = evaluator.import_jid + import_state.save + end end trait :public do diff --git a/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb b/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb index d8f01dcb76b..77f5b2ffa37 100644 --- a/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb @@ -218,7 +218,7 @@ describe Gitlab::GithubImport::Importer::RepositoryImporter do describe '#fail_import' 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) end diff --git a/spec/lib/gitlab/github_import/parallel_importer_spec.rb b/spec/lib/gitlab/github_import/parallel_importer_spec.rb index 20b48c1de68..f5df38c9aaf 100644 --- a/spec/lib/gitlab/github_import/parallel_importer_spec.rb +++ b/spec/lib/gitlab/github_import/parallel_importer_spec.rb @@ -36,7 +36,7 @@ describe Gitlab::GithubImport::ParallelImporter do it 'updates the import JID of the project' do 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 diff --git a/spec/lib/gitlab/legacy_github_import/importer_spec.rb b/spec/lib/gitlab/legacy_github_import/importer_spec.rb index 20514486727..d2df21d7bb5 100644 --- a/spec/lib/gitlab/legacy_github_import/importer_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/importer_spec.rb @@ -174,7 +174,7 @@ describe Gitlab::LegacyGithubImport::Importer do 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 diff --git a/spec/models/project_import_state_spec.rb b/spec/models/project_import_state_spec.rb index f7033b28c76..e3b2d971419 100644 --- a/spec/models/project_import_state_spec.rb +++ b/spec/models/project_import_state_spec.rb @@ -10,4 +10,116 @@ describe ProjectImportState, type: :model do describe 'validations' do it { is_expected.to validate_presence_of(:project) } 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 diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index d23bdbc2c30..6d42b4a72ae 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1702,6 +1702,16 @@ describe Project do end end + describe 'handling import URL' do + it 'returns the sanitized URL' do + project = create(:project, :import_started, import_url: 'http://user:pass@test.com') + + project.import_state.finish + + expect(project.reload.import_url).to eq('http://test.com') + end + end + describe '#container_registry_url' do let(:project) { create(:project) } @@ -1815,106 +1825,6 @@ describe Project do 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_any_instance_of(RepositoryImportWorker).to receive(:perform).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 def create_pipeline(status = 'success') create(:ci_pipeline, project: project, @@ -1994,6 +1904,42 @@ describe Project do 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 let(:import_jid) { '123' } @@ -3430,13 +3376,14 @@ describe Project do describe '#after_import' do let(:project) { create(:project) } + let(:import_state) { create(:import_state, project: project) } it 'runs the correct hooks' do expect(project.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(:remove_import_jid) + expect(import_state).to receive(:remove_jid) expect(project).to receive(:after_create_default_branch) expect(project).to receive(:refresh_markdown_cache!) @@ -3446,6 +3393,10 @@ describe Project do context 'branch protection' do let(:project) { create(:project, :repository) } + before do + create(:import_state, :started, project: project) + end + it 'does not protect when branch protection is disabled' do stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE) @@ -3501,37 +3452,6 @@ describe Project do 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 it 'returns true when the wiki repository exists' do project = create(:project, :wiki_repo) diff --git a/spec/requests/api/project_import_spec.rb b/spec/requests/api/project_import_spec.rb index c8fa4754810..204702b8a5a 100644 --- a/spec/requests/api/project_import_spec.rb +++ b/spec/requests/api/project_import_spec.rb @@ -42,7 +42,7 @@ describe API::ProjectImport do end 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) post api('/projects/import', user), namespace: 'nonexistent', path: 'test-import2', file: fixture_file_upload(file) @@ -52,7 +52,7 @@ describe API::ProjectImport do end 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)), path: 'test-import3', @@ -64,7 +64,7 @@ describe API::ProjectImport do end 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' @@ -119,7 +119,7 @@ describe API::ProjectImport do let(:existing_project) { create(:project, namespace: user.namespace) } 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) @@ -139,7 +139,7 @@ describe API::ProjectImport do end 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 end end diff --git a/spec/services/projects/create_from_template_service_spec.rb b/spec/services/projects/create_from_template_service_spec.rb index 141ccf7c4d8..da078dd36c6 100644 --- a/spec/services/projects/create_from_template_service_spec.rb +++ b/spec/services/projects/create_from_template_service_spec.rb @@ -47,7 +47,7 @@ describe Projects::CreateFromTemplateService do end it 'is not scheduled' do - expect(project.import_scheduled?).to be(false) + expect(project.import_scheduled?).to be_nil end it 'repository is empty' do diff --git a/spec/workers/concerns/gitlab/github_import/stage_methods_spec.rb b/spec/workers/concerns/gitlab/github_import/stage_methods_spec.rb index 241e8a2b6d3..d85a87f2cb0 100644 --- a/spec/workers/concerns/gitlab/github_import/stage_methods_spec.rb +++ b/spec/workers/concerns/gitlab/github_import/stage_methods_spec.rb @@ -58,14 +58,16 @@ describe Gitlab::GithubImport::StageMethods do end describe '#find_project' do + let(:import_state) { create(:import_state, project: project) } + 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) end 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 end diff --git a/spec/workers/concerns/project_import_options_spec.rb b/spec/workers/concerns/project_import_options_spec.rb index b6c111df8b9..3699fd83a9a 100644 --- a/spec/workers/concerns/project_import_options_spec.rb +++ b/spec/workers/concerns/project_import_options_spec.rb @@ -28,13 +28,23 @@ describe ProjectImportOptions do 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 it 'logs the appropriate error message for forked projects' do 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 diff --git a/spec/workers/gitlab/github_import/advance_stage_worker_spec.rb b/spec/workers/gitlab/github_import/advance_stage_worker_spec.rb index 0f78c5cc644..fc7aafbc0c9 100644 --- a/spec/workers/gitlab/github_import/advance_stage_worker_spec.rb +++ b/spec/workers/gitlab/github_import/advance_stage_worker_spec.rb @@ -17,8 +17,8 @@ describe Gitlab::GithubImport::AdvanceStageWorker, :clean_gitlab_redis_shared_st context 'when there are remaining jobs' do before do allow(worker) - .to receive(:find_project) - .and_return(project) + .to receive(:find_import_state) + .and_return(import_state) end it 'reschedules itself' do @@ -38,8 +38,8 @@ describe Gitlab::GithubImport::AdvanceStageWorker, :clean_gitlab_redis_shared_st context 'when there are no remaining jobs' do before do allow(worker) - .to receive(:find_project) - .and_return(project) + .to receive(:find_import_state) + .and_return(import_state) allow(worker) .to receive(:wait_for_jobs) @@ -48,8 +48,8 @@ describe Gitlab::GithubImport::AdvanceStageWorker, :clean_gitlab_redis_shared_st end it 'schedules the next stage' do - expect(project) - .to receive(:refresh_import_jid_expiration) + expect(import_state) + .to receive(:refresh_jid_expiration) expect(Gitlab::GithubImport::Stage::FinishImportWorker) .to receive(:perform_async) @@ -96,22 +96,18 @@ describe Gitlab::GithubImport::AdvanceStageWorker, :clean_gitlab_redis_shared_st end end - describe '#find_project' do - it 'returns a Project' do - project.update_column(:import_status, 'started') + describe '#find_import_state' do + it 'returns a ProjectImportState' do + 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) - - # 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' }) + expect(found).to be_an_instance_of(ProjectImportState) + expect(found.attributes.keys).to match_array(%w(id jid)) end 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 diff --git a/spec/workers/gitlab/github_import/refresh_import_jid_worker_spec.rb b/spec/workers/gitlab/github_import/refresh_import_jid_worker_spec.rb index 25ada575a44..7ff133f1049 100644 --- a/spec/workers/gitlab/github_import/refresh_import_jid_worker_spec.rb +++ b/spec/workers/gitlab/github_import/refresh_import_jid_worker_spec.rb @@ -29,7 +29,7 @@ describe Gitlab::GithubImport::RefreshImportJidWorker do context 'when the job is running' do it 'refreshes the import JID and reschedules itself' do allow(worker) - .to receive(:find_project) + .to receive(:find_import_state) .with(project.id) .and_return(project) @@ -39,7 +39,7 @@ describe Gitlab::GithubImport::RefreshImportJidWorker do .and_return(true) expect(project) - .to receive(:refresh_import_jid_expiration) + .to receive(:refresh_jid_expiration) expect(worker.class) .to receive(:perform_in_the_future) @@ -52,7 +52,7 @@ describe Gitlab::GithubImport::RefreshImportJidWorker do context 'when the job is no longer running' do it 'returns' do allow(worker) - .to receive(:find_project) + .to receive(:find_import_state) .with(project.id) .and_return(project) @@ -62,18 +62,18 @@ describe Gitlab::GithubImport::RefreshImportJidWorker do .and_return(false) expect(project) - .not_to receive(:refresh_import_jid_expiration) + .not_to receive(:refresh_jid_expiration) worker.perform(project.id, '123') end end end - describe '#find_project' do - it 'returns a Project' do + describe '#find_import_state' do + it 'returns a ProjectImportState' do 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 # it 'only selects the import JID field' do @@ -84,14 +84,14 @@ describe Gitlab::GithubImport::RefreshImportJidWorker do # .to eq({ 'id' => nil, 'import_jid' => '123abc' }) # 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) - expect(worker.find_project(project.id)).to be_nil + expect(worker.find_import_state(project.id)).to be_nil end - it 'returns nil for a non-existing project' do - expect(worker.find_project(-1)).to be_nil + it 'returns nil for a non-existing find_import_state' do + expect(worker.find_import_state(-1)).to be_nil end end end diff --git a/spec/workers/gitlab/github_import/stage/import_base_data_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_base_data_worker_spec.rb index 8c80d660287..ad6154cc4a4 100644 --- a/spec/workers/gitlab/github_import/stage/import_base_data_worker_spec.rb +++ b/spec/workers/gitlab/github_import/stage/import_base_data_worker_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe Gitlab::GithubImport::Stage::ImportBaseDataWorker do let(:project) { create(:project) } + let(:import_state) { create(:import_state, project: project) } let(:worker) { described_class.new } describe '#import' do @@ -18,7 +19,7 @@ describe Gitlab::GithubImport::Stage::ImportBaseDataWorker do expect(importer).to receive(:execute) end - expect(project).to receive(:refresh_import_jid_expiration) + expect(import_state).to receive(:refresh_jid_expiration) expect(Gitlab::GithubImport::Stage::ImportPullRequestsWorker) .to receive(:perform_async) diff --git a/spec/workers/gitlab/github_import/stage/import_pull_requests_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_pull_requests_worker_spec.rb index 2fc91a3e80a..1fbb073a34a 100644 --- a/spec/workers/gitlab/github_import/stage/import_pull_requests_worker_spec.rb +++ b/spec/workers/gitlab/github_import/stage/import_pull_requests_worker_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe Gitlab::GithubImport::Stage::ImportPullRequestsWorker do let(:project) { create(:project) } + let(:import_state) { create(:import_state, project: project) } let(:worker) { described_class.new } describe '#import' do @@ -19,8 +20,8 @@ describe Gitlab::GithubImport::Stage::ImportPullRequestsWorker do .to receive(:execute) .and_return(waiter) - expect(project) - .to receive(:refresh_import_jid_expiration) + expect(import_state) + .to receive(:refresh_jid_expiration) expect(Gitlab::GithubImport::AdvanceStageWorker) .to receive(:perform_async) diff --git a/spec/workers/repository_import_worker_spec.rb b/spec/workers/repository_import_worker_spec.rb index d07e40377d4..87ac4bc05c1 100644 --- a/spec/workers/repository_import_worker_spec.rb +++ b/spec/workers/repository_import_worker_spec.rb @@ -9,13 +9,13 @@ describe RepositoryImportWorker do describe '#perform' do let(:project) { create(:project, :import_scheduled) } + let(:import_state) { project.import_state } context 'when worker was reset without cleanup' do it 'imports the project successfully' do jid = '12345678' started_project = create(:project) - - create(:import_state, :started, project: started_project, jid: jid) + started_import_state = create(:import_state, :started, project: started_project, jid: jid) allow(subject).to receive(:jid).and_return(jid) @@ -23,12 +23,12 @@ describe RepositoryImportWorker do .and_return({ status: :ok }) # Works around https://github.com/rspec/rspec-mocks/issues/910 - expect(Project).to receive(:find).with(project.id).and_return(project) - expect(project.repository).to receive(:expire_emptiness_caches) - expect(project.wiki.repository).to receive(:expire_emptiness_caches) - expect(project).to receive(:import_finish) + expect(Project).to receive(:find).with(started_project.id).and_return(started_project) + expect(started_project.repository).to receive(:expire_emptiness_caches) + expect(started_project.wiki.repository).to receive(:expire_emptiness_caches) + expect(started_import_state).to receive(:finish) - subject.perform(project.id) + subject.perform(started_project.id) end end @@ -41,7 +41,7 @@ describe RepositoryImportWorker do expect(Project).to receive(:find).with(project.id).and_return(project) expect(project.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) end @@ -51,26 +51,27 @@ describe RepositoryImportWorker 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 } - 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 do subject.perform(project.id) 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 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 } - 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 do subject.perform(project.id) 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 @@ -90,8 +91,8 @@ describe RepositoryImportWorker do .to receive(:async?) .and_return(true) - expect_any_instance_of(Project) - .not_to receive(:import_finish) + expect_any_instance_of(ProjectImportState) + .not_to receive(:finish) subject.perform(project.id) end |