From c393d88df3b815bf6cbfad37ef20b8e70313563d Mon Sep 17 00:00:00 2001
From: Ahmad Sherif <me@ahmadsherif.com>
Date: Thu, 6 Jul 2017 20:34:25 +0200
Subject: [PATCH] Migrate Gitlab::Git::Repository#commit_count to Gitaly
Closes gitaly#355
---
GITALY_SERVER_VERSION | 2 +-
Gemfile | 2 +-
Gemfile.lock | 4 +-
lib/gitlab/git/repository.rb | 16 ++++--
lib/gitlab/gitaly_client/commit.rb | 9 +++
spec/lib/gitlab/git/repository_spec.rb | 79 ++++++++++++--------------
6 files changed, 59 insertions(+), 53 deletions(-)
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION
index a803cc227fe..a5510516948 100644
--- a/GITALY_SERVER_VERSION
+++ b/GITALY_SERVER_VERSION
@@ -1 +1 @@
-0.14.0
+0.15.0
diff --git a/Gemfile b/Gemfile
index 323827ed01b..0b4d712e905 100644
--- a/Gemfile
+++ b/Gemfile
@@ -386,7 +386,7 @@ gem 'vmstat', '~> 2.3.0'
gem 'sys-filesystem', '~> 1.1.6'
# Gitaly GRPC client
-gem 'gitaly', '~> 0.9.0'
+gem 'gitaly', '~> 0.13.0'
gem 'toml-rb', '~> 0.3.15', require: false
diff --git a/Gemfile.lock b/Gemfile.lock
index 430e1ba257b..68f8ad9af37 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -278,7 +278,7 @@ GEM
po_to_json (>= 1.0.0)
rails (>= 3.2.0)
gherkin-ruby (0.3.2)
- gitaly (0.9.0)
+ gitaly (0.13.0)
google-protobuf (~> 3.1)
grpc (~> 1.0)
github-linguist (4.7.6)
@@ -980,7 +980,7 @@ DEPENDENCIES
gettext (~> 3.2.2)
gettext_i18n_rails (~> 1.8.0)
gettext_i18n_rails_js (~> 1.2.0)
- gitaly (~> 0.9.0)
+ gitaly (~> 0.13.0)
github-linguist (~> 4.7.0)
gitlab-flowdock-git-hook (~> 1.0.1)
gitlab-markup (~> 1.5.1)
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index 23d0c8a9bdb..7c0c42c2437 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -565,11 +565,17 @@ module Gitlab
# Return total commits count accessible from passed ref
def commit_count(ref)
- walker = Rugged::Walker.new(rugged)
- walker.sorting(Rugged::SORT_TOPO | Rugged::SORT_REVERSE)
- oid = rugged.rev_parse_oid(ref)
- walker.push(oid)
- walker.count
+ gitaly_migrate(:commit_count) do |is_enabled|
+ if is_enabled
+ gitaly_commit_client.commit_count(ref)
+ else
+ walker = Rugged::Walker.new(rugged)
+ walker.sorting(Rugged::SORT_TOPO | Rugged::SORT_REVERSE)
+ oid = rugged.rev_parse_oid(ref)
+ walker.push(oid)
+ walker.count
+ end
+ end
end
# Sets HEAD to the commit specified by +ref+; +ref+ can be a branch or
diff --git a/lib/gitlab/gitaly_client/commit.rb b/lib/gitlab/gitaly_client/commit.rb
index b8877619797..aafc0520664 100644
--- a/lib/gitlab/gitaly_client/commit.rb
+++ b/lib/gitlab/gitaly_client/commit.rb
@@ -56,6 +56,15 @@ module Gitlab
entry
end
+ def commit_count(ref)
+ request = Gitaly::CountCommitsRequest.new(
+ repository: @gitaly_repo,
+ revision: ref
+ )
+
+ GitalyClient.call(@repository.storage, :commit_service, :count_commits, request).count
+ end
+
private
def commit_diff_request_params(commit, options = {})
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index f1cdd86edb9..ed76b1d3eff 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -3,6 +3,20 @@ require "spec_helper"
describe Gitlab::Git::Repository, seed_helper: true do
include Gitlab::EncodingHelper
+ shared_examples 'wrapping gRPC errors' do |gitaly_client_class, gitaly_client_method|
+ it 'wraps gRPC not found error' do
+ expect_any_instance_of(gitaly_client_class).to receive(gitaly_client_method)
+ .and_raise(GRPC::NotFound)
+ expect { subject }.to raise_error(Gitlab::Git::Repository::NoRepository)
+ end
+
+ it 'wraps gRPC unknown error' do
+ expect_any_instance_of(gitaly_client_class).to receive(gitaly_client_method)
+ .and_raise(GRPC::Unknown)
+ expect { subject }.to raise_error(Gitlab::Git::CommandError)
+ end
+ end
+
let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) }
describe "Respond to" do
@@ -35,16 +49,8 @@ describe Gitlab::Git::Repository, seed_helper: true do
repository.root_ref
end
- it 'wraps GRPC not found' do
- expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:default_branch_name)
- .and_raise(GRPC::NotFound)
- expect { repository.root_ref }.to raise_error(Gitlab::Git::Repository::NoRepository)
- end
-
- it 'wraps GRPC exceptions' do
- expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:default_branch_name)
- .and_raise(GRPC::Unknown)
- expect { repository.root_ref }.to raise_error(Gitlab::Git::CommandError)
+ it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::Ref, :default_branch_name do
+ subject { repository.root_ref }
end
end
@@ -130,17 +136,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
subject
end
- it 'wraps GRPC not found' do
- expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:branch_names)
- .and_raise(GRPC::NotFound)
- expect { subject }.to raise_error(Gitlab::Git::Repository::NoRepository)
- end
-
- it 'wraps GRPC other exceptions' do
- expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:branch_names)
- .and_raise(GRPC::Unknown)
- expect { subject }.to raise_error(Gitlab::Git::CommandError)
- end
+ it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::Ref, :branch_names
end
describe '#tag_names' do
@@ -168,17 +164,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
subject
end
- it 'wraps GRPC not found' do
- expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:tag_names)
- .and_raise(GRPC::NotFound)
- expect { subject }.to raise_error(Gitlab::Git::Repository::NoRepository)
- end
-
- it 'wraps GRPC exceptions' do
- expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:tag_names)
- .and_raise(GRPC::Unknown)
- expect { subject }.to raise_error(Gitlab::Git::CommandError)
- end
+ it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::Ref, :tag_names
end
shared_examples 'archive check' do |extenstion|
@@ -406,8 +392,21 @@ describe Gitlab::Git::Repository, seed_helper: true do
end
describe '#commit_count' do
- it { expect(repository.commit_count("master")).to eq(25) }
- it { expect(repository.commit_count("feature")).to eq(9) }
+ shared_examples 'counting commits' do
+ it { expect(repository.commit_count("master")).to eq(25) }
+ it { expect(repository.commit_count("feature")).to eq(9) }
+ end
+
+ context 'when Gitaly commit_count feature is enabled' do
+ it_behaves_like 'counting commits'
+ it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::Commit, :commit_count do
+ subject { repository.commit_count('master') }
+ end
+ end
+
+ context 'when Gitaly commit_count feature is disabled', skip_gitaly_mock: true do
+ it_behaves_like 'counting commits'
+ end
end
describe "#reset" do
@@ -1266,16 +1265,8 @@ describe Gitlab::Git::Repository, seed_helper: true do
@repo.local_branches
end
- it 'wraps GRPC not found' do
- expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:local_branches)
- .and_raise(GRPC::NotFound)
- expect { @repo.local_branches }.to raise_error(Gitlab::Git::Repository::NoRepository)
- end
-
- it 'wraps GRPC exceptions' do
- expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:local_branches)
- .and_raise(GRPC::Unknown)
- expect { @repo.local_branches }.to raise_error(Gitlab::Git::CommandError)
+ it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::Ref, :local_branches do
+ subject { @repo.local_branches }
end
end
--
GitLab