Merge branch 'id-stale-branches' into 'master'

Add endpoint for fetching diverging commit counts

See merge request gitlab-org/gitlab-ce!29802

app/controllers/projects/branches_controller.rb

@@ -25,15 +25,6 @@ class Projects::BranchesController < Projects::ApplicationController
         @refs_pipelines = @project.ci_pipelines.latest_successful_for_refs(@branches.map(&:name))
         @merged_branch_names = repository.merged_branch_names(@branches.map(&:name))
 
-        # n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/48097
-        Gitlab::GitalyClient.allow_n_plus_1_calls do
-          @max_commits = @branches.reduce(0) do |memo, branch|
-            diverging_commit_counts = repository.diverging_commit_counts(branch)
-            [memo, diverging_commit_counts.values_at(:behind, :ahead, :distance)]
-              .flatten.compact.max
-          end
-        end
-
         # https://gitlab.com/gitlab-org/gitlab-ce/issues/48097
         Gitlab::GitalyClient.allow_n_plus_1_calls do
           render
@@ -51,6 +42,19 @@ class Projects::BranchesController < Projects::ApplicationController
     @branches = @repository.recent_branches
   end
 
+  def diverging_commit_counts
+    respond_to do |format|
+      format.json do
+        service = Branches::DivergingCommitCountsService.new(repository)
+        branches = BranchesFinder.new(repository, params.permit(names: [])).execute
+
+        Gitlab::GitalyClient.allow_n_plus_1_calls do
+          render json: branches.to_h { |branch| [branch.name, service.call(branch)] }
+        end
+      end
+    end
+  end
+
   # rubocop: disable CodeReuse/ActiveRecord
   def create
     branch_name = strip_tags(sanitize(params[:branch_name]))

app/finders/branches_finder.rb

@@ -9,6 +9,7 @@ class BranchesFinder
   def execute
     branches = repository.branches_sorted_by(sort)
     branches = by_search(branches)
+    branches = by_names(branches)
     branches
   end
 
@@ -16,6 +17,10 @@ class BranchesFinder
 
   attr_reader :repository, :params
 
+  def names
+    @params[:names].presence
+  end
+
   def search
     @params[:search].presence
   end
@@ -59,4 +64,13 @@ class BranchesFinder
   def find_exact_match_index(matches, term)
     matches.index { |branch| branch.name.casecmp(term) == 0 }
   end
+
+  def by_names(branches)
+    return branches unless names
+
+    branch_names = names.to_set
+    branches.filter do |branch|
+      branch_names.include?(branch.name)
+    end
+  end
 end

app/models/repository.rb

@@ -282,46 +282,6 @@ class Repository
     ref_exists?(keep_around_ref_name(sha))
   end
 
-  def diverging_commit_counts(branch)
-    return diverging_commit_counts_without_max(branch) if Feature.enabled?('gitaly_count_diverging_commits_no_max')
-
-    ## TODO: deprecate the below code after 12.0
-    @root_ref_hash ||= raw_repository.commit(root_ref).id
-    cache.fetch(:"diverging_commit_counts_#{branch.name}") do
-      # Rugged seems to throw a `ReferenceError` when given branch_names rather
-      # than SHA-1 hashes
-      branch_sha = branch.dereferenced_target.sha
-
-      number_commits_behind, number_commits_ahead =
-        raw_repository.diverging_commit_count(
-          @root_ref_hash,
-          branch_sha,
-          max_count: MAX_DIVERGING_COUNT)
-
-      if number_commits_behind + number_commits_ahead >= MAX_DIVERGING_COUNT
-        { distance: MAX_DIVERGING_COUNT }
-      else
-        { behind: number_commits_behind, ahead: number_commits_ahead }
-      end
-    end
-  end
-
-  def diverging_commit_counts_without_max(branch)
-    @root_ref_hash ||= raw_repository.commit(root_ref).id
-    cache.fetch(:"diverging_commit_counts_without_max_#{branch.name}") do
-      # Rugged seems to throw a `ReferenceError` when given branch_names rather
-      # than SHA-1 hashes
-      branch_sha = branch.dereferenced_target.sha
-
-      number_commits_behind, number_commits_ahead =
-        raw_repository.diverging_commit_count(
-          @root_ref_hash,
-          branch_sha)
-
-      { behind: number_commits_behind, ahead: number_commits_ahead }
-    end
-  end
-
   def archive_metadata(ref, storage_path, format = "tar.gz", append_sha:, path: nil)
     raw_repository.archive_metadata(
       ref,

app/services/branches/diverging_commit_counts_service.rb

@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+
+module Branches
+  class DivergingCommitCountsService
+    def initialize(repository)
+      @repository = repository
+      @cache = Gitlab::RepositoryCache.new(repository)
+    end
+
+    def call(branch)
+      if Feature.enabled?('gitaly_count_diverging_commits_no_max')
+        diverging_commit_counts_without_max(branch)
+      else
+        diverging_commit_counts(branch)
+      end
+    end
+
+    private
+
+    attr_reader :repository, :cache
+
+    delegate :raw_repository, to: :repository
+
+    def diverging_commit_counts(branch)
+      ## TODO: deprecate the below code after 12.0
+      @root_ref_hash ||= raw_repository.commit(repository.root_ref).id
+      cache.fetch(:"diverging_commit_counts_#{branch.name}") do
+        number_commits_behind, number_commits_ahead =
+          repository.raw_repository.diverging_commit_count(
+            @root_ref_hash,
+            branch.dereferenced_target.sha,
+            max_count: Repository::MAX_DIVERGING_COUNT)
+
+        if number_commits_behind + number_commits_ahead >= Repository::MAX_DIVERGING_COUNT
+          { distance: Repository::MAX_DIVERGING_COUNT }
+        else
+          { behind: number_commits_behind, ahead: number_commits_ahead }
+        end
+      end
+    end
+
+    def diverging_commit_counts_without_max(branch)
+      @root_ref_hash ||= raw_repository.commit(repository.root_ref).id
+      cache.fetch(:"diverging_commit_counts_without_max_#{branch.name}") do
+        number_commits_behind, number_commits_ahead =
+          raw_repository.diverging_commit_count(
+            @root_ref_hash,
+            branch.dereferenced_target.sha)
+
+        { behind: number_commits_behind, ahead: number_commits_ahead }
+      end
+    end
+  end
+end

app/views/projects/branches/_branch.html.haml

@@ -1,6 +1,6 @@
 - merged = local_assigns.fetch(:merged, false)
 - commit = @repository.commit(branch.dereferenced_target)
-- diverging_commit_counts = @repository.diverging_commit_counts(branch)
+- diverging_commit_counts = Branches::DivergingCommitCountsService.new(@repository).call(branch)
 - number_commits_distance = diverging_commit_counts[:distance]
 - number_commits_behind = diverging_commit_counts[:behind]
 - number_commits_ahead = diverging_commit_counts[:ahead]

changelogs/unreleased/id-stale-branches.yml

@@ -0,0 +1,5 @@
+---
+title: Add endpoint for fetching diverging commit counts
+merge_request: 29802
+author:
+type: performance

config/routes/repository.rb

@@ -52,7 +52,10 @@ scope format: false do
     end
 
     get '/branches/:state', to: 'branches#index', as: :branches_filtered, constraints: { state: /active|stale|all/ }
-    resources :branches, only: [:index, :new, :create, :destroy]
+    resources :branches, only: [:index, :new, :create, :destroy] do
+      get :diverging_commit_counts, on: :collection
+    end
+
     delete :merged_branches, controller: 'branches', action: :destroy_all_merged
     resources :tags, only: [:index, :show, :new, :create, :destroy] do
       resource :release, controller: 'tags/releases', only: [:edit, :update]

spec/controllers/projects/branches_controller_spec.rb

@@ -507,4 +507,27 @@ describe Projects::BranchesController do
       end
     end
   end
+
+  describe 'GET diverging_commit_counts' do
+    before do
+      sign_in(user)
+
+      get :diverging_commit_counts,
+          format: :json,
+          params: {
+            namespace_id: project.namespace,
+            project_id: project,
+            names: ['fix', 'add-pdf-file', 'branch-merged']
+          }
+    end
+
+    it 'returns the commit counts behind and ahead of default branch' do
+      parsed_response = JSON.parse(response.body)
+      expect(parsed_response).to eq(
+        "fix" => { "behind" => 29, "ahead" => 2 },
+        "branch-merged" => { "behind" => 1, "ahead" => 0 },
+        "add-pdf-file" => { "behind" => 0, "ahead" => 3 }
+      )
+    end
+  end
 end

spec/finders/branches_finder_spec.rb

@@ -62,6 +62,15 @@ describe BranchesFinder do
 
         expect(result.count).to eq(0)
       end
+
+      it 'filters branches by provided names' do
+        branches_finder = described_class.new(repository, { names: ['fix', 'csv', 'lfs', 'does-not-exist'] })
+
+        result = branches_finder.execute
+
+        expect(result.count).to eq(3)
+        expect(result.map(&:name)).to eq(%w{csv fix lfs})
+      end
     end
 
     context 'filter and sort' do

spec/models/repository_spec.rb

@@ -2298,48 +2298,6 @@ describe Repository do
     end
   end
 
-  describe '#diverging_commit_counts' do
-    let(:diverged_branch) { repository.find_branch('fix') }
-    let(:root_ref_sha) { repository.raw_repository.commit(repository.root_ref).id }
-    let(:diverged_branch_sha) { diverged_branch.dereferenced_target.sha }
-
-    it 'returns the commit counts behind and ahead of default branch' do
-      result = repository.diverging_commit_counts(diverged_branch)
-
-      expect(result).to eq(behind: 29, ahead: 2)
-    end
-
-    context 'when gitaly_count_diverging_commits_no_max is enabled' do
-      before do
-        stub_feature_flags(gitaly_count_diverging_commits_no_max: true)
-      end
-
-      it 'calls diverging_commit_count without max count' do
-        expect(repository.raw_repository)
-          .to receive(:diverging_commit_count)
-          .with(root_ref_sha, diverged_branch_sha)
-          .and_return([29, 2])
-
-        repository.diverging_commit_counts(diverged_branch)
-      end
-    end
-
-    context 'when gitaly_count_diverging_commits_no_max is disabled' do
-      before do
-        stub_feature_flags(gitaly_count_diverging_commits_no_max: false)
-      end
-
-      it 'calls diverging_commit_count with max count' do
-        expect(repository.raw_repository)
-          .to receive(:diverging_commit_count)
-          .with(root_ref_sha, diverged_branch_sha, max_count: Repository::MAX_DIVERGING_COUNT)
-          .and_return([29, 2])
-
-        repository.diverging_commit_counts(diverged_branch)
-      end
-    end
-  end
-
   describe '#refresh_method_caches' do
     it 'refreshes the caches of the given types' do
       expect(repository).to receive(:expire_method_caches)

spec/services/branches/diverging_commit_counts_service_spec.rb

@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Branches::DivergingCommitCountsService do
+  let(:project) { create(:project, :repository) }
+  let(:repository) { project.repository }
+
+  describe '#call' do
+    let(:diverged_branch) { repository.find_branch('fix') }
+    let(:root_ref_sha) { repository.raw_repository.commit(repository.root_ref).id }
+    let(:diverged_branch_sha) { diverged_branch.dereferenced_target.sha }
+
+    let(:service) { described_class.new(repository) }
+
+    it 'returns the commit counts behind and ahead of default branch' do
+      result = service.call(diverged_branch)
+
+      expect(result).to eq(behind: 29, ahead: 2)
+    end
+
+    context 'when gitaly_count_diverging_commits_no_max is enabled' do
+      before do
+        stub_feature_flags(gitaly_count_diverging_commits_no_max: true)
+      end
+
+      it 'calls diverging_commit_count without max count' do
+        expect(repository.raw_repository)
+          .to receive(:diverging_commit_count)
+          .with(root_ref_sha, diverged_branch_sha)
+          .and_return([29, 2])
+
+        service.call(diverged_branch)
+      end
+    end
+
+    context 'when gitaly_count_diverging_commits_no_max is disabled' do
+      before do
+        stub_feature_flags(gitaly_count_diverging_commits_no_max: false)
+      end
+
+      it 'calls diverging_commit_count with max count' do
+        expect(repository.raw_repository)
+          .to receive(:diverging_commit_count)
+          .with(root_ref_sha, diverged_branch_sha, max_count: Repository::MAX_DIVERGING_COUNT)
+          .and_return([29, 2])
+
+        service.call(diverged_branch)
+      end
+    end
+  end
+end