Migrate Gitlab::Git::Repository#count_commits to Gitalyfeature/migrate-count-commits-to-gitaly

Closes gitaly#415

5 files changed, 57 insertions, 31 deletions

diff --git a/Gemfile b/Gemfile
index a9a1cbc144d..403b104a9d6 100644
--- a/Gemfile
+++ b/Gemfile
@@ -391,7 +391,7 @@ gem 'vmstat', '~> 2.3.0'
 gem 'sys-filesystem', '~> 1.1.6'
 
 # Gitaly GRPC client
-gem 'gitaly', '~> 0.21.0'
+gem 'gitaly', '~> 0.23.0'
 
 gem 'toml-rb', '~> 0.3.15', require: false
 
diff --git a/Gemfile.lock b/Gemfile.lock
index 5a327a14c4a..9f90965a567 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -269,7 +269,7 @@ GEM
       po_to_json (>= 1.0.0)
       rails (>= 3.2.0)
     gherkin-ruby (0.3.2)
-    gitaly (0.21.0)
+    gitaly (0.23.0)
       google-protobuf (~> 3.1)
       grpc (~> 1.0)
     github-linguist (4.7.6)
@@ -978,7 +978,7 @@ DEPENDENCIES
   gettext (~> 3.2.2)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.2.0)
-  gitaly (~> 0.21.0)
+  gitaly (~> 0.23.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 88529ba2c47..d79589dfca5 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -300,17 +300,14 @@ module Gitlab
         raw_log(options).map { |c| Commit.decorate(c) }
       end
 
-      # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/382
       def count_commits(options)
-        cmd = %W[#{Gitlab.config.git.bin_path} --git-dir=#{path} rev-list]
-        cmd << "--after=#{options[:after].iso8601}" if options[:after]
-        cmd << "--before=#{options[:before].iso8601}" if options[:before]
-        cmd += %W[--count #{options[:ref]}]
-        cmd += %W[-- #{options[:path]}] if options[:path].present?
-
-        raw_output = IO.popen(cmd) { |io| io.read }
-
-        raw_output.to_i
+        gitaly_migrate(:count_commits) do |is_enabled|
+          if is_enabled
+            count_commits_by_gitaly(options)
+          else
+            count_commits_by_shelling_out(options)
+          end
+        end
       end
 
       def sha_from_ref(ref)
@@ -998,6 +995,22 @@ module Gitlab
         gitaly_ref_client.tags
       end
 
+      def count_commits_by_gitaly(options)
+        gitaly_commit_client.commit_count(options[:ref], options)
+      end
+
+      def count_commits_by_shelling_out(options)
+        cmd = %W[#{Gitlab.config.git.bin_path} --git-dir=#{path} rev-list]
+        cmd << "--after=#{options[:after].iso8601}" if options[:after]
+        cmd << "--before=#{options[:before].iso8601}" if options[:before]
+        cmd += %W[--count #{options[:ref]}]
+        cmd += %W[-- #{options[:path]}] if options[:path].present?
+
+        raw_output = IO.popen(cmd) { |io| io.read }
+
+        raw_output.to_i
+      end
+
       def gitaly_migrate(method, &block)
         Gitlab::GitalyClient.migrate(method, &block)
       rescue GRPC::NotFound => e
diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb
index a834781b1f1..b1424a458e9 100644
--- a/lib/gitlab/gitaly_client/commit_service.rb
+++ b/lib/gitlab/gitaly_client/commit_service.rb
@@ -85,11 +85,14 @@ module Gitlab
         end
       end
 
-      def commit_count(ref)
+      def commit_count(ref, options = {})
         request = Gitaly::CountCommitsRequest.new(
           repository: @gitaly_repo,
           revision: ref
         )
+        request.after = Google::Protobuf::Timestamp.new(seconds: options[:after].to_i) if options[:after].present?
+        request.before = Google::Protobuf::Timestamp.new(seconds: options[:before].to_i) if options[:before].present?
+        request.path = options[:path] if options[:path].present?
 
         GitalyClient.call(@repository.storage, :commit_service, :count_commits, request).count
       end
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index 8e4a1f31ced..9bfad0c9bdf 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -361,20 +361,20 @@ describe Gitlab::Git::Repository, seed_helper: true do
   end
 
   describe '#commit_count' do
-    shared_examples 'counting commits' do
+    shared_examples 'simple commit counting' 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 'simple commit counting'
       it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::CommitService, :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'
+      it_behaves_like 'simple commit counting'
     end
   end
 
@@ -797,29 +797,39 @@ describe Gitlab::Git::Repository, seed_helper: true do
   end
 
   describe '#count_commits' do
-    context 'with after timestamp' do
-      it 'returns the number of commits after timestamp' do
-        options = { ref: 'master', limit: nil, after: Time.iso8601('2013-03-03T20:15:01+00:00') }
+    shared_examples 'extended commit counting' do
+      context 'with after timestamp' do
+        it 'returns the number of commits after timestamp' do
+          options = { ref: 'master', limit: nil, after: Time.iso8601('2013-03-03T20:15:01+00:00') }
 
-        expect(repository.count_commits(options)).to eq(25)
+          expect(repository.count_commits(options)).to eq(25)
+        end
       end
-    end
 
-    context 'with before timestamp' do
-      it 'returns the number of commits after timestamp' do
-        options = { ref: 'feature', limit: nil, before: Time.iso8601('2015-03-03T20:15:01+00:00') }
+      context 'with before timestamp' do
+        it 'returns the number of commits before timestamp' do
+          options = { ref: 'feature', limit: nil, before: Time.iso8601('2015-03-03T20:15:01+00:00') }
 
-        expect(repository.count_commits(options)).to eq(9)
+          expect(repository.count_commits(options)).to eq(9)
+        end
       end
-    end
 
-    context 'with path' do
-      it 'returns the number of commits with path ' do
-        options = { ref: 'master', limit: nil, path: "encoding" }
+      context 'with path' do
+        it 'returns the number of commits with path ' do
+          options = { ref: 'master', limit: nil, path: "encoding" }
 
-        expect(repository.count_commits(options)).to eq(2)
+          expect(repository.count_commits(options)).to eq(2)
+        end
       end
     end
+
+    context 'when Gitaly count_commits feature is enabled' do
+      it_behaves_like 'extended commit counting'
+    end
+
+    context 'when Gitaly count_commits feature is disabled', skip_gitaly_mock: true do
+      it_behaves_like 'extended commit counting'
+    end
   end
 
   describe "branch_names_contains" do