diff options
author | James Edwards-Jones <jedwardsjones@gitlab.com> | 2017-04-22 05:26:58 +0300 |
---|---|---|
committer | James Edwards-Jones <jedwardsjones@gitlab.com> | 2017-05-04 22:03:46 +0300 |
commit | 020295fffc2329b7852c2739082986fd6b569d9e (patch) | |
tree | ff5c16475b3e30a6003d8d39f03484fb937e9e6f | |
parent | 8e156d662b7895ce8777f34882422c3714cc65c7 (diff) |
Use regex to skip unnecessary reference processing in ProcessCommitWorker
-rw-r--r-- | app/models/concerns/mentionable.rb | 16 | ||||
-rw-r--r-- | app/models/concerns/mentionable/reference_regexes.rb | 22 | ||||
-rw-r--r-- | app/services/git_push_service.rb | 6 | ||||
-rw-r--r-- | app/workers/process_commit_worker.rb | 3 | ||||
-rw-r--r-- | spec/models/concerns/mentionable_spec.rb | 49 | ||||
-rw-r--r-- | spec/services/git_push_service_spec.rb | 11 | ||||
-rw-r--r-- | spec/workers/process_commit_worker_spec.rb | 8 |
7 files changed, 112 insertions, 3 deletions
diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb index 7e56e371b27..5ac56ac6fa0 100644 --- a/app/models/concerns/mentionable.rb +++ b/app/models/concerns/mentionable.rb @@ -78,6 +78,8 @@ module Mentionable # Extract GFM references to other Mentionables from this Mentionable. Always excludes its #local_reference. def referenced_mentionables(current_user = self.author) + return [] unless matches_cross_reference_regex? + refs = all_references(current_user) refs = (refs.issues + refs.merge_requests + refs.commits) @@ -87,6 +89,20 @@ module Mentionable refs.reject { |ref| ref == local_reference } 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+. def create_cross_references!(author = self.author, without = []) refs = referenced_mentionables(author) diff --git a/app/models/concerns/mentionable/reference_regexes.rb b/app/models/concerns/mentionable/reference_regexes.rb new file mode 100644 index 00000000000..1848230ec7e --- /dev/null +++ b/app/models/concerns/mentionable/reference_regexes.rb @@ -0,0 +1,22 @@ +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 diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index 45411c779cc..b8bd7720265 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -85,8 +85,10 @@ class GitPushService < BaseService default = is_default_branch? push_commits.last(PROCESS_COMMIT_LIMIT).each do |commit| - ProcessCommitWorker. - perform_async(project.id, current_user.id, commit.to_hash, default) + if commit.matches_cross_reference_regex? + ProcessCommitWorker. + perform_async(project.id, current_user.id, commit.to_hash, default) + end end end diff --git a/app/workers/process_commit_worker.rb b/app/workers/process_commit_worker.rb index 2f7967cf531..d6ed0e253ad 100644 --- a/app/workers/process_commit_worker.rb +++ b/app/workers/process_commit_worker.rb @@ -23,6 +23,9 @@ class ProcessCommitWorker return unless user commit = build_commit(project, commit_hash) + + return unless commit.matches_cross_reference_regex? + author = commit.author || user process_commit_message(project, commit, user, author, default) diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb index 2092576e981..e382c7120de 100644 --- a/spec/models/concerns/mentionable_spec.rb +++ b/spec/models/concerns/mentionable_spec.rb @@ -163,3 +163,52 @@ describe Issue, "Mentionable" do 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 diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 0477cac6677..8c2415b4e07 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -622,12 +622,21 @@ describe GitPushService, services: true do it 'only schedules a limited number of commits' do 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 service.process_commit_messages 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 def execute_service(project, user, oldrev, newrev, ref) diff --git a/spec/workers/process_commit_worker_spec.rb b/spec/workers/process_commit_worker_spec.rb index 9afe2e610b9..6295856b461 100644 --- a/spec/workers/process_commit_worker_spec.rb +++ b/spec/workers/process_commit_worker_spec.rb @@ -20,6 +20,14 @@ describe ProcessCommitWorker do worker.perform(project.id, -1, commit.to_hash) 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 expect(worker).to receive(:process_commit_message).and_call_original |