Merge branch '215315-repository-identifier-2' into 'master'

Add 3-segment format for gl_repository

Closes #215315

See merge request gitlab-org/gitlab!32641

lib/gitlab/gl_repository.rb

@@ -12,14 +12,15 @@ module Gitlab
     WIKI = RepoType.new(
       name: :wiki,
       access_checker_class: Gitlab::GitAccessWiki,
-      repository_resolver: -> (project) { project&.wiki&.repository },
+      repository_resolver: -> (container) { container&.wiki&.repository },
+      project_resolver: -> (container) { container.is_a?(Project) ? container : nil },
       suffix: :wiki
     ).freeze
     SNIPPET = RepoType.new(
       name: :snippet,
       access_checker_class: Gitlab::GitAccessSnippet,
       repository_resolver: -> (snippet) { snippet&.repository },
-      container_resolver: -> (id) { Snippet.find_by_id(id) },
+      container_class: Snippet,
       project_resolver: -> (snippet) { snippet&.project },
       guest_read_ability: :read_snippet
     ).freeze
@@ -42,16 +43,12 @@ module Gitlab
     end
 
     def self.parse(gl_repository)
-      type_name, _id = gl_repository.split('-').first
-      type = types[type_name]
+      result = ::Gitlab::GlRepository::Identifier.new(gl_repository)
 
-      unless type
-        raise ArgumentError, "Invalid GL Repository \"#{gl_repository}\""
-      end
+      repo_type = result.repo_type
+      container = result.fetch_container!
 
-      container = type.fetch_container!(gl_repository)
-
-      [container, type.project_for(container), type]
+      [container, repo_type.project_for(container), repo_type]
     end
 
     def self.default_type

lib/gitlab/gl_repository/identifier.rb

+# frozen_string_literal: true
+
+module Gitlab
+  class GlRepository
+    class Identifier
+      attr_reader :gl_repository, :repo_type
+
+      def initialize(gl_repository)
+        @gl_repository = gl_repository
+        @segments = gl_repository.split('-')
+
+        raise_error if segments.size > 3
+
+        @repo_type = find_repo_type
+        @container_id = find_container_id
+        @container_class = find_container_class
+      end
+
+      def fetch_container!
+        container_class.find_by_id(container_id)
+      end
+
+      private
+
+      attr_reader :segments, :container_class, :container_id
+
+      def find_repo_type
+        type_name = three_segments_format? ? segments.last : segments.first
+        type = Gitlab::GlRepository.types[type_name]
+
+        raise_error unless type
+
+        type
+      end
+
+      def find_container_class
+        if three_segments_format?
+          case segments[0]
+          when 'project'
+            Project
+          when 'group'
+            Group
+          else
+            raise_error
+          end
+        else
+          repo_type.container_class
+        end
+      end
+
+      def find_container_id
+        id = Integer(segments[1], 10, exception: false)
+
+        raise_error unless id
+
+        id
+      end
+
+      # gl_repository can either have 2 or 3 segments:
+      # "wiki-1" is the older 2-segment format, where container is implied.
+      # "group-1-wiki" is the newer 3-segment format, including container information.
+      #
+      # TODO: convert all 2-segment format to 3-segment:
+      # https://gitlab.com/gitlab-org/gitlab/-/issues/219192
+      def three_segments_format?
+        segments.size == 3
+      end
+
+      def raise_error
+        raise ArgumentError, "Invalid GL Repository \"#{gl_repository}\""
+      end
+    end
+  end
+end

lib/gitlab/gl_repository/repo_type.rb

@@ -6,7 +6,7 @@ module Gitlab
       attr_reader :name,
                   :access_checker_class,
                   :repository_resolver,
-                  :container_resolver,
+                  :container_class,
                   :project_resolver,
                   :guest_read_ability,
                   :suffix
@@ -15,34 +15,25 @@ module Gitlab
         name:,
         access_checker_class:,
         repository_resolver:,
-        container_resolver: default_container_resolver,
+        container_class: default_container_class,
         project_resolver: nil,
         guest_read_ability: :download_code,
         suffix: nil)
         @name = name
         @access_checker_class = access_checker_class
         @repository_resolver = repository_resolver
-        @container_resolver = container_resolver
+        @container_class = container_class
         @project_resolver = project_resolver
         @guest_read_ability = guest_read_ability
         @suffix = suffix
       end
 
       def identifier_for_container(container)
-        "#{name}-#{container.id}"
-      end
-
-      def fetch_id(identifier)
-        match = /\A#{name}-(?<id>\d+)\z/.match(identifier)
-        match[:id] if match
-      end
+        if container.is_a?(Group)
+          return "#{container.class.name.underscore}-#{container.id}-#{name}"
+        end
 
-      def fetch_container!(identifier)
-        id = fetch_id(identifier)
-
-        raise ArgumentError, "Invalid GL Repository \"#{identifier}\"" unless id
-
-        container_resolver.call(id)
+        "#{name}-#{container.id}"
       end
 
       def wiki?
@@ -85,8 +76,8 @@ module Gitlab
 
       private
 
-      def default_container_resolver
-        -> (id) { Project.find_by_id(id) }
+      def default_container_class
+        Project
       end
     end
   end

spec/lib/gitlab/gl_repository/identifier_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Gitlab::GlRepository::Identifier do
+  let_it_be(:project) { create(:project) }
+  let_it_be(:personal_snippet) { create(:personal_snippet, author: project.owner) }
+  let_it_be(:project_snippet) { create(:project_snippet, project: project, author: project.owner) }
+  let_it_be(:group) { create(:group) }
+
+  shared_examples 'parsing gl_repository identifier' do
+    subject { described_class.new(identifier) }
+
+    it 'returns correct information' do
+      aggregate_failures do
+        expect(subject.repo_type).to eq(expected_type)
+        expect(subject.fetch_container!).to eq(expected_container)
+      end
+    end
+  end
+
+  describe 'project repository' do
+    it_behaves_like 'parsing gl_repository identifier' do
+      let(:record_id) { project.id }
+      let(:identifier) { "project-#{record_id}" }
+      let(:expected_container) { project }
+      let(:expected_type) { Gitlab::GlRepository::PROJECT }
+    end
+  end
+
+  describe 'wiki' do
+    it_behaves_like 'parsing gl_repository identifier' do
+      let(:record_id) { project.id }
+      let(:identifier) { "wiki-#{record_id}" }
+      let(:expected_container) { project }
+      let(:expected_type) { Gitlab::GlRepository::WIKI }
+    end
+
+    context 'group wiki' do
+      it_behaves_like 'parsing gl_repository identifier' do
+        let(:record_id) { group.id }
+        let(:identifier) { "group-#{record_id}-wiki" }
+        let(:expected_container) { group }
+        let(:expected_type) { Gitlab::GlRepository::WIKI }
+      end
+    end
+  end
+
+  describe 'snippet' do
+    context 'when PersonalSnippet' do
+      it_behaves_like 'parsing gl_repository identifier' do
+        let(:record_id) { personal_snippet.id }
+        let(:identifier) { "snippet-#{record_id}" }
+        let(:expected_container) { personal_snippet }
+        let(:expected_type) { Gitlab::GlRepository::SNIPPET }
+      end
+    end
+
+    context 'when ProjectSnippet' do
+      it_behaves_like 'parsing gl_repository identifier' do
+        let(:record_id) { project_snippet.id }
+        let(:identifier) { "snippet-#{record_id}" }
+        let(:expected_container) { project_snippet }
+        let(:expected_type) { Gitlab::GlRepository::SNIPPET }
+      end
+    end
+  end
+
+  describe 'design' do
+    it_behaves_like 'parsing gl_repository identifier' do
+      let(:record_id) { project.id }
+      let(:identifier) { "design-#{project.id}" }
+      let(:expected_container) { project }
+      let(:expected_type) { Gitlab::GlRepository::DESIGN }
+    end
+  end
+
+  describe 'incorrect format' do
+    def expect_error_raised_for(identifier)
+      expect { described_class.new(identifier) }.to raise_error(ArgumentError)
+    end
+
+    it 'raises error for incorrect id' do
+      expect_error_raised_for('wiki-noid')
+    end
+
+    it 'raises error for incorrect type' do
+      expect_error_raised_for('foo-2')
+    end
+
+    it 'raises error for incorrect three-segments container' do
+      expect_error_raised_for('snippet-2-wiki')
+    end
+
+    it 'raises error for one segment' do
+      expect_error_raised_for('snippet')
+    end
+
+    it 'raises error for for than three segments' do
+      expect_error_raised_for('project-1-wiki-bar')
+    end
+  end
+end

spec/lib/gitlab/gl_repository/repo_type_spec.rb

@@ -13,7 +13,7 @@ describe Gitlab::GlRepository::RepoType do
 
   describe Gitlab::GlRepository::PROJECT do
     it_behaves_like 'a repo type' do
-      let(:expected_id) { project.id.to_s }
+      let(:expected_id) { project.id }
       let(:expected_identifier) { "project-#{expected_id}" }
       let(:expected_suffix) { '' }
       let(:expected_container) { project }
@@ -42,7 +42,7 @@ describe Gitlab::GlRepository::RepoType do
 
   describe Gitlab::GlRepository::WIKI do
     it_behaves_like 'a repo type' do
-      let(:expected_id) { project.id.to_s }
+      let(:expected_id) { project.id }
       let(:expected_identifier) { "wiki-#{expected_id}" }
       let(:expected_suffix) { '.wiki' }
       let(:expected_container) { project }
@@ -67,12 +67,24 @@ describe Gitlab::GlRepository::RepoType do
         expect(described_class.valid?(design_path)).to be_falsey
       end
     end
+
+    context 'group wiki' do
+      let_it_be(:group) { create(:group) }
+
+      it_behaves_like 'a repo type' do
+        let(:expected_id) { group.id }
+        let(:expected_identifier) { "group-#{expected_id}-wiki" }
+        let(:expected_suffix) { '.wiki' }
+        let(:expected_container) { group }
+        let(:expected_repository) { expected_container.wiki.repository }
+      end
+    end
   end
 
   describe Gitlab::GlRepository::SNIPPET do
     context 'when PersonalSnippet' do
       it_behaves_like 'a repo type' do
-        let(:expected_id) { personal_snippet.id.to_s }
+        let(:expected_id) { personal_snippet.id }
         let(:expected_identifier) { "snippet-#{expected_id}" }
         let(:expected_suffix) { '' }
         let(:expected_repository) { personal_snippet.repository }
@@ -101,7 +113,7 @@ describe Gitlab::GlRepository::RepoType do
 
     context 'when ProjectSnippet' do
       it_behaves_like 'a repo type' do
-        let(:expected_id) { project_snippet.id.to_s }
+        let(:expected_id) { project_snippet.id }
         let(:expected_identifier) { "snippet-#{expected_id}" }
         let(:expected_suffix) { '' }
         let(:expected_repository) { project_snippet.repository }
@@ -131,7 +143,7 @@ describe Gitlab::GlRepository::RepoType do
   describe Gitlab::GlRepository::DESIGN do
     it_behaves_like 'a repo type' do
       let(:expected_identifier) { "design-#{project.id}" }
-      let(:expected_id) { project.id.to_s }
+      let(:expected_id) { project.id }
       let(:expected_suffix) { '.design' }
       let(:expected_repository) { project.design_repository }
       let(:expected_container) { project }

spec/lib/gitlab/gl_repository_spec.rb

@@ -5,16 +5,21 @@ require 'spec_helper'
 describe ::Gitlab::GlRepository do
   describe '.parse' do
     let_it_be(:project) { create(:project, :repository) }
+    let_it_be(:group) { create(:group) }
     let_it_be(:snippet) { create(:personal_snippet) }
 
     it 'parses a project gl_repository' do
       expect(described_class.parse("project-#{project.id}")).to eq([project, project, Gitlab::GlRepository::PROJECT])
     end
 
-    it 'parses a wiki gl_repository' do
+    it 'parses a project wiki gl_repository' do
       expect(described_class.parse("wiki-#{project.id}")).to eq([project, project, Gitlab::GlRepository::WIKI])
     end
 
+    it 'parses a group wiki gl_repository' do
+      expect(described_class.parse("group-#{group.id}-wiki")).to eq([group, nil, Gitlab::GlRepository::WIKI])
+    end
+
     it 'parses a snippet gl_repository' do
       expect(described_class.parse("snippet-#{snippet.id}")).to eq([snippet, nil, Gitlab::GlRepository::SNIPPET])
     end

spec/support/shared_examples/lib/gitlab/repo_type_shared_examples.rb

@@ -7,26 +7,6 @@ RSpec.shared_examples 'a repo type' do
     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 '#fetch_container!' do
-    it 'returns the container' do
-      expect(described_class.fetch_container!(expected_identifier)).to eq expected_container
-    end
-
-    it 'raises an exception if the identifier is invalid' do
-      expect { described_class.fetch_container!('project-noid') }.to raise_error ArgumentError
-    end
-  end
-
   describe '#path_suffix' do
     subject { described_class.path_suffix }
 

spec/workers/post_receive_spec.rb

@@ -355,7 +355,7 @@ describe PostReceive do
 
   context "webhook" do
     it "fetches the correct project" do
-      expect(Project).to receive(:find_by).with(id: project.id.to_s)
+      expect(Project).to receive(:find_by).with(id: project.id)
 
       perform
     end