Commit 9004b19e authored by Doug Stull's avatar Doug Stull

Merge branch 'fix_n_1_for_note_metadata' into 'master'

Fix N+1 problem for system notes metadata association

See merge request gitlab-org/gitlab!76951
parents 0180d550 3bc94ab8
...@@ -66,6 +66,11 @@ module Issuable ...@@ -66,6 +66,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(:project).loaded? } loaded? && to_a.all? { |note| note.association(:project).loaded? }
end end
def system_note_metadata_loaded?
# We check first if we're loaded to not load unnecessarily.
loaded? && to_a.all? { |note| note.association(:system_note_metadata).loaded? }
end
end end
has_many :note_authors, -> { distinct }, through: :notes, source: :author has_many :note_authors, -> { distinct }, through: :notes, source: :author
...@@ -534,6 +539,7 @@ module Issuable ...@@ -534,6 +539,7 @@ module Issuable
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? includes << :project unless notes.projects_loaded?
includes << :system_note_metadata unless notes.system_note_metadata_loaded?
if includes.any? if includes.any?
notes.includes(includes) notes.includes(includes)
......
...@@ -13,6 +13,7 @@ RSpec.describe Resolvers::Users::ParticipantsResolver do ...@@ -13,6 +13,7 @@ RSpec.describe Resolvers::Users::ParticipantsResolver do
let_it_be(:note) do let_it_be(:note) do
create( create(
:note, :note,
:system,
:confidential, :confidential,
project: project, project: project,
noteable: issue, noteable: issue,
...@@ -20,6 +21,8 @@ RSpec.describe Resolvers::Users::ParticipantsResolver do ...@@ -20,6 +21,8 @@ RSpec.describe Resolvers::Users::ParticipantsResolver do
) )
end end
let_it_be(:note_metadata) { create(:system_note_metadata, note: note) }
subject(:resolved_items) { resolve(described_class, args: {}, ctx: { current_user: current_user }, obj: issue)&.items } subject(:resolved_items) { resolve(described_class, args: {}, ctx: { current_user: current_user }, obj: issue)&.items }
before do before do
...@@ -50,18 +53,31 @@ RSpec.describe Resolvers::Users::ParticipantsResolver do ...@@ -50,18 +53,31 @@ RSpec.describe Resolvers::Users::ParticipantsResolver 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 context 'N+1 queries' do
query = -> { resolve(described_class, args: {}, ctx: { current_user: current_user }, obj: issue)&.items } let(:query) { -> { resolve(described_class, args: {}, ctx: { current_user: current_user }, obj: issue)&.items } }
before do
# warm-up # warm-up
query.call query.call
end
it 'does not execute N+1 for project relation' do
control_count = ActiveRecord::QueryRecorder.new { query.call } control_count = ActiveRecord::QueryRecorder.new { query.call }
create(:note, :confidential, project: project, noteable: issue, author: create(:user)) create(:note, :confidential, project: project, noteable: issue, author: create(:user))
expect { query.call }.not_to exceed_query_limit(control_count) expect { query.call }.not_to exceed_query_limit(control_count)
end end
it 'does not execute N+1 for system note metadata relation' do
control_count = ActiveRecord::QueryRecorder.new { query.call }
new_note = create(:note, :system, project: project, noteable: issue, author: create(:user))
create(:system_note_metadata, note: new_note)
expect { query.call }.not_to exceed_query_limit(control_count)
end
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