summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorIgor Drozdov <idrozdov@gitlab.com>2019-06-18 17:20:11 +0300
committerIgor Drozdov <idrozdov@gitlab.com>2019-06-28 16:22:35 +0300
commitca5cd7b7fb5108d30d0f6b74e31da736024592dd (patch)
treeaec9468afa579b128b7e09f6e78e7357d6413b85
parent546355f734f74c040d0ef0917ade50751fd90731 (diff)
downloadgitlab-ce-ca5cd7b7fb5108d30d0f6b74e31da736024592dd.tar.gz
Add endpoint for fetching diverging commit countsid-stale-branches
Extract diverging_commit_counts into a service class
-rw-r--r--app/controllers/projects/branches_controller.rb22
-rw-r--r--app/finders/branches_finder.rb14
-rw-r--r--app/models/repository.rb40
-rw-r--r--app/services/branches/diverging_commit_counts_service.rb54
-rw-r--r--app/views/projects/branches/_branch.html.haml2
-rw-r--r--changelogs/unreleased/id-stale-branches.yml5
-rw-r--r--config/routes/repository.rb5
-rw-r--r--spec/controllers/projects/branches_controller_spec.rb23
-rw-r--r--spec/finders/branches_finder_spec.rb9
-rw-r--r--spec/models/repository_spec.rb42
-rw-r--r--spec/services/branches/diverging_commit_counts_service_spec.rb52
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 b96315bfe8b..8220b29a401 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 c5ab7e57272..0b5de185ae5 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -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)
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