Skip to content
Snippets Groups Projects
Commit 32d53f08 authored by Bojan Marjanovic's avatar Bojan Marjanovic :five:
Browse files

Add internal_id allocation for GitHub Import

Introduces iid allocation to prevent unique constraint violation
and possible import failure

Changelog: fixed
parent 0da7a380
No related branches found
No related tags found
1 merge request!100670Resolve duplicate key value violates on GitHubImport
......@@ -174,6 +174,13 @@ def define_instance_internal_id_methods(scope, column, init)
#
# bulk_insert(attributes)
# end
#
# - track_#{scope}_#{column}!
# This method can be used to set a new greatest IID value during import operations.
#
# Example:
#
# MyClass.track_project_iid!(project, value)
def define_singleton_internal_id_methods(scope, column, init)
define_singleton_method("with_#{scope}_#{column}_supply") do |scope_value, &block|
subject = find_by(scope => scope_value) || self
......@@ -183,6 +190,16 @@ def define_singleton_internal_id_methods(scope, column, init)
supply = Supply.new(-> { InternalId.generate_next(subject, scope_attrs, usage, init) })
block.call(supply)
end
define_singleton_method("track_#{scope}_#{column}!") do |scope_value, value|
InternalId.track_greatest(
self,
::AtomicInternalId.scope_attrs(scope_value),
::AtomicInternalId.scope_usage(self),
value,
init
)
end
end
end
......
......@@ -26,6 +26,11 @@ def import(client, project)
RefreshImportJidWorker.perform_in_the_future(project.id, jid)
info(project.id, message: "starting importer", importer: 'Importer::RepositoryImporter')
# If a user creates an issue while the import is in progress, this can lead to an import failure.
# The workaround is to allocate IIDs before starting the importer.
allocate_issues_internal_id!(project, client)
importer = Importer::RepositoryImporter.new(project, client)
importer.execute
......@@ -56,6 +61,19 @@ def counter
def abort_on_failure
true
end
private
def allocate_issues_internal_id!(project, client)
return if InternalId.exists?(project: project, usage: :issues) # rubocop: disable CodeReuse/ActiveRecord
options = { state: 'all', sort: 'number', direction: 'desc', per_page: '1' }
last_github_issue = client.each_object(:issues, project.import_source, options).first
return unless last_github_issue
Issue.track_project_iid!(project, last_github_issue[:number])
end
end
end
end
......
......@@ -3,10 +3,11 @@
require 'spec_helper'
RSpec.describe AtomicInternalId do
let(:milestone) { build(:milestone) }
let_it_be(:project) { create(:project) }
let(:milestone) { build(:milestone, project: project) }
let(:iid) { double('iid', to_i: 42) }
let(:external_iid) { 100 }
let(:scope_attrs) { { project: milestone.project } }
let(:scope_attrs) { { project: project } }
let(:usage) { :milestones }
describe '#save!' do
......@@ -248,4 +249,12 @@ def self.name
end.to change { InternalId.find_by(project: milestone.project, usage: :milestones)&.last_value.to_i }.by(4)
end
end
describe '.track_project_iid!' do
it 'tracks the present value' do
expect do
::Issue.track_project_iid!(milestone.project, external_iid)
end.to change { InternalId.find_by(project: milestone.project, usage: :issues)&.last_value.to_i }.to(external_iid)
end
end
end
......@@ -19,18 +19,69 @@
end
context 'when the import succeeds' do
it 'schedules the importing of the base data' do
client = double(:client)
context 'with issues' do
it 'schedules the importing of the base data' do
client = double(:client)
options = { state: 'all', sort: 'number', direction: 'desc', per_page: '1' }
expect_next_instance_of(Gitlab::GithubImport::Importer::RepositoryImporter) do |instance|
expect(instance).to receive(:execute).and_return(true)
expect_next_instance_of(Gitlab::GithubImport::Importer::RepositoryImporter) do |instance|
expect(instance).to receive(:execute).and_return(true)
end
expect(InternalId).to receive(:exists?).and_return(false)
expect(client).to receive(:each_object).with(
:issues, project.import_source, options
).and_return([{ number: 5 }].each)
expect(Issue).to receive(:track_project_iid!).with(project, 5)
expect(Gitlab::GithubImport::Stage::ImportBaseDataWorker)
.to receive(:perform_async)
.with(project.id)
worker.import(client, project)
end
end
expect(Gitlab::GithubImport::Stage::ImportBaseDataWorker)
.to receive(:perform_async)
.with(project.id)
context 'without issues' do
it 'schedules the importing of the base data' do
client = double(:client)
options = { state: 'all', sort: 'number', direction: 'desc', per_page: '1' }
expect_next_instance_of(Gitlab::GithubImport::Importer::RepositoryImporter) do |instance|
expect(instance).to receive(:execute).and_return(true)
end
expect(InternalId).to receive(:exists?).and_return(false)
expect(client).to receive(:each_object).with(:issues, project.import_source, options).and_return([nil].each)
expect(Issue).not_to receive(:track_project_iid!)
worker.import(client, project)
expect(Gitlab::GithubImport::Stage::ImportBaseDataWorker)
.to receive(:perform_async)
.with(project.id)
worker.import(client, project)
end
end
context 'when retrying' do
it 'does not allocate internal ids' do
client = double(:client)
expect_next_instance_of(Gitlab::GithubImport::Importer::RepositoryImporter) do |instance|
expect(instance).to receive(:execute).and_return(true)
end
expect(InternalId).to receive(:exists?).and_return(true)
expect(client).not_to receive(:each_object)
expect(Issue).not_to receive(:track_project_iid!)
expect(Gitlab::GithubImport::Stage::ImportBaseDataWorker)
.to receive(:perform_async)
.with(project.id)
worker.import(client, project)
end
end
end
......@@ -43,6 +94,10 @@
expect(instance).to receive(:execute).and_raise(exception_class)
end
expect(InternalId).to receive(:exists?).and_return(false)
expect(client).to receive(:each_object).and_return([nil].each)
expect(Issue).not_to receive(:track_project_iid!)
expect(Gitlab::Import::ImportFailureService).to receive(:track)
.with(
project_id: project.id,
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment