simplify and improve default cache-control headers (github#30162)

simply and perfect cache-control headers

middleware/anchor-redirect.js

@@ -1,5 +1,7 @@
 import express from 'express'
+
 import { readCompressedJsonFileFallbackLazily } from '../lib/read-json-file.js'
+import { defaultCacheControl } from './cache-control.js'
 
 const clientSideRestAPIRedirects = readCompressedJsonFileFallbackLazily(
   './lib/redirects/static/client-side-rest-api-redirects.json'
@@ -15,6 +17,9 @@ router.get('/', function redirects(req, res, next) {
   if (!req.query.hash) {
     return res.status(400).send("Missing 'hash' query string")
   }
+
+  defaultCacheControl(res)
+
   const redirectFrom = `${req.query.path}#${req.query.hash}`
   res.status(200).send({ to: clientSideRestAPIRedirects()[redirectFrom] } || null)
 })

middleware/api/search.js

@@ -3,7 +3,7 @@ import express from 'express'
 import searchVersions from '../../lib/search/versions.js'
 import languages from '../../lib/languages.js'
 import { allVersions } from '../../lib/all-versions.js'
-import { cacheControlFactory } from '../cache-control.js'
+import { defaultCacheControl } from '../cache-control.js'
 import catchMiddlewareError from '../catch-middleware-error.js'
 import { getSearchResults, ELASTICSEARCH_URL } from './es-search.js'
 
@@ -13,8 +13,6 @@ const languagesSet = new Set(Object.keys(languages))
 
 const router = express.Router()
 
-const cacheControl = cacheControlFactory(60 * 60 * 24)
-
 const DEFAULT_SIZE = 10
 const MAX_SIZE = 50 // How much you return has a strong impact on performance
 const DEFAULT_PAGE = 1
@@ -137,7 +135,7 @@ router.get(
       }
     })
     if (process.env.NODE_ENV !== 'development') {
-      cacheControl(res)
+      defaultCacheControl(res)
     }
 
     res.setHeader('x-search-legacy', 'yes')
@@ -227,7 +225,7 @@ router.get(
       // So the only distinguishing key is the request URL.
       // Because of that, it's safe to allow the reverse proxy (a.k.a the CDN)
       // cache and hold on to this.
-      cacheControl(res)
+      defaultCacheControl(res)
     }
 
     // The v1 version of the output matches perfectly what comes out

middleware/cache-control.js

@@ -38,3 +38,20 @@ export function cacheControlFactory(
     res.set(key, directives)
   }
 }
+
+// 24 hours for CDN, we soft-purge this with each deploy
+const defaultCDNCacheControl = cacheControlFactory(60 * 60 * 24, {
+  key: 'surrogate-control',
+})
+
+// Shorter because between deployments and their (sort) purges,
+// we don't want the browser to overly cache because with them we
+// can't control purging.
+const defaultBrowserCacheControl = cacheControlFactory(60)
+
+// A general default configuration that is useful to almost all responses
+// that can be cached.
+export function defaultCacheControl(res) {
+  defaultCDNCacheControl(res)
+  defaultBrowserCacheControl(res)
+}

middleware/render-page.js

@@ -8,12 +8,7 @@ import statsd from '../lib/statsd.js'
 import { allVersions } from '../lib/all-versions.js'
 import { isConnectionDropped } from './halt-on-dropped-connection.js'
 import { nextApp, nextHandleRequest } from './next.js'
-import { cacheControlFactory } from './cache-control.js'
-
-const browserCacheControl = cacheControlFactory(60) // 1 minute for browsers
-const cdnCacheControl = cacheControlFactory(60 * 60 * 24, {
-  key: 'surrogate-control',
-}) // 24 hours for CDN, we purge this with each deploy
+import { defaultCacheControl } from './cache-control.js'
 
 async function buildRenderedPage(req) {
   const { context } = req
@@ -50,8 +45,6 @@ export default async function renderPage(req, res, next) {
 
   const { page } = context
   const path = req.pagePath || req.path
-  browserCacheControl(res)
-  cdnCacheControl(res)
 
   // render a 404 page
   if (!page) {
@@ -130,5 +123,7 @@ export default async function renderPage(req, res, next) {
     }
   }
 
+  defaultCacheControl(res)
+
   return nextHandleRequest(req, res)
 }

tests/content/api-search.js

@@ -15,6 +15,7 @@ import { jest, test, expect } from '@jest/globals'
 
 import { describeIfElasticsearchURL } from '../helpers/conditional-runs.js'
 import { get } from '../helpers/e2etest.js'
+import { SURROGATE_ENUMS } from '../../middleware/set-fastly-surrogate-key.js'
 
 if (!process.env.ELASTICSEARCH_URL) {
   console.warn(
@@ -62,7 +63,10 @@ describeIfElasticsearchURL('search middleware', () => {
     // Check that it can be cached at the CDN
     expect(res.headers['set-cookie']).toBeUndefined()
     expect(res.headers['cache-control']).toContain('public')
-    expect(res.headers['cache-control']).toMatch(/max-age=\d+/)
+    expect(res.headers['cache-control']).toMatch(/max-age=[1-9]/)
+    expect(res.headers['surrogate-control']).toContain('public')
+    expect(res.headers['surrogate-control']).toMatch(/max-age=[1-9]/)
+    expect(res.headers['surrogate-key']).toBe(SURROGATE_ENUMS.DEFAULT)
   })
 
   test('debug search', async () => {

tests/unit/anchor-redirect.js

@@ -1,5 +1,7 @@
 import { describe, expect } from '@jest/globals'
+
 import { get } from '../helpers/e2etest.js'
+import { SURROGATE_ENUMS } from '../../middleware/set-fastly-surrogate-key.js'
 import clientSideRedirects from '../../lib/redirects/static/client-side-rest-api-redirects.json'
 
 describe('anchor-redirect middleware', () => {
@@ -41,4 +43,15 @@ describe('anchor-redirect middleware', () => {
     const { to } = JSON.parse(res.text)
     expect(to).toBe(undefined)
   })
+  test('reasonably aggressive cache-control headers', async () => {
+    const sp = new URLSearchParams()
+    sp.set('path', 'foo')
+    sp.set('hash', 'bar')
+    const res = await get('/anchor-redirect?' + sp)
+    expect(res.headers['cache-control']).toContain('public')
+    expect(res.headers['cache-control']).toMatch(/max-age=[1-9]/)
+    expect(res.headers['surrogate-control']).toContain('public')
+    expect(res.headers['surrogate-control']).toMatch(/max-age=[1-9]/)
+    expect(res.headers['surrogate-key']).toBe(SURROGATE_ENUMS.DEFAULT)
+  })
 })