Merge branch 'bvl-allow-more-repos-per-resource-ce' into 'master'

Allow multiple repositories per project

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

app/controllers/projects/git_http_client_controller.rb

@@ -78,7 +78,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController
   end
   def parse_repo_path
-    @project, @wiki, @redirected_path = Gitlab::RepoPath.parse("#{params[:namespace_id]}/#{params[:project_id]}")
+    @project, @repo_type, @redirected_path = Gitlab::RepoPath.parse("#{params[:namespace_id]}/#{params[:project_id]}")
   end
   def render_missing_personal_access_token
@@ -89,13 +89,19 @@ class Projects::GitHttpClientController < Projects::ApplicationController
   end
   def repository
-    wiki? ? project.wiki.repository : project.repository
+    repo_type.repository_for(project)
   end
   def wiki?
-    parse_repo_path unless defined?(@wiki)
-    @wiki
+    repo_type.wiki?
+  end
+  def repo_type
+    parse_repo_path unless defined?(@repo_type)
+    # When there a project did not exist, the parsed repo_type would be empty.
+    # In that case, we want to continue with a regular project repository. As we
+    # could create the project if the user pushing is allowed to do so.
+    @repo_type || Gitlab::GlRepository::PROJECT
   end
   def handle_basic_authentication(login, password)

app/controllers/projects/git_http_controller.rb

@@ -55,7 +55,7 @@ class Projects::GitHttpController < Projects::GitHttpClientController
   def render_ok
     set_workhorse_internal_api_content_type
-    render json: Gitlab::Workhorse.git_http_ok(repository, wiki?, user, action_name)
+    render json: Gitlab::Workhorse.git_http_ok(repository, repo_type, user, action_name)
   end
   def render_403(exception)
@@ -99,7 +99,7 @@ class Projects::GitHttpController < Projects::GitHttpClientController
   end
   def access_klass
-    @access_klass ||= wiki? ? Gitlab::GitAccessWiki : Gitlab::GitAccess
+    @access_klass ||= repo_type.access_checker_class
   end
   def project_path

app/models/project.rb

@@ -2003,12 +2003,8 @@ class Project < ActiveRecord::Base
     @storage = nil if storage_version_changed?
   end
-  def gl_repository(is_wiki:)
-    Gitlab::GlRepository.gl_repository(self, is_wiki)
-  end
-
-  def reference_counter(wiki: false)
-    Gitlab::ReferenceCounter.new(gl_repository(is_wiki: wiki))
+  def reference_counter(type: Gitlab::GlRepository::PROJECT)
+    Gitlab::ReferenceCounter.new(type.identifier_for_subject(self))
   end
   def badges
@@ -2152,7 +2148,7 @@ class Project < ActiveRecord::Base
   end
   def wiki_reference_count
-    reference_counter(wiki: true).value
+    reference_counter(type: Gitlab::GlRepository::WIKI).value
   end
   def check_repository_absence!

app/models/project_wiki.rb

@@ -59,7 +59,7 @@ class ProjectWiki
   # Returns the Gitlab::Git::Wiki object.
   def wiki
     @wiki ||= begin
-      gl_repository = Gitlab::GlRepository.gl_repository(project, true)
+      gl_repository = Gitlab::GlRepository::WIKI.identifier_for_subject(project)
       raw_repository = Gitlab::Git::Repository.new(project.repository_storage, disk_path + '.git', gl_repository, full_path)
       create_repo!(raw_repository) unless raw_repository.exists?
@@ -151,7 +151,7 @@ class ProjectWiki
   end
   def repository
-    @repository ||= Repository.new(full_path, @project, disk_path: disk_path, is_wiki: true)
+    @repository ||= Repository.new(full_path, @project, disk_path: disk_path, repo_type: Gitlab::GlRepository::WIKI)
   end
   def default_branch

app/models/repository.rb

@@ -19,7 +19,7 @@ class Repository
   include Gitlab::RepositoryCacheAdapter
-  attr_accessor :full_path, :disk_path, :project, :is_wiki
+  attr_accessor :full_path, :disk_path, :project, :repo_type
   delegate :ref_name_for_sha, to: :raw_repository
   delegate :bundle_to_disk, to: :raw_repository
@@ -60,12 +60,12 @@ class Repository
     xcode_config: :xcode_project?
   }.freeze
-  def initialize(full_path, project, disk_path: nil, is_wiki: false)
+  def initialize(full_path, project, disk_path: nil, repo_type: Gitlab::GlRepository::PROJECT)
     @full_path = full_path
     @disk_path = disk_path || full_path
     @project = project
     @commit_cache = {}
-    @is_wiki = is_wiki
+    @repo_type = repo_type
   end
   def ==(other)
@@ -1112,7 +1112,7 @@ class Repository
   def initialize_raw_repository
     Gitlab::Git::Repository.new(project.repository_storage,
                                 disk_path + '.git',
-                                Gitlab::GlRepository.gl_repository(project, is_wiki),
+                                repo_type.identifier_for_subject(project),
                                 project.full_path)
   end
 end

app/workers/post_receive.rb

@@ -4,7 +4,7 @@ class PostReceive
   include ApplicationWorker
   def perform(gl_repository, identifier, changes, push_options = [])
-    project, is_wiki = Gitlab::GlRepository.parse(gl_repository)
+    project, repo_type = Gitlab::GlRepository.parse(gl_repository)
     if project.nil?
       log("Triggered hook for non-existing project with gl_repository \"#{gl_repository}\"")
@@ -17,7 +17,7 @@ class PostReceive
     Sidekiq.logger.info "changes: #{changes.inspect}" if ENV['SIDEKIQ_LOG_ARGUMENTS']
     post_received = Gitlab::GitPostReceive.new(project, identifier, changes, push_options)
-    if is_wiki
+    if repo_type.wiki?
       process_wiki_changes(post_received)
     else
       process_project_changes(post_received)

lib/api/helpers/internal_helpers.rb

@@ -5,9 +5,11 @@ module API
     module InternalHelpers
       attr_reader :redirected_path
-      def wiki?
-        set_project unless defined?(@wiki) # rubocop:disable Gitlab/ModuleWithInstanceVariables
-        @wiki # rubocop:disable Gitlab/ModuleWithInstanceVariables
+      delegate :wiki?, to: :repo_type
+
+      def repo_type
+        set_project unless defined?(@repo_type) # rubocop:disable Gitlab/ModuleWithInstanceVariables
+        @repo_type # rubocop:disable Gitlab/ModuleWithInstanceVariables
       end
       def project
@@ -67,10 +69,10 @@ module API
       # rubocop:disable Gitlab/ModuleWithInstanceVariables
       def set_project
         if params[:gl_repository]
-          @project, @wiki = Gitlab::GlRepository.parse(params[:gl_repository])
+          @project, @repo_type = Gitlab::GlRepository.parse(params[:gl_repository])
           @redirected_path = nil
         else
-          @project, @wiki, @redirected_path = Gitlab::RepoPath.parse(params[:project])
+          @project, @repo_type, @redirected_path = Gitlab::RepoPath.parse(params[:project])
         end
       end
       # rubocop:enable Gitlab/ModuleWithInstanceVariables
@@ -78,7 +80,7 @@ module API
       # Project id to pass between components that don't share/don't have
       # access to the same filesystem mounts
       def gl_repository
-        Gitlab::GlRepository.gl_repository(project, wiki?)
+        repo_type.identifier_for_subject(project)
       end
       def gl_project_path
@@ -92,7 +94,7 @@ module API
       # Return the repository depending on whether we want the wiki or the
       # regular repository
       def repository
-        if wiki?
+        if repo_type.wiki?
           project.wiki.repository
         else
           project.repository

lib/api/internal.rb

@@ -59,7 +59,7 @@ module API
             actor
           end
-        access_checker_klass = wiki? ? Gitlab::GitAccessWiki : Gitlab::GitAccess
+        access_checker_klass = repo_type.access_checker_class
         access_checker = access_checker_klass.new(actor, project,
           protocol, authentication_abilities: ssh_authentication_abilities,
                     namespace_path: namespace_path, project_path: project_path,

lib/gitlab/gl_repository.rb

@@ -2,23 +2,38 @@
 module Gitlab
   module GlRepository
-    def self.gl_repository(project, is_wiki)
-      "#{is_wiki ? 'wiki' : 'project'}-#{project.id}"
+    PROJECT = RepoType.new(
+      name: :project,
+      access_checker_class: Gitlab::GitAccess,
+      repository_accessor: -> (project) { project.repository }
+    ).freeze
+    WIKI = RepoType.new(
+      name: :wiki,
+      access_checker_class: Gitlab::GitAccessWiki,
+      repository_accessor: -> (project) { project.wiki.repository }
+    ).freeze
+
+    TYPES = {
+      PROJECT.name.to_s => PROJECT,
+      WIKI.name.to_s => WIKI
+    }.freeze
+
+    def self.types
+      TYPES
     end
-    # rubocop: disable CodeReuse/ActiveRecord
     def self.parse(gl_repository)
-      match_data = /\A(project|wiki)-([1-9][0-9]*)\z/.match(gl_repository)
-      unless match_data
+      type_name, _id = gl_repository.split('-').first
+      type = types[type_name]
+      subject_id = type&.fetch_id(gl_repository)
+
+      unless subject_id
         raise ArgumentError, "Invalid GL Repository \"#{gl_repository}\""
       end
-      type, id = match_data.captures
-      project = Project.find_by(id: id)
-      wiki = type == 'wiki'
-      [project, wiki]
+      project = Project.find_by_id(subject_id)
+      [project, type]
     end
-    # rubocop: enable CodeReuse/ActiveRecord
   end
 end

lib/gitlab/gl_repository/repo_type.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module GlRepository
+    class RepoType
+      attr_reader :name,
+                  :access_checker_class,
+                  :repository_accessor
+
+      def initialize(name:, access_checker_class:, repository_accessor:)
+        @name = name
+        @access_checker_class = access_checker_class
+        @repository_accessor = repository_accessor
+      end
+
+      def identifier_for_subject(subject)
+        "#{name}-#{subject.id}"
+      end
+
+      def fetch_id(identifier)
+        match = /\A#{name}-(?<id>\d+)\z/.match(identifier)
+        match[:id] if match
+      end
+
+      def wiki?
+        self == WIKI
+      end
+
+      def project?
+        self == PROJECT
+      end
+
+      def path_suffix
+        project? ? "" : ".#{name}"
+      end
+
+      def repository_for(subject)
+        repository_accessor.call(subject)
+      end
+    end
+  end
+end

lib/gitlab/repo_path.rb

@@ -5,19 +5,26 @@ module Gitlab
     NotFoundError = Class.new(StandardError)
     def self.parse(repo_path)
-      wiki = false
       project_path = repo_path.sub(/\.git\z/, '').sub(%r{\A/}, '')
-      project, was_redirected = find_project(project_path)
-
-      if project_path.end_with?('.wiki') && project.nil?
-        project, was_redirected = find_project(project_path.chomp('.wiki'))
-        wiki = true
+      # Detect the repo type based on the path, the first one tried is the project
+      # type, which does not have a suffix.
+      Gitlab::GlRepository.types.each do |_name, type|
+        # If the project path does not end with the defined suffix, try the next
+        # type.
+        # We'll always try to find a project with an empty suffix (for the
+        # `Gitlab::GlRepository::PROJECT` type.
+        next unless project_path.end_with?(type.path_suffix)
+
+        project, was_redirected = find_project(project_path.chomp(type.path_suffix))
+        redirected_path = project_path if was_redirected
+
+        # If we found a matching project, then the type was matched, no need to
+        # continue looking.
+        return [project, type, redirected_path] if project
       end
-      redirected_path = project_path if was_redirected
-
-      [project, wiki, redirected_path]
+      nil
     end
     def self.find_project(project_path)

lib/gitlab/workhorse.rb

@@ -20,14 +20,14 @@ module Gitlab
     SECRET_LENGTH = 32
     class << self
-      def git_http_ok(repository, is_wiki, user, action, show_all_refs: false)
+      def git_http_ok(repository, repo_type, user, action, show_all_refs: false)
         raise "Unsupported action: #{action}" unless ALLOWED_GIT_HTTP_ACTIONS.include?(action.to_s)
         project = repository.project
         attrs = {
           GL_ID: Gitlab::GlId.gl_id(user),
-          GL_REPOSITORY: Gitlab::GlRepository.gl_repository(project, is_wiki),
+          GL_REPOSITORY: repo_type.identifier_for_subject(project),
           GL_USERNAME: user&.username,
           ShowAllRefs: show_all_refs,
           Repository: repository.gitaly_repository.to_h,

spec/lib/gitlab/gl_repository/repo_type_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Gitlab::GlRepository::RepoType do
+  set(:project) { create(:project) }
+
+  shared_examples 'a repo type' do
+    describe "#identifier_for_subject" do
+      subject { described_class.identifier_for_subject(project) }
+
+      it { is_expected.to eq(expected_identifier) }
+    end
+
+    describe "#fetch_id" do
+      it "finds an id match in the identifier" do
+        expect(described_class.fetch_id(expected_identifier)).to eq(expected_id)
+      end
+
+      it 'does not break on other identifiers' do
+        expect(described_class.fetch_id("wiki-noid")).to eq(nil)
+      end
+    end
+
+    describe "#path_suffix" do
+      subject { described_class.path_suffix }
+
+      it { is_expected.to eq(expected_suffix) }
+    end
+
+    describe "#repository_for" do
+      it "finds the repository for the repo type" do
+        expect(described_class.repository_for(project)).to eq(expected_repository)
+      end
+    end
+  end
+
+  describe Gitlab::GlRepository::PROJECT do
+    it_behaves_like 'a repo type' do
+      let(:expected_identifier) { "project-#{project.id}" }
+      let(:expected_id) { project.id.to_s }
+      let(:expected_suffix) { "" }
+      let(:expected_repository) { project.repository }
+    end
+
+    it "knows its type" do
+      expect(described_class).not_to be_wiki
+      expect(described_class).to be_project
+    end
+  end
+
+  describe Gitlab::GlRepository::WIKI do
+    it_behaves_like 'a repo type' do
+      let(:expected_identifier) { "wiki-#{project.id}" }
+      let(:expected_id) { project.id.to_s }
+      let(:expected_suffix) { ".wiki" }
+      let(:expected_repository) { project.wiki.repository }
+    end
+
+    it "knows its type" do
+      expect(described_class).to be_wiki
+      expect(described_class).not_to be_project
+    end
+  end
+end

spec/lib/gitlab/gl_repository_spec.rb

@@ -5,11 +5,11 @@ describe ::Gitlab::GlRepository do
     set(:project) { create(:project, :repository) }
     it 'parses a project gl_repository' do
-      expect(described_class.parse("project-#{project.id}")).to eq([project, false])
+      expect(described_class.parse("project-#{project.id}")).to eq([project, Gitlab::GlRepository::PROJECT])
     end
     it 'parses a wiki gl_repository' do
-      expect(described_class.parse("wiki-#{project.id}")).to eq([project, true])
+      expect(described_class.parse("wiki-#{project.id}")).to eq([project, Gitlab::GlRepository::WIKI])
     end
     it 'throws an argument error on an invalid gl_repository' do

spec/lib/gitlab/repo_path_spec.rb

@@ -6,43 +6,47 @@ describe ::Gitlab::RepoPath do
     context 'a repository storage path' do
       it 'parses a full repository path' do
-        expect(described_class.parse(project.repository.full_path)).to eq([project, false, nil])
+        expect(described_class.parse(project.repository.full_path)).to eq([project, Gitlab::GlRepository::PROJECT, nil])
       end
       it 'parses a full wiki path' do
-        expect(described_class.parse(project.wiki.repository.full_path)).to eq([project, true, nil])
+        expect(described_class.parse(project.wiki.repository.full_path)).to eq([project, Gitlab::GlRepository::WIKI, nil])
       end
     end
     context 'a relative path' do
       it 'parses a relative repository path' do
-        expect(described_class.parse(project.full_path + '.git')).to eq([project, false, nil])
+        expect(described_class.parse(project.full_path + '.git')).to eq([project, Gitlab::GlRepository::PROJECT, nil])
       end
       it 'parses a relative wiki path' do
-        expect(described_class.parse(project.full_path + '.wiki.git')).to eq([project, true, nil])
+        expect(described_class.parse(project.full_path + '.wiki.git')).to eq([project, Gitlab::GlRepository::WIKI, nil])
       end
       it 'parses a relative path starting with /' do
-        expect(described_class.parse('/' + project.full_path + '.git')).to eq([project, false, nil])
+        expect(described_class.parse('/' + project.full_path + '.git')).to eq([project, Gitlab::GlRepository::PROJECT, nil])
       end
       context 'of a redirected project' do
         let(:redirect) { project.route.create_redirect('foo/bar') }
         it 'parses a relative repository path' do
-          expect(described_class.parse(redirect.path + '.git')).to eq([project, false, 'foo/bar'])
+          expect(described_class.parse(redirect.path + '.git')).to eq([project, Gitlab::GlRepository::PROJECT, 'foo/bar'])
         end
         it 'parses a relative wiki path' do
-          expect(described_class.parse(redirect.path + '.wiki.git')).to eq([project, true, 'foo/bar.wiki'])
+          expect(described_class.parse(redirect.path + '.wiki.git')).to eq([project, Gitlab::GlRepository::WIKI, 'foo/bar.wiki'])
         end
         it 'parses a relative path starting with /' do
-          expect(described_class.parse('/' + redirect.path + '.git')).to eq([project, false, 'foo/bar'])
+          expect(described_class.parse('/' + redirect.path + '.git')).to eq([project, Gitlab::GlRepository::PROJECT, 'foo/bar'])
         end
       end
     end
+
+    it "returns nil for non existent paths" do
+      expect(described_class.parse("path/non-existent.git")).to eq(nil)
+    end
   end
   describe '.find_project' do

spec/lib/gitlab/workhorse_spec.rb

@@ -250,11 +250,11 @@ describe Gitlab::Workhorse do
       }
     end
-    subject { described_class.git_http_ok(repository, false, user, action) }
+    subject { described_class.git_http_ok(repository, Gitlab::GlRepository::PROJECT, user, action) }
     it { expect(subject).to include(params) }
-    context 'when is_wiki' do
+    context 'when the repo_type is a wiki' do
       let(:params) do
         {
           GL_ID: "user-#{user.id}",
@@ -264,7 +264,7 @@ describe Gitlab::Workhorse do
         }
       end
-      subject { described_class.git_http_ok(repository, true, user, action) }
+      subject { described_class.git_http_ok(repository, Gitlab::GlRepository::WIKI, user, action) }
       it { expect(subject).to include(params) }
     end
@@ -304,7 +304,7 @@ describe Gitlab::Workhorse do
         end
         context 'show_all_refs enabled' do
-          subject { described_class.git_http_ok(repository, false, user, action, show_all_refs: true) }
+          subject { described_class.git_http_ok(repository, Gitlab::GlRepository::PROJECT, user, action, show_all_refs: true) }
           it { is_expected.to include(ShowAllRefs: true) }
         end
@@ -322,7 +322,7 @@ describe Gitlab::Workhorse do
         it { expect(subject).to include(gitaly_params) }
         context 'show_all_refs enabled' do
-          subject { described_class.git_http_ok(repository, false, user, action, show_all_refs: true) }
+          subject { described_class.git_http_ok(repository, Gitlab::GlRepository::PROJECT, user, action, show_all_refs: true) }
           it { is_expected.to include(ShowAllRefs: true) }
         end

spec/models/project_spec.rb

@@ -3428,7 +3428,7 @@ describe Project do
       end
       it 'schedules HashedStorage::ProjectMigrateWorker with delayed start when the project repo is in use' do
-        Gitlab::ReferenceCounter.new(project.gl_repository(is_wiki: false)).increase
+        Gitlab::ReferenceCounter.new(Gitlab::GlRepository::PROJECT.identifier_for_subject(project)).increase
         expect(HashedStorage::ProjectMigrateWorker).to receive(:perform_in)
@@ -3436,7 +3436,7 @@ describe Project do
       end
       it 'schedules HashedStorage::ProjectMigrateWorker with delayed start when the wiki repo is in use' do
-        Gitlab::ReferenceCounter.new(project.gl_repository(is_wiki: true)).increase
+        Gitlab::ReferenceCounter.new(Gitlab::GlRepository::WIKI.identifier_for_subject(project)).increase
         expect(HashedStorage::ProjectMigrateWorker).to receive(:perform_in)
@@ -3569,16 +3569,6 @@ describe Project do
     end
   end
-  describe '#gl_repository' do
-    let(:project) { create(:project) }
-
-    it 'delegates to Gitlab::GlRepository.gl_repository' do
-      expect(Gitlab::GlRepository).to receive(:gl_repository).with(project, true)
-
-      project.gl_repository(is_wiki: true)
-    end
-  end
-
   describe '#has_ci?' do
     set(:project) { create(:project) }
     let(:repository) { double }

spec/requests/api/internal_spec.rb

@@ -321,7 +321,7 @@ describe API::Internal do
       end
       context 'with env passed as a JSON' do
-        let(:gl_repository) { project.gl_repository(is_wiki: true) }
+        let(:gl_repository) { Gitlab::GlRepository::WIKI.identifier_for_subject(project) }
         it 'sets env in RequestStore' do
           obj_dir_relative = './objects'
@@ -975,9 +975,9 @@ describe API::Internal do
   def gl_repository_for(project_or_wiki)
     case project_or_wiki
     when ProjectWiki
-      project_or_wiki.project.gl_repository(is_wiki: true)
+      Gitlab::GlRepository::WIKI.identifier_for_subject(project_or_wiki.project)
     when Project
-      project_or_wiki.gl_repository(is_wiki: false)
+      Gitlab::GlRepository::PROJECT.identifier_for_subject(project_or_wiki)
     else
       nil
     end