Merge branch 'ajk-gl-identifier' into 'master'

Refactor identifier parsing

See merge request gitlab-org/gitlab!34638

lib/gitlab/gl_repository.rb

@@ -43,10 +43,10 @@ module Gitlab
     end
 
     def self.parse(gl_repository)
-      result = ::Gitlab::GlRepository::Identifier.new(gl_repository)
+      identifier = ::Gitlab::GlRepository::Identifier.parse(gl_repository)
 
-      repo_type = result.repo_type
-      container = result.fetch_container!
+      repo_type = identifier.repo_type
+      container = identifier.container
 
       [container, repo_type.project_for(container), repo_type]
     end

lib/gitlab/gl_repository/identifier.rb

@@ -3,71 +3,83 @@
 module Gitlab
   class GlRepository
     class Identifier
-      attr_reader :gl_repository, :repo_type
+      include Gitlab::Utils::StrongMemoize
 
-      def initialize(gl_repository)
-        @gl_repository = gl_repository
-        @segments = gl_repository.split('-')
+      InvalidIdentifier = Class.new(ArgumentError)
 
-        raise_error if segments.size > 3
+      def self.parse(gl_repository)
+        segments = gl_repository&.split('-')
 
-        @repo_type = find_repo_type
-        @container_id = find_container_id
-        @container_class = find_container_class
+        # gl_repository can either have 2 or 3 segments:
+        #
+        # TODO: convert all 2-segment format to 3-segment:
+        # https://gitlab.com/gitlab-org/gitlab/-/issues/219192
+        identifier = case segments&.size
+                     when 2
+                       TwoPartIdentifier.new(*segments)
+                     when 3
+                       ThreePartIdentifier.new(*segments)
                      end
 
-      def fetch_container!
-        container_class.find_by_id(container_id)
-      end
+        return identifier if identifier&.valid?
 
-      private
+        raise InvalidIdentifier, %Q(Invalid GL Repository "#{gl_repository}")
+      end
 
-      attr_reader :segments, :container_class, :container_id
+      # The older 2-segment format, where the container is implied.
+      # eg. project-1, wiki-1
+      class TwoPartIdentifier < Identifier
+        def initialize(repo_type_name, container_id_str)
+          @container_id_str = container_id_str
+          @repo_type_name = repo_type_name
+        end
 
-      def find_repo_type
-        type_name = three_segments_format? ? segments.last : segments.first
-        type = Gitlab::GlRepository.types[type_name]
+        private
 
-        raise_error unless type
+        def container_class
+          repo_type.container_class
+        end
+      end
 
-        type
+      # The newer 3-segment format, where the container is explicit
+      # eg. group-1-wiki, project-1-wiki
+      class ThreePartIdentifier < Identifier
+        def initialize(container_type, container_id_str, repo_type_name)
+          @container_id_str = container_id_str
+          @container_type = container_type
+          @repo_type_name = repo_type_name
         end
 
-      def find_container_class
-        if three_segments_format?
-          case segments[0]
+        private
+
+        def container_class
+          case @container_type
           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
+      def repo_type
+        strong_memoize(:repo_type) { Gitlab::GlRepository.types[repo_type_name] }
+      end
 
-        id
+      def container
+        strong_memoize(:container) { container_class.find_by_id(container_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
+      def valid?
+        repo_type.present? && container_class.present? && container_id&.positive?
       end
 
-      def raise_error
-        raise ArgumentError, "Invalid GL Repository \"#{gl_repository}\""
+      private
+
+      attr_reader :container_id_str, :repo_type_name
+
+      def container_id
+        strong_memoize(:container_id) { Integer(container_id_str, 10, exception: false) }
       end
     end
   end

spec/lib/gitlab/gl_repository/identifier_spec.rb

@@ -14,6 +14,21 @@ RSpec.describe Gitlab::GlRepository::Identifier do
       let(:expected_container) { project }
       let(:expected_type) { Gitlab::GlRepository::PROJECT }
     end
+
+    pending 'https://gitlab.com/gitlab-org/gitlab/-/issues/219192' do
+      it_behaves_like 'parsing gl_repository identifier' do
+        let(:record_id) { project.id }
+        let(:identifier) { "project-#{record_id}-code" }
+        let(:expected_container) { project }
+        let(:expected_type) { Gitlab::GlRepository::PROJECT }
+      end
+    end
+
+    it_behaves_like 'parsing gl_repository identifier' do
+      let(:identifier) { "project-1000000" }
+      let(:expected_container) { nil }
+      let(:expected_type) { Gitlab::GlRepository::PROJECT }
+    end
   end
 
   describe 'wiki' do
@@ -23,6 +38,13 @@ RSpec.describe Gitlab::GlRepository::Identifier do
       let(:expected_container) { project }
       let(:expected_type) { Gitlab::GlRepository::WIKI }
     end
+
+    it_behaves_like 'parsing gl_repository identifier' do
+      let(:record_id) { project.id }
+      let(:identifier) { "project-#{record_id}-wiki" }
+      let(:expected_container) { project }
+      let(:expected_type) { Gitlab::GlRepository::WIKI }
+    end
   end
 
   describe 'snippet' do
@@ -54,29 +76,30 @@ RSpec.describe Gitlab::GlRepository::Identifier do
     end
   end
 
-  describe 'incorrect format' do
-    def expect_error_raised_for(identifier)
-      expect { described_class.new(identifier) }.to raise_error(ArgumentError)
+  context 'when the format is incorrect' do
+    where(:identifier) do
+      [
+        'wiki-noid',
+        'foo-2',
+        'project-0',
+        '2-project',
+        'snippet-2-wiki',
+        'project-wibble-wiki',
+        'wiki-1-project',
+        'snippet',
+        'project-1-wiki-bar'
+      ]
     end
 
-    it 'raises error for incorrect id' do
-      expect_error_raised_for('wiki-noid')
+    with_them do
+      it 'raises InvalidIdentifier' do
+        expect { described_class.parse(identifier) }.to raise_error(described_class::InvalidIdentifier)
       end
-
-    it 'raises error for incorrect type' do
-      expect_error_raised_for('foo-2')
-    end
-
-    it 'raises error for incorrect three-segment 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 more than three segments' do
-      expect_error_raised_for('project-1-wiki-bar')
+    it 'raises InvalidIdentifier on project-1-project' do
+      pending 'https://gitlab.com/gitlab-org/gitlab/-/issues/219192'
+      expect { described_class.parse('project-1-project') }.to raise_error(described_class::InvalidIdentifier)
     end
   end
 end

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

 # frozen_string_literal: true
 
 RSpec.shared_examples 'parsing gl_repository identifier' do
-  subject { described_class.new(identifier) }
+  subject { described_class.parse(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
+    expect(subject).to have_attributes(
+      repo_type: expected_type,
+      container: expected_container
+    )
   end
 end