Merge branch 'restore-get_id' into 'master'

Restore get_id in ExtractsPath

fixes #21334

`Addressable::URI.normalize_component()` around `get_id` which was introduced by 68cea38e94886e69099a3faa0d1e2fbde2e71899 and removed in !5878 is still intentionally omitted. Using `normalize_component()` instead of `unescape()` left `%` as is but still broke e.g. spaces (which were replaced by `%20`).

See merge request !5974

3 files changed, 21 insertions, 11 deletions

diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 2a6385c1029..fc52cd2f367 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -5,7 +5,7 @@ class ProjectsController < Projects::ApplicationController
   before_action :project, except: [:new, :create]
   before_action :repository, except: [:new, :create]
   before_action :assign_ref_vars, only: [:show], if: :repo_exists?
-  before_action :assign_tree_vars, only: [:show], if: [:repo_exists?, :project_view_files?]
+  before_action :tree, only: [:show], if: [:repo_exists?, :project_view_files?]
 
   # Authorize
   before_action :authorize_admin_project!, only: [:edit, :update, :housekeeping, :download_export, :export, :remove_export, :generate_new_export]
@@ -332,11 +332,4 @@ class ProjectsController < Projects::ApplicationController
   def get_id
     project.repository.root_ref
   end
-
-  # ExtractsPath will set @id = project.path on the show route, but it has to be the
-  # branch name for the tree view to work correctly.
-  def assign_tree_vars
-    @id = get_id
-    tree
-  end
 end
diff --git a/lib/extracts_path.rb b/lib/extracts_path.rb
index a293fa2752f..a4558d157c0 100644
--- a/lib/extracts_path.rb
+++ b/lib/extracts_path.rb
@@ -94,9 +94,7 @@ module ExtractsPath
     @options = params.select {|key, value| allowed_options.include?(key) && !value.blank? }
     @options = HashWithIndifferentAccess.new(@options)
 
-    @id = params[:id] || params[:ref]
-    @id += "/" + params[:path] unless params[:path].blank?
-
+    @id = get_id
     @ref, @path = extract_ref(@id)
     @repo = @project.repository
     if @options[:extended_sha1].blank?
@@ -118,4 +116,13 @@ module ExtractsPath
   def tree
     @tree ||= @repo.tree(@commit.id, @path)
   end
+
+  private
+
+  # overriden in subclasses, do not remove
+  def get_id
+    id = params[:id] || params[:ref]
+    id += "/" + params[:path] unless params[:path].blank?
+    id
+  end
 end
diff --git a/spec/lib/extracts_path_spec.rb b/spec/lib/extracts_path_spec.rb
index 86d04ecfa36..e10c1f5c547 100644
--- a/spec/lib/extracts_path_spec.rb
+++ b/spec/lib/extracts_path_spec.rb
@@ -51,6 +51,16 @@ describe ExtractsPath, lib: true do
         expect(@path).to eq(params[:path])
       end
     end
+
+    context 'subclass overrides get_id' do
+      it 'uses ref returned by get_id' do
+        allow_any_instance_of(self.class).to receive(:get_id){ '38008cb17ce1466d8fec2dfa6f6ab8dcfe5cf49e' }
+
+        assign_ref_vars
+
+        expect(@id).to eq(get_id)
+      end
+    end
   end
 
   describe '#extract_ref' do