diff options
author | Jose Ivan Vargas Lopez <jvargas@gitlab.com> | 2017-09-28 20:06:45 +0000 |
---|---|---|
committer | Jose Ivan Vargas Lopez <jvargas@gitlab.com> | 2017-09-28 20:06:45 +0000 |
commit | 3a429e0f5ad76d5d2f2a4932253130314a8a2044 (patch) | |
tree | 766417d7f9990dd011741802c6b469e97099af7e | |
parent | 7d4ad67b8fa0124680eebb20f39f0eb06c524e58 (diff) | |
parent | e38fcc8cfdbc9c4691022532dd2cee8dace20a1c (diff) | |
download | gitlab-ce-9-5-stable-patch-6.tar.gz |
Merge branch '38319-9-5-stable' into '9-5-stable-patch-6'9-5-stable-patch-6
Handle error when fetching ref for MR with deleted source branch
See merge request gitlab-org/gitlab-ce!14550
-rw-r--r-- | app/models/merge_request.rb | 2 | ||||
-rw-r--r-- | app/models/repository.rb | 6 | ||||
-rw-r--r-- | changelogs/unreleased/38319-nomethoderror-undefined-method-sha-for-nil-nilclass.yml | 5 | ||||
-rw-r--r-- | spec/models/merge_request_spec.rb | 34 | ||||
-rw-r--r-- | spec/models/repository_spec.rb | 47 |
5 files changed, 88 insertions, 6 deletions
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 5043711c2ea..74ea71a52c3 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -950,8 +950,6 @@ class MergeRequest < ActiveRecord::Base source_project.repository, source_branch) do |commit| if commit target_project.repository.write_ref(ref_path, commit.sha) - else - raise Rugged::ReferenceError, 'source repository is empty' end end end diff --git a/app/models/repository.rb b/app/models/repository.rb index 46cb033834d..e55daf48dbb 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -996,7 +996,11 @@ class Repository 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 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/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 5baa7c81ecc..0028b60749b 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1660,12 +1660,40 @@ describe MergeRequest do end describe '#fetch_ref' do - it 'sets "ref_fetched" flag to true' do + before do subject.update!(ref_fetched: nil) + end - subject.fetch_ref + context 'when the branch exists' do + it 'writes the ref' do + expect(subject.target_project.repository).to receive(:write_ref).with(subject.ref_path, /\h{40}/) - expect(subject.reload.ref_fetched).to be_truthy + subject.fetch_ref + end + + it 'sets "ref_fetched" flag to true' do + subject.fetch_ref + + expect(subject.reload.ref_fetched).to be_truthy + end + end + + context 'when the branch does not exist' do + before do + subject.source_branch = 'definitely-not-master' + end + + it 'does not write the ref' do + expect(subject.target_project.repository).not_to receive(:write_ref) + + subject.fetch_ref + end + + it 'sets "ref_fetched" flag to true' do + subject.fetch_ref + + expect(subject.reload.ref_fetched).to be_truthy + end end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 3f815bb38ef..131340cee4c 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -2110,4 +2110,51 @@ describe Repository, models: true do end 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(::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(:forked_project) { create(:project, :repository) } + let(:start_repository) { forked_project.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(::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 end |