Commit 450955ab authored by Igor Drozdov's avatar Igor Drozdov

Resolve N + 1 for commits notes API

When API request performed for multiple commits notes
the number of SQL requests depends on the number of MRs
parent a3f68244
...@@ -109,6 +109,7 @@ class Note < ApplicationRecord ...@@ -109,6 +109,7 @@ class Note < ApplicationRecord
scope :with_updated_at, ->(time) { where(updated_at: time) } scope :with_updated_at, ->(time) { where(updated_at: time) }
scope :inc_author_project, -> { includes(:project, :author) } scope :inc_author_project, -> { includes(:project, :author) }
scope :inc_author, -> { includes(:author) } scope :inc_author, -> { includes(:author) }
scope :with_api_entity_associations, -> { preload(:note_diff_file, :author) }
scope :inc_relations_for_view, -> do scope :inc_relations_for_view, -> do
includes(:project, { author: :status }, :updated_by, :resolved_by, :award_emoji, includes(:project, { author: :status }, :updated_by, :resolved_by, :award_emoji,
{ system_note_metadata: :description_version }, :note_diff_file, :diff_note_positions, :suggestions) { system_note_metadata: :description_version }, :note_diff_file, :diff_note_positions, :suggestions)
......
---
title: Resolve N + 1 for commits notes API
merge_request: 57641
author:
type: performance
...@@ -186,16 +186,14 @@ module API ...@@ -186,16 +186,14 @@ module API
use :pagination use :pagination
requires :sha, type: String, desc: 'A commit sha, or the name of a branch or tag' requires :sha, type: String, desc: 'A commit sha, or the name of a branch or tag'
end end
# rubocop: disable CodeReuse/ActiveRecord
get ':id/repository/commits/:sha/comments', requirements: API::COMMIT_ENDPOINT_REQUIREMENTS do get ':id/repository/commits/:sha/comments', requirements: API::COMMIT_ENDPOINT_REQUIREMENTS do
commit = user_project.commit(params[:sha]) commit = user_project.commit(params[:sha])
not_found! 'Commit' unless commit not_found! 'Commit' unless commit
notes = commit.notes.order(:created_at) notes = commit.notes.with_api_entity_associations.fresh
present paginate(notes), with: Entities::CommitNote present paginate(notes), with: Entities::CommitNote
end end
# rubocop: enable CodeReuse/ActiveRecord
desc 'Cherry pick commit into a branch' do desc 'Cherry pick commit into a branch' do
detail 'This feature was introduced in GitLab 8.15' detail 'This feature was introduced in GitLab 8.15'
......
...@@ -1439,6 +1439,22 @@ RSpec.describe API::Commits do ...@@ -1439,6 +1439,22 @@ RSpec.describe API::Commits do
it_behaves_like 'ref comments' it_behaves_like 'ref comments'
end end
end end
context 'multiple notes' do
let!(:note) { create(:diff_note_on_commit, project: project) }
let(:commit) { note.commit }
let(:commit_id) { note.commit_id }
it 'are returned without N + 1' do
get api(route, current_user) # warm up the cache
control_count = ActiveRecord::QueryRecorder.new { get api(route, current_user) }.count
create(:diff_note_on_commit, project: project, author: create(:user))
expect { get api(route, current_user) }.not_to exceed_query_limit(control_count)
end
end
end end
context 'when the commit is present on two projects' do context 'when the commit is present on two projects' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment