Add endpoint for fetching diverging commit counts
Extract diverging_commit_counts into a service class
This commit is contained in:
parent
546355f734
commit
ca5cd7b7fb
11 changed files with 175 additions and 93 deletions
|
@ -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]))
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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,
|
||||
|
|
54
app/services/branches/diverging_commit_counts_service.rb
Normal file
54
app/services/branches/diverging_commit_counts_service.rb
Normal file
|
@ -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
|
|
@ -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]
|
||||
|
|
5
changelogs/unreleased/id-stale-branches.yml
Normal file
5
changelogs/unreleased/id-stale-branches.yml
Normal file
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Add endpoint for fetching diverging commit counts
|
||||
merge_request: 29802
|
||||
author:
|
||||
type: performance
|
|
@ -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]
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -2285,48 +2285,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)
|
||||
|
|
|
@ -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
|
Loading…
Reference in a new issue