Commit fede3a0b authored by Andreas Brandl's avatar Andreas Brandl

Flush InternalId records after import

After the import has finished, we flush (delete) the InternalId records
related to the project at hand. This means we're starting over with
tracking correct internal id values, a new record will be created
automatically when the next internal id is generated.

The GitHub importer assigns iid values by using supplied values from
GitHub. We skip tracking internal id values during the import in favor
of higher concurrency. Deleting any InternalId records after the import
has finished is an extra measure to guarantee consistency.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/54270.
parent 13f5f7e6
...@@ -66,6 +66,17 @@ class InternalId < ActiveRecord::Base ...@@ -66,6 +66,17 @@ class InternalId < ActiveRecord::Base
InternalIdGenerator.new(subject, scope, usage, init).generate InternalIdGenerator.new(subject, scope, usage, init).generate
end end
# Flushing records is generally safe in a sense that those
# records are going to be re-created when needed.
#
# A filter condition has to be provided to not accidentally flush
# records for all projects.
def flush_records!(filter)
raise ArgumentError, "filter cannot be empty" if filter.blank?
where(filter).delete_all
end
def available? def available?
@available_flag ||= ActiveRecord::Migrator.current_version >= REQUIRED_SCHEMA_VERSION # rubocop:disable Gitlab/PredicateMemoization @available_flag ||= ActiveRecord::Migrator.current_version >= REQUIRED_SCHEMA_VERSION # rubocop:disable Gitlab/PredicateMemoization
end end
......
...@@ -1585,6 +1585,13 @@ class Project < ActiveRecord::Base ...@@ -1585,6 +1585,13 @@ class Project < ActiveRecord::Base
def after_import def after_import
repository.after_import repository.after_import
wiki.repository.after_import wiki.repository.after_import
# The import assigns iid values on its own, e.g. by re-using GitHub ids.
# Flush existing InternalId records for this project for consistency reasons.
# Those records are going to be recreated with the next normal creation
# of a model instance (e.g. an Issue).
InternalId.flush_records!(project: self)
import_state.finish import_state.finish
import_state.remove_jid import_state.remove_jid
update_project_counter_caches update_project_counter_caches
......
...@@ -13,6 +13,29 @@ describe InternalId do ...@@ -13,6 +13,29 @@ describe InternalId do
it { is_expected.to validate_presence_of(:usage) } it { is_expected.to validate_presence_of(:usage) }
end end
describe '.flush_records!' do
subject { described_class.flush_records!(project: project) }
let(:another_project) { create(:project) }
before do
create_list(:issue, 2, project: project)
create_list(:issue, 2, project: another_project)
end
it 'deletes all records for the given project' do
expect { subject }.to change { described_class.where(project: project).count }.from(1).to(0)
end
it 'retains records for other projects' do
expect { subject }.not_to change { described_class.where(project: another_project).count }
end
it 'does not allow an empty filter' do
expect { described_class.flush_records!({}) }.to raise_error(/filter cannot be empty/)
end
end
describe '.generate_next' do describe '.generate_next' do
subject { described_class.generate_next(issue, scope, usage, init) } subject { described_class.generate_next(issue, scope, usage, init) }
......
...@@ -3767,6 +3767,7 @@ describe Project do ...@@ -3767,6 +3767,7 @@ describe Project do
expect(import_state).to receive(:remove_jid) expect(import_state).to receive(:remove_jid)
expect(project).to receive(:after_create_default_branch) expect(project).to receive(:after_create_default_branch)
expect(project).to receive(:refresh_markdown_cache!) expect(project).to receive(:refresh_markdown_cache!)
expect(InternalId).to receive(:flush_records!).with(project: project)
project.after_import project.after_import
end 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