diff --git a/lib/coalesce.js b/lib/coalesce.js new file mode 100644 index 0000000000000000000000000000000000000000..4875b409c7925377f9eaefd3a0d6770145181b49 --- /dev/null +++ b/lib/coalesce.js @@ -0,0 +1,5 @@ +'use strict' + +module.exports = function coalesce(...candidates) { + return candidates.find(c => c !== undefined && c !== null) +} diff --git a/lib/coalesce.spec.js b/lib/coalesce.spec.js new file mode 100644 index 0000000000000000000000000000000000000000..ed596d4276f0b4d7f3e4270244405a933f101dda --- /dev/null +++ b/lib/coalesce.spec.js @@ -0,0 +1,23 @@ +'use strict' + +const { test, given } = require('sazerac') +const coalesce = require('./coalesce') + +// Sticking with our one-line spread implementation, and defaulting to +// `undefined` instead of `null`, though h/t to +// https://github.com/royriojas/coalescy for these tests! + +describe('coalesce', function() { + test(coalesce, function() { + given().expect(undefined) + given(null, []).expect([]) + given(null, [], {}).expect([]) + given(null, undefined, 0, {}).expect(0) + + const a = null, + c = 0, + d = 1 + let b + given(a, b, c, d).expect(0) + }) +}) diff --git a/lib/request-handler.js b/lib/request-handler.js index 7fc717b278eb900cf0ba41b68c3900b1d339c9c5..e9938da800fa3790e02a19416581e569ff453af5 100644 --- a/lib/request-handler.js +++ b/lib/request-handler.js @@ -11,6 +11,7 @@ const analytics = require('./analytics') const { makeSend } = require('./result-sender') const queryString = require('query-string') const { Inaccessible } = require('../services/errors') +const { setCacheHeaders } = require('../services/cache-headers') // We avoid calling the vendor's server for computation of the information in a // number of badges. @@ -57,23 +58,6 @@ function flattenQueryParams(queryParams) { return Array.from(union).sort() } -function getBadgeMaxAge(handlerOptions, queryParams) { - let maxAge = isInt(process.env.BADGE_MAX_AGE_SECONDS) - ? parseInt(process.env.BADGE_MAX_AGE_SECONDS) - : 120 - if (handlerOptions.cacheLength) { - // If we've set a more specific cache length for this badge (or category), - // use that instead of env.BADGE_MAX_AGE_SECONDS. - maxAge = handlerOptions.cacheLength - } - if (isInt(queryParams.maxAge) && parseInt(queryParams.maxAge) > maxAge) { - // Only allow queryParams.maxAge to override the default if it is greater - // than the default. - maxAge = parseInt(queryParams.maxAge) - } - return maxAge -} - // handlerOptions can contain: // - handler: The service's request handler function // - queryParams: An array of the field names of any custom query parameters @@ -89,31 +73,23 @@ function getBadgeMaxAge(handlerOptions, queryParams) { // (undesirable and hard to debug). // // Pass just the handler function as shorthand. -function handleRequest(handlerOptions) { +function handleRequest(cacheHeaderConfig, handlerOptions) { if (typeof handlerOptions === 'function') { handlerOptions = { handler: handlerOptions } } const allowedKeys = flattenQueryParams(handlerOptions.queryParams) + const { cacheLength: serviceCacheLengthSeconds } = handlerOptions return (queryParams, match, end, ask) => { const reqTime = new Date() - const maxAge = getBadgeMaxAge(handlerOptions, queryParams) - // send both Cache-Control max-age and Expires - // in case the client implements HTTP/1.0 but not HTTP/1.1 - if (maxAge === 0) { - ask.res.setHeader('Cache-Control', 'no-cache, no-store, must-revalidate') - ask.res.setHeader('Expires', reqTime.toGMTString()) - } else { - ask.res.setHeader('Cache-Control', `max-age=${maxAge}`) - ask.res.setHeader( - 'Expires', - new Date(+reqTime + maxAge * 1000).toGMTString() - ) - } - - ask.res.setHeader('Date', reqTime.toGMTString()) + setCacheHeaders({ + cacheHeaderConfig, + serviceCacheLengthSeconds, + queryParams, + res: ask.res, + }) analytics.noteRequest(queryParams, match) @@ -269,14 +245,9 @@ function clearRequestCache() { requestCache.clear() } -function isInt(number) { - return number !== undefined && /^[0-9]+$/.test(number) -} - module.exports = { handleRequest, clearRequestCache, // Expose for testing. _requestCache: requestCache, - getBadgeMaxAge, } diff --git a/lib/request-handler.spec.js b/lib/request-handler.spec.js index 26b46e3e528508e3e31aec603f339f73cd131adb..cd269f77d7e39f6e017d71cd4b37e19662252e10 100644 --- a/lib/request-handler.spec.js +++ b/lib/request-handler.spec.js @@ -1,7 +1,6 @@ 'use strict' const { expect } = require('chai') -const { test, given } = require('sazerac') const fetch = require('node-fetch') const config = require('./test-config') const Camp = require('camp') @@ -11,7 +10,6 @@ const { handleRequest, clearRequestCache, _requestCache, - getBadgeMaxAge, } = require('./request-handler') const baseUri = `http://127.0.0.1:${config.port}` @@ -32,8 +30,6 @@ describe('The request handler', function() { before(analytics.load) let camp - const initialBadgeMaxAge = process.env.BADGE_MAX_AGE_SECONDS - beforeEach(function(done) { camp = Camp.start({ port: config.port, hostname: '::' }) camp.on('listening', () => done()) @@ -44,14 +40,15 @@ describe('The request handler', function() { camp.close(() => done()) camp = null } - process.env.BADGE_MAX_AGE_SECONDS = initialBadgeMaxAge }) + const standardCacheHeaders = { defaultCacheLengthSeconds: 120 } + describe('the options object calling style', function() { beforeEach(function() { camp.route( /^\/testing\/([^/]+)\.(svg|png|gif|jpg|json)$/, - handleRequest({ handler: fakeHandler }) + handleRequest(standardCacheHeaders, { handler: fakeHandler }) ) }) @@ -66,7 +63,7 @@ describe('The request handler', function() { beforeEach(function() { camp.route( /^\/testing\/([^/]+)\.(svg|png|gif|jpg|json)$/, - handleRequest(fakeHandler) + handleRequest(standardCacheHeaders, fakeHandler) ) }) @@ -82,38 +79,50 @@ describe('The request handler', function() { let handlerCallCount beforeEach(function() { handlerCallCount = 0 + }) + + function register({ cacheHeaderConfig }) { camp.route( /^\/testing\/([^/]+)\.(svg|png|gif|jpg|json)$/, - handleRequest((queryParams, match, sendBadge, request) => { - ++handlerCallCount - fakeHandler(queryParams, match, sendBadge, request) - }) + handleRequest( + cacheHeaderConfig, + (queryParams, match, sendBadge, request) => { + ++handlerCallCount + fakeHandler(queryParams, match, sendBadge, request) + } + ) ) - }) + } - it('should cache identical requests', async function() { - await performTwoRequests('/testing/123.svg', '/testing/123.svg') - expect(handlerCallCount).to.equal(1) - }) + context('With standard cache settings', function() { + beforeEach(function() { + register({ cacheHeaderConfig: standardCacheHeaders }) + }) - it('should differentiate known query parameters', async function() { - await performTwoRequests( - '/testing/123.svg?label=foo', - '/testing/123.svg?label=bar' - ) - expect(handlerCallCount).to.equal(2) - }) + it('should cache identical requests', async function() { + await performTwoRequests('/testing/123.svg', '/testing/123.svg') + expect(handlerCallCount).to.equal(1) + }) - it('should ignore unknown query parameters', async function() { - await performTwoRequests( - '/testing/123.svg?foo=1', - '/testing/123.svg?foo=2' - ) - expect(handlerCallCount).to.equal(1) + it('should differentiate known query parameters', async function() { + await performTwoRequests( + '/testing/123.svg?label=foo', + '/testing/123.svg?label=bar' + ) + expect(handlerCallCount).to.equal(2) + }) + + it('should ignore unknown query parameters', async function() { + await performTwoRequests( + '/testing/123.svg?foo=1', + '/testing/123.svg?foo=2' + ) + expect(handlerCallCount).to.equal(1) + }) }) - it('should set the expires header to current time + BADGE_MAX_AGE_SECONDS', async function() { - process.env.BADGE_MAX_AGE_SECONDS = 900 + it('should set the expires header to current time + defaultCacheLengthSeconds', async function() { + register({ cacheHeaderConfig: { defaultCacheLengthSeconds: 900 } }) const res = await fetch(`${baseUri}/testing/123.json`) const expectedExpiry = new Date( +new Date(res.headers.get('date')) + 900000 @@ -123,7 +132,7 @@ describe('The request handler', function() { }) it('should set the expires header to current time + maxAge', async function() { - process.env.BADGE_MAX_AGE_SECONDS = 0 + register({ cacheHeaderConfig: { defaultCacheLengthSeconds: 0 } }) const res = await fetch(`${baseUri}/testing/123.json?maxAge=3600`) const expectedExpiry = new Date( +new Date(res.headers.get('date')) + 3600000 @@ -132,8 +141,8 @@ describe('The request handler', function() { expect(res.headers.get('cache-control')).to.equal('max-age=3600') }) - it('should ignore maxAge if maxAge < BADGE_MAX_AGE_SECONDS', async function() { - process.env.BADGE_MAX_AGE_SECONDS = 600 + it('should ignore maxAge if maxAge < defaultCacheLengthSeconds', async function() { + register({ cacheHeaderConfig: { defaultCacheLengthSeconds: 600 } }) const res = await fetch(`${baseUri}/testing/123.json?maxAge=300`) const expectedExpiry = new Date( +new Date(res.headers.get('date')) + 600000 @@ -143,7 +152,7 @@ describe('The request handler', function() { }) it('should set Cache-Control: no-cache, no-store, must-revalidate if maxAge=0', async function() { - process.env.BADGE_MAX_AGE_SECONDS = 0 + register({ cacheHeaderConfig: { defaultCacheLengthSeconds: 0 } }) const res = await fetch(`${baseUri}/testing/123.json`) expect(res.headers.get('expires')).to.equal(res.headers.get('date')) expect(res.headers.get('cache-control')).to.equal( @@ -151,17 +160,10 @@ describe('The request handler', function() { ) }) - it('should set the expires header to current time + 120 if BADGE_MAX_AGE_SECONDS not set', async function() { - delete process.env.BADGE_MAX_AGE_SECONDS - const res = await fetch(`${baseUri}/testing/123.json`) - const expectedExpiry = new Date( - +new Date(res.headers.get('date')) + 120000 - ).toGMTString() - expect(res.headers.get('expires')).to.equal(expectedExpiry) - expect(res.headers.get('cache-control')).to.equal('max-age=120') - }) - describe('the cache key', function() { + beforeEach(function() { + register({ cacheHeaderConfig: standardCacheHeaders }) + }) const expectedCacheKey = '/testing/123.json?colorB=123&label=foo' it('should match expected and use canonical order - 1', async function() { const res = await fetch( @@ -186,7 +188,7 @@ describe('The request handler', function() { handlerCallCount = 0 camp.route( /^\/testing\/([^/]+)\.(svg|png|gif|jpg|json)$/, - handleRequest({ + handleRequest(standardCacheHeaders, { queryParams: ['foo'], handler: (queryParams, match, sendBadge, request) => { ++handlerCallCount @@ -205,14 +207,4 @@ describe('The request handler', function() { }) }) }) - - describe('getBadgeMaxAge function', function() { - process.env.BADGE_MAX_AGE_SECONDS = 120 - test(getBadgeMaxAge, () => { - given({}, {}).expect(120) - given({ cacheLength: 900 }, {}).expect(900) - given({ cacheLength: 900 }, { maxAge: 1000 }).expect(1000) - given({ cacheLength: 900 }, { maxAge: 400 }).expect(900) - }) - }) }) diff --git a/lib/server-config.js b/lib/server-config.js index 01832d822cbad8e45b34ccb11470c95b34e91a9d..3861158f79ddc88e9cacaf025d94ac00a7e14cb7 100644 --- a/lib/server-config.js +++ b/lib/server-config.js @@ -63,7 +63,8 @@ const config = { baseUri: process.env.GITHUB_URL || 'https://api.github.com', debug: { enabled: envFlag(process.env.GITHUB_DEBUG_ENABLED, false), - intervalSeconds: process.env.GITHUB_DEBUG_INTERVAL_SECONDS || 300, + intervalSeconds: + parseInt(process.env.GITHUB_DEBUG_INTERVAL_SECONDS) || 300, }, }, trace: envFlag(process.env.TRACE_SERVICES), @@ -71,6 +72,10 @@ const config = { profiling: { makeBadge: envFlag(process.env.PROFILE_MAKE_BADGE), }, + cacheHeaders: { + defaultCacheLengthSeconds: + parseInt(process.env.BADGE_MAX_AGE_SECONDS) || 120, + }, rateLimit: envFlag(process.env.RATE_LIMIT, true), handleInternalErrors: envFlag(process.env.HANDLE_INTERNAL_ERRORS, true), } diff --git a/package-lock.json b/package-lock.json index a25e2a2148860b682877d8ce89d105ba2fe47f06..d69417c7a8555de81949203796e28707d1701c31 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1423,8 +1423,7 @@ "acorn": { "version": "2.7.0", "resolved": "https://registry.npmjs.org/acorn/-/acorn-2.7.0.tgz", - "integrity": "sha1-q259nYhqrKiwhbwzEreaGYQz8Oc=", - "optional": true + "integrity": "sha1-q259nYhqrKiwhbwzEreaGYQz8Oc=" }, "acorn-dynamic-import": { "version": "2.0.2", @@ -1717,7 +1716,7 @@ }, "util": { "version": "0.10.3", - "resolved": "https://registry.npmjs.org/util/-/util-0.10.3.tgz", + "resolved": "http://registry.npmjs.org/util/-/util-0.10.3.tgz", "integrity": "sha1-evsa/lCAUkZInj23/g7TeTNqwPk=", "dev": true, "requires": { @@ -2422,6 +2421,15 @@ } } }, + "chai-datetime": { + "version": "1.5.0", + "resolved": "https://registry.npmjs.org/chai-datetime/-/chai-datetime-1.5.0.tgz", + "integrity": "sha1-N0LxiwJMdbdqK37uKRZiMkRnWWw=", + "dev": true, + "requires": { + "chai": ">1.9.0" + } + }, "chai-string": { "version": "1.5.0", "resolved": "https://registry.npmjs.org/chai-string/-/chai-string-1.5.0.tgz", @@ -3458,8 +3466,7 @@ "cssom": { "version": "0.3.2", "resolved": "https://registry.npmjs.org/cssom/-/cssom-0.3.2.tgz", - "integrity": "sha1-uANhcMefB6kP8vFuIihAJ6JDhIs=", - "optional": true + "integrity": "sha1-uANhcMefB6kP8vFuIihAJ6JDhIs=" }, "cssstyle": { "version": "0.2.37", @@ -3894,7 +3901,7 @@ }, "readable-stream": { "version": "2.3.6", - "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "integrity": "sha512-tQtKA9WIAhBF3+VLAseyMqZeBjW0AHJoxOtYqSUZNJxauErmLbVm2FW1y+J/YA9dUrAC39ITejlZWhVIwawkKw==", "dev": true, "requires": { @@ -5258,7 +5265,7 @@ }, "readable-stream": { "version": "2.3.6", - "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "integrity": "sha512-tQtKA9WIAhBF3+VLAseyMqZeBjW0AHJoxOtYqSUZNJxauErmLbVm2FW1y+J/YA9dUrAC39ITejlZWhVIwawkKw==", "dev": true, "requires": { @@ -5405,7 +5412,7 @@ }, "readable-stream": { "version": "2.3.6", - "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "integrity": "sha512-tQtKA9WIAhBF3+VLAseyMqZeBjW0AHJoxOtYqSUZNJxauErmLbVm2FW1y+J/YA9dUrAC39ITejlZWhVIwawkKw==", "dev": true, "requires": { @@ -5481,8 +5488,7 @@ "ansi-regex": { "version": "2.1.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "aproba": { "version": "1.2.0", @@ -5503,14 +5509,12 @@ "balanced-match": { "version": "1.0.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "brace-expansion": { "version": "1.1.11", "bundled": true, "dev": true, - "optional": true, "requires": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" @@ -5525,20 +5529,17 @@ "code-point-at": { "version": "1.1.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "concat-map": { "version": "0.0.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "console-control-strings": { "version": "1.1.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "core-util-is": { "version": "1.0.2", @@ -5655,8 +5656,7 @@ "inherits": { "version": "2.0.3", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "ini": { "version": "1.3.5", @@ -5668,7 +5668,6 @@ "version": "1.0.0", "bundled": true, "dev": true, - "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -5683,7 +5682,6 @@ "version": "3.0.4", "bundled": true, "dev": true, - "optional": true, "requires": { "brace-expansion": "^1.1.7" } @@ -5691,14 +5689,12 @@ "minimist": { "version": "0.0.8", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "minipass": { "version": "2.2.4", "bundled": true, "dev": true, - "optional": true, "requires": { "safe-buffer": "^5.1.1", "yallist": "^3.0.0" @@ -5717,7 +5713,6 @@ "version": "0.5.1", "bundled": true, "dev": true, - "optional": true, "requires": { "minimist": "0.0.8" } @@ -5798,8 +5793,7 @@ "number-is-nan": { "version": "1.0.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "object-assign": { "version": "4.1.1", @@ -5811,7 +5805,6 @@ "version": "1.4.0", "bundled": true, "dev": true, - "optional": true, "requires": { "wrappy": "1" } @@ -5897,8 +5890,7 @@ "safe-buffer": { "version": "5.1.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "safer-buffer": { "version": "2.1.2", @@ -5934,7 +5926,6 @@ "version": "1.0.2", "bundled": true, "dev": true, - "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", @@ -5954,7 +5945,6 @@ "version": "3.0.1", "bundled": true, "dev": true, - "optional": true, "requires": { "ansi-regex": "^2.0.0" } @@ -5998,14 +5988,12 @@ "wrappy": { "version": "1.0.2", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "yallist": { "version": "3.0.2", "bundled": true, - "dev": true, - "optional": true + "dev": true } } }, @@ -6497,7 +6485,7 @@ }, "readable-stream": { "version": "2.3.6", - "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "integrity": "sha512-tQtKA9WIAhBF3+VLAseyMqZeBjW0AHJoxOtYqSUZNJxauErmLbVm2FW1y+J/YA9dUrAC39ITejlZWhVIwawkKw==", "requires": { "core-util-is": "~1.0.0", @@ -7984,7 +7972,7 @@ "dependencies": { "chalk": { "version": "1.1.3", - "resolved": "https://registry.npmjs.org/chalk/-/chalk-1.1.3.tgz", + "resolved": "http://registry.npmjs.org/chalk/-/chalk-1.1.3.tgz", "integrity": "sha1-qBFcVeSnAv5NFQq9OHKCKn4J/Jg=", "dev": true, "requires": { @@ -8443,6 +8431,12 @@ "resolved": "https://registry.npmjs.org/mdn-data/-/mdn-data-1.1.4.tgz", "integrity": "sha512-FSYbp3lyKjyj3E7fMl6rYvUdX0FBXaluGqlFoYESWQlyUTq8R+wp0rkFxoYFqZlHCvsUXGjyJmLQSnXToYhOSA==" }, + "media-typer": { + "version": "0.3.0", + "resolved": "http://registry.npmjs.org/media-typer/-/media-typer-0.3.0.tgz", + "integrity": "sha1-hxDXrwqmJvj/+hzgAWhUUmMlV0g=", + "dev": true + }, "mem": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/mem/-/mem-1.1.0.tgz", @@ -8482,7 +8476,7 @@ }, "readable-stream": { "version": "2.3.6", - "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "integrity": "sha512-tQtKA9WIAhBF3+VLAseyMqZeBjW0AHJoxOtYqSUZNJxauErmLbVm2FW1y+J/YA9dUrAC39ITejlZWhVIwawkKw==", "dev": true, "requires": { @@ -8607,6 +8601,18 @@ } } }, + "merge-descriptors": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/merge-descriptors/-/merge-descriptors-1.0.1.tgz", + "integrity": "sha1-sAqqVW3YtEVoFQ7J0blT8/kMu2E=", + "dev": true + }, + "methods": { + "version": "1.1.2", + "resolved": "https://registry.npmjs.org/methods/-/methods-1.1.2.tgz", + "integrity": "sha1-VSmk1nZUE07cxSZmVoNbD4Ua/O4=", + "dev": true + }, "micromatch": { "version": "3.1.10", "resolved": "https://registry.npmjs.org/micromatch/-/micromatch-3.1.10.tgz", @@ -8983,6 +8989,12 @@ "integrity": "sha512-MFh0d/Wa7vkKO3Y3LlacqAEeHK0mckVqzDieUKTT+KGxi+zIpeVsFxymkIiRpbpDziHc290Xr9A1O4Om7otoRA==", "dev": true }, + "net": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/net/-/net-1.0.2.tgz", + "integrity": "sha1-0XV+yaf7I3HYPPR1XOPifhCCk4g=", + "dev": true + }, "next": { "version": "6.1.2", "resolved": "https://registry.npmjs.org/next/-/next-6.1.2.tgz", @@ -10139,7 +10151,7 @@ }, "readable-stream": { "version": "2.3.6", - "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "integrity": "sha512-tQtKA9WIAhBF3+VLAseyMqZeBjW0AHJoxOtYqSUZNJxauErmLbVm2FW1y+J/YA9dUrAC39ITejlZWhVIwawkKw==", "dev": true, "requires": { @@ -10163,6 +10175,24 @@ } } }, + "node-mocks-http": { + "version": "1.7.3", + "resolved": "https://registry.npmjs.org/node-mocks-http/-/node-mocks-http-1.7.3.tgz", + "integrity": "sha512-wayzLNhEroH3lJj113pFKQ1cd1GKG1mXoZR1HcKp/o9a9lTGGgVY/hYeLajiIFr/z4tXFKOdfJickqqihBtn9g==", + "dev": true, + "requires": { + "accepts": "^1.3.5", + "depd": "^1.1.0", + "fresh": "^0.5.2", + "merge-descriptors": "^1.0.1", + "methods": "^1.1.2", + "mime": "^1.3.4", + "net": "^1.0.2", + "parseurl": "^1.3.1", + "range-parser": "^1.2.0", + "type-is": "^1.6.16" + } + }, "node-modules-regexp": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/node-modules-regexp/-/node-modules-regexp-1.0.0.tgz", @@ -10333,7 +10363,6 @@ "resolved": false, "integrity": "sha1-DNkKVhCT810KmSVsIrcGlDP60Rc=", "dev": true, - "optional": true, "requires": { "kind-of": "^3.0.2", "longest": "^1.0.1", @@ -10703,8 +10732,7 @@ "version": "1.1.6", "resolved": false, "integrity": "sha512-NcdALwpXkTm5Zvvbk7owOUSvVvBKDgKP5/ewfXEznmQFfs4ZRmanOeKBTjRVjka3QFoN6XJ+9F3USqfHqTaU5w==", - "dev": true, - "optional": true + "dev": true }, "is-builtin-module": { "version": "1.0.0", @@ -10800,7 +10828,6 @@ "resolved": false, "integrity": "sha1-MeohpzS6ubuw8yRm2JOupR5KPGQ=", "dev": true, - "optional": true, "requires": { "is-buffer": "^1.1.5" } @@ -10853,8 +10880,7 @@ "version": "1.0.1", "resolved": false, "integrity": "sha1-MKCy2jj3N3DoKUoNIuZiXtd9AJc=", - "dev": true, - "optional": true + "dev": true }, "lru-cache": { "version": "4.1.3", @@ -11157,8 +11183,7 @@ "version": "1.6.1", "resolved": false, "integrity": "sha1-jcrkcOHIirwtYA//Sndihtp15jc=", - "dev": true, - "optional": true + "dev": true }, "require-directory": { "version": "2.1.1", @@ -11921,7 +11946,7 @@ }, "readable-stream": { "version": "2.3.6", - "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "integrity": "sha512-tQtKA9WIAhBF3+VLAseyMqZeBjW0AHJoxOtYqSUZNJxauErmLbVm2FW1y+J/YA9dUrAC39ITejlZWhVIwawkKw==", "dev": true, "requires": { @@ -12064,6 +12089,12 @@ "better-assert": "~1.0.0" } }, + "parseurl": { + "version": "1.3.2", + "resolved": "https://registry.npmjs.org/parseurl/-/parseurl-1.3.2.tgz", + "integrity": "sha1-/CidTtiZMRlGDBViUyYs3I3mW/M=", + "dev": true + }, "pascalcase": { "version": "0.1.1", "resolved": "https://registry.npmjs.org/pascalcase/-/pascalcase-0.1.1.tgz", @@ -12821,7 +12852,7 @@ }, "readable-stream": { "version": "2.3.6", - "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "integrity": "sha512-tQtKA9WIAhBF3+VLAseyMqZeBjW0AHJoxOtYqSUZNJxauErmLbVm2FW1y+J/YA9dUrAC39ITejlZWhVIwawkKw==", "dev": true, "requires": { @@ -14024,7 +14055,7 @@ }, "readable-stream": { "version": "2.3.6", - "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "integrity": "sha512-tQtKA9WIAhBF3+VLAseyMqZeBjW0AHJoxOtYqSUZNJxauErmLbVm2FW1y+J/YA9dUrAC39ITejlZWhVIwawkKw==", "requires": { "core-util-is": "~1.0.0", @@ -14236,7 +14267,7 @@ }, "readable-stream": { "version": "2.3.6", - "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "integrity": "sha512-tQtKA9WIAhBF3+VLAseyMqZeBjW0AHJoxOtYqSUZNJxauErmLbVm2FW1y+J/YA9dUrAC39ITejlZWhVIwawkKw==", "dev": true, "requires": { @@ -14297,7 +14328,7 @@ }, "readable-stream": { "version": "2.3.6", - "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "integrity": "sha512-tQtKA9WIAhBF3+VLAseyMqZeBjW0AHJoxOtYqSUZNJxauErmLbVm2FW1y+J/YA9dUrAC39ITejlZWhVIwawkKw==", "dev": true, "requires": { @@ -14879,6 +14910,33 @@ "resolved": "https://registry.npmjs.org/type-detect/-/type-detect-4.0.3.tgz", "integrity": "sha1-Dj8mcLRAmbC0bChNE2p+9Jx0wuo=" }, + "type-is": { + "version": "1.6.16", + "resolved": "https://registry.npmjs.org/type-is/-/type-is-1.6.16.tgz", + "integrity": "sha512-HRkVv/5qY2G6I8iab9cI7v1bOIdhm94dVjQCPFElW9W+3GeDOSHmy2EBYe4VTApuzolPcmgFTN3ftVJRKR2J9Q==", + "dev": true, + "requires": { + "media-typer": "0.3.0", + "mime-types": "~2.1.18" + }, + "dependencies": { + "mime-db": { + "version": "1.37.0", + "resolved": "https://registry.npmjs.org/mime-db/-/mime-db-1.37.0.tgz", + "integrity": "sha512-R3C4db6bgQhlIhPU48fUtdVmKnflq+hRdad7IyKhtFj06VPNVdk2RhiYL3UjQIlso8L+YxAtFkobT0VK+S/ybg==", + "dev": true + }, + "mime-types": { + "version": "2.1.21", + "resolved": "https://registry.npmjs.org/mime-types/-/mime-types-2.1.21.tgz", + "integrity": "sha512-3iL6DbwpyLzjR3xHSFNFeb9Nz/M8WDkX33t1GFQnFOllWk8pOrh/LSrB5OXlnlW5P9LH73X6loW/eogc+F5lJg==", + "dev": true, + "requires": { + "mime-db": "~1.37.0" + } + } + } + }, "typedarray": { "version": "0.0.6", "resolved": "https://registry.npmjs.org/typedarray/-/typedarray-0.0.6.tgz", diff --git a/package.json b/package.json index 40031e7a6335a44becbacea67e5a46cf4f9ea576..3bd2f7d08faf5a3f6165a4d02cda18d3180b0ff8 100644 --- a/package.json +++ b/package.json @@ -114,6 +114,7 @@ "babel-eslint": "^10.0.0", "caller": "^1.0.1", "chai": "^4.1.2", + "chai-datetime": "^1.5.0", "chai-string": "^1.4.0", "chainsmoker": "^0.1.0", "child-process-promise": "^2.2.1", @@ -154,6 +155,7 @@ "next": "^6.1.1", "nock": "^10.0.0", "node-fetch": "^2.3.0", + "node-mocks-http": "^1.7.3", "nyc": "^13.0.1", "opn-cli": "^4.0.0", "prettier": "1.15.3", diff --git a/server.js b/server.js index 132b7495d464c4888cb81f2e0ce1ec25e3395cdc..b554d25215102cd8b5bbc1e77ce0d7c56f2e0b32 100644 --- a/server.js +++ b/server.js @@ -105,6 +105,7 @@ loadServiceClasses().forEach(serviceClass => { camp, handleRequest: cache, githubApiProvider }, { handleInternalErrors: config.handleInternalErrors, + cacheHeaders: config.cacheHeaders, profiling: config.profiling, } ) @@ -113,7 +114,7 @@ loadServiceClasses().forEach(serviceClass => // User defined sources - JSON response camp.route( /^\/badge\/dynamic\/(json|xml|yaml)\.(svg|png|gif|jpg|json)$/, - cache({ + cache(config.cacheHeaders, { queryParams: ['uri', 'url', 'query', 'prefix', 'suffix'], handler: function(query, match, sendBadge, request) { const type = match[1] @@ -226,22 +227,6 @@ camp.route( }) ) -// Production cache debugging. -let bitFlip = false -camp.route(/^\/flip\.svg$/, (data, match, end, ask) => { - const cacheSecs = 60 - ask.res.setHeader('Cache-Control', `max-age=${cacheSecs}`) - const reqTime = new Date() - const date = new Date(+reqTime + cacheSecs * 1000).toGMTString() - ask.res.setHeader('Expires', date) - const badgeData = getBadgeData('flip', data) - bitFlip = !bitFlip - badgeData.text[1] = bitFlip ? 'on' : 'off' - badgeData.colorscheme = bitFlip ? 'brightgreen' : 'red' - const svg = makeBadge(badgeData) - makeSend('svg', ask.res, end)(svg) -}) - // Any badge, old version. This route must be registered last. camp.route(/^\/([^/]+)\/(.+).png$/, (queryParams, match, end, ask) => { const [, label, message] = match diff --git a/server.spec.js b/server.spec.js index 781e8e5011de3a8c04dcf5522792bd8b3db8309f..9320288b3a0fc1d62657969df163f4bcf29319a3 100644 --- a/server.spec.js +++ b/server.spec.js @@ -2,6 +2,7 @@ const { expect } = require('chai') const config = require('./lib/test-config') +const serverConfig = require('./lib/server-config') const fetch = require('node-fetch') const fs = require('fs') const isPng = require('is-png') @@ -33,7 +34,7 @@ describe('The server', function() { // at _combinedTickCallback (internal/process/next_tick.js:141:11) // at process._tickDomainCallback (internal/process/next_tick.js:218:9) loadServiceClasses().forEach(serviceClass => - serviceClass.register({ camp: dummyCamp, handleRequest }, {}) + serviceClass.register({ camp: dummyCamp, handleRequest }, serverConfig) ) dummyCamp.close() dummyCamp = undefined @@ -50,7 +51,7 @@ describe('The server', function() { const baseUri = `http://127.0.0.1:${config.port}` let server - before('Start running the server', function() { + before('Start the server', function() { this.timeout(5000) server = serverHelpers.start() }) diff --git a/services/base-json.spec.js b/services/base-json.spec.js index ae47d85361ef15dc93ca460ad711b45e593890bf..bc8ba92a1081525a4b49b667875d33d2205fd7be 100644 --- a/services/base-json.spec.js +++ b/services/base-json.spec.js @@ -32,7 +32,7 @@ class DummyJsonService extends BaseJsonService { describe('BaseJsonService', function() { describe('Making requests', function() { - let sendAndCacheRequest, serviceInstance + let sendAndCacheRequest beforeEach(function() { sendAndCacheRequest = sinon.stub().returns( Promise.resolve({ @@ -40,25 +40,22 @@ describe('BaseJsonService', function() { res: { statusCode: 200 }, }) ) - serviceInstance = new DummyJsonService( - { sendAndCacheRequest }, - { handleInternalErrors: false } - ) }) it('invokes _sendAndCacheRequest', async function() { - await serviceInstance.invokeHandler({}, {}) + await DummyJsonService.invoke( + { sendAndCacheRequest }, + { handleInternalErrors: false } + ) expect(sendAndCacheRequest).to.have.been.calledOnceWith( 'http://example.com/foo.json', - { - headers: { Accept: 'application/json' }, - } + { headers: { Accept: 'application/json' } } ) }) it('forwards options to _sendAndCacheRequest', async function() { - Object.assign(serviceInstance, { + class WithOptions extends DummyJsonService { async handle() { const { value } = await this._requestJson({ schema: dummySchema, @@ -66,10 +63,13 @@ describe('BaseJsonService', function() { options: { method: 'POST', qs: { queryParam: 123 } }, }) return { message: value } - }, - }) + } + } - await serviceInstance.invokeHandler({}, {}) + await WithOptions.invoke( + { sendAndCacheRequest }, + { handleInternalErrors: false } + ) expect(sendAndCacheRequest).to.have.been.calledOnceWith( 'http://example.com/foo.json', @@ -88,12 +88,12 @@ describe('BaseJsonService', function() { buffer: '{"requiredString": "some-string"}', res: { statusCode: 200 }, }) - const serviceInstance = new DummyJsonService( - { sendAndCacheRequest }, - { handleInternalErrors: false } - ) - const serviceData = await serviceInstance.invokeHandler({}, {}) - expect(serviceData).to.deep.equal({ + expect( + await DummyJsonService.invoke( + { sendAndCacheRequest }, + { handleInternalErrors: false } + ) + ).to.deep.equal({ message: 'some-string', }) }) @@ -103,12 +103,12 @@ describe('BaseJsonService', function() { buffer: '{"unexpectedKey": "some-string"}', res: { statusCode: 200 }, }) - const serviceInstance = new DummyJsonService( - { sendAndCacheRequest }, - { handleInternalErrors: false } - ) - const serviceData = await serviceInstance.invokeHandler({}, {}) - expect(serviceData).to.deep.equal({ + expect( + await DummyJsonService.invoke( + { sendAndCacheRequest }, + { handleInternalErrors: false } + ) + ).to.deep.equal({ color: 'lightgray', message: 'invalid response data', }) @@ -119,12 +119,12 @@ describe('BaseJsonService', function() { buffer: 'not json', res: { statusCode: 200 }, }) - const serviceInstance = new DummyJsonService( - { sendAndCacheRequest }, - { handleInternalErrors: false } - ) - const serviceData = await serviceInstance.invokeHandler({}, {}) - expect(serviceData).to.deep.equal({ + expect( + await DummyJsonService.invoke( + { sendAndCacheRequest }, + { handleInternalErrors: false } + ) + ).to.deep.equal({ color: 'lightgray', message: 'unparseable json response', }) diff --git a/services/base-non-memory-caching.js b/services/base-non-memory-caching.js new file mode 100644 index 0000000000000000000000000000000000000000..225983a2fb051648f0c6805002af1f4ba68b3e3b --- /dev/null +++ b/services/base-non-memory-caching.js @@ -0,0 +1,55 @@ +'use strict' + +const makeBadge = require('../gh-badges/lib/make-badge') +const { makeSend } = require('../lib/result-sender') +const BaseService = require('./base') +const { setCacheHeaders } = require('./cache-headers') + +// Badges are subject to two independent types of caching: in-memory and +// downstream. +// +// Services deriving from `NonMemoryCachingBaseService` are not cached in +// memory on the server. This means that each request that hits the server +// triggers another call to the handler. When using badges for server +// diagnostics, that's useful! +// +// In constrast, The `handle()` function of most other `BaseService` +// subclasses is wrapped in onboard, in-memory caching. See `lib /request- +// handler.js` and `BaseService.prototype.register()`. +// +// All services, including those extending NonMemoryCachingBaseServices, may +// be cached _downstream_. This is governed by cache headers, which are +// configured by the service, the user's request, and the server's default +// cache length. +module.exports = class NonMemoryCachingBaseService extends BaseService { + static register({ camp }, serviceConfig) { + const { cacheHeaders: cacheHeaderConfig } = serviceConfig + const { _cacheLength: serviceCacheLengthSeconds } = this + + camp.route(this._regex, async (queryParams, match, end, ask) => { + const namedParams = this._namedParamsForMatch(match) + const serviceData = await this.invoke( + {}, + serviceConfig, + namedParams, + queryParams + ) + + const badgeData = this._makeBadgeData(queryParams, serviceData) + // The final capture group is the extension. + const format = match.slice(-1)[0] + badgeData.format = format + + const svg = makeBadge(badgeData) + + setCacheHeaders({ + cacheHeaderConfig, + serviceCacheLengthSeconds, + queryParams, + res: ask.res, + }) + + makeSend(format, ask.res, end)(svg) + }) + } +} diff --git a/services/base-static.js b/services/base-static.js index cbaac7ea03b3a661dd0b5cbd282f0659a5d9e8e4..6d146ce21e264da4c579eb3a6f8475fe8c4a3d26 100644 --- a/services/base-static.js +++ b/services/base-static.js @@ -4,53 +4,49 @@ const makeBadge = require('../gh-badges/lib/make-badge') const { makeSend } = require('../lib/result-sender') const analytics = require('../lib/analytics') const BaseService = require('./base') - -const serverStartTime = new Date(new Date().toGMTString()) +const { + serverHasBeenUpSinceResourceCached, + setCacheHeadersForStaticResource, +} = require('./cache-headers') module.exports = class BaseStaticService extends BaseService { - // Note: Since this is a static service, it is not `async`. - handle(namedParams, queryParams) { - throw new Error(`Handler not implemented for ${this.constructor.name}`) - } - static register({ camp }, serviceConfig) { - camp.route(this._regex, (queryParams, match, end, ask) => { + const { + profiling: { makeBadge: shouldProfileMakeBadge }, + } = serviceConfig + + camp.route(this._regex, async (queryParams, match, end, ask) => { analytics.noteRequest(queryParams, match) - if (+new Date(ask.req.headers['if-modified-since']) >= +serverStartTime) { + if (serverHasBeenUpSinceResourceCached(ask.req)) { // Send Not Modified. ask.res.statusCode = 304 ask.res.end() return } - const serviceInstance = new this({}, serviceConfig) const namedParams = this._namedParamsForMatch(match) - let serviceData - try { - // Note: no `await`. - serviceData = serviceInstance.handle(namedParams, queryParams) - } catch (error) { - serviceData = serviceInstance._handleError(error) - } + const serviceData = await this.invoke( + {}, + serviceConfig, + namedParams, + queryParams + ) const badgeData = this._makeBadgeData(queryParams, serviceData) - // The final capture group is the extension. const format = match.slice(-1)[0] badgeData.format = format - if (serviceConfig.profiling.makeBadge) { + if (shouldProfileMakeBadge) { console.time('makeBadge total') } const svg = makeBadge(badgeData) - if (serviceConfig.profiling.makeBadge) { + if (shouldProfileMakeBadge) { console.timeEnd('makeBadge total') } - const cacheDuration = 3600 * 24 * 1 // 1 day. - ask.res.setHeader('Cache-Control', `max-age=${cacheDuration}`) - ask.res.setHeader('Last-Modified', serverStartTime.toGMTString()) + setCacheHeadersForStaticResource(ask.res) makeSend(format, ask.res, end)(svg) }) diff --git a/services/base-svg-scraping.spec.js b/services/base-svg-scraping.spec.js index 550f8b1c0e4f8aeefb014a07dc00ebf4afeb2e1a..cfa37b2bcea82147bf956660d44252b1df0dfcd0 100644 --- a/services/base-svg-scraping.spec.js +++ b/services/base-svg-scraping.spec.js @@ -53,7 +53,7 @@ describe('BaseSvgScrapingService', function() { }) describe('Making requests', function() { - let sendAndCacheRequest, serviceInstance + let sendAndCacheRequest beforeEach(function() { sendAndCacheRequest = sinon.stub().returns( Promise.resolve({ @@ -61,25 +61,22 @@ describe('BaseSvgScrapingService', function() { res: { statusCode: 200 }, }) ) - serviceInstance = new DummySvgScrapingService( - { sendAndCacheRequest }, - { handleInternalErrors: false } - ) }) it('invokes _sendAndCacheRequest with the expected header', async function() { - await serviceInstance.invokeHandler({}, {}) + await DummySvgScrapingService.invoke( + { sendAndCacheRequest }, + { handleInternalErrors: false } + ) expect(sendAndCacheRequest).to.have.been.calledOnceWith( 'http://example.com/foo.svg', - { - headers: { Accept: 'image/svg+xml' }, - } + { headers: { Accept: 'image/svg+xml' } } ) }) it('forwards options to _sendAndCacheRequest', async function() { - Object.assign(serviceInstance, { + class WithCustomOptions extends DummySvgScrapingService { async handle() { const { value } = await this._requestSvg({ schema, @@ -90,10 +87,13 @@ describe('BaseSvgScrapingService', function() { }, }) return { message: value } - }, - }) + } + } - await serviceInstance.invokeHandler({}, {}) + await WithCustomOptions.invoke( + { sendAndCacheRequest }, + { handleInternalErrors: false } + ) expect(sendAndCacheRequest).to.have.been.calledOnceWith( 'http://example.com/foo.svg', @@ -112,36 +112,36 @@ describe('BaseSvgScrapingService', function() { buffer: exampleSvg, res: { statusCode: 200 }, }) - const serviceInstance = new DummySvgScrapingService( - { sendAndCacheRequest }, - { handleInternalErrors: false } - ) - const serviceData = await serviceInstance.invokeHandler({}, {}) - expect(serviceData).to.deep.equal({ + expect( + await DummySvgScrapingService.invoke( + { sendAndCacheRequest }, + { handleInternalErrors: false } + ) + ).to.deep.equal({ message: exampleMessage, }) }) it('allows overriding the valueMatcher', async function() { - const sendAndCacheRequest = async () => ({ - buffer: '<desc>a different message</desc>', - res: { statusCode: 200 }, - }) - const serviceInstance = new DummySvgScrapingService( - { sendAndCacheRequest }, - { handleInternalErrors: false } - ) - Object.assign(serviceInstance, { + class WithValueMatcher extends BaseSvgScrapingService { async handle() { return this._requestSvg({ schema, valueMatcher: />([^<>]+)<\/desc>/, url: 'http://example.com/foo.svg', }) - }, + } + } + const sendAndCacheRequest = async () => ({ + buffer: '<desc>a different message</desc>', + res: { statusCode: 200 }, }) - const serviceData = await serviceInstance.invokeHandler({}, {}) - expect(serviceData).to.deep.equal({ + expect( + await WithValueMatcher.invoke( + { sendAndCacheRequest }, + { handleInternalErrors: false } + ) + ).to.deep.equal({ message: 'a different message', }) }) @@ -151,12 +151,12 @@ describe('BaseSvgScrapingService', function() { buffer: 'not svg yo', res: { statusCode: 200 }, }) - const serviceInstance = new DummySvgScrapingService( - { sendAndCacheRequest }, - { handleInternalErrors: false } - ) - const serviceData = await serviceInstance.invokeHandler({}, {}) - expect(serviceData).to.deep.equal({ + expect( + await DummySvgScrapingService.invoke( + { sendAndCacheRequest }, + { handleInternalErrors: false } + ) + ).to.deep.equal({ color: 'lightgray', message: 'unparseable svg response', }) diff --git a/services/base-xml.spec.js b/services/base-xml.spec.js index 801cb5885c9d90f381e9ad6590bfb818ac3c96d9..5c3ad696abc5de5f33672f3755526171d3fce59d 100644 --- a/services/base-xml.spec.js +++ b/services/base-xml.spec.js @@ -32,7 +32,7 @@ class DummyXmlService extends BaseXmlService { describe('BaseXmlService', function() { describe('Making requests', function() { - let sendAndCacheRequest, serviceInstance + let sendAndCacheRequest beforeEach(function() { sendAndCacheRequest = sinon.stub().returns( Promise.resolve({ @@ -40,25 +40,22 @@ describe('BaseXmlService', function() { res: { statusCode: 200 }, }) ) - serviceInstance = new DummyXmlService( - { sendAndCacheRequest }, - { handleInternalErrors: false } - ) }) it('invokes _sendAndCacheRequest', async function() { - await serviceInstance.invokeHandler({}, {}) + await DummyXmlService.invoke( + { sendAndCacheRequest }, + { handleInternalErrors: false } + ) expect(sendAndCacheRequest).to.have.been.calledOnceWith( 'http://example.com/foo.xml', - { - headers: { Accept: 'application/xml, text/xml' }, - } + { headers: { Accept: 'application/xml, text/xml' } } ) }) it('forwards options to _sendAndCacheRequest', async function() { - Object.assign(serviceInstance, { + class WithCustomOptions extends BaseXmlService { async handle() { const { value } = await this._requestXml({ schema: dummySchema, @@ -66,10 +63,13 @@ describe('BaseXmlService', function() { options: { method: 'POST', qs: { queryParam: 123 } }, }) return { message: value } - }, - }) + } + } - await serviceInstance.invokeHandler({}, {}) + await WithCustomOptions.invoke( + { sendAndCacheRequest }, + { handleInternalErrors: false } + ) expect(sendAndCacheRequest).to.have.been.calledOnceWith( 'http://example.com/foo.xml', @@ -88,12 +88,12 @@ describe('BaseXmlService', function() { buffer: '<requiredString>some-string</requiredString>', res: { statusCode: 200 }, }) - const serviceInstance = new DummyXmlService( - { sendAndCacheRequest }, - { handleInternalErrors: false } - ) - const serviceData = await serviceInstance.invokeHandler({}, {}) - expect(serviceData).to.deep.equal({ + expect( + await DummyXmlService.invoke( + { sendAndCacheRequest }, + { handleInternalErrors: false } + ) + ).to.deep.equal({ message: 'some-string', }) }) @@ -115,14 +115,12 @@ describe('BaseXmlService', function() { '<requiredString>some-string with trailing whitespace </requiredString>', res: { statusCode: 200 }, }) - const serviceInstance = new DummyXmlServiceWithParserOption( - { sendAndCacheRequest }, - { handleInternalErrors: false } - ) - - const serviceData = await serviceInstance.invokeHandler({}, {}) - - expect(serviceData).to.deep.equal({ + expect( + await DummyXmlServiceWithParserOption.invoke( + { sendAndCacheRequest }, + { handleInternalErrors: false } + ) + ).to.deep.equal({ message: 'some-string with trailing whitespace ', }) }) @@ -132,12 +130,12 @@ describe('BaseXmlService', function() { buffer: '<unexpectedAttribute>some-string</unexpectedAttribute>', res: { statusCode: 200 }, }) - const serviceInstance = new DummyXmlService( - { sendAndCacheRequest }, - { handleInternalErrors: false } - ) - const serviceData = await serviceInstance.invokeHandler({}, {}) - expect(serviceData).to.deep.equal({ + expect( + await DummyXmlService.invoke( + { sendAndCacheRequest }, + { handleInternalErrors: false } + ) + ).to.deep.equal({ color: 'lightgray', message: 'invalid response data', }) @@ -148,12 +146,12 @@ describe('BaseXmlService', function() { buffer: 'not xml', res: { statusCode: 200 }, }) - const serviceInstance = new DummyXmlService( - { sendAndCacheRequest }, - { handleInternalErrors: false } - ) - const serviceData = await serviceInstance.invokeHandler({}, {}) - expect(serviceData).to.deep.equal({ + expect( + await DummyXmlService.invoke( + { sendAndCacheRequest }, + { handleInternalErrors: false } + ) + ).to.deep.equal({ color: 'lightgray', message: 'unparseable xml response', }) diff --git a/services/base.js b/services/base.js index f1279c06b948b6d7832c3aecca855b0c7b2a5cda..13b13ab117af9cc90bbabd907ccb3ffeb0a999b5 100644 --- a/services/base.js +++ b/services/base.js @@ -11,6 +11,7 @@ const { InvalidParameter, Deprecated, } = require('./errors') +const coalesce = require('../lib/coalesce') const validate = require('../lib/validate') const { checkErrorResponse } = require('../lib/error-helper') const { @@ -23,10 +24,6 @@ const { staticBadgeUrl } = require('../lib/make-badge-url') const trace = require('./trace') const validateExample = require('./validate-example') -function coalesce(...candidates) { - return candidates.find(c => c !== undefined) -} - class BaseService { constructor({ sendAndCacheRequest }, { handleInternalErrors }) { this._requestFetcher = sendAndCacheRequest @@ -249,6 +246,7 @@ class BaseService { build: 30, license: 3600, version: 300, + debug: 60, } return cacheLengths[this.category] } @@ -321,20 +319,28 @@ class BaseService { } } - async invokeHandler(namedParams, queryParams) { - trace.logTrace( - 'inbound', - emojic.womanCook, - 'Service class', - this.constructor.name - ) + static async invoke( + context = {}, + config = {}, + namedParams = {}, + queryParams = {} + ) { + trace.logTrace('inbound', emojic.womanCook, 'Service class', this.name) trace.logTrace('inbound', emojic.ticket, 'Named params', namedParams) trace.logTrace('inbound', emojic.crayon, 'Query params', queryParams) + + const serviceInstance = new this(context, config) + + let serviceData try { - return await this.handle(namedParams, queryParams) + serviceData = await serviceInstance.handle(namedParams, queryParams) } catch (error) { - return this._handleError(error) + serviceData = serviceInstance._handleError(error) } + + trace.logTrace('outbound', emojic.shield, 'Service data', serviceData) + + return serviceData } static _makeBadgeData(overrides, serviceData) { @@ -385,27 +391,25 @@ class BaseService { } static register({ camp, handleRequest, githubApiProvider }, serviceConfig) { + const { cacheHeaders: cacheHeaderConfig } = serviceConfig camp.route( this._regex, - handleRequest({ + handleRequest(cacheHeaderConfig, { queryParams: this.route.queryParams, handler: async (queryParams, match, sendBadge, request) => { - const serviceInstance = new this( + const namedParams = this._namedParamsForMatch(match) + const serviceData = await this.invoke( { sendAndCacheRequest: request.asPromise, sendAndCacheRequestWithCallbacks: request, githubApiProvider, }, - serviceConfig - ) - const namedParams = this._namedParamsForMatch(match) - const serviceData = await serviceInstance.invokeHandler( + serviceConfig, namedParams, queryParams ) - trace.logTrace('outbound', emojic.shield, 'Service data', serviceData) - const badgeData = this._makeBadgeData(queryParams, serviceData) + const badgeData = this._makeBadgeData(queryParams, serviceData) // The final capture group is the extension. const format = match.slice(-1)[0] sendBadge(format, badgeData) diff --git a/services/base.spec.js b/services/base.spec.js index 3dbaf798356602cb27e3d3e59dd241c2c7d6fa06..34fdbca2757e1f005efc64cebd79c08fe69f1ba5 100644 --- a/services/base.spec.js +++ b/services/base.spec.js @@ -198,12 +198,14 @@ describe('BaseService', function() { }) it('Invokes the handler as expected', async function() { - const serviceInstance = new DummyService({}, defaultConfig) - const serviceData = await serviceInstance.invokeHandler( - { namedParamA: 'bar.bar.bar' }, - { queryParamA: '!' } - ) - expect(serviceData).to.deep.equal({ + expect( + await DummyService.invoke( + {}, + defaultConfig, + { namedParamA: 'bar.bar.bar' }, + { queryParamA: '!' } + ) + ).to.deep.equal({ message: 'Hello namedParamA: bar.bar.bar with queryParamA: !', }) }) @@ -220,11 +222,10 @@ describe('BaseService', function() { sandbox.stub(trace, 'logTrace') }) it('Invokes the logger as expected', async function() { - const serviceInstance = new DummyService({}, defaultConfig) - await serviceInstance.invokeHandler( - { - namedParamA: 'bar.bar.bar', - }, + await DummyService.invoke( + {}, + defaultConfig, + { namedParamA: 'bar.bar.bar' }, { queryParamA: '!' } ) expect(trace.logTrace).to.be.calledWithMatch( @@ -237,9 +238,7 @@ describe('BaseService', function() { 'inbound', sinon.match.string, 'Named params', - { - namedParamA: 'bar.bar.bar', - } + { namedParamA: 'bar.bar.bar' } ) expect(trace.logTrace).to.be.calledWith( 'inbound', @@ -252,17 +251,17 @@ describe('BaseService', function() { describe('Error handling', function() { it('Handles internal errors', async function() { - const serviceInstance = new DummyService( - {}, - { handleInternalErrors: true } - ) - serviceInstance.handle = () => { - throw Error("I've made a huge mistake") + class ThrowingService extends DummyService { + async handle() { + throw Error("I've made a huge mistake") + } } expect( - await serviceInstance.invokeHandler({ - namedParamA: 'bar.bar.bar', - }) + await ThrowingService.invoke( + {}, + { handleInternalErrors: true }, + { namedParamA: 'bar.bar.bar' } + ) ).to.deep.equal({ color: 'lightgray', label: 'shields', @@ -271,19 +270,14 @@ describe('BaseService', function() { }) describe('Handles known subtypes of ShieldsInternalError', function() { - let serviceInstance - beforeEach(function() { - serviceInstance = new DummyService({}, {}) - }) - it('handles NotFound errors', async function() { - serviceInstance.handle = () => { - throw new NotFound() + class ThrowingService extends DummyService { + async handle() { + throw new NotFound() + } } expect( - await serviceInstance.invokeHandler({ - namedParamA: 'bar.bar.bar', - }) + await ThrowingService.invoke({}, {}, { namedParamA: 'bar.bar.bar' }) ).to.deep.equal({ color: 'red', message: 'not found', @@ -291,13 +285,13 @@ describe('BaseService', function() { }) it('handles Inaccessible errors', async function() { - serviceInstance.handle = () => { - throw new Inaccessible() + class ThrowingService extends DummyService { + async handle() { + throw new Inaccessible() + } } expect( - await serviceInstance.invokeHandler({ - namedParamA: 'bar.bar.bar', - }) + await ThrowingService.invoke({}, {}, { namedParamA: 'bar.bar.bar' }) ).to.deep.equal({ color: 'lightgray', message: 'inaccessible', @@ -305,13 +299,13 @@ describe('BaseService', function() { }) it('handles InvalidResponse errors', async function() { - serviceInstance.handle = () => { - throw new InvalidResponse() + class ThrowingService extends DummyService { + async handle() { + throw new InvalidResponse() + } } expect( - await serviceInstance.invokeHandler({ - namedParamA: 'bar.bar.bar', - }) + await ThrowingService.invoke({}, {}, { namedParamA: 'bar.bar.bar' }) ).to.deep.equal({ color: 'lightgray', message: 'invalid', @@ -319,13 +313,13 @@ describe('BaseService', function() { }) it('handles Deprecated', async function() { - serviceInstance.handle = () => { - throw new Deprecated() + class ThrowingService extends DummyService { + async handle() { + throw new Deprecated() + } } expect( - await serviceInstance.invokeHandler({ - namedParamA: 'bar.bar.bar', - }) + await ThrowingService.invoke({}, {}, { namedParamA: 'bar.bar.bar' }) ).to.deep.equal({ color: 'lightgray', message: 'no longer available', @@ -333,13 +327,13 @@ describe('BaseService', function() { }) it('handles InvalidParameter errors', async function() { - serviceInstance.handle = () => { - throw new InvalidParameter() + class ThrowingService extends DummyService { + async handle() { + throw new InvalidParameter() + } } expect( - await serviceInstance.invokeHandler({ - namedParamA: 'bar.bar.bar', - }) + await ThrowingService.invoke({}, {}, { namedParamA: 'bar.bar.bar' }) ).to.deep.equal({ color: 'red', message: 'invalid parameter', @@ -431,7 +425,7 @@ describe('BaseService', function() { it('handles the request', async function() { expect(mockHandleRequest).to.have.been.calledOnce - const { handler: requestHandler } = mockHandleRequest.getCall(0).args[0] + const { handler: requestHandler } = mockHandleRequest.getCall(0).args[1] const mockSendBadge = sinon.spy() const mockRequest = { diff --git a/services/cache-headers.js b/services/cache-headers.js new file mode 100644 index 0000000000000000000000000000000000000000..baca92da875c18444e9091fe966335fcdea4c8a3 --- /dev/null +++ b/services/cache-headers.js @@ -0,0 +1,99 @@ +'use strict' + +const assert = require('assert') +const Joi = require('joi') +const coalesce = require('../lib/coalesce') + +const serverStartTimeGMTString = new Date().toGMTString() +const serverStartTimestamp = Date.now() + +const queryParamSchema = Joi.object({ + // Not using nonNegativeInteger because it's not required. + maxAge: Joi.number() + .integer() + .min(0), +}).required() + +function coalesceCacheLength( + cacheHeaderConfig, + serviceCacheLengthSeconds, + queryParams +) { + const { defaultCacheLengthSeconds } = cacheHeaderConfig + // The config returns a number so this would only fail if we break the + // wiring. Better to fail obviously than silently. + assert(defaultCacheLengthSeconds !== undefined) + + const ourCacheLength = coalesce( + serviceCacheLengthSeconds, + defaultCacheLengthSeconds + ) + + const { value: { maxAge: overrideCacheLength } = {}, error } = Joi.validate( + queryParams, + queryParamSchema, + { allowUnknown: true } + ) + + if (!error && overrideCacheLength !== undefined) { + // The user can request _more_ caching, but not less. + return Math.max(overrideCacheLength, ourCacheLength) + } else { + return ourCacheLength + } +} + +function setHeadersForCacheLength(res, cacheLengthSeconds) { + const now = new Date() + const nowGMTString = now.toGMTString() + + // Send both Cache-Control max-age and Expires in case the client implements + // HTTP/1.0 but not HTTP/1.1. + let cacheControl, expires + if (cacheLengthSeconds === 0) { + // Prevent as much downstream caching as possible. + cacheControl = 'no-cache, no-store, must-revalidate' + expires = nowGMTString + } else { + cacheControl = `max-age=${cacheLengthSeconds}` + expires = new Date(now.getTime() + cacheLengthSeconds * 1000).toGMTString() + } + + res.setHeader('Date', nowGMTString) + res.setHeader('Cache-Control', cacheControl) + res.setHeader('Expires', expires) +} + +function setCacheHeaders({ + cacheHeaderConfig, + serviceCacheLengthSeconds, + queryParams, + res, +}) { + const cacheLengthSeconds = coalesceCacheLength( + cacheHeaderConfig, + serviceCacheLengthSeconds, + queryParams + ) + setHeadersForCacheLength(res, cacheLengthSeconds) +} + +const staticCacheControlHeader = `max-age=${24 * 3600}` // 1 day. +function setCacheHeadersForStaticResource(res) { + res.setHeader('Cache-Control', staticCacheControlHeader) + res.setHeader('Last-Modified', serverStartTimeGMTString) +} + +function serverHasBeenUpSinceResourceCached(req) { + return ( + serverStartTimestamp <= new Date(req.headers['if-modified-since']).getTime() + ) +} + +module.exports = { + coalesceCacheLength, + setCacheHeaders, + setHeadersForCacheLength, + setCacheHeadersForStaticResource, + serverHasBeenUpSinceResourceCached, +} diff --git a/services/cache-headers.spec.js b/services/cache-headers.spec.js new file mode 100644 index 0000000000000000000000000000000000000000..2a3562ffdca2c230ce74ff3c5a9d7e1b38404cc8 --- /dev/null +++ b/services/cache-headers.spec.js @@ -0,0 +1,173 @@ +'use strict' + +const { test, given } = require('sazerac') +const chai = require('chai') +const { expect } = require('chai') +const sinon = require('sinon') +const httpMocks = require('node-mocks-http') +const { + coalesceCacheLength, + setHeadersForCacheLength, + setCacheHeaders, + setCacheHeadersForStaticResource, + serverHasBeenUpSinceResourceCached, +} = require('./cache-headers') + +chai.use(require('chai-datetime')) + +describe('Cache header functions', function() { + let res + beforeEach(function() { + res = httpMocks.createResponse() + }) + + describe('coalesceCacheLength', function() { + const exampleCacheConfig = { defaultCacheLengthSeconds: 777 } + test(coalesceCacheLength, () => { + given(exampleCacheConfig, undefined, {}).expect(777) + given(exampleCacheConfig, 900, {}).expect(900) + given(exampleCacheConfig, 900, { maxAge: 1000 }).expect(1000) + given(exampleCacheConfig, 900, { maxAge: 400 }).expect(900) + given(exampleCacheConfig, 900, { maxAge: '-1000' }).expect(900) + given(exampleCacheConfig, 900, { maxAge: '' }).expect(900) + given(exampleCacheConfig, 900, { maxAge: 'not a number' }).expect(900) + }) + }) + + describe('setHeadersForCacheLength', function() { + let sandbox + beforeEach(function() { + sandbox = sinon.createSandbox() + sandbox.useFakeTimers() + }) + afterEach(function() { + sandbox.restore() + sandbox = undefined + }) + + it('should set the correct Date header', function() { + // Confidence check. + expect(res._headers.date).to.equal(undefined) + + // Act. + setHeadersForCacheLength(res, 123) + + // Assert. + const now = new Date().toGMTString() + expect(res._headers.date).to.equal(now) + }) + + context('cacheLengthSeconds is zero', function() { + beforeEach(function() { + setHeadersForCacheLength(res, 0) + }) + + it('should set the expected Cache-Control header', function() { + expect(res._headers['cache-control']).to.equal( + 'no-cache, no-store, must-revalidate' + ) + }) + + it('should set the expected Expires header', function() { + expect(res._headers.expires).to.equal(new Date().toGMTString()) + }) + }) + + context('cacheLengthSeconds is nonzero', function() { + beforeEach(function() { + setHeadersForCacheLength(res, 123) + }) + + it('should set the expected Cache-Control header', function() { + expect(res._headers['cache-control']).to.equal('max-age=123') + }) + + it('should set the expected Expires header', function() { + const expires = new Date(Date.now() + 123 * 1000).toGMTString() + expect(res._headers.expires).to.equal(expires) + }) + }) + }) + + describe('setCacheHeaders', function() { + it('sets the expected fields', function() { + const expectedFields = ['date', 'cache-control', 'expires'] + expectedFields.forEach(field => + expect(res._headers[field]).to.equal(undefined) + ) + + setCacheHeaders({ + cacheHeaderConfig: { defaultCacheLengthSeconds: 1234 }, + serviceCacheLengthSeconds: 567, + queryParams: { maxAge: 999999 }, + res, + }) + + expectedFields.forEach(field => + expect(res._headers[field]) + .to.be.a('string') + .and.have.lengthOf.at.least(1) + ) + }) + }) + + describe('setCacheHeadersForStaticResource', function() { + beforeEach(function() { + setCacheHeadersForStaticResource(res) + }) + + it('should set the expected Cache-Control header', function() { + expect(res._headers['cache-control']).to.equal(`max-age=${24 * 3600}`) + }) + + it('should set the expected Last-Modified header', function() { + const lastModified = res._headers['last-modified'] + expect(new Date(lastModified)).to.be.withinTime( + // Within the last 60 seconds. + new Date(Date.now() - 60 * 1000), + new Date() + ) + }) + }) + + describe('serverHasBeenUpSinceResourceCached', function() { + // The stringified req's are hard to understand. I thought Sazerac + // provided a way to override the describe message, though I can't find it. + context('when there is no If-Modified-Since header', function() { + it('returns false', function() { + const req = httpMocks.createRequest() + expect(serverHasBeenUpSinceResourceCached(req)).to.equal(false) + }) + }) + context('when the If-Modified-Since header is invalid', function() { + it('returns false', function() { + const req = httpMocks.createRequest({ + headers: { 'If-Modified-Since': 'this-is-not-a-date' }, + }) + expect(serverHasBeenUpSinceResourceCached(req)).to.equal(false) + }) + }) + context( + 'when the If-Modified-Since header is before the process started', + function() { + it('returns false', function() { + const req = httpMocks.createRequest({ + headers: { 'If-Modified-Since': '2018-02-01T05:00:00.000Z' }, + }) + expect(serverHasBeenUpSinceResourceCached(req)).to.equal(false) + }) + } + ) + context( + 'when the If-Modified-Since header is after the process started', + function() { + it('returns true', function() { + const req = httpMocks.createRequest({ + headers: { 'If-Modified-Since': new Date().toGMTString() }, + }) + expect(serverHasBeenUpSinceResourceCached(req)).to.equal(true) + }) + } + ) + }) +}) diff --git a/services/flip/flip.service.js b/services/flip/flip.service.js new file mode 100644 index 0000000000000000000000000000000000000000..07bcb963cf6f47c7dadfa41850047e598a22c3bd --- /dev/null +++ b/services/flip/flip.service.js @@ -0,0 +1,33 @@ +'use strict' + +const NonMemoryCachingBaseService = require('../base-non-memory-caching') + +let bitFlip = false + +// Production cache debugging. +module.exports = class Flip extends NonMemoryCachingBaseService { + static get category() { + return 'debug' + } + + static get route() { + return { base: 'flip', pattern: '' } + } + + static get defaultBadgeData() { + return { label: 'flip' } + } + + static render({ bit }) { + if (bit) { + return { message: 'on', color: 'brightgreen' } + } else { + return { message: 'off', color: 'red' } + } + } + + handle() { + bitFlip = !bitFlip + return this.constructor.render({ bit: bitFlip }) + } +} diff --git a/services/flip/flip.tester.js b/services/flip/flip.tester.js new file mode 100644 index 0000000000000000000000000000000000000000..49250b1c2e0dcd9b297a7dc23ff1cfb668f350ef --- /dev/null +++ b/services/flip/flip.tester.js @@ -0,0 +1,12 @@ +'use strict' + +const t = require('../create-service-tester')() +module.exports = t + +t.create('First request') + .get('.json') + .expectJSON({ name: 'flip', value: 'on' }) + +t.create('Second request') + .get('.json') + .expectJSON({ name: 'flip', value: 'off' }) diff --git a/services/legacy-service.js b/services/legacy-service.js index 9dbcddda8e7e10bb45be95e7fe5383e7da2b390c..ebf6e3514e7fd12d0f5befeb557d52d0ac7dfebb 100644 --- a/services/legacy-service.js +++ b/services/legacy-service.js @@ -8,13 +8,11 @@ class LegacyService extends BaseService { throw Error(`registerLegacyRouteHandler() not implemented for ${this.name}`) } - static register( - { camp, handleRequest: cache, githubApiProvider }, - serviceConfig - ) { + static register({ camp, handleRequest, githubApiProvider }, serviceConfig) { + const { cache: cacheHeaderConfig } = serviceConfig this.registerLegacyRouteHandler({ camp, - cache, + cache: (...args) => handleRequest(cacheHeaderConfig, ...args), githubApiProvider, }) } diff --git a/services/time/time.service.js b/services/time/time.service.js index e3c3e858e5d1e2de11d7dea24a23ac0921e7a0ef..7638b98d37678b25b119deee580374bd4231ee05 100644 --- a/services/time/time.service.js +++ b/services/time/time.service.js @@ -1,8 +1,8 @@ 'use strict' -const BaseService = require('../base') +const NonMemoryCachingBaseService = require('../base-non-memory-caching') -module.exports = class Time extends BaseService { +module.exports = class Time extends NonMemoryCachingBaseService { async handle() { return { message: new Date() } }