From 2dc7184f3ebc0e75c0a922fe600f04f9e919a383 Mon Sep 17 00:00:00 2001
From: chris48s <chris48s@users.noreply.github.com>
Date: Fri, 26 Jul 2024 10:17:51 +0100
Subject: [PATCH] Cache text only static badges for longer (#10403)

* cache text only static badges for longer

* explaining cache-control header on redirects more clearly
---
 core/base-service/base-static.js   |  6 +++++-
 core/base-service/cache-headers.js |  7 +++++--
 core/base-service/redirector.js    |  8 ++++++--
 core/server/server.spec.js         | 11 ++++++++++-
 4 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/core/base-service/base-static.js b/core/base-service/base-static.js
index d2523cce54..70c9969a17 100644
--- a/core/base-service/base-static.js
+++ b/core/base-service/base-static.js
@@ -47,7 +47,11 @@ export default class BaseStaticService extends BaseService {
       const format = (match.slice(-1)[0] || '.svg').replace(/^\./, '')
       badgeData.format = format
 
-      setCacheHeadersForStaticResource(ask.res)
+      let maxAge = 24 * 3600 // 1 day
+      if (!queryParams.logo && !badgeData.isError) {
+        maxAge = 5 * 24 * 3600 // 5 days
+      }
+      setCacheHeadersForStaticResource(ask.res, maxAge)
 
       const svg = makeBadge(badgeData)
       makeSend(format, ask.res, end)(svg)
diff --git a/core/base-service/cache-headers.js b/core/base-service/cache-headers.js
index 881af81d04..bbecf6a537 100644
--- a/core/base-service/cache-headers.js
+++ b/core/base-service/cache-headers.js
@@ -90,8 +90,11 @@ function setCacheHeaders({
   setHeadersForCacheLength(res, cacheLengthSeconds)
 }
 
-const staticCacheControlHeader = `max-age=${24 * 3600}, s-maxage=${24 * 3600}` // 1 day.
-function setCacheHeadersForStaticResource(res) {
+function setCacheHeadersForStaticResource(
+  res,
+  maxAge = 24 * 3600, // 1 day
+) {
+  const staticCacheControlHeader = `max-age=${maxAge}, s-maxage=${maxAge}`
   res.setHeader('Cache-Control', staticCacheControlHeader)
   res.setHeader('Last-Modified', serverStartTimeGMTString)
 }
diff --git a/core/base-service/redirector.js b/core/base-service/redirector.js
index 00dabed077..2a1f6b0f98 100644
--- a/core/base-service/redirector.js
+++ b/core/base-service/redirector.js
@@ -111,8 +111,12 @@ export default function redirector(attrs) {
         ask.res.statusCode = 301
         ask.res.setHeader('Location', redirectUrl)
 
-        // To avoid caching mistakes for a long time, and to make this simpler
-        // to reason about, use the same cache semantics as the static badge.
+        /* To avoid caching mistakes forever
+           (in the absence of cache control directives that specify otherwise,
+           301 redirects are cached without any expiry date)
+           and to make this simpler to reason about,
+           use the same cache semantics as the static badge.
+        */
         setCacheHeadersForStaticResource(ask.res)
 
         ask.res.end()
diff --git a/core/server/server.spec.js b/core/server/server.spec.js
index 32c0e04bf8..741b2e92da 100644
--- a/core/server/server.spec.js
+++ b/core/server/server.spec.js
@@ -59,8 +59,17 @@ describe('The server', function () {
       expect(headers['cache-control']).to.equal('max-age=300, s-maxage=300')
     })
 
-    it('should serve badges with custom maxAge', async function () {
+    it('should serve static badges without logo with maxAge=432000', async function () {
       const { headers } = await got(`${baseUrl}badge/foo-bar-blue`)
+      expect(headers['cache-control']).to.equal(
+        'max-age=432000, s-maxage=432000',
+      )
+    })
+
+    it('should serve badges with with logo with maxAge=86400', async function () {
+      const { headers } = await got(
+        `${baseUrl}badge/foo-bar-blue?logo=javascript`,
+      )
       expect(headers['cache-control']).to.equal('max-age=86400, s-maxage=86400')
     })
 
-- 
GitLab