Commit 1bec2fb3 authored by Yorick Peterse's avatar Yorick Peterse

Merge branch...

Merge branch '30098-banzai-filter-mergerequestreferencefilter-has-an-n-1-query-problem' into 'master'

Resolve "Banzai::Filter::MergeRequestReferenceFilter has three N+1 query problems"

Closes #30098

See merge request !10252
parents b45f6c65 cf394a03
---
title: Improve Markdown rendering when a lot of merge requests are referenced
merge_request: 10252
author:
...@@ -11,8 +11,8 @@ module Banzai ...@@ -11,8 +11,8 @@ module Banzai
MergeRequest MergeRequest
end end
def find_object(project, id) def find_object(project, iid)
project.merge_requests.find_by(iid: id) merge_requests_per_project[project][iid]
end end
def url_for_object(mr, project) def url_for_object(mr, project)
...@@ -21,6 +21,31 @@ module Banzai ...@@ -21,6 +21,31 @@ module Banzai
only_path: context[:only_path]) only_path: context[:only_path])
end end
def project_from_ref(ref)
projects_per_reference[ref || current_project_path]
end
# Returns a Hash containing the merge_requests per Project instance.
def merge_requests_per_project
@merge_requests_per_project ||= begin
hash = Hash.new { |h, k| h[k] = {} }
projects_per_reference.each do |path, project|
merge_request_ids = references_per_project[path]
merge_requests = project.merge_requests
.where(iid: merge_request_ids.to_a)
.includes(target_project: :namespace)
merge_requests.each do |merge_request|
hash[project][merge_request.iid.to_i] = merge_request
end
end
hash
end
end
def object_link_text_extras(object, matches) def object_link_text_extras(object, matches)
extras = super extras = super
......
...@@ -21,6 +21,19 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do ...@@ -21,6 +21,19 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do
end end
end end
describe 'performance' do
let(:another_issue) { create(:issue, project: project) }
it 'does not have a N+1 query problem' do
single_reference = "Issue #{issue.to_reference}"
multiple_references = "Issues #{issue.to_reference} and #{another_issue.to_reference}"
control_count = ActiveRecord::QueryRecorder.new { reference_filter(single_reference).to_html }.count
expect { reference_filter(multiple_references).to_html }.not_to exceed_query_limit(control_count)
end
end
context 'internal reference' do context 'internal reference' do
it_behaves_like 'a reference containing an element node' it_behaves_like 'a reference containing an element node'
......
...@@ -17,6 +17,19 @@ describe Banzai::Filter::MergeRequestReferenceFilter, lib: true do ...@@ -17,6 +17,19 @@ describe Banzai::Filter::MergeRequestReferenceFilter, lib: true do
end end
end end
describe 'performance' do
let(:another_merge) { create(:merge_request, source_project: project, source_branch: 'fix') }
it 'does not have a N+1 query problem' do
single_reference = "Merge request #{merge.to_reference}"
multiple_references = "Merge requests #{merge.to_reference} and #{another_merge.to_reference}"
control_count = ActiveRecord::QueryRecorder.new { reference_filter(single_reference).to_html }.count
expect { reference_filter(multiple_references).to_html }.not_to exceed_query_limit(control_count)
end
end
context 'internal reference' do context 'internal reference' do
let(:reference) { merge.to_reference } let(:reference) { merge.to_reference }
......
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