Avoid N+1 queries in JiraConnect::client

We expose a user notes count for each MR. Counting
user notes executes an extra SQL query for each MR in the list.
This commit loads all counts in one query and passes it to the
serializer as an option. If the option is not available, it will
fall back to execute a count query.

lib/atlassian/jira_connect/client.rb

@@ -20,6 +20,7 @@ module Atlassian
               commits: commits,
               branches: branches,
               merge_requests: merge_requests,
+              user_notes_count: user_notes_count(merge_requests),
               update_sequence_id: update_sequence_id
             )
           ]
@@ -37,6 +38,14 @@ module Atlassian
 
       private
 
+      def user_notes_count(merge_requests)
+        return unless merge_requests
+
+        Note.count_for_collection(merge_requests.map(&:id), 'MergeRequest').map do |count_group|
+          [count_group.noteable_id, count_group.count]
+        end.to_h
+      end
+
       def jwt_token(http_method, uri)
         claims = Atlassian::Jwt.build_claims(
           Atlassian::JiraConnect.app_key,

lib/atlassian/jira_connect/serializers/pull_request_entity.rb

@@ -20,7 +20,13 @@ module Atlassian
         end
         expose :title
         expose :author, using: JiraConnect::Serializers::AuthorEntity
-        expose :user_notes_count, as: :commentCount
+        expose :commentCount do |mr|
+          if options[:user_notes_count]
+            options[:user_notes_count].fetch(mr.id, 0)
+          else
+            mr.user_notes_count
+          end
+        end
         expose :source_branch, as: :sourceBranch
         expose :target_branch, as: :destinationBranch
         expose :lastUpdate do |mr|

lib/atlassian/jira_connect/serializers/repository_entity.rb

@@ -21,7 +21,11 @@ module Atlassian
           JiraConnect::Serializers::BranchEntity.represent options[:branches], project: project, update_sequence_id: options[:update_sequence_id]
         end
         expose :pullRequests do |project, options|
-          JiraConnect::Serializers::PullRequestEntity.represent options[:merge_requests], project: project, update_sequence_id: options[:update_sequence_id]
+          JiraConnect::Serializers::PullRequestEntity.represent(
+            options[:merge_requests],
+            update_sequence_id: options[:update_sequence_id],
+            user_notes_count: options[:user_notes_count]
+          )
         end
       end
     end

spec/lib/atlassian/jira_connect/client_spec.rb

@@ -20,8 +20,11 @@ RSpec.describe Atlassian::JiraConnect::Client do
   end
 
   describe '#store_dev_info' do
-    it "calls the API with auth headers" do
-      expected_jwt = Atlassian::Jwt.encode(
+    let_it_be(:project) { create_default(:project, :repository) }
+    let_it_be(:merge_requests) { create_list(:merge_request, 2, :unique_branches) }
+
+    let(:expected_jwt) do
+      Atlassian::Jwt.encode(
         Atlassian::Jwt.build_claims(
           Atlassian::JiraConnect.app_key,
           '/rest/devinfo/0.10/bulk',
@@ -29,7 +32,9 @@ RSpec.describe Atlassian::JiraConnect::Client do
         ),
         'sample_secret'
       )
+    end
 
+    before do
       stub_full_request('https://gitlab-test.atlassian.net/rest/devinfo/0.10/bulk', method: :post)
         .with(
           headers: {
@@ -37,8 +42,18 @@ RSpec.describe Atlassian::JiraConnect::Client do
             'Content-Type' => 'application/json'
           }
         )
+    end
+
+    it "calls the API with auth headers" do
+      subject.store_dev_info(project: project)
+    end
+
+    it 'avoids N+1 database queries' do
+      control_count = ActiveRecord::QueryRecorder.new { subject.store_dev_info(project: project, merge_requests: merge_requests) }.count
+
+      merge_requests << create(:merge_request, :unique_branches)
 
-      subject.store_dev_info(project: create(:project))
+      expect { subject.store_dev_info(project: project, merge_requests: merge_requests) }.not_to exceed_query_limit(control_count)
     end
   end
 end

spec/lib/atlassian/jira_connect/serializers/pull_request_entity_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Atlassian::JiraConnect::Serializers::PullRequestEntity do
+  let_it_be(:project) { create_default(:project, :repository) }
+  let_it_be(:merge_requests) { create_list(:merge_request, 2, :unique_branches) }
+  let_it_be(:notes) { create_list(:note, 2, system: false, noteable: merge_requests.first) }
+
+  subject { described_class.represent(merge_requests).as_json }
+
+  it 'exposes commentCount' do
+    expect(subject.first[:commentCount]).to eq(2)
+  end
+
+  context 'with user_notes_count option' do
+    let(:user_notes_count) { merge_requests.map { |merge_request| [merge_request.id, 1] }.to_h }
+
+    subject { described_class.represent(merge_requests, user_notes_count: user_notes_count).as_json }
+
+    it 'avoids N+1 database queries' do
+      control_count = ActiveRecord::QueryRecorder.new do
+        described_class.represent(merge_requests, user_notes_count: user_notes_count)
+      end.count
+
+      merge_requests << create(:merge_request, :unique_branches)
+
+      expect { subject }.not_to exceed_query_limit(control_count)
+    end
+
+    it 'uses counts from user_notes_count' do
+      expect(subject.map { |entity| entity[:commentCount] }).to match_array([1, 1, 1])
+    end
+
+    context 'when count is missing for some MRs' do
+      let(:user_notes_count) { [[merge_requests.last.id, 1]].to_h }
+
+      it 'uses 0 as default when count for the MR is not available' do
+        expect(subject.map { |entity| entity[:commentCount] }).to match_array([0, 0, 1])
+      end
+    end
+  end
+end