Skip to content

Commit

Permalink
PERF: Cache ToS and Privacy Policy paths (discourse#21860)
Browse files Browse the repository at this point in the history
Checking if the topic exists happened often and that can cause
performance issues.
  • Loading branch information
nbianca authored Jun 7, 2023
1 parent 987ec60 commit 5fc1586
Show file tree
Hide file tree
Showing 8 changed files with 64 additions and 32 deletions.
4 changes: 2 additions & 2 deletions app/serializers/post_action_type_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ def name
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
Expand Down
12 changes: 2 additions & 10 deletions app/serializers/site_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -288,23 +288,15 @@ def include_denied_emojis?
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?
tos_url.present?
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?
Expand Down
8 changes: 4 additions & 4 deletions app/views/layouts/_noscript_footer.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,17 @@
<a href='<%= path "/guidelines" %>' itemprop="url"><%= t 'guidelines_topic.title' %> </a>
</span>
</li>
<% if path = tos_path.presence %>
<% if tos_url.present? %>
<li itemscope itemtype='http://schema.org/SiteNavigationElement'>
<span itemprop='name'>
<a href='<%= path %>' itemprop="url"><%= t 'tos_topic.title' %> </a>
<a href='<%= tos_url %>' itemprop="url"><%= t 'tos_topic.title' %> </a>
</span>
</li>
<% end %>
<% if path = privacy_path.presence %>
<% if privacy_policy_url.present? %>
<li itemscope itemtype='http://schema.org/SiteNavigationElement'>
<span itemprop='name'>
<a href='<%= path %>' itemprop="url"><%= t 'privacy_topic.title' %> </a>
<a href='<%= privacy_policy_url %>' itemprop="url"><%= t 'privacy_topic.title' %> </a>
</span>
</li>
<% end %>
Expand Down
8 changes: 4 additions & 4 deletions app/views/static/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@
<li class='nav-item-faq'><a class='<%= @page == 'faq' ? 'active' : '' %>' href='<%=faq_path%>'><%= t 'js.faq' %></a></li>
<% end %>
<% end %>
<% if path = tos_path.presence %>
<li class='nav-item-tos'><a href='<%= path %>' class='<%= @page == 'tos' ? 'active' : '' %>'><%= t 'js.tos' %></a></li>
<% if tos_url.present? %>
<li class='nav-item-tos'><a href='<%= tos_url %>' class='<%= @page == 'tos' ? 'active' : '' %>'><%= t 'js.tos' %></a></li>
<% end %>
<% if path = privacy_path.presence %>
<li class='nav-item-privacy'><a href='<%= path %>' class='<%= @page == 'privacy' ? 'active' : '' %>'><%= t 'js.privacy' %></a></li>
<% if privacy_policy_url.present? %>
<li class='nav-item-privacy'><a href='<%= privacy_policy_url %>' class='<%= @page == 'privacy' ? 'active' : '' %>'><%= t 'js.privacy' %></a></li>
<% end %>
</ul>

Expand Down
2 changes: 2 additions & 0 deletions config/initializers/014-track-setting-changes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@

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
Expand Down
16 changes: 4 additions & 12 deletions lib/configurable_urls.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,11 @@ def faq_path
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
40 changes: 40 additions & 0 deletions lib/discourse.rb
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,46 @@ class << self
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
Expand Down
6 changes: 6 additions & 0 deletions lib/post_destroyer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,9 @@ def destroy
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

Expand Down Expand Up @@ -122,6 +125,9 @@ def recover
)
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

Expand Down

0 comments on commit 5fc1586

Please sign in to comment.