Merge branch '38319-nomethoderror-undefined-method-sha-for-nil-nilclass' into 'master'

Resolve "NoMethodError: undefined method `sha' for nil:NilClass"

Closes #38319

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

3 files changed, 90 insertions, 2 deletions

diff --git a/changelogs/unreleased/38319-nomethoderror-undefined-method-sha-for-nil-nilclass.yml b/changelogs/unreleased/38319-nomethoderror-undefined-method-sha-for-nil-nilclass.yml
new file mode 100644
index 00000000000..f3c39827590
--- /dev/null
+++ b/changelogs/unreleased/38319-nomethoderror-undefined-method-sha-for-nil-nilclass.yml
@@ -0,0 +1,5 @@
+---
+title: Fix 500 error on merged merge requests when GitLab is restored from a backup
+merge_request:
+author:
+type: fixed
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index 8c1df650567..6baff362dad 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -947,7 +947,11 @@ module Gitlab
         if start_repository == self
           yield commit(start_branch_name)
         else
-          sha = start_repository.commit(start_branch_name).sha
+          start_commit = start_repository.commit(start_branch_name)
+
+          return yield nil unless start_commit
+
+          sha = start_commit.sha
 
           if branch_commit = commit(sha)
             yield branch_commit
@@ -976,8 +980,9 @@ module Gitlab
         with_repo_branch_commit(source_repository, source_branch) do |commit|
           if commit
             write_ref(local_ref, commit.sha)
+            true
           else
-            raise Rugged::ReferenceError, 'source repository is empty'
+            false
           end
         end
       end
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index 4fc26c625a5..5effaf2b043 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -1332,6 +1332,84 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
   end
 
+  describe '#with_repo_branch_commit' do
+    context 'when comparing with the same repository' do
+      let(:start_repository) { repository }
+
+      context 'when the branch exists' do
+        let(:start_branch_name) { 'master' }
+
+        it 'yields the commit' do
+          expect { |b| repository.with_repo_branch_commit(start_repository, start_branch_name, &b) }
+            .to yield_with_args(an_instance_of(Gitlab::Git::Commit))
+        end
+      end
+
+      context 'when the branch does not exist' do
+        let(:start_branch_name) { 'definitely-not-master' }
+
+        it 'yields nil' do
+          expect { |b| repository.with_repo_branch_commit(start_repository, start_branch_name, &b) }
+            .to yield_with_args(nil)
+        end
+      end
+    end
+
+    context 'when comparing with another repository' do
+      let(:start_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') }
+
+      context 'when the branch exists' do
+        let(:start_branch_name) { 'master' }
+
+        it 'yields the commit' do
+          expect { |b| repository.with_repo_branch_commit(start_repository, start_branch_name, &b) }
+            .to yield_with_args(an_instance_of(Gitlab::Git::Commit))
+        end
+      end
+
+      context 'when the branch does not exist' do
+        let(:start_branch_name) { 'definitely-not-master' }
+
+        it 'yields nil' do
+          expect { |b| repository.with_repo_branch_commit(start_repository, start_branch_name, &b) }
+            .to yield_with_args(nil)
+        end
+      end
+    end
+  end
+
+  describe '#fetch_source_branch' do
+    let(:local_ref) { 'refs/merge-requests/1/head' }
+
+    context 'when the branch exists' do
+      let(:source_branch) { 'master' }
+
+      it 'writes the ref' do
+        expect(repository).to receive(:write_ref).with(local_ref, /\h{40}/)
+
+        repository.fetch_source_branch(repository, source_branch, local_ref)
+      end
+
+      it 'returns true' do
+        expect(repository.fetch_source_branch(repository, source_branch, local_ref)).to eq(true)
+      end
+    end
+
+    context 'when the branch does not exist' do
+      let(:source_branch) { 'definitely-not-master' }
+
+      it 'does not write the ref' do
+        expect(repository).not_to receive(:write_ref)
+
+        repository.fetch_source_branch(repository, source_branch, local_ref)
+      end
+
+      it 'returns false' do
+        expect(repository.fetch_source_branch(repository, source_branch, local_ref)).to eq(false)
+      end
+    end
+  end
+
   def create_remote_branch(repository, remote_name, branch_name, source_branch_name)
     source_branch = repository.branches.find { |branch| branch.name == source_branch_name }
     rugged = repository.rugged