Merge branch '43098-controller-projects-issuescontroller-show-executes-more-than-100-sql-queries' into 'master'

Resolve "Controller Projects::IssuesController#show executes more than 100 SQL queries"

Closes #43098

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

4 files changed, 85 insertions, 10 deletions

diff --git a/spec/lib/banzai/reference_parser/issue_parser_spec.rb b/spec/lib/banzai/reference_parser/issue_parser_spec.rb
index 0a63567ee40..cb7f8b20dda 100644
--- a/spec/lib/banzai/reference_parser/issue_parser_spec.rb
+++ b/spec/lib/banzai/reference_parser/issue_parser_spec.rb
@@ -117,4 +117,27 @@ describe Banzai::ReferenceParser::IssueParser do
       expect(subject.records_for_nodes(nodes)).to eq({ link => issue })
     end
   end
+
+  context 'when checking multiple merge requests on another project' do
+    let(:other_project) { create(:project, :public) }
+    let(:other_issue) { create(:issue, project: other_project) }
+
+    let(:control_links) do
+      [issue_link(other_issue)]
+    end
+
+    let(:actual_links) do
+      control_links + [issue_link(create(:issue, project: other_project))]
+    end
+
+    def issue_link(issue)
+      Nokogiri::HTML.fragment(%Q{<a data-issue="#{issue.id}"></a>}).children[0]
+    end
+
+    before do
+      project.add_developer(user)
+    end
+
+    it_behaves_like 'no N+1 queries'
+  end
 end
diff --git a/spec/lib/banzai/reference_parser/merge_request_parser_spec.rb b/spec/lib/banzai/reference_parser/merge_request_parser_spec.rb
index 775749ae3a7..14542342cf6 100644
--- a/spec/lib/banzai/reference_parser/merge_request_parser_spec.rb
+++ b/spec/lib/banzai/reference_parser/merge_request_parser_spec.rb
@@ -4,14 +4,13 @@ describe Banzai::ReferenceParser::MergeRequestParser do
   include ReferenceParserHelpers
 
   let(:user) { create(:user) }
-  let(:merge_request) { create(:merge_request) }
-  subject { described_class.new(merge_request.target_project, user) }
+  let(:project) { create(:project, :public) }
+  let(:merge_request) { create(:merge_request, source_project: project) }
+  subject { described_class.new(project, user) }
   let(:link) { empty_html_link }
 
   describe '#nodes_visible_to_user' do
     context 'when the link has a data-issue attribute' do
-      let(:project) { merge_request.target_project }
-
       before do
         project.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC)
         link['data-merge-request'] = merge_request.id.to_s
@@ -40,4 +39,27 @@ describe Banzai::ReferenceParser::MergeRequestParser do
       end
     end
   end
+
+  context 'when checking multiple merge requests on another project' do
+    let(:other_project) { create(:project, :public) }
+    let(:other_merge_request) { create(:merge_request, source_project: other_project) }
+
+    let(:control_links) do
+      [merge_request_link(other_merge_request)]
+    end
+
+    let(:actual_links) do
+      control_links + [merge_request_link(create(:merge_request, :conflict, source_project: other_project))]
+    end
+
+    def merge_request_link(merge_request)
+      Nokogiri::HTML.fragment(%Q{<a data-merge-request="#{merge_request.id}"></a>}).children[0]
+    end
+
+    before do
+      project.add_developer(user)
+    end
+
+    it_behaves_like 'no N+1 queries'
+  end
 end
diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb
index ea76e604153..905d82b3bb1 100644
--- a/spec/policies/project_policy_spec.rb
+++ b/spec/policies/project_policy_spec.rb
@@ -11,10 +11,10 @@ describe ProjectPolicy do
 
   let(:base_guest_permissions) do
     %i[
-      read_project read_board read_list read_wiki read_issue read_label
-      read_milestone read_project_snippet read_project_member
-      read_note create_project create_issue create_note
-      upload_file
+      read_project read_board read_list read_wiki read_issue
+      read_project_for_iids read_issue_iid read_merge_request_iid read_label
+      read_milestone read_project_snippet read_project_member read_note
+      create_project create_issue create_note upload_file
     ]
   end
 
@@ -120,7 +120,7 @@ describe ProjectPolicy do
         project.issues_enabled = false
         project.save!
 
-        expect_disallowed :read_issue, :create_issue, :update_issue, :admin_issue
+        expect_disallowed :read_issue, :read_issue_iid, :create_issue, :update_issue, :admin_issue
       end
     end
 
@@ -131,7 +131,7 @@ describe ProjectPolicy do
         project.issues_enabled = false
         project.save!
 
-        expect_disallowed :read_issue, :create_issue, :update_issue, :admin_issue
+        expect_disallowed :read_issue, :read_issue_iid, :create_issue, :update_issue, :admin_issue
       end
     end
   end
diff --git a/spec/support/reference_parser_helpers.rb b/spec/support/reference_parser_helpers.rb
index 01689194eac..5d5e80851e6 100644
--- a/spec/support/reference_parser_helpers.rb
+++ b/spec/support/reference_parser_helpers.rb
@@ -2,4 +2,34 @@ module ReferenceParserHelpers
   def empty_html_link
     Nokogiri::HTML.fragment('<a></a>').children[0]
   end
+
+  shared_examples 'no N+1 queries' do
+    it 'avoids N+1 queries in #nodes_visible_to_user', :request_store do
+      record_queries = lambda do |links|
+        ActiveRecord::QueryRecorder.new do
+          described_class.new(project, user).nodes_visible_to_user(user, links)
+        end
+      end
+
+      control = record_queries.call(control_links)
+      actual = record_queries.call(actual_links)
+
+      expect(actual.count).to be <= control.count
+      expect(actual.cached_count).to be <= control.cached_count
+    end
+
+    it 'avoids N+1 queries in #records_for_nodes', :request_store do
+      record_queries = lambda do |links|
+        ActiveRecord::QueryRecorder.new do
+          described_class.new(project, user).records_for_nodes(links)
+        end
+      end
+
+      control = record_queries.call(control_links)
+      actual = record_queries.call(actual_links)
+
+      expect(actual.count).to be <= control.count
+      expect(actual.cached_count).to be <= control.cached_count
+    end
+  end
 end