Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Cache-Control and Vary definitions #24347

Merged
merged 1 commit into from
Apr 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion app/controllers/accounts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
4 changes: 0 additions & 4 deletions app/controllers/activitypub/base_controller.rb
Original file line number Diff 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
3 changes: 2 additions & 1 deletion app/controllers/activitypub/collections_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 3 additions & 5 deletions app/controllers/activitypub/outboxes_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
3 changes: 2 additions & 1 deletion app/controllers/activitypub/replies_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions app/controllers/admin/base_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ class BaseController < ApplicationController
layout 'admin'

before_action :set_body_classes
before_action :set_cache_headers

after_action :verify_authorized

private
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions app/controllers/api/base_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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?
Expand Down
2 changes: 0 additions & 2 deletions app/controllers/api/v1/custom_emojis_controller.rb
Original file line number Diff line number Diff line change
@@ -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) }
Expand Down
1 change: 0 additions & 1 deletion app/controllers/api/v1/instances/activity_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion app/controllers/api/v1/instances/peers_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion app/controllers/api/v1/instances_controller.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/auth/registrations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
14 changes: 9 additions & 5 deletions app/controllers/concerns/cache_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)

Expand Down
12 changes: 1 addition & 11 deletions app/controllers/custom_css_controller.rb
Original file line number Diff line number Diff line change
@@ -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
5 changes: 5 additions & 0 deletions app/controllers/disputes/base_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
11 changes: 4 additions & 7 deletions app/controllers/emojis_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions app/controllers/filters/statuses_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
5 changes: 5 additions & 0 deletions app/controllers/filters_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
3 changes: 2 additions & 1 deletion app/controllers/follower_accounts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
3 changes: 2 additions & 1 deletion app/controllers/following_accounts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
5 changes: 5 additions & 0 deletions app/controllers/invites_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down Expand Up @@ -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
5 changes: 1 addition & 4 deletions app/controllers/manifests_controller.rb
Original file line number Diff line number Diff line change
@@ -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'
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/oauth/authorizations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 5 additions & 0 deletions app/controllers/oauth/authorized_applications_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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!

Expand All @@ -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
5 changes: 5 additions & 0 deletions app/controllers/relationships_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?

Expand Down Expand Up @@ -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
2 changes: 1 addition & 1 deletion app/controllers/settings/base_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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!
Expand Down
Loading