mirror of
https://github.com/discourse/discourse.git
synced 2025-06-06 03:06:53 +08:00
PERF: Prevent N+1 queries when loading theme/component descriptions (#32305)
Theme/component description is fetched via the `locale_fields` and `theme_translation_overrides` associations on the `Theme` model. We're currently not preloading these associations when serializing themes/components, which causes an N+1 performance issue when accessing the themes/components listing pages.
This commit is contained in:
@ -87,14 +87,21 @@ class Theme < ActiveRecord::Base
|
||||
:child_themes,
|
||||
:theme_settings,
|
||||
:settings_field,
|
||||
:locale_fields,
|
||||
:color_scheme,
|
||||
:theme_translation_overrides,
|
||||
theme_fields: %i[upload theme_settings_migration],
|
||||
)
|
||||
end
|
||||
|
||||
scope :include_basic_relations, -> { includes(:parent_themes, :remote_theme, :user) }
|
||||
scope :include_basic_relations,
|
||||
-> do
|
||||
includes(
|
||||
:remote_theme,
|
||||
:user,
|
||||
:locale_fields,
|
||||
:theme_translation_overrides,
|
||||
parent_themes: %i[locale_fields theme_translation_overrides],
|
||||
)
|
||||
end
|
||||
|
||||
delegate :remote_url, to: :remote_theme, private: true, allow_nil: true
|
||||
|
||||
|
@ -1596,4 +1596,105 @@ HTML
|
||||
expect(ColorScheme.unscoped.exists?(id: scheme.id)).to eq(false)
|
||||
end
|
||||
end
|
||||
|
||||
describe ".include_basic_relations" do
|
||||
fab!(:parent_theme_1) do
|
||||
Fabricate(
|
||||
:theme,
|
||||
theme_fields: [
|
||||
ThemeField.new(
|
||||
name: "en",
|
||||
type_id: ThemeField.types[:yaml],
|
||||
target_id: Theme.targets[:translations],
|
||||
value: <<~YAML,
|
||||
en:
|
||||
theme_metadata:
|
||||
description: "Description of my theme"
|
||||
YAML
|
||||
),
|
||||
],
|
||||
)
|
||||
end
|
||||
|
||||
fab!(:parent_theme_2) do
|
||||
Fabricate(
|
||||
:theme,
|
||||
theme_fields: [
|
||||
ThemeField.new(
|
||||
name: "en",
|
||||
type_id: ThemeField.types[:yaml],
|
||||
target_id: Theme.targets[:translations],
|
||||
value: <<~YAML,
|
||||
en:
|
||||
theme_metadata:
|
||||
description: "Description of my theme 2"
|
||||
YAML
|
||||
),
|
||||
],
|
||||
)
|
||||
end
|
||||
|
||||
fab!(:component_1) do
|
||||
Fabricate(
|
||||
:theme,
|
||||
component: true,
|
||||
parent_themes: [parent_theme_1],
|
||||
theme_fields: [
|
||||
ThemeField.new(
|
||||
name: "en",
|
||||
type_id: ThemeField.types[:yaml],
|
||||
target_id: Theme.targets[:translations],
|
||||
value: <<~YAML,
|
||||
en:
|
||||
theme_metadata:
|
||||
description: "Description of my component"
|
||||
YAML
|
||||
),
|
||||
],
|
||||
)
|
||||
end
|
||||
|
||||
fab!(:component_2) do
|
||||
Fabricate(
|
||||
:theme,
|
||||
component: true,
|
||||
parent_themes: [parent_theme_2],
|
||||
theme_fields: [
|
||||
ThemeField.new(
|
||||
name: "en",
|
||||
type_id: ThemeField.types[:yaml],
|
||||
target_id: Theme.targets[:translations],
|
||||
value: <<~YAML,
|
||||
en:
|
||||
theme_metadata:
|
||||
description: "Description of my component 2"
|
||||
YAML
|
||||
),
|
||||
],
|
||||
)
|
||||
end
|
||||
|
||||
it "doesn't result in N+1 queries for descriptions" do
|
||||
components = Theme.include_basic_relations.where(component: true, id: component_1.id)
|
||||
|
||||
queries_for_one =
|
||||
track_sql_queries do
|
||||
components.each do |component|
|
||||
ComponentIndexSerializer.new(component, root: false).as_json
|
||||
end
|
||||
end
|
||||
|
||||
components =
|
||||
Theme.include_basic_relations.where(component: true, id: [component_1.id, component_2.id])
|
||||
|
||||
queries_for_two =
|
||||
track_sql_queries do
|
||||
components.each do |component|
|
||||
ComponentIndexSerializer.new(component, root: false).as_json
|
||||
end
|
||||
end
|
||||
|
||||
expect(queries_for_two.size).to eq(queries_for_one.size)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
Reference in New Issue
Block a user