From 7a07ecebaf507632c255b7743c858a861845f4f8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kim=20Carlb=C3=A4cker?= <kim.carlbacker@gmail.com>
Date: Thu, 8 Jun 2017 00:47:10 +0000
Subject: [PATCH] nil-check Repository::is_ancestor?

---
 app/models/repository.rb       |  2 ++
 spec/models/repository_spec.rb | 42 ++++++++++++++++++++++++++--------
 2 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/app/models/repository.rb b/app/models/repository.rb
index 07e0b3bae4f..00a0b407512 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -946,6 +946,8 @@ class Repository
   end
 
   def is_ancestor?(ancestor_id, descendant_id)
+    return false if ancestor_id.nil? || descendant_id.nil?
+    
     Gitlab::GitalyClient.migrate(:is_ancestor) do |is_enabled|
       if is_enabled
         raw_repository.is_ancestor?(ancestor_id, descendant_id)
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 718b7d5e86b..a6d4d92c450 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -1905,19 +1905,43 @@ describe Repository, models: true do
   end
 
   describe '#is_ancestor?' do
-    context 'Gitaly is_ancestor feature enabled' do
-      let(:commit) { repository.commit }
-      let(:ancestor) { commit.parents.first }
+    let(:commit) { repository.commit }
+    let(:ancestor) { commit.parents.first }
 
-      before do
-        allow(Gitlab::GitalyClient).to receive(:enabled?).and_return(true)
-        allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:is_ancestor).and_return(true)
+    context 'with Gitaly enabled' do
+      it 'it is an ancestor' do
+        expect(repository.is_ancestor?(ancestor.id, commit.id)).to eq(true)
       end
 
-      it "asks Gitaly server if it's an ancestor" do
-        expect_any_instance_of(Gitlab::GitalyClient::Commit).to receive(:is_ancestor).with(ancestor.id, commit.id)
+      it 'it is not an ancestor' do
+        expect(repository.is_ancestor?(commit.id, ancestor.id)).to eq(false)
+      end
 
-        repository.is_ancestor?(ancestor.id, commit.id)
+      it 'returns false on nil-values' do
+        expect(repository.is_ancestor?(nil, commit.id)).to eq(false)
+        expect(repository.is_ancestor?(ancestor.id, nil)).to eq(false)
+        expect(repository.is_ancestor?(nil, nil)).to eq(false)
+      end
+    end
+
+    context 'with Gitaly disabled' do
+      before do
+        allow(Gitlab::GitalyClient).to receive(:enabled?).and_return(false)
+        allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:is_ancestor).and_return(false)
+      end
+
+      it 'it is an ancestor' do
+        expect(repository.is_ancestor?(ancestor.id, commit.id)).to eq(true)
+      end
+
+      it 'it is not an ancestor' do
+        expect(repository.is_ancestor?(commit.id, ancestor.id)).to eq(false)
+      end
+
+      it 'returns false on nil-values' do
+        expect(repository.is_ancestor?(nil, commit.id)).to eq(false)
+        expect(repository.is_ancestor?(ancestor.id, nil)).to eq(false)
+        expect(repository.is_ancestor?(nil, nil)).to eq(false)
       end
     end
   end