Commit 19a290e7 authored by Yorick Peterse's avatar Yorick Peterse

Reduce queries in IssueReferenceFilter

This reduces the number of queries executed in IssueReferenceFilter by
retrieving the various projects/issues that may be referenced in batches
_before_ iterating over all the HTML nodes.

A chunk of the logic resides in AbstractReferenceFilter so it can be
re-used by other filters in the future.
parent fdcafe72
...@@ -103,7 +103,7 @@ module Banzai ...@@ -103,7 +103,7 @@ module Banzai
ref_pattern = object_class.reference_pattern ref_pattern = object_class.reference_pattern
link_pattern = object_class.link_reference_pattern link_pattern = object_class.link_reference_pattern
each_node do |node| nodes.each do |node|
if text_node?(node) && ref_pattern if text_node?(node) && ref_pattern
replace_text_when_pattern_matches(node, ref_pattern) do |content| replace_text_when_pattern_matches(node, ref_pattern) do |content|
object_link_filter(content, ref_pattern) object_link_filter(content, ref_pattern)
...@@ -206,6 +206,55 @@ module Banzai ...@@ -206,6 +206,55 @@ module Banzai
text text
end end
# Returns a Hash containing all object references (e.g. issue IDs) per the
# project they belong to.
def references_per_project
@references_per_project ||= begin
refs = Hash.new { |hash, key| hash[key] = Set.new }
regex = Regexp.union(object_class.reference_pattern,
object_class.link_reference_pattern)
nodes.each do |node|
node.to_html.scan(regex) do
project = $~[:project] || current_project_path
refs[project] << $~[object_sym]
end
end
refs
end
end
# Returns a Hash containing referenced projects grouped per their full
# path.
def projects_per_reference
@projects_per_reference ||= begin
hash = {}
refs = Set.new
references_per_project.each do |project_ref, _|
refs << project_ref
end
find_projects_for_paths(refs.to_a).each do |project|
hash[project.path_with_namespace] = project
end
hash
end
end
# Returns the projects for the given paths.
def find_projects_for_paths(paths)
Project.where_paths_in(paths).includes(:namespace)
end
def current_project_path
@current_project_path ||= project.path_with_namespace
end
private private
def project_refs_cache def project_refs_cache
......
...@@ -11,13 +11,40 @@ module Banzai ...@@ -11,13 +11,40 @@ module Banzai
Issue Issue
end end
def find_object(project, id) def find_object(project, iid)
project.get_issue(id) issues_per_project[project][iid]
end end
def url_for_object(issue, project) def url_for_object(issue, project)
IssuesHelper.url_for_issue(issue.iid, project, only_path: context[:only_path]) IssuesHelper.url_for_issue(issue.iid, project, only_path: context[:only_path])
end end
def project_from_ref(ref)
projects_per_reference[ref || current_project_path]
end
# Returns a Hash containing the issues per Project instance.
def issues_per_project
@issues_per_project ||= begin
hash = Hash.new { |h, k| h[k] = {} }
projects_per_reference.each do |path, project|
issue_ids = references_per_project[path]
next unless project.default_issues_tracker?
project.issues.where(iid: issue_ids.to_a).each do |issue|
hash[project][issue.iid] = issue
end
end
hash
end
end
def find_projects_for_paths(paths)
super(paths).includes(:gitlab_issue_tracker_service)
end
end end
end end
end end
require 'spec_helper'
describe Banzai::Filter::AbstractReferenceFilter do
let(:project) { create(:empty_project) }
describe '#references_per_project' do
it 'returns a Hash containing references grouped per project paths' do
doc = Nokogiri::HTML.fragment("#1 #{project.to_reference}#2")
filter = described_class.new(doc, project: project)
expect(filter).to receive(:object_class).twice.and_return(Issue)
expect(filter).to receive(:object_sym).twice.and_return(:issue)
refs = filter.references_per_project
expect(refs).to be_an_instance_of(Hash)
expect(refs[project.to_reference]).to eq(Set.new(%w[1 2]))
end
end
describe '#projects_per_reference' do
it 'returns a Hash containing projects grouped per project paths' do
doc = Nokogiri::HTML.fragment('')
filter = described_class.new(doc, project: project)
expect(filter).to receive(:references_per_project).
and_return({ project.path_with_namespace => Set.new(%w[1]) })
expect(filter.projects_per_reference).
to eq({ project.path_with_namespace => project })
end
end
describe '#find_projects_for_paths' do
it 'returns a list of Projects for a list of paths' do
doc = Nokogiri::HTML.fragment('')
filter = described_class.new(doc, project: project)
expect(filter.find_projects_for_paths([project.path_with_namespace])).
to eq([project])
end
end
describe '#current_project_path' do
it 'returns the path of the current project' do
doc = Nokogiri::HTML.fragment('')
filter = described_class.new(doc, project: project)
expect(filter.current_project_path).to eq(project.path_with_namespace)
end
end
end
...@@ -25,7 +25,9 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do ...@@ -25,7 +25,9 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do
let(:reference) { issue.to_reference } let(:reference) { issue.to_reference }
it 'ignores valid references when using non-default tracker' do it 'ignores valid references when using non-default tracker' do
expect(project).to receive(:get_issue).with(issue.iid).and_return(nil) expect_any_instance_of(described_class).to receive(:find_object).
with(project, issue.iid).
and_return(nil)
exp = act = "Issue #{reference}" exp = act = "Issue #{reference}"
expect(reference_filter(act).to_html).to eq exp expect(reference_filter(act).to_html).to eq exp
...@@ -107,8 +109,9 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do ...@@ -107,8 +109,9 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do
let(:reference) { issue.to_reference(project) } let(:reference) { issue.to_reference(project) }
it 'ignores valid references when cross-reference project uses external tracker' do it 'ignores valid references when cross-reference project uses external tracker' do
expect_any_instance_of(Project).to receive(:get_issue). expect_any_instance_of(described_class).to receive(:find_object).
with(issue.iid).and_return(nil) with(project2, issue.iid).
and_return(nil)
exp = act = "Issue #{reference}" exp = act = "Issue #{reference}"
expect(reference_filter(act).to_html).to eq exp expect(reference_filter(act).to_html).to eq exp
......
...@@ -312,7 +312,8 @@ describe GitPushService, services: true do ...@@ -312,7 +312,8 @@ describe GitPushService, services: true do
end end
it "doesn't close issues when external issue tracker is in use" do it "doesn't close issues when external issue tracker is in use" do
allow(project).to receive(:default_issues_tracker?).and_return(false) allow_any_instance_of(Project).to receive(:default_issues_tracker?).
and_return(false)
# The push still shouldn't create cross-reference notes. # The push still shouldn't create cross-reference notes.
expect do expect do
......
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