From 485c867fc20083b4fb305736ec437e06e81aeca3 Mon Sep 17 00:00:00 2001
From: Vasilii Iakliushin <viakliushin@gitlab.com>
Date: Mon, 6 May 2024 14:38:34 +0200
Subject: [PATCH] Use ExtractsRef::RefExtractor in ExtractsRef

Contrbutes to https://gitlab.com/gitlab-org/gitlab/-/issues/425379

* Replace `#assign_ref_vars` with `RefExtractor`
* Remove not used anymore `#extract_ref_path`
---
 lib/extracts_ref.rb                           | 25 +++++++------------
 lib/extracts_ref/ref_extractor.rb             |  6 ++---
 spec/lib/extracts_ref/ref_extractor_spec.rb   | 10 ++++++++
 .../ref_extraction_shared_examples.rb         |  8 +++++-
 4 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/lib/extracts_ref.rb b/lib/extracts_ref.rb
index 59557a8eb3768c..afa6e7804c178f 100644
--- a/lib/extracts_ref.rb
+++ b/lib/extracts_ref.rb
@@ -74,18 +74,18 @@ def extract_ref(id)
   # that will be handled as well.
   # rubocop:disable Gitlab/ModuleWithInstanceVariables
   def assign_ref_vars
-    @id, @ref, @path = extract_ref_path
-    @repo = repository_container.repository
-    raise InvalidPathError if @ref.match?(/\s/)
+    ref_extractor = ExtractsRef::RefExtractor.new(repository_container, params.permit(:id, :ref, :path, :ref_type))
+    ref_extractor.extract!
+
+    @id = ref_extractor.id
+    @ref = ref_extractor.ref
+    @path = ref_extractor.path
+    @repo = ref_extractor.repo
 
     return unless @ref.present?
 
-    @commit = if ref_type
-                @fully_qualified_ref = ExtractsRef::RefExtractor.qualify_ref(@ref, ref_type)
-                @repo.commit(@fully_qualified_ref)
-              else
-                @repo.commit(@ref)
-              end
+    @commit = ref_extractor.commit
+    @fully_qualified_ref = ref_extractor.fully_qualified_ref
   end
   # rubocop:enable Gitlab/ModuleWithInstanceVariables
 
@@ -93,13 +93,6 @@ def tree
     @tree ||= @repo.tree(@commit.id, @path) # rubocop:disable Gitlab/ModuleWithInstanceVariables
   end
 
-  def extract_ref_path
-    id = get_id
-    ref, path = extract_ref(id)
-
-    [id, ref, path]
-  end
-
   def ref_type
     ExtractsRef.ref_type(params[:ref_type])
   end
diff --git a/lib/extracts_ref/ref_extractor.rb b/lib/extracts_ref/ref_extractor.rb
index e716c64e63aed5..b67548ef01e35c 100644
--- a/lib/extracts_ref/ref_extractor.rb
+++ b/lib/extracts_ref/ref_extractor.rb
@@ -11,7 +11,7 @@ class RefExtractor
     REF_TYPES = [BRANCH_REF_TYPE, TAG_REF_TYPE].freeze
 
     attr_reader :repository_container, :params
-    attr_accessor :id, :ref, :commit, :path, :fully_qualified_ref
+    attr_accessor :id, :ref, :commit, :path, :fully_qualified_ref, :repo
 
     class << self
       def ref_type(type)
@@ -37,7 +37,7 @@ def unqualify_ref(ref, type)
 
     def initialize(repository_container, params, override_id: nil)
       @repository_container = repository_container
-      @params = params.extract!(:id, :ref, :path, :ref_type)
+      @params = params.slice(:id, :ref, :path, :ref_type)
       @override_id = override_id
     end
 
@@ -129,7 +129,7 @@ def extract_raw_ref(id)
       return ['', ''] unless repository_container
 
       # If the ref appears to be a SHA, we're done, just split the string
-      return $~.captures if id =~ /^(\h{40})(.+)/
+      return $~.captures if id =~ /^(\h{40}\h{24}?)(.*)/
 
       # No slash means we must have a ref and no path
       return [id, ''] unless id.include?('/')
diff --git a/spec/lib/extracts_ref/ref_extractor_spec.rb b/spec/lib/extracts_ref/ref_extractor_spec.rb
index 23b283967cac77..38fe0d4ee121d1 100644
--- a/spec/lib/extracts_ref/ref_extractor_spec.rb
+++ b/spec/lib/extracts_ref/ref_extractor_spec.rb
@@ -20,6 +20,16 @@
     allow(container.repository).to receive(:ref_names).and_return(ref_names)
   end
 
+  describe '#initialize' do
+    let(:params) { { id: 1, ref: 2, path: 3, ref_type: 4 } }
+
+    it 'does not mutate provided params' do
+      ref_extractor
+
+      expect(params).to eq(id: 1, ref: 2, path: 3, ref_type: 4)
+    end
+  end
+
   describe '#extract_vars!' do
     it_behaves_like 'extracts ref vars'
 
diff --git a/spec/support/shared_examples/ref_extraction_shared_examples.rb b/spec/support/shared_examples/ref_extraction_shared_examples.rb
index f51c3a16406411..4be8beaedc3984 100644
--- a/spec/support/shared_examples/ref_extraction_shared_examples.rb
+++ b/spec/support/shared_examples/ref_extraction_shared_examples.rb
@@ -77,12 +77,18 @@
         expect(ref_extractor.extract_ref('v2.0.0')).to eq(['v2.0.0', ''])
       end
 
-      it 'extracts a valid commit ref' do
+      it 'extracts a valid commit SHA1 ref without a path' do
         expect(ref_extractor.extract_ref('f4b14494ef6abf3d144c28e4af0c20143383e062')).to eq(
           ['f4b14494ef6abf3d144c28e4af0c20143383e062', '']
         )
       end
 
+      it 'extracts a valid commit SHA256 ref without a path' do
+        expect(ref_extractor.extract_ref('34627760127d5ff2a644771225af09bbd79f28a54a0a4c03c1881bf2c26dc13c')).to eq(
+          ['34627760127d5ff2a644771225af09bbd79f28a54a0a4c03c1881bf2c26dc13c', '']
+        )
+      end
+
       it 'falls back to a primitive split for an invalid ref' do
         expect(ref_extractor.extract_ref('stable')).to eq(['stable', ''])
       end
-- 
GitLab