Skip to content

Commit ee9e02a

Browse files
authored
Merge pull request #1740 from codidact/art/1738/activerecord-caching
Handle ActiveRecord caching properly
2 parents 16a4657 + 5a2f36a commit ee9e02a

File tree

11 files changed

+142
-20
lines changed

11 files changed

+142
-20
lines changed

app/controllers/advertisement_controller.rb

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,10 @@ def specific_question
3535

3636
def specific_category
3737
@category = Category.unscoped.find(params[:id])
38-
@post = Rails.cache.fetch "ca_random_category_post/#{params[:id]}",
39-
expires_in: 5.minutes do
40-
select_random_post(@category, days: params[:days]&.to_i, score: params[:score]&.to_f)
41-
end
38+
@post = Rails.cache.fetch_collection "ca_random_category_post/#{params[:id]}",
39+
expires_in: 5.minutes do
40+
random_post_collection(@category, days: params[:days]&.to_i, score: params[:score]&.to_f)
41+
end.sample
4242

4343
if @post.nil?
4444
not_found!
@@ -53,9 +53,9 @@ def specific_category
5353
end
5454

5555
def random_question
56-
@post = Rails.cache.fetch 'ca_random_hot_post', expires_in: 5.minutes do
57-
select_random_post
58-
end
56+
@post = Rails.cache.fetch_collection 'ca_random_hot_post', expires_in: 5.minutes do
57+
random_post_collection
58+
end.sample
5959
if @post.nil?
6060
return community
6161
end
@@ -95,7 +95,7 @@ def promoted_post
9595
# @param score [Float] the minimum post score to consider
9696
# @param count [Integer] a maximum number of posts to query for; the final post will be randomly selected from this
9797
# @return [Post]
98-
def select_random_post(category = nil, days: nil, score: nil, count: nil)
98+
def random_post_collection(category = nil, days: nil, score: nil, count: nil)
9999
if category.nil?
100100
category = Category.where(use_for_advertisement: true)
101101
end
@@ -106,7 +106,7 @@ def select_random_post(category = nil, days: nil, score: nil, count: nil)
106106
.where(posts: { last_activity: days.days.ago..DateTime.now })
107107
.where(posts: { category: category })
108108
.where('posts.score > ?', score.nil? ? SiteSetting['HotPostsScoreThreshold'] : score)
109-
.order('posts.score DESC').limit(count.nil? ? SiteSetting['HotQuestionsCount'] : count).all.sample
109+
.order('posts.score DESC').limit(count.nil? ? SiteSetting['HotQuestionsCount'] : count).all
110110
end
111111

112112
def send_resp(data)

app/controllers/application_controller.rb

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -205,9 +205,10 @@ def setup_request_context
205205
RequestContext.clear!
206206

207207
host_name = request.raw_host_with_port # include port to support multiple localhost instances
208-
RequestContext.community = @community = Rails.cache.fetch("#{host_name}/community", expires_in: 1.hour) do
209-
Community.unscoped.find_by(host: host_name)
210-
end
208+
@community = Rails.cache.fetch_collection("#{host_name}/community", expires_in: 1.hour) do
209+
Community.unscoped.where(host: host_name)
210+
end.first
211+
RequestContext.community = @community
211212

212213
Rails.logger.info " Host #{host_name}, community ##{RequestContext.community_id} " \
213214
"(#{RequestContext.community&.name})"
@@ -230,7 +231,7 @@ def setup_user
230231
end
231232

232233
def pull_pinned_links_and_hot_questions
233-
@pinned_links = Rails.cache.fetch('pinned_links', expires_in: 2.hours) do
234+
@pinned_links = Rails.cache.fetch_collection('pinned_links', expires_in: 2.hours) do
234235
Rack::MiniProfiler.step 'pinned_links: cache miss' do
235236
PinnedLink.where(active: true).where('shown_before IS NULL OR shown_before > NOW()').all
236237
end
@@ -247,7 +248,7 @@ def pull_pinned_links_and_hot_questions
247248
# I.e., if pinned_post_ids contains null, the selection will never return records
248249
pinned_post_ids = @pinned_links.map(&:post_id).compact
249250

250-
@hot_questions = Rails.cache.fetch('hot_questions', expires_in: 4.hours) do
251+
@hot_questions = Rails.cache.fetch_collection('hot_questions', expires_in: 4.hours) do
251252
Rack::MiniProfiler.step 'hot_questions: cache miss' do
252253
Post.undeleted.not_locked.where(closed: false)
253254
.where(last_activity: (Rails.env.development? ? 365 : 7).days.ago..DateTime.now)
@@ -261,7 +262,7 @@ def pull_pinned_links_and_hot_questions
261262
end
262263

263264
def pull_categories
264-
@header_categories = Rails.cache.fetch('header_categories') do
265+
@header_categories = Rails.cache.fetch_collection('header_categories') do
265266
Category.all.order(sequence: :asc, id: :asc)
266267
end
267268
end

app/helpers/advertisements/article_helper.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ module Advertisements::ArticleHelper
66
# rubocop:disable Metrics/MethodLength
77
# rubocop:disable Metrics/BlockLength
88
def article_ad(article)
9+
# TODO: trying to cache like this is probably a terrible idea - review options
910
Rails.cache.fetch "posts/#{article.id}/ad", expires_in: 60.minutes do
1011
ad = Image.new(600, 500)
1112
ad.background_color = 'white'

app/helpers/advertisements/codidact_helper.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ module Advertisements::CodidactHelper
55

66
# rubocop:disable Metrics/BlockLength
77
def codidact_ad
8+
# TODO: trying to cache like this is probably a terrible idea - review options
89
Rails.cache.fetch 'network/codidact_ad', expires_in: 60.minutes, include_community: false do
910
ad = Image.new(600, 500)
1011
ad.background_color = 'white'

app/helpers/advertisements/community_helper.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ module Advertisements::CommunityHelper
55

66
# rubocop:disable Metrics/BlockLength
77
def community_ad
8+
# TODO: trying to cache like this is probably a terrible idea - review options
89
Rails.cache.fetch 'community_ad', expires_in: 60.minutes do
910
ad = Image.new(600, 500)
1011
ad.background_color = 'white'

app/helpers/advertisements/question_helper.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ module Advertisements::QuestionHelper
66
# rubocop:disable Metrics/MethodLength
77
# rubocop:disable Metrics/BlockLength
88
def question_ad(question)
9+
# TODO: trying to cache like this is probably a terrible idea - review options
910
Rails.cache.fetch "posts/#{question.id}/ad", expires_in: 60.minutes do
1011
ad = Image.new(600, 500)
1112
ad.background_color = 'white'

app/helpers/users/avatar_helper.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ def user_auto_avatar(size, user: nil, letter: nil, color: nil)
3737
"network/avatars/#{letter}+#{color}/#{size}px"
3838
end
3939

40+
# TODO: trying to cache like this is probably a terrible idea - review options
4041
Rails.cache.fetch cache_key, include_community: false, expires_in: 24.hours do
4142
ava = Image.new(size, size)
4243
text_color = yiq_contrast(color, 'black', 'white')

app/models/category.rb

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,12 @@ def self.accessible_to(user)
5656

5757
def self.by_lowercase_name(name)
5858
categories = Rails.cache.fetch 'categories/by_lowercase_name' do
59-
Category.all.to_h { |c| [c.name.downcase, c] }
59+
Category.all.to_h { |c| [c.name.downcase, c.id] }
6060
end
61-
categories[name]
61+
Category.find_by(id: categories[name])
6262
end
6363

64+
# @todo: Do we need this method?
6465
def self.by_id(id)
6566
categories = Rails.cache.fetch 'categories/by_id' do
6667
Category.all.to_h { |c| [c.id, c] }

app/models/post_type.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ class PostType < ApplicationRecord
88
validates :answer_type_id, presence: true, if: :has_answers?
99

1010
def reactions
11-
Rails.cache.fetch "post_type/#{name}/reactions" do
11+
Rails.cache.fetch_collection "post_type/#{name}/reactions" do
1212
return [] unless has_reactions
1313

1414
if has_only_specific_reactions
@@ -39,13 +39,13 @@ def self.[](key)
3939
end
4040

4141
def self.top_level
42-
Rails.cache.fetch 'network/post_types/top_level', include_community: false do
42+
Rails.cache.fetch_collection 'network/post_types/top_level', include_community: false do
4343
where(is_top_level: true)
4444
end
4545
end
4646

4747
def self.second_level
48-
Rails.cache.fetch 'network/post_types/second_level', include_community: false do
48+
Rails.cache.fetch_collection 'network/post_types/second_level', include_community: false do
4949
where(is_top_level: false)
5050
end
5151
end

lib/namespaced_env_cache.rb

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,65 @@ def persistent(name, **opts, &block)
7272
end
7373
end
7474

75+
##
76+
# Cache an ActiveRecord collection. Supports only a basic collection of one type of object. Column selections or
77+
# joins etc. will NOT be respected when the collection is read back out.
78+
# @param name [String] cache key name
79+
# @param value [ActiveRecord::Relation] collection to cache
80+
# @param opts [Hash] options hash - any unlisted options will be passed to the underlying cache
81+
# @option opts [Boolean] :include_community whether to include the community ID in the cache key
82+
def write_collection(name, value, **opts)
83+
types = value.map(&:class).uniq
84+
if types.size > 1
85+
raise TypeError, "Can't cache more than one type of object via write_collection"
86+
end
87+
88+
data = [types[0].to_s, *value.map(&:id)]
89+
namespaced = construct_ns_key(name, include_community: include_community(opts))
90+
@underlying.write(namespaced, data, **opts)
91+
end
92+
93+
##
94+
# Read an ActiveRecord collection from cache. Returns a basic collection of the records that were cached, with
95+
# no selects or joins applied.
96+
# @param name [String] cache key name
97+
# @param opts [Hash] options hash - any unlisted options will be passed to the underlying cache
98+
# @options opts [Boolean] :include_community whether to include the community ID in the cache key
99+
def read_collection(name, **opts)
100+
namespaced = construct_ns_key(name, include_community: include_community(opts))
101+
data = @underlying.read(namespaced, **opts)
102+
return nil if data.nil?
103+
type = data.slice!(0)
104+
begin
105+
type.constantize.where(id: data)
106+
rescue NameError
107+
delete(name)
108+
nil
109+
end
110+
end
111+
112+
##
113+
# Fetch an ActiveRecord collection from cache if it is present, otherwise cache the value returned by +block+.
114+
# @param name [String] cache key name
115+
# @param opts [Hash] options hash - any unlisted options will be passed to the underlying cache
116+
# @option opts [Boolean] :include_community whether to include the community ID in the cache key
117+
# @yieldreturn [ActiveRecord::Relation]
118+
def fetch_collection(name, **opts, &block)
119+
existing = if exist?(name, include_community: include_community(opts))
120+
read_collection(name, **opts)
121+
end
122+
if existing.nil?
123+
unless block_given?
124+
raise ArgumentError, "Can't fetch collection without a block given"
125+
end
126+
data = block.call
127+
write_collection(name, data, **opts)
128+
data
129+
else
130+
existing
131+
end
132+
end
133+
75134
# We have to statically report that we support cache versioning even though this depends on the underlying class.
76135
# However, this is not really a problem since all cache stores provided by activesupport support the feature and
77136
# we only use the redis cache (by activesupport) for QPixel.

0 commit comments

Comments
 (0)