Merge branch 'feature/migrate-count-commits-to-gitaly' into 'master'

Migrate Gitlab::Git::Repository#count_commits to Gitaly

Closes gitaly#415

See merge request !13121

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
 

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)

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)
@@ -1005,6 +1002,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

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

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