Remove caching in cache_collection (mastodon#29862)

app/controllers/concerns/cache_concern.rb

@@ -28,29 +28,19 @@ def set_cache_headers
     response.headers['Vary'] = public_fetch_mode? ? 'Accept, Authorization' : 'Accept, Signature, Authorization'
   end
 
+  # TODO: Rename this method, as it does not perform any caching anymore.
   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)
-    return [] if raw.empty?
+    records = raw.to_a
 
-    cached_keys_with_value = Rails.cache.read_multi(*raw).transform_keys(&:id)
-    uncached_ids           = raw.map(&:id) - cached_keys_with_value.keys
+    klass.preload_cacheable_associations(records)
 
-    klass.reload_stale_associations!(cached_keys_with_value.values) if klass.respond_to?(:reload_stale_associations!)
-
-    unless uncached_ids.empty?
-      uncached = klass.unscoped.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] }
+    records
   end
 
+  # TODO: Rename this method, as it does not perform any caching anymore.
   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

app/models/concerns/cacheable.rb

@@ -14,6 +14,10 @@ def with_includes
       includes(@cache_associated)
     end
 
+    def preload_cacheable_associations(records)
+      ActiveRecord::Associations::Preloader.new.preload(records, @cache_associated)
+    end
+
     def cache_ids
       select(:id, :updated_at)
     end

app/models/feed.rb

@@ -31,7 +31,7 @@ def from_redis(limit, max_id, since_id, min_id, visibilities)
     statuses = Status.where(id: unhydrated)
     statuses = statuses.joins(:account).merge(Account.without_suspended)
     statuses = statuses.where(visibility: visibilities).limit(limit) unless visibilities.empty?
-    statuses.cache_ids
+    statuses
   end
 
   def key

app/models/group_feed.rb

@@ -26,7 +26,7 @@ def get(limit, max_id = nil, since_id = nil, min_id = nil)
     scope.merge!(without_bot_scope) if without_bot?
     scope.merge!(hashtag_scope) if tagged?
 
-    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
 
   private

app/models/personal_feed.rb

@@ -21,7 +21,7 @@ def get(limit, max_id = nil, since_id = nil, min_id = nil)
     scope.merge!(media_only_scope) if media_only?
     scope.merge!(without_media_scope) if without_media?
 
-    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
 
   private

app/models/public_feed.rb

@@ -36,7 +36,7 @@ def get(limit, max_id = nil, since_id = nil, min_id = nil)
     scope.merge!(without_media_scope) if without_media?
     scope.merge!(without_bot_scope) if without_bot?
 
-    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
 
   private

app/models/status.rb

@@ -607,26 +607,6 @@ def relations_map_for_status(account_id, statuses)
       }
     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!
-
-      return if account_ids.empty?
-
-      accounts = Account.where(id: account_ids).includes(:account_stat, :user).index_by(&:id)
-
-      cached_items.each do |item|
-        item.account = accounts[item.account_id]
-        item.reblog.account = accounts[item.reblog.account_id] if item.reblog?
-      end
-    end
-
     def permitted_statuses_from_ids(ids, account_id)
       statuses          = Status.with_accounts(ids).to_a
       account_ids       = statuses.map(&:account_id).uniq
@@ -660,7 +640,7 @@ def permitted_for(target_account, account)
       end
     end
 
-    def from_0(text)
+    def from_text(text)
       return [] if text.blank?
 
       text.scan(FetchLinkCardService::URL_PATTERN).map(&:first).uniq.filter_map do |url|

app/models/tag_feed.rb

@@ -37,7 +37,7 @@ def get(limit, max_id = nil, since_id = nil, min_id = nil)
     scope.merge!(without_media_scope) if without_media?
     scope.merge!(without_bot_scope) if without_bot?
 
-    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
 
   private

spec/controllers/application_controller_spec.rb

@@ -306,37 +306,4 @@ def route_unprocessable_entity
 
     include_examples 'respond_with_error', 422
   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

spec/models/home_feed_spec.rb

@@ -25,7 +25,6 @@
         results = subject.get(3)
 
         expect(results.map(&:id)).to eq [3, 2]
-        expect(results.first.attributes.keys).to eq %w(id updated_at)
       end
     end