Commit 5e3076f2 authored by Douwe Maan's avatar Douwe Maan

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

Removes all the irrelevant code and columns that were migrated from the Project…

See merge request gitlab-org/gitlab-ce!21497
parents 57d98273 bad26e2d
......@@ -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&.key?(: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
......@@ -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
......@@ -451,8 +460,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
......@@ -628,6 +637,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?
......@@ -659,7 +676,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
......@@ -721,130 +738,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
......@@ -1643,8 +1536,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!
......@@ -1684,32 +1577,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)
......@@ -1986,17 +1858,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
......
......@@ -69,4 +69,33 @@ class ProjectImportState < ActiveRecord::Base
ensure
@errors = original_errors
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
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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,
......
......@@ -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
......
......@@ -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?
......
......@@ -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
---
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
......@@ -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.
......
......@@ -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] }
......
......@@ -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)
......
......@@ -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)
......
......@@ -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
......
......@@ -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)
......
......@@ -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
......
......@@ -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)
......
......@@ -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
......
......@@ -2834,6 +2834,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 ""
......@@ -3487,6 +3490,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 ""
......@@ -6039,6 +6045,9 @@ msgstr ""
msgid "Source is not available"
msgstr ""
msgid "Source project cannot be found."
msgstr ""
msgid "Spam Logs"
msgstr ""
......
......@@ -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]])
......
......@@ -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
......
......@@ -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
......@@ -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
......
......@@ -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
......
......@@ -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
......@@ -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
......
......@@ -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
......@@ -1686,6 +1686,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) }
......@@ -1799,106 +1809,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,
......@@ -1978,6 +1888,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' }
......@@ -3414,13 +3360,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!)
......@@ -3430,6 +3377,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)
......@@ -3485,37 +3436,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)
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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
......@@ -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
......@@ -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
......@@ -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)
......
......@@ -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)
......
......@@ -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
......
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