Merge branch 'id-remove-gitaly-request-from-project-show' into 'master'

Remove Gitaly request from projects#show

See merge request gitlab-org/gitlab!49565

app/controllers/projects_controller.rb

@@ -19,7 +19,6 @@ class ProjectsController < Projects::ApplicationController
   before_action :redirect_git_extension, only: [:show]
   before_action :project, except: [:index, :new, :create, :resolve]
   before_action :repository, except: [:index, :new, :create, :resolve]
-  before_action :assign_ref_vars, if: -> { action_name == 'show' && repo_exists? }
   before_action :project_export_enabled, only: [:export, :download_export, :remove_export, :generate_new_export]
   before_action :present_project, only: [:edit]
   before_action :authorize_download_code!, only: [:refs]
@@ -137,6 +136,8 @@ class ProjectsController < Projects::ApplicationController
   end
 
   def show
+    @id, @ref, @path = extract_ref_path
+
     if @project.import_in_progress?
       redirect_to project_import_path(@project, custom_import_params)
       return

changelogs/unreleased/id-remove-gitaly-request-from-project-show.yml

+---
+title: Remove unnecessary Gitaly calls from projects#show
+merge_request: 49565
+author:
+type: performance

lib/extracts_ref.rb

@@ -62,8 +62,7 @@ module ExtractsRef
   #
   # rubocop:disable Gitlab/ModuleWithInstanceVariables
   def assign_ref_vars
-    @id = get_id
-    @ref, @path = extract_ref(@id)
+    @id, @ref, @path = extract_ref_path
     @repo = repository_container.repository
 
     raise InvalidPathError if @ref.match?(/\s/)
@@ -76,6 +75,13 @@ module ExtractsRef
     @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
+
   private
 
   def extract_raw_ref(id)

spec/controllers/projects_controller_spec.rb

@@ -277,11 +277,16 @@ RSpec.describe ProjectsController do
 
       render_views
 
+      def get_show
+        get :show, params: { namespace_id: public_project.namespace, id: public_project }
+      end
+
       it "renders the activity view" do
         allow(controller).to receive(:current_user).and_return(user)
         allow(user).to receive(:project_view).and_return('activity')
 
-        get :show, params: { namespace_id: public_project.namespace, id: public_project }
+        get_show
+
         expect(response).to render_template('_activity')
       end
 
@@ -289,7 +294,8 @@ RSpec.describe ProjectsController do
         allow(controller).to receive(:current_user).and_return(user)
         allow(user).to receive(:project_view).and_return('files')
 
-        get :show, params: { namespace_id: public_project.namespace, id: public_project }
+        get_show
+
         expect(response).to render_template('_files')
       end
 
@@ -297,9 +303,18 @@ RSpec.describe ProjectsController do
         allow(controller).to receive(:current_user).and_return(user)
         allow(user).to receive(:project_view).and_return('readme')
 
-        get :show, params: { namespace_id: public_project.namespace, id: public_project }
+        get_show
+
         expect(response).to render_template('_readme')
       end
+
+      it 'does not make Gitaly requests', :request_store, :clean_gitlab_redis_cache do
+        # Warm up to populate repository cache
+        get_show
+        RequestStore.clear!
+
+        expect { get_show }.not_to change { Gitlab::GitalyClient.get_request_count }
+      end
     end
 
     context "when the url contains .atom" do