Migrate blame loading to Gitalyfeature/migrate-load-blame-to-gitaly

Closes gitaly#421

7 files changed, 97 insertions, 64 deletions

diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION
index 4e8f395fa5e..1b58cc10180 100644
--- a/GITALY_SERVER_VERSION
+++ b/GITALY_SERVER_VERSION
@@ -1 +1 @@
-0.26.0
+0.27.0
diff --git a/Gemfile b/Gemfile
index 403b104a9d6..34f231a04b8 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.23.0'
+gem 'gitaly', '~> 0.24.0'
 
 gem 'toml-rb', '~> 0.3.15', require: false
 
diff --git a/Gemfile.lock b/Gemfile.lock
index 9f90965a567..a17aa3ee1ed 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.23.0)
+    gitaly (0.24.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.23.0)
+  gitaly (~> 0.24.0)
   github-linguist (~> 4.7.0)
   gitlab-flowdock-git-hook (~> 1.0.1)
   gitlab-markup (~> 1.5.1)
diff --git a/lib/gitlab/git/blame.rb b/lib/gitlab/git/blame.rb
index 0deaab01b5b..8dbe25e55f6 100644
--- a/lib/gitlab/git/blame.rb
+++ b/lib/gitlab/git/blame.rb
@@ -1,5 +1,3 @@
-# Gitaly note: JV: needs 1 RPC for #load_blame.
-
 module Gitlab
   module Git
     class Blame
@@ -26,15 +24,29 @@ module Gitlab
 
       private
 
-      # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/376
       def load_blame
-        cmd = %W(#{Gitlab.config.git.bin_path} --git-dir=#{@repo.path} blame -p #{@sha} -- #{@path})
-        # Read in binary mode to ensure ASCII-8BIT
-        raw_output = IO.popen(cmd, 'rb') {|io| io.read }
+        raw_output = @repo.gitaly_migrate(:blame) do |is_enabled|
+          if is_enabled
+            load_blame_by_gitaly
+          else
+            load_blame_by_shelling_out
+          end
+        end
+
         output = encode_utf8(raw_output)
         process_raw_blame output
       end
 
+      def load_blame_by_gitaly
+        @repo.gitaly_commit_client.raw_blame(@sha, @path)
+      end
+
+      def load_blame_by_shelling_out
+        cmd = %W(#{Gitlab.config.git.bin_path} --git-dir=#{@repo.path} blame -p #{@sha} -- #{@path})
+        # Read in binary mode to ensure ASCII-8BIT
+        IO.popen(cmd, 'rb') {|io| io.read }
+      end
+
       def process_raw_blame(output)
         lines, final = [], []
         info, commits = {}, {}
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index ffe2c8b91bb..1c3beb5e834 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -683,6 +683,14 @@ module Gitlab
         @gitaly_repository_client ||= Gitlab::GitalyClient::RepositoryService.new(self)
       end
 
+      def gitaly_migrate(method, &block)
+        Gitlab::GitalyClient.migrate(method, &block)
+      rescue GRPC::NotFound => e
+        raise NoRepository.new(e)
+      rescue GRPC::BadStatus => e
+        raise CommandError.new(e)
+      end
+
       private
 
       # Gitaly note: JV: Trying to get rid of the 'filter' option so we can implement this with 'git'.
@@ -1017,14 +1025,6 @@ module Gitlab
 
         raw_output.to_i
       end
-
-      def gitaly_migrate(method, &block)
-        Gitlab::GitalyClient.migrate(method, &block)
-      rescue GRPC::NotFound => e
-        raise NoRepository.new(e)
-      rescue GRPC::BadStatus => e
-        raise CommandError.new(e)
-      end
     end
   end
 end
diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb
index b1424a458e9..1ae13677b42 100644
--- a/lib/gitlab/gitaly_client/commit_service.rb
+++ b/lib/gitlab/gitaly_client/commit_service.rb
@@ -128,6 +128,17 @@ module Gitlab
         response.languages.map { |l| { value: l.share.round(2), label: l.name, color: l.color, highlight: l.color } }
       end
 
+      def raw_blame(revision, path)
+        request = Gitaly::RawBlameRequest.new(
+          repository: @gitaly_repo,
+          revision: revision,
+          path: path
+        )
+
+        response = GitalyClient.call(@repository.storage, :commit_service, :raw_blame, request)
+        response.reduce("") { |memo, msg| memo << msg.data }
+      end
+
       private
 
       def commit_diff_request_params(commit, options = {})
diff --git a/spec/lib/gitlab/git/blame_spec.rb b/spec/lib/gitlab/git/blame_spec.rb
index 66c016d14b3..800c245b130 100644
--- a/spec/lib/gitlab/git/blame_spec.rb
+++ b/spec/lib/gitlab/git/blame_spec.rb
@@ -7,63 +7,73 @@ describe Gitlab::Git::Blame, seed_helper: true do
     Gitlab::Git::Blame.new(repository, SeedRepo::Commit::ID, "CONTRIBUTING.md")
   end
 
-  context "each count" do
-    it do
-      data = []
-      blame.each do |commit, line|
-        data << {
-          commit: commit,
-          line: line
-        }
-      end
-
-      expect(data.size).to eq(95)
-      expect(data.first[:commit]).to be_kind_of(Gitlab::Git::Commit)
-      expect(data.first[:line]).to eq("# Contribute to GitLab")
-      expect(data.first[:line]).to be_utf8
-    end
-  end
+  shared_examples 'blaming a file' do
+    context "each count" do
+      it do
+        data = []
+        blame.each do |commit, line|
+          data << {
+            commit: commit,
+            line: line
+          }
+        end
 
-  context "ISO-8859 encoding" do
-    let(:blame) do
-      Gitlab::Git::Blame.new(repository, SeedRepo::EncodingCommit::ID, "encoding/iso8859.txt")
+        expect(data.size).to eq(95)
+        expect(data.first[:commit]).to be_kind_of(Gitlab::Git::Commit)
+        expect(data.first[:line]).to eq("# Contribute to GitLab")
+        expect(data.first[:line]).to be_utf8
+      end
     end
 
-    it 'converts to UTF-8' do
-      data = []
-      blame.each do |commit, line|
-        data << {
-          commit: commit,
-          line: line
-        }
+    context "ISO-8859 encoding" do
+      let(:blame) do
+        Gitlab::Git::Blame.new(repository, SeedRepo::EncodingCommit::ID, "encoding/iso8859.txt")
       end
 
-      expect(data.size).to eq(1)
-      expect(data.first[:commit]).to be_kind_of(Gitlab::Git::Commit)
-      expect(data.first[:line]).to eq("Ä ü")
-      expect(data.first[:line]).to be_utf8
-    end
-  end
+      it 'converts to UTF-8' do
+        data = []
+        blame.each do |commit, line|
+          data << {
+            commit: commit,
+            line: line
+          }
+        end
 
-  context "unknown encoding" do
-    let(:blame) do
-      Gitlab::Git::Blame.new(repository, SeedRepo::EncodingCommit::ID, "encoding/iso8859.txt")
+        expect(data.size).to eq(1)
+        expect(data.first[:commit]).to be_kind_of(Gitlab::Git::Commit)
+        expect(data.first[:line]).to eq("Ä ü")
+        expect(data.first[:line]).to be_utf8
+      end
     end
 
-    it 'converts to UTF-8' do
-      expect(CharlockHolmes::EncodingDetector).to receive(:detect).and_return(nil)
-      data = []
-      blame.each do |commit, line|
-        data << {
+    context "unknown encoding" do
+      let(:blame) do
+        Gitlab::Git::Blame.new(repository, SeedRepo::EncodingCommit::ID, "encoding/iso8859.txt")
+      end
+
+      it 'converts to UTF-8' do
+        expect(CharlockHolmes::EncodingDetector).to receive(:detect).and_return(nil)
+        data = []
+        blame.each do |commit, line|
+          data << {
             commit: commit,
             line: line
-        }
-      end
+          }
+        end
 
-      expect(data.size).to eq(1)
-      expect(data.first[:commit]).to be_kind_of(Gitlab::Git::Commit)
-      expect(data.first[:line]).to eq(" ")
-      expect(data.first[:line]).to be_utf8
+        expect(data.size).to eq(1)
+        expect(data.first[:commit]).to be_kind_of(Gitlab::Git::Commit)
+        expect(data.first[:line]).to eq(" ")
+        expect(data.first[:line]).to be_utf8
+      end
     end
   end
+
+  context 'when Gitaly blame feature is enabled' do
+    it_behaves_like 'blaming a file'
+  end
+
+  context 'when Gitaly blame feature is disabled', skip_gitaly_mock: true do
+    it_behaves_like 'blaming a file'
+  end
 end