Merge branch 'sh-reduce-gitaly-calls-submodules' into 'master'

Eliminate Gitaly N+1 queries loading submodules

See merge request gitlab-org/gitlab!23292

app/helpers/submodule_helper.rb

@@ -7,9 +7,7 @@ module SubmoduleHelper
 
   # links to files listing for submodule if submodule is a project on this server
   def submodule_links(submodule_item, ref = nil, repository = @repository)
-    url = repository.submodule_url_for(ref, submodule_item.path)
-
-    submodule_links_for_url(submodule_item.id, url, repository)
+    repository.submodule_links.for(submodule_item, ref)
   end
 
   def submodule_links_for_url(submodule_item_id, url, repository)

app/models/repository.rb

@@ -1094,6 +1094,10 @@ class Repository
                                        message: message)
   end
 
+  def submodule_links
+    @submodule_links ||= ::Gitlab::SubmoduleLinks.new(self)
+  end
+
   def update_submodule(user, submodule, commit_sha, message:, branch:)
     with_cache_hooks do
       raw.update_submodule(

changelogs/unreleased/sh-reduce-gitaly-calls-submodules.yml

+---
+title: Eliminate Gitaly N+1 queries loading submodules
+merge_request: 23292
+author:
+type: performance

spec/helpers/submodule_helper_spec.rb

@@ -8,6 +8,11 @@ describe SubmoduleHelper do
   let(:submodule_item) { double(id: 'hash', path: 'rack') }
   let(:config) { Gitlab.config.gitlab }
   let(:repo) { double }
+  let(:submodules) { Gitlab::SubmoduleLinks.new(repo) }
+
+  before do
+    allow(repo).to receive(:submodule_links).and_return(submodules)
+  end
 
   shared_examples 'submodule_links' do
     context 'submodule on self' do
@@ -163,7 +168,7 @@ describe SubmoduleHelper do
       let(:repo) { double(:repo, project: project) }
 
       def expect_relative_link_to_resolve_to(relative_path, expected_path)
-        allow(repo).to receive(:submodule_url_for).and_return(relative_path)
+        stub_url(relative_path)
         result = subject
 
         expect(result).to eq([expected_path, "#{expected_path}/tree/#{submodule_item.id}"])
@@ -183,7 +188,7 @@ describe SubmoduleHelper do
 
       context 'repo path resolves to be located at root (namespace absent)' do
         it 'returns nil' do
-          allow(repo).to receive(:submodule_url_for).and_return('../../test.git')
+          stub_url('../../test.git')
 
           result = subject
 
@@ -193,7 +198,7 @@ describe SubmoduleHelper do
 
       context 'repo path resolves to be located underneath current project path' do
         it 'returns nil because it is not possible to have repo nested under another repo' do
-          allow(repo).to receive(:submodule_url_for).and_return('./test.git')
+          stub_url('./test.git')
 
           result = subject
 
@@ -263,6 +268,7 @@ describe SubmoduleHelper do
   end
 
   def stub_url(url)
+    allow(submodules).to receive(:submodule_url_for).and_return(url)
     allow(repo).to receive(:submodule_url_for).and_return(url)
   end
 end

spec/models/repository_spec.rb

@@ -2714,4 +2714,10 @@ describe Repository do
         .to change { Gitlab::GitalyClient.get_request_count }.by(1)
     end
   end
+
+  describe '#submodule_links' do
+    it 'returns an instance of Gitlab::SubmoduleLinks' do
+      expect(repository.submodule_links).to be_a(Gitlab::SubmoduleLinks)
+    end
+  end
 end