Merge branch 'rugged-reference-cache-rescue' into 'master'

Stop rescuing Rugged::Reference in Repository cache

See merge request gitlab-org/gitlab-ce!16770

4 files changed, 14 insertions, 18 deletions

diff --git a/app/models/repository.rb b/app/models/repository.rb
index 406e3a7f73b..edfb236a91a 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -951,7 +951,7 @@ class Repository
           end
 
         instance_variable_set(ivar, value)
-      rescue Rugged::ReferenceError, Gitlab::Git::Repository::NoRepository
+      rescue Gitlab::Git::Repository::NoRepository
         # Even if the above `#exists?` check passes these errors might still
         # occur (for example because of a non-existing HEAD). We want to
         # gracefully handle this and not cache anything
diff --git a/lib/gitlab/git/tree.rb b/lib/gitlab/git/tree.rb
index 5cf336af3c6..ba6058fd3c9 100644
--- a/lib/gitlab/git/tree.rb
+++ b/lib/gitlab/git/tree.rb
@@ -83,6 +83,8 @@ module Gitlab
               commit_id: sha
             )
           end
+        rescue Rugged::ReferenceError
+          []
         end
       end
 
diff --git a/spec/lib/gitlab/git/tree_spec.rb b/spec/lib/gitlab/git/tree_spec.rb
index 86f7bcb8e38..001e406a930 100644
--- a/spec/lib/gitlab/git/tree_spec.rb
+++ b/spec/lib/gitlab/git/tree_spec.rb
@@ -80,22 +80,18 @@ describe Gitlab::Git::Tree, seed_helper: true do
   end
 
   describe '#where' do
-    context 'with gitaly disabled' do
-      before do
-        allow(Gitlab::GitalyClient).to receive(:feature_enabled?).and_return(false)
-      end
-
-      it 'calls #tree_entries_from_rugged' do
-        expect(described_class).to receive(:tree_entries_from_rugged)
-
-        described_class.where(repository, SeedRepo::Commit::ID, '/')
+    shared_examples '#where' do
+      it 'returns an empty array when called with an invalid ref' do
+        expect(described_class.where(repository, 'foobar-does-not-exist')).to eq([])
       end
     end
 
-    it 'gets the tree entries from GitalyClient' do
-      expect_any_instance_of(Gitlab::GitalyClient::CommitService).to receive(:tree_entries)
+    context 'with gitaly' do
+      it_behaves_like '#where'
+    end
 
-      described_class.where(repository, SeedRepo::Commit::ID, '/')
+    context 'without gitaly', :skip_gitaly_mock do
+      it_behaves_like '#where'
     end
   end
 end
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index d4070b320ed..1102b1c9006 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -772,8 +772,7 @@ describe Repository do
         user, 'LICENSE', 'Copyright!',
         message: 'Add LICENSE', branch_name: 'master')
 
-      allow(repository).to receive(:file_on_head)
-        .and_raise(Rugged::ReferenceError)
+      allow(repository).to receive(:root_ref).and_raise(Gitlab::Git::Repository::NoRepository)
 
       expect(repository.license_blob).to be_nil
     end
@@ -885,7 +884,7 @@ describe Repository do
     end
 
     it 'returns nil for empty repository' do
-      allow(repository).to receive(:file_on_head).and_raise(Rugged::ReferenceError)
+      allow(repository).to receive(:root_ref).and_raise(Gitlab::Git::Repository::NoRepository)
       expect(repository.gitlab_ci_yml).to be_nil
     end
   end
@@ -1937,8 +1936,7 @@ describe Repository do
 
   describe '#avatar' do
     it 'returns nil if repo does not exist' do
-      expect(repository).to receive(:file_on_head)
-        .and_raise(Rugged::ReferenceError)
+      allow(repository).to receive(:root_ref).and_raise(Gitlab::Git::Repository::NoRepository)
 
       expect(repository.avatar).to eq(nil)
     end