Merge branch 'ab-49754-gh-importer-internal-ids' into 'master'

GitHub importer: Keep track of internal_ids

Closes #49754

See merge request gitlab-org/gitlab-ce!20926

5 files changed, 176 insertions, 1 deletions

diff --git a/spec/lib/gitlab/github_import/bulk_importing_spec.rb b/spec/lib/gitlab/github_import/bulk_importing_spec.rb
index 91229d9c7d4..861710f7e9b 100644
--- a/spec/lib/gitlab/github_import/bulk_importing_spec.rb
+++ b/spec/lib/gitlab/github_import/bulk_importing_spec.rb
@@ -58,5 +58,17 @@ describe Gitlab::GithubImport::BulkImporting do
 
       importer.bulk_insert(model, rows, batch_size: 5)
     end
+
+    it 'calls pre_hook for each slice if given' do
+      rows = [{ title: 'Foo' }] * 10
+      model = double(:model, table_name: 'kittens')
+      pre_hook = double('pre_hook', call: nil)
+      allow(Gitlab::Database).to receive(:bulk_insert)
+
+      expect(pre_hook).to receive(:call).with(rows[0..4])
+      expect(pre_hook).to receive(:call).with(rows[5..9])
+
+      importer.bulk_insert(model, rows, batch_size: 5, pre_hook: pre_hook)
+    end
   end
 end
diff --git a/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb b/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb
index 81fe97c1e49..3f7a12144d5 100644
--- a/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb
+++ b/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb
@@ -78,6 +78,11 @@ describe Gitlab::GithubImport::Importer::IssueImporter, :clean_gitlab_redis_cach
         .to receive(:id_for)
         .with(issue)
         .and_return(milestone.id)
+
+      allow(importer.user_finder)
+        .to receive(:author_id_for)
+        .with(issue)
+        .and_return([user.id, true])
     end
 
     context 'when the issue author could be found' do
@@ -172,6 +177,23 @@ describe Gitlab::GithubImport::Importer::IssueImporter, :clean_gitlab_redis_cach
 
       expect(importer.create_issue).to be_a_kind_of(Numeric)
     end
+
+    it 'triggers internal_id functionality to track greatest iids' do
+      allow(importer.user_finder)
+        .to receive(:author_id_for)
+        .with(issue)
+        .and_return([user.id, true])
+
+      issue = build_stubbed(:issue, project: project)
+      allow(Gitlab::GithubImport)
+        .to receive(:insert_and_return_id)
+        .and_return(issue.id)
+      allow(project.issues).to receive(:find).with(issue.id).and_return(issue)
+
+      expect(issue).to receive(:ensure_project_iid!)
+
+      importer.create_issue
+    end
   end
 
   describe '#create_assignees' do
diff --git a/spec/lib/gitlab/github_import/importer/milestones_importer_spec.rb b/spec/lib/gitlab/github_import/importer/milestones_importer_spec.rb
index b1cac3b6e46..db0be760c7b 100644
--- a/spec/lib/gitlab/github_import/importer/milestones_importer_spec.rb
+++ b/spec/lib/gitlab/github_import/importer/milestones_importer_spec.rb
@@ -29,13 +29,25 @@ describe Gitlab::GithubImport::Importer::MilestonesImporter, :clean_gitlab_redis
 
       expect(importer)
         .to receive(:bulk_insert)
-        .with(Milestone, [milestone_hash])
+        .with(Milestone, [milestone_hash], any_args)
 
       expect(importer)
         .to receive(:build_milestones_cache)
 
       importer.execute
     end
+
+    it 'tracks internal ids' do
+      milestone_hash = { iid: 1, title: '1.0', project_id: project.id }
+      allow(importer)
+        .to receive(:build_milestones)
+        .and_return([milestone_hash])
+
+      expect(InternalId).to receive(:track_greatest)
+        .with(nil, { project: project }, :milestones, 1, any_args)
+
+      importer.execute
+    end
   end
 
   describe '#build_milestones' do
diff --git a/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb
index 3422a1e82fc..44c920043b4 100644
--- a/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb
+++ b/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb
@@ -111,6 +111,16 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi
         expect(mr).to be_instance_of(MergeRequest)
         expect(exists).to eq(false)
       end
+
+      it 'triggers internal_id functionality to track greatest iids' do
+        mr = build_stubbed(:merge_request, source_project: project, target_project: project)
+        allow(Gitlab::GithubImport).to receive(:insert_and_return_id).and_return(mr.id)
+        allow(project.merge_requests).to receive(:find).with(mr.id).and_return(mr)
+
+        expect(mr).to receive(:ensure_target_project_iid!)
+
+        importer.create_merge_request
+      end
     end
 
     context 'when the author could not be found' do
diff --git a/spec/migrations/delete_inconsistent_internal_id_records_spec.rb b/spec/migrations/delete_inconsistent_internal_id_records_spec.rb
new file mode 100644
index 00000000000..becb71cf427
--- /dev/null
+++ b/spec/migrations/delete_inconsistent_internal_id_records_spec.rb
@@ -0,0 +1,119 @@
+# frozen_string_literal: true
+# rubocop:disable RSpec/FactoriesInMigrationSpecs
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20180723130817_delete_inconsistent_internal_id_records.rb')
+
+describe DeleteInconsistentInternalIdRecords, :migration do
+  let!(:project1) { create(:project) }
+  let!(:project2) { create(:project) }
+  let!(:project3) { create(:project) }
+
+  let(:internal_id_query) { ->(project) { InternalId.where(usage: InternalId.usages[scope.to_s.tableize], project: project) } }
+
+  let(:create_models) do
+    3.times { create(scope, project: project1) }
+    3.times { create(scope, project: project2) }
+    3.times { create(scope, project: project3) }
+  end
+
+  shared_examples_for 'deleting inconsistent internal_id records' do
+    before do
+      create_models
+
+      internal_id_query.call(project1).first.tap do |iid|
+        iid.last_value = iid.last_value - 2
+        # This is an inconsistent record
+        iid.save!
+      end
+
+      internal_id_query.call(project3).first.tap do |iid|
+        iid.last_value = iid.last_value + 2
+        # This is a consistent record
+        iid.save!
+      end
+    end
+
+    it "deletes inconsistent issues" do
+      expect { migrate! }.to change { internal_id_query.call(project1).size }.from(1).to(0)
+    end
+
+    it "retains consistent issues" do
+      expect { migrate! }.not_to change { internal_id_query.call(project2).size }
+    end
+
+    it "retains consistent records, especially those with a greater last_value" do
+      expect { migrate! }.not_to change { internal_id_query.call(project3).size }
+    end
+  end
+
+  context 'for issues' do
+    let(:scope) { :issue }
+    it_behaves_like 'deleting inconsistent internal_id records'
+  end
+
+  context 'for merge_requests' do
+    let(:scope) { :merge_request }
+
+    let(:create_models) do
+      3.times { |i| create(scope, target_project: project1, source_project: project1, source_branch: i.to_s) }
+      3.times { |i| create(scope, target_project: project2, source_project: project2, source_branch: i.to_s) }
+      3.times { |i| create(scope, target_project: project3, source_project: project3, source_branch: i.to_s) }
+    end
+
+    it_behaves_like 'deleting inconsistent internal_id records'
+  end
+
+  context 'for deployments' do
+    let(:scope) { :deployment }
+    it_behaves_like 'deleting inconsistent internal_id records'
+  end
+
+  context 'for milestones (by project)' do
+    let(:scope) { :milestone }
+    it_behaves_like 'deleting inconsistent internal_id records'
+  end
+
+  context 'for ci_pipelines' do
+    let(:scope) { :ci_pipeline }
+    it_behaves_like 'deleting inconsistent internal_id records'
+  end
+
+  context 'for milestones (by group)' do
+    # milestones (by group) is a little different than all of the other models
+    let!(:group1) { create(:group) }
+    let!(:group2) { create(:group) }
+    let!(:group3) { create(:group) }
+
+    let(:internal_id_query) { ->(group) { InternalId.where(usage: InternalId.usages['milestones'], namespace: group) } }
+
+    before do
+      3.times { create(:milestone, group: group1) }
+      3.times { create(:milestone, group: group2) }
+      3.times { create(:milestone, group: group3) }
+
+      internal_id_query.call(group1).first.tap do |iid|
+        iid.last_value = iid.last_value - 2
+        # This is an inconsistent record
+        iid.save!
+      end
+
+      internal_id_query.call(group3).first.tap do |iid|
+        iid.last_value = iid.last_value + 2
+        # This is a consistent record
+        iid.save!
+      end
+    end
+
+    it "deletes inconsistent issues" do
+      expect { migrate! }.to change { internal_id_query.call(group1).size }.from(1).to(0)
+    end
+
+    it "retains consistent issues" do
+      expect { migrate! }.not_to change { internal_id_query.call(group2).size }
+    end
+
+    it "retains consistent records, especially those with a greater last_value" do
+      expect { migrate! }.not_to change { internal_id_query.call(group3).size }
+    end
+  end
+end