From a7f30cfcf775bb09ebab0aa9d84385c0b8971295 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Fri, 23 Dec 2022 04:29:49 +0800 Subject: [PATCH] PERF: Fix N+1 queries on /categories route (#19585) Featured topics are eventually serialized by `ListableTopicSerializer` which calls `Topic#image_url` which requires us to preload `Topic#topic_thumbnails`. --- app/models/category_list.rb | 9 ++++++++- spec/requests/categories_controller_spec.rb | 11 +++++++---- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/app/models/category_list.rb b/app/models/category_list.rb index ef8e78329e..21ebb279c8 100644 --- a/app/models/category_list.rb +++ b/app/models/category_list.rb @@ -62,7 +62,13 @@ class CategoryList category_featured_topics = CategoryFeaturedTopic.select([:category_id, :topic_id]).order(:rank) - @all_topics = Topic.where(id: category_featured_topics.map(&:topic_id)).includes(:shared_draft, :category) + @all_topics = Topic + .where(id: category_featured_topics.map(&:topic_id)) + .includes( + :shared_draft, + :category, + { topic_thumbnails: [:optimized_image, :upload] } + ) @all_topics = @all_topics.joins(:tags).where(tags: { name: @options[:tag] }) if @options[:tag].present? @@ -140,6 +146,7 @@ class CategoryList end allowed_topic_create = Set.new(Category.topic_create_allowed(@guardian).pluck(:id)) + categories_with_descendants.each do |category| category.notification_level = notification_levels[category.id] || default_notification_level category.permission = CategoryGroup.permission_types[:full] if allowed_topic_create.include?(category.id) diff --git a/spec/requests/categories_controller_spec.rb b/spec/requests/categories_controller_spec.rb index facf7c0cb8..8b85331555 100644 --- a/spec/requests/categories_controller_spec.rb +++ b/spec/requests/categories_controller_spec.rb @@ -302,10 +302,12 @@ RSpec.describe CategoriesController do expect(category_response["subcategory_list"][0]["id"]).to eq(subcategory.id) end - it "does not n+1 with multiple topics" do + it "does not result in N+1 queries problem with multiple topics" do category1 = Fabricate(:category) category2 = Fabricate(:category) + upload = Fabricate(:upload) topic1 = Fabricate(:topic, category: category1) + topic2 = Fabricate(:topic, category: category1, image_upload: upload) CategoryFeaturedTopic.feature_topics SiteSetting.desktop_category_page_style = "categories_with_featured_topics" @@ -320,9 +322,10 @@ RSpec.describe CategoriesController do end category_response = response.parsed_body["category_list"]["categories"].find { |c| c["id"] == category1.id } - expect(category_response["topics"].count).to eq(1) + expect(category_response["topics"].count).to eq(2) - topic2 = Fabricate(:topic, category: category2) + upload = Fabricate(:upload) + topic3 = Fabricate(:topic, category: category2, image_upload: upload) CategoryFeaturedTopic.feature_topics second_request_queries = track_sql_queries do @@ -332,7 +335,7 @@ RSpec.describe CategoriesController do category1_response = response.parsed_body["category_list"]["categories"].find { |c| c["id"] == category1.id } category2_response = response.parsed_body["category_list"]["categories"].find { |c| c["id"] == category2.id } - expect(category1_response["topics"].size).to eq(1) + expect(category1_response["topics"].size).to eq(2) expect(category2_response["topics"].size).to eq(1) expect(first_request_queries.count).to eq(second_request_queries.count)