Merge branch 'gitaly-submodule-url-for' into 'master'

Migrate #submodule_url_for to Gitaly

See merge request !12629

lib/gitlab/git/repository.rb

@@ -554,11 +554,14 @@ module Gitlab
       #   # => git@localhost:rack.git
       #
       def submodule_url_for(ref, path)
-        if submodules(ref).any?
-          submodule = submodules(ref)[path]
-
-          if submodule
-            submodule['url']
+        Gitlab::GitalyClient.migrate(:submodule_url_for) do |is_enabled|
+          if is_enabled
+            gitaly_submodule_url_for(ref, path)
+          else
+            if submodules(ref).any?
+              submodule = submodules(ref)[path]
+              submodule['url'] if submodule
+            end
           end
         end
       end
@@ -915,6 +918,18 @@ module Gitlab
         fill_submodule_ids(commit, parser.parse)
       end
 
+      def gitaly_submodule_url_for(ref, path)
+        # We don't care about the contents so 1 byte is enough. Can't request 0 bytes, 0 means unlimited.
+        commit_object = gitaly_commit_client.tree_entry(ref, path, 1)
+
+        return unless commit_object && commit_object.type == :COMMIT
+
+        gitmodules = gitaly_commit_client.tree_entry(ref, '.gitmodules', Blob::MAX_DATA_DISPLAY_SIZE)
+        found_module = GitmodulesParser.new(gitmodules.data).parse[path]
+
+        found_module && found_module['url']
+      end
+
       def alternate_object_directories
         Gitlab::Git::Env.all.values_at(*ALLOWED_OBJECT_DIRECTORIES_VARIABLES).compact
       end

spec/lib/gitlab/git/repository_spec.rb

@@ -328,6 +328,38 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
   end
 
+  describe '#submodule_url_for' do
+    let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) }
+    let(:ref) { 'master' }
+
+    def submodule_url(path)
+      repository.submodule_url_for(ref, path)
+    end
+
+    it { expect(submodule_url('six')).to eq('git://github.com/randx/six.git') }
+    it { expect(submodule_url('nested/six')).to eq('git://github.com/randx/six.git') }
+    it { expect(submodule_url('deeper/nested/six')).to eq('git://github.com/randx/six.git') }
+    it { expect(submodule_url('invalid/path')).to eq(nil) }
+
+    context 'uncommitted submodule dir' do
+      let(:ref) { 'fix-existing-submodule-dir' }
+
+      it { expect(submodule_url('submodule-existing-dir')).to eq(nil) }
+    end
+
+    context 'tags' do
+      let(:ref) { 'v1.2.1' }
+
+      it { expect(submodule_url('six')).to eq('git://github.com/randx/six.git') }
+    end
+
+    context 'no submodules at commit' do
+      let(:ref) { '6d39438' }
+
+      it { expect(submodule_url('six')).to eq(nil) }
+    end
+  end
+
   context '#submodules' do
     let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) }