From 38e58b3f46b78b307bcf7576a00849c41f495b52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20Bou=C3=A7as?= Date: Fri, 20 Dec 2024 10:42:50 +0000 Subject: [PATCH] fix: make `revalidateTags` no-op when list of tags is empty (#2727) * fix: make `revalidateTags` no-op when list of tags is empty * fix: oops * test: add integration test for site-wide purge calls * fix: filter out empty tags * test: correct afterEach import --------- Co-authored-by: pieh --- src/run/handlers/cache.cts | 30 +++++--- .../simple/app/unstable_cache/page.js | 21 +++++ tests/integration/simple-app.test.ts | 76 ++++++++++++++++++- 3 files changed, 115 insertions(+), 12 deletions(-) create mode 100644 tests/fixtures/simple/app/unstable_cache/page.js diff --git a/src/run/handlers/cache.cts b/src/run/handlers/cache.cts index 2ec5cb4bb3..c00a4d5e14 100644 --- a/src/run/handlers/cache.cts +++ b/src/run/handlers/cache.cts @@ -348,16 +348,20 @@ export class NetlifyCacheHandler implements CacheHandlerForMultipleVersions { if (requestContext?.didPagesRouterOnDemandRevalidate) { // encode here to deal with non ASCII characters in the key const tag = `_N_T_${key === '/index' ? '/' : encodeURI(key)}` + const tags = tag.split(/,|%2c/gi).filter(Boolean) + + if (tags.length === 0) { + return + } + getLogger().debug(`Purging CDN cache for: [${tag}]`) requestContext.trackBackgroundWork( - purgeCache({ tags: tag.split(/,|%2c/gi), userAgent: purgeCacheUserAgent }).catch( - (error) => { - // TODO: add reporting here - getLogger() - .withError(error) - .error(`[NetlifyCacheHandler]: Purging the cache for tag ${tag} failed`) - }, - ), + purgeCache({ tags, userAgent: purgeCacheUserAgent }).catch((error) => { + // TODO: add reporting here + getLogger() + .withError(error) + .error(`[NetlifyCacheHandler]: Purging the cache for tag ${tag} failed`) + }), ) } } @@ -380,9 +384,13 @@ export class NetlifyCacheHandler implements CacheHandlerForMultipleVersions { private async doRevalidateTag(tagOrTags: string | string[], ...args: any) { getLogger().withFields({ tagOrTags, args }).debug('NetlifyCacheHandler.revalidateTag') - const tags = (Array.isArray(tagOrTags) ? tagOrTags : [tagOrTags]).flatMap((tag) => - tag.split(/,|%2c/gi), - ) + const tags = (Array.isArray(tagOrTags) ? tagOrTags : [tagOrTags]) + .flatMap((tag) => tag.split(/,|%2c/gi)) + .filter(Boolean) + + if (tags.length === 0) { + return + } const data: TagManifest = { revalidatedAt: Date.now(), diff --git a/tests/fixtures/simple/app/unstable_cache/page.js b/tests/fixtures/simple/app/unstable_cache/page.js new file mode 100644 index 0000000000..6dc19a2df5 --- /dev/null +++ b/tests/fixtures/simple/app/unstable_cache/page.js @@ -0,0 +1,21 @@ +import { unstable_cache } from 'next/cache' + +export const dynamic = 'force-dynamic' + +const getData = unstable_cache( + async () => { + return { + timestamp: Date.now(), + } + }, + [], + { + revalidate: 1, + }, +) + +export default async function Page() { + const data = await getData() + + return 
{JSON.stringify(data, null, 2)}
+} diff --git a/tests/integration/simple-app.test.ts b/tests/integration/simple-app.test.ts index c082a8c480..790836fdf8 100644 --- a/tests/integration/simple-app.test.ts +++ b/tests/integration/simple-app.test.ts @@ -4,9 +4,21 @@ import { cp } from 'node:fs/promises' import { createRequire } from 'node:module' import { join } from 'node:path' import { gunzipSync } from 'node:zlib' +import { HttpResponse, http, passthrough } from 'msw' +import { setupServer } from 'msw/node' import { gt, prerelease } from 'semver' import { v4 } from 'uuid' -import { Mock, afterAll, beforeAll, beforeEach, describe, expect, test, vi } from 'vitest' +import { + Mock, + afterAll, + afterEach, + beforeAll, + beforeEach, + describe, + expect, + test, + vi, +} from 'vitest' import { getPatchesToApply } from '../../src/build/content/server.js' import { type FixtureTestContext } from '../utils/contexts.js' import { @@ -36,9 +48,32 @@ vi.mock('node:fs/promises', async (importOriginal) => { } }) +let server: ReturnType + // Disable the verbose logging of the lambda-local runtime getLogger().level = 'alert' +const purgeAPI = vi.fn() + +beforeAll(() => { + server = setupServer( + http.post('https://api.netlify.com/api/v1/purge', async ({ request }) => { + purgeAPI(await request.json()) + + return HttpResponse.json({ + ok: true, + }) + }), + http.all(/.*/, () => passthrough()), + ) + server.listen() +}) + +afterAll(() => { + // Disable API mocking after the tests are done. + server.close() +}) + beforeEach(async (ctx) => { // set for each test a new deployID and siteID ctx.deployID = generateRandomObjectID() @@ -48,9 +83,15 @@ beforeEach(async (ctx) => { // hide debug logs in tests vi.spyOn(console, 'debug').mockImplementation(() => {}) + purgeAPI.mockClear() + await startMockBlobStore(ctx) }) +afterEach(() => { + vi.unstubAllEnvs() +}) + test('Test that the simple next app is working', async (ctx) => { await createFixture('simple', ctx) await runPlugin(ctx) @@ -210,6 +251,39 @@ test('cacheable route handler is cached on cdn (revalidate=f ) }) +test('purge API is not used when unstable_cache cache entry gets stale', async (ctx) => { + await createFixture('simple', ctx) + await runPlugin(ctx) + + // set the NETLIFY_PURGE_API_TOKEN to get pass token check and allow fetch call to be made + vi.stubEnv('NETLIFY_PURGE_API_TOKEN', 'mock') + + const page1 = await invokeFunction(ctx, { + url: '/unstable_cache', + }) + const data1 = load(page1.body)('pre').text() + + // allow for cache entry to get stale + await new Promise((res) => setTimeout(res, 2000)) + + const page2 = await invokeFunction(ctx, { + url: '/unstable_cache', + }) + const data2 = load(page2.body)('pre').text() + + const page3 = await invokeFunction(ctx, { + url: '/unstable_cache', + }) + const data3 = load(page3.body)('pre').text() + + expect(purgeAPI, 'Purge API should not be hit').toHaveBeenCalledTimes(0) + expect( + data2, + 'Should use stale cache entry for current request and invalidate it in background', + ).toBe(data1) + expect(data3, 'Should use updated cache entry').not.toBe(data2) +}) + test('cacheable route handler is cached on cdn (revalidate=15)', async (ctx) => { await createFixture('simple', ctx) await runPlugin(ctx)