Merge branch 'fix/support-gitaly-commit-diff-flags' into 'master'

Add support for commit diff ignore_whitespace_change and paths flags

Closes gitaly#198

See merge request !11028

7 files changed, 46 insertions, 49 deletions

diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION
index a3df0a6959e..78bc1abd14f 100644
--- a/GITALY_SERVER_VERSION
+++ b/GITALY_SERVER_VERSION
@@ -1 +1 @@
-0.8.0
+0.10.0
diff --git a/Gemfile b/Gemfile
index 5dbb0bdfddb..0cffee6db56 100644
--- a/Gemfile
+++ b/Gemfile
@@ -367,6 +367,6 @@ gem 'vmstat', '~> 2.3.0'
 gem 'sys-filesystem', '~> 1.1.6'
 
 # Gitaly GRPC client
-gem 'gitaly', '~> 0.5.0'
+gem 'gitaly', '~> 0.6.0'
 
 gem 'toml-rb', '~> 0.3.15', require: false
diff --git a/Gemfile.lock b/Gemfile.lock
index 01c35a935f2..c0c56aa9602 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -263,7 +263,7 @@ GEM
       po_to_json (>= 1.0.0)
       rails (>= 3.2.0)
     gherkin-ruby (0.3.2)
-    gitaly (0.5.0)
+    gitaly (0.6.0)
       google-protobuf (~> 3.1)
       grpc (~> 1.0)
     github-linguist (4.7.6)
@@ -434,7 +434,7 @@ GEM
       rugged (~> 0.24)
     little-plugger (1.1.4)
     locale (2.1.2)
-    logging (2.1.0)
+    logging (2.2.2)
       little-plugger (~> 1.1)
       multi_json (~> 1.10)
     loofah (2.0.3)
@@ -922,7 +922,7 @@ DEPENDENCIES
   gettext (~> 3.2.2)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.2.0)
-  gitaly (~> 0.5.0)
+  gitaly (~> 0.6.0)
   github-linguist (~> 4.7.0)
   gitlab-flowdock-git-hook (~> 1.0.1)
   gitlab-markup (~> 1.5.1)
diff --git a/app/models/commit.rb b/app/models/commit.rb
index 88a015cdb77..9359b323ed4 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -326,13 +326,14 @@ class Commit
   end
 
   def raw_diffs(*args)
-    # NOTE: This feature is intentionally disabled until
-    # https://gitlab.com/gitlab-org/gitaly/issues/178 is resolved
-    # if Gitlab::GitalyClient.feature_enabled?(:commit_raw_diffs)
-    #   Gitlab::GitalyClient::Commit.diff_from_parent(self, *args)
-    # else
-    raw.diffs(*args)
-    # end
+    use_gitaly = Gitlab::GitalyClient.feature_enabled?(:commit_raw_diffs)
+    deltas_only = args.last.is_a?(Hash) && args.last[:deltas_only]
+
+    if use_gitaly && !deltas_only
+      Gitlab::GitalyClient::Commit.diff_from_parent(self, *args)
+    else
+      raw.diffs(*args)
+    end
   end
 
   def diffs(diff_options = nil)
diff --git a/lib/gitlab/gitaly_client/commit.rb b/lib/gitlab/gitaly_client/commit.rb
index 27db1e19bc1..0b001a9903d 100644
--- a/lib/gitlab/gitaly_client/commit.rb
+++ b/lib/gitlab/gitaly_client/commit.rb
@@ -32,7 +32,9 @@ module Gitlab
           request = Gitaly::CommitDiffRequest.new(
             repository: gitaly_repo,
             left_commit_id: parent_id,
-            right_commit_id: commit.id
+            right_commit_id: commit.id,
+            ignore_whitespace_change: options.fetch(:ignore_whitespace_change, false),
+            paths: options.fetch(:paths, []),
           )
 
           Gitlab::Git::DiffCollection.new(stub.commit_diff(request), options)
diff --git a/spec/lib/gitlab/gitaly_client/commit_spec.rb b/spec/lib/gitlab/gitaly_client/commit_spec.rb
index 58f11ff8906..abe08ccdfa1 100644
--- a/spec/lib/gitlab/gitaly_client/commit_spec.rb
+++ b/spec/lib/gitlab/gitaly_client/commit_spec.rb
@@ -7,11 +7,6 @@ describe Gitlab::GitalyClient::Commit do
     let(:repository_message) { project.repository.gitaly_repository }
     let(:commit) { project.commit('913c66a37b4a45b9769037c55c2d238bd0942d2e') }
 
-    before do
-      allow(Gitaly::Diff::Stub).to receive(:new).and_return(diff_stub)
-      allow(diff_stub).to receive(:commit_diff).and_return([])
-    end
-
     context 'when a commit has a parent' do
       it 'sends an RPC request with the parent ID as left commit' do
         request = Gitaly::CommitDiffRequest.new(
@@ -20,7 +15,7 @@ describe Gitlab::GitalyClient::Commit do
           right_commit_id: commit.id,
         )
 
-        expect(diff_stub).to receive(:commit_diff).with(request)
+        expect_any_instance_of(Gitaly::Diff::Stub).to receive(:commit_diff).with(request)
 
         described_class.diff_from_parent(commit)
       end
@@ -35,7 +30,7 @@ describe Gitlab::GitalyClient::Commit do
           right_commit_id: initial_commit.id,
         )
 
-        expect(diff_stub).to receive(:commit_diff).with(request)
+        expect_any_instance_of(Gitaly::Diff::Stub).to receive(:commit_diff).with(request)
 
         described_class.diff_from_parent(initial_commit)
       end
@@ -50,7 +45,7 @@ describe Gitlab::GitalyClient::Commit do
     it 'passes options to Gitlab::Git::DiffCollection' do
       options = { max_files: 31, max_lines: 13 }
 
-      expect(Gitlab::Git::DiffCollection).to receive(:new).with([], options)
+      expect(Gitlab::Git::DiffCollection).to receive(:new).with(kind_of(Enumerable), options)
 
       described_class.diff_from_parent(commit, options)
     end
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index 08b2169fea7..852889d4540 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -389,32 +389,31 @@ eos
     end
   end
 
-  # describe '#raw_diffs' do
-  # TODO: Uncomment when feature is reenabled
-  #   context 'Gitaly commit_raw_diffs feature enabled' do
-  #     before do
-  #       allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:commit_raw_diffs).and_return(true)
-  #     end
-  #
-  #     context 'when a truthy deltas_only is not passed to args' do
-  #       it 'fetches diffs from Gitaly server' do
-  #         expect(Gitlab::GitalyClient::Commit).to receive(:diff_from_parent).
-  #           with(commit)
-  #
-  #         commit.raw_diffs
-  #       end
-  #     end
-  #
-  #     context 'when a truthy deltas_only is passed to args' do
-  #       it 'fetches diffs using Rugged' do
-  #         opts = { deltas_only: true }
-  #
-  #         expect(Gitlab::GitalyClient::Commit).not_to receive(:diff_from_parent)
-  #         expect(commit.raw).to receive(:diffs).with(opts)
-  #
-  #         commit.raw_diffs(opts)
-  #       end
-  #     end
-  #   end
-  # end
+  describe '#raw_diffs' do
+    context 'Gitaly commit_raw_diffs feature enabled' do
+      before do
+        allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:commit_raw_diffs).and_return(true)
+      end
+
+      context 'when a truthy deltas_only is not passed to args' do
+        it 'fetches diffs from Gitaly server' do
+          expect(Gitlab::GitalyClient::Commit).to receive(:diff_from_parent).
+            with(commit)
+
+          commit.raw_diffs
+        end
+      end
+
+      context 'when a truthy deltas_only is passed to args' do
+        it 'fetches diffs using Rugged' do
+          opts = { deltas_only: true }
+
+          expect(Gitlab::GitalyClient::Commit).not_to receive(:diff_from_parent)
+          expect(commit.raw).to receive(:diffs).with(opts)
+
+          commit.raw_diffs(opts)
+        end
+      end
+    end
+  end
 end