Improve performance of PR importfix-pull-request-importer

This removes unneeded `.reload` call which
makes AR to load ALL objects, and create its
in-memory representation.

3 files changed, 79 insertions, 1 deletions

diff --git a/changelogs/unreleased/fix-pull-request-importer.yml b/changelogs/unreleased/fix-pull-request-importer.yml
new file mode 100644
index 00000000000..5f642a0710b
--- /dev/null
+++ b/changelogs/unreleased/fix-pull-request-importer.yml
@@ -0,0 +1,5 @@
+---
+title: Improve performance of PR import
+merge_request: 27121
+author:
+type: performance
diff --git a/lib/gitlab/import/merge_request_helpers.rb b/lib/gitlab/import/merge_request_helpers.rb
index b3fe1fc0685..4bc39868389 100644
--- a/lib/gitlab/import/merge_request_helpers.rb
+++ b/lib/gitlab/import/merge_request_helpers.rb
@@ -22,7 +22,7 @@ module Gitlab
           # additional work that is strictly necessary.
           merge_request_id = insert_and_return_id(attributes, project.merge_requests)
 
-          merge_request = project.merge_requests.reload.find(merge_request_id)
+          merge_request = project.merge_requests.reset.find(merge_request_id)
 
           [merge_request, false]
         end
diff --git a/spec/lib/gitlab/import/merge_request_helpers_spec.rb b/spec/lib/gitlab/import/merge_request_helpers_spec.rb
new file mode 100644
index 00000000000..cc0f2baf905
--- /dev/null
+++ b/spec/lib/gitlab/import/merge_request_helpers_spec.rb
@@ -0,0 +1,73 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Import::MergeRequestHelpers, type: :helper do
+  set(:project) { create(:project, :repository) }
+  set(:user) { create(:user) }
+
+  describe '.create_merge_request_without_hooks' do
+    let(:iid) { 42 }
+
+    let(:attributes) do
+      {
+        iid: iid,
+        title: 'My Pull Request',
+        description: 'This is my pull request',
+        source_project_id: project.id,
+        target_project_id: project.id,
+        source_branch: 'master-42',
+        target_branch: 'master',
+        state: :merged,
+        author_id: user.id,
+        assignee_id: user.id
+      }
+    end
+
+    subject { helper.create_merge_request_without_hooks(project, attributes, iid) }
+
+    context 'when merge request does not exist' do
+      it 'returns a new object' do
+        expect(subject.first).not_to be_nil
+        expect(subject.second).to eq(false)
+      end
+
+      it 'does load all existing objects' do
+        5.times do |iid|
+          MergeRequest.create!(
+            attributes.merge(iid: iid, source_branch: iid.to_s))
+        end
+
+        # does ensure that we only load object twice
+        # 1. by #insert_and_return_id
+        # 2. by project.merge_requests.find
+        expect_any_instance_of(MergeRequest).to receive(:attributes)
+          .twice.times.and_call_original
+
+        expect(subject.first).not_to be_nil
+        expect(subject.second).to eq(false)
+      end
+    end
+
+    context 'when merge request does exist' do
+      before do
+        MergeRequest.create!(attributes)
+      end
+
+      it 'returns an existing object' do
+        expect(subject.first).not_to be_nil
+        expect(subject.second).to eq(true)
+      end
+    end
+
+    context 'when project is deleted' do
+      before do
+        project.delete
+      end
+
+      it 'returns an existing object' do
+        expect(subject.first).to be_nil
+      end
+    end
+  end
+end