Commit 020295ff authored by James Edwards-Jones's avatar James Edwards-Jones

Use regex to skip unnecessary reference processing in ProcessCommitWorker

parent 8e156d66
...@@ -78,6 +78,8 @@ module Mentionable ...@@ -78,6 +78,8 @@ module Mentionable
# Extract GFM references to other Mentionables from this Mentionable. Always excludes its #local_reference. # Extract GFM references to other Mentionables from this Mentionable. Always excludes its #local_reference.
def referenced_mentionables(current_user = self.author) def referenced_mentionables(current_user = self.author)
return [] unless matches_cross_reference_regex?
refs = all_references(current_user) refs = all_references(current_user)
refs = (refs.issues + refs.merge_requests + refs.commits) refs = (refs.issues + refs.merge_requests + refs.commits)
...@@ -87,6 +89,20 @@ module Mentionable ...@@ -87,6 +89,20 @@ module Mentionable
refs.reject { |ref| ref == local_reference } refs.reject { |ref| ref == local_reference }
end end
# Uses regex to quickly determine if mentionables might be referenced
# Allows heavy processing to be skipped
def matches_cross_reference_regex?
reference_pattern = if project.default_issues_tracker?
ReferenceRegexes::DEFAULT_PATTERN
else
ReferenceRegexes::EXTERNAL_PATTERN
end
self.class.mentionable_attrs.any? do |attr, _|
__send__(attr) =~ reference_pattern
end
end
# Create a cross-reference Note for each GFM reference to another Mentionable found in the +mentionable_attrs+. # Create a cross-reference Note for each GFM reference to another Mentionable found in the +mentionable_attrs+.
def create_cross_references!(author = self.author, without = []) def create_cross_references!(author = self.author, without = [])
refs = referenced_mentionables(author) refs = referenced_mentionables(author)
......
module Mentionable
module ReferenceRegexes
def self.reference_pattern(link_patterns, issue_pattern)
Regexp.union(link_patterns,
issue_pattern,
Commit.reference_pattern,
MergeRequest.reference_pattern)
end
DEFAULT_PATTERN = begin
issue_pattern = Issue.reference_pattern
link_patterns = Regexp.union([Issue, Commit, MergeRequest].map(&:link_reference_pattern))
reference_pattern(link_patterns, issue_pattern)
end
EXTERNAL_PATTERN = begin
issue_pattern = ExternalIssue.reference_pattern
link_patterns = URI.regexp(%w(http https))
reference_pattern(link_patterns, issue_pattern)
end
end
end
...@@ -85,8 +85,10 @@ class GitPushService < BaseService ...@@ -85,8 +85,10 @@ class GitPushService < BaseService
default = is_default_branch? default = is_default_branch?
push_commits.last(PROCESS_COMMIT_LIMIT).each do |commit| push_commits.last(PROCESS_COMMIT_LIMIT).each do |commit|
ProcessCommitWorker. if commit.matches_cross_reference_regex?
perform_async(project.id, current_user.id, commit.to_hash, default) ProcessCommitWorker.
perform_async(project.id, current_user.id, commit.to_hash, default)
end
end end
end end
......
...@@ -23,6 +23,9 @@ class ProcessCommitWorker ...@@ -23,6 +23,9 @@ class ProcessCommitWorker
return unless user return unless user
commit = build_commit(project, commit_hash) commit = build_commit(project, commit_hash)
return unless commit.matches_cross_reference_regex?
author = commit.author || user author = commit.author || user
process_commit_message(project, commit, user, author, default) process_commit_message(project, commit, user, author, default)
......
...@@ -163,3 +163,52 @@ describe Issue, "Mentionable" do ...@@ -163,3 +163,52 @@ describe Issue, "Mentionable" do
end end
end end
end end
describe Commit, 'Mentionable' do
let(:project) { create(:project, :public, :repository) }
let(:commit) { project.commit }
describe '#matches_cross_reference_regex?' do
it "is false when message doesn't reference anything" do
allow(commit.raw).to receive(:message).and_return "WIP: Do something"
expect(commit.matches_cross_reference_regex?).to be false
end
it 'is true if issue #number mentioned in title' do
allow(commit.raw).to receive(:message).and_return "#1"
expect(commit.matches_cross_reference_regex?).to be true
end
it 'is true if references an MR' do
allow(commit.raw).to receive(:message).and_return "See merge request !12"
expect(commit.matches_cross_reference_regex?).to be true
end
it 'is true if references a commit' do
allow(commit.raw).to receive(:message).and_return "a1b2c3d4"
expect(commit.matches_cross_reference_regex?).to be true
end
it 'is true if issue referenced by url' do
issue = create(:issue, project: project)
allow(commit.raw).to receive(:message).and_return Gitlab::UrlBuilder.build(issue)
expect(commit.matches_cross_reference_regex?).to be true
end
context 'with external issue tracker' do
let(:project) { create(:jira_project) }
it 'is true if external issues referenced' do
allow(commit.raw).to receive(:message).and_return 'JIRA-123'
expect(commit.matches_cross_reference_regex?).to be true
end
end
end
end
...@@ -622,12 +622,21 @@ describe GitPushService, services: true do ...@@ -622,12 +622,21 @@ describe GitPushService, services: true do
it 'only schedules a limited number of commits' do it 'only schedules a limited number of commits' do
allow(service).to receive(:push_commits). allow(service).to receive(:push_commits).
and_return(Array.new(1000, double(:commit, to_hash: {}))) and_return(Array.new(1000, double(:commit, to_hash: {}, matches_cross_reference_regex?: true)))
expect(ProcessCommitWorker).to receive(:perform_async).exactly(100).times expect(ProcessCommitWorker).to receive(:perform_async).exactly(100).times
service.process_commit_messages service.process_commit_messages
end end
it "skips commits which don't include cross-references" do
allow(service).to receive(:push_commits).
and_return([double(:commit, to_hash: {}, matches_cross_reference_regex?: false)])
expect(ProcessCommitWorker).not_to receive(:perform_async)
service.process_commit_messages
end
end end
def execute_service(project, user, oldrev, newrev, ref) def execute_service(project, user, oldrev, newrev, ref)
......
...@@ -20,6 +20,14 @@ describe ProcessCommitWorker do ...@@ -20,6 +20,14 @@ describe ProcessCommitWorker do
worker.perform(project.id, -1, commit.to_hash) worker.perform(project.id, -1, commit.to_hash)
end end
it 'does not process the commit when no issues are referenced' do
allow(worker).to receive(:build_commit).and_return(double(matches_cross_reference_regex?: false))
expect(worker).not_to receive(:process_commit_message)
worker.perform(project.id, user.id, commit.to_hash)
end
it 'processes the commit message' do it 'processes the commit message' do
expect(worker).to receive(:process_commit_message).and_call_original expect(worker).to receive(:process_commit_message).and_call_original
......
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