From e3d115cf99438554160b8486bc38a55687cdc752 Mon Sep 17 00:00:00 2001
From: Paul Melnikow <github@paulmelnikow.com>
Date: Sat, 23 Feb 2019 20:33:38 -0500
Subject: [PATCH] Rename maxAge to cacheSeconds (#3090)
Close #3069
---
README.md | 20 ++++++-------
core/badge-urls/make-badge-url.js | 2 +-
core/badge-urls/make-badge-url.spec.js | 4 +--
core/base-service/cache-headers.js | 22 +++++++-------
core/base-service/cache-headers.spec.js | 30 +++++++++++++------
core/base-service/legacy-request-handler.js | 4 +--
.../legacy-request-handler.spec.js | 10 +++----
frontend/components/usage.js | 11 +++----
lib/badge-data.js | 3 +-
services/endpoint/endpoint.tester.js | 4 +--
10 files changed, 63 insertions(+), 47 deletions(-)
diff --git a/README.md b/README.md
index c1ed2f6093..afcb6d4d01 100644
--- a/README.md
+++ b/README.md
@@ -59,16 +59,16 @@ This repo hosts:
## Examples
-- code coverage percentage: 
-- stable release version: 
-- package manager release: 
-- status of third-party dependencies: 
-- static code analysis grade: 
-- [SemVer](https://semver.org/) version observance: 
-- amount of [Liberapay](https://liberapay.com/) donations per week: 
-- Python package downloads: 
-- Chrome Web Store extension rating: 
-- [Uptime Robot](https://uptimerobot.com) percentage: 
+- code coverage percentage: 
+- stable release version: 
+- package manager release: 
+- status of third-party dependencies: 
+- static code analysis grade: 
+- [SemVer](https://semver.org/) version observance: 
+- amount of [Liberapay](https://liberapay.com/) donations per week: 
+- Python package downloads: 
+- Chrome Web Store extension rating: 
+- [Uptime Robot](https://uptimerobot.com) percentage: 
[Make your own badges!][custom badges]
(Quick example: `https://img.shields.io/badge/left-right-f39f37.svg`)
diff --git a/core/badge-urls/make-badge-url.js b/core/badge-urls/make-badge-url.js
index f6e891ac3f..7dd9a3a85c 100644
--- a/core/badge-urls/make-badge-url.js
+++ b/core/badge-urls/make-badge-url.js
@@ -14,7 +14,7 @@ function badgeUrlFromPath({
const outExt = format.length ? `.${format}` : ''
const outQueryString = queryString.stringify({
- maxAge: longCache ? '2592000' : undefined,
+ cacheSeconds: longCache ? '2592000' : undefined,
style,
...queryParams,
})
diff --git a/core/badge-urls/make-badge-url.spec.js b/core/badge-urls/make-badge-url.spec.js
index 55931b9436..1d0f183cff 100644
--- a/core/badge-urls/make-badge-url.spec.js
+++ b/core/badge-urls/make-badge-url.spec.js
@@ -18,7 +18,7 @@ describe('Badge URL generation functions', function() {
style: 'flat-square',
longCache: true,
}).expect(
- 'http://example.com/npm/v/gh-badges.svg?maxAge=2592000&style=flat-square'
+ 'http://example.com/npm/v/gh-badges.svg?cacheSeconds=2592000&style=flat-square'
)
})
@@ -30,7 +30,7 @@ describe('Badge URL generation functions', function() {
style: 'flat-square',
longCache: true,
}).expect(
- 'http://example.com/npm/v/gh-badges.svg?maxAge=2592000&style=flat-square'
+ 'http://example.com/npm/v/gh-badges.svg?cacheSeconds=2592000&style=flat-square'
)
})
diff --git a/core/base-service/cache-headers.js b/core/base-service/cache-headers.js
index 99697464c8..eace490079 100644
--- a/core/base-service/cache-headers.js
+++ b/core/base-service/cache-headers.js
@@ -7,23 +7,25 @@ const coalesce = require('./coalesce')
const serverStartTimeGMTString = new Date().toGMTString()
const serverStartTimestamp = Date.now()
+const isOptionalNonNegativeInteger = Joi.number()
+ .integer()
+ .min(0)
+
const queryParamSchema = Joi.object({
- // Not using nonNegativeInteger because it's not required.
- maxAge: Joi.number()
- .integer()
- .min(0),
+ cacheSeconds: isOptionalNonNegativeInteger,
+ maxAge: isOptionalNonNegativeInteger,
})
+ .oxor('cacheSeconds', 'maxAge')
.unknown(true)
.required()
function overrideCacheLengthFromQueryParams(queryParams) {
try {
- const { maxAge: overrideCacheLength } = Joi.attempt(
- queryParams,
- queryParamSchema,
- { allowUnknown: true }
- )
- return overrideCacheLength
+ const {
+ cacheSeconds: overrideCacheLength,
+ maxAge: legacyOverrideCacheLength,
+ } = Joi.attempt(queryParams, queryParamSchema)
+ return coalesce(overrideCacheLength, legacyOverrideCacheLength)
} catch (e) {
return undefined
}
diff --git a/core/base-service/cache-headers.spec.js b/core/base-service/cache-headers.spec.js
index a2806065a2..ebe4e61df5 100644
--- a/core/base-service/cache-headers.spec.js
+++ b/core/base-service/cache-headers.spec.js
@@ -33,32 +33,44 @@ describe('Cache header functions', function() {
given({
cacheHeaderConfig,
serviceDefaultCacheLengthSeconds: 900,
- queryParams: { maxAge: 1000 },
+ queryParams: { cacheSeconds: 1000 },
}).expect(1000)
given({
cacheHeaderConfig,
serviceDefaultCacheLengthSeconds: 900,
- queryParams: { maxAge: 1000, other: 'here', maybe: 'bogus' },
+ queryParams: { cacheSeconds: 1000, other: 'here', maybe: 'bogus' },
}).expect(1000)
given({
cacheHeaderConfig,
serviceDefaultCacheLengthSeconds: 900,
- queryParams: { maxAge: 400 },
+ queryParams: { cacheSeconds: 400 },
+ }).expect(900)
+ given({
+ cacheHeaderConfig,
+ serviceDefaultCacheLengthSeconds: 900,
+ queryParams: { cacheSeconds: '-1000' },
}).expect(900)
given({
cacheHeaderConfig,
serviceDefaultCacheLengthSeconds: 900,
- queryParams: { maxAge: '-1000' },
+ queryParams: { cacheSeconds: '' },
}).expect(900)
given({
cacheHeaderConfig,
serviceDefaultCacheLengthSeconds: 900,
- queryParams: { maxAge: '' },
+ queryParams: { cacheSeconds: 'not a number' },
}).expect(900)
given({
cacheHeaderConfig,
serviceDefaultCacheLengthSeconds: 900,
- queryParams: { maxAge: 'not a number' },
+ // Legacy name.
+ queryParams: { maxAge: 1000 },
+ }).expect(1000)
+ given({
+ cacheHeaderConfig,
+ serviceDefaultCacheLengthSeconds: 900,
+ // Both legacy and new name provided -> both ignored.
+ queryParams: { maxAge: 1000, cacheSeconds: 1000 },
}).expect(900)
given({
cacheHeaderConfig,
@@ -79,12 +91,12 @@ describe('Cache header functions', function() {
given({
cacheHeaderConfig,
serviceOverrideCacheLengthSeconds: 800,
- queryParams: { maxAge: 500 },
+ queryParams: { cacheSeconds: 500 },
}).expect(800)
given({
cacheHeaderConfig,
serviceOverrideCacheLengthSeconds: 900,
- queryParams: { maxAge: 800 },
+ queryParams: { cacheSeconds: 800 },
}).expect(900)
})
})
@@ -154,7 +166,7 @@ describe('Cache header functions', function() {
setCacheHeaders({
cacheHeaderConfig: { defaultCacheLengthSeconds: 1234 },
serviceDefaultCacheLengthSeconds: 567,
- queryParams: { maxAge: 999999 },
+ queryParams: { cacheSeconds: 999999 },
res,
})
diff --git a/core/base-service/legacy-request-handler.js b/core/base-service/legacy-request-handler.js
index f5e0903857..ecfeb5218f 100644
--- a/core/base-service/legacy-request-handler.js
+++ b/core/base-service/legacy-request-handler.js
@@ -103,8 +103,8 @@ function handleRequest(cacheHeaderConfig, handlerOptions) {
// by-badge basis). Then in turn that can be overridden by
// `serviceOverrideCacheLengthSeconds` (which we expect to be used only in
// the dynamic badge) but only if `serviceOverrideCacheLengthSeconds` is
- // longer than `serviceDefaultCacheLengthSeconds` and then the `maxAge`
- // query param can also override both of those but again only if `maxAge`
+ // longer than `serviceDefaultCacheLengthSeconds` and then the `cacheSeconds`
+ // query param can also override both of those but again only if `cacheSeconds`
// is longer.
//
// When the legacy services have been rewritten, all the code in here
diff --git a/core/base-service/legacy-request-handler.spec.js b/core/base-service/legacy-request-handler.spec.js
index b1dbc55195..5c465499c8 100644
--- a/core/base-service/legacy-request-handler.spec.js
+++ b/core/base-service/legacy-request-handler.spec.js
@@ -264,9 +264,9 @@ describe('The request handler', function() {
expect(res.headers.get('cache-control')).to.equal('max-age=300')
})
- it('should set the expires header to current time + maxAge', async function() {
+ it('should set the expires header to current time + cacheSeconds', async function() {
register({ cacheHeaderConfig: { defaultCacheLengthSeconds: 0 } })
- const res = await fetch(`${baseUrl}/testing/123.json?maxAge=3600`)
+ const res = await fetch(`${baseUrl}/testing/123.json?cacheSeconds=3600`)
const expectedExpiry = new Date(
+new Date(res.headers.get('date')) + 3600000
).toGMTString()
@@ -274,9 +274,9 @@ describe('The request handler', function() {
expect(res.headers.get('cache-control')).to.equal('max-age=3600')
})
- it('should ignore maxAge if maxAge < defaultCacheLengthSeconds', async function() {
+ it('should ignore cacheSeconds when shorter than defaultCacheLengthSeconds', async function() {
register({ cacheHeaderConfig: { defaultCacheLengthSeconds: 600 } })
- const res = await fetch(`${baseUrl}/testing/123.json?maxAge=300`)
+ const res = await fetch(`${baseUrl}/testing/123.json?cacheSeconds=300`)
const expectedExpiry = new Date(
+new Date(res.headers.get('date')) + 600000
).toGMTString()
@@ -284,7 +284,7 @@ describe('The request handler', function() {
expect(res.headers.get('cache-control')).to.equal('max-age=600')
})
- it('should set Cache-Control: no-cache, no-store, must-revalidate if maxAge=0', async function() {
+ it('should set Cache-Control: no-cache, no-store, must-revalidate if cache seconds is 0', async function() {
register({ cacheHeaderConfig: { defaultCacheLengthSeconds: 0 } })
const res = await fetch(`${baseUrl}/testing/123.json`)
expect(res.headers.get('expires')).to.equal(res.headers.get('date'))
diff --git a/frontend/components/usage.js b/frontend/components/usage.js
index 321af34151..d06d142252 100644
--- a/frontend/components/usage.js
+++ b/frontend/components/usage.js
@@ -387,13 +387,14 @@ export default class Usage extends React.PureComponent {
<QueryParam
documentation={
<span>
- Set the HTTP cache lifetime in secs (rules are applied to
- infer a default value on a per-badge basis, any values
- specified below the default will be ignored)
+ Set the HTTP cache lifetime (rules are applied to infer a
+ default value on a per-badge basis, any values specified below
+ the default will be ignored). The legacy name "maxAge" is also
+ supported.
</span>
}
- key="maxAge"
- snippet="?maxAge=3600"
+ key="cacheSeconds"
+ snippet="?cacheSeconds=3600"
/>
</tbody>
</QueryParamTable>
diff --git a/lib/badge-data.js b/lib/badge-data.js
index 3b4356f7fd..ad8366456f 100644
--- a/lib/badge-data.js
+++ b/lib/badge-data.js
@@ -34,8 +34,9 @@ function makeLabel(defaultLabel, overrides) {
// - color and colorB
// - labelColor and colorA
// - maxAge
+// - cacheSeconds
//
-// Note: maxAge is handled by cache(), not this function.
+// Note: `maxAge` and `cacheSeconds` are handled by cache(), not this function.
function makeBadgeData(defaultLabel, overrides) {
const colorA = coalesce(overrides.labelColor, overrides.colorA)
const colorB = coalesce(overrides.color, overrides.colorB)
diff --git a/services/endpoint/endpoint.tester.js b/services/endpoint/endpoint.tester.js
index 0612a58751..afe210cbf8 100644
--- a/services/endpoint/endpoint.tester.js
+++ b/services/endpoint/endpoint.tester.js
@@ -241,7 +241,7 @@ t.create('cacheSeconds')
.expectHeader('cache-control', 'max-age=500')
t.create('user can override service cacheSeconds')
- .get('.json?url=https://example.com/badge&maxAge=1000')
+ .get('.json?url=https://example.com/badge&cacheSeconds=1000')
.intercept(nock =>
nock('https://example.com/')
.get('/badge')
@@ -255,7 +255,7 @@ t.create('user can override service cacheSeconds')
.expectHeader('cache-control', 'max-age=1000')
t.create('user does not override longer service cacheSeconds')
- .get('.json?url=https://example.com/badge&maxAge=450')
+ .get('.json?url=https://example.com/badge&cacheSeconds=450')
.intercept(nock =>
nock('https://example.com/')
.get('/badge')
--
GitLab