Rename cache_* methods to preload_* in controller concern (#30209)
				
					
				
			This commit is contained in:
		@@ -25,7 +25,7 @@ class AccountsController < ApplicationController
 | 
			
		||||
 | 
			
		||||
        limit     = params[:limit].present? ? [params[:limit].to_i, PAGE_SIZE_MAX].min : PAGE_SIZE
 | 
			
		||||
        @statuses = filtered_statuses.without_reblogs.limit(limit)
 | 
			
		||||
        @statuses = cache_collection(@statuses, Status)
 | 
			
		||||
        @statuses = preload_collection(@statuses, Status)
 | 
			
		||||
      end
 | 
			
		||||
 | 
			
		||||
      format.json do
 | 
			
		||||
 
 | 
			
		||||
@@ -18,7 +18,7 @@ class ActivityPub::CollectionsController < ActivityPub::BaseController
 | 
			
		||||
  def set_items
 | 
			
		||||
    case params[:id]
 | 
			
		||||
    when 'featured'
 | 
			
		||||
      @items = for_signed_account { cache_collection(@account.pinned_statuses, Status) }
 | 
			
		||||
      @items = for_signed_account { preload_collection(@account.pinned_statuses, Status) }
 | 
			
		||||
      @items = @items.map { |item| item.distributable? ? item : ActivityPub::TagManager.instance.uri_for(item) }
 | 
			
		||||
    when 'tags'
 | 
			
		||||
      @items = for_signed_account { @account.featured_tags }
 | 
			
		||||
 
 | 
			
		||||
@@ -60,7 +60,7 @@ class ActivityPub::OutboxesController < ActivityPub::BaseController
 | 
			
		||||
  def set_statuses
 | 
			
		||||
    return unless page_requested?
 | 
			
		||||
 | 
			
		||||
    @statuses = cache_collection_paginated_by_id(
 | 
			
		||||
    @statuses = preload_collection_paginated_by_id(
 | 
			
		||||
      AccountStatusesFilter.new(@account, signed_request_account).results,
 | 
			
		||||
      Status,
 | 
			
		||||
      LIMIT,
 | 
			
		||||
 
 | 
			
		||||
@@ -19,11 +19,11 @@ class Api::V1::Accounts::StatusesController < Api::BaseController
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  def load_statuses
 | 
			
		||||
    @account.unavailable? ? [] : cached_account_statuses
 | 
			
		||||
    @account.unavailable? ? [] : preloaded_account_statuses
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  def cached_account_statuses
 | 
			
		||||
    cache_collection_paginated_by_id(
 | 
			
		||||
  def preloaded_account_statuses
 | 
			
		||||
    preload_collection_paginated_by_id(
 | 
			
		||||
      AccountStatusesFilter.new(@account, current_account, params).results,
 | 
			
		||||
      Status,
 | 
			
		||||
      limit_param(DEFAULT_STATUSES_LIMIT),
 | 
			
		||||
 
 | 
			
		||||
@@ -13,11 +13,11 @@ class Api::V1::BookmarksController < Api::BaseController
 | 
			
		||||
  private
 | 
			
		||||
 | 
			
		||||
  def load_statuses
 | 
			
		||||
    cached_bookmarks
 | 
			
		||||
    preloaded_bookmarks
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  def cached_bookmarks
 | 
			
		||||
    cache_collection(results.map(&:status), Status)
 | 
			
		||||
  def preloaded_bookmarks
 | 
			
		||||
    preload_collection(results.map(&:status), Status)
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  def results
 | 
			
		||||
 
 | 
			
		||||
@@ -13,11 +13,11 @@ class Api::V1::FavouritesController < Api::BaseController
 | 
			
		||||
  private
 | 
			
		||||
 | 
			
		||||
  def load_statuses
 | 
			
		||||
    cached_favourites
 | 
			
		||||
    preloaded_favourites
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  def cached_favourites
 | 
			
		||||
    cache_collection(results.map(&:status), Status)
 | 
			
		||||
  def preloaded_favourites
 | 
			
		||||
    preload_collection(results.map(&:status), Status)
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  def results
 | 
			
		||||
 
 | 
			
		||||
@@ -41,7 +41,7 @@ class Api::V1::Notifications::RequestsController < Api::BaseController
 | 
			
		||||
    )
 | 
			
		||||
 | 
			
		||||
    NotificationRequest.preload_cache_collection(requests) do |statuses|
 | 
			
		||||
      cache_collection(statuses, Status)
 | 
			
		||||
      preload_collection(statuses, Status)
 | 
			
		||||
    end
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
 
 | 
			
		||||
@@ -41,7 +41,7 @@ class Api::V1::NotificationsController < Api::BaseController
 | 
			
		||||
    )
 | 
			
		||||
 | 
			
		||||
    Notification.preload_cache_collection_target_statuses(notifications) do |target_statuses|
 | 
			
		||||
      cache_collection(target_statuses, Status)
 | 
			
		||||
      preload_collection(target_statuses, Status)
 | 
			
		||||
    end
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
 
 | 
			
		||||
@@ -26,13 +26,13 @@ class Api::V1::StatusesController < Api::BaseController
 | 
			
		||||
  DESCENDANTS_DEPTH_LIMIT = 20
 | 
			
		||||
 | 
			
		||||
  def index
 | 
			
		||||
    @statuses = cache_collection(@statuses, Status)
 | 
			
		||||
    @statuses = preload_collection(@statuses, Status)
 | 
			
		||||
    render json: @statuses, each_serializer: REST::StatusSerializer
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  def show
 | 
			
		||||
    cache_if_unauthenticated!
 | 
			
		||||
    @status = cache_collection([@status], Status).first
 | 
			
		||||
    @status = preload_collection([@status], Status).first
 | 
			
		||||
    render json: @status, serializer: REST::StatusSerializer
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
@@ -51,8 +51,8 @@ class Api::V1::StatusesController < Api::BaseController
 | 
			
		||||
 | 
			
		||||
    ancestors_results   = @status.in_reply_to_id.nil? ? [] : @status.ancestors(ancestors_limit, current_account)
 | 
			
		||||
    descendants_results = @status.descendants(descendants_limit, current_account, descendants_depth_limit)
 | 
			
		||||
    loaded_ancestors    = cache_collection(ancestors_results, Status)
 | 
			
		||||
    loaded_descendants  = cache_collection(descendants_results, Status)
 | 
			
		||||
    loaded_ancestors    = preload_collection(ancestors_results, Status)
 | 
			
		||||
    loaded_descendants  = preload_collection(descendants_results, Status)
 | 
			
		||||
 | 
			
		||||
    @context = Context.new(ancestors: loaded_ancestors, descendants: loaded_descendants)
 | 
			
		||||
    statuses = [@status] + @context.ancestors + @context.descendants
 | 
			
		||||
 
 | 
			
		||||
@@ -21,11 +21,11 @@ class Api::V1::Timelines::HomeController < Api::V1::Timelines::BaseController
 | 
			
		||||
  private
 | 
			
		||||
 | 
			
		||||
  def load_statuses
 | 
			
		||||
    cached_home_statuses
 | 
			
		||||
    preloaded_home_statuses
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  def cached_home_statuses
 | 
			
		||||
    cache_collection home_statuses, Status
 | 
			
		||||
  def preloaded_home_statuses
 | 
			
		||||
    preload_collection home_statuses, Status
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  def home_statuses
 | 
			
		||||
 
 | 
			
		||||
@@ -21,11 +21,11 @@ class Api::V1::Timelines::ListController < Api::V1::Timelines::BaseController
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  def set_statuses
 | 
			
		||||
    @statuses = cached_list_statuses
 | 
			
		||||
    @statuses = preloaded_list_statuses
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  def cached_list_statuses
 | 
			
		||||
    cache_collection list_statuses, Status
 | 
			
		||||
  def preloaded_list_statuses
 | 
			
		||||
    preload_collection list_statuses, Status
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  def list_statuses
 | 
			
		||||
 
 | 
			
		||||
@@ -18,11 +18,11 @@ class Api::V1::Timelines::PublicController < Api::V1::Timelines::BaseController
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  def load_statuses
 | 
			
		||||
    cached_public_statuses_page
 | 
			
		||||
    preloaded_public_statuses_page
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  def cached_public_statuses_page
 | 
			
		||||
    cache_collection(public_statuses, Status)
 | 
			
		||||
  def preloaded_public_statuses_page
 | 
			
		||||
    preload_collection(public_statuses, Status)
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  def public_statuses
 | 
			
		||||
 
 | 
			
		||||
@@ -23,11 +23,11 @@ class Api::V1::Timelines::TagController < Api::V1::Timelines::BaseController
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  def load_statuses
 | 
			
		||||
    cached_tagged_statuses
 | 
			
		||||
    preloaded_tagged_statuses
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  def cached_tagged_statuses
 | 
			
		||||
    @tag.nil? ? [] : cache_collection(tag_timeline_statuses, Status)
 | 
			
		||||
  def preloaded_tagged_statuses
 | 
			
		||||
    @tag.nil? ? [] : preload_collection(tag_timeline_statuses, Status)
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  def tag_timeline_statuses
 | 
			
		||||
 
 | 
			
		||||
@@ -20,7 +20,7 @@ class Api::V1::Trends::StatusesController < Api::BaseController
 | 
			
		||||
 | 
			
		||||
  def set_statuses
 | 
			
		||||
    @statuses = if enabled?
 | 
			
		||||
                  cache_collection(statuses_from_trends.offset(offset_param).limit(limit_param(DEFAULT_STATUSES_LIMIT)), Status)
 | 
			
		||||
                  preload_collection(statuses_from_trends.offset(offset_param).limit(limit_param(DEFAULT_STATUSES_LIMIT)), Status)
 | 
			
		||||
                else
 | 
			
		||||
                  []
 | 
			
		||||
                end
 | 
			
		||||
 
 | 
			
		||||
@@ -9,6 +9,7 @@ class ApplicationController < ActionController::Base
 | 
			
		||||
  include UserTrackingConcern
 | 
			
		||||
  include SessionTrackingConcern
 | 
			
		||||
  include CacheConcern
 | 
			
		||||
  include PreloadingConcern
 | 
			
		||||
  include DomainControlHelper
 | 
			
		||||
  include DatabaseHelper
 | 
			
		||||
  include AuthorizedFetchHelper
 | 
			
		||||
 
 | 
			
		||||
@@ -45,20 +45,4 @@ module CacheConcern
 | 
			
		||||
      Rails.cache.write(key, response.body, expires_in: expires_in, raw: true)
 | 
			
		||||
    end
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  # TODO: Rename this method, as it does not perform any caching anymore.
 | 
			
		||||
  def cache_collection(raw, klass)
 | 
			
		||||
    return raw unless klass.respond_to?(:preload_cacheable_associations)
 | 
			
		||||
 | 
			
		||||
    records = raw.to_a
 | 
			
		||||
 | 
			
		||||
    klass.preload_cacheable_associations(records)
 | 
			
		||||
 | 
			
		||||
    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.to_a_paginated_by_id(limit, options), klass
 | 
			
		||||
  end
 | 
			
		||||
end
 | 
			
		||||
 
 | 
			
		||||
							
								
								
									
										17
									
								
								app/controllers/concerns/preloading_concern.rb
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										17
									
								
								app/controllers/concerns/preloading_concern.rb
									
									
									
									
									
										Normal file
									
								
							@@ -0,0 +1,17 @@
 | 
			
		||||
# frozen_string_literal: true
 | 
			
		||||
 | 
			
		||||
module PreloadingConcern
 | 
			
		||||
  extend ActiveSupport::Concern
 | 
			
		||||
 | 
			
		||||
  def preload_collection(scope, klass)
 | 
			
		||||
    return scope unless klass.respond_to?(:preload_cacheable_associations)
 | 
			
		||||
 | 
			
		||||
    scope.to_a.tap do |records|
 | 
			
		||||
      klass.preload_cacheable_associations(records)
 | 
			
		||||
    end
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  def preload_collection_paginated_by_id(scope, klass, limit, options)
 | 
			
		||||
    preload_collection scope.to_a_paginated_by_id(limit, options), klass
 | 
			
		||||
  end
 | 
			
		||||
end
 | 
			
		||||
@@ -45,7 +45,7 @@ class TagsController < ApplicationController
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  def set_statuses
 | 
			
		||||
    @statuses = cache_collection(TagFeed.new(@tag, nil, local: @local).get(limit_param), Status)
 | 
			
		||||
    @statuses = preload_collection(TagFeed.new(@tag, nil, local: @local).get(limit_param), Status)
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  def limit_param
 | 
			
		||||
 
 | 
			
		||||
@@ -2,20 +2,20 @@
 | 
			
		||||
 | 
			
		||||
require 'rails_helper'
 | 
			
		||||
 | 
			
		||||
RSpec.describe CacheConcern do
 | 
			
		||||
RSpec.describe PreloadingConcern do
 | 
			
		||||
  controller(ApplicationController) do
 | 
			
		||||
    include CacheConcern
 | 
			
		||||
    include PreloadingConcern
 | 
			
		||||
 | 
			
		||||
    def empty_array
 | 
			
		||||
      render plain: cache_collection([], Status).size
 | 
			
		||||
      render plain: preload_collection([], Status).size
 | 
			
		||||
    end
 | 
			
		||||
 | 
			
		||||
    def empty_relation
 | 
			
		||||
      render plain: cache_collection(Status.none, Status).size
 | 
			
		||||
      render plain: preload_collection(Status.none, Status).size
 | 
			
		||||
    end
 | 
			
		||||
 | 
			
		||||
    def account_statuses_favourites
 | 
			
		||||
      render plain: cache_collection(Status.where(account_id: params[:id]), Status).map(&:favourites_count)
 | 
			
		||||
      render plain: preload_collection(Status.where(account_id: params[:id]), Status).map(&:favourites_count)
 | 
			
		||||
    end
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
@@ -27,7 +27,7 @@ RSpec.describe CacheConcern do
 | 
			
		||||
    end
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  describe '#cache_collection' do
 | 
			
		||||
  describe '#preload_collection' do
 | 
			
		||||
    context 'when given an empty array' do
 | 
			
		||||
      it 'returns an empty array' do
 | 
			
		||||
        get :empty_array
 | 
			
		||||
		Reference in New Issue
	
	Block a user