Merge branch 'id-reduce-sql-requests-for-issue-links' into 'master'

Reduce SQL requests number for issue links [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!57602

app/helpers/labels_helper.rb

@@ -164,8 +164,8 @@ module LabelsHelper
   end
 
   def label_subscription_status(label, project)
-    return 'group-level' if label.lazy_subscribed?(current_user)
-    return 'project-level' if label.lazy_subscribed?(current_user, project)
+    return 'group-level' if label.subscribed?(current_user)
+    return 'project-level' if label.subscribed?(current_user, project)
 
     'unsubscribed'
   end
@@ -181,7 +181,7 @@ module LabelsHelper
   end
 
   def label_subscription_toggle_button_text(label, project = nil)
-    label.lazy_subscribed?(current_user, project) ? 'Unsubscribe' : 'Subscribe'
+    label.subscribed?(current_user, project) ? 'Unsubscribe' : 'Subscribe'
   end
 
   def create_label_title(subject)

app/models/concerns/subscribable.rb

@@ -17,16 +17,6 @@ module Subscribable
   def subscribed?(user, project = nil)
     return false unless user
 
-    if (subscription = subscriptions.find_by(user: user, project: project))
-      subscription.subscribed
-    else
-      subscribed_without_subscriptions?(user, project)
-    end
-  end
-
-  def lazy_subscribed?(user, project = nil)
-    return false unless user
-
     if (subscription = lazy_subscription(user, project)&.itself)
       subscription.subscribed
     else
@@ -75,8 +65,10 @@ module Subscribable
   def toggle_subscription(user, project = nil)
     unsubscribe_from_other_levels(user, project)
 
+    new_value = !subscribed?(user, project)
+
     find_or_initialize_subscription(user, project)
-      .update(subscribed: !subscribed?(user, project))
+      .update(subscribed: new_value)
   end
 
   def subscribe(user, project = nil)
@@ -117,6 +109,8 @@ module Subscribable
   end
 
   def find_or_initialize_subscription(user, project)
+    BatchLoader::Executor.clear_current
+
     subscriptions
       .find_or_initialize_by(user_id: user.id, project_id: project.try(:id))
   end

app/models/issue.rb

@@ -115,6 +115,7 @@ class Issue < ApplicationRecord
 
   scope :preload_associated_models, -> { preload(:assignees, :labels, project: :namespace) }
   scope :with_web_entity_associations, -> { preload(:author, project: [:project_feature, :route, namespace: :route]) }
+  scope :preload_awardable, -> { preload(:award_emoji) }
   scope :with_label_attributes, ->(label_attributes) { joins(:labels).where(labels: label_attributes) }
   scope :with_alert_management_alerts, -> { joins(:alert_management_alert) }
   scope :with_prometheus_alert_events, -> { joins(:issues_prometheus_alert_events) }
@@ -343,6 +344,8 @@ class Issue < ApplicationRecord
                        .preload(preload)
                        .reorder('issue_link_id')
 
+    related_issues = yield related_issues if block_given?
+
     cross_project_filter = -> (issues) { issues.where(project: project) }
     Ability.issues_readable_by_user(related_issues,
       current_user,

changelogs/unreleased/id-reduce-sql-requests-for-issue-links.yml

+---
+title: Reduce SQL requests number for issue links
+merge_request: 57602
+author:
+type: performance

lib/api/issue_links.rb

@@ -18,7 +18,10 @@ module API
       end
       get ':id/issues/:issue_iid/links' do
         source_issue = find_project_issue(params[:issue_iid])
-        related_issues = source_issue.related_issues(current_user)
+        related_issues = source_issue.related_issues(current_user) do |issues|
+          issues.with_api_entity_associations.preload_awardable
+        end
+        related_issues.each { |issue| issue.lazy_subscription(current_user, user_project) } # preload subscriptions
 
         present related_issues,
                 with: Entities::RelatedIssue,

spec/models/concerns/subscribable_spec.rb

@@ -194,10 +194,6 @@ RSpec.describe Subscribable, 'Subscribable' do
     end
   end
 
-  describe '#lazy_subscribed?' do
-    it_behaves_like 'returns expected values', :lazy_subscribed?
-  end
-
   describe '#lazy_subscription' do
     let(:labels) { create_list(:group_label, 5) }
 

spec/requests/api/issue_links_spec.rb

@@ -12,26 +12,40 @@ RSpec.describe API::IssueLinks do
   end
 
   describe 'GET /links' do
+    def perform_request(user = nil, params = {})
+      get api("/projects/#{project.id}/issues/#{issue.iid}/links", user), params: params
+    end
+
     context 'when unauthenticated' do
       it 'returns 401' do
-        get api("/projects/#{project.id}/issues/#{issue.iid}/links")
+        perform_request
 
         expect(response).to have_gitlab_http_status(:unauthorized)
       end
     end
 
     context 'when authenticated' do
-      it 'returns related issues' do
-        target_issue = create(:issue, project: project)
-        create(:issue_link, source: issue, target: target_issue)
+      let_it_be(:issue_link1) { create(:issue_link, source: issue, target: create(:issue, project: project)) }
+      let_it_be(:issue_link2) { create(:issue_link, source: issue, target: create(:issue, project: project)) }
 
-        get api("/projects/#{project.id}/issues/#{issue.iid}/links", user)
+      it 'returns related issues' do
+        perform_request(user)
 
         expect(response).to have_gitlab_http_status(:ok)
         expect(json_response).to be_an Array
-        expect(json_response.length).to eq(1)
+        expect(json_response.length).to eq(2)
         expect(response).to match_response_schema('public_api/v4/issue_links')
       end
+
+      it 'returns multiple links without N + 1' do
+        perform_request(user)
+
+        control_count = ActiveRecord::QueryRecorder.new { perform_request(user) }.count
+
+        create(:issue_link, source: issue, target: create(:issue, project: project))
+
+        expect { perform_request(user) }.not_to exceed_query_limit(control_count)
+      end
     end
   end