Merge branch '18709-reduce-git-calls' into 'master'

Remove calls to Rugged::BranchCollection#each from extracts_path before_action

See merge request !4802

8 files changed, 19 insertions, 12 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 59b5d6f8e9b..e2d65c632dc 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -135,6 +135,7 @@ v 8.9.0 (unreleased)
   - Use Git cached counters for branches and tags on project page
   - Filter parameters for request_uri value on instrumented transactions.
   - Remove duplicated keys add UNIQUE index to keys fingerprint column
+  - ExtractsPath get ref_names from repository cache, if not there access git.
   - Cache user todo counts from TodoService
   - Ensure Todos counters doesn't count Todos for projects pending delete
 
diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb
index 776ba92c9ab..996909a28c6 100644
--- a/app/controllers/projects/application_controller.rb
+++ b/app/controllers/projects/application_controller.rb
@@ -74,7 +74,7 @@ class Projects::ApplicationController < ApplicationController
   end
 
   def require_branch_head
-    unless @repository.branch_names.include?(@ref)
+    unless @repository.branch_exists?(@ref)
       redirect_to(
         namespace_project_tree_path(@project.namespace, @project, @ref),
         notice: "This action is not allowed unless you are on a branch"
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 78ceaf3237f..2b1f50fd01e 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -303,8 +303,14 @@ class ProjectsController < Projects::ApplicationController
     project.repository_exists? && !project.empty_repo?
   end
 
-  # Override get_id from ExtractsPath, which returns the branch and file path
+  # Override extract_ref from ExtractsPath, which returns the branch and file path
   # for the blob/tree, which in this case is just the root of the default branch.
+  # This way we avoid to access the repository.ref_names.
+  def extract_ref(_id)
+    [get_id, '']
+  end
+
+  # Override get_id from ExtractsPath in this case is just the root of the default branch.
   def get_id
     project.repository.root_ref
   end
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 82421d74de9..41859841834 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -116,7 +116,7 @@ module ApplicationHelper
     return false if project.merge_requests.where(source_branch: event.branch_name).opened.any?
 
     # Skip if user removed branch right after that
-    return false unless project.repository.branch_names.include?(event.branch_name)
+    return false unless project.repository.branch_exists?(event.branch_name)
 
     true
   end
diff --git a/app/helpers/branches_helper.rb b/app/helpers/branches_helper.rb
index 3ee3fc74f0c..c533659b600 100644
--- a/app/helpers/branches_helper.rb
+++ b/app/helpers/branches_helper.rb
@@ -10,7 +10,7 @@ module BranchesHelper
   end
 
   def can_push_branch?(project, branch_name)
-    return false unless project.repository.branch_names.include?(branch_name)
+    return false unless project.repository.branch_exists?(branch_name)
 
     ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(branch_name)
   end
diff --git a/app/models/repository.rb b/app/models/repository.rb
index bbd7682d8e7..221c87164ca 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -191,8 +191,12 @@ class Repository
     end
   end
 
+  def ref_names
+    branch_names + tag_names
+  end
+
   def branch_names
-    cache.fetch(:branch_names) { branches.map(&:name) }
+    @branch_names ||= cache.fetch(:branch_names) { branches.map(&:name) }
   end
 
   def branch_exists?(branch_name)
@@ -267,6 +271,7 @@ class Repository
 
   def expire_branches_cache
     cache.expire(:branch_names)
+    @branch_names = nil
     @local_branches = nil
   end
 
@@ -332,10 +337,6 @@ class Repository
     @lookup_cache ||= {}
   end
 
-  def expire_branch_names
-    cache.expire(:branch_names)
-  end
-
   def expire_avatar_cache(branch_name = nil, revision = nil)
     # Avatars are pulled from the default branch, thus if somebody pushes to a
     # different branch there's no need to expire anything.
diff --git a/app/models/user.rb b/app/models/user.rb
index 2e458329cb9..876ccc69d8d 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -487,9 +487,8 @@ class User < ActiveRecord::Base
     events.recent.find do |event|
       project = Project.find_by_id(event.project_id)
       next unless project
-      repo = project.repository
 
-      if repo.branch_names.include?(event.branch_name)
+      if project.repository.branch_exists?(event.branch_name)
         merge_requests = MergeRequest.where("created_at >= ?", event.created_at).
             where(source_project_id: project.id,
                   source_branch: event.branch_name)
diff --git a/spec/workers/merge_worker_spec.rb b/spec/workers/merge_worker_spec.rb
index 1abd87d7d33..b5e1fdb8ded 100644
--- a/spec/workers/merge_worker_spec.rb
+++ b/spec/workers/merge_worker_spec.rb
@@ -9,7 +9,7 @@ describe MergeWorker do
 
     before do
       source_project.team << [author, :master]
-      source_project.repository.expire_branch_names
+      source_project.repository.expire_branches_cache
     end
 
     it 'clears cache of source repo after removing source branch' do