Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 38e58b3

Browse filesBrowse files
eduardoboucaspieh
andauthored
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 <misiek.piechowiak@gmail.com>
1 parent 871f7b9 commit 38e58b3
Copy full SHA for 38e58b3

File tree

Expand file treeCollapse file tree

3 files changed

+115
-12
lines changed
Filter options
Expand file treeCollapse file tree

3 files changed

+115
-12
lines changed

‎src/run/handlers/cache.cts

Copy file name to clipboardExpand all lines: src/run/handlers/cache.cts
+19-11Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -348,16 +348,20 @@ export class NetlifyCacheHandler implements CacheHandlerForMultipleVersions {
348348
if (requestContext?.didPagesRouterOnDemandRevalidate) {
349349
// encode here to deal with non ASCII characters in the key
350350
const tag = `_N_T_${key === '/index' ? '/' : encodeURI(key)}`
351+
const tags = tag.split(/,|%2c/gi).filter(Boolean)
352+
353+
if (tags.length === 0) {
354+
return
355+
}
356+
351357
getLogger().debug(`Purging CDN cache for: [${tag}]`)
352358
requestContext.trackBackgroundWork(
353-
purgeCache({ tags: tag.split(/,|%2c/gi), userAgent: purgeCacheUserAgent }).catch(
354-
(error) => {
355-
// TODO: add reporting here
356-
getLogger()
357-
.withError(error)
358-
.error(`[NetlifyCacheHandler]: Purging the cache for tag ${tag} failed`)
359-
},
360-
),
359+
purgeCache({ tags, userAgent: purgeCacheUserAgent }).catch((error) => {
360+
// TODO: add reporting here
361+
getLogger()
362+
.withError(error)
363+
.error(`[NetlifyCacheHandler]: Purging the cache for tag ${tag} failed`)
364+
}),
361365
)
362366
}
363367
}
@@ -380,9 +384,13 @@ export class NetlifyCacheHandler implements CacheHandlerForMultipleVersions {
380384
private async doRevalidateTag(tagOrTags: string | string[], ...args: any) {
381385
getLogger().withFields({ tagOrTags, args }).debug('NetlifyCacheHandler.revalidateTag')
382386

383-
const tags = (Array.isArray(tagOrTags) ? tagOrTags : [tagOrTags]).flatMap((tag) =>
384-
tag.split(/,|%2c/gi),
385-
)
387+
const tags = (Array.isArray(tagOrTags) ? tagOrTags : [tagOrTags])
388+
.flatMap((tag) => tag.split(/,|%2c/gi))
389+
.filter(Boolean)
390+
391+
if (tags.length === 0) {
392+
return
393+
}
386394

387395
const data: TagManifest = {
388396
revalidatedAt: Date.now(),
+21Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import { unstable_cache } from 'next/cache'
2+
3+
export const dynamic = 'force-dynamic'
4+
5+
const getData = unstable_cache(
6+
async () => {
7+
return {
8+
timestamp: Date.now(),
9+
}
10+
},
11+
[],
12+
{
13+
revalidate: 1,
14+
},
15+
)
16+
17+
export default async function Page() {
18+
const data = await getData()
19+
20+
return <pre>{JSON.stringify(data, null, 2)}</pre>
21+
}

‎tests/integration/simple-app.test.ts

Copy file name to clipboardExpand all lines: tests/integration/simple-app.test.ts
+75-1Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,21 @@ import { cp } from 'node:fs/promises'
44
import { createRequire } from 'node:module'
55
import { join } from 'node:path'
66
import { gunzipSync } from 'node:zlib'
7+
import { HttpResponse, http, passthrough } from 'msw'
8+
import { setupServer } from 'msw/node'
79
import { gt, prerelease } from 'semver'
810
import { v4 } from 'uuid'
9-
import { Mock, afterAll, beforeAll, beforeEach, describe, expect, test, vi } from 'vitest'
11+
import {
12+
Mock,
13+
afterAll,
14+
afterEach,
15+
beforeAll,
16+
beforeEach,
17+
describe,
18+
expect,
19+
test,
20+
vi,
21+
} from 'vitest'
1022
import { getPatchesToApply } from '../../src/build/content/server.js'
1123
import { type FixtureTestContext } from '../utils/contexts.js'
1224
import {
@@ -36,9 +48,32 @@ vi.mock('node:fs/promises', async (importOriginal) => {
3648
}
3749
})
3850

51+
let server: ReturnType<typeof setupServer>
52+
3953
// Disable the verbose logging of the lambda-local runtime
4054
getLogger().level = 'alert'
4155

56+
const purgeAPI = vi.fn()
57+
58+
beforeAll(() => {
59+
server = setupServer(
60+
http.post('https://api.netlify.com/api/v1/purge', async ({ request }) => {
61+
purgeAPI(await request.json())
62+
63+
return HttpResponse.json({
64+
ok: true,
65+
})
66+
}),
67+
http.all(/.*/, () => passthrough()),
68+
)
69+
server.listen()
70+
})
71+
72+
afterAll(() => {
73+
// Disable API mocking after the tests are done.
74+
server.close()
75+
})
76+
4277
beforeEach<FixtureTestContext>(async (ctx) => {
4378
// set for each test a new deployID and siteID
4479
ctx.deployID = generateRandomObjectID()
@@ -48,9 +83,15 @@ beforeEach<FixtureTestContext>(async (ctx) => {
4883
// hide debug logs in tests
4984
vi.spyOn(console, 'debug').mockImplementation(() => {})
5085

86+
purgeAPI.mockClear()
87+
5188
await startMockBlobStore(ctx)
5289
})
5390

91+
afterEach(() => {
92+
vi.unstubAllEnvs()
93+
})
94+
5495
test<FixtureTestContext>('Test that the simple next app is working', async (ctx) => {
5596
await createFixture('simple', ctx)
5697
await runPlugin(ctx)
@@ -210,6 +251,39 @@ test<FixtureTestContext>('cacheable route handler is cached on cdn (revalidate=f
210251
)
211252
})
212253

254+
test<FixtureTestContext>('purge API is not used when unstable_cache cache entry gets stale', async (ctx) => {
255+
await createFixture('simple', ctx)
256+
await runPlugin(ctx)
257+
258+
// set the NETLIFY_PURGE_API_TOKEN to get pass token check and allow fetch call to be made
259+
vi.stubEnv('NETLIFY_PURGE_API_TOKEN', 'mock')
260+
261+
const page1 = await invokeFunction(ctx, {
262+
url: '/unstable_cache',
263+
})
264+
const data1 = load(page1.body)('pre').text()
265+
266+
// allow for cache entry to get stale
267+
await new Promise((res) => setTimeout(res, 2000))
268+
269+
const page2 = await invokeFunction(ctx, {
270+
url: '/unstable_cache',
271+
})
272+
const data2 = load(page2.body)('pre').text()
273+
274+
const page3 = await invokeFunction(ctx, {
275+
url: '/unstable_cache',
276+
})
277+
const data3 = load(page3.body)('pre').text()
278+
279+
expect(purgeAPI, 'Purge API should not be hit').toHaveBeenCalledTimes(0)
280+
expect(
281+
data2,
282+
'Should use stale cache entry for current request and invalidate it in background',
283+
).toBe(data1)
284+
expect(data3, 'Should use updated cache entry').not.toBe(data2)
285+
})
286+
213287
test<FixtureTestContext>('cacheable route handler is cached on cdn (revalidate=15)', async (ctx) => {
214288
await createFixture('simple', ctx)
215289
await runPlugin(ctx)

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.