PERF: Fix n+1 for categories + featured topics (#16188)
`topic.featured_topic` and `topic.category` are used by `TopicGuardian#can_see_topic?`
This commit is contained in:
parent
28906cff92
commit
8664712c1a
|
@ -62,7 +62,7 @@ class CategoryList
|
||||||
|
|
||||||
category_featured_topics = CategoryFeaturedTopic.select([:category_id, :topic_id]).order(:rank)
|
category_featured_topics = CategoryFeaturedTopic.select([:category_id, :topic_id]).order(:rank)
|
||||||
|
|
||||||
@all_topics = Topic.where(id: category_featured_topics.map(&:topic_id))
|
@all_topics = Topic.where(id: category_featured_topics.map(&:topic_id)).includes(:shared_draft, :category)
|
||||||
|
|
||||||
if @guardian.authenticated?
|
if @guardian.authenticated?
|
||||||
@all_topics = @all_topics
|
@all_topics = @all_topics
|
||||||
|
|
|
@ -159,6 +159,42 @@ describe CategoriesController do
|
||||||
expect(category_response["subcategory_list"][0]["id"]).to eq(subcategory.id)
|
expect(category_response["subcategory_list"][0]["id"]).to eq(subcategory.id)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it "does not n+1 with multiple topics" do
|
||||||
|
category1 = Fabricate(:category)
|
||||||
|
category2 = Fabricate(:category)
|
||||||
|
topic1 = Fabricate(:topic, category: category1)
|
||||||
|
|
||||||
|
CategoryFeaturedTopic.feature_topics
|
||||||
|
SiteSetting.desktop_category_page_style = "categories_with_featured_topics"
|
||||||
|
|
||||||
|
# warmup
|
||||||
|
get "/categories.json"
|
||||||
|
expect(response.status).to eq(200)
|
||||||
|
|
||||||
|
first_request_queries = track_sql_queries do
|
||||||
|
get "/categories.json"
|
||||||
|
expect(response.status).to eq(200)
|
||||||
|
end
|
||||||
|
|
||||||
|
category_response = response.parsed_body["category_list"]["categories"].find { |c| c["id"] == category1.id }
|
||||||
|
expect(category_response["topics"].count).to eq(1)
|
||||||
|
|
||||||
|
topic2 = Fabricate(:topic, category: category2)
|
||||||
|
CategoryFeaturedTopic.feature_topics
|
||||||
|
|
||||||
|
second_request_queries = track_sql_queries do
|
||||||
|
get "/categories.json"
|
||||||
|
expect(response.status).to eq(200)
|
||||||
|
end
|
||||||
|
|
||||||
|
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(category2_response["topics"].size).to eq(1)
|
||||||
|
|
||||||
|
expect(first_request_queries.count).to eq(second_request_queries.count)
|
||||||
|
end
|
||||||
|
|
||||||
it 'does not show uncategorized unless allow_uncategorized_topics' do
|
it 'does not show uncategorized unless allow_uncategorized_topics' do
|
||||||
SiteSetting.desktop_category_page_style = "categories_boxes_with_topics"
|
SiteSetting.desktop_category_page_style = "categories_boxes_with_topics"
|
||||||
|
|
||||||
|
|
|
@ -181,4 +181,17 @@ module Helpers
|
||||||
target.send(:remove_const, const)
|
target.send(:remove_const, const)
|
||||||
target.const_set(const, old)
|
target.const_set(const, old)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def track_sql_queries
|
||||||
|
queries = []
|
||||||
|
callback = ->(*, payload) {
|
||||||
|
queries << payload.fetch(:sql) unless ["CACHE", "SCHEMA"].include?(payload.fetch(:name))
|
||||||
|
}
|
||||||
|
|
||||||
|
ActiveSupport::Notifications.subscribed(callback, "sql.active_record") do
|
||||||
|
yield
|
||||||
|
end
|
||||||
|
|
||||||
|
queries
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in New Issue