Commit d2eb7d48 authored by Michael Kozono's avatar Michael Kozono

Merge branch '219309-fix-mr-n-plus-1' into 'master'

Resolve "Search API merge_requests scope preload project features"

Closes #219309

See merge request gitlab-org/gitlab!36712
parents 576d29a4 42b0158b
...@@ -251,16 +251,11 @@ class MergeRequest < ApplicationRecord ...@@ -251,16 +251,11 @@ class MergeRequest < ApplicationRecord
end end
scope :join_project, -> { joins(:target_project) } scope :join_project, -> { joins(:target_project) }
scope :references_project, -> { references(:target_project) } scope :references_project, -> { references(:target_project) }
PROJECT_ROUTE_AND_NAMESPACE_ROUTE = [
target_project: [:route, { namespace: :route }],
source_project: [:route, { namespace: :route }]
].freeze
scope :with_api_entity_associations, -> { scope :with_api_entity_associations, -> {
preload(:assignees, :author, :unresolved_notes, :labels, :milestone, preload_routables
.preload(:assignees, :author, :unresolved_notes, :labels, :milestone,
:timelogs, :latest_merge_request_diff, :timelogs, :latest_merge_request_diff,
*PROJECT_ROUTE_AND_NAMESPACE_ROUTE, target_project: :project_feature,
metrics: [:latest_closed_by, :merged_by]) metrics: [:latest_closed_by, :merged_by])
} }
...@@ -269,6 +264,10 @@ class MergeRequest < ApplicationRecord ...@@ -269,6 +264,10 @@ class MergeRequest < ApplicationRecord
end end
scope :by_target_branch, ->(branch_name) { where(target_branch: branch_name) } scope :by_target_branch, ->(branch_name) { where(target_branch: branch_name) }
scope :preload_source_project, -> { preload(:source_project) } scope :preload_source_project, -> { preload(:source_project) }
scope :preload_routables, -> do
preload(target_project: [:route, { namespace: :route }],
source_project: [:route, { namespace: :route }])
end
scope :with_auto_merge_enabled, -> do scope :with_auto_merge_enabled, -> do
with_state(:opened).where(auto_merge_enabled: true) with_state(:opened).where(auto_merge_enabled: true)
......
...@@ -71,8 +71,8 @@ class MergeTrain < ApplicationRecord ...@@ -71,8 +71,8 @@ class MergeTrain < ApplicationRecord
scope :by_id, -> (sort = :asc) { order(id: sort) } scope :by_id, -> (sort = :asc) { order(id: sort) }
scope :preload_api_entities, -> do scope :preload_api_entities, -> do
preload(:user, merge_request: MergeRequest::PROJECT_ROUTE_AND_NAMESPACE_ROUTE, preload(:user, :merge_request, pipeline: Ci::Pipeline::PROJECT_ROUTE_AND_NAMESPACE_ROUTE)
pipeline: Ci::Pipeline::PROJECT_ROUTE_AND_NAMESPACE_ROUTE) .merge(MergeRequest.preload_routables)
end end
class << self class << self
......
---
title: Avoid N+1 of merge requests associations in Search
merge_request: 36712
author:
type: performance
...@@ -374,6 +374,19 @@ RSpec.describe SearchService do ...@@ -374,6 +374,19 @@ RSpec.describe SearchService do
subject(:result) { search_service.search_objects } subject(:result) { search_service.search_objects }
shared_examples "redaction limits N+1 queries" do |limit:|
it 'does not exceed the query limit' do
# issuing the query to remove the data loading call
unredacted_results.to_a
# only the calls from the redaction are left
query = ActiveRecord::QueryRecorder.new { result }
# these are the project authorization calls, which are not preloaded
expect(query.count).to be <= limit
end
end
def found_blob(project) def found_blob(project)
Gitlab::Search::FoundBlob.new(project: project) Gitlab::Search::FoundBlob.new(project: project)
end end
...@@ -427,6 +440,12 @@ RSpec.describe SearchService do ...@@ -427,6 +440,12 @@ RSpec.describe SearchService do
it 'redacts the inaccessible merge request' do it 'redacts the inaccessible merge request' do
expect(result).to contain_exactly(readable) expect(result).to contain_exactly(readable)
end end
context 'with :with_api_entity_associations' do
let(:unredacted_results) { ar_relation(MergeRequest.with_api_entity_associations, readable, unreadable) }
it_behaves_like "redaction limits N+1 queries", limit: 7
end
end end
context 'project repository blobs' do context 'project repository blobs' do
...@@ -460,6 +479,10 @@ RSpec.describe SearchService do ...@@ -460,6 +479,10 @@ RSpec.describe SearchService do
it 'redacts the inaccessible snippet' do it 'redacts the inaccessible snippet' do
expect(result).to contain_exactly(readable) expect(result).to contain_exactly(readable)
end end
context 'with :with_api_entity_associations' do
it_behaves_like "redaction limits N+1 queries", limit: 12
end
end end
context 'personal snippets' do context 'personal snippets' do
...@@ -471,6 +494,10 @@ RSpec.describe SearchService do ...@@ -471,6 +494,10 @@ RSpec.describe SearchService do
it 'redacts the inaccessible snippet' do it 'redacts the inaccessible snippet' do
expect(result).to contain_exactly(readable) expect(result).to contain_exactly(readable)
end end
context 'with :with_api_entity_associations' do
it_behaves_like "redaction limits N+1 queries", limit: 3
end
end end
context 'commits' do context 'commits' do
......
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