Remove caching in cache_collection
(#29862)
This commit is contained in:
parent
ba5551fd1d
commit
5973d7a4b6
8 changed files with 14 additions and 86 deletions
|
@ -28,29 +28,19 @@ module CacheConcern
|
||||||
response.headers['Vary'] = public_fetch_mode? ? 'Accept' : 'Accept, Signature'
|
response.headers['Vary'] = public_fetch_mode? ? 'Accept' : 'Accept, Signature'
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# TODO: Rename this method, as it does not perform any caching anymore.
|
||||||
def cache_collection(raw, klass)
|
def cache_collection(raw, klass)
|
||||||
return raw unless klass.respond_to?(:with_includes)
|
return raw unless klass.respond_to?(:preload_cacheable_associations)
|
||||||
|
|
||||||
raw = raw.cache_ids.to_a if raw.is_a?(ActiveRecord::Relation)
|
records = raw.to_a
|
||||||
return [] if raw.empty?
|
|
||||||
|
|
||||||
cached_keys_with_value = Rails.cache.read_multi(*raw).transform_keys(&:id)
|
klass.preload_cacheable_associations(records)
|
||||||
uncached_ids = raw.map(&:id) - cached_keys_with_value.keys
|
|
||||||
|
|
||||||
klass.reload_stale_associations!(cached_keys_with_value.values) if klass.respond_to?(:reload_stale_associations!)
|
records
|
||||||
|
|
||||||
unless uncached_ids.empty?
|
|
||||||
uncached = klass.where(id: uncached_ids).with_includes.index_by(&:id)
|
|
||||||
|
|
||||||
uncached.each_value do |item|
|
|
||||||
Rails.cache.write(item, item)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
raw.filter_map { |item| cached_keys_with_value[item.id] || uncached[item.id] }
|
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# TODO: Rename this method, as it does not perform any caching anymore.
|
||||||
def cache_collection_paginated_by_id(raw, klass, limit, options)
|
def cache_collection_paginated_by_id(raw, klass, limit, options)
|
||||||
cache_collection raw.cache_ids.to_a_paginated_by_id(limit, options), klass
|
cache_collection raw.to_a_paginated_by_id(limit, options), klass
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -14,6 +14,10 @@ module Cacheable
|
||||||
includes(@cache_associated)
|
includes(@cache_associated)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def preload_cacheable_associations(records)
|
||||||
|
ActiveRecord::Associations::Preloader.new.preload(records, @cache_associated)
|
||||||
|
end
|
||||||
|
|
||||||
def cache_ids
|
def cache_ids
|
||||||
select(:id, :updated_at)
|
select(:id, :updated_at)
|
||||||
end
|
end
|
||||||
|
|
|
@ -28,7 +28,7 @@ class Feed
|
||||||
unhydrated = redis.zrangebyscore(key, "(#{min_id}", "(#{max_id}", limit: [0, limit], with_scores: true).map(&:first).map(&:to_i)
|
unhydrated = redis.zrangebyscore(key, "(#{min_id}", "(#{max_id}", limit: [0, limit], with_scores: true).map(&:first).map(&:to_i)
|
||||||
end
|
end
|
||||||
|
|
||||||
Status.where(id: unhydrated).cache_ids
|
Status.where(id: unhydrated)
|
||||||
end
|
end
|
||||||
|
|
||||||
def key
|
def key
|
||||||
|
|
|
@ -29,7 +29,7 @@ class PublicFeed
|
||||||
scope.merge!(media_only_scope) if media_only?
|
scope.merge!(media_only_scope) if media_only?
|
||||||
scope.merge!(language_scope) if account&.chosen_languages.present?
|
scope.merge!(language_scope) if account&.chosen_languages.present?
|
||||||
|
|
||||||
scope.cache_ids.to_a_paginated_by_id(limit, max_id: max_id, since_id: since_id, min_id: min_id)
|
scope.to_a_paginated_by_id(limit, max_id: max_id, since_id: since_id, min_id: min_id)
|
||||||
end
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
|
@ -344,38 +344,6 @@ class Status < ApplicationRecord
|
||||||
StatusPin.select('status_id').where(status_id: status_ids).where(account_id: account_id).each_with_object({}) { |p, h| h[p.status_id] = true }
|
StatusPin.select('status_id').where(status_id: status_ids).where(account_id: account_id).each_with_object({}) { |p, h| h[p.status_id] = true }
|
||||||
end
|
end
|
||||||
|
|
||||||
def reload_stale_associations!(cached_items)
|
|
||||||
account_ids = []
|
|
||||||
|
|
||||||
cached_items.each do |item|
|
|
||||||
account_ids << item.account_id
|
|
||||||
account_ids << item.reblog.account_id if item.reblog?
|
|
||||||
end
|
|
||||||
|
|
||||||
account_ids.uniq!
|
|
||||||
|
|
||||||
status_ids = cached_items.map { |item| item.reblog? ? item.reblog_of_id : item.id }.uniq
|
|
||||||
|
|
||||||
return if account_ids.empty?
|
|
||||||
|
|
||||||
accounts = Account.where(id: account_ids).includes(:account_stat, :user).index_by(&:id)
|
|
||||||
|
|
||||||
status_stats = StatusStat.where(status_id: status_ids).index_by(&:status_id)
|
|
||||||
|
|
||||||
cached_items.each do |item|
|
|
||||||
item.account = accounts[item.account_id]
|
|
||||||
item.reblog.account = accounts[item.reblog.account_id] if item.reblog?
|
|
||||||
|
|
||||||
if item.reblog?
|
|
||||||
status_stat = status_stats[item.reblog.id]
|
|
||||||
item.reblog.status_stat = status_stat if status_stat.present?
|
|
||||||
else
|
|
||||||
status_stat = status_stats[item.id]
|
|
||||||
item.status_stat = status_stat if status_stat.present?
|
|
||||||
end
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
def from_text(text)
|
def from_text(text)
|
||||||
return [] if text.blank?
|
return [] if text.blank?
|
||||||
|
|
||||||
|
|
|
@ -33,7 +33,7 @@ class TagFeed < PublicFeed
|
||||||
scope.merge!(account_filters_scope) if account?
|
scope.merge!(account_filters_scope) if account?
|
||||||
scope.merge!(media_only_scope) if media_only?
|
scope.merge!(media_only_scope) if media_only?
|
||||||
|
|
||||||
scope.cache_ids.to_a_paginated_by_id(limit, max_id: max_id, since_id: since_id, min_id: min_id)
|
scope.to_a_paginated_by_id(limit, max_id: max_id, since_id: since_id, min_id: min_id)
|
||||||
end
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
|
@ -242,37 +242,4 @@ describe ApplicationController, type: :controller do
|
||||||
|
|
||||||
include_examples 'respond_with_error', 422
|
include_examples 'respond_with_error', 422
|
||||||
end
|
end
|
||||||
|
|
||||||
describe 'cache_collection' do
|
|
||||||
class C < ApplicationController
|
|
||||||
public :cache_collection
|
|
||||||
end
|
|
||||||
|
|
||||||
shared_examples 'receives :with_includes' do |fabricator, klass|
|
|
||||||
it 'uses raw if it is not an ActiveRecord::Relation' do
|
|
||||||
record = Fabricate(fabricator)
|
|
||||||
expect(C.new.cache_collection([record], klass)).to eq [record]
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
shared_examples 'cacheable' do |fabricator, klass|
|
|
||||||
include_examples 'receives :with_includes', fabricator, klass
|
|
||||||
|
|
||||||
it 'calls cache_ids of raw if it is an ActiveRecord::Relation' do
|
|
||||||
record = Fabricate(fabricator)
|
|
||||||
relation = klass.none
|
|
||||||
allow(relation).to receive(:cache_ids).and_return([record])
|
|
||||||
expect(C.new.cache_collection(relation, klass)).to eq [record]
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
it 'returns raw unless class responds to :with_includes' do
|
|
||||||
raw = Object.new
|
|
||||||
expect(C.new.cache_collection(raw, Object)).to eq raw
|
|
||||||
end
|
|
||||||
|
|
||||||
context 'Status' do
|
|
||||||
include_examples 'cacheable', :status, Status
|
|
||||||
end
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|
|
@ -25,7 +25,6 @@ RSpec.describe HomeFeed, type: :model do
|
||||||
results = subject.get(3)
|
results = subject.get(3)
|
||||||
|
|
||||||
expect(results.map(&:id)).to eq [3, 2]
|
expect(results.map(&:id)).to eq [3, 2]
|
||||||
expect(results.first.attributes.keys).to eq %w(id updated_at)
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue