- 
                Notifications
    You must be signed in to change notification settings 
- Fork 92
feat: support revalidateTag with SWR behavior #3173
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
Changes from 2 commits
e0a4e48
              a4b1046
              76d9e12
              e13e932
              37c3e8e
              8f0a2d4
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|  | @@ -11,47 +11,55 @@ import { getLogger, getRequestContext } from './request-context.cjs' | |||||
|  | ||||||
| const purgeCacheUserAgent = `${nextRuntimePkgName}@${nextRuntimePkgVersion}` | ||||||
|  | ||||||
| /** | ||||||
| * Get timestamp of the last revalidation for a tag | ||||||
| */ | ||||||
| async function getTagRevalidatedAt( | ||||||
| async function getTagManifest( | ||||||
| tag: string, | ||||||
| cacheStore: MemoizedKeyValueStoreBackedByRegionalBlobStore, | ||||||
| ): Promise<number | null> { | ||||||
| ): Promise<TagManifest | null> { | ||||||
| const tagManifest = await cacheStore.get<TagManifest>(tag, 'tagManifest.get') | ||||||
| if (!tagManifest) { | ||||||
| return null | ||||||
| } | ||||||
| return tagManifest.revalidatedAt | ||||||
| return tagManifest | ||||||
| } | ||||||
|  | ||||||
| /** | ||||||
| * Get the most recent revalidation timestamp for a list of tags | ||||||
| */ | ||||||
| export async function getMostRecentTagRevalidationTimestamp(tags: string[]) { | ||||||
| export async function getMostRecentTagExpirationTimestamp(tags: string[]) { | ||||||
| if (tags.length === 0) { | ||||||
| return 0 | ||||||
| } | ||||||
|  | ||||||
| const cacheStore = getMemoizedKeyValueStoreBackedByRegionalBlobStore({ consistency: 'strong' }) | ||||||
|  | ||||||
| const timestampsOrNulls = await Promise.all( | ||||||
| tags.map((tag) => getTagRevalidatedAt(tag, cacheStore)), | ||||||
| ) | ||||||
| const timestampsOrNulls = await Promise.all(tags.map((tag) => getTagManifest(tag, cacheStore))) | ||||||
|  | ||||||
| const timestamps = timestampsOrNulls.filter((timestamp) => timestamp !== null) | ||||||
| if (timestamps.length === 0) { | ||||||
| const expirationTimestamps = timestampsOrNulls | ||||||
| .filter((timestamp) => timestamp !== null) | ||||||
|         
                  pieh marked this conversation as resolved.
              Outdated
          
            Show resolved
            Hide resolved | ||||||
| .map((manifest) => manifest.expiredAt) | ||||||
| if (expirationTimestamps.length === 0) { | ||||||
| return 0 | ||||||
| } | ||||||
| return Math.max(...timestamps) | ||||||
| return Math.max(...expirationTimestamps) | ||||||
| } | ||||||
|  | ||||||
| export type TagStaleOrExpired = | ||||||
|         
                  pieh marked this conversation as resolved.
              Outdated
          
            Show resolved
            Hide resolved | ||||||
| // FRESH | ||||||
| | { stale: false; expired: false } | ||||||
| // STALE | ||||||
| | { stale: true; expired: false; expireAt: number } | ||||||
| // EXPIRED (should be treated similarly to MISS) | ||||||
| | { stale: true; expired: true } | ||||||
|  | ||||||
| /** | ||||||
| * Check if any of the tags were invalidated since the given timestamp | ||||||
| * Check if any of the tags expired since the given timestamp | ||||||
| */ | ||||||
| export function isAnyTagStale(tags: string[], timestamp: number): Promise<boolean> { | ||||||
| export function isAnyTagStaleOrExpired( | ||||||
| tags: string[], | ||||||
| timestamp: number, | ||||||
| ): Promise<TagStaleOrExpired> { | ||||||
| if (tags.length === 0 || !timestamp) { | ||||||
| return Promise.resolve(false) | ||||||
| return Promise.resolve({ stale: false, expired: false }) | ||||||
| } | ||||||
|  | ||||||
| const cacheStore = getMemoizedKeyValueStoreBackedByRegionalBlobStore({ consistency: 'strong' }) | ||||||
|  | @@ -60,37 +68,74 @@ export function isAnyTagStale(tags: string[], timestamp: number): Promise<boolea | |||||
| // 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 | ||||||
| // expired 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>[] = [] | ||||||
| // "Worst case" scenario is none of tag was expired in which case we need to wait | ||||||
| // for all blob store checks to finish before we can be certain that no tag is expired. | ||||||
| return new Promise<TagStaleOrExpired>((resolve, reject) => { | ||||||
| const tagManifestPromises: Promise<TagStaleOrExpired>[] = [] | ||||||
|  | ||||||
| for (const tag of tags) { | ||||||
| const lastRevalidationTimestampPromise = getTagRevalidatedAt(tag, cacheStore) | ||||||
| const tagManifestPromise = getTagManifest(tag, cacheStore) | ||||||
|  | ||||||
| tagManifestPromises.push( | ||||||
| lastRevalidationTimestampPromise.then((lastRevalidationTimestamp) => { | ||||||
| if (!lastRevalidationTimestamp) { | ||||||
| tagManifestPromise.then((tagManifest) => { | ||||||
| if (!tagManifest) { | ||||||
| // tag was never revalidated | ||||||
| return false | ||||||
| return { stale: false, expired: false } | ||||||
| } | ||||||
| const stale = tagManifest.staleAt >= timestamp | ||||||
| const expired = tagManifest.expiredAt >= timestamp && tagManifest.expiredAt <= Date.now() | ||||||
|  | ||||||
| if (expired && stale) { | ||||||
| const expiredResult: TagStaleOrExpired = { | ||||||
| stale, | ||||||
| expired, | ||||||
| } | ||||||
| // resolve outer promise immediately if any of the tags is expired | ||||||
| resolve(expiredResult) | ||||||
| return expiredResult | ||||||
| } | ||||||
| const isStale = lastRevalidationTimestamp >= timestamp | ||||||
| if (isStale) { | ||||||
| // resolve outer promise immediately if any of the tags is stale | ||||||
| resolve(true) | ||||||
| return true | ||||||
|  | ||||||
| if (stale) { | ||||||
| const staleResult: TagStaleOrExpired = { | ||||||
| stale, | ||||||
| expired, | ||||||
| expireAt: tagManifest.expiredAt, | ||||||
| } | ||||||
| return staleResult | ||||||
| } | ||||||
| return false | ||||||
| return { stale: false, expired: false } | ||||||
| }), | ||||||
| ) | ||||||
| } | ||||||
|  | ||||||
| // make sure we resolve promise after all blobs are checked (if we didn't resolve as stale yet) | ||||||
| // make sure we resolve promise after all blobs are checked (if we didn't resolve as expired yet) | ||||||
| Promise.all(tagManifestPromises) | ||||||
| .then((tagManifestAreStale) => { | ||||||
| resolve(tagManifestAreStale.some((tagIsStale) => tagIsStale)) | ||||||
| .then((tagManifestsAreStaleOrExpired) => { | ||||||
| let result: TagStaleOrExpired = { stale: false, expired: false } | ||||||
|  | ||||||
| for (const tagResult of tagManifestsAreStaleOrExpired) { | ||||||
| if (tagResult.expired) { | ||||||
| // if any of the tags is expired, the whole thing is expired | ||||||
| result = tagResult | ||||||
| break | ||||||
| } | ||||||
|  | ||||||
| if (tagResult.stale) { | ||||||
| result = { | ||||||
| stale: true, | ||||||
| expired: false, | ||||||
| expireAt: | ||||||
| // make sure to use expireAt that is lowest of all tags | ||||||
| result.stale && !result.expired && typeof result.expireAt === 'number' | ||||||
| ? Math.min(result.expireAt, tagResult.expireAt) | ||||||
| : tagResult.expireAt, | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|  | ||||||
| resolve(result) | ||||||
| }) | ||||||
| .catch(reject) | ||||||
| }) | ||||||
|  | @@ -122,15 +167,21 @@ export function purgeEdgeCache(tagOrTags: string | string[]): Promise<void> { | |||||
| }) | ||||||
| } | ||||||
|  | ||||||
| async function doRevalidateTagAndPurgeEdgeCache(tags: string[]): Promise<void> { | ||||||
| getLogger().withFields({ tags }).debug('doRevalidateTagAndPurgeEdgeCache') | ||||||
| async function doRevalidateTagAndPurgeEdgeCache( | ||||||
| tags: string[], | ||||||
| durations?: { expire?: number }, | ||||||
|          | ||||||
| durations?: { expire?: number }, | |
| durations?: { expireSeconds?: number }, | 
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.
the durations type is from Next.js - https://github.com/vercel/next.js/blob/fffa2831b61fa74852736eeaad2f17fbdd553bce/packages/next/src/server/lib/incremental-cache/index.ts#L78
I do not need to use the exact type here, but I do need to use original type at least in our CacheHandler and would need to add conversion from Next.js type to our type which doesn't seem worth it?
But for clarity I will add some jsdocs or code comments along the call chain to clarify that expire is in seconds
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.
added type with some descriptions 8f0a2d4
        
          
              
                Outdated
          
        
      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.
suggestion:
| durations?: { expire?: number }, | |
| durations?: { expireSeconds?: number }, | 
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.
same as #3173 (comment)
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.
left over from refactor:
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.
github didn't allow me to commit suggestion so this was done as part of 37c3e8e#diff-7fe4e66d51e744fc6fd6321c633def547066010bb05aff0ef621b64d84ec5442R35