PERF: Cache categories in Site model.
Profiling showed that we were roughly 10% of a request time creating all the ActiveRecord objects for categories in the `Site` model on a site with 61 categories. Instead of querying for the categories each time based on which categories the user can see, we can just preload all of the categories upfront and filter out the categories that the user can not see.
This commit is contained in:
parent
63299bc14f
commit
7dc0f88acd
|
@ -91,6 +91,7 @@ class Category < ActiveRecord::Base
|
|||
after_commit :trigger_category_created_event, on: :create
|
||||
after_commit :trigger_category_updated_event, on: :update
|
||||
after_commit :trigger_category_destroyed_event, on: :destroy
|
||||
after_commit :clear_site_cache
|
||||
|
||||
after_save_commit :index_search
|
||||
|
||||
|
@ -957,6 +958,10 @@ class Category < ActiveRecord::Base
|
|||
|
||||
result.map { |row| [row.group_id, row.permission_type] }
|
||||
end
|
||||
|
||||
def clear_site_cache
|
||||
Site.clear_cache
|
||||
end
|
||||
end
|
||||
|
||||
# == Schema Information
|
||||
|
|
|
@ -3,6 +3,10 @@
|
|||
class CategoryTag < ActiveRecord::Base
|
||||
belongs_to :category
|
||||
belongs_to :tag
|
||||
|
||||
after_commit do
|
||||
Site.clear_cache
|
||||
end
|
||||
end
|
||||
|
||||
# == Schema Information
|
||||
|
|
|
@ -3,6 +3,10 @@
|
|||
class CategoryTagGroup < ActiveRecord::Base
|
||||
belongs_to :category
|
||||
belongs_to :tag_group
|
||||
|
||||
after_commit do
|
||||
Site.clear_cache
|
||||
end
|
||||
end
|
||||
|
||||
# == Schema Information
|
||||
|
|
|
@ -28,16 +28,42 @@ class Site
|
|||
UserField.order(:position).all
|
||||
end
|
||||
|
||||
def categories
|
||||
@categories ||= begin
|
||||
CATEGORIES_CACHE_KEY = "site_categories"
|
||||
|
||||
def self.clear_cache
|
||||
Discourse.cache.delete(CATEGORIES_CACHE_KEY)
|
||||
end
|
||||
|
||||
def self.all_categories_cache
|
||||
# Categories do not change often so there is no need for us to run the
|
||||
# same query and spend time creating ActiveRecord objects for every requests.
|
||||
#
|
||||
# Do note that any new association added to the eager loading needs a
|
||||
# corresponding ActiveRecord callback to clear the categories cache.
|
||||
Discourse.cache.fetch(CATEGORIES_CACHE_KEY, expires_in: 30.minutes) do
|
||||
categories = Category
|
||||
.includes(:uploaded_logo, :uploaded_background, :tags, :tag_groups)
|
||||
.secured(@guardian)
|
||||
.includes(:uploaded_logo, :uploaded_background, :tags, :tag_groups, :required_tag_group)
|
||||
.joins('LEFT JOIN topics t on t.id = categories.topic_id')
|
||||
.select('categories.*, t.slug topic_slug')
|
||||
.order(:position)
|
||||
.to_a
|
||||
|
||||
categories = categories.to_a
|
||||
ActiveModel::ArraySerializer.new(
|
||||
categories,
|
||||
each_serializer: SiteCategorySerializer
|
||||
).as_json
|
||||
end
|
||||
end
|
||||
|
||||
def categories
|
||||
@categories ||= begin
|
||||
categories = []
|
||||
|
||||
self.class.all_categories_cache.each do |category|
|
||||
if @guardian.can_see_serialized_category?(category_id: category["id"], read_restricted: category["read_restricted"])
|
||||
categories << OpenStruct.new(category)
|
||||
end
|
||||
end
|
||||
|
||||
with_children = Set.new
|
||||
categories.each do |c|
|
||||
|
|
|
@ -30,10 +30,10 @@ class SiteSerializer < ApplicationSerializer
|
|||
:shared_drafts_category_id,
|
||||
:custom_emoji_translation,
|
||||
:watched_words_replace,
|
||||
:watched_words_link
|
||||
:watched_words_link,
|
||||
:categories
|
||||
)
|
||||
|
||||
has_many :categories, serializer: SiteCategorySerializer, embed: :objects
|
||||
has_many :archetypes, embed: :objects, serializer: ArchetypeSerializer
|
||||
has_many :user_fields, embed: :objects, serializer: UserFieldSerializer
|
||||
has_many :auth_providers, embed: :objects, serializer: AuthProviderSerializer
|
||||
|
@ -190,6 +190,10 @@ class SiteSerializer < ApplicationSerializer
|
|||
WordWatcher.word_matcher_regexps(:link)
|
||||
end
|
||||
|
||||
def categories
|
||||
object.categories.map { |c| c.to_h }
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def ordered_flags(flags)
|
||||
|
|
|
@ -46,6 +46,11 @@ module CategoryGuardian
|
|||
nil
|
||||
end
|
||||
|
||||
def can_see_serialized_category?(category_id:, read_restricted:)
|
||||
return true if !read_restricted
|
||||
secure_category_ids.include?(category_id)
|
||||
end
|
||||
|
||||
def can_see_category?(category)
|
||||
return false unless category
|
||||
return true if is_admin?
|
||||
|
|
|
@ -10,13 +10,34 @@ describe SiteSerializer do
|
|||
category.custom_fields["enable_marketplace"] = true
|
||||
category.save_custom_fields
|
||||
|
||||
data = MultiJson.dump(described_class.new(Site.new(guardian), scope: guardian, root: false))
|
||||
expect(data).not_to include("enable_marketplace")
|
||||
serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json
|
||||
c1 = serialized[:categories].find { |c| c[:id] == category.id }
|
||||
|
||||
expect(c1[:preloaded_custom_fields]).to eq(nil)
|
||||
|
||||
Site.preloaded_category_custom_fields << "enable_marketplace"
|
||||
|
||||
data = MultiJson.dump(described_class.new(Site.new(guardian), scope: guardian, root: false))
|
||||
expect(data).to include("enable_marketplace")
|
||||
serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json
|
||||
c1 = serialized[:categories].find { |c| c[:id] == category.id }
|
||||
|
||||
expect(c1[:preloaded_custom_fields]["enable_marketplace"]).to eq("t")
|
||||
end
|
||||
|
||||
it "includes category tags" do
|
||||
tag = Fabricate(:tag)
|
||||
tag_group = Fabricate(:tag_group)
|
||||
tag_group_2 = Fabricate(:tag_group)
|
||||
|
||||
category.tags << tag
|
||||
category.tag_groups << tag_group
|
||||
category.update!(required_tag_group: tag_group_2)
|
||||
|
||||
serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json
|
||||
c1 = serialized[:categories].find { |c| c[:id] == category.id }
|
||||
|
||||
expect(c1[:allowed_tags]).to contain_exactly(tag.name)
|
||||
expect(c1[:allowed_tag_groups]).to contain_exactly(tag_group.name)
|
||||
expect(c1[:required_tag_group_name]).to eq(tag_group_2.name)
|
||||
end
|
||||
|
||||
it "returns correct notification level for categories" do
|
||||
|
|
Loading…
Reference in New Issue