diff options
author | Yorick Peterse <yorickpeterse@gmail.com> | 2017-03-28 16:01:34 +0000 |
---|---|---|
committer | James Lopez <james@jameslopez.es> | 2017-04-03 12:45:04 +0200 |
commit | f064a842afce77b64d0e6305130821f8b21cba8b (patch) | |
tree | 0c5d9aa197a29a588e0325a88fe2d2ec7b08b92c | |
parent | 398f16b6579891572baae09e9f1b8ac386c858b2 (diff) | |
download | gitlab-ce-f064a842afce77b64d0e6305130821f8b21cba8b.tar.gz |
Merge branch '30098-banzai-filter-mergerequestreferencefilter-has-an-n-1-query-problem' into 'master'
Resolve "Banzai::Filter::MergeRequestReferenceFilter has three N+1 query problems"
Closes #30098
See merge request !10252
4 files changed, 57 insertions, 2 deletions
diff --git a/changelogs/unreleased/30098-banzai-filter-mergerequestreferencefilter-has-an-n-1-query-problem.yml b/changelogs/unreleased/30098-banzai-filter-mergerequestreferencefilter-has-an-n-1-query-problem.yml new file mode 100644 index 00000000000..f3f4e065aef --- /dev/null +++ b/changelogs/unreleased/30098-banzai-filter-mergerequestreferencefilter-has-an-n-1-query-problem.yml @@ -0,0 +1,4 @@ +--- +title: Improve Markdown rendering when a lot of merge requests are referenced +merge_request: 10252 +author: diff --git a/lib/banzai/filter/merge_request_reference_filter.rb b/lib/banzai/filter/merge_request_reference_filter.rb index ac5216d9cfb..3888acf935e 100644 --- a/lib/banzai/filter/merge_request_reference_filter.rb +++ b/lib/banzai/filter/merge_request_reference_filter.rb @@ -11,8 +11,8 @@ module Banzai MergeRequest end - def find_object(project, id) - project.merge_requests.find_by(iid: id) + def find_object(project, iid) + merge_requests_per_project[project][iid] end def url_for_object(mr, project) @@ -21,6 +21,31 @@ module Banzai only_path: context[:only_path]) end + def project_from_ref(ref) + projects_per_reference[ref || current_project_path] + end + + # Returns a Hash containing the merge_requests per Project instance. + def merge_requests_per_project + @merge_requests_per_project ||= begin + hash = Hash.new { |h, k| h[k] = {} } + + projects_per_reference.each do |path, project| + merge_request_ids = references_per_project[path] + + merge_requests = project.merge_requests + .where(iid: merge_request_ids.to_a) + .includes(target_project: :namespace) + + merge_requests.each do |merge_request| + hash[project][merge_request.iid.to_i] = merge_request + end + end + + hash + end + end + def object_link_text_extras(object, matches) extras = super diff --git a/spec/lib/banzai/filter/issue_reference_filter_spec.rb b/spec/lib/banzai/filter/issue_reference_filter_spec.rb index 11607d4fb26..f1082495fcc 100644 --- a/spec/lib/banzai/filter/issue_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/issue_reference_filter_spec.rb @@ -21,6 +21,19 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do end end + describe 'performance' do + let(:another_issue) { create(:issue, project: project) } + + it 'does not have a N+1 query problem' do + single_reference = "Issue #{issue.to_reference}" + multiple_references = "Issues #{issue.to_reference} and #{another_issue.to_reference}" + + control_count = ActiveRecord::QueryRecorder.new { reference_filter(single_reference).to_html }.count + + expect { reference_filter(multiple_references).to_html }.not_to exceed_query_limit(control_count) + end + end + context 'internal reference' do it_behaves_like 'a reference containing an element node' diff --git a/spec/lib/banzai/filter/merge_request_reference_filter_spec.rb b/spec/lib/banzai/filter/merge_request_reference_filter_spec.rb index 3d3d36061f4..40232f6e426 100644 --- a/spec/lib/banzai/filter/merge_request_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/merge_request_reference_filter_spec.rb @@ -17,6 +17,19 @@ describe Banzai::Filter::MergeRequestReferenceFilter, lib: true do end end + describe 'performance' do + let(:another_merge) { create(:merge_request, source_project: project, source_branch: 'fix') } + + it 'does not have a N+1 query problem' do + single_reference = "Merge request #{merge.to_reference}" + multiple_references = "Merge requests #{merge.to_reference} and #{another_merge.to_reference}" + + control_count = ActiveRecord::QueryRecorder.new { reference_filter(single_reference).to_html }.count + + expect { reference_filter(multiple_references).to_html }.not_to exceed_query_limit(control_count) + end + end + context 'internal reference' do let(:reference) { merge.to_reference } |