PERF: Cache categories in Site model.

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.

diff --git a/app/models/category.rb b/app/models/category.rb
index 6692d2d..93ee755 100644
--- a/app/models/category.rb
+++ b/app/models/category.rb
@@ -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
diff --git a/app/models/category_tag.rb b/app/models/category_tag.rb
index 1e21409..e9cba7c 100644
--- a/app/models/category_tag.rb
+++ b/app/models/category_tag.rb
@@ -3,6 +3,10 @@
 class CategoryTag < ActiveRecord::Base
   belongs_to :category
   belongs_to :tag
+
+  after_commit do
+    Site.clear_cache
+  end
 end
 
 # == Schema Information
diff --git a/app/models/category_tag_group.rb b/app/models/category_tag_group.rb
index 06e64ad..ea27bc5 100644
--- a/app/models/category_tag_group.rb
+++ b/app/models/category_tag_group.rb
@@ -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
diff --git a/app/models/site.rb b/app/models/site.rb
index f8c131e..12b5204 100644
--- a/app/models/site.rb
+++ b/app/models/site.rb
@@ -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|
diff --git a/app/serializers/site_serializer.rb b/app/serializers/site_serializer.rb
index 40da58e..c7d8d5d 100644
--- a/app/serializers/site_serializer.rb
+++ b/app/serializers/site_serializer.rb
@@ -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)
diff --git a/lib/guardian/category_guardian.rb b/lib/guardian/category_guardian.rb
index 4e9050f..8fc6fe2 100644
--- a/lib/guardian/category_guardian.rb
+++ b/lib/guardian/category_guardian.rb
@@ -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?
diff --git a/spec/serializers/site_serializer_spec.rb b/spec/serializers/site_serializer_spec.rb
index 1a53cfd..5bf5856 100644
--- a/spec/serializers/site_serializer_spec.rb
+++ b/spec/serializers/site_serializer_spec.rb
@@ -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

GitHub sha: 7dc0f88acd7cef0f4cc5944ef634f8ecc4886a27

This commit appears in #13341 which was approved by ZogStriP and lis2. It was merged by tgxworld.

This commit has been mentioned on Discourse Meta. There might be relevant details there: