diff --git a/lib/cache/memory-cache-store.js b/lib/cache/memory-cache-store.js index 6fa7356b366..4640092b5f5 100644 --- a/lib/cache/memory-cache-store.js +++ b/lib/cache/memory-cache-store.js @@ -76,17 +76,9 @@ class MemoryCacheStore { const topLevelKey = `${key.origin}:${key.path}` const now = Date.now() - const entry = this.#entries.get(topLevelKey)?.find((entry) => ( - entry.deleteAt > now && - entry.method === key.method && - (entry.vary == null || Object.keys(entry.vary).every(headerName => { - if (entry.vary[headerName] === null) { - return key.headers[headerName] === undefined - } + const entries = this.#entries.get(topLevelKey) - return entry.vary[headerName] === key.headers[headerName] - })) - )) + const entry = entries ? findEntry(key, entries, now) : null return entry == null ? undefined @@ -140,10 +132,17 @@ class MemoryCacheStore { entries = [] store.#entries.set(topLevelKey, entries) } - entries.push(entry) + const previousEntry = findEntry(key, entries, Date.now()) + if (previousEntry) { + const index = entries.indexOf(previousEntry) + entries.splice(index, 1, entry) + store.#size -= previousEntry.size + } else { + entries.push(entry) + store.#count += 1 + } store.#size += entry.size - store.#count += 1 if (store.#size > store.#maxSize || store.#count > store.#maxCount) { for (const [key, entries] of store.#entries) { @@ -180,4 +179,18 @@ class MemoryCacheStore { } } +function findEntry (key, entries, now) { + return entries.find((entry) => ( + entry.deleteAt > now && + entry.method === key.method && + (entry.vary == null || Object.keys(entry.vary).every(headerName => { + if (entry.vary[headerName] === null) { + return key.headers[headerName] === undefined + } + + return entry.vary[headerName] === key.headers[headerName] + })) + )) +} + module.exports = MemoryCacheStore diff --git a/test/cache-interceptor/cache-store-test-utils.js b/test/cache-interceptor/cache-store-test-utils.js index c956902ab81..b97fe01f8d5 100644 --- a/test/cache-interceptor/cache-store-test-utils.js +++ b/test/cache-interceptor/cache-store-test-utils.js @@ -164,6 +164,87 @@ function cacheStoreTests (CacheStore) { equal(await store.get(key), undefined) }) + test('a stale request is overwritten', async () => { + const clock = FakeTimers.install({ + shouldClearNativeTimers: true + }) + + after(() => clock.uninstall()) + + /** + * @type {import('../../types/cache-interceptor.d.ts').default.CacheKey} + */ + const key = { + origin: 'localhost', + path: '/', + method: 'GET', + headers: {} + } + + /** + * @type {import('../../types/cache-interceptor.d.ts').default.CacheValue} + */ + const value = { + statusCode: 200, + statusMessage: '', + headers: { foo: 'bar' }, + cacheControlDirectives: {}, + cachedAt: Date.now(), + staleAt: Date.now() + 1000, + // deleteAt is different because stale-while-revalidate, stale-if-error, ... + deleteAt: Date.now() + 5000 + } + + const body = [Buffer.from('asd'), Buffer.from('123')] + + const store = new CacheStore() + + // Sanity check + equal(store.get(key), undefined) + + { + const writable = store.createWriteStream(key, value) + notEqual(writable, undefined) + writeBody(writable, body) + } + + clock.tick(1500) + + { + const result = await store.get(structuredClone(key)) + notEqual(result, undefined) + await compareGetResults(result, value, body) + } + + /** + * @type {import('../../types/cache-interceptor.d.ts').default.CacheValue} + */ + const value2 = { + statusCode: 200, + statusMessage: '', + headers: { foo: 'baz' }, + cacheControlDirectives: {}, + cachedAt: Date.now(), + staleAt: Date.now() + 1000, + // deleteAt is different because stale-while-revalidate, stale-if-error, ... + deleteAt: Date.now() + 5000 + } + + const body2 = [Buffer.from('foo'), Buffer.from('123')] + + { + const writable = store.createWriteStream(key, value2) + notEqual(writable, undefined) + writeBody(writable, body2) + } + + { + const result = await store.get(structuredClone(key)) + notEqual(result, undefined) + await compareGetResults(result, value2, body2) + } + }) + test('vary directives used to decide which response to use', async () => { /** * @type {import('../../types/cache-interceptor.d.ts').default.CacheKey} diff --git a/test/cache-interceptor/cache-tests-worker.mjs b/test/cache-interceptor/cache-tests-worker.mjs index 285c33f34a8..b9b3308ecbf 100644 --- a/test/cache-interceptor/cache-tests-worker.mjs +++ b/test/cache-interceptor/cache-tests-worker.mjs @@ -50,8 +50,10 @@ if (environment.ignoredTests) { const client = new Agent().compose(interceptors.cache(environment.opts)) setGlobalDispatcher(client) +globalThis.fetch = fetch + // Run the suite -await runTestSuite(tests, fetch, true, process.env.BASE_URL) +await runTestSuite(tests, true, process.env.BASE_URL) let exitCode = 0 @@ -207,6 +209,10 @@ function printStats (stats) { retried } = stats + if (total < 0) { + throw new Error('Total tests cannot be negative') + } + console.log(`\n Total tests: ${total}`) console.log(` ${styleText('gray', 'Skipped')}: ${skipped} (${((skipped / total) * 100).toFixed(1)}%)`) console.log(` ${styleText('green', 'Passed')}: ${passed} (${((passed / total) * 100).toFixed(1)}%)`) diff --git a/test/cache-interceptor/sqlite-cache-store-tests.js b/test/cache-interceptor/sqlite-cache-store-tests.js index c1f49d3a4c3..43573714d1b 100644 --- a/test/cache-interceptor/sqlite-cache-store-tests.js +++ b/test/cache-interceptor/sqlite-cache-store-tests.js @@ -38,6 +38,8 @@ test('SqliteCacheStore works nicely with multiple stores', async (t) => { }) t.after(async () => { + storeA.close() + storeB.close() await rm(sqliteLocation) }) diff --git a/test/interceptors/cache.js b/test/interceptors/cache.js index b458bff29bd..e21634a5754 100644 --- a/test/interceptors/cache.js +++ b/test/interceptors/cache.js @@ -167,6 +167,225 @@ describe('Cache Interceptor', () => { strictEqual(requestCount, 2) }) + test('expires caching', async () => { + const clock = FakeTimers.install({ + shouldClearNativeTimers: true + }) + + let requestsToOrigin = 0 + let serverError + const server = createServer({ joinDuplicateHeaders: true }, (req, res) => { + const now = new Date() + now.setSeconds(now.getSeconds() + 1) + res.setHeader('date', 0) + res.setHeader('expires', now.toGMTString()) + requestsToOrigin++ + res.end('asd') + }).listen(0) + + const client = new Client(`http://localhost:${server.address().port}`) + .compose(interceptors.cache()) + + after(async () => { + server.close() + await client.close() + clock.uninstall() + }) + + await once(server, 'listening') + + strictEqual(requestsToOrigin, 0) + + /** + * @type {import('../../types/dispatcher').default.RequestOptions} + */ + const request = { + origin: 'localhost', + method: 'GET', + path: '/' + } + + // Send initial request. This should reach the origin + { + const res = await client.request(request) + if (serverError) { + throw serverError + } + + equal(requestsToOrigin, 1) + strictEqual(await res.body.text(), 'asd') + } + + // This is cached + { + const res = await client.request(request) + if (serverError) { + throw serverError + } + + equal(requestsToOrigin, 1) + strictEqual(await res.body.text(), 'asd') + } + + clock.tick(1500) + + // Response is now stale, the origin should get a request + { + const res = await client.request(request) + equal(requestsToOrigin, 2) + strictEqual(await res.body.text(), 'asd') + } + + // Response is now cached, the origin should not get a request + { + const res = await client.request(request) + equal(requestsToOrigin, 2) + strictEqual(await res.body.text(), 'asd') + } + }) + + test('expires caching with Etag', async () => { + const clock = FakeTimers.install({ + shouldClearNativeTimers: true + }) + + let requestsToOrigin = 0 + let serverError + const server = createServer({ joinDuplicateHeaders: true }, (req, res) => { + const now = new Date() + now.setSeconds(now.getSeconds() + 1) + res.setHeader('date', 0) + res.setHeader('expires', now.toGMTString()) + res.setHeader('etag', 'asd123') + requestsToOrigin++ + res.end('asd') + }).listen(0) + + const client = new Client(`http://localhost:${server.address().port}`) + .compose(interceptors.cache()) + + after(async () => { + server.close() + await client.close() + clock.uninstall() + }) + + await once(server, 'listening') + + strictEqual(requestsToOrigin, 0) + + /** + * @type {import('../../types/dispatcher').default.RequestOptions} + */ + const request = { + origin: 'localhost', + method: 'GET', + path: '/' + } + + // Send initial request. This should reach the origin + { + const res = await client.request(request) + if (serverError) { + throw serverError + } + + equal(requestsToOrigin, 1) + strictEqual(await res.body.text(), 'asd') + } + + // This is cached + { + const res = await client.request(request) + if (serverError) { + throw serverError + } + + equal(requestsToOrigin, 1) + strictEqual(await res.body.text(), 'asd') + } + + clock.tick(1500) + + // Response is now stale, the origin should get a request + { + const res = await client.request(request) + equal(requestsToOrigin, 2) + strictEqual(await res.body.text(), 'asd') + } + + // Response is now cached, the origin should not get a request + { + const res = await client.request(request) + equal(requestsToOrigin, 2) + strictEqual(await res.body.text(), 'asd') + } + }) + + test.only('max-age caching', async () => { + const clock = FakeTimers.install({ + shouldClearNativeTimers: true + }) + + let requestsToOrigin = 0 + let serverError + const server = createServer({ joinDuplicateHeaders: true }, (req, res) => { + res.setHeader('date', 0) + res.setHeader('cache-control', 's-maxage=1') + requestsToOrigin++ + res.end('asd') + }).listen(0) + + const client = new Client(`http://localhost:${server.address().port}`) + .compose(interceptors.cache()) + + after(async () => { + server.close() + await client.close() + clock.uninstall() + }) + + await once(server, 'listening') + + strictEqual(requestsToOrigin, 0) + + /** + * @type {import('../../types/dispatcher').default.RequestOptions} + */ + const request = { + origin: 'localhost', + method: 'GET', + path: '/' + } + + // Send initial request. This should reach the origin + { + const res = await client.request(request) + if (serverError) { + throw serverError + } + + equal(requestsToOrigin, 1) + strictEqual(await res.body.text(), 'asd') + } + + clock.tick(1500) + + // Response is now stale, the origin should get a request + { + const res = await client.request(request) + equal(requestsToOrigin, 2) + strictEqual(await res.body.text(), 'asd') + } + + // Response is now cached, the origin should not get a request + { + const res = await client.request(request) + equal(requestsToOrigin, 2) + strictEqual(await res.body.text(), 'asd') + } + }) + test('stale responses are revalidated before deleteAt (if-modified-since)', async () => { const clock = FakeTimers.install({ shouldClearNativeTimers: true