diff options
| -rw-r--r-- | app/models/project.rb | 2 | ||||
| -rw-r--r-- | changelogs/unreleased/sh-optimize-merge-request-project-lookup.yml | 5 | ||||
| -rw-r--r-- | lib/gitlab/import/merge_request_helpers.rb | 2 | ||||
| -rw-r--r-- | spec/models/merge_request_spec.rb | 14 | ||||
| -rw-r--r-- | spec/models/project_spec.rb | 4 |
5 files changed, 25 insertions, 2 deletions
diff --git a/app/models/project.rb b/app/models/project.rb index d593cbb223a..106cb92e0cc 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -181,7 +181,7 @@ class Project < ActiveRecord::Base has_one :import_export_upload, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent # Merge Requests for target project should be removed with it - has_many :merge_requests, foreign_key: 'target_project_id' + has_many :merge_requests, foreign_key: 'target_project_id', inverse_of: :target_project has_many :source_of_merge_requests, foreign_key: 'source_project_id', class_name: 'MergeRequest' has_many :issues has_many :labels, class_name: 'ProjectLabel' diff --git a/changelogs/unreleased/sh-optimize-merge-request-project-lookup.yml b/changelogs/unreleased/sh-optimize-merge-request-project-lookup.yml new file mode 100644 index 00000000000..241b89c4633 --- /dev/null +++ b/changelogs/unreleased/sh-optimize-merge-request-project-lookup.yml @@ -0,0 +1,5 @@ +--- +title: Reduce SQL queries needed to load open merge requests +merge_request: 22709 +author: +type: performance diff --git a/lib/gitlab/import/merge_request_helpers.rb b/lib/gitlab/import/merge_request_helpers.rb index 97dc1a987c4..9215067d973 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.find(merge_request_id) + merge_request = project.merge_requests.reload.find(merge_request_id) # We use .insert_and_return_id which effectively disables all callbacks. # Trigger iid logic here to make sure we track internal id values consistently. diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index c8943f2d86f..85a4ebac66c 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -13,6 +13,20 @@ describe MergeRequest do it { is_expected.to belong_to(:merge_user).class_name("User") } it { is_expected.to belong_to(:assignee) } it { is_expected.to have_many(:merge_request_diffs) } + + context 'for forks' do + let!(:project) { create(:project) } + let!(:fork) { fork_project(project) } + let!(:merge_request) { create(:merge_request, target_project: project, source_project: fork) } + + it 'does not load another project due to inverse relationship' do + expect(project.merge_requests.first.target_project.object_id).to eq(project.object_id) + end + + it 'finds the associated merge request' do + expect(project.merge_requests.find(merge_request.id)).to eq(merge_request) + end + end end describe '#squash_in_progress?' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index e66838edd1a..83b3f308ec3 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -88,6 +88,10 @@ describe Project do it { is_expected.to have_many(:project_deploy_tokens) } it { is_expected.to have_many(:deploy_tokens).through(:project_deploy_tokens) } + it 'has an inverse relationship with merge requests' do + expect(described_class.reflect_on_association(:merge_requests).has_inverse?).to eq(:target_project) + end + context 'after initialized' do it "has a project_feature" do expect(described_class.new.project_feature).to be_present |