From 65e82211cdaffa3132832dc42756913d668985c3 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 16 May 2024 04:03:46 -0400 Subject: [PATCH] Rename `cache_*` methods to `preload_*` in controller concern (#30209) --- app/controllers/accounts_controller.rb | 2 +- .../activitypub/collections_controller.rb | 2 +- .../activitypub/outboxes_controller.rb | 2 +- .../api/v1/accounts/statuses_controller.rb | 6 +++--- app/controllers/api/v1/bookmarks_controller.rb | 6 +++--- app/controllers/api/v1/favourites_controller.rb | 6 +++--- .../api/v1/notifications/requests_controller.rb | 2 +- .../api/v1/notifications_controller.rb | 2 +- app/controllers/api/v1/statuses_controller.rb | 8 ++++---- .../api/v1/timelines/home_controller.rb | 6 +++--- .../api/v1/timelines/list_controller.rb | 6 +++--- .../api/v1/timelines/public_controller.rb | 6 +++--- .../api/v1/timelines/tag_controller.rb | 6 +++--- .../api/v1/trends/statuses_controller.rb | 2 +- app/controllers/application_controller.rb | 1 + app/controllers/concerns/cache_concern.rb | 16 ---------------- app/controllers/concerns/preloading_concern.rb | 17 +++++++++++++++++ app/controllers/tags_controller.rb | 2 +- ...ncern_spec.rb => preloading_concern_spec.rb} | 12 ++++++------ 19 files changed, 56 insertions(+), 54 deletions(-) create mode 100644 app/controllers/concerns/preloading_concern.rb rename spec/controllers/concerns/{cache_concern_spec.rb => preloading_concern_spec.rb} (79%) diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index 32549e1516..c3131edce9 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -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 diff --git a/app/controllers/activitypub/collections_controller.rb b/app/controllers/activitypub/collections_controller.rb index d5632902fe..c25362c9bc 100644 --- a/app/controllers/activitypub/collections_controller.rb +++ b/app/controllers/activitypub/collections_controller.rb @@ -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 } diff --git a/app/controllers/activitypub/outboxes_controller.rb b/app/controllers/activitypub/outboxes_controller.rb index 8079e011dd..b8baf64e1a 100644 --- a/app/controllers/activitypub/outboxes_controller.rb +++ b/app/controllers/activitypub/outboxes_controller.rb @@ -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, diff --git a/app/controllers/api/v1/accounts/statuses_controller.rb b/app/controllers/api/v1/accounts/statuses_controller.rb index 35ea9c8ec1..c42f27776c 100644 --- a/app/controllers/api/v1/accounts/statuses_controller.rb +++ b/app/controllers/api/v1/accounts/statuses_controller.rb @@ -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), diff --git a/app/controllers/api/v1/bookmarks_controller.rb b/app/controllers/api/v1/bookmarks_controller.rb index b6bb987b6b..f7671a9032 100644 --- a/app/controllers/api/v1/bookmarks_controller.rb +++ b/app/controllers/api/v1/bookmarks_controller.rb @@ -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 diff --git a/app/controllers/api/v1/favourites_controller.rb b/app/controllers/api/v1/favourites_controller.rb index 73da538f5c..18ca9ab866 100644 --- a/app/controllers/api/v1/favourites_controller.rb +++ b/app/controllers/api/v1/favourites_controller.rb @@ -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 diff --git a/app/controllers/api/v1/notifications/requests_controller.rb b/app/controllers/api/v1/notifications/requests_controller.rb index 6a26cc0e8a..0e58379a38 100644 --- a/app/controllers/api/v1/notifications/requests_controller.rb +++ b/app/controllers/api/v1/notifications/requests_controller.rb @@ -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 diff --git a/app/controllers/api/v1/notifications_controller.rb b/app/controllers/api/v1/notifications_controller.rb index c41a0bb05d..1d0aa10d2e 100644 --- a/app/controllers/api/v1/notifications_controller.rb +++ b/app/controllers/api/v1/notifications_controller.rb @@ -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 diff --git a/app/controllers/api/v1/statuses_controller.rb b/app/controllers/api/v1/statuses_controller.rb index 36a9ec6325..5f7e66617d 100644 --- a/app/controllers/api/v1/statuses_controller.rb +++ b/app/controllers/api/v1/statuses_controller.rb @@ -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 diff --git a/app/controllers/api/v1/timelines/home_controller.rb b/app/controllers/api/v1/timelines/home_controller.rb index 36fdbea647..d5d1828666 100644 --- a/app/controllers/api/v1/timelines/home_controller.rb +++ b/app/controllers/api/v1/timelines/home_controller.rb @@ -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 diff --git a/app/controllers/api/v1/timelines/list_controller.rb b/app/controllers/api/v1/timelines/list_controller.rb index 14b884ecd9..d8cdbdb74c 100644 --- a/app/controllers/api/v1/timelines/list_controller.rb +++ b/app/controllers/api/v1/timelines/list_controller.rb @@ -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 diff --git a/app/controllers/api/v1/timelines/public_controller.rb b/app/controllers/api/v1/timelines/public_controller.rb index 35af8dc4b5..d164854d6a 100644 --- a/app/controllers/api/v1/timelines/public_controller.rb +++ b/app/controllers/api/v1/timelines/public_controller.rb @@ -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 diff --git a/app/controllers/api/v1/timelines/tag_controller.rb b/app/controllers/api/v1/timelines/tag_controller.rb index 4ba439dbb2..3bf8f374e1 100644 --- a/app/controllers/api/v1/timelines/tag_controller.rb +++ b/app/controllers/api/v1/timelines/tag_controller.rb @@ -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 diff --git a/app/controllers/api/v1/trends/statuses_controller.rb b/app/controllers/api/v1/trends/statuses_controller.rb index 48bfe11991..c6fbbce167 100644 --- a/app/controllers/api/v1/trends/statuses_controller.rb +++ b/app/controllers/api/v1/trends/statuses_controller.rb @@ -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 diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 8ba10d64c0..66e0f7e305 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -9,6 +9,7 @@ class ApplicationController < ActionController::Base include UserTrackingConcern include SessionTrackingConcern include CacheConcern + include PreloadingConcern include DomainControlHelper include DatabaseHelper include AuthorizedFetchHelper diff --git a/app/controllers/concerns/cache_concern.rb b/app/controllers/concerns/cache_concern.rb index 4656539f85..1823b5b8ed 100644 --- a/app/controllers/concerns/cache_concern.rb +++ b/app/controllers/concerns/cache_concern.rb @@ -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 diff --git a/app/controllers/concerns/preloading_concern.rb b/app/controllers/concerns/preloading_concern.rb new file mode 100644 index 0000000000..61e2213649 --- /dev/null +++ b/app/controllers/concerns/preloading_concern.rb @@ -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 diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index b0bdbde956..d6c0d872c8 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -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 diff --git a/spec/controllers/concerns/cache_concern_spec.rb b/spec/controllers/concerns/preloading_concern_spec.rb similarity index 79% rename from spec/controllers/concerns/cache_concern_spec.rb rename to spec/controllers/concerns/preloading_concern_spec.rb index fffd2b266e..795afbc45e 100644 --- a/spec/controllers/concerns/cache_concern_spec.rb +++ b/spec/controllers/concerns/preloading_concern_spec.rb @@ -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