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