FIX: Include locale in cache key for not_found_topics (#11406)

This ensures that users are only served cached content in their own language. This commit also refactors to make use of the `Discourse.cache` framework rather than direct redis access
This commit is contained in:
David Taylor 2020-12-07 12:24:18 +00:00 committed by GitHub
parent 154c8c3fef
commit 8b33e2f73d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 14 additions and 9 deletions

View File

@ -803,16 +803,13 @@ class ApplicationController < ActionController::Base
@current_user = current_user rescue nil @current_user = current_user rescue nil
if !SiteSetting.login_required? || @current_user if !SiteSetting.login_required? || @current_user
key = "page_not_found_topics" key = "page_not_found_topics:#{I18n.locale}"
if @topics_partial = Discourse.redis.get(key) @topics_partial = Discourse.cache.fetch(key, expires_in: 10.minutes) do
@topics_partial = @topics_partial.html_safe
else
category_topic_ids = Category.pluck(:topic_id).compact 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) @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) @recent = Topic.includes(:category).where.not(id: category_topic_ids).recent(10)
@topics_partial = render_to_string partial: '/exceptions/not_found_topics', formats: [:html] render_to_string partial: '/exceptions/not_found_topics', formats: [:html]
Discourse.redis.setex(key, 10.minutes, @topics_partial) end.html_safe
end
end end
@container_class = "wrap not-found-container" @container_class = "wrap not-found-container"

View File

@ -373,7 +373,7 @@ RSpec.describe ApplicationController do
it 'should handle 404 to a css file' do it 'should handle 404 to a css file' do
Discourse.redis.del("page_not_found_topics") Discourse.cache.delete("page_not_found_topics:#{I18n.locale}")
topic1 = Fabricate(:topic) topic1 = Fabricate(:topic)
get '/stylesheets/mobile_1_4cd559272273fe6d3c7db620c617d596a5fdf240.css', headers: { 'HTTP_ACCEPT' => 'text/css,*/*,q=0.1' } get '/stylesheets/mobile_1_4cd559272273fe6d3c7db620c617d596a5fdf240.css', headers: { 'HTTP_ACCEPT' => 'text/css,*/*,q=0.1' }
@ -394,7 +394,8 @@ RSpec.describe ApplicationController do
end end
it 'should cache results' do it 'should cache results' do
Discourse.redis.del("page_not_found_topics") Discourse.cache.delete("page_not_found_topics:#{I18n.locale}")
Discourse.cache.delete("page_not_found_topics:fr")
topic1 = Fabricate(:topic) topic1 = Fabricate(:topic)
get '/t/nope-nope/99999999' get '/t/nope-nope/99999999'
@ -406,6 +407,13 @@ RSpec.describe ApplicationController do
expect(response.status).to eq(404) expect(response.status).to eq(404)
expect(response.body).to include(topic1.title) expect(response.body).to include(topic1.title)
expect(response.body).to_not include(topic2.title) expect(response.body).to_not include(topic2.title)
# Different locale should have different cache
SiteSetting.default_locale = :fr
get '/t/nope-nope/99999999'
expect(response.status).to eq(404)
expect(response.body).to include(topic1.title)
expect(response.body).to include(topic2.title)
end end
end end
end end