From 1fd0e82da7c96d0237164610e019e19831b5bb14 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Tue, 27 Dec 2022 09:05:37 +0800 Subject: [PATCH] PERF: Fix N+1 queries problem on topic view page (#19629) `User#flair_group` was not preloaded leading to the N+1 queries problem when multiple users have flair groups. --- lib/topic_view.rb | 4 +-- spec/requests/topics_controller_spec.rb | 48 +++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/lib/topic_view.rb b/lib/topic_view.rb index c6c8b8f49f4..b963b5c59db 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -477,7 +477,7 @@ class TopicView def participants @participants ||= begin participants = {} - User.where(id: post_counts_by_user.keys).includes(:primary_group).each { |u| participants[u.id] = u } + User.where(id: post_counts_by_user.keys).includes(:primary_group, :flair_group).each { |u| participants[u.id] = u } participants end end @@ -774,7 +774,7 @@ class TopicView def filter_posts_by_ids(post_ids) @posts = Post.where(id: post_ids, topic_id: @topic.id) .includes( - { user: :primary_group }, + { user: [:primary_group, :flair_group] }, :reply_to_user, :deleted_by, :incoming_email, diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index af824cd5d5e..959354ce21f 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -2068,6 +2068,54 @@ RSpec.describe TopicsController do expect(response.status).to eq(200) end + it 'does not result in N+1 queries problem when multiple topic participants have primary or flair group configured' do + user2 = Fabricate(:user) + user3 = Fabricate(:user) + post2 = Fabricate(:post, topic: topic, user: user2) + post3 = Fabricate(:post, topic: topic, user: user3) + group = Fabricate(:group) + user2.update!(primary_group: group) + user3.update!(flair_group: group) + + # warm up + get "/t/#{topic.id}.json" + expect(response.status).to eq(200) + + first_request_queries = track_sql_queries do + get "/t/#{topic.id}.json" + + expect(response.status).to eq(200) + + expect(response.parsed_body["details"]["participants"].map { |u| u["id"] }).to contain_exactly( + post_author1.id, + user2.id, + user3.id + ) + end + + group2 = Fabricate(:group) + user4 = Fabricate(:user, flair_group: group2) + user5 = Fabricate(:user, primary_group: group2) + post4 = Fabricate(:post, topic: topic, user: user4) + post5 = Fabricate(:post, topic: topic, user: user5) + + second_request_queries = track_sql_queries do + get "/t/#{topic.id}.json" + + expect(response.status).to eq(200) + + expect(response.parsed_body["details"]["participants"].map { |u| u["id"] }).to contain_exactly( + post_author1.id, + user2.id, + user3.id, + user4.id, + user5.id + ) + end + + expect(second_request_queries.count).to eq(first_request_queries.count) + end + context 'when a topic with nil slug exists' do before do nil_slug_topic = Fabricate(:topic)