Merge branch 'bvl-limit-fork-queries-on-project-show' into 'master'

Cache the forks in a namespace in the RequestStore

Closes #40625

See merge request gitlab-org/gitlab-ce!15663

app/models/namespace.rb

@@ -139,7 +139,17 @@ class Namespace < ActiveRecord::Base
   def find_fork_of(project)
     return nil unless project.fork_network
 
-    project.fork_network.find_forks_in(projects).first
+    if RequestStore.active?
+      forks_in_namespace = RequestStore.fetch("namespaces:#{id}:forked_projects") do
+        Hash.new do |found_forks, project|
+          found_forks[project] = project.fork_network.find_forks_in(projects).first
+        end
+      end
+
+      forks_in_namespace[project]
+    else
+      project.fork_network.find_forks_in(projects).first
+    end
   end
 
   def lfs_enabled?

changelogs/unreleased/bvl-limit-fork-queries-on-project-show.yml

+---
+title: Reduce requests for project forks on show page of projects that have forks
+merge_request: 15663
+author:
+type: performance

spec/controllers/projects_controller_spec.rb

@@ -261,6 +261,27 @@ describe ProjectsController do
         expect(response).to redirect_to(namespace_project_path)
       end
     end
+
+    context 'when the project is forked and has a repository', :request_store do
+      let(:public_project) { create(:project, :public, :repository) }
+      let(:other_user) { create(:user) }
+
+      render_views
+
+      before do
+        # View the project as a user that does not have any rights
+        sign_in(other_user)
+
+        fork_project(public_project)
+      end
+
+      it 'does not increase the number of queries when the project is forked' do
+        expected_query = /#{public_project.fork_network.find_forks_in(other_user.namespace).to_sql}/
+
+        expect { get(:show, namespace_id: public_project.namespace, id: public_project) }
+          .not_to exceed_query_limit(1).for_query(expected_query)
+      end
+    end
   end
 
   describe "#update" do

spec/models/namespace_spec.rb

@@ -531,7 +531,7 @@ describe Namespace do
     end
   end
 
-  describe '#has_forks_of?' do
+  describe '#find_fork_of?' do
     let(:project) { create(:project, :public) }
     let!(:forked_project) { fork_project(project, namespace.owner, namespace: namespace) }
 
@@ -550,5 +550,13 @@ describe Namespace do
 
       expect(other_namespace.find_fork_of(project)).to eq(other_fork)
     end
+
+    context 'with request store enabled', :request_store do
+      it 'only queries once' do
+        expect(project.fork_network).to receive(:find_forks_in).once.and_call_original
+
+        2.times { namespace.find_fork_of(project) }
+      end
+    end
   end
 end

spec/support/query_recorder.rb

@@ -41,7 +41,8 @@ RSpec::Matchers.define :exceed_query_limit do |expected|
   supports_block_expectations
 
   match do |block|
-    query_count(&block) > expected_count + threshold
+    @subject_block = block
+    actual_count > expected_count + threshold
   end
 
   failure_message_when_negated do |actual|
@@ -55,6 +56,11 @@ RSpec::Matchers.define :exceed_query_limit do |expected|
     self
   end
 
+  def for_query(query)
+    @query = query
+    self
+  end
+
   def threshold
     @threshold.to_i
   end
@@ -68,12 +74,15 @@ RSpec::Matchers.define :exceed_query_limit do |expected|
   end
 
   def actual_count
-    @recorder.count
+    @actual_count ||= if @query
+                        recorder.log.select { |recorded| recorded =~ @query }.size
+                      else
+                        recorder.count
+                      end
   end
 
-  def query_count(&block)
-    @recorder = ActiveRecord::QueryRecorder.new(&block)
-    @recorder.count
+  def recorder
+    @recorder ||= ActiveRecord::QueryRecorder.new(&@subject_block)
   end
 
   def count_queries(queries)