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

4 files changed, 57 insertions, 2 deletions

diff --git a/changelogs/unreleased/30098-banzai-filter-mergerequestreferencefilter-has-an-n-1-query-problem.yml b/changelogs/unreleased/30098-banzai-filter-mergerequestreferencefilter-has-an-n-1-query-problem.yml
new file mode 100644
index 00000000000..f3f4e065aef
--- /dev/null
+++ b/changelogs/unreleased/30098-banzai-filter-mergerequestreferencefilter-has-an-n-1-query-problem.yml
@@ -0,0 +1,4 @@
+---
+title: Improve Markdown rendering when a lot of merge requests are referenced
+merge_request: 10252
+author:
diff --git a/lib/banzai/filter/merge_request_reference_filter.rb b/lib/banzai/filter/merge_request_reference_filter.rb
index ac5216d9cfb..3888acf935e 100644
--- a/lib/banzai/filter/merge_request_reference_filter.rb
+++ b/lib/banzai/filter/merge_request_reference_filter.rb
@@ -11,8 +11,8 @@ module Banzai
         MergeRequest
       end
 
-      def find_object(project, id)
-        project.merge_requests.find_by(iid: id)
+      def find_object(project, iid)
+        merge_requests_per_project[project][iid]
       end
 
       def url_for_object(mr, project)
@@ -21,6 +21,31 @@ module Banzai
                                             only_path: context[:only_path])
       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)
         extras = super
 
diff --git a/spec/lib/banzai/filter/issue_reference_filter_spec.rb b/spec/lib/banzai/filter/issue_reference_filter_spec.rb
index 11607d4fb26..f1082495fcc 100644
--- a/spec/lib/banzai/filter/issue_reference_filter_spec.rb
+++ b/spec/lib/banzai/filter/issue_reference_filter_spec.rb
@@ -21,6 +21,19 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do
     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
     it_behaves_like 'a reference containing an element node'
 
diff --git a/spec/lib/banzai/filter/merge_request_reference_filter_spec.rb b/spec/lib/banzai/filter/merge_request_reference_filter_spec.rb
index 3d3d36061f4..40232f6e426 100644
--- a/spec/lib/banzai/filter/merge_request_reference_filter_spec.rb
+++ b/spec/lib/banzai/filter/merge_request_reference_filter_spec.rb
@@ -17,6 +17,19 @@ describe Banzai::Filter::MergeRequestReferenceFilter, lib: true do
     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
     let(:reference) { merge.to_reference }