-
Notifications
You must be signed in to change notification settings - Fork 87
refactor: move tags handling from cache-handler module to dedicated tags-handler to allow for reuse #2872
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+148
−103
Merged
refactor: move tags handling from cache-handler module to dedicated tags-handler to allow for reuse #2872
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
a16c162
refactor: move tags handling from cache-handler module to dedicated t…
pieh b9bec2e
chore: rename lastTagRevalidationTimestamp to getTagRevalidatedAt
pieh 6dece23
chore: rename data to tagManifest when upserting revalidation time fo…
pieh 46e5e5b
chore: move cache purging debug log to avoid logging when no tags are…
pieh 9fb72e1
chore: adjust debug log prefix as this tag manifest handling will not…
pieh 4ce9230
chore: use existing helper purgeEdgeCache instead of calling purgeCac…
pieh b6995a5
fix: using context.waitUntil after response was already returned is n…
pieh 35a389c
Merge branch 'main' into refactor/seperate-tags-handling
pieh File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,141 @@ | ||
| import { purgeCache } from '@netlify/functions' | ||
|
|
||
| import { name as nextRuntimePkgName, version as nextRuntimePkgVersion } from '../../../package.json' | ||
| import { TagManifest } from '../../shared/blob-types.cjs' | ||
| import { | ||
| getMemoizedKeyValueStoreBackedByRegionalBlobStore, | ||
| MemoizedKeyValueStoreBackedByRegionalBlobStore, | ||
| } from '../storage/storage.cjs' | ||
|
|
||
| import { getLogger, getRequestContext } from './request-context.cjs' | ||
|
|
||
| const purgeCacheUserAgent = `${nextRuntimePkgName}@${nextRuntimePkgVersion}` | ||
|
|
||
| /** | ||
| * Get timestamp of the last revalidation for a tag | ||
| */ | ||
| async function getTagRevalidatedAt( | ||
| tag: string, | ||
| cacheStore: MemoizedKeyValueStoreBackedByRegionalBlobStore, | ||
| ): Promise<number | null> { | ||
| const tagManifest = await cacheStore.get<TagManifest>(tag, 'tagManifest.get') | ||
| if (!tagManifest) { | ||
| return null | ||
| } | ||
| return tagManifest.revalidatedAt | ||
mrstork marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| /** | ||
| * Check if any of the tags were invalidated since the given timestamp | ||
| */ | ||
| export function isAnyTagStale(tags: string[], timestamp: number): Promise<boolean> { | ||
| if (tags.length === 0 || !timestamp) { | ||
| return Promise.resolve(false) | ||
| } | ||
|
|
||
| const cacheStore = getMemoizedKeyValueStoreBackedByRegionalBlobStore({ consistency: 'strong' }) | ||
|
|
||
| // Full-route cache and fetch caches share a lot of tags | ||
| // but we will only do actual blob read once withing a single request due to cacheStore | ||
| // memoization. | ||
| // Additionally, we will resolve the promise as soon as we find first | ||
| // stale tag, so that we don't wait for all of them to resolve (but keep all | ||
| // running in case future `CacheHandler.get` calls would be able to use results). | ||
| // "Worst case" scenario is none of tag was invalidated in which case we need to wait | ||
| // for all blob store checks to finish before we can be certain that no tag is stale. | ||
| return new Promise<boolean>((resolve, reject) => { | ||
| const tagManifestPromises: Promise<boolean>[] = [] | ||
|
|
||
| for (const tag of tags) { | ||
| const lastRevalidationTimestampPromise = getTagRevalidatedAt(tag, cacheStore) | ||
|
|
||
| tagManifestPromises.push( | ||
| lastRevalidationTimestampPromise.then((lastRevalidationTimestamp) => { | ||
| if (!lastRevalidationTimestamp) { | ||
| // tag was never revalidated | ||
| return false | ||
| } | ||
| const isStale = lastRevalidationTimestamp >= timestamp | ||
| if (isStale) { | ||
| // resolve outer promise immediately if any of the tags is stale | ||
| resolve(true) | ||
| return true | ||
| } | ||
| return false | ||
| }), | ||
| ) | ||
| } | ||
|
|
||
| // make sure we resolve promise after all blobs are checked (if we didn't resolve as stale yet) | ||
| Promise.all(tagManifestPromises) | ||
| .then((tagManifestAreStale) => { | ||
| resolve(tagManifestAreStale.some((tagIsStale) => tagIsStale)) | ||
| }) | ||
| .catch(reject) | ||
| }) | ||
| } | ||
|
|
||
| /** | ||
| * Transform a tag or tags into an array of tags and handle white space splitting and encoding | ||
| */ | ||
| function getCacheTagsFromTagOrTags(tagOrTags: string | string[]): string[] { | ||
| return (Array.isArray(tagOrTags) ? tagOrTags : [tagOrTags]) | ||
| .flatMap((tag) => tag.split(/,|%2c/gi)) | ||
| .filter(Boolean) | ||
| } | ||
|
|
||
| export function purgeEdgeCache(tagOrTags: string | string[]): Promise<void> { | ||
| const tags = getCacheTagsFromTagOrTags(tagOrTags) | ||
|
|
||
| if (tags.length === 0) { | ||
| return Promise.resolve() | ||
| } | ||
|
|
||
| getLogger().debug(`[NextRuntime] Purging CDN cache for: [${tags}.join(', ')]`) | ||
|
|
||
| return purgeCache({ tags, userAgent: purgeCacheUserAgent }).catch((error) => { | ||
| // TODO: add reporting here | ||
| getLogger() | ||
| .withError(error) | ||
| .error(`[NextRuntime] Purging the cache for tags [${tags.join(',')}] failed`) | ||
| }) | ||
| } | ||
|
|
||
| async function doRevalidateTagAndPurgeEdgeCache(tags: string[]): Promise<void> { | ||
| getLogger().withFields({ tags }).debug('doRevalidateTagAndPurgeEdgeCache') | ||
|
|
||
| if (tags.length === 0) { | ||
| return | ||
| } | ||
|
|
||
| const tagManifest: TagManifest = { | ||
| revalidatedAt: Date.now(), | ||
| } | ||
|
|
||
| const cacheStore = getMemoizedKeyValueStoreBackedByRegionalBlobStore({ consistency: 'strong' }) | ||
|
|
||
| await Promise.all( | ||
| tags.map(async (tag) => { | ||
| try { | ||
| await cacheStore.set(tag, tagManifest, 'tagManifest.set') | ||
| } catch (error) { | ||
| getLogger().withError(error).log(`[NextRuntime] Failed to update tag manifest for ${tag}`) | ||
| } | ||
| }), | ||
| ) | ||
|
|
||
| await purgeEdgeCache(tags) | ||
| } | ||
|
|
||
| export function markTagsAsStaleAndPurgeEdgeCache(tagOrTags: string | string[]) { | ||
| const tags = getCacheTagsFromTagOrTags(tagOrTags) | ||
|
|
||
| const revalidateTagPromise = doRevalidateTagAndPurgeEdgeCache(tags) | ||
|
|
||
| const requestContext = getRequestContext() | ||
| if (requestContext) { | ||
| requestContext.trackBackgroundWork(revalidateTagPromise) | ||
| } | ||
|
|
||
| return revalidateTagPromise | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we wanted, we could return a number here as well and simplify the return type of this function to just
Promise<number>. Perhaps-1?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pros & cons
returning a number may simplify types and possibly remove the need to do specific
nullchecking, but I think it's actually better to return non-number here exactly to force consumer of this function to consider what should be done when there is no value so it forces to take a second how to handle scenario if there is no tag manifest as this should prevent bugs that are result of oversight if someone would assume there is always a value hereNext.js
use cachehandler wants to return0in cases like that - https://github.com/vercel/next.js/blob/071e31705f4451bdeb8ada4126a0da9c49d11761/packages/next/src/server/lib/cache-handlers/types.ts#L102-L108 so maybe we should follow this, but because this is used not just for new cache handler I'd prefer more verbose handling to ensure that consumers handling don't have potentially incompatible oversights