Further simplification of parsing

This moves all error handling to a single call to `valid?`, eliminating
the need to store the input string on the identifier, and allowing
removal of several methods. In return, all other methods are made
nil-safe.

lib/gitlab/gl_repository/identifier.rb

@@ -3,10 +3,12 @@
 module Gitlab
   class GlRepository
     class Identifier
-      IllegalIdentifier = Class.new(ArgumentError)
+      include Gitlab::Utils::StrongMemoize
 
-      def self.parse(gl_repository_str)
-        segments = gl_repository_str&.split('-')
+      InvalidIdentifier = Class.new(ArgumentError)
+
+      def self.parse(gl_repository)
+        segments = gl_repository&.split('-')
 
         # gl_repository can either have 2 or 3 segments:
         # "wiki-1" is the older 2-segment format, where container is implied.
@@ -14,68 +16,70 @@ def self.parse(gl_repository_str)
         #
         # 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
+        identifier = case segments&.size
+                     when 2
+                       TwoPartIdentifier.new(*segments)
+                     when 3
+                       ThreePartIdentifier.new(*segments)
+                     end
+
+        return identifier if identifier&.valid?
+
+        raise InvalidIdentifier, %Q(Invalid GL Repository "#{gl_repository}")
       end
 
       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
+        def initialize(repo_type_name, container_id_str)
           @container_id_str = container_id_str
+          @repo_type_name = repo_type_name
         end
 
+        private
+
         def container_class
-          repo_type.container_class
+          repo_type&.container_class
         end
       end
 
       class ThreePartIdentifier < Identifier
-        attr_reader :container_type
-
-        def initialize(gl_repository_str, container_type, container_id_str, repo_type_name)
-          @gl_repository_str = gl_repository_str
-          @container_type = container_type
+        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
 
+        private
+
         def container_class
-          case container_type
+          case @container_type
           when 'project'
             Project
           when 'group'
             Group
-          else
-            raise_error
           end
         end
       end
 
+      def repo_type
+        strong_memoize(:repo_type) {Gitlab::GlRepository.types[repo_type_name] }
+      end
+
       def container
-        @container ||= container_class.find_by_id(container_id)
+        strong_memoize(:container) do
+          container_class&.find_by_id(container_id) if container_id
+        end
       end
 
-      def repo_type
-        @repo_type ||= (Gitlab::GlRepository.types[repo_type_name] || raise_error)
+      def valid?
+        repo_type.present? && container_class.present? && container_id&.positive?
       end
 
       private
 
-      attr_reader :gl_repository_str, :container_id_str, :repo_type_name
+      attr_reader :container_id_str, :repo_type_name
 
       def container_id
-        Integer(container_id_str, 10, exception: false) || raise_error
-      end
-
-      def raise_error
-        raise IllegalIdentifier, %Q(Invalid GL Repository "#{gl_repository_str}")
+        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,19 @@
       let(:expected_container) { project }
       let(:expected_type) { Gitlab::GlRepository::PROJECT }
     end
+
+    it_behaves_like 'parsing gl_repository identifier' do
+      let(:record_id) { project.id }
+      let(:identifier) { "project-#{record_id}-project" }
+      let(:expected_container) { project }
+      let(:expected_type) { Gitlab::GlRepository::PROJECT }
+    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 +36,13 @@
       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
@@ -59,7 +79,11 @@
       [
         'wiki-noid',
         'foo-2',
+        'project-0',
+        '2-project',
         'snippet-2-wiki',
+        'project-wibble-wiki',
+        'wiki-1-project',
         'snippet',
         'project-1-wiki-bar'
       ]

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

@@ -12,14 +12,7 @@
 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)
+    expect { described_class.parse(identifier) }.to raise_error(described_class::InvalidIdentifier)
   end
 end