Merge branch 'add-repository-url-builder' into 'master'

Add RepositoryUrlBuilder for SSH/HTTP URLs

See merge request gitlab-org/gitlab!28780

app/helpers/submodule_helper.rb

@@ -72,7 +72,7 @@ module SubmoduleHelper
                                      project].join('')
 
     url_with_dotgit = url_no_dotgit + '.git'
-    url_with_dotgit == Gitlab::Shell.url_to_repo([namespace, '/', project].join(''))
+    url_with_dotgit == Gitlab::RepositoryUrlBuilder.build([namespace, '/', project].join(''))
   end
 
   def relative_self_url?(url)

app/models/concerns/has_repository.rb

@@ -87,26 +87,15 @@ module HasRepository
   end
 
   def url_to_repo
-    Gitlab::Shell.url_to_repo(full_path)
+    ssh_url_to_repo
   end
 
   def ssh_url_to_repo
-    url_to_repo
+    Gitlab::RepositoryUrlBuilder.build(repository.full_path, protocol: :ssh)
   end
 
   def http_url_to_repo
-    custom_root = Gitlab::CurrentSettings.custom_http_clone_url_root
-
-    url = if custom_root.present?
-            Gitlab::Utils.append_path(
-              custom_root,
-              web_url(only_path: true)
-            )
-          else
-            web_url
-          end
-
-    "#{url}.git"
+    Gitlab::RepositoryUrlBuilder.build(repository.full_path, protocol: :http)
   end
 
   def web_url(only_path: nil)

app/models/project_wiki.rb

@@ -49,15 +49,15 @@ class ProjectWiki
   end
 
   def url_to_repo
-    Gitlab::Shell.url_to_repo(full_path)
+    ssh_url_to_repo
   end
 
   def ssh_url_to_repo
-    url_to_repo
+    Gitlab::RepositoryUrlBuilder.build(repository.full_path, protocol: :ssh)
   end
 
   def http_url_to_repo
-    @project.http_url_to_repo.sub(%r{git\z}, 'wiki.git')
+    Gitlab::RepositoryUrlBuilder.build(repository.full_path, protocol: :http)
   end
 
   def wiki_base_path

app/models/snippet.rb

@@ -258,10 +258,12 @@ class Snippet < ApplicationRecord
     super
   end
 
+  override :repository
   def repository
     @repository ||= Repository.new(full_path, self, shard: repository_storage, disk_path: disk_path, repo_type: Gitlab::GlRepository::SNIPPET)
   end
 
+  override :repository_size_checker
   def repository_size_checker
     strong_memoize(:repository_size_checker) do
       ::Gitlab::RepositorySizeChecker.new(
@@ -271,6 +273,7 @@ class Snippet < ApplicationRecord
     end
   end
 
+  override :storage
   def storage
     @storage ||= Storage::Hashed.new(self, prefix: Storage::Hashed::SNIPPET_REPOSITORY_PATH_PREFIX)
   end
@@ -278,6 +281,7 @@ class Snippet < ApplicationRecord
   # This is the full_path used to identify the
   # the snippet repository. It will be used mostly
   # for logging purposes.
+  override :full_path
   def full_path
     return unless persisted?
 
@@ -290,10 +294,6 @@ class Snippet < ApplicationRecord
     end
   end
 
-  def url_to_repo
-    Gitlab::Shell.url_to_repo(full_path.delete('@'))
-  end
-
   def repository_storage
     snippet_repository&.shard_name || self.class.pick_repository_storage
   end

lib/gitlab/repository_url_builder.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module RepositoryUrlBuilder
+    class << self
+      def build(path, protocol: :ssh)
+        # TODO: See https://gitlab.com/gitlab-org/gitlab/-/issues/213021
+        path = path.sub('@snippets', 'snippets')
+
+        case protocol
+        when :ssh
+          ssh_url(path)
+        when :http
+          http_url(path)
+        else
+          raise NotImplementedError.new("No URL builder defined for protocol #{protocol}")
+        end
+      end
+
+      private
+
+      def ssh_url(path)
+        Gitlab.config.gitlab_shell.ssh_path_prefix + "#{path}.git"
+      end
+
+      def http_url(path)
+        root = Gitlab::CurrentSettings.custom_http_clone_url_root.presence || Gitlab::Routing.url_helpers.root_url
+
+        Gitlab::Utils.append_path(root, "#{path}.git")
+      end
+    end
+  end
+end

lib/gitlab/shell.rb

@@ -47,14 +47,6 @@ module Gitlab
         @version ||= File.read(gitlab_shell_version_file).chomp if File.readable?(gitlab_shell_version_file)
       end
 
-      # Return a SSH url for a given project path
-      #
-      # @param [String] full_path project path (URL)
-      # @return [String] SSH URL
-      def url_to_repo(full_path)
-        Gitlab.config.gitlab_shell.ssh_path_prefix + "#{full_path}.git"
-      end
-
       private
 
       def gitlab_shell_path

spec/lib/gitlab/repository_url_builder_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::RepositoryUrlBuilder do
+  describe '.build' do
+    using RSpec::Parameterized::TableSyntax
+
+    where(:factory, :path_generator) do
+      :project          | ->(project) { project.full_path }
+      :project_snippet  | ->(snippet) { "#{snippet.project.full_path}/snippets/#{snippet.id}" }
+      :project_wiki     | ->(wiki)    { "#{wiki.project.full_path}.wiki" }
+
+      :personal_snippet | ->(snippet) { "snippets/#{snippet.id}" }
+    end
+
+    with_them do
+      let(:container) { build_stubbed(factory) }
+      let(:repository) { container.repository }
+      let(:path) { path_generator.call(container) }
+      let(:url) { subject.build(repository.full_path, protocol: protocol) }
+
+      context 'when passing SSH protocol' do
+        let(:protocol) { :ssh }
+
+        it 'returns the SSH URL to the repository' do
+          expect(url).to eq("#{Gitlab.config.gitlab_shell.ssh_path_prefix}#{path}.git")
+        end
+      end
+
+      context 'when passing HTTP protocol' do
+        let(:protocol) { :http }
+
+        it 'returns the HTTP URL to the repo without a username' do
+          expect(url).to eq("#{Gitlab.config.gitlab.url}/#{path}.git")
+          expect(url).not_to include('@')
+        end
+
+        it 'includes the custom HTTP clone root if set' do
+          clone_root = 'https://git.example.com:51234/mygitlab'
+          stub_application_setting(custom_http_clone_url_root: clone_root)
+
+          expect(url).to eq("#{clone_root}/#{path}.git")
+        end
+      end
+
+      context 'when passing an unsupported protocol' do
+        let(:protocol) { :ftp }
+
+        it 'raises an exception' do
+          expect { url }.to raise_error(NotImplementedError)
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/shell_spec.rb

@@ -10,14 +10,6 @@ describe Gitlab::Shell do
 
   it { is_expected.to respond_to :remove_repository }
 
-  describe '.url_to_repo' do
-    let(:full_path) { 'diaspora/disaspora-rails' }
-
-    subject { described_class.url_to_repo(full_path) }
-
-    it { is_expected.to eq(Gitlab.config.gitlab_shell.ssh_path_prefix + full_path + '.git') }
-  end
-
   describe 'memoized secret_token' do
     let(:secret_file) { 'tmp/tests/.secret_shell_test' }
     let(:link_file) { 'tmp/tests/shell-secret-test/.gitlab_shell_secret' }

spec/models/project_wiki_spec.rb

@@ -34,7 +34,7 @@ describe ProjectWiki do
 
   describe "#url_to_repo" do
     it "returns the correct ssh url to the repo" do
-      expect(subject.url_to_repo).to eq(Gitlab::Shell.url_to_repo(subject.full_path))
+      expect(subject.url_to_repo).to eq(Gitlab::RepositoryUrlBuilder.build(subject.repository.full_path, protocol: :ssh))
     end
   end
 
@@ -45,27 +45,8 @@ describe ProjectWiki do
   end
 
   describe "#http_url_to_repo" do
-    let(:project) { create :project }
-
-    context 'when a custom HTTP clone URL root is not set' do
-      it 'returns the full http url to the repo' do
-        expected_url = "#{Gitlab.config.gitlab.url}/#{subject.full_path}.git"
-
-        expect(project_wiki.http_url_to_repo).to eq(expected_url)
-        expect(project_wiki.http_url_to_repo).not_to include('@')
-      end
-    end
-
-    context 'when a custom HTTP clone URL root is set' do
-      before do
-        stub_application_setting(custom_http_clone_url_root: 'https://git.example.com:51234')
-      end
-
-      it 'returns the full http url to the repo, with the root replaced with the custom one' do
-        expected_url = "https://git.example.com:51234/#{subject.full_path}.git"
-
-        expect(project_wiki.http_url_to_repo).to eq(expected_url)
-      end
+    it "returns the correct http url to the repo" do
+      expect(subject.http_url_to_repo).to eq(Gitlab::RepositoryUrlBuilder.build(subject.repository.full_path, protocol: :http))
     end
   end
 

spec/models/snippet_spec.rb

@@ -735,22 +735,6 @@ describe Snippet do
     end
   end
 
-  describe '#url_to_repo' do
-    subject { snippet.url_to_repo }
-
-    context 'with personal snippet' do
-      let(:snippet) { create(:personal_snippet) }
-
-      it { is_expected.to eq(Gitlab.config.gitlab_shell.ssh_path_prefix + "snippets/#{snippet.id}.git") }
-    end
-
-    context 'with project snippet' do
-      let(:snippet) { create(:project_snippet) }
-
-      it { is_expected.to eq(Gitlab.config.gitlab_shell.ssh_path_prefix + "#{snippet.project.full_path}/snippets/#{snippet.id}.git") }
-    end
-  end
-
   describe '#versioned_enabled_for?' do
     let_it_be(:user) { create(:user) }