From 623c576c8afb90bd4e815f4d1bae51af5d28ed1c Mon Sep 17 00:00:00 2001 From: flakey5 <73616808+flakey5@users.noreply.github.com> Date: Sat, 24 Oct 2026 23:02:20 -0700 Subject: [PATCH 1/3] src, scripts: better directory cache Changes the way the KV directory cache works: `scripts/handle-mainline-release.mjs` is triggered by the update links action. Updates the cache entries for the directories that have symlinks. Doesn't add the cache entry for the new released version anymore. When KV listings are enabled, we try fetching the directory from the KV directory cache like usual. If the directory isn't found however, then we fall back to R2. If R2 has the directory, then we return that result and then cache the R2 result in the KV cache for however long `CACHE_KV_TTL` is (currently an arbitary 6 hours). Fixes #1034 Signed-off-by: flakey5 <73616808+flakey5@users.noreply.github.com> --- .env.example | 14 +- .github/workflows/build-directory-cache.yaml | 82 ------ .github/workflows/update-links.yml | 7 +- scripts/constants.mjs | 9 +- ...-cache.mjs => handle-mainline-release.mjs} | 33 +-- scripts/utils/addSymlinksToDirectoryCache.mjs | 8 +- src/constants/cache.ts | 5 + src/middleware/r2Middleware.ts | 46 ++- src/providers/kvProvider.test.ts | 272 +++++++++++++----- src/providers/kvProvider.ts | 26 +- src/providers/r2Provider.ts | 17 -- 11 files changed, 290 insertions(+), 229 deletions(-) delete mode 100644 .github/workflows/build-directory-cache.yaml rename scripts/{update-directory-cache.mjs => handle-mainline-release.mjs} (73%) diff --git a/.env.example b/.env.example index 06d77ed6..a249715f 100644 --- a/.env.example +++ b/.env.example @@ -1,30 +1,30 @@ # Your Cloudflare account tag. -# +# # Needed for: # - Directory cache scripts CLOUDFLARE_ACCOUNT_ID= # Cloudflare V4 API token. -# +# # Needed for: # - Directory cache scripts -# +# # Required permissions: # - `Workers KV Storage`: Edit # - `Workers R2 Storage`: Read -# +# # See https://developers.cloudflare.com/fundamentals/api/get-started/create-token/ CLOUDFLARE_API_TOKEN= # S3 credentials for your R2 bucket. -# +# # Needed for: # - Directory listings in the worker. # - Directory cache scripts -# +# # Required permissions: # - `Object Read Only` -# +# # See https://dash.cloudflare.com/?account=/r2/api-tokens S3_ACCESS_KEY_ID= S3_ACCESS_KEY_SECRET= diff --git a/.github/workflows/build-directory-cache.yaml b/.github/workflows/build-directory-cache.yaml deleted file mode 100644 index 3685f270..00000000 --- a/.github/workflows/build-directory-cache.yaml +++ /dev/null @@ -1,82 +0,0 @@ -name: Build Directory Cache - -on: - workflow_dispatch: - -permissions: - contents: write # needed by stefanzweifel/git-auto-commit-action - -jobs: - build-directory-cache: - name: Build Directory Cache - runs-on: ubuntu-latest - - steps: - - uses: step-security/harden-runner@e14015d583714f6e62063499dc959a02595150a1 # v2.21.1 - with: - egress-policy: audit - - - name: Git Checkout - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - persist-credentials: true # needed by stefanzweifel/git-auto-commit-action - - - name: Cache Dependencies - uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 - with: - path: | - ~/.npm - node_modules/.cache - key: ${{ runner.os }}-npm-${{ hashFiles('**/workflows/format.yml') }} - restore-keys: ${{ runner.os }}-npm- - - - name: Setup Node - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 - with: - node-version: lts/* - cache: 'npm' - - - name: Install dependencies - run: npm ci && npm update nodejs-latest-linker --save - - - name: Build Directory Cache - run: node scripts/build-directory-cache.mjs && node --run format - env: - CLOUDFLARE_API_TOKEN: ${{ secrets.CF_API_TOKEN }} - S3_ACCESS_KEY_ID: ${{ secrets.CF_ACCESS_KEY_ID }} - S3_ACCESS_KEY_SECRET: ${{ secrets.CF_SECRET_ACCESS_KEY }} - - - name: Commit Changes - id: git_auto_commit - uses: stefanzweifel/git-auto-commit-action@4a55954c782fc1ea30b9056cd3e7a2b40ca8887d # v7.2.0 - with: - commit_options: '--no-verify --no-signoff' - commit_message: 'chore: update redirect links' - branch: build-directory-cache - create_branch: true - - - name: Open and Merge Pull Request - if: steps.git_auto_commit.outputs.changes_detected == 'true' - run: | - gh pr create --fill - gh pr merge --squash --delete-branch --admin - env: - GITHUB_TOKEN: ${{ secrets.GH_BOT_TOKEN }} - - - name: Deploy to Production - if: steps.git_auto_commit.outputs.changes_detected == 'true' - run: | - gh workflow run deploy.yml - env: - GITHUB_TOKEN: ${{ secrets.GH_BOT_TOKEN }} - - - name: Alert on Failure - if: failure() && github.repository == 'nodejs/release-cloudflare-worker' - uses: rtCamp/action-slack-notify@33ca3be66c6f378fe1610fd1d5258632dbed5e58 # 2.4.0 - env: - SLACK_COLOR: '#DE512A' - SLACK_ICON: https://github.com/nodejs.png?size=48 - SLACK_TITLE: Build Directory Cache failed (${{ github.ref }}) - SLACK_MESSAGE: The `build-directory-cache.yaml` action has failed. - SLACK_USERNAME: nodejs-bot - SLACK_WEBHOOK: ${{ secrets.SLACK_WEBHOOK }} diff --git a/.github/workflows/update-links.yml b/.github/workflows/update-links.yml index b3d87f7f..f919bf73 100644 --- a/.github/workflows/update-links.yml +++ b/.github/workflows/update-links.yml @@ -7,12 +7,11 @@ on: # Triggered by https://github.com/nodejs/node/blob/main/.github/workflows/update-release-links.yml workflow_dispatch: inputs: + # Not used but kept around to not break Node core ci since it passes this version: description: 'Node.js version (ex/ `v20.0.0`)' - required: true + required: false type: string - schedule: - - cron: '0 0 * * *' concurrency: group: update-redirect-links @@ -60,7 +59,7 @@ jobs: run: npm ci && npm update nodejs-latest-linker --save - name: Update Directory Cache - run: node scripts/update-directory-cache.mjs "$VERSION_INPUT" && node --run format + run: node scripts/handle-mainline-release.mjs && node --run format env: VERSION_INPUT: "${{ inputs.version || '' }}" CLOUDFLARE_API_TOKEN: ${{ secrets.CF_API_TOKEN }} diff --git a/scripts/constants.mjs b/scripts/constants.mjs index bea9ac9a..73b45ab0 100644 --- a/scripts/constants.mjs +++ b/scripts/constants.mjs @@ -61,11 +61,4 @@ export const STATIC_FILE_SYMLINKS_PATH = join( * Paths in the R2 bucket that we should always be updating whenever a new * version releases. */ -export const ALWAYS_UPDATED_PATHS = [ - RELEASE_DIR, - DOCS_DIR, - NIGHTLY_RELEASES_DIR, - RC_RELEASES_DIR, - TEST_RELEASES_DIR, - V8_CANARY_RELEASES_DIR, -]; +export const ALWAYS_UPDATED_PATHS = [RELEASE_DIR, DOCS_DIR]; diff --git a/scripts/update-directory-cache.mjs b/scripts/handle-mainline-release.mjs similarity index 73% rename from scripts/update-directory-cache.mjs rename to scripts/handle-mainline-release.mjs index b5f861a7..4713df2f 100644 --- a/scripts/update-directory-cache.mjs +++ b/scripts/handle-mainline-release.mjs @@ -1,11 +1,10 @@ #!/usr/bin/env node /** - * Update the most commonly updated paths in the directory cache for a Node.js - * release. + * Updates the cached directory listings for paths that have symlinks for the + * mainline release bucket. * - * Usage: ./update-directory-cache.mjs [version] - * Example: ./update-directory-cache.mjs v24.0.0 + * Usage: ./handle-mainline-release.mjs * * If no version is provided, this just updates what's in {@link ALWAYS_UPDATED_PATHS} */ @@ -22,8 +21,6 @@ import { getLatestVersionMapping } from './utils/getLatestVersionMapping.mjs'; import { createCloudflareClient, writeKeysToKv } from './utils/kv.mjs'; import { createS3Client, listR2DirectoryRecursive } from './utils/r2.mjs'; -const VERSION = process.argv[2]?.toLowerCase(); - // Ensure all necessary environment variables are set for (const envVar of [ 'CLOUDFLARE_API_TOKEN', @@ -67,26 +64,6 @@ await Promise.all( ) ); -if (VERSION !== undefined && VERSION !== '') { - if (!VERSION.startsWith('v')) { - throw new TypeError( - 'provided version not in correct format, expected vX.X.X' - ); - } - - console.log(`Listing version ${VERSION}`); - - const versionDirectory = await listR2DirectoryRecursive( - s3Client, - PROD_BUCKET, - `${RELEASE_DIR}${VERSION}/` - ); - - for (const [k, v] of versionDirectory.entries()) { - directories.set(k, v); - } -} - const latestVersions = await getLatestVersionMapping( s3Client, directories.get(RELEASE_DIR), @@ -95,6 +72,8 @@ const latestVersions = await getLatestVersionMapping( await addSymlinksToDirectoryCache(s3Client, directories, latestVersions); -// Update keys in the directory cache +// Update keys in the directory cache. These intentionally do not have an +// expiration set since they were made with info that the worker doesn't have +// when deployed. console.log(`Writing ${directories.size} keys to directory cache...`); await writeKeysToKv(cfClient, DIRECTORY_CACHE_NAMESPACE_ID, directories); diff --git a/scripts/utils/addSymlinksToDirectoryCache.mjs b/scripts/utils/addSymlinksToDirectoryCache.mjs index 1d4239cf..77b33547 100644 --- a/scripts/utils/addSymlinksToDirectoryCache.mjs +++ b/scripts/utils/addSymlinksToDirectoryCache.mjs @@ -124,10 +124,10 @@ export async function addStaticFileSymlinksToCache( */ const fileSymlinks = JSON.parse(fileSymlinksFile); - // This file is an exception for two reasons: - // 1. It's dynamic and points to the latest Node version - // 2. It's handled specially and is technically in the root directory, - // which we should never be listing thus don't need to add it to the + // These files are an exception for two reasons: + // 1. They're dynamic and point to the latest Node version + // 2. They're handled specially and are technically in the root directory, + // which we should never be listing thus don't need to add them to the // directory cache. // So, let's remove them then add them back once we're done handling the rest // of the static file symlinks. diff --git a/src/constants/cache.ts b/src/constants/cache.ts index 80f06117..8fa91057 100644 --- a/src/constants/cache.ts +++ b/src/constants/cache.ts @@ -4,3 +4,8 @@ export const CACHE_HEADERS = { mutable: 'public, max-age=3600, s-maxage=14400', failure: 'private, no-cache, no-store, max-age=0, must-revalidate', }; + +/** + * Ttl for elements in the KV cache in seconds. This is completely arbitrary + */ +export const KV_CACHE_TTL = 60 * 60 * 6; diff --git a/src/middleware/r2Middleware.ts b/src/middleware/r2Middleware.ts index 5665e1ec..c848c9fd 100644 --- a/src/middleware/r2Middleware.ts +++ b/src/middleware/r2Middleware.ts @@ -2,7 +2,11 @@ import * as Sentry from '@sentry/cloudflare'; import { CACHE_HEADERS } from '../constants/cache'; import docsDirectory from '../constants/docsDirectory.json' assert { type: 'json' }; import type { Context } from '../context'; -import type { GetFileResult, HeadFileResult } from '../providers/provider'; +import type { + GetFileResult, + HeadFileResult, + Provider, +} from '../providers/provider'; import { R2Provider } from '../providers/r2Provider'; import responses from '../responses'; import { hasTrailingSlash, isDirectoryPath } from '../utils/path'; @@ -13,8 +17,12 @@ import type { Request } from '../routes/request'; import { renderDirectoryListing } from '../utils/directoryListing'; import { getOriginalUrl, parseConditionalHeaders } from '../utils/request'; import { once } from '../utils/memo'; +import { KvProvider } from '../providers/kvProvider'; -const getProvider = once((ctx: Context) => new R2Provider({ ctx })); +const getR2Provider = once((ctx: Context) => new R2Provider({ ctx })); +const getKvProvider = once( + (ctx: Context, r2Provider: R2Provider) => new KvProvider(ctx, r2Provider) +); export class R2Middleware implements Middleware { async handle(request: Request, ctx: Context): Promise { @@ -35,6 +43,32 @@ export class R2Middleware implements Middleware { } } +/** + * Temp helper function for deciding if we wanna use the KV directory cache or + * just R2 + */ +function determineDirectoryProvider(path: string, ctx: Context): Provider { + const r2Provider = getR2Provider(ctx); + + if (ctx.env.USE_KV) { + if (ctx.env.KV_DIRECTORIES === undefined) { + // Using kv for all directories + return getKvProvider(ctx, r2Provider); + } + + for (const prefix of ctx.env.KV_DIRECTORIES) { + if (path.startsWith(prefix)) { + // A path we wanna use kv for + return getKvProvider(ctx, r2Provider); + } + } + + // If we're here then it's not a directory we wanna use kv for + } + + return r2Provider; +} + async function handleDirectory( request: Request, r2Path: string, @@ -46,7 +80,9 @@ async function handleDirectory( return Response.redirect(`${url}/`, 301); } - const result = await getProvider(ctx).readDirectory(r2Path); + const result = await determineDirectoryProvider(r2Path, ctx).readDirectory( + r2Path + ); if (result === undefined) { return responses.directoryNotFound(request.method); @@ -110,7 +146,7 @@ async function headFile( r2Path: string, ctx: Context ): Promise { - const result = await getProvider(ctx).headFile(r2Path); + const result = await getR2Provider(ctx).headFile(r2Path); if (result === undefined) { return responses.fileNotFound(request.method); @@ -127,7 +163,7 @@ async function getFile( r2Path: string, ctx: Context ): Promise { - const provider = getProvider(ctx); + const provider = getR2Provider(ctx); let result: GetFileResult | undefined; try { diff --git a/src/providers/kvProvider.test.ts b/src/providers/kvProvider.test.ts index ea493ffd..c2c7f7f0 100644 --- a/src/providers/kvProvider.test.ts +++ b/src/providers/kvProvider.test.ts @@ -1,90 +1,224 @@ import { describe, expect, test } from 'vitest'; import { KvProvider } from './kvProvider'; +import type { Context } from '../context'; +import type { R2Provider } from './r2Provider'; +import type { ReadDirectoryResult } from './provider'; +import { KV_CACHE_TTL } from '../constants/cache'; describe('readDirectory', () => { test('returns expected response when a directory exists', async () => { - const kvProvider = new KvProvider({ - ctx: { - env: { - DIRECTORY_CACHE: { - // @ts-expect-error don't need actual type info here - async get(path: string): Promise { - expect(path).toStrictEqual('some/file/path.json'); + let directoryCacheInvoked = false; + const ctx: Context = { + env: { + DIRECTORY_CACHE: { + // @ts-expect-error don't need actual type info here + async get(path: string): Promise { + directoryCacheInvoked = true; + expect(path).toStrictEqual('some/directory/path/'); - return { - subdirectories: [ - 'subdirectory1/', - 'subdirectory2/', - 'subdirectory3/', - ], - hasIndexHtmlFile: false, - files: [ - { - name: 'file1.txt', - lastModified: '2024-11-04T16:21:30.218Z', - size: 123, - }, - { - name: 'file2.txt', - lastModified: '2024-11-04T16:21:30.218Z', - size: 321, - }, - { - name: 'file3.txt', - lastModified: '2024-11-04T16:21:30.218Z', - size: 456, - }, - ], - lastModified: '2024-11-04T16:21:30.218Z', - }; - }, + return { + subdirectories: [ + 'subdirectory1/', + 'subdirectory2/', + 'subdirectory3/', + ], + hasIndexHtmlFile: false, + files: [ + { + name: 'file1.txt', + lastModified: '2024-11-04T16:21:30.218Z', + size: 123, + }, + { + name: 'file2.txt', + lastModified: '2024-11-04T16:21:30.218Z', + size: 321, + }, + { + name: 'file3.txt', + lastModified: '2024-11-04T16:21:30.218Z', + size: 456, + }, + ], + lastModified: '2024-11-04T16:21:30.218Z', + }; }, }, }, + }; + + const kvProvider = new KvProvider(ctx, undefined); + + expect( + await kvProvider.readDirectory('some/directory/path/') + ).toStrictEqual({ + subdirectories: ['subdirectory1/', 'subdirectory2/', 'subdirectory3/'], + hasIndexHtmlFile: false, + files: [ + { + name: 'file1.txt', + lastModified: new Date('2024-11-04T16:21:30.218Z'), + size: 123, + }, + { + name: 'file2.txt', + lastModified: new Date('2024-11-04T16:21:30.218Z'), + size: 321, + }, + { + name: 'file3.txt', + lastModified: new Date('2024-11-04T16:21:30.218Z'), + size: 456, + }, + ], + lastModified: new Date('2024-11-04T16:21:30.218Z'), }); + expect(directoryCacheInvoked).toBeTruthy(); + }); - expect(await kvProvider.readDirectory('some/file/path.json')).toStrictEqual( - { - subdirectories: ['subdirectory1/', 'subdirectory2/', 'subdirectory3/'], - hasIndexHtmlFile: false, - files: [ - { - name: 'file1.txt', - lastModified: new Date('2024-11-04T16:21:30.218Z'), - size: 123, - }, - { - name: 'file2.txt', - lastModified: new Date('2024-11-04T16:21:30.218Z'), - size: 321, + test('returns `undefined` for unknown directory & no R2Provider', async () => { + let directoryCacheInvoked = false; + const ctx: Context = { + env: { + DIRECTORY_CACHE: { + // @ts-expect-error don't need actual type info here + async get(): Promise { + directoryCacheInvoked = true; + return null; }, - { - name: 'file3.txt', - lastModified: new Date('2024-11-04T16:21:30.218Z'), - size: 456, - }, - ], - lastModified: new Date('2024-11-04T16:21:30.218Z'), - } - ); + }, + }, + }; + + const kvProvider = new KvProvider(ctx, undefined); + + expect( + await kvProvider.readDirectory('some/directory/path/') + ).toBeUndefined(); + expect(directoryCacheInvoked).toBeTruthy(); }); - test('returns `undefined` for unknown directory', async () => { - const kvProvider = new KvProvider({ - ctx: { - env: { - DIRECTORY_CACHE: { - // @ts-expect-error don't need actual type info here - async get(): Promise { - return null; - }, + test('falls back to R2Provider for unknown directories if it exists', async () => { + let directoryCacheGetInvoked = false; + let directoryCachePutInvoked = false; + const ctx: Context = { + env: { + DIRECTORY_CACHE: { + // @ts-expect-error don't need actual type info here + async get(): Promise { + directoryCacheGetInvoked = true; + return null; + }, + async put(key, value, settings): Promise { + directoryCachePutInvoked = true; + + expect(key).toStrictEqual('some/directory/path'); + expect(value).toStrictEqual({ + subdirectories: [ + 'subdirectory1/', + 'subdirectory2/', + 'subdirectory3/', + ], + hasIndexHtmlFile: false, + files: [ + { + name: 'file1.txt', + lastModified: '2024-11-04T16:21:30.218Z', + size: 123, + }, + { + name: 'file2.txt', + lastModified: '2024-11-04T16:21:30.218Z', + size: 321, + }, + { + name: 'file3.txt', + lastModified: '2024-11-04T16:21:30.218Z', + size: 456, + }, + ], + lastModified: '2024-11-04T16:21:30.218Z', + }); + expect(settings?.expirationTtl).toEqual(KV_CACHE_TTL); }, }, }, - }); + // @ts-expect-error don't need full type + execution: { + waitUntil(_) { + // We're not actually waiting on the promise here which is a little + // racy between the promise & the test ending but it's only by like + // two ticks max probs + }, + }, + }; + + let r2ProviderInvoked = false; + + // @ts-expect-error don't need the full type here + const r2Provider: R2Provider = { + readDirectory: async (path): Promise => { + r2ProviderInvoked = true; + directoryCacheGetInvoked = true; + expect(path).toStrictEqual('some/directory/path/'); + + return { + subdirectories: [ + 'subdirectory1/', + 'subdirectory2/', + 'subdirectory3/', + ], + hasIndexHtmlFile: false, + files: [ + { + name: 'file1.txt', + lastModified: new Date('2024-11-04T16:21:30.218Z'), + size: 123, + }, + { + name: 'file2.txt', + lastModified: new Date('2024-11-04T16:21:30.218Z'), + size: 321, + }, + { + name: 'file3.txt', + lastModified: new Date('2024-11-04T16:21:30.218Z'), + size: 456, + }, + ], + lastModified: new Date('2024-11-04T16:21:30.218Z'), + }; + }, + }; + + const kvProvider = new KvProvider(ctx, r2Provider); expect( - await kvProvider.readDirectory('some/file/path.json') - ).toBeUndefined(); + await kvProvider.readDirectory('some/directory/path/') + ).toStrictEqual({ + subdirectories: ['subdirectory1/', 'subdirectory2/', 'subdirectory3/'], + hasIndexHtmlFile: false, + files: [ + { + name: 'file1.txt', + lastModified: new Date('2024-11-04T16:21:30.218Z'), + size: 123, + }, + { + name: 'file2.txt', + lastModified: new Date('2024-11-04T16:21:30.218Z'), + size: 321, + }, + { + name: 'file3.txt', + lastModified: new Date('2024-11-04T16:21:30.218Z'), + size: 456, + }, + ], + lastModified: new Date('2024-11-04T16:21:30.218Z'), + }); + expect(directoryCacheGetInvoked).toBeTruthy(); + expect(directoryCachePutInvoked).toBeTruthy(); + expect(r2ProviderInvoked).toBeTruthy(); }); }); diff --git a/src/providers/kvProvider.ts b/src/providers/kvProvider.ts index 97e23900..9ee38d74 100644 --- a/src/providers/kvProvider.ts +++ b/src/providers/kvProvider.ts @@ -1,4 +1,5 @@ import { KV_RETRY_LIMIT } from '../../lib/limits.mjs'; +import { KV_CACHE_TTL } from '../constants/cache'; import type { Context } from '../context'; import { retryWrapper } from '../utils/provider'; import type { @@ -8,16 +9,15 @@ import type { Provider, ReadDirectoryResult, } from './provider'; - -type KvProviderCtorOptions = { - ctx: Context; -}; +import type { R2Provider } from './r2Provider'; export class KvProvider implements Provider { #ctx: Context; + #r2Provider?: R2Provider; - constructor({ ctx }: KvProviderCtorOptions) { + constructor(ctx: Context, r2Provider?: R2Provider) { this.#ctx = ctx; + this.#r2Provider = r2Provider; } headFile(_: string): Promise { @@ -37,7 +37,21 @@ export class KvProvider implements Provider { }, KV_RETRY_LIMIT); if (result === null) { - return undefined; + if (this.#r2Provider === undefined) { + return undefined; + } + + const result = await this.#r2Provider.readDirectory(path); + + if (result !== undefined) { + this.#ctx.execution.waitUntil( + this.#ctx.env.DIRECTORY_CACHE.put(path, JSON.stringify(result), { + expirationTtl: KV_CACHE_TTL, + }) + ); + } + + return result; } // Convert last modified from a string to Date object diff --git a/src/providers/r2Provider.ts b/src/providers/r2Provider.ts index 270f9b45..ff767bf5 100644 --- a/src/providers/r2Provider.ts +++ b/src/providers/r2Provider.ts @@ -14,7 +14,6 @@ import type { ReadDirectoryResult, } from './provider'; import { S3Provider } from './s3Provider'; -import { KvProvider } from './kvProvider'; type CachedFile = { name: string; @@ -101,22 +100,6 @@ export class R2Provider implements Provider { } async readDirectory(path: string): Promise { - const kvProvider = new KvProvider({ - ctx: this.ctx, - }); - - if (this.ctx.env.USE_KV) { - if (this.ctx.env.KV_DIRECTORIES !== undefined) { - for (const prefix of this.ctx.env.KV_DIRECTORIES) { - if (path.startsWith(prefix)) { - return await kvProvider.readDirectory(path); - } - } - } else { - return await kvProvider.readDirectory(path); - } - } - let result: ReadDirectoryResult | undefined; if (path in CACHED_DIRECTORIES) { const cachedResult: CachedDirectory = From 2b2357dcf26ee67662205576a59f0b90d5889316 Mon Sep 17 00:00:00 2001 From: flakey5 <73616808+flakey5@users.noreply.github.com> Date: Sat, 24 Oct 2026 23:30:40 -0700 Subject: [PATCH 2/3] code review Signed-off-by: flakey5 <73616808+flakey5@users.noreply.github.com> --- e2e-tests/directory.kv.test.ts | 80 +++++++++++++++++++++++++++++++++- 1 file changed, 78 insertions(+), 2 deletions(-) diff --git a/e2e-tests/directory.kv.test.ts b/e2e-tests/directory.kv.test.ts index 9c6cfe42..4a61e48e 100644 --- a/e2e-tests/directory.kv.test.ts +++ b/e2e-tests/directory.kv.test.ts @@ -1,5 +1,5 @@ -import { env, createExecutionContext } from 'cloudflare:test'; -import { test, beforeAll, expect, vi } from 'vitest'; +import { env, fetchMock, createExecutionContext } from 'cloudflare:test'; +import { test, beforeAll, expect, vi, afterEach } from 'vitest'; import { populateDirectoryCacheWithDevBucket, populateR2WithDevBucket, @@ -13,9 +13,46 @@ const mockedEnv: Env = { ENVIRONMENT: 'e2e-tests', LOG_ERRORS: true, USE_KV: true, + S3_ENDPOINT: 'https://s3.mock', + S3_ACCESS_KEY_ID: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', + S3_ACCESS_KEY_SECRET: + 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', }; +const s3Url = new URL(mockedEnv.S3_ENDPOINT); +s3Url.host = `${mockedEnv.BUCKET_NAME}.${s3Url.host}`; + +const S3_DIRECTORY_RESULT = ` + + dist-prod + + + 1000 + false + + nodejs/release/v1.0.0/latest/ + + + "asd123" + nodejs/release/v1.0.0/index.json + 2023-09-12T05:43:00.000Z + 18 + +`; + +const S3_EMPTY_DIRECTORY_RESULT = ` + + dist-prod + + + 1000 + false +`; + beforeAll(async () => { + fetchMock.activate(); + fetchMock.disableNetConnect(); + await populateR2WithDevBucket(); await populateDirectoryCacheWithDevBucket(); @@ -42,6 +79,10 @@ beforeAll(async () => { ); }); +afterEach(() => { + fetchMock.assertNoPendingInterceptors(); +}) + // Ensure essential endpoints are routable for (const path of ['/dist/', '/docs/', '/api/', '/download/', '/metrics/']) { test(`GET \`${path}\` returns 200`, async () => { @@ -61,6 +102,16 @@ for (const path of ['/dist/', '/docs/', '/api/', '/download/', '/metrics/']) { } test('GET `/dist/unknown-directory/` returns 404', async () => { + fetchMock + .get(s3Url.origin) + .intercept({ + path: /.*/, + query: { + prefix: 'nodejs/release/unknown-directory/', + }, + }) + .reply(200, S3_EMPTY_DIRECTORY_RESULT); + const ctx = createExecutionContext(); const res = await worker.fetch( @@ -74,6 +125,31 @@ test('GET `/dist/unknown-directory/` returns 404', async () => { expect(await res.text()).toStrictEqual('Directory not found'); }); +test('GET `/dist/missing-in-kv-but-in-r2/` returns 200', async () => { + fetchMock + .get(s3Url.origin) + .intercept({ + path: /.*/, + query: { + prefix: 'nodejs/release/missing-in-kv-but-in-r2/', + }, + }) + .reply(200, S3_DIRECTORY_RESULT); + + const ctx = createExecutionContext(); + + const res = await worker.fetch( + new Request(`https://localhost/dist/missing-in-kv-but-in-r2/`), + mockedEnv, + ctx + ); + + // Consume body promise + await res.text(); + + expect(res.status).toBe(200); +}) + test('GET `/dist` redirects to `/dist/`', async () => { const ctx = createExecutionContext(); From 4ff2b1ec95b94336802c5d94827e4ca00fddae3d Mon Sep 17 00:00:00 2001 From: flakey5 <73616808+flakey5@users.noreply.github.com> Date: Sat, 24 Oct 2026 23:30:58 -0700 Subject: [PATCH 3/3] code review Signed-off-by: flakey5 <73616808+flakey5@users.noreply.github.com> --- e2e-tests/directory.kv.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/e2e-tests/directory.kv.test.ts b/e2e-tests/directory.kv.test.ts index 4a61e48e..2c1eccb5 100644 --- a/e2e-tests/directory.kv.test.ts +++ b/e2e-tests/directory.kv.test.ts @@ -81,7 +81,7 @@ beforeAll(async () => { afterEach(() => { fetchMock.assertNoPendingInterceptors(); -}) +}); // Ensure essential endpoints are routable for (const path of ['/dist/', '/docs/', '/api/', '/download/', '/metrics/']) { @@ -148,7 +148,7 @@ test('GET `/dist/missing-in-kv-but-in-r2/` returns 200', async () => { await res.text(); expect(res.status).toBe(200); -}) +}); test('GET `/dist` redirects to `/dist/`', async () => { const ctx = createExecutionContext();