Commit 8ba07f7d authored by Nick Thomas's avatar Nick Thomas

Make Commit#has_been_reverted? cheaper

There is no way in git to efficiently tell if a commit has been
reverted. Instead, GitLab creates system notes when we perform this
action in the web UI. These are introspected to answer the question;
doing so requires us to extract commit references from those notes.

With this change, we stop extracting commit references from the message
of the commit that is under question. This is user-controlled content
and we can't control the number of references in it. Further, since the
revert happens *after* that commit message is written, there is no way
for it to contain references to a reverting commit.
parent 2665e40c
......@@ -415,7 +415,7 @@ class Commit
end
def has_been_reverted?(current_user, notes_association = nil)
ext = all_references(current_user)
ext = Gitlab::ReferenceExtractor.new(project, current_user)
notes_association ||= notes_with_associations
notes_association.system.each do |note|
......
---
title: Improve performance of the "has this commit been reverted?" check
merge_request: 26784
author:
type: performance
......@@ -137,29 +137,4 @@ describe CommitRange do
end
end
end
describe '#has_been_reverted?' do
let(:user) { create(:user) }
let(:issue) { create(:issue, author: user, project: project) }
it 'returns true if the commit has been reverted' do
create(:note_on_issue,
noteable: issue,
system: true,
note: commit1.revert_description(user),
project: issue.project)
expect_next_instance_of(Commit) do |commit|
expect(commit).to receive(:reverts_commit?)
.with(commit1, user)
.and_return(true)
end
expect(commit1.has_been_reverted?(user, issue.notes_with_associations)).to eq(true)
end
it 'returns false if the commit has not been reverted' do
expect(commit1.has_been_reverted?(user, issue.notes_with_associations)).to eq(false)
end
end
end
......@@ -821,4 +821,29 @@ eos
expect(commit.has_signature?).to be_falsey
end
end
describe '#has_been_reverted?' do
let(:user) { create(:user) }
let(:issue) { create(:issue, author: user, project: project) }
it 'returns true if the commit has been reverted' do
create(:note_on_issue,
noteable: issue,
system: true,
note: commit.revert_description(user),
project: issue.project)
expect_next_instance_of(Commit) do |revert_commit|
expect(revert_commit).to receive(:reverts_commit?)
.with(commit, user)
.and_return(true)
end
expect(commit.has_been_reverted?(user, issue.notes_with_associations)).to eq(true)
end
it 'returns false if the commit has not been reverted' do
expect(commit.has_been_reverted?(user, issue.notes_with_associations)).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