Commit fad40e75 authored by Vasilii Iakliushin's avatar Vasilii Iakliushin

Fix N+1 problem for notes association

Follow-up for https://gitlab.com/gitlab-org/gitlab/-/issues/347407

In previous MR I added a permission verification when user requests
the list of participants. But it increased the number of db queries.

I found that notes association does not preload project object and
that generates additional queries.

Changelog: fixed
parent 783ce2d8
...@@ -61,6 +61,11 @@ module Issuable ...@@ -61,6 +61,11 @@ module Issuable
# We check first if we're loaded to not load unnecessarily. # We check first if we're loaded to not load unnecessarily.
loaded? && to_a.all? { |note| note.association(:award_emoji).loaded? } loaded? && to_a.all? { |note| note.association(:award_emoji).loaded? }
end end
def projects_loaded?
# We check first if we're loaded to not load unnecessarily.
loaded? && to_a.all? { |note| note.association(:project).loaded? }
end
end end
has_many :note_authors, -> { distinct }, through: :notes, source: :author has_many :note_authors, -> { distinct }, through: :notes, source: :author
...@@ -524,6 +529,7 @@ module Issuable ...@@ -524,6 +529,7 @@ module Issuable
includes = [] includes = []
includes << :author unless notes.authors_loaded? includes << :author unless notes.authors_loaded?
includes << :award_emoji unless notes.award_emojis_loaded? includes << :award_emoji unless notes.award_emojis_loaded?
includes << :project unless notes.projects_loaded?
if includes.any? if includes.any?
notes.includes(includes) notes.includes(includes)
......
...@@ -49,6 +49,19 @@ RSpec.describe Resolvers::Users::ParticipantsResolver do ...@@ -49,6 +49,19 @@ RSpec.describe Resolvers::Users::ParticipantsResolver do
it 'returns all participants for this user' do it 'returns all participants for this user' do
is_expected.to match_array([issue.author, note.author]) is_expected.to match_array([issue.author, note.author])
end end
it 'does not execute N+1 for project relation' do
query = -> { resolve(described_class, args: {}, ctx: { current_user: current_user }, obj: issue)&.items }
# warm-up
query.call
control_count = ActiveRecord::QueryRecorder.new { query.call }
create(:note, :confidential, project: project, noteable: issue, author: create(:user))
expect { query.call }.not_to exceed_query_limit(control_count)
end
end end
end end
end end
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