Commit 1b317374 authored by Tiago Botelho's avatar Tiago Botelho
Browse files

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
parent 2c51eecfe5be
......@@ -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 @@ class ProjectImportStatus < ProjectIdentity
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 @@ class Project < BasicProjectDetails
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 @@ def execute
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 @@ def execute
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 @@ def update_clone_time
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 @@ def execute
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 @@ def credentials
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
......
......@@ -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 ""
......
......@@ -126,7 +126,7 @@ def assign_session_tokens
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 @@
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 @@
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 @@
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 @@
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 @@
transient do
import_url { generate(:url) }
import_type nil
end
trait :repository do
......@@ -32,7 +33,11 @@
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 @@
# 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 @@
# 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 '#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 @@
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 @@
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 '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
......@@ -1702,6 +1702,16 @@ def create_pipeline
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 @@ def create_pipeline
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 @@ def create_build(new_pipeline = pipeline, name = 'test')
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 @@ def domain_variable
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 @@ def domain_variable
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 @@ def domain_variable
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