From df990afadca43bcead8b1c31f12a0471caa15ae7 Mon Sep 17 00:00:00 2001 From: Vaidehi Joshi Date: Wed, 5 Aug 2020 12:34:12 -0700 Subject: [PATCH] Conditionally remove `no-cache` from Fastly headers (#9631) [deploy] * Rename FastlyHeaders to CachingHeaders, conditionally experiment with removing no-cache We suspect that it is safe to remove `no-cache` entirely from the Cache-Control headers that are sent along to Fastly, but there is no great way to confirm this hypothesis. We're testing this by experimenting on one single article. We will conditionally default to the same headers that we send to Nginx, and send those headers to Fastly (by removing `no-cache` and adding in `max-age`, which should be ignored since Fastly also has Surrogate-Control headers added to it, which include a `max-age`, which Fastly should prioritize over any other `max-age`. Once we've confirmed that this doesn't cause issues on the Fastly side, we can remove this check and use the same Cache-Control headers for both Fastly and Nginx. * Add check for request.env[REQUEST_PATH].present? --- app/controllers/application_controller.rb | 2 +- .../{fastly_headers.rb => caching_headers.rb} | 16 +++++++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) rename app/controllers/concerns/{fastly_headers.rb => caching_headers.rb} (74%) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index af9863b7b..d2d6f66da 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -6,7 +6,7 @@ class ApplicationController < ActionController::Base include SessionCurrentUser include ValidRequest include Pundit - include FastlyHeaders + include CachingHeaders include ImageUploads include VerifySetupCompleted diff --git a/app/controllers/concerns/fastly_headers.rb b/app/controllers/concerns/caching_headers.rb similarity index 74% rename from app/controllers/concerns/fastly_headers.rb rename to app/controllers/concerns/caching_headers.rb index 1100f1e3a..bf0c1db49 100644 --- a/app/controllers/concerns/fastly_headers.rb +++ b/app/controllers/concerns/caching_headers.rb @@ -1,4 +1,4 @@ -module FastlyHeaders +module CachingHeaders extend ActiveSupport::Concern # The following is from the now deprecated fastly-rails gem @@ -12,11 +12,21 @@ module FastlyHeaders # custom config example: # {cache_control: 'public, no-cache, maxage=xyz', surrogate_control: 'max-age: 100'} def set_cache_control_headers( - max_age = 1.day.to_i, cache_control: "public, no-cache", + max_age = 1.day.to_i, surrogate_control: nil, stale_while_revalidate: nil, stale_if_error: 26_400 ) - request.session_options[:skip] = true # no cookies + # TODO: [@forem/delightful]: Once we've tested that removing `no-cache` from the Cache-Control headers + # does not adversely affect fastly, we should remove this specific check entirely. + article_requested = request.env["REQUEST_PATH"].present? && + request.env["REQUEST_PATH"].include?("the-3-biggest-misconceptions-about-diversity-3hkm") + cache_control = if article_requested + "public, max-age=86400" + else + "public, no-cache" + end + + request.session_options[:skip] = true # no cookies response.headers["Cache-Control"] = cache_control response.headers["Surrogate-Control"] = surrogate_control.presence || build_surrogate_control(