From 5fc1586abf125b3339a09d85eb51e167b42dbc45 Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Wed, 7 Jun 2023 20:31:20 +0200 Subject: [PATCH] PERF: Cache ToS and Privacy Policy paths (#21860) Checking if the topic exists happened often and that can cause performance issues. --- .../post_action_type_serializer.rb | 4 +- app/serializers/site_serializer.rb | 12 +----- app/views/layouts/_noscript_footer.html.erb | 8 ++-- app/views/static/show.html.erb | 8 ++-- .../initializers/014-track-setting-changes.rb | 2 + lib/configurable_urls.rb | 16 ++------ lib/discourse.rb | 40 +++++++++++++++++++ lib/post_destroyer.rb | 6 +++ 8 files changed, 64 insertions(+), 32 deletions(-) diff --git a/app/serializers/post_action_type_serializer.rb b/app/serializers/post_action_type_serializer.rb index a829d91fd8e..c898b582a11 100644 --- a/app/serializers/post_action_type_serializer.rb +++ b/app/serializers/post_action_type_serializer.rb @@ -18,11 +18,11 @@ class PostActionTypeSerializer < ApplicationSerializer end def description - i18n("description", tos_url: tos_path, base_path: Discourse.base_path) + i18n("description", tos_url: tos_url, base_path: Discourse.base_path) end def short_description - i18n("short_description", tos_url: tos_path, base_path: Discourse.base_path) + i18n("short_description", tos_url: tos_url, base_path: Discourse.base_path) end def name_key diff --git a/app/serializers/site_serializer.rb b/app/serializers/site_serializer.rb index b874cfb5d75..13434860993 100644 --- a/app/serializers/site_serializer.rb +++ b/app/serializers/site_serializer.rb @@ -288,11 +288,7 @@ class SiteSerializer < ApplicationSerializer end def tos_url - if SiteSetting.tos_url.present? - SiteSetting.tos_url - elsif SiteSetting.tos_topic_id > 0 && Topic.exists?(id: SiteSetting.tos_topic_id) - "#{Discourse.base_path}/tos" - end + Discourse.tos_url end def include_tos_url? @@ -300,11 +296,7 @@ class SiteSerializer < ApplicationSerializer end def privacy_policy_url - if SiteSetting.privacy_policy_url.present? - SiteSetting.privacy_policy_url - elsif SiteSetting.privacy_topic_id > 0 && Topic.exists?(id: SiteSetting.privacy_topic_id) - "#{Discourse.base_path}/privacy" - end + Discourse.privacy_policy_url end def include_privacy_policy_url? diff --git a/app/views/layouts/_noscript_footer.html.erb b/app/views/layouts/_noscript_footer.html.erb index 74919f43b79..1d1f706b641 100644 --- a/app/views/layouts/_noscript_footer.html.erb +++ b/app/views/layouts/_noscript_footer.html.erb @@ -16,17 +16,17 @@ - <% if path = tos_path.presence %> + <% if tos_url.present? %>
  • - +
    • <% end %> - <% if path = privacy_path.presence %> + <% if privacy_policy_url.present? %>
  • - +
    • <% end %> diff --git a/app/views/static/show.html.erb b/app/views/static/show.html.erb index cdd829664fc..be8c9cd830f 100644 --- a/app/views/static/show.html.erb +++ b/app/views/static/show.html.erb @@ -10,11 +10,11 @@
  • ' href='<%=faq_path%>'><%= t 'js.faq' %>
    • <% end %> <% end %> - <% if path = tos_path.presence %> -
  • '><%= t 'js.tos' %>
    • + <% if tos_url.present? %> +
  • '><%= t 'js.tos' %>
    • <% end %> - <% if path = privacy_path.presence %> -
  • '><%= t 'js.privacy' %>
    • + <% if privacy_policy_url.present? %> +
  • '><%= t 'js.privacy' %>
    • <% end %> diff --git a/config/initializers/014-track-setting-changes.rb b/config/initializers/014-track-setting-changes.rb index 98d2132e4bf..d6b3d750338 100644 --- a/config/initializers/014-track-setting-changes.rb +++ b/config/initializers/014-track-setting-changes.rb @@ -66,6 +66,8 @@ DiscourseEvent.on(:site_setting_changed) do |name, old_value, new_value| Emoji.clear_cache && Discourse.request_refresh! if name == :emoji_deny_list + Discourse.clear_urls! if %i[tos_topic_id privacy_topic_id].include?(name) + # Update seeded topics if %i[title site_description].include?(name) topics = SeedData::Topics.with_default_locale diff --git a/lib/configurable_urls.rb b/lib/configurable_urls.rb index c92e3fefbe6..feb256ab162 100644 --- a/lib/configurable_urls.rb +++ b/lib/configurable_urls.rb @@ -5,19 +5,11 @@ module ConfigurableUrls SiteSetting.faq_url.blank? ? "#{Discourse.base_path}/faq" : SiteSetting.faq_url end - def tos_path - if SiteSetting.tos_url.present? - SiteSetting.tos_url - elsif SiteSetting.tos_topic_id > 0 && Topic.exists?(id: SiteSetting.tos_topic_id) - "#{Discourse.base_path}/tos" - end + def tos_url + Discourse.tos_url end - def privacy_path - if SiteSetting.privacy_policy_url.present? - SiteSetting.privacy_policy_url - elsif SiteSetting.privacy_topic_id > 0 && Topic.exists?(id: SiteSetting.privacy_topic_id) - "#{Discourse.base_path}/privacy" - end + def privacy_policy_url + Discourse.privacy_policy_url end end diff --git a/lib/discourse.rb b/lib/discourse.rb index bbb568246fa..728b559b618 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -597,6 +597,46 @@ module Discourse alias_method :base_url_no_path, :base_url_no_prefix end + def self.urls_cache + @urls_cache ||= DistributedCache.new("urls_cache") + end + + def self.tos_url + if SiteSetting.tos_url.present? + SiteSetting.tos_url + else + urls_cache["tos"] ||= ( + if SiteSetting.tos_topic_id > 0 && Topic.exists?(id: SiteSetting.tos_topic_id) + "#{Discourse.base_path}/tos" + else + :nil + end + ) + + urls_cache["tos"] != :nil ? urls_cache["tos"] : nil + end + end + + def self.privacy_policy_url + if SiteSetting.privacy_policy_url.present? + SiteSetting.privacy_policy_url + else + urls_cache["privacy_policy"] ||= ( + if SiteSetting.privacy_topic_id > 0 && Topic.exists?(id: SiteSetting.privacy_topic_id) + "#{Discourse.base_path}/privacy" + else + :nil + end + ) + + urls_cache["privacy_policy"] != :nil ? urls_cache["privacy_policy"] : nil + end + end + + def self.clear_urls! + urls_cache.clear + end + LAST_POSTGRES_READONLY_KEY = "postgres:last_readonly" READONLY_MODE_KEY_TTL ||= 60 diff --git a/lib/post_destroyer.rb b/lib/post_destroyer.rb index b989fe83fab..cb4b829e545 100644 --- a/lib/post_destroyer.rb +++ b/lib/post_destroyer.rb @@ -90,6 +90,9 @@ class PostDestroyer UserActionManager.topic_destroyed(@topic) DiscourseEvent.trigger(:topic_destroyed, @topic, @user) WebHook.enqueue_topic_hooks(:topic_destroyed, @topic, topic_payload) if has_topic_web_hooks + if SiteSetting.tos_topic_id == @topic.id || SiteSetting.privacy_topic_id == @topic.id + Discourse.clear_urls! + end end end @@ -122,6 +125,9 @@ class PostDestroyer ) end update_imap_sync(@post, false) + if SiteSetting.tos_topic_id == @topic.id || SiteSetting.privacy_topic_id == @topic.id + Discourse.clear_urls! + end end end