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 <[email protected]>

Author: eduardoboucas

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(),

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 <pre>{JSON.stringify(data, null, 2)}</pre>
+}

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<typeof setupServer>
+
 // 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<FixtureTestContext>(async (ctx) => {
   // set for each test a new deployID and siteID
   ctx.deployID = generateRandomObjectID()
@@ -48,9 +83,15 @@ beforeEach<FixtureTestContext>(async (ctx) => {
   // hide debug logs in tests
   vi.spyOn(console, 'debug').mockImplementation(() => {})
 
+  purgeAPI.mockClear()
+
   await startMockBlobStore(ctx)
 })
 
+afterEach(() => {
+  vi.unstubAllEnvs()
+})
+
 test<FixtureTestContext>('Test that the simple next app is working', async (ctx) => {
   await createFixture('simple', ctx)
   await runPlugin(ctx)
@@ -210,6 +251,39 @@ test<FixtureTestContext>('cacheable route handler is cached on cdn (revalidate=f
   )
 })
 
+test<FixtureTestContext>('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<FixtureTestContext>('cacheable route handler is cached on cdn (revalidate=15)', async (ctx) => {
   await createFixture('simple', ctx)
   await runPlugin(ctx)