diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb index 7e56e371b27bd046e4a1eaca1944b8a265779b8b..5ac56ac6fa01a7125b2573ad26659c8ae773f603 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 0000000000000000000000000000000000000000..1848230ec7e0b4f15993958f641082249230e617 --- /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 45411c779cc4edef7bcd7b8c3a7d39cdcf011957..b8bd7720265ebe02a03a0cef88eda5bdc81d41fe 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 2f7967cf531ff2b6736cee6ea39d8fb71fa7b7a3..d6ed0e253ad8be709294fa30ba0e0164c5a94c43 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 2092576e98101e21d332a050c05e3e6c9063732b..e382c7120dea1d8394e12f5de6a8ff011dd18ff0 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 0477cac66770db33e792bd3a99d45cbf8b9084e4..8c2415b4e0799767c4e1fa253e6515d4f70ef42d 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 9afe2e610b91060ae2b1a697439e50358f0ca8aa..6295856b46123a01ae01377daa5736fb484a7f87 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