From feb1682814019a660e36f6e31158ed5045c7d878 Mon Sep 17 00:00:00 2001 From: chris48s Date: Mon, 29 Nov 2021 21:21:03 +0000 Subject: [PATCH] Clean up cache module; affects [feedz jenkinsplugin myget node nuget packagist travis wordpress] (#7319) * update terminology - "regular update" to "cached resource" - "interval" to "ttl" - move file and update imports * set a default TTL, don't explicitly pass params if we want the default * add tests * update docs --- .../resource-cache.js} | 30 ++++++------ core/base-service/resource-cache.spec.js | 47 +++++++++++++++++++ core/server/server.js | 4 +- doc/production-hosting.md | 4 +- .../jenkins/jenkins-plugin-version.service.js | 6 +-- services/myget/myget.tester.js | 4 +- services/node/node-version-color.js | 9 ++-- services/nuget/nuget-helpers.js | 7 ++- services/php-version.js | 5 +- services/wordpress/wordpress-version-color.js | 6 +-- 10 files changed, 82 insertions(+), 40 deletions(-) rename core/{legacy/regular-update.js => base-service/resource-cache.js} (59%) create mode 100644 core/base-service/resource-cache.spec.js diff --git a/core/legacy/regular-update.js b/core/base-service/resource-cache.js similarity index 59% rename from core/legacy/regular-update.js rename to core/base-service/resource-cache.js index 4b77cb0152..f759e14714 100644 --- a/core/legacy/regular-update.js +++ b/core/base-service/resource-cache.js @@ -2,36 +2,38 @@ * @module */ -import { InvalidResponse } from '../base-service/errors.js' -import { fetch } from '../../core/base-service/got.js' -import checkErrorResponse from '../../core/base-service/check-error-response.js' +import { InvalidResponse } from './errors.js' +import { fetch } from './got.js' +import checkErrorResponse from './check-error-response.js' + +const oneDay = 24 * 3600 * 1000 // 1 day in milliseconds // Map from URL to { timestamp: last fetch time, data: data }. -let regularUpdateCache = Object.create(null) +let resourceCache = Object.create(null) /** * Make a HTTP request using an in-memory cache * * @param {object} attrs Refer to individual attrs * @param {string} attrs.url URL to request - * @param {number} attrs.intervalMillis Number of milliseconds to keep cached value for + * @param {number} attrs.ttl Number of milliseconds to keep cached value for * @param {boolean} [attrs.json=true] True if we expect to parse the response as JSON * @param {Function} [attrs.scraper=buffer => buffer] Function to extract value from the response * @param {object} [attrs.options={}] Options to pass to got - * @param {Function} [attrs.requestFetcher=fetcher] Custom fetch function + * @param {Function} [attrs.requestFetcher=fetch] Custom fetch function * @returns {*} Parsed response */ -async function regularUpdate({ +async function getCachedResource({ url, - intervalMillis, + ttl = oneDay, json = true, scraper = buffer => buffer, options = {}, requestFetcher = fetch, }) { const timestamp = Date.now() - const cached = regularUpdateCache[url] - if (cached != null && timestamp - cached.timestamp < intervalMillis) { + const cached = resourceCache[url] + if (cached != null && timestamp - cached.timestamp < ttl) { return cached.data } @@ -54,12 +56,12 @@ async function regularUpdate({ } const data = scraper(reqData) - regularUpdateCache[url] = { timestamp, data } + resourceCache[url] = { timestamp, data } return data } -function clearRegularUpdateCache() { - regularUpdateCache = Object.create(null) +function clearResourceCache() { + resourceCache = Object.create(null) } -export { regularUpdate, clearRegularUpdateCache } +export { getCachedResource, clearResourceCache } diff --git a/core/base-service/resource-cache.spec.js b/core/base-service/resource-cache.spec.js new file mode 100644 index 0000000000..4344c458af --- /dev/null +++ b/core/base-service/resource-cache.spec.js @@ -0,0 +1,47 @@ +import { expect } from 'chai' +import nock from 'nock' +import { getCachedResource, clearResourceCache } from './resource-cache.js' + +describe('Resource Cache', function () { + beforeEach(function () { + clearResourceCache() + }) + + it('should use cached response if valid', async function () { + let resp + + nock('https://www.foobar.com').get('/baz').reply(200, { value: 1 }) + resp = await getCachedResource({ url: 'https://www.foobar.com/baz' }) + expect(resp).to.deep.equal({ value: 1 }) + expect(nock.isDone()).to.equal(true) + nock.cleanAll() + + nock('https://www.foobar.com').get('/baz').reply(200, { value: 2 }) + resp = await getCachedResource({ url: 'https://www.foobar.com/baz' }) + expect(resp).to.deep.equal({ value: 1 }) + expect(nock.isDone()).to.equal(false) + nock.cleanAll() + }) + + it('should not use cached response if expired', async function () { + let resp + + nock('https://www.foobar.com').get('/baz').reply(200, { value: 1 }) + resp = await getCachedResource({ + url: 'https://www.foobar.com/baz', + ttl: 1, + }) + expect(resp).to.deep.equal({ value: 1 }) + expect(nock.isDone()).to.equal(true) + nock.cleanAll() + + nock('https://www.foobar.com').get('/baz').reply(200, { value: 2 }) + resp = await getCachedResource({ + url: 'https://www.foobar.com/baz', + ttl: 1, + }) + expect(resp).to.deep.equal({ value: 2 }) + expect(nock.isDone()).to.equal(true) + nock.cleanAll() + }) +}) diff --git a/core/server/server.js b/core/server/server.js index defe835f3e..386cb52f97 100644 --- a/core/server/server.js +++ b/core/server/server.js @@ -15,7 +15,7 @@ import { setRoutes } from '../../services/suggest.js' import { loadServiceClasses } from '../base-service/loader.js' import { makeSend } from '../base-service/legacy-result-sender.js' import { handleRequest } from '../base-service/legacy-request-handler.js' -import { clearRegularUpdateCache } from '../legacy/regular-update.js' +import { clearResourceCache } from '../base-service/resource-cache.js' import { rasterRedirectUrl } from '../badge-urls/make-badge-url.js' import { fileSize, nonNegativeInteger } from '../../services/validators.js' import log from './log.js' @@ -531,7 +531,7 @@ class Server { static resetGlobalState() { // This state should be migrated to instance state. When possible, do not add new // global state. - clearRegularUpdateCache() + clearResourceCache() } reset() { diff --git a/doc/production-hosting.md b/doc/production-hosting.md index 8a8120853d..e39eb3ca44 100644 --- a/doc/production-hosting.md +++ b/doc/production-hosting.md @@ -49,11 +49,11 @@ Shields has mercifully little persistent state: 1. The GitHub tokens we collect are saved on each server in a cloud Redis database. They can also be fetched from the [GitHub auth admin endpoint][] for debugging. -2. The server keeps the [regular-update cache][] in memory. It is neither +2. The server keeps the [resource cache][] in memory. It is neither persisted nor inspectable. [github auth admin endpoint]: https://github.com/badges/shields/blob/master/services/github/auth/admin.js -[regular-update cache]: https://github.com/badges/shields/blob/master/core/legacy/regular-update.js +[resource cache]: https://github.com/badges/shields/blob/master/core/base-service/resource-cache.js ## Configuration diff --git a/services/jenkins/jenkins-plugin-version.service.js b/services/jenkins/jenkins-plugin-version.service.js index 8813bf5425..46888ce2a2 100644 --- a/services/jenkins/jenkins-plugin-version.service.js +++ b/services/jenkins/jenkins-plugin-version.service.js @@ -1,4 +1,4 @@ -import { regularUpdate } from '../../core/legacy/regular-update.js' +import { getCachedResource } from '../../core/base-service/resource-cache.js' import { renderVersionBadge } from '../version.js' import { BaseService, NotFound } from '../index.js' @@ -31,9 +31,9 @@ export default class JenkinsPluginVersion extends BaseService { } async fetch() { - return regularUpdate({ + return getCachedResource({ url: 'https://updates.jenkins-ci.org/current/update-center.actual.json', - intervalMillis: 4 * 3600 * 1000, + ttl: 4 * 3600 * 1000, // 4 hours in milliseconds scraper: json => Object.keys(json.plugins).reduce((previous, current) => { previous[current] = json.plugins[current].version diff --git a/services/myget/myget.tester.js b/services/myget/myget.tester.js index c916a025a0..1596b4a5ff 100644 --- a/services/myget/myget.tester.js +++ b/services/myget/myget.tester.js @@ -37,7 +37,7 @@ t.create('total downloads (not found)') .get('/myget/mongodb/dt/not-a-real-package.json') .expectBadge({ label: 'downloads', message: 'package not found' }) -// This tests the erroring behavior in regular-update. +// This tests the erroring behavior in getCachedResource. t.create('total downloads (connection error)') .get('/myget/mongodb/dt/MongoDB.Driver.Core.json') .networkOff() @@ -46,7 +46,7 @@ t.create('total downloads (connection error)') message: 'inaccessible', }) -// This tests the erroring behavior in regular-update. +// This tests the erroring behavior in getCachedResource. t.create('total downloads (unexpected first response)') .get('/myget/mongodb/dt/MongoDB.Driver.Core.json') .intercept(nock => diff --git a/services/node/node-version-color.js b/services/node/node-version-color.js index 12edd1d73f..9d36a36f84 100644 --- a/services/node/node-version-color.js +++ b/services/node/node-version-color.js @@ -1,6 +1,6 @@ import moment from 'moment' import semver from 'semver' -import { regularUpdate } from '../../core/legacy/regular-update.js' +import { getCachedResource } from '../../core/base-service/resource-cache.js' const dateFormat = 'YYYY-MM-DD' @@ -9,9 +9,8 @@ async function getVersion(version) { if (version) { semver = `-${version}.x` } - return regularUpdate({ + return getCachedResource({ url: `https://nodejs.org/dist/latest${semver}/SHASUMS256.txt`, - intervalMillis: 24 * 3600 * 1000, json: false, scraper: shasums => { // tarball index start, tarball index end @@ -36,10 +35,8 @@ async function getCurrentVersion() { } async function getLtsVersions() { - const versions = await regularUpdate({ + const versions = await getCachedResource({ url: 'https://raw.githubusercontent.com/nodejs/Release/master/schedule.json', - intervalMillis: 24 * 3600 * 1000, - json: true, scraper: ltsVersionsScraper, }) return Promise.all(versions.map(getVersion)) diff --git a/services/nuget/nuget-helpers.js b/services/nuget/nuget-helpers.js index cc5b6905a8..c3fa42b605 100644 --- a/services/nuget/nuget-helpers.js +++ b/services/nuget/nuget-helpers.js @@ -1,7 +1,7 @@ import semver from 'semver' import { metric, addv } from '../text-formatters.js' import { downloadCount as downloadCountColor } from '../color-formatters.js' -import { regularUpdate } from '../../core/legacy/regular-update.js' +import { getCachedResource } from '../../core/base-service/resource-cache.js' function renderVersionBadge({ version, feed }) { let color @@ -51,7 +51,7 @@ function randomElementFrom(items) { */ async function searchServiceUrl(baseUrl, serviceType = 'SearchQueryService') { // Should we really be caching all these NuGet feeds in memory? - const searchQueryServices = await regularUpdate({ + const searchQueryServices = await getCachedResource({ url: `${baseUrl}/index.json`, // The endpoint changes once per year (ie, a period of n = 1 year). // We minimize the users' waiting time for information. @@ -61,8 +61,7 @@ async function searchServiceUrl(baseUrl, serviceType = 'SearchQueryService') { // right endpoint. // So the waiting time within n years is n*l/x + x years, for which a // derivation yields an optimum at x = sqrt(n*l), roughly 42 minutes. - intervalMillis: 42 * 60 * 1000, - json: true, + ttl: 42 * 60 * 1000, scraper: json => json.resources.filter(resource => resource['@type'] === serviceType), }) diff --git a/services/php-version.js b/services/php-version.js index 06c3485d2e..3298b15081 100644 --- a/services/php-version.js +++ b/services/php-version.js @@ -4,7 +4,7 @@ * https://getcomposer.org/doc/04-schema.md#version). */ import { fetch } from '../core/base-service/got.js' -import { regularUpdate } from '../core/legacy/regular-update.js' +import { getCachedResource } from '../core/base-service/resource-cache.js' import { listCompare } from './version.js' import { omitv } from './text-formatters.js' @@ -217,9 +217,8 @@ function versionReduction(versions, phpReleases) { } async function getPhpReleases(githubApiProvider) { - return regularUpdate({ + return getCachedResource({ url: '/repos/php/php-src/git/refs/tags', - intervalMillis: 24 * 3600 * 1000, // 1 day scraper: tags => Array.from( new Set( diff --git a/services/wordpress/wordpress-version-color.js b/services/wordpress/wordpress-version-color.js index 449c8d4fc4..1e01ded09a 100644 --- a/services/wordpress/wordpress-version-color.js +++ b/services/wordpress/wordpress-version-color.js @@ -1,5 +1,5 @@ import semver from 'semver' -import { regularUpdate } from '../../core/legacy/regular-update.js' +import { getCachedResource } from '../../core/base-service/resource-cache.js' // TODO: Incorporate this schema. // const schema = Joi.object() @@ -19,10 +19,8 @@ import { regularUpdate } from '../../core/legacy/regular-update.js' // .required() async function getOfferedVersions() { - return regularUpdate({ + return getCachedResource({ url: 'https://api.wordpress.org/core/version-check/1.7/', - intervalMillis: 24 * 3600 * 1000, - json: true, scraper: json => json.offers.map(v => v.version), }) }