Fix CI pipelines N+1 in Issues::ReferencedMergeRequestsService

Whether the preloading belongs in the service or the controller is arguable,
here. As the service is only used for one controller action, it seems reasonable
to put it in the service, but that is not a definitive answer.

Adding the preloads for MR project routes here doesn't seem to work, perhaps
because of https://github.com/rails/rails/issues/32140.

2 files changed, 26 insertions, 4 deletions

diff --git a/app/services/issues/referenced_merge_requests_service.rb b/app/services/issues/referenced_merge_requests_service.rb
index 40375a0c861..c64ac806ac6 100644
--- a/app/services/issues/referenced_merge_requests_service.rb
+++ b/app/services/issues/referenced_merge_requests_service.rb
@@ -3,10 +3,14 @@
 module Issues
   class ReferencedMergeRequestsService < Issues::BaseService
     def execute(issue)
-      [
-        sort_by_iid(referenced_merge_requests(issue)),
-        sort_by_iid(closed_by_merge_requests(issue))
-      ]
+      referenced = referenced_merge_requests(issue)
+      closed_by = closed_by_merge_requests(issue)
+      preloader = ActiveRecord::Associations::Preloader.new
+
+      preloader.preload(referenced + closed_by,
+                        head_pipeline: { project: [:route, { namespace: :route }] })
+
+      [sort_by_iid(referenced), sort_by_iid(closed_by)]
     end
 
     def referenced_merge_requests(issue)
diff --git a/spec/services/issues/referenced_merge_requests_service_spec.rb b/spec/services/issues/referenced_merge_requests_service_spec.rb
index 3349b97e688..d57bcf55af1 100644
--- a/spec/services/issues/referenced_merge_requests_service_spec.rb
+++ b/spec/services/issues/referenced_merge_requests_service_spec.rb
@@ -47,6 +47,24 @@ describe Issues::ReferencedMergeRequestsService do
 
         expect { service.execute(issue) }.not_to exceed_query_limit(control_count)
       end
+
+      it 'preloads the head pipeline for each merge request, and its routes' do
+        # Hack to ensure no data is preserved on issue before starting the spec,
+        # to avoid false negatives
+        reloaded_issue = Issue.find(issue.id)
+
+        pipeline_routes = lambda do |merge_requests|
+          merge_requests.map { |mr| mr.head_pipeline&.project&.full_path }
+        end
+
+        closing_mr_other_project.update!(head_pipeline: create(:ci_pipeline))
+        control_count = ActiveRecord::QueryRecorder.new { service.execute(reloaded_issue).each(&pipeline_routes) }
+
+        closing_mr.update!(head_pipeline: create(:ci_pipeline))
+
+        expect { service.execute(issue).each(&pipeline_routes) }
+          .not_to exceed_query_limit(control_count)
+      end
     end
   end