From 90ce44867595073d91ace4aacbb948f1e4f1ecb0 Mon Sep 17 00:00:00 2001 From: Dan Ungureanu <dan@ungureanu.me> Date: Tue, 12 Feb 2019 12:20:33 +0200 Subject: [PATCH] PERF: Cache build_not_found_page --- app/controllers/application_controller.rb | 19 +++++++++++---- .../exceptions/_not_found_topics.html.erb | 20 ++++++++++++++++ app/views/exceptions/not_found.html.erb | 23 +------------------ spec/requests/application_controller_spec.rb | 15 ++++++++++++ 4 files changed, 50 insertions(+), 27 deletions(-) create mode 100644 app/views/exceptions/_not_found_topics.html.erb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 846ffc2c5fc..e625b8e2c2b 100644 --- a/app/controllers/application_controller.rb +++ b/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' diff --git a/app/views/exceptions/_not_found_topics.html.erb b/app/views/exceptions/_not_found_topics.html.erb new file mode 100644 index 00000000000..59cf088c33e --- /dev/null +++ b/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' %>…</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' %>…</a> + </div> +</div> diff --git a/app/views/exceptions/not_found.html.erb b/app/views/exceptions/not_found.html.erb index e787e78dcd2..a1173a8feb1 100644 --- a/app/views/exceptions/not_found.html.erb +++ b/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' %>…</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' %>…</a> - </div> - </div> -<% end %> +<%= @topics_partial %> <%- unless @hide_search%> <div class="row"> diff --git a/spec/requests/application_controller_spec.rb b/spec/requests/application_controller_spec.rb index 0e9f2510ca5..d2d63ab0102 100644 --- a/spec/requests/application_controller_spec.rb +++ b/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