Merge branch 'mao/48221-issues_show_sql_count' into 'master'

Banzai label ref finder - minimize SQL calls by sharing context more aggresively

Closes #48221

See merge request gitlab-org/gitlab-ce!22070

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

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

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

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
 

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

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

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