Refactor identifier parsing

This applies OO principles to simplify indentifier parsing, representing
the two syntaxes with two subclasses of indentifier representation.

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,39 +3,51 @@
 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)
+      IllegalIdentifier = Class.new(ArgumentError)
+
+      def self.parse(gl_repository_str)
+        segments = gl_repository_str&.split('-')
+
+        # 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
+        case segments&.size
+        when 2
+          TwoPartIdentifier.new(gl_repository_str, *segments)
+        when 3
+          ThreePartIdentifier.new(gl_repository_str, *segments)
+        else
+          raise IllegalIdentifier, %Q(Invalid GL Repository "#{gl_repository_str}")
+        end
       end
 
-      private
-
-      attr_reader :segments, :container_class, :container_id
+      class TwoPartIdentifier < Identifier
+        def initialize(gl_repository_str, repo_type_name, container_id_str)
+          @gl_repository_str = gl_repository_str
+          @repo_type_name = repo_type_name
+          @container_id_str = container_id_str
+        end
 
-      def find_repo_type
-        type_name = three_segments_format? ? segments.last : segments.first
-        type = Gitlab::GlRepository.types[type_name]
+        def container_class
+          repo_type.container_class
+        end
+      end
 
-        raise_error unless type
+      class ThreePartIdentifier < Identifier
+        attr_reader :container_type
 
-        type
-      end
+        def initialize(gl_repository_str, container_type, container_id_str, repo_type_name)
+          @gl_repository_str = gl_repository_str
+          @container_type = container_type
+          @container_id_str = container_id_str
+          @repo_type_name = repo_type_name
+        end
 
-      def find_container_class
-        if three_segments_format?
-          case segments[0]
+        def container_class
+          case container_type
           when 'project'
             Project
           when 'group'
@@ -43,31 +55,27 @@ module Gitlab
           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 container
+        @container ||= container_class.find_by_id(container_id)
+      end
 
-        id
+      def repo_type
+        @repo_type ||= (Gitlab::GlRepository.types[repo_type_name] || raise_error)
       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
+      private
+
+      attr_reader :gl_repository_str, :container_id_str, :repo_type_name
+
+      def container_id
+        Integer(container_id_str, 10, exception: false) || raise_error
       end
 
       def raise_error
-        raise ArgumentError, "Invalid GL Repository \"#{gl_repository}\""
+        raise IllegalIdentifier, %Q(Invalid GL Repository "#{gl_repository_str}")
       end
     end
   end

spec/lib/gitlab/gl_repository/identifier_spec.rb

@@ -54,29 +54,19 @@ 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',
+        'snippet-2-wiki',
+        'snippet',
+        'project-1-wiki-bar'
+      ]
     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-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')
+    with_them do
+      it_behaves_like 'illegal gl_identifier'
     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)
+    expect(subject).to have_attributes(
+      repo_type: expected_type,
+      container: expected_container
+    )
+  end
+end
+
+RSpec.shared_examples 'illegal gl_identifier' do
+  subject do
+    described_class.parse(identifier).tap do |ident|
+      ident.repo_type
+      ident.container
     end
   end
+
+  it 'raises an error' do
+    expect { subject }.to raise_error(described_class::IllegalIdentifier)
+  end
 end