refactors branches finder code27379-branches-controller-index-is-slow

4 files changed, 47 insertions, 16 deletions

diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb
index c40f9b7f75f..fb16b5c4b22 100644
--- a/app/controllers/projects/branches_controller.rb
+++ b/app/controllers/projects/branches_controller.rb
@@ -2,15 +2,16 @@ class Projects::BranchesController < Projects::ApplicationController
   include ActionView::Helpers::SanitizeHelper
   include SortingHelper
 
+  before_action :finder, only: [:index]
+
   # Authorize
   before_action :require_non_empty_project, except: :create
   before_action :authorize_download_code!
   before_action :authorize_push_code!, only: [:new, :create, :destroy, :destroy_all_merged]
 
   def index
-    @sort = params[:sort].presence || sort_value_name
-    @branches = BranchesFinder.new(@repository, params).execute
-    @branches = Kaminari.paginate_array(@branches).page(params[:page])
+    @sort = finder.sort
+    @branches = Kaminari.paginate_array(finder.execute).page(params[:page])
 
     @max_commits = @branches.reduce(0) do |memo, branch|
       diverging_commit_counts = repository.diverging_commit_counts(branch)
@@ -81,6 +82,10 @@ class Projects::BranchesController < Projects::ApplicationController
 
   private
 
+  def finder
+    @finder ||= BranchesFinder.new(@repository, { sort: sort_value_name }.merge(params))
+  end
+
   def ref
     if params[:ref]
       ref_escaped = sanitize(strip_tags(params[:ref]))
diff --git a/app/finders/branches_finder.rb b/app/finders/branches_finder.rb
index 533076585c0..9fd32a23d64 100644
--- a/app/finders/branches_finder.rb
+++ b/app/finders/branches_finder.rb
@@ -5,8 +5,14 @@ class BranchesFinder
   end
 
   def execute
-    branches = @repository.branches_sorted_by(sort)
-    filter_by_name(branches)
+    rugged_branches = @repository.rugged_branches_sorted_by(sort)
+    rugged_branches = by_name(rugged_branches)
+
+    rugged_branches.map { |ref| Gitlab::Git::Branch.new(@repository.raw_repository, ref.name, ref.target) rescue nil }.compact
+  end
+
+  def sort
+    @params[:sort].presence || SortingHelper.sort_value_name
   end
 
   private
@@ -17,15 +23,8 @@ class BranchesFinder
     @params[:search].presence
   end
 
-  def sort
-    @params[:sort].presence || 'name'
-  end
-
-  def filter_by_name(branches)
-    if search
-      branches.select { |branch| branch.name.include?(search) }
-    else
-      branches
-    end
+  def by_name(branches)
+    return branches unless search
+    branches.select { |ref| ref.name.include?(search) }
   end
 end
diff --git a/app/models/repository.rb b/app/models/repository.rb
index e7cc8d6e083..5cf1ee928f4 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -670,6 +670,23 @@ class Repository
     end
   end
 
+  def sort_rugged_branches_by_updated
+    rugged_branches.sort { |a, b| a.target.committer[:time] <=> b.target.committer[:time] }
+  end
+
+  def rugged_branches_sorted_by(value)
+    case value
+    when 'name'
+      rugged_branches.sort_by(&:name)
+    when 'updated_desc'
+      sort_rugged_branches_by_updated.reverse
+    when 'updated_asc'
+      sort_rugged_branches_by_updated
+    else
+      rugged_branches
+    end
+  end
+
   def tags_sorted_by(value)
     case value
     when 'name'
@@ -741,6 +758,12 @@ class Repository
 
   alias_method :branches, :local_branches
 
+  def local_rugged_branches
+    @local_rugged_branches ||= raw_repository.local_rugged_branches
+  end
+
+  alias_method :rugged_branches, :local_rugged_branches
+
   def tags
     @tags ||= raw_repository.tags
   end
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index 6540730ca7a..f65ebaea667 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -86,8 +86,12 @@ module Gitlab
         Gitlab::Git::Branch.new(self, rugged_ref.name, rugged_ref.target) if rugged_ref
       end
 
+      def local_rugged_branches
+        rugged.branches.each(:local)
+      end
+
       def local_branches
-        rugged.branches.each(:local).map do |branch|
+        local_rugged_branches.map do |branch|
           Gitlab::Git::Branch.new(self, branch.name, branch.target)
         end
       end