From c8754e98abb9e3d7ad38f8ebe0cfe401e272d9c8 Mon Sep 17 00:00:00 2001 From: Dorseuil Nicolas Date: Fri, 31 Jan 2025 13:40:26 +0100 Subject: [PATCH 01/10] add new tag cache mode --- packages/open-next/src/adapters/cache.ts | 81 ++++++---- .../open-next/src/adapters/dynamo-provider.ts | 8 + .../src/core/routing/cacheInterceptor.ts | 19 +-- .../incrementalCache/multi-tier-ddb-s3.ts | 2 +- .../src/overrides/tagCache/constants.ts | 1 + .../src/overrides/tagCache/ddb-nextMode.ts | 153 ++++++++++++++++++ .../open-next/src/overrides/tagCache/dummy.ts | 1 + .../src/overrides/tagCache/dynamodb-lite.ts | 1 + .../src/overrides/tagCache/dynamodb.ts | 1 + .../src/overrides/tagCache/fs-dev.ts | 1 + packages/open-next/src/types/cache.ts | 7 +- packages/open-next/src/types/open-next.ts | 1 + packages/open-next/src/types/overrides.ts | 16 +- packages/open-next/src/utils/cache.ts | 36 +++++ 14 files changed, 283 insertions(+), 45 deletions(-) create mode 100644 packages/open-next/src/overrides/tagCache/ddb-nextMode.ts create mode 100644 packages/open-next/src/utils/cache.ts diff --git a/packages/open-next/src/adapters/cache.ts b/packages/open-next/src/adapters/cache.ts index 168d92e1d..484b2e8ba 100644 --- a/packages/open-next/src/adapters/cache.ts +++ b/packages/open-next/src/adapters/cache.ts @@ -1,5 +1,6 @@ import { isBinaryContentType } from "../utils/binary"; import { debug, error, warn } from "./logger"; +import { getTagFromValue, hasBeenRevalidated } from "utils/cache"; interface CachedFetchValue { kind: "FETCH"; @@ -134,14 +135,15 @@ export default class Cache { if (cachedEntry?.value === undefined) return null; - const _lastModified = await globalThis.tagCache.getLastModified( + const _tags = [...(tags ?? []), ...(softTags ?? [])]; + const _lastModified = cachedEntry.lastModified ?? Date.now(); + const _hasBeenRevalidated = await hasBeenRevalidated( key, + _tags, cachedEntry?.lastModified, ); - if (_lastModified === -1) { - // If some tags are stale we need to force revalidation - return null; - } + + if (_hasBeenRevalidated) return null; // For cases where we don't have tags, we need to ensure that the soft tags are not being revalidated // We only need to check for the path as it should already contain all the tags @@ -154,11 +156,12 @@ export default class Cache { !tag.endsWith("page"), ); if (path) { - const pathLastModified = await globalThis.tagCache.getLastModified( + const hasPathBeenUpdated = await hasBeenRevalidated( path.replace("_N_T_/", ""), + [], cachedEntry.lastModified, ); - if (pathLastModified === -1) { + if (hasPathBeenUpdated) { // In case the path has been revalidated, we don't want to use the fetch cache return null; } @@ -184,20 +187,23 @@ export default class Cache { return null; } - const meta = cachedEntry.value.meta; - const _lastModified = await globalThis.tagCache.getLastModified( + const cacheData = cachedEntry?.value; + + const meta = cacheData?.meta; + const tags = getTagFromValue(cacheData); + const _lastModified = cachedEntry.lastModified ?? Date.now(); + const _hasBeenRevalidated = await hasBeenRevalidated( key, - cachedEntry?.lastModified, + tags, + _lastModified, ); - if (_lastModified === -1) { - // If some tags are stale we need to force revalidation - return null; - } - const cacheData = cachedEntry?.value; + if (cacheData === undefined || _hasBeenRevalidated) return null; + const store = globalThis.__openNextAls.getStore(); if (store) { store.lastModified = _lastModified; } + if (cacheData?.type === "route") { return { lastModified: _lastModified, @@ -372,23 +378,7 @@ export default class Cache { ? (data.headers?.["x-next-cache-tags"]?.split(",") ?? []) : []; debug("derivedTags", derivedTags); - // Get all tags stored in dynamodb for the given key - // If any of the derived tags are not stored in dynamodb for the given key, write them - const storedTags = await globalThis.tagCache.getByPath(key); - const tagsToWrite = derivedTags.filter( - (tag) => !storedTags.includes(tag), - ); - if (tagsToWrite.length > 0) { - await globalThis.tagCache.writeTags( - tagsToWrite.map((tag) => ({ - path: key, - tag: tag, - // In case the tags are not there we just need to create them - // but we don't want them to return from `getLastModified` as they are not stale - revalidatedAt: 1, - })), - ); - } + await this.updateTagsOnSet(key, derivedTags); debug("Finished setting cache"); } catch (e) { error("Failed to set cache", e); @@ -405,6 +395,9 @@ export default class Cache { } try { const _tags = Array.isArray(tags) ? tags : [tags]; + if (globalThis.tagCache.mode === "nextMode") { + return globalThis.tagCache.writeTags(_tags); + } for (const tag of _tags) { debug("revalidateTag", tag); // Find all keys with the given tag @@ -468,4 +461,28 @@ export default class Cache { error("Failed to revalidate tag", e); } } + + private async updateTagsOnSet(key: string, derivedTags: string[]) { + if ( + globalThis.openNextConfig.dangerous?.disableTagCache || + globalThis.tagCache.mode === "nextMode" + ) { + return; + } + // Get all tags stored in dynamodb for the given key + // If any of the derived tags are not stored in dynamodb for the given key, write them + const storedTags = await globalThis.tagCache.getByPath(key); + const tagsToWrite = derivedTags.filter((tag) => !storedTags.includes(tag)); + if (tagsToWrite.length > 0) { + await globalThis.tagCache.writeTags( + tagsToWrite.map((tag) => ({ + path: key, + tag: tag, + // In case the tags are not there we just need to create them + // but we don't want them to return from `getLastModified` as they are not stale + revalidatedAt: 1, + })), + ); + } + } } diff --git a/packages/open-next/src/adapters/dynamo-provider.ts b/packages/open-next/src/adapters/dynamo-provider.ts index d91b2b4cc..6aacd7159 100644 --- a/packages/open-next/src/adapters/dynamo-provider.ts +++ b/packages/open-next/src/adapters/dynamo-provider.ts @@ -47,6 +47,14 @@ async function defaultHandler( async function insert( requestType: InitializationFunctionEvent["requestType"], ): Promise { + // If it is in nextMode, we don't need to do anything + if (tagCache.mode === "nextMode") { + return { + type: "initializationFunction", + requestType, + resourceId: PHYSICAL_RESOURCE_ID, + }; + } const file = readFileSync("dynamodb-cache.json", "utf8"); const data: DataType[] = JSON.parse(file); diff --git a/packages/open-next/src/core/routing/cacheInterceptor.ts b/packages/open-next/src/core/routing/cacheInterceptor.ts index 25a33d2a5..b8794945f 100644 --- a/packages/open-next/src/core/routing/cacheInterceptor.ts +++ b/packages/open-next/src/core/routing/cacheInterceptor.ts @@ -5,9 +5,10 @@ import type { InternalEvent, InternalResult } from "types/open-next"; import type { CacheValue } from "types/overrides"; import { emptyReadableStream, toReadableStream } from "utils/stream"; -import { debug } from "../../adapters/logger.js"; -import { localizePath } from "./i18n/index.js"; -import { generateMessageGroupId } from "./queue.js"; +import { debug } from "../../adapters/logger"; +import { localizePath } from "./i18n"; +import { generateMessageGroupId } from "./queue"; +import { getTagFromValue, hasBeenRevalidated } from "utils/cache"; const CACHE_ONE_YEAR = 60 * 60 * 24 * 365; const CACHE_ONE_MONTH = 60 * 60 * 24 * 30; @@ -161,15 +162,15 @@ export async function cacheInterceptor( if (!cachedData?.value) { return event; } - - if (cachedData?.value?.type === "app") { - // We need to check the tag cache now - const _lastModified = await globalThis.tagCache.getLastModified( + // We need to check the tag cache now + if (cachedData.value?.type === "app") { + const tags = getTagFromValue(cachedData.value); + const _hasBeenRevalidated = await hasBeenRevalidated( localizedPath, + tags, cachedData.lastModified, ); - if (_lastModified === -1) { - // If some tags are stale we need to force revalidation + if (_hasBeenRevalidated) { return event; } } diff --git a/packages/open-next/src/overrides/incrementalCache/multi-tier-ddb-s3.ts b/packages/open-next/src/overrides/incrementalCache/multi-tier-ddb-s3.ts index 9dc25b513..49c254258 100644 --- a/packages/open-next/src/overrides/incrementalCache/multi-tier-ddb-s3.ts +++ b/packages/open-next/src/overrides/incrementalCache/multi-tier-ddb-s3.ts @@ -14,7 +14,7 @@ const maxCacheSize = process.env.OPEN_NEXT_LOCAL_CACHE_SIZE : 1000; const localCache = new LRUCache<{ - value: CacheValue; + value: CacheValue; lastModified: number; }>(maxCacheSize); diff --git a/packages/open-next/src/overrides/tagCache/constants.ts b/packages/open-next/src/overrides/tagCache/constants.ts index c94261b34..c3e9f82fb 100644 --- a/packages/open-next/src/overrides/tagCache/constants.ts +++ b/packages/open-next/src/overrides/tagCache/constants.ts @@ -1,4 +1,5 @@ export const MAX_DYNAMO_BATCH_WRITE_ITEM_COUNT = 25; +export const MAX_DYNAMO_BATCH_GET_ITEM_COUNT = 100; /** * Sending to dynamo X commands at a time, using about X * 25 write units per batch to not overwhelm DDB diff --git a/packages/open-next/src/overrides/tagCache/ddb-nextMode.ts b/packages/open-next/src/overrides/tagCache/ddb-nextMode.ts new file mode 100644 index 000000000..87b597569 --- /dev/null +++ b/packages/open-next/src/overrides/tagCache/ddb-nextMode.ts @@ -0,0 +1,153 @@ +import type { NextModeTagCache } from "types/overrides"; + +import { AwsClient } from "aws4fetch"; +import { RecoverableError } from "utils/error"; +import { customFetchClient } from "utils/fetch"; + +import { debug, error } from "../../adapters/logger"; +import { chunk, parseNumberFromEnv } from "../../adapters/util"; +import { + MAX_DYNAMO_BATCH_WRITE_ITEM_COUNT, + getDynamoBatchWriteCommandConcurrency, +} from "./constants"; +import path from "node:path"; + +let awsClient: AwsClient | null = null; + +const getAwsClient = () => { + const { CACHE_BUCKET_REGION } = process.env; + if (awsClient) { + return awsClient; + } + awsClient = new AwsClient({ + accessKeyId: process.env.AWS_ACCESS_KEY_ID!, + secretAccessKey: process.env.AWS_SECRET_ACCESS_KEY!, + sessionToken: process.env.AWS_SESSION_TOKEN, + region: CACHE_BUCKET_REGION, + retries: parseNumberFromEnv(process.env.AWS_SDK_S3_MAX_ATTEMPTS), + }); + return awsClient; +}; +const awsFetch = ( + body: RequestInit["body"], + type: "query" | "batchWrite" = "query", +) => { + const { CACHE_BUCKET_REGION } = process.env; + const client = getAwsClient(); + return customFetchClient(client)( + `https://dynamodb.${CACHE_BUCKET_REGION}.amazonaws.com`, + { + method: "POST", + headers: { + "Content-Type": "application/x-amz-json-1.0", + "X-Amz-Target": `DynamoDB_20120810.${ + type === "query" ? "BatchGetItem" : "BatchWriteItem" + }`, + }, + body, + }, + ); +}; + +function buildDynamoKey(key: string) { + const { NEXT_BUILD_ID } = process.env; + // FIXME: We should probably use something else than path.join here + // this could transform some fetch cache key into a valid path + return path.posix.join(NEXT_BUILD_ID ?? "", "_tag", key); +} + +// We use the same key for both path and tag +// That's mostly for compatibility reason so that it's easier to use this with existing infra +// FIXME: Allow a simpler object without an unnecessary path key +function buildDynamoObject(tag: string, revalidatedAt?: number) { + return { + path: { S: buildDynamoKey(tag) }, + tag: { S: buildDynamoKey(tag) }, + revalidatedAt: { N: `${revalidatedAt ?? Date.now()}` }, + }; +} + +export default { + name: "ddb-nextMode", + mode: "nextMode", + hasBeenRevalidated: async (tags: string[], lastModified?: number) => { + if (globalThis.openNextConfig.dangerous?.disableTagCache) { + return false; + } + const { CACHE_DYNAMO_TABLE } = process.env; + // It's unlikely that we will have more than 100 items to query + // If that's the case, you should not use this tagCache implementation + const response = await awsFetch( + JSON.stringify({ + RequestItems: { + [CACHE_DYNAMO_TABLE ?? ""]: { + Keys: tags.map((tag) => ({ + path: { S: buildDynamoKey(tag) }, + tag: { S: buildDynamoKey(tag) }, + })), + }, + }, + }), + "query", + ); + if (response.status !== 200) { + throw new RecoverableError( + `Failed to query dynamo item: ${response.status}`, + ); + } + // Now we need to check for every item if lastModified is greater than the revalidatedAt + const { Responses } = await response.json(); + if (!Responses) { + return false; + } + const revalidatedTags = Responses[CACHE_DYNAMO_TABLE ?? ""].filter( + (item: any) => + Number.parseInt(item.revalidatedAt.N) > (lastModified ?? 0), + ); + debug("retrieved tags", revalidatedTags); + return revalidatedTags.length > 0; + }, + writeTags: async (tags: string[]) => { + try { + const { CACHE_DYNAMO_TABLE } = process.env; + if (globalThis.openNextConfig.dangerous?.disableTagCache) { + return; + } + const dataChunks = chunk(tags, MAX_DYNAMO_BATCH_WRITE_ITEM_COUNT).map( + (Items) => ({ + RequestItems: { + [CACHE_DYNAMO_TABLE ?? ""]: Items.map((tag) => ({ + PutRequest: { + Item: { + ...buildDynamoObject(tag), + }, + }, + })), + }, + }), + ); + const toInsert = chunk( + dataChunks, + getDynamoBatchWriteCommandConcurrency(), + ); + for (const paramsChunk of toInsert) { + await Promise.all( + paramsChunk.map(async (params) => { + const response = await awsFetch( + JSON.stringify(params), + "batchWrite", + ); + if (response.status !== 200) { + throw new RecoverableError( + `Failed to batch write dynamo item: ${response.status}`, + ); + } + return response; + }), + ); + } + } catch (e) { + error("Failed to batch write dynamo item", e); + } + }, +} satisfies NextModeTagCache; diff --git a/packages/open-next/src/overrides/tagCache/dummy.ts b/packages/open-next/src/overrides/tagCache/dummy.ts index 0428938ae..fc21e862c 100644 --- a/packages/open-next/src/overrides/tagCache/dummy.ts +++ b/packages/open-next/src/overrides/tagCache/dummy.ts @@ -3,6 +3,7 @@ import type { TagCache } from "types/overrides"; // We don't want to throw error on this one because we might use it when we don't need tag cache const dummyTagCache: TagCache = { name: "dummy", + mode: "original", getByPath: async () => { return []; }, diff --git a/packages/open-next/src/overrides/tagCache/dynamodb-lite.ts b/packages/open-next/src/overrides/tagCache/dynamodb-lite.ts index a08c2b375..490646afb 100644 --- a/packages/open-next/src/overrides/tagCache/dynamodb-lite.ts +++ b/packages/open-next/src/overrides/tagCache/dynamodb-lite.ts @@ -66,6 +66,7 @@ function buildDynamoObject(path: string, tags: string, revalidatedAt?: number) { } const tagCache: TagCache = { + mode: "original", async getByPath(path) { try { if (globalThis.openNextConfig.dangerous?.disableTagCache) { diff --git a/packages/open-next/src/overrides/tagCache/dynamodb.ts b/packages/open-next/src/overrides/tagCache/dynamodb.ts index 3df081d28..8b9406f87 100644 --- a/packages/open-next/src/overrides/tagCache/dynamodb.ts +++ b/packages/open-next/src/overrides/tagCache/dynamodb.ts @@ -42,6 +42,7 @@ function buildDynamoObject(path: string, tags: string, revalidatedAt?: number) { } const tagCache: TagCache = { + mode: "original", async getByPath(path) { try { if (globalThis.openNextConfig.dangerous?.disableTagCache) { diff --git a/packages/open-next/src/overrides/tagCache/fs-dev.ts b/packages/open-next/src/overrides/tagCache/fs-dev.ts index a6992d302..4de4d0a75 100644 --- a/packages/open-next/src/overrides/tagCache/fs-dev.ts +++ b/packages/open-next/src/overrides/tagCache/fs-dev.ts @@ -14,6 +14,7 @@ let tags = JSON.parse(tagContent) as { const tagCache: TagCache = { name: "fs-dev", + mode: "original", getByPath: async (path: string) => { return tags .filter((tagPathMapping) => tagPathMapping.path.S === path) diff --git a/packages/open-next/src/types/cache.ts b/packages/open-next/src/types/cache.ts index 2c01e122d..962819f78 100644 --- a/packages/open-next/src/types/cache.ts +++ b/packages/open-next/src/types/cache.ts @@ -70,9 +70,14 @@ export interface CacheHandlerValue { export type Extension = "cache" | "fetch"; +type MetaHeaders = { + "x-next-cache-tags"?: string; + [k: string]: string | string[] | undefined; +}; + export interface Meta { status?: number; - headers?: Record; + headers?: MetaHeaders; postponed?: string; } diff --git a/packages/open-next/src/types/open-next.ts b/packages/open-next/src/types/open-next.ts index 0a23d76ff..d4cf063fa 100644 --- a/packages/open-next/src/types/open-next.ts +++ b/packages/open-next/src/types/open-next.ts @@ -149,6 +149,7 @@ export type IncludedTagCache = | "dynamodb" | "dynamodb-lite" | "fs-dev" + | "ddb-nextMode" | "dummy"; export type IncludedImageLoader = "s3" | "host" | "fs-dev" | "dummy"; diff --git a/packages/open-next/src/types/overrides.ts b/packages/open-next/src/types/overrides.ts index 8a77e588a..db805c60d 100644 --- a/packages/open-next/src/types/overrides.ts +++ b/packages/open-next/src/types/overrides.ts @@ -96,16 +96,28 @@ export type IncrementalCache = { // Tag cache -export type TagCache = { +type BaseTagCache = { + name: string; +}; + +export type NextModeTagCache = BaseTagCache & { + mode: "nextMode"; + hasBeenRevalidated(tags: string[], lastModified?: number): Promise; + writeTags(tags: string[]): Promise; +}; + +export type OriginalTagCache = BaseTagCache & { + mode?: "original"; getByTag(tag: string): Promise; getByPath(path: string): Promise; getLastModified(path: string, lastModified?: number): Promise; writeTags( tags: { tag: string; path: string; revalidatedAt?: number }[], ): Promise; - name: string; }; +export type TagCache = NextModeTagCache | OriginalTagCache; + export type WrapperHandler< E extends BaseEventOrResult = InternalEvent, R extends BaseEventOrResult = InternalResult, diff --git a/packages/open-next/src/utils/cache.ts b/packages/open-next/src/utils/cache.ts new file mode 100644 index 000000000..4d113d343 --- /dev/null +++ b/packages/open-next/src/utils/cache.ts @@ -0,0 +1,36 @@ +import type { CacheValue } from "types/overrides"; + +export async function hasBeenRevalidated( + key: string, + tags: string[], + lastModified?: number, +): Promise { + if (globalThis.openNextConfig.dangerous?.disableTagCache) { + return false; + } + if (globalThis.tagCache.mode === "nextMode") { + const hasBeenRevalidated = await globalThis.tagCache.hasBeenRevalidated( + tags, + lastModified, + ); + return hasBeenRevalidated; + } + // TODO: refactor this, we should introduce a new method in the tagCache interface so that both implementations use hasBeenRevalidated + const _lastModified = await globalThis.tagCache.getLastModified( + key, + lastModified, + ); + return _lastModified === -1; +} + +export function getTagFromValue(value?: CacheValue) { + if (!value) { + return []; + } + // The try catch is necessary for older version of next.js that may fail on this + try { + return value.meta?.headers?.["x-next-cache-tags"]?.split(",") ?? []; + } catch (e) { + return []; + } +} From db344936f8ca6419c2cfac4ec7e4ea72915602db Mon Sep 17 00:00:00 2001 From: Dorseuil Nicolas Date: Fri, 31 Jan 2025 13:52:32 +0100 Subject: [PATCH 02/10] cleanup cache --- packages/open-next/src/adapters/cache.ts | 84 ++---------------------- packages/open-next/src/types/cache.ts | 26 ++++++-- 2 files changed, 27 insertions(+), 83 deletions(-) diff --git a/packages/open-next/src/adapters/cache.ts b/packages/open-next/src/adapters/cache.ts index 484b2e8ba..39d71390d 100644 --- a/packages/open-next/src/adapters/cache.ts +++ b/packages/open-next/src/adapters/cache.ts @@ -1,86 +1,12 @@ +import type { + CacheHandlerValue, + IncrementalCacheContext, + IncrementalCacheValue, +} from "types/cache"; import { isBinaryContentType } from "../utils/binary"; import { debug, error, warn } from "./logger"; import { getTagFromValue, hasBeenRevalidated } from "utils/cache"; -interface CachedFetchValue { - kind: "FETCH"; - data: { - headers: { [k: string]: string }; - body: string; - url: string; - status?: number; - tags?: string[]; - }; - revalidate: number; -} - -interface CachedRedirectValue { - kind: "REDIRECT"; - props: Object; -} - -interface CachedRouteValue { - kind: "ROUTE" | "APP_ROUTE"; - // this needs to be a RenderResult so since renderResponse - // expects that type instead of a string - body: Buffer; - status: number; - headers: Record; -} - -interface CachedImageValue { - kind: "IMAGE"; - etag: string; - buffer: Buffer; - extension: string; - isMiss?: boolean; - isStale?: boolean; -} - -interface IncrementalCachedPageValue { - kind: "PAGE" | "PAGES"; - // this needs to be a string since the cache expects to store - // the string value - html: string; - pageData: Object; - status?: number; - headers?: Record; -} - -interface IncrementalCachedAppPageValue { - kind: "APP_PAGE"; - // this needs to be a string since the cache expects to store - // the string value - html: string; - rscData: Buffer; - headers?: Record; - postponed?: string; - status?: number; -} - -type IncrementalCacheValue = - | CachedRedirectValue - | IncrementalCachedPageValue - | IncrementalCachedAppPageValue - | CachedImageValue - | CachedFetchValue - | CachedRouteValue; - -type IncrementalCacheContext = { - revalidate?: number | false | undefined; - fetchCache?: boolean | undefined; - fetchUrl?: string | undefined; - fetchIdx?: number | undefined; - tags?: string[] | undefined; -}; - -interface CacheHandlerValue { - lastModified?: number; - age?: number; - cacheState?: string; - value: IncrementalCacheValue | null; -} - function isFetchCache( options?: | boolean diff --git a/packages/open-next/src/types/cache.ts b/packages/open-next/src/types/cache.ts index 962819f78..6a6a36bd8 100644 --- a/packages/open-next/src/types/cache.ts +++ b/packages/open-next/src/types/cache.ts @@ -16,7 +16,7 @@ interface CachedRedirectValue { } interface CachedRouteValue { - kind: "ROUTE"; + kind: "ROUTE" | "APP_ROUTE"; // this needs to be a RenderResult so since renderResponse // expects that type instead of a string body: Buffer; @@ -34,7 +34,7 @@ interface CachedImageValue { } interface IncrementalCachedPageValue { - kind: "PAGE"; + kind: "PAGE" | "PAGES"; // this needs to be a string since the cache expects to store // the string value html: string; @@ -43,9 +43,21 @@ interface IncrementalCachedPageValue { headers?: Record; } -type IncrementalCacheValue = +interface IncrementalCachedAppPageValue { + kind: "APP_PAGE"; + // this needs to be a string since the cache expects to store + // the string value + html: string; + rscData: Buffer; + headers?: Record; + postponed?: string; + status?: number; +} + +export type IncrementalCacheValue = | CachedRedirectValue | IncrementalCachedPageValue + | IncrementalCachedAppPageValue | CachedImageValue | CachedFetchValue | CachedRouteValue; @@ -60,7 +72,6 @@ export interface CacheHandlerContext { _requestHeaders: never; fetchCacheKeyPrefix?: string; } - export interface CacheHandlerValue { lastModified?: number; age?: number; @@ -86,3 +97,10 @@ export type TagCacheMetaFile = { path: { S: string }; revalidatedAt: { N: string }; }; +export type IncrementalCacheContext = { + revalidate?: number | false | undefined; + fetchCache?: boolean | undefined; + fetchUrl?: string | undefined; + fetchIdx?: number | undefined; + tags?: string[] | undefined; +}; From 2427ad85d3b2c573ca2a5b1c7db52d8b21d52648 Mon Sep 17 00:00:00 2001 From: Dorseuil Nicolas Date: Fri, 31 Jan 2025 14:09:35 +0100 Subject: [PATCH 03/10] add automatic invalidation for nextMode tag cache --- packages/open-next/src/adapters/cache.ts | 22 ++++++++++++++++++- .../src/overrides/tagCache/ddb-nextMode.ts | 1 + packages/open-next/src/types/overrides.ts | 3 +++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/packages/open-next/src/adapters/cache.ts b/packages/open-next/src/adapters/cache.ts index 39d71390d..f7eefc0f6 100644 --- a/packages/open-next/src/adapters/cache.ts +++ b/packages/open-next/src/adapters/cache.ts @@ -322,7 +322,27 @@ export default class Cache { try { const _tags = Array.isArray(tags) ? tags : [tags]; if (globalThis.tagCache.mode === "nextMode") { - return globalThis.tagCache.writeTags(_tags); + const paths = (await globalThis.tagCache.getPathsByTags?.(_tags)) ?? []; + + await globalThis.tagCache.writeTags(_tags); + if (paths.length > 0) { + // TODO: we should introduce a new method in cdnInvalidationHandler to invalidate paths by tags for cdn that supports it + // It also means that we'll need to provide the tags used in every request to the wrapper or converter. + await globalThis.cdnInvalidationHandler.invalidatePaths( + paths.map((path) => ({ + initialPath: path, + rawPath: path, + resolvedRoutes: [ + { + route: path, + // TODO: ideally here we should check if it's an app router page or route + type: "app", + }, + ], + })), + ); + } + return; } for (const tag of _tags) { debug("revalidateTag", tag); diff --git a/packages/open-next/src/overrides/tagCache/ddb-nextMode.ts b/packages/open-next/src/overrides/tagCache/ddb-nextMode.ts index 87b597569..8b4323edc 100644 --- a/packages/open-next/src/overrides/tagCache/ddb-nextMode.ts +++ b/packages/open-next/src/overrides/tagCache/ddb-nextMode.ts @@ -67,6 +67,7 @@ function buildDynamoObject(tag: string, revalidatedAt?: number) { }; } +// This implementation does not support automatic invalidation of paths by the cdn export default { name: "ddb-nextMode", mode: "nextMode", diff --git a/packages/open-next/src/types/overrides.ts b/packages/open-next/src/types/overrides.ts index db805c60d..b9128247b 100644 --- a/packages/open-next/src/types/overrides.ts +++ b/packages/open-next/src/types/overrides.ts @@ -104,6 +104,9 @@ export type NextModeTagCache = BaseTagCache & { mode: "nextMode"; hasBeenRevalidated(tags: string[], lastModified?: number): Promise; writeTags(tags: string[]): Promise; + // Optional method to get paths by tags + // It is used to automatically invalidate paths in the CDN + getPathsByTags?: (tags: string[]) => Promise; }; export type OriginalTagCache = BaseTagCache & { From 4b531afd9ce6dc8d885986bf8c29747675d611b1 Mon Sep 17 00:00:00 2001 From: Dorseuil Nicolas Date: Fri, 31 Jan 2025 14:39:01 +0100 Subject: [PATCH 04/10] updated test --- .../tests-unit/tests/adapters/cache.test.ts | 65 +++++++++++++++++++ .../core/routing/cacheInterceptor.test.ts | 6 ++ 2 files changed, 71 insertions(+) diff --git a/packages/tests-unit/tests/adapters/cache.test.ts b/packages/tests-unit/tests/adapters/cache.test.ts index 14e9fdb32..eb7dafae3 100644 --- a/packages/tests-unit/tests/adapters/cache.test.ts +++ b/packages/tests-unit/tests/adapters/cache.test.ts @@ -32,6 +32,8 @@ describe("CacheHandler", () => { const tagCache = { name: "mock", + mode: "original", + hasBeenRevalidated: vi.fn(), getByTag: vi.fn(), getByPath: vi.fn(), getLastModified: vi @@ -140,6 +142,17 @@ describe("CacheHandler", () => { expect(result).toBeNull(); }); + it("Should return null with nextMode tag cache that has been revalidated", async () => { + tagCache.mode = "nextMode"; + tagCache.hasBeenRevalidated.mockResolvedValueOnce(true); + + const result = await cache.get("key", { kind: "FETCH" }); + expect(getFetchCacheSpy).toHaveBeenCalled(); + expect(result).toBeNull(); + // Reset the tagCache mode + tagCache.mode = "original"; + }); + it("Should return null when incremental cache throws", async () => { incrementalCache.get.mockRejectedValueOnce( new Error("Error retrieving cache"), @@ -178,6 +191,24 @@ describe("CacheHandler", () => { expect(result).toBeNull(); }); + it("Should return null with nextMode tag cache that has been revalidated", async () => { + tagCache.mode = "nextMode"; + tagCache.hasBeenRevalidated.mockResolvedValueOnce(true); + incrementalCache.get.mockResolvedValueOnce({ + value: { + type: "route", + }, + lastModified: Date.now(), + }); + + const result = await cache.get("key", { kindHint: "app" }); + + expect(getIncrementalCache).toHaveBeenCalled(); + expect(result).toBeNull(); + // Reset the tagCache mode + tagCache.mode = "original"; + }); + it("Should return value when cache data type is route", async () => { incrementalCache.get.mockResolvedValueOnce({ value: { @@ -546,5 +577,39 @@ describe("CacheHandler", () => { expect(invalidateCdnHandler.invalidatePaths).not.toHaveBeenCalled(); }); + + it("Should only call writeTags for nextMode", async () => { + globalThis.tagCache.mode = "nextMode"; + await cache.revalidateTag(["tag1", "tag2"]); + + expect(tagCache.writeTags).toHaveBeenCalledTimes(1); + expect(tagCache.writeTags).toHaveBeenCalledWith(["tag1", "tag2"]); + expect(invalidateCdnHandler.invalidatePaths).not.toHaveBeenCalled(); + }); + + it("Should call writeTags and invalidateCdnHandler.invalidatePaths for nextMode that supports getPathsByTags", async () => { + globalThis.tagCache.mode = "nextMode"; + globalThis.tagCache.getPathsByTags = vi + .fn() + .mockResolvedValueOnce(["/path"]); + await cache.revalidateTag("tag"); + + expect(tagCache.writeTags).toHaveBeenCalledTimes(1); + expect(tagCache.writeTags).toHaveBeenCalledWith(["tag"]); + expect(invalidateCdnHandler.invalidatePaths).toHaveBeenCalledWith([ + { + initialPath: "/path", + rawPath: "/path", + resolvedRoutes: [ + { + type: "app", + route: "/path", + }, + ], + }, + ]); + // Reset the getPathsByTags + globalThis.tagCache.getPathsByTags = undefined; + }); }); }); diff --git a/packages/tests-unit/tests/core/routing/cacheInterceptor.test.ts b/packages/tests-unit/tests/core/routing/cacheInterceptor.test.ts index f460e7bc3..e75288e0a 100644 --- a/packages/tests-unit/tests/core/routing/cacheInterceptor.test.ts +++ b/packages/tests-unit/tests/core/routing/cacheInterceptor.test.ts @@ -81,6 +81,12 @@ globalThis.queue = queue; beforeEach(() => { vi.useFakeTimers().setSystemTime("2024-01-02T00:00:00Z"); vi.clearAllMocks(); + globalThis.openNextConfig = { + dangerous: { + disableTagCache: false, + disableIncrementalCache: false, + }, + }; }); describe("cacheInterceptor", () => { From f28136d7a9ffed00069665cd9fd28b73de2e6108 Mon Sep 17 00:00:00 2001 From: Dorseuil Nicolas Date: Fri, 31 Jan 2025 14:52:19 +0100 Subject: [PATCH 05/10] changeset --- .changeset/silver-pets-care.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/silver-pets-care.md diff --git a/.changeset/silver-pets-care.md b/.changeset/silver-pets-care.md new file mode 100644 index 000000000..9df230028 --- /dev/null +++ b/.changeset/silver-pets-care.md @@ -0,0 +1,5 @@ +--- +"@opennextjs/aws": minor +--- + +introduce a new optional mode for the tag cache From a7be25760448813162cb9beb1003bbf6b5b9df3f Mon Sep 17 00:00:00 2001 From: Dorseuil Nicolas Date: Fri, 31 Jan 2025 15:04:58 +0100 Subject: [PATCH 06/10] fix linting --- packages/open-next/src/adapters/cache.ts | 2 +- packages/open-next/src/core/routing/cacheInterceptor.ts | 2 +- packages/open-next/src/overrides/tagCache/ddb-nextMode.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/open-next/src/adapters/cache.ts b/packages/open-next/src/adapters/cache.ts index f7eefc0f6..112a0a64c 100644 --- a/packages/open-next/src/adapters/cache.ts +++ b/packages/open-next/src/adapters/cache.ts @@ -3,9 +3,9 @@ import type { IncrementalCacheContext, IncrementalCacheValue, } from "types/cache"; +import { getTagFromValue, hasBeenRevalidated } from "utils/cache"; import { isBinaryContentType } from "../utils/binary"; import { debug, error, warn } from "./logger"; -import { getTagFromValue, hasBeenRevalidated } from "utils/cache"; function isFetchCache( options?: diff --git a/packages/open-next/src/core/routing/cacheInterceptor.ts b/packages/open-next/src/core/routing/cacheInterceptor.ts index b8794945f..0b75c599b 100644 --- a/packages/open-next/src/core/routing/cacheInterceptor.ts +++ b/packages/open-next/src/core/routing/cacheInterceptor.ts @@ -5,10 +5,10 @@ import type { InternalEvent, InternalResult } from "types/open-next"; import type { CacheValue } from "types/overrides"; import { emptyReadableStream, toReadableStream } from "utils/stream"; +import { getTagFromValue, hasBeenRevalidated } from "utils/cache"; import { debug } from "../../adapters/logger"; import { localizePath } from "./i18n"; import { generateMessageGroupId } from "./queue"; -import { getTagFromValue, hasBeenRevalidated } from "utils/cache"; const CACHE_ONE_YEAR = 60 * 60 * 24 * 365; const CACHE_ONE_MONTH = 60 * 60 * 24 * 30; diff --git a/packages/open-next/src/overrides/tagCache/ddb-nextMode.ts b/packages/open-next/src/overrides/tagCache/ddb-nextMode.ts index 8b4323edc..2b78b2182 100644 --- a/packages/open-next/src/overrides/tagCache/ddb-nextMode.ts +++ b/packages/open-next/src/overrides/tagCache/ddb-nextMode.ts @@ -4,13 +4,13 @@ import { AwsClient } from "aws4fetch"; import { RecoverableError } from "utils/error"; import { customFetchClient } from "utils/fetch"; +import path from "node:path"; import { debug, error } from "../../adapters/logger"; import { chunk, parseNumberFromEnv } from "../../adapters/util"; import { MAX_DYNAMO_BATCH_WRITE_ITEM_COUNT, getDynamoBatchWriteCommandConcurrency, } from "./constants"; -import path from "node:path"; let awsClient: AwsClient | null = null; From 6324a23bb1aa6f7ae9330194c991f21dfa8ed273 Mon Sep 17 00:00:00 2001 From: Dorseuil Nicolas Date: Thu, 6 Feb 2025 19:28:00 +0100 Subject: [PATCH 07/10] review fix --- packages/open-next/src/adapters/cache.ts | 10 ++++- .../src/core/routing/cacheInterceptor.ts | 4 +- .../src/overrides/tagCache/constants.ts | 1 - .../{ddb-nextMode.ts => dynamodb-nextMode.ts} | 5 +++ packages/open-next/src/types/open-next.ts | 2 +- packages/open-next/src/types/overrides.ts | 40 +++++++++++++++++++ packages/open-next/src/utils/cache.ts | 8 +--- 7 files changed, 58 insertions(+), 12 deletions(-) rename packages/open-next/src/overrides/tagCache/{ddb-nextMode.ts => dynamodb-nextMode.ts} (95%) diff --git a/packages/open-next/src/adapters/cache.ts b/packages/open-next/src/adapters/cache.ts index 112a0a64c..67f63f95b 100644 --- a/packages/open-next/src/adapters/cache.ts +++ b/packages/open-next/src/adapters/cache.ts @@ -3,7 +3,7 @@ import type { IncrementalCacheContext, IncrementalCacheValue, } from "types/cache"; -import { getTagFromValue, hasBeenRevalidated } from "utils/cache"; +import { getTagsFromValue, hasBeenRevalidated } from "utils/cache"; import { isBinaryContentType } from "../utils/binary"; import { debug, error, warn } from "./logger"; @@ -116,7 +116,7 @@ export default class Cache { const cacheData = cachedEntry?.value; const meta = cacheData?.meta; - const tags = getTagFromValue(cacheData); + const tags = getTagsFromValue(cacheData); const _lastModified = cachedEntry.lastModified ?? Date.now(); const _hasBeenRevalidated = await hasBeenRevalidated( key, @@ -295,6 +295,12 @@ export default class Cache { break; } } + if ( + globalThis.openNextConfig.dangerous?.disableTagCache || + globalThis.tagCache.mode === "nextMode" + ) { + return; + } // Write derivedTags to dynamodb // If we use an in house version of getDerivedTags in build we should use it here instead of next's one const derivedTags: string[] = diff --git a/packages/open-next/src/core/routing/cacheInterceptor.ts b/packages/open-next/src/core/routing/cacheInterceptor.ts index 0b75c599b..d645877c9 100644 --- a/packages/open-next/src/core/routing/cacheInterceptor.ts +++ b/packages/open-next/src/core/routing/cacheInterceptor.ts @@ -5,7 +5,7 @@ import type { InternalEvent, InternalResult } from "types/open-next"; import type { CacheValue } from "types/overrides"; import { emptyReadableStream, toReadableStream } from "utils/stream"; -import { getTagFromValue, hasBeenRevalidated } from "utils/cache"; +import { getTagsFromValue, hasBeenRevalidated } from "utils/cache"; import { debug } from "../../adapters/logger"; import { localizePath } from "./i18n"; import { generateMessageGroupId } from "./queue"; @@ -164,7 +164,7 @@ export async function cacheInterceptor( } // We need to check the tag cache now if (cachedData.value?.type === "app") { - const tags = getTagFromValue(cachedData.value); + const tags = getTagsFromValue(cachedData.value); const _hasBeenRevalidated = await hasBeenRevalidated( localizedPath, tags, diff --git a/packages/open-next/src/overrides/tagCache/constants.ts b/packages/open-next/src/overrides/tagCache/constants.ts index c3e9f82fb..c94261b34 100644 --- a/packages/open-next/src/overrides/tagCache/constants.ts +++ b/packages/open-next/src/overrides/tagCache/constants.ts @@ -1,5 +1,4 @@ export const MAX_DYNAMO_BATCH_WRITE_ITEM_COUNT = 25; -export const MAX_DYNAMO_BATCH_GET_ITEM_COUNT = 100; /** * Sending to dynamo X commands at a time, using about X * 25 write units per batch to not overwhelm DDB diff --git a/packages/open-next/src/overrides/tagCache/ddb-nextMode.ts b/packages/open-next/src/overrides/tagCache/dynamodb-nextMode.ts similarity index 95% rename from packages/open-next/src/overrides/tagCache/ddb-nextMode.ts rename to packages/open-next/src/overrides/tagCache/dynamodb-nextMode.ts index 2b78b2182..7d16bebfc 100644 --- a/packages/open-next/src/overrides/tagCache/ddb-nextMode.ts +++ b/packages/open-next/src/overrides/tagCache/dynamodb-nextMode.ts @@ -75,6 +75,11 @@ export default { if (globalThis.openNextConfig.dangerous?.disableTagCache) { return false; } + if (tags.length > 100) { + throw new RecoverableError( + "Cannot query more than 100 tags at once. You should not be using this tagCache implementation for this amount of tags", + ); + } const { CACHE_DYNAMO_TABLE } = process.env; // It's unlikely that we will have more than 100 items to query // If that's the case, you should not use this tagCache implementation diff --git a/packages/open-next/src/types/open-next.ts b/packages/open-next/src/types/open-next.ts index d4cf063fa..5fa45d85d 100644 --- a/packages/open-next/src/types/open-next.ts +++ b/packages/open-next/src/types/open-next.ts @@ -148,8 +148,8 @@ export type IncludedIncrementalCache = export type IncludedTagCache = | "dynamodb" | "dynamodb-lite" + | "dynamodb-nextMode" | "fs-dev" - | "ddb-nextMode" | "dummy"; export type IncludedImageLoader = "s3" | "host" | "fs-dev" | "dummy"; diff --git a/packages/open-next/src/types/overrides.ts b/packages/open-next/src/types/overrides.ts index b9128247b..388448be0 100644 --- a/packages/open-next/src/types/overrides.ts +++ b/packages/open-next/src/types/overrides.ts @@ -100,6 +100,27 @@ type BaseTagCache = { name: string; }; +/** + * On get : +We have to check for every tag (after reading the incremental cache) that they have not been revalidated. + +In DynamoDB, this would require 1 GetItem per tag (including internal one), more realistically 1 BatchGetItem per get (In terms of pricing, it would be billed as multiple single GetItem) + +On set : +We don't have to do anything here + +On revalidateTag for each tag : +We have to update a single entry for this tag + +Pros : +- No need to prepopulate DDB +- Very little write + +Cons : +- Might be slower on read +- One page request (i.e. GET request) could require to check a lot of tags (And some of them multiple time when used with the fetch cache) +- Almost impossible to do automatic cdn revalidation by itself +*/ export type NextModeTagCache = BaseTagCache & { mode: "nextMode"; hasBeenRevalidated(tags: string[], lastModified?: number): Promise; @@ -109,6 +130,25 @@ export type NextModeTagCache = BaseTagCache & { getPathsByTags?: (tags: string[]) => Promise; }; +/** + * On get : +We just check for the cache key in the tag cache. If it has been revalidated we just return null, otherwise we continue + +On set : +We have to write both the incremental cache and check the tag cache for non existing tag/key combination. For non existing tag/key combination, we have to add them + +On revalidateTag for each tag : +We have to update every possible combination for the requested tag + +Pros : +- Very fast on read +- Only one query per get (On DynamoDB it's a lot cheaper) +- Can allow for automatic cdn invalidation on revalidateTag + +Cons : +- Lots of write on set and revalidateTag +- Needs to be prepopulated at build time to work properly + */ export type OriginalTagCache = BaseTagCache & { mode?: "original"; getByTag(tag: string): Promise; diff --git a/packages/open-next/src/utils/cache.ts b/packages/open-next/src/utils/cache.ts index 4d113d343..539b6fd1e 100644 --- a/packages/open-next/src/utils/cache.ts +++ b/packages/open-next/src/utils/cache.ts @@ -9,11 +9,7 @@ export async function hasBeenRevalidated( return false; } if (globalThis.tagCache.mode === "nextMode") { - const hasBeenRevalidated = await globalThis.tagCache.hasBeenRevalidated( - tags, - lastModified, - ); - return hasBeenRevalidated; + return await globalThis.tagCache.hasBeenRevalidated(tags, lastModified); } // TODO: refactor this, we should introduce a new method in the tagCache interface so that both implementations use hasBeenRevalidated const _lastModified = await globalThis.tagCache.getLastModified( @@ -23,7 +19,7 @@ export async function hasBeenRevalidated( return _lastModified === -1; } -export function getTagFromValue(value?: CacheValue) { +export function getTagsFromValue(value?: CacheValue) { if (!value) { return []; } From 6ee31219e0c348db7aba2670cc28554ea6b6cbf0 Mon Sep 17 00:00:00 2001 From: Dorseuil Nicolas Date: Thu, 20 Feb 2025 10:54:35 +0100 Subject: [PATCH 08/10] don't check tag cache for page router --- packages/open-next/src/adapters/cache.ts | 6 +++--- .../open-next/src/core/routing/cacheInterceptor.ts | 2 +- packages/open-next/src/types/cache.ts | 1 + packages/open-next/src/utils/cache.ts | 13 +++++++++++-- 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/packages/open-next/src/adapters/cache.ts b/packages/open-next/src/adapters/cache.ts index 67f63f95b..8b680bafc 100644 --- a/packages/open-next/src/adapters/cache.ts +++ b/packages/open-next/src/adapters/cache.ts @@ -66,7 +66,7 @@ export default class Cache { const _hasBeenRevalidated = await hasBeenRevalidated( key, _tags, - cachedEntry?.lastModified, + cachedEntry, ); if (_hasBeenRevalidated) return null; @@ -85,7 +85,7 @@ export default class Cache { const hasPathBeenUpdated = await hasBeenRevalidated( path.replace("_N_T_/", ""), [], - cachedEntry.lastModified, + cachedEntry, ); if (hasPathBeenUpdated) { // In case the path has been revalidated, we don't want to use the fetch cache @@ -121,7 +121,7 @@ export default class Cache { const _hasBeenRevalidated = await hasBeenRevalidated( key, tags, - _lastModified, + cachedEntry, ); if (cacheData === undefined || _hasBeenRevalidated) return null; diff --git a/packages/open-next/src/core/routing/cacheInterceptor.ts b/packages/open-next/src/core/routing/cacheInterceptor.ts index d645877c9..5c1bdce85 100644 --- a/packages/open-next/src/core/routing/cacheInterceptor.ts +++ b/packages/open-next/src/core/routing/cacheInterceptor.ts @@ -168,7 +168,7 @@ export async function cacheInterceptor( const _hasBeenRevalidated = await hasBeenRevalidated( localizedPath, tags, - cachedData.lastModified, + cachedData, ); if (_hasBeenRevalidated) { return event; diff --git a/packages/open-next/src/types/cache.ts b/packages/open-next/src/types/cache.ts index 6a6a36bd8..b37997f38 100644 --- a/packages/open-next/src/types/cache.ts +++ b/packages/open-next/src/types/cache.ts @@ -97,6 +97,7 @@ export type TagCacheMetaFile = { path: { S: string }; revalidatedAt: { N: string }; }; + export type IncrementalCacheContext = { revalidate?: number | false | undefined; fetchCache?: boolean | undefined; diff --git a/packages/open-next/src/utils/cache.ts b/packages/open-next/src/utils/cache.ts index 539b6fd1e..98c028004 100644 --- a/packages/open-next/src/utils/cache.ts +++ b/packages/open-next/src/utils/cache.ts @@ -1,13 +1,22 @@ -import type { CacheValue } from "types/overrides"; +import type { CacheValue, WithLastModified } from "types/overrides"; export async function hasBeenRevalidated( key: string, tags: string[], - lastModified?: number, + cacheEntry: WithLastModified>, ): Promise { if (globalThis.openNextConfig.dangerous?.disableTagCache) { return false; } + const value = cacheEntry.value; + if (!value) { + // We should never reach this point + return true; + } + if ("type" in cacheEntry && cacheEntry.type === "page") { + return false; + } + const lastModified = cacheEntry.lastModified ?? Date.now(); if (globalThis.tagCache.mode === "nextMode") { return await globalThis.tagCache.hasBeenRevalidated(tags, lastModified); } From a08a093d5689588ec44e0914128299cb49e4d6ea Mon Sep 17 00:00:00 2001 From: Dorseuil Nicolas Date: Fri, 21 Feb 2025 11:00:49 +0100 Subject: [PATCH 09/10] refactor updateOnTagsSet --- packages/open-next/src/adapters/cache.ts | 35 ++++++++++++------------ 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/packages/open-next/src/adapters/cache.ts b/packages/open-next/src/adapters/cache.ts index 8b680bafc..67c57e859 100644 --- a/packages/open-next/src/adapters/cache.ts +++ b/packages/open-next/src/adapters/cache.ts @@ -295,22 +295,8 @@ export default class Cache { break; } } - if ( - globalThis.openNextConfig.dangerous?.disableTagCache || - globalThis.tagCache.mode === "nextMode" - ) { - return; - } - // Write derivedTags to dynamodb - // If we use an in house version of getDerivedTags in build we should use it here instead of next's one - const derivedTags: string[] = - data?.kind === "FETCH" - ? (ctx?.tags ?? data?.data?.tags ?? []) // before version 14 next.js used data?.data?.tags so we keep it for backward compatibility - : data?.kind === "PAGE" - ? (data.headers?.["x-next-cache-tags"]?.split(",") ?? []) - : []; - debug("derivedTags", derivedTags); - await this.updateTagsOnSet(key, derivedTags); + + await this.updateTagsOnSet(key, data, ctx); debug("Finished setting cache"); } catch (e) { error("Failed to set cache", e); @@ -414,13 +400,28 @@ export default class Cache { } } - private async updateTagsOnSet(key: string, derivedTags: string[]) { + private async updateTagsOnSet( + key: string, + data?: IncrementalCacheValue, + ctx?: IncrementalCacheContext, + ) { if ( globalThis.openNextConfig.dangerous?.disableTagCache || globalThis.tagCache.mode === "nextMode" ) { return; } + + // Write derivedTags to the tag cache + // If we use an in house version of getDerivedTags in build we should use it here instead of next's one + const derivedTags: string[] = + data?.kind === "FETCH" + ? (ctx?.tags ?? data?.data?.tags ?? []) // before version 14 next.js used data?.data?.tags so we keep it for backward compatibility + : data?.kind === "PAGE" + ? (data.headers?.["x-next-cache-tags"]?.split(",") ?? []) + : []; + debug("derivedTags", derivedTags); + // Get all tags stored in dynamodb for the given key // If any of the derived tags are not stored in dynamodb for the given key, write them const storedTags = await globalThis.tagCache.getByPath(key); From 4eef3a79631b56305a03d21b1ee51074f99e4e9a Mon Sep 17 00:00:00 2001 From: Dorseuil Nicolas Date: Fri, 21 Feb 2025 11:40:11 +0100 Subject: [PATCH 10/10] review fix --- packages/open-next/src/adapters/cache.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/open-next/src/adapters/cache.ts b/packages/open-next/src/adapters/cache.ts index 67c57e859..9a5719433 100644 --- a/packages/open-next/src/adapters/cache.ts +++ b/packages/open-next/src/adapters/cache.ts @@ -113,9 +113,9 @@ export default class Cache { return null; } - const cacheData = cachedEntry?.value; + const cacheData = cachedEntry.value; - const meta = cacheData?.meta; + const meta = cacheData.meta; const tags = getTagsFromValue(cacheData); const _lastModified = cachedEntry.lastModified ?? Date.now(); const _hasBeenRevalidated = await hasBeenRevalidated( @@ -123,7 +123,7 @@ export default class Cache { tags, cachedEntry, ); - if (cacheData === undefined || _hasBeenRevalidated) return null; + if (_hasBeenRevalidated) return null; const store = globalThis.__openNextAls.getStore(); if (store) { @@ -400,6 +400,8 @@ export default class Cache { } } + // TODO: We should delete/update tags in this method + // This will require an update to the tag cache interface private async updateTagsOnSet( key: string, data?: IncrementalCacheValue, @@ -407,11 +409,12 @@ export default class Cache { ) { if ( globalThis.openNextConfig.dangerous?.disableTagCache || - globalThis.tagCache.mode === "nextMode" + globalThis.tagCache.mode === "nextMode" || + // Here it means it's a delete + !data ) { return; } - // Write derivedTags to the tag cache // If we use an in house version of getDerivedTags in build we should use it here instead of next's one const derivedTags: string[] =