Refactor Cache-Control and Vary definitions

mastodon · Apr 11, 2023 · 58c4b33 · 58c4b33
1 parent f9940eb
commit 58c4b33
Show file tree

Hide file tree

Showing 64 changed files with 424 additions and 173 deletions.
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
@@ -1224,9 +1224,6 @@ Rails/ActiveRecordCallbacksOrder:
 Rails/ApplicationController:
   Exclude:
     - 'app/controllers/health_controller.rb'
-    - 'app/controllers/well_known/host_meta_controller.rb'
-    - 'app/controllers/well_known/nodeinfo_controller.rb'
-    - 'app/controllers/well_known/webfinger_controller.rb'
 
 # Configuration parameters: Database, Include.
 # SupportedDatabases: mysql, postgresql

diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb
@@ -7,8 +7,9 @@ class AccountsController < ApplicationController
   include AccountControllerConcern
   include SignatureAuthentication
 
+  vary_by -> { public_fetch_mode? ? 'Accept' : 'Accept, Signature' }
+
   before_action :require_account_signature!, if: -> { request.format == :json && authorized_fetch_mode? }
-  before_action :set_cache_headers
 
   skip_around_action :set_locale, if: -> { [:json, :rss].include?(request.format&.to_sym) }
   skip_before_action :require_functional!, unless: :whitelist_mode?

diff --git a/app/controllers/activitypub/base_controller.rb b/app/controllers/activitypub/base_controller.rb
@@ -7,10 +7,6 @@ class ActivityPub::BaseController < Api::BaseController
 
   private
 
-  def set_cache_headers
-    response.headers['Vary'] = 'Signature' if authorized_fetch_mode?
-  end
-
   def skip_temporary_suspension_response?
     false
   end

diff --git a/app/controllers/activitypub/collections_controller.rb b/app/controllers/activitypub/collections_controller.rb
@@ -4,11 +4,12 @@ class ActivityPub::CollectionsController < ActivityPub::BaseController
   include SignatureVerification
   include AccountOwnedConcern
 
+  vary_by -> { 'Signature' if authorized_fetch_mode? }
+
   before_action :require_account_signature!, if: :authorized_fetch_mode?
   before_action :set_items
   before_action :set_size
   before_action :set_type
-  before_action :set_cache_headers
 
   def show
     expires_in 3.minutes, public: public_fetch_mode?

diff --git a/app/controllers/activitypub/followers_synchronizations_controller.rb b/app/controllers/activitypub/followers_synchronizations_controller.rb
@@ -4,9 +4,10 @@ class ActivityPub::FollowersSynchronizationsController < ActivityPub::BaseContro
   include SignatureVerification
   include AccountOwnedConcern
 
+  vary_by -> { 'Signature' if authorized_fetch_mode? }
+
   before_action :require_account_signature!
   before_action :set_items
-  before_action :set_cache_headers
 
   def show
     expires_in 0, public: false

diff --git a/app/controllers/activitypub/outboxes_controller.rb b/app/controllers/activitypub/outboxes_controller.rb
@@ -6,16 +6,18 @@ class ActivityPub::OutboxesController < ActivityPub::BaseController
   include SignatureVerification
   include AccountOwnedConcern
 
+  vary_by -> { 'Signature' if authorized_fetch_mode? || page_requested? }
+
   before_action :require_account_signature!, if: :authorized_fetch_mode?
   before_action :set_statuses
-  before_action :set_cache_headers
 
   def show
     if page_requested?
       expires_in(1.minute, public: public_fetch_mode? && signed_request_account.nil?)
     else
       expires_in(3.minutes, public: public_fetch_mode?)
     end
+
     render json: outbox_presenter, serializer: ActivityPub::OutboxSerializer, adapter: ActivityPub::Adapter, content_type: 'application/activity+json'
   end
 
@@ -80,8 +82,4 @@ def page_params
   def set_account
     @account = params[:account_username].present? ? Account.find_local!(username_param) : Account.representative
   end
-
-  def set_cache_headers
-    response.headers['Vary'] = 'Signature' if authorized_fetch_mode? || page_requested?
-  end
 end
diff --git a/app/controllers/activitypub/replies_controller.rb b/app/controllers/activitypub/replies_controller.rb
@@ -7,9 +7,10 @@ class ActivityPub::RepliesController < ActivityPub::BaseController
 
   DESCENDANTS_LIMIT = 60
 
+  vary_by -> { 'Signature' if authorized_fetch_mode? }
+
   before_action :require_account_signature!, if: :authorized_fetch_mode?
   before_action :set_status
-  before_action :set_cache_headers
   before_action :set_replies
 
   def index

diff --git a/app/controllers/admin/base_controller.rb b/app/controllers/admin/base_controller.rb
@@ -8,6 +8,8 @@ class BaseController < ApplicationController
     layout 'admin'
 
     before_action :set_body_classes
+    before_action :set_cache_headers
+
     after_action :verify_authorized
 
     private
@@ -16,6 +18,10 @@ def set_body_classes
       @body_classes = 'admin'
     end
 
+    def set_cache_headers
+      response.cache_control.replace(private: true, no_store: true)
+    end
+
     def set_user
       @user = Account.find(params[:account_id]).user || raise(ActiveRecord::RecordNotFound)
     end

diff --git a/app/controllers/api/base_controller.rb b/app/controllers/api/base_controller.rb
@@ -12,7 +12,7 @@ class Api::BaseController < ApplicationController
 
   before_action :require_authenticated_user!, if: :disallow_unauthenticated_api_access?
   before_action :require_not_suspended!
-  before_action :set_cache_headers
+  before_action :set_cache_control_defaults
 
   protect_from_forgery with: :null_session
 
@@ -148,8 +148,8 @@ def authorize_if_got_token!(*scopes)
     doorkeeper_authorize!(*scopes) if doorkeeper_token
   end
 
-  def set_cache_headers
-    response.headers['Cache-Control'] = 'private, no-store'
+  def set_cache_control_defaults
+    response.cache_control.replace(private: true, no_store: true)
   end
 
   def disallow_unauthenticated_api_access?

diff --git a/app/controllers/api/v1/custom_emojis_controller.rb b/app/controllers/api/v1/custom_emojis_controller.rb
@@ -1,8 +1,6 @@
 # frozen_string_literal: true
 
 class Api::V1::CustomEmojisController < Api::BaseController
-  skip_before_action :set_cache_headers
-
   def index
     expires_in 3.minutes, public: true
     render_with_cache(each_serializer: REST::CustomEmojiSerializer) { CustomEmoji.listed.includes(:category) }

diff --git a/app/controllers/api/v1/instances/activity_controller.rb b/app/controllers/api/v1/instances/activity_controller.rb
@@ -3,7 +3,6 @@
 class Api::V1::Instances::ActivityController < Api::BaseController
   before_action :require_enabled_api!
 
-  skip_before_action :set_cache_headers
   skip_before_action :require_authenticated_user!, unless: :whitelist_mode?
 
   def show

diff --git a/app/controllers/api/v1/instances/peers_controller.rb b/app/controllers/api/v1/instances/peers_controller.rb
@@ -3,7 +3,6 @@
 class Api::V1::Instances::PeersController < Api::BaseController
   before_action :require_enabled_api!
 
-  skip_before_action :set_cache_headers
   skip_before_action :require_authenticated_user!, unless: :whitelist_mode?
 
   def index

diff --git a/app/controllers/api/v1/instances_controller.rb b/app/controllers/api/v1/instances_controller.rb
@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 class Api::V1::InstancesController < Api::BaseController
-  skip_before_action :set_cache_headers
   skip_before_action :require_authenticated_user!, unless: :whitelist_mode?
 
   def show

diff --git a/app/controllers/auth/registrations_controller.rb b/app/controllers/auth/registrations_controller.rb
@@ -152,6 +152,6 @@ def require_rules_acceptance!
   end
 
   def set_cache_headers
-    response.headers['Cache-Control'] = 'private, no-store'
+    response.cache_control.replace(private: true, no_store: true)
   end
 end
diff --git a/app/controllers/concerns/cache_concern.rb b/app/controllers/concerns/cache_concern.rb
@@ -155,8 +155,16 @@ def lookup(instance)
     end
   end
 
+  class_methods do
+    def vary_by(value)
+      before_action do |controller|
+        response.headers['Vary'] = value.respond_to?(:call) ? controller.instance_exec(&value) : value
+      end
+    end
+  end
+
   def render_with_cache(**options)
-    raise ArgumentError, 'only JSON render calls are supported' unless options.key?(:json) || block_given?
+    raise ArgumentError, 'Only JSON render calls are supported' unless options.key?(:json) || block_given?
 
     key        = options.delete(:key) || [[params[:controller], params[:action]].join('/'), options[:json].respond_to?(:cache_key) ? options[:json].cache_key : nil, options[:fields].nil? ? nil : options[:fields].join(',')].compact.join(':')
     expires_in = options.delete(:expires_in) || 3.minutes
@@ -176,10 +184,6 @@ def render_with_cache(**options)
     end
   end
 
-  def set_cache_headers
-    response.headers['Vary'] = public_fetch_mode? ? 'Accept' : 'Accept, Signature'
-  end
-
   def cache_collection(raw, klass)
     return raw unless klass.respond_to?(:with_includes)
 

diff --git a/app/controllers/custom_css_controller.rb b/app/controllers/custom_css_controller.rb
@@ -1,18 +1,8 @@
 # frozen_string_literal: true
 
-class CustomCssController < ApplicationController
-  skip_before_action :store_current_location
-  skip_before_action :require_functional!
-  skip_before_action :update_user_sign_in
-  skip_before_action :set_session_activity
-
-  skip_around_action :set_locale
-
-  before_action :set_cache_headers
-
+class CustomCssController < ActionController::Base # rubocop:disable Rails/ApplicationController
   def show
     expires_in 3.minutes, public: true
-    request.session_options[:skip] = true
     render content_type: 'text/css'
   end
 end
diff --git a/app/controllers/disputes/base_controller.rb b/app/controllers/disputes/base_controller.rb
@@ -9,10 +9,15 @@ class Disputes::BaseController < ApplicationController
 
   before_action :set_body_classes
   before_action :authenticate_user!
+  before_action :set_cache_headers
 
   private
 
   def set_body_classes
     @body_classes = 'admin'
   end
+
+  def set_cache_headers
+    response.cache_control.replace(private: true, no_store: true)
+  end
 end
diff --git a/app/controllers/emojis_controller.rb b/app/controllers/emojis_controller.rb
@@ -2,15 +2,12 @@
 
 class EmojisController < ApplicationController
   before_action :set_emoji
-  before_action :set_cache_headers
+
+  vary_by -> { 'Signature' if authorized_fetch_mode? }
 
   def show
-    respond_to do |format|
-      format.json do
-        expires_in 3.minutes, public: true
-        render_with_cache json: @emoji, content_type: 'application/activity+json', serializer: ActivityPub::EmojiSerializer, adapter: ActivityPub::Adapter
-      end
-    end
+    expires_in 3.minutes, public: true
+    render_with_cache json: @emoji, content_type: 'application/activity+json', serializer: ActivityPub::EmojiSerializer, adapter: ActivityPub::Adapter
   end
 
   private

diff --git a/app/controllers/filters/statuses_controller.rb b/app/controllers/filters/statuses_controller.rb
@@ -7,6 +7,7 @@ class Filters::StatusesController < ApplicationController
   before_action :set_filter
   before_action :set_status_filters
   before_action :set_body_classes
+  before_action :set_cache_headers
 
   PER_PAGE = 20
 
@@ -44,4 +45,8 @@ def action_from_button
   def set_body_classes
     @body_classes = 'admin'
   end
+
+  def set_cache_headers
+    response.cache_control.replace(private: true, no_store: true)
+  end
 end
diff --git a/app/controllers/filters_controller.rb b/app/controllers/filters_controller.rb
@@ -6,6 +6,7 @@ class FiltersController < ApplicationController
   before_action :authenticate_user!
   before_action :set_filter, only: [:edit, :update, :destroy]
   before_action :set_body_classes
+  before_action :set_cache_headers
 
   def index
     @filters = current_account.custom_filters.includes(:keywords, :statuses).order(:phrase)
@@ -54,4 +55,8 @@ def resource_params
   def set_body_classes
     @body_classes = 'admin'
   end
+
+  def set_cache_headers
+    response.cache_control.replace(private: true, no_store: true)
+  end
 end
diff --git a/app/controllers/follower_accounts_controller.rb b/app/controllers/follower_accounts_controller.rb
@@ -5,8 +5,9 @@ class FollowerAccountsController < ApplicationController
   include SignatureVerification
   include WebAppControllerConcern
 
+  vary_by -> { public_fetch_mode? ? 'Accept' : 'Accept, Signature' }
+
   before_action :require_account_signature!, if: -> { request.format == :json && authorized_fetch_mode? }
-  before_action :set_cache_headers
 
   skip_around_action :set_locale, if: -> { request.format == :json }
   skip_before_action :require_functional!, unless: :whitelist_mode?

diff --git a/app/controllers/following_accounts_controller.rb b/app/controllers/following_accounts_controller.rb
@@ -5,8 +5,9 @@ class FollowingAccountsController < ApplicationController
   include SignatureVerification
   include WebAppControllerConcern
 
+  vary_by -> { public_fetch_mode? ? 'Accept' : 'Accept, Signature' }
+
   before_action :require_account_signature!, if: -> { request.format == :json && authorized_fetch_mode? }
-  before_action :set_cache_headers
 
   skip_around_action :set_locale, if: -> { request.format == :json }
   skip_before_action :require_functional!, unless: :whitelist_mode?

diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb
@@ -7,6 +7,7 @@ class InvitesController < ApplicationController
 
   before_action :authenticate_user!
   before_action :set_body_classes
+  before_action :set_cache_headers
 
   def index
     authorize :invite, :create?
@@ -49,4 +50,8 @@ def resource_params
   def set_body_classes
     @body_classes = 'admin'
   end
+
+  def set_cache_headers
+    response.cache_control.replace(private: true, no_store: true)
+  end
 end
diff --git a/app/controllers/manifests_controller.rb b/app/controllers/manifests_controller.rb
@@ -1,9 +1,6 @@
 # frozen_string_literal: true
 
-class ManifestsController < ApplicationController
-  skip_before_action :store_current_location
-  skip_before_action :require_functional!
-
+class ManifestsController < ActionController::Base # rubocop:disable Rails/ApplicationController
   def show
     expires_in 3.minutes, public: true
     render json: InstancePresenter.new, serializer: ManifestSerializer, root: 'instance'

diff --git a/app/controllers/oauth/authorizations_controller.rb b/app/controllers/oauth/authorizations_controller.rb
@@ -34,6 +34,6 @@ def truthy_param?(key)
   end
 
   def set_cache_headers
-    response.headers['Cache-Control'] = 'private, no-store'
+    response.cache_control.replace(private: true, no_store: true)
   end
 end
diff --git a/app/controllers/oauth/authorized_applications_controller.rb b/app/controllers/oauth/authorized_applications_controller.rb
@@ -7,6 +7,7 @@ class Oauth::AuthorizedApplicationsController < Doorkeeper::AuthorizedApplicatio
   before_action :authenticate_resource_owner!
   before_action :require_not_suspended!, only: :destroy
   before_action :set_body_classes
+  before_action :set_cache_headers
 
   skip_before_action :require_functional!
 
@@ -30,4 +31,8 @@ def store_current_location
   def require_not_suspended!
     forbidden if current_account.suspended?
   end
+
+  def set_cache_headers
+    response.cache_control.replace(private: true, no_store: true)
+  end
 end
diff --git a/app/controllers/relationships_controller.rb b/app/controllers/relationships_controller.rb
@@ -7,6 +7,7 @@ class RelationshipsController < ApplicationController
   before_action :set_accounts, only: :show
   before_action :set_relationships, only: :show
   before_action :set_body_classes
+  before_action :set_cache_headers
 
   helper_method :following_relationship?, :followed_by_relationship?, :mutual_relationship?
 
@@ -70,4 +71,8 @@ def action_from_button
   def set_body_classes
     @body_classes = 'admin'
   end
+
+  def set_cache_headers
+    response.cache_control.replace(private: true, no_store: true)
+  end
 end
diff --git a/app/controllers/settings/base_controller.rb b/app/controllers/settings/base_controller.rb
@@ -14,7 +14,7 @@ def set_body_classes
   end
 
   def set_cache_headers
-    response.headers['Cache-Control'] = 'private, no-store'
+    response.cache_control.replace(private: true, no_store: true)
   end
 
   def require_not_suspended!
-Original file line number
+Diff line change
@@ Expand Up / @@ -7,10 +7,6 @@ class ActivityPub::BaseController < Api::BaseController @@
       private
-      def set_cache_headers
-        response.headers['Vary'] = 'Signature' if authorized_fetch_mode?
-      end
       def skip_temporary_suspension_response?
         false
       end
@@ Expand Down @@