PERF: Cache build_not_found_page

app/controllers/application_controller.rb

@@ -717,12 +717,21 @@ class ApplicationController < ActionController::Base
       layout = 'application' if layout == 'no_ember'
     end
 
-    category_topic_ids = Category.pluck(:topic_id).compact
+    if !SiteSetting.login_required? || current_user
+      key = "page_not_found_topics"
+      if @topics_partial = $redis.get(key)
+        @topics_partial = @topics_partial.html_safe
+      else
+        category_topic_ids = Category.pluck(:topic_id).compact
+        @top_viewed = TopicQuery.new(nil, except_topic_ids: category_topic_ids).list_top_for("monthly").topics.first(10)
+        @recent = Topic.includes(:category).where.not(id: category_topic_ids).recent(10)
+        @topics_partial = render_to_string partial: '/exceptions/not_found_topics'
+        $redis.setex(key, 10.minutes, @topics_partial)
+      end
+    end
+
     @container_class = "wrap not-found-container"
-    @top_viewed = TopicQuery.new(nil, except_topic_ids: category_topic_ids).list_top_for("monthly").topics.first(10)
-    @recent = Topic.includes(:category).where.not(id: category_topic_ids).recent(10)
-    @slug =  params[:slug].class == String ? params[:slug] : ''
-    @slug =  (params[:id].class == String ? params[:id] : '') if @slug.blank?
+    @slug = params[:slug].presence || params[:id].presence || ""
     @slug.tr!('-', ' ')
     @hide_search = true if SiteSetting.login_required
     render_to_string status: status, layout: layout, formats: [:html], template: '/exceptions/not_found'

app/views/exceptions/_not_found_topics.html.erb

@@ -0,0 +1,20 @@
+<div class="row page-not-found-topics">
+  <div class="popular-topics">
+    <h2 class="popular-topics-title"><%= t 'page_not_found.popular_topics' %></h2>
+    <% @top_viewed.each do |t| %>
+      <div class='not-found-topic'>
+        <%= link_to t.title, t.relative_url %><%= category_badge(t.category) %>
+      </div>
+    <% end %>
+    <a href="<%= path "/top" %>" class="btn btn-default"><%= t 'page_not_found.see_more' %>&hellip;</a>
+  </div>
+  <div class="recent-topics">
+    <h2 class="recent-topics-title"><%= t 'page_not_found.recent_topics' %></h2>
+    <% @recent.each do |t| %>
+      <div class='not-found-topic'>
+        <%= link_to t.title, t.relative_url %><%= category_badge(t.category) %>
+      </div>
+    <% end %>
+    <a href="<%= path "/latest" %>" class="btn btn-default"><%= t 'page_not_found.see_more' %>&hellip;</a>
+  </div>
+</div>

app/views/exceptions/not_found.html.erb

@@ -2,28 +2,7 @@
 
 <%= build_plugin_html 'server:not-found-before-topics' %>
 
-<% unless SiteSetting.login_required? && current_user.nil? %>
-  <div class="row page-not-found-topics">
-    <div class="popular-topics">
-      <h2 class="popular-topics-title"><%= t 'page_not_found.popular_topics' %></h2>
-      <% @top_viewed.each do |t| %>
-        <div class='not-found-topic'>
-          <%= link_to t.title, t.relative_url %><%= category_badge(t.category) %>
-        </div>
-      <% end %>
-      <a href="<%= path "/top" %>" class="btn btn-default"><%= t 'page_not_found.see_more' %>&hellip;</a>
-    </div>
-    <div class="recent-topics">
-      <h2 class="recent-topics-title"><%= t 'page_not_found.recent_topics' %></h2>
-      <% @recent.each do |t| %>
-        <div class='not-found-topic'>
-          <%= link_to t.title, t.relative_url %><%= category_badge(t.category) %>
-        </div>
-      <% end %>
-      <a href="<%= path "/latest" %>" class="btn btn-default"><%= t 'page_not_found.see_more' %>&hellip;</a>
-    </div>
-  </div>
-<% end %>
+<%= @topics_partial %>
 
 <%- unless @hide_search%>
   <div class="row">

spec/requests/application_controller_spec.rb

@@ -143,6 +143,21 @@ RSpec.describe ApplicationController do
         expect(response.status).to eq(404)
         expect(response.body).to_not include('google.com/search')
       end
+
+      it 'should cache results' do
+        $redis.del("page_not_found_topics")
+
+        topic1 = Fabricate(:topic)
+        get '/t/nope-nope/99999999'
+        expect(response.status).to eq(404)
+        expect(response.body).to include(topic1.title)
+
+        topic2 = Fabricate(:topic)
+        get '/t/nope-nope/99999999'
+        expect(response.status).to eq(404)
+        expect(response.body).to include(topic1.title)
+        expect(response.body).to_not include(topic2.title)
+      end
     end
   end