diff --git a/app/finders/labels_finder.rb b/app/finders/labels_finder.rb index 82e0b2ed9e1..d000af21be3 100644 --- a/app/finders/labels_finder.rb +++ b/app/finders/labels_finder.rb @@ -129,7 +129,7 @@ class LabelsFinder < UnionFinder end def project? - params[:project_id].present? + params[:project].present? || params[:project_id].present? end def projects? @@ -152,7 +152,7 @@ class LabelsFinder < UnionFinder return @project if defined?(@project) if project? - @project = Project.find(params[:project_id]) + @project = params[:project] || Project.find(params[:project_id]) @project = nil unless authorized_to_read_labels?(@project) else @project = nil diff --git a/changelogs/unreleased/mao-48221-issues_show_sql_count.yml b/changelogs/unreleased/mao-48221-issues_show_sql_count.yml new file mode 100644 index 00000000000..d634d15946e --- /dev/null +++ b/changelogs/unreleased/mao-48221-issues_show_sql_count.yml @@ -0,0 +1,5 @@ +--- +title: Banzai label ref finder - minimize SQL calls by sharing context more aggresively +merge_request: 22070 +author: +type: performance diff --git a/lib/banzai/cross_project_reference.rb b/lib/banzai/cross_project_reference.rb index 3f1e95d4cc0..43f913a8859 100644 --- a/lib/banzai/cross_project_reference.rb +++ b/lib/banzai/cross_project_reference.rb @@ -13,6 +13,7 @@ module Banzai # Returns a Project, or nil if the reference can't be found def parent_from_ref(ref) return context[:project] || context[:group] unless ref + return context[:project] if context[:project]&.full_path == ref Project.find_by_full_path(ref) end diff --git a/lib/banzai/filter/label_reference_filter.rb b/lib/banzai/filter/label_reference_filter.rb index b92e9e55bb9..04ec38209c7 100644 --- a/lib/banzai/filter/label_reference_filter.rb +++ b/lib/banzai/filter/label_reference_filter.rb @@ -48,7 +48,7 @@ module Banzai include_ancestor_groups: true, only_group_labels: true } else - { project_id: parent.id, + { project: parent, include_ancestor_groups: true } end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 5b347b1109a..44419ed6a2a 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -637,6 +637,18 @@ describe Projects::IssuesController do project_id: project, id: id end + + it 'avoids (most) N+1s loading labels' do + label = create(:label, project: project).to_reference + labels = create_list(:label, 10, project: project).map(&:to_reference) + issue = create(:issue, project: project, description: 'Test issue') + + control_count = ActiveRecord::QueryRecorder.new { issue.update(description: [issue.description, label].join(' ')) }.count + + # Follow-up to get rid of this `2 * label.count` requirement: https://gitlab.com/gitlab-org/gitlab-ce/issues/52230 + expect { issue.update(description: [issue.description, labels].join(' ')) } + .not_to exceed_query_limit(control_count + 2 * labels.count) + end end describe 'GET #realtime_changes' do diff --git a/spec/lib/banzai/cross_project_reference_spec.rb b/spec/lib/banzai/cross_project_reference_spec.rb index aadfe7637dd..ba995e16be7 100644 --- a/spec/lib/banzai/cross_project_reference_spec.rb +++ b/spec/lib/banzai/cross_project_reference_spec.rb @@ -1,16 +1,21 @@ require 'spec_helper' describe Banzai::CrossProjectReference do - include described_class + let(:including_class) { Class.new.include(described_class).new } + + before do + allow(including_class).to receive(:context).and_return({}) + allow(including_class).to receive(:parent_from_ref).and_call_original + end describe '#parent_from_ref' do context 'when no project was referenced' do it 'returns the project from context' do project = double - allow(self).to receive(:context).and_return({ project: project }) + allow(including_class).to receive(:context).and_return({ project: project }) - expect(parent_from_ref(nil)).to eq project + expect(including_class.parent_from_ref(nil)).to eq project end end @@ -18,15 +23,15 @@ describe Banzai::CrossProjectReference do it 'returns the group from context' do group = double - allow(self).to receive(:context).and_return({ group: group }) + allow(including_class).to receive(:context).and_return({ group: group }) - expect(parent_from_ref(nil)).to eq group + expect(including_class.parent_from_ref(nil)).to eq group end end context 'when referenced project does not exist' do it 'returns nil' do - expect(parent_from_ref('invalid/reference')).to be_nil + expect(including_class.parent_from_ref('invalid/reference')).to be_nil end end @@ -37,7 +42,7 @@ describe Banzai::CrossProjectReference do expect(Project).to receive(:find_by_full_path) .with('cross/reference').and_return(project2) - expect(parent_from_ref('cross/reference')).to eq project2 + expect(including_class.parent_from_ref('cross/reference')).to eq project2 end end end diff --git a/spec/lib/banzai/filter/commit_range_reference_filter_spec.rb b/spec/lib/banzai/filter/commit_range_reference_filter_spec.rb index e1af5a15371..cbff2fdab14 100644 --- a/spec/lib/banzai/filter/commit_range_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/commit_range_reference_filter_spec.rb @@ -60,6 +60,7 @@ describe Banzai::Filter::CommitRangeReferenceFilter do exp = act = "See #{commit1.id.reverse}...#{commit2.id}" allow(project.repository).to receive(:commit).with(commit1.id.reverse) + allow(project.repository).to receive(:commit).with(commit2.id) expect(reference_filter(act).to_html).to eq exp end