Eager loads routes and namespaces to avoid n+1 queries

New Merge Request page is performing badly on projects with lots of
forks due to N+1 queries while generating forked projects list view.

We should consider limiting the number of forked projects we load by
paginating or ajax search instead at some point, but this fix will
improve the situation for the time being.

app/finders/merge_request_target_project_finder.rb

@@ -11,15 +11,23 @@ class MergeRequestTargetProjectFinder
   end
 
   # rubocop: disable CodeReuse/ActiveRecord
-  def execute
-    if @source_project.fork_network
-      @source_project.fork_network.projects
-        .public_or_visible_to_user(current_user)
-        .non_archived
-        .with_feature_available_for_user(:merge_requests, current_user)
+  def execute(include_routes: false)
+    if source_project.fork_network
+      include_routes ? projects.inc_routes : projects
     else
       Project.where(id: source_project)
     end
   end
   # rubocop: enable CodeReuse/ActiveRecord
+
+  private
+
+  def projects
+    source_project
+      .fork_network
+      .projects
+      .public_or_visible_to_user(current_user)
+      .non_archived
+      .with_feature_available_for_user(:merge_requests, current_user)
+  end
 end

app/helpers/merge_requests_helper.rb

@@ -88,7 +88,7 @@ module MergeRequestsHelper
 
   def target_projects(project)
     MergeRequestTargetProjectFinder.new(current_user: current_user, source_project: project)
-      .execute
+      .execute(include_routes: true)
   end
 
   def merge_request_button_visibility(merge_request, closed)

app/models/project.rb

@@ -409,6 +409,7 @@ class Project < ApplicationRecord
   scope :for_milestones, ->(ids) { joins(:milestones).where('milestones.id' => ids).distinct }
   scope :with_push, -> { joins(:events).where('events.action = ?', Event::PUSHED) }
   scope :with_project_feature, -> { joins('LEFT JOIN project_features ON projects.id = project_features.project_id') }
+  scope :inc_routes, -> { includes(:route, namespace: :route) }
   scope :with_statistics, -> { includes(:statistics) }
   scope :with_service, ->(service) { joins(service).eager_load(service) }
   scope :with_shared_runners, -> { where(shared_runners_enabled: true) }

spec/finders/merge_request_target_project_finder_spec.rb

@@ -27,6 +27,22 @@ describe MergeRequestTargetProjectFinder do
 
       expect(finder.execute).to contain_exactly(other_fork, forked_project)
     end
+
+    it 'does not include routes by default' do
+      row = finder.execute.first
+
+      expect(row.association(:route).loaded?).to be_falsey
+      expect(row.association(:namespace).loaded?).to be_falsey
+      expect(row.namespace.association(:route).loaded?).to be_falsey
+    end
+
+    it 'includes routes when requested' do
+      row = finder.execute(include_routes: true).first
+
+      expect(row.association(:route).loaded?).to be_truthy
+      expect(row.association(:namespace).loaded?).to be_truthy
+      expect(row.namespace.association(:route).loaded?).to be_truthy
+    end
   end
 
   context 'public projects' do

spec/models/project_spec.rb

@@ -1786,11 +1786,11 @@ describe Project do
     end
   end
 
-  describe '.including_namespace_and_owner' do
+  describe '.eager_load_namespace_and_owner' do
     it 'eager loads the namespace and namespace owner' do
       create(:project)
 
-      row = described_class.eager_load_namespace_and_owner.to_a.first
+      row = described_class.eager_load_namespace_and_owner.first
       recorder = ActiveRecord::QueryRecorder.new { row.namespace.owner }
 
       expect(recorder.count).to be_zero

spec/requests/projects/merge_requests/creations_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'merge requests creations' do
+  describe 'GET /:namespace/:project/merge_requests/new' do
+    include ProjectForksHelper
+
+    let(:project) { create(:project, :repository) }
+    let(:user) { project.owner }
+
+    before do
+      login_as(user)
+    end
+
+    def get_new
+      get namespace_project_new_merge_request_path(namespace_id: project.namespace, project_id: project)
+    end
+
+    it 'avoids N+1 DB queries even with forked projects' do
+      control = ActiveRecord::QueryRecorder.new(skip_cached: false) { get_new }
+
+      5.times { fork_project(project, user) }
+
+      expect { get_new }.not_to exceed_query_limit(control)
+    end
+  end
+end