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

This reverts merge request !32641

lib/gitlab/gl_repository.rb

@@ -12,15 +12,14 @@ module Gitlab
     WIKI = RepoType.new(
       name: :wiki,
       access_checker_class: Gitlab::GitAccessWiki,
-      repository_resolver: -> (container) { container&.wiki&.repository },
-      project_resolver: -> (container) { container.is_a?(Project) ? container : nil },
+      repository_resolver: -> (project) { project&.wiki&.repository },
       suffix: :wiki
     ).freeze
     SNIPPET = RepoType.new(
       name: :snippet,
       access_checker_class: Gitlab::GitAccessSnippet,
       repository_resolver: -> (snippet) { snippet&.repository },
-      container_class: Snippet,
+      container_resolver: -> (id) { Snippet.find_by_id(id) },
       project_resolver: -> (snippet) { snippet&.project },
       guest_read_ability: :read_snippet
     ).freeze
@@ -43,12 +42,16 @@ module Gitlab
     end
 
     def self.parse(gl_repository)
-      result = ::Gitlab::GlRepository::Identifier.new(gl_repository)
+      type_name, _id = gl_repository.split('-').first
+      type = types[type_name]
 
-      repo_type = result.repo_type
-      container = result.fetch_container!
+      unless type
+        raise ArgumentError, "Invalid GL Repository \"#{gl_repository}\""
+      end
 
-      [container, repo_type.project_for(container), repo_type]
+      container = type.fetch_container!(gl_repository)
+
+      [container, type.project_for(container), 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_class,
+                  :container_resolver,
                   :project_resolver,
                   :guest_read_ability,
                   :suffix
@@ -15,27 +15,36 @@ module Gitlab
         name:,
         access_checker_class:,
         repository_resolver:,
-        container_class: default_container_class,
+        container_resolver: default_container_resolver,
         project_resolver: nil,
         guest_read_ability: :download_code,
         suffix: nil)
         @name = name
         @access_checker_class = access_checker_class
         @repository_resolver = repository_resolver
-        @container_class = container_class
+        @container_resolver = container_resolver
         @project_resolver = project_resolver
         @guest_read_ability = guest_read_ability
         @suffix = suffix
       end
 
       def identifier_for_container(container)
-        if container.is_a?(Group)
-          return "#{container.class.name.underscore}-#{container.id}-#{name}"
-        end
-
         "#{name}-#{container.id}"
       end
 
+      def fetch_id(identifier)
+        match = /\A#{name}-(?<id>\d+)\z/.match(identifier)
+        match[:id] if match
+      end
+
+      def fetch_container!(identifier)
+        id = fetch_id(identifier)
+
+        raise ArgumentError, "Invalid GL Repository \"#{identifier}\"" unless id
+
+        container_resolver.call(id)
+      end
+
       def wiki?
         self == WIKI
       end
@@ -76,8 +85,8 @@ module Gitlab
 
       private
 
-      def default_container_class
-        Project
+      def default_container_resolver
+        -> (id) { Project.find_by_id(id) }
       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 }
+      let(:expected_id) { project.id.to_s }
       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 }
+      let(:expected_id) { project.id.to_s }
       let(:expected_identifier) { "wiki-#{expected_id}" }
       let(:expected_suffix) { '.wiki' }
       let(:expected_container) { project }
@@ -67,24 +67,12 @@ 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 }
+        let(:expected_id) { personal_snippet.id.to_s }
         let(:expected_identifier) { "snippet-#{expected_id}" }
         let(:expected_suffix) { '' }
         let(:expected_repository) { personal_snippet.repository }
@@ -113,7 +101,7 @@ describe Gitlab::GlRepository::RepoType do
 
     context 'when ProjectSnippet' do
       it_behaves_like 'a repo type' do
-        let(:expected_id) { project_snippet.id }
+        let(:expected_id) { project_snippet.id.to_s }
         let(:expected_identifier) { "snippet-#{expected_id}" }
         let(:expected_suffix) { '' }
         let(:expected_repository) { project_snippet.repository }
@@ -143,7 +131,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 }
+      let(:expected_id) { project.id.to_s }
       let(:expected_suffix) { '.design' }
       let(:expected_repository) { project.design_repository }
       let(:expected_container) { project }

spec/lib/gitlab/gl_repository_spec.rb

@@ -5,21 +5,16 @@ 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 project wiki gl_repository' do
+    it 'parses a 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,6 +7,26 @@ 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)
+      expect(Project).to receive(:find_by).with(id: project.id.to_s)
 
       perform
     end