diff options
-rw-r--r-- | app/controllers/projects/branches_controller.rb | 22 | ||||
-rw-r--r-- | app/finders/branches_finder.rb | 14 | ||||
-rw-r--r-- | app/models/repository.rb | 40 | ||||
-rw-r--r-- | app/services/branches/diverging_commit_counts_service.rb | 54 | ||||
-rw-r--r-- | app/views/projects/branches/_branch.html.haml | 2 | ||||
-rw-r--r-- | changelogs/unreleased/id-stale-branches.yml | 5 | ||||
-rw-r--r-- | config/routes/repository.rb | 5 | ||||
-rw-r--r-- | spec/controllers/projects/branches_controller_spec.rb | 23 | ||||
-rw-r--r-- | spec/finders/branches_finder_spec.rb | 9 | ||||
-rw-r--r-- | spec/models/repository_spec.rb | 42 | ||||
-rw-r--r-- | spec/services/branches/diverging_commit_counts_service_spec.rb | 52 |
11 files changed, 175 insertions, 93 deletions
diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb index fc708400657..da5efe4f21c 100644 --- a/app/controllers/projects/branches_controller.rb +++ b/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])) diff --git a/app/finders/branches_finder.rb b/app/finders/branches_finder.rb index 45d5591e81b..b462c8053fa 100644 --- a/app/finders/branches_finder.rb +++ b/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 diff --git a/app/models/repository.rb b/app/models/repository.rb index e05d3dd58ac..992ed7485e5 100644 --- a/app/models/repository.rb +++ b/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, diff --git a/app/services/branches/diverging_commit_counts_service.rb b/app/services/branches/diverging_commit_counts_service.rb new file mode 100644 index 00000000000..f947cec1663 --- /dev/null +++ b/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 diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml index 3638334d61c..f97b259f8f2 100644 --- a/app/views/projects/branches/_branch.html.haml +++ b/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] diff --git a/changelogs/unreleased/id-stale-branches.yml b/changelogs/unreleased/id-stale-branches.yml new file mode 100644 index 00000000000..2f35c5a12c9 --- /dev/null +++ b/changelogs/unreleased/id-stale-branches.yml @@ -0,0 +1,5 @@ +--- +title: Add endpoint for fetching diverging commit counts +merge_request: 29802 +author: +type: performance diff --git a/config/routes/repository.rb b/config/routes/repository.rb index 1ea0ae72614..b89e1c7f9af 100644 --- a/config/routes/repository.rb +++ b/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] diff --git a/spec/controllers/projects/branches_controller_spec.rb b/spec/controllers/projects/branches_controller_spec.rb index cf201c9f735..287ea9f425d 100644 --- a/spec/controllers/projects/branches_controller_spec.rb +++ b/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 diff --git a/spec/finders/branches_finder_spec.rb b/spec/finders/branches_finder_spec.rb index 7d164539d9a..3fc86f3e408 100644 --- a/spec/finders/branches_finder_spec.rb +++ b/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 diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 0bd17dbacd7..13da7bd7407 100644 --- a/spec/models/repository_spec.rb +++ b/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) diff --git a/spec/services/branches/diverging_commit_counts_service_spec.rb b/spec/services/branches/diverging_commit_counts_service_spec.rb new file mode 100644 index 00000000000..bfdbebdb7c1 --- /dev/null +++ b/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 |