Commit 54f985e6 authored by Douwe Maan's avatar Douwe Maan

Merge branch '33377-fix-null-ref-in-repo-is-ancestor' into 'master'

nil-check Repository::is_ancestor?

Closes #33377

See merge request !11976
parents 27f68edd 7a07eceb
......@@ -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)
......
......@@ -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 }
context 'with Gitaly enabled' do
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
context 'with Gitaly disabled' do
before do
allow(Gitlab::GitalyClient).to receive(:enabled?).and_return(true)
allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:is_ancestor).and_return(true)
allow(Gitlab::GitalyClient).to receive(:enabled?).and_return(false)
allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:is_ancestor).and_return(false)
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 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
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
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment