From 946513f1ca9ba5acddbff22aac93283016515198 Mon Sep 17 00:00:00 2001 From: Ricardo Devis Agullo Date: Sun, 21 Jun 2026 21:39:05 +0200 Subject: [PATCH 1/5] feat(storage): paginate listSubDirectories and removeDir for S3 and GS Implements continuation/pageToken handling so listing and removing directories works correctly when buckets return paginated results. Adds mock-based tests for paginated list and remove behavior. --- .../__mocks__/@google-cloud/storage.js | 81 +++++++++++++------ .../oc-gs-storage-adapter/__test__/gs.test.ts | 20 +++++ packages/oc-gs-storage-adapter/src/index.ts | 71 +++++++++++++--- .../__mocks__/@aws-sdk/client-s3.js | 68 +++++++++++++--- .../oc-s3-storage-adapter/__test__/s3.test.ts | 20 +++++ packages/oc-s3-storage-adapter/src/index.ts | 69 +++++++++++----- 6 files changed, 261 insertions(+), 68 deletions(-) diff --git a/packages/oc-gs-storage-adapter/__mocks__/@google-cloud/storage.js b/packages/oc-gs-storage-adapter/__mocks__/@google-cloud/storage.js index d9b3772..891a827 100644 --- a/packages/oc-gs-storage-adapter/__mocks__/@google-cloud/storage.js +++ b/packages/oc-gs-storage-adapter/__mocks__/@google-cloud/storage.js @@ -12,32 +12,61 @@ const googleStorage = jest.genMockFromModule('@google-cloud/storage'); const _Storage = class { constructor() { this.bucket = jest.fn(bucket => ({ - getFiles: () => { - const files = - bucket === 'my-empty-bucket' - ? [] - : [ - [ - { - name: 'components/image/1.0.0/app.js' - }, - { - name: 'components/image/1.0.0/server.js' - }, - { - name: 'components/image/1.0.0/.env' - }, - { - name: 'components/image/1.0.1/new-server.js' - }, - { - name: 'components/image/1.0.1/new-.env' - }, - { - name: 'components/image/1.0.1/new-app.js' - } - ] - ]; + getFiles: (options = {}) => { + if (bucket === 'my-empty-bucket') { + return Promise.resolve([]); + } + + if (bucket === 'paginated-bucket') { + const makeFile = name => ({ + name, + delete: jest.fn(() => Promise.resolve()) + }); + + if (!options.pageToken) { + return Promise.resolve([ + [ + makeFile('components/a/1.0.0/app.js'), + makeFile('components/a/1.0.0/server.js') + ], + { pageToken: 'page-2' } + ]); + } + + return Promise.resolve([ + [makeFile('components/a/2.0.0/app.js')], + null + ]); + } + + const files = [ + [ + { + name: 'components/image/1.0.0/app.js', + delete: jest.fn(() => Promise.resolve()) + }, + { + name: 'components/image/1.0.0/server.js', + delete: jest.fn(() => Promise.resolve()) + }, + { + name: 'components/image/1.0.0/.env', + delete: jest.fn(() => Promise.resolve()) + }, + { + name: 'components/image/1.0.1/new-server.js', + delete: jest.fn(() => Promise.resolve()) + }, + { + name: 'components/image/1.0.1/new-.env', + delete: jest.fn(() => Promise.resolve()) + }, + { + name: 'components/image/1.0.1/new-app.js', + delete: jest.fn(() => Promise.resolve()) + } + ] + ]; return Promise.resolve(files); }, upload: (filePath, { destination }) => { diff --git a/packages/oc-gs-storage-adapter/__test__/gs.test.ts b/packages/oc-gs-storage-adapter/__test__/gs.test.ts index 317a7be..4b8036b 100644 --- a/packages/oc-gs-storage-adapter/__test__/gs.test.ts +++ b/packages/oc-gs-storage-adapter/__test__/gs.test.ts @@ -185,6 +185,26 @@ test('test getJson force mode', async () => { }); }); +test('listSubDirectories follows pageToken across pages', async () => { + const client = gs({ ...validOptions, bucket: 'paginated-bucket' }); + + const data = await client.listSubDirectories('components/a'); + + // Page 1 yields 1.0.0, page 2 yields 2.0.0. Both are only returned if the + // adapter follows the nextQuery pageToken across pages. + expect(data).toEqual(['1.0.0', '2.0.0']); +}); + +test('removeDir follows pageToken across pages and deletes all files', async () => { + const client = gs({ ...validOptions, bucket: 'paginated-bucket' }); + + const result = (await client.removeDir('components/a')) as unknown[]; + + // Page 1 lists two files, page 2 lists one file. Three deletes only happen + // if the adapter paginates using the nextQuery pageToken. + expect(result).toHaveLength(3); +}); + test('test getUrl ', () => { const client = gs(validOptions); expect(client.getUrl('test', '1.0.0', 'test.js')).toBe('/test/1.0.0/test.js'); diff --git a/packages/oc-gs-storage-adapter/src/index.ts b/packages/oc-gs-storage-adapter/src/index.ts index 7144142..ea05527 100644 --- a/packages/oc-gs-storage-adapter/src/index.ts +++ b/packages/oc-gs-storage-adapter/src/index.ts @@ -118,19 +118,41 @@ export default function gsAdapter(conf: GsConfig): StorageAdapter { ? dir : dir + '/'; - const options = { - prefix: normalisedPath - }; - try { - const results = await getClient().bucket(bucketName).getFiles(options); + const collected: { name: string }[] = []; + let pageToken: string | undefined; + + do { + const options: { + prefix: string; + autoPaginate: false; + pageToken?: string; + } = { + prefix: normalisedPath, + autoPaginate: false + }; + if (pageToken) { + options.pageToken = pageToken; + } + + const results = await getClient() + .bucket(bucketName) + .getFiles(options); + const files = results[0] ?? []; + const nextQuery = results[1] as { pageToken?: string } | undefined; + + for (const file of files) { + collected.push(file); + } - const files = results[0]; - if (files.length === 0) { + pageToken = nextQuery?.pageToken; + } while (pageToken); + + if (collected.length === 0) { throw 'no files'; } - const result = files + const result = collected //remove prefix .map(file => file.name.replace(normalisedPath, '')) // only get files that aren't in root directory @@ -258,11 +280,36 @@ export default function gsAdapter(conf: GsConfig): StorageAdapter { ? dir : `${dir}/`; - const [files] = await getClient() - .bucket(bucketName) - .getFiles({ prefix: normalisedPath }); + const collected: { delete: () => Promise }[] = []; + let pageToken: string | undefined; + + do { + const options: { + prefix: string; + autoPaginate: false; + pageToken?: string; + } = { + prefix: normalisedPath, + autoPaginate: false + }; + if (pageToken) { + options.pageToken = pageToken; + } + + const results = await getClient() + .bucket(bucketName) + .getFiles(options); + const files = results[0] ?? []; + const nextQuery = results[1] as { pageToken?: string } | undefined; + + for (const file of files) { + collected.push(file as { delete: () => Promise }); + } + + pageToken = nextQuery?.pageToken; + } while (pageToken); - return Promise.all(files.map(file => file.delete())); + return Promise.all(collected.map(file => file.delete())); }; return removeFromContainer(); diff --git a/packages/oc-s3-storage-adapter/__mocks__/@aws-sdk/client-s3.js b/packages/oc-s3-storage-adapter/__mocks__/@aws-sdk/client-s3.js index fcb1e9f..adc7c9d 100644 --- a/packages/oc-s3-storage-adapter/__mocks__/@aws-sdk/client-s3.js +++ b/packages/oc-s3-storage-adapter/__mocks__/@aws-sdk/client-s3.js @@ -56,21 +56,67 @@ const _S3 = class { }); this.listObjects = jest.fn(val => { - const CommonPrefixes = - val.Bucket === 'my-empty-bucket' - ? [] - : [ - { - Prefix: 'components/image/1.0.0/' - }, - { - Prefix: 'components/image/1.0.1/' - } - ]; + if (val.Bucket === 'my-empty-bucket') { + return Promise.resolve({ CommonPrefixes: [], Contents: [] }); + } + + if (val.Bucket === 'paginated-bucket') { + const isDelimited = val.Delimiter === '/'; + + if (isDelimited) { + // listSubDirectories path: S3 returns NextMarker when Delimiter is set. + if (!val.Marker) { + return Promise.resolve({ + CommonPrefixes: [ + { Prefix: 'components/a/1.0.0/' }, + { Prefix: 'components/a/1.0.1/' } + ], + IsTruncated: true, + NextMarker: 'marker-2' + }); + } + + return Promise.resolve({ + CommonPrefixes: [{ Prefix: 'components/a/2.0.0/' }], + IsTruncated: false + }); + } + + // removeDir path: no Delimiter, so S3 v1 does NOT return NextMarker; + // the client must use the last Key in Contents as the next Marker. + if (!val.Marker) { + return Promise.resolve({ + Contents: [ + { Key: 'components/a/1.0.0/file.js' }, + { Key: 'components/a/1.0.0/package.json' } + ], + IsTruncated: true + }); + } + + return Promise.resolve({ + Contents: [ + { Key: 'components/a/2.0.0/file.js' }, + { Key: 'components/a/2.0.0/package.json' } + ], + IsTruncated: false + }); + } + + const CommonPrefixes = [ + { + Prefix: 'components/image/1.0.0/' + }, + { + Prefix: 'components/image/1.0.1/' + } + ]; return Promise.resolve({ CommonPrefixes }); }); + this.deleteObject = jest.fn(() => Promise.resolve({})); + this.putObject = jest.fn(data => { if (data && data.Key && data.Key.indexOf('error') >= 0) { if (data.Key.indexOf('throw') >= 0) { diff --git a/packages/oc-s3-storage-adapter/__test__/s3.test.ts b/packages/oc-s3-storage-adapter/__test__/s3.test.ts index 8a9b4ad..acfbc3e 100644 --- a/packages/oc-s3-storage-adapter/__test__/s3.test.ts +++ b/packages/oc-s3-storage-adapter/__test__/s3.test.ts @@ -200,6 +200,26 @@ test('test getJson force mode', async () => { }); }); +test('listSubDirectories follows continuation tokens across pages', async () => { + const client = s3({ ...validOptions, bucket: 'paginated-bucket' }); + + const data = await client.listSubDirectories('components/a'); + + // Page 1 returns 1.0.0 and 1.0.1, page 2 returns 2.0.0. All three are + // only returned if the adapter follows NextMarker across pages. + expect(data).toEqual(['1.0.0', '1.0.1', '2.0.0']); +}); + +test('removeDir follows continuation tokens across pages and deletes all keys', async () => { + const client = s3({ ...validOptions, bucket: 'paginated-bucket' }); + + const result = (await client.removeDir('components/a')) as unknown[]; + + // Page 1 lists two keys, page 2 lists two keys. Four deletes only happen + // if the adapter paginates using the last Key as the next Marker. + expect(result).toHaveLength(4); +}); + test('test getUrl ', () => { const client = s3(validOptions); expect(client.getUrl('test', '1.0.0', 'test.js')).toBe('/test/1.0.0/test.js'); diff --git a/packages/oc-s3-storage-adapter/src/index.ts b/packages/oc-s3-storage-adapter/src/index.ts index c5308d4..dbd9c26 100644 --- a/packages/oc-s3-storage-adapter/src/index.ts +++ b/packages/oc-s3-storage-adapter/src/index.ts @@ -185,24 +185,37 @@ export default function s3Adapter(conf: S3Config): StorageAdapter { ? dir : `${dir}/`; - const data = await getClient().listObjects({ - Bucket: bucket, - Prefix: normalisedPath, - Delimiter: '/' - }); + const prefixes: string[] = []; + let marker: string | undefined; + + do { + const data = await getClient().listObjects({ + Bucket: bucket, + Prefix: normalisedPath, + Delimiter: '/', + Marker: marker + }); + + for (const commonPrefix of data.CommonPrefixes ?? []) { + if (commonPrefix.Prefix) { + prefixes.push(commonPrefix.Prefix); + } + } - if (data.CommonPrefixes?.length === 0) { + // S3 returns NextMarker when a Delimiter is set and the list is truncated. + // Guard against an infinite loop if S3 ever omits it while truncated. + marker = data.IsTruncated && data.NextMarker ? data.NextMarker : undefined; + } while (marker); + + if (prefixes.length === 0) { throw { code: strings.errors.STORAGE.DIR_NOT_FOUND_CODE, msg: strings.errors.STORAGE.DIR_NOT_FOUND(dir) }; } - const result = _.map(data.CommonPrefixes, commonPrefix => - commonPrefix.Prefix!.substr( - normalisedPath.length, - commonPrefix.Prefix!.length - normalisedPath.length - 1 - ) + const result = _.map(prefixes, prefix => + prefix.substr(normalisedPath.length, prefix.length - normalisedPath.length - 1) ); return result; @@ -281,14 +294,32 @@ export default function s3Adapter(conf: S3Config): StorageAdapter { ? dir : `${dir}/`; - const data = await getClient().listObjects({ - Bucket: bucket, - Prefix: normalisedPath - }); - const files = - data.Contents?.map(content => content.Key).filter( - key => key != undefined - ) ?? []; + const files: string[] = []; + let marker: string | undefined; + + do { + const data = await getClient().listObjects({ + Bucket: bucket, + Prefix: normalisedPath, + Marker: marker + }); + + for (const content of data.Contents ?? []) { + if (content.Key != null) { + files.push(content.Key); + } + } + + // Without a Delimiter, S3 does not return NextMarker; per the v1 + // ListObjects spec, use the last Key in Contents as the next Marker. + if (data.IsTruncated && data.NextMarker) { + marker = data.NextMarker; + } else if (data.IsTruncated && data.Contents && data.Contents.length > 0) { + marker = data.Contents[data.Contents.length - 1].Key; + } else { + marker = undefined; + } + } while (marker); return Promise.all(files.map(file => removeFile(file))); }; From 13cb3b8584197eff4415068180139fc67dca29fc Mon Sep 17 00:00:00 2001 From: Ricardo Devis Agullo Date: Sun, 21 Jun 2026 21:52:34 +0200 Subject: [PATCH 2/5] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- packages/oc-s3-storage-adapter/src/index.ts | 23 ++++++++++++--------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/packages/oc-s3-storage-adapter/src/index.ts b/packages/oc-s3-storage-adapter/src/index.ts index dbd9c26..03eb5dc 100644 --- a/packages/oc-s3-storage-adapter/src/index.ts +++ b/packages/oc-s3-storage-adapter/src/index.ts @@ -297,7 +297,7 @@ export default function s3Adapter(conf: S3Config): StorageAdapter { const files: string[] = []; let marker: string | undefined; - do { + while (true) { const data = await getClient().listObjects({ Bucket: bucket, Prefix: normalisedPath, @@ -310,16 +310,19 @@ export default function s3Adapter(conf: S3Config): StorageAdapter { } } - // Without a Delimiter, S3 does not return NextMarker; per the v1 - // ListObjects spec, use the last Key in Contents as the next Marker. - if (data.IsTruncated && data.NextMarker) { - marker = data.NextMarker; - } else if (data.IsTruncated && data.Contents && data.Contents.length > 0) { - marker = data.Contents[data.Contents.length - 1].Key; - } else { - marker = undefined; + if (!data.IsTruncated) { + break; } - } while (marker); + + const nextMarker = + data.NextMarker ?? data.Contents?.[data.Contents.length - 1]?.Key; + + if (!nextMarker || nextMarker === marker) { + throw new Error('S3 listObjects returned no (or unchanged) pagination marker while truncated'); + } + + marker = nextMarker; + } return Promise.all(files.map(file => removeFile(file))); }; From 7c8d52ee4bad77660e1ccb75ed7a57698104c6a0 Mon Sep 17 00:00:00 2001 From: Ricardo Devis Agullo Date: Sun, 21 Jun 2026 21:52:46 +0200 Subject: [PATCH 3/5] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- packages/oc-gs-storage-adapter/src/index.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/oc-gs-storage-adapter/src/index.ts b/packages/oc-gs-storage-adapter/src/index.ts index ea05527..fbb1117 100644 --- a/packages/oc-gs-storage-adapter/src/index.ts +++ b/packages/oc-gs-storage-adapter/src/index.ts @@ -284,6 +284,7 @@ export default function gsAdapter(conf: GsConfig): StorageAdapter { let pageToken: string | undefined; do { + const requestPageToken = pageToken; const options: { prefix: string; autoPaginate: false; @@ -292,8 +293,8 @@ export default function gsAdapter(conf: GsConfig): StorageAdapter { prefix: normalisedPath, autoPaginate: false }; - if (pageToken) { - options.pageToken = pageToken; + if (requestPageToken) { + options.pageToken = requestPageToken; } const results = await getClient() @@ -306,7 +307,11 @@ export default function gsAdapter(conf: GsConfig): StorageAdapter { collected.push(file as { delete: () => Promise }); } - pageToken = nextQuery?.pageToken; + const nextPageToken = nextQuery?.pageToken; + if (nextPageToken && nextPageToken === requestPageToken) { + throw new Error('GCS getFiles returned an unchanged pageToken; aborting to avoid an infinite loop'); + } + pageToken = nextPageToken; } while (pageToken); return Promise.all(collected.map(file => file.delete())); From 3e9c40fa13e7d2d6f8462b4d410b0f9b8c9ffb44 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 21 Jun 2026 19:55:05 +0000 Subject: [PATCH 4/5] Guard GS pagination token advancement --- packages/oc-gs-storage-adapter/src/index.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/oc-gs-storage-adapter/src/index.ts b/packages/oc-gs-storage-adapter/src/index.ts index fbb1117..8f6aba8 100644 --- a/packages/oc-gs-storage-adapter/src/index.ts +++ b/packages/oc-gs-storage-adapter/src/index.ts @@ -123,6 +123,7 @@ export default function gsAdapter(conf: GsConfig): StorageAdapter { let pageToken: string | undefined; do { + const requestPageToken = pageToken; const options: { prefix: string; autoPaginate: false; @@ -131,8 +132,8 @@ export default function gsAdapter(conf: GsConfig): StorageAdapter { prefix: normalisedPath, autoPaginate: false }; - if (pageToken) { - options.pageToken = pageToken; + if (requestPageToken) { + options.pageToken = requestPageToken; } const results = await getClient() @@ -145,7 +146,13 @@ export default function gsAdapter(conf: GsConfig): StorageAdapter { collected.push(file); } - pageToken = nextQuery?.pageToken; + const nextPageToken = nextQuery?.pageToken; + if (nextPageToken && nextPageToken === requestPageToken) { + throw new Error( + 'GCS getFiles returned an unchanged pageToken; aborting to avoid an infinite loop' + ); + } + pageToken = nextPageToken; } while (pageToken); if (collected.length === 0) { From 1e185ab6091e6e376ae3046a3452bf99bfb3e14f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 21 Jun 2026 19:55:46 +0000 Subject: [PATCH 5/5] Fix S3 listSubDirectories pagination marker handling --- packages/oc-s3-storage-adapter/src/index.ts | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/packages/oc-s3-storage-adapter/src/index.ts b/packages/oc-s3-storage-adapter/src/index.ts index 03eb5dc..0f914cd 100644 --- a/packages/oc-s3-storage-adapter/src/index.ts +++ b/packages/oc-s3-storage-adapter/src/index.ts @@ -188,7 +188,7 @@ export default function s3Adapter(conf: S3Config): StorageAdapter { const prefixes: string[] = []; let marker: string | undefined; - do { + while (true) { const data = await getClient().listObjects({ Bucket: bucket, Prefix: normalisedPath, @@ -202,10 +202,21 @@ export default function s3Adapter(conf: S3Config): StorageAdapter { } } - // S3 returns NextMarker when a Delimiter is set and the list is truncated. - // Guard against an infinite loop if S3 ever omits it while truncated. - marker = data.IsTruncated && data.NextMarker ? data.NextMarker : undefined; - } while (marker); + if (!data.IsTruncated) { + break; + } + + const nextMarker = + data.NextMarker ?? + data.Contents?.[data.Contents.length - 1]?.Key ?? + data.CommonPrefixes?.[data.CommonPrefixes.length - 1]?.Prefix; + + if (!nextMarker || nextMarker === marker) { + throw new Error('S3 listObjects returned no (or unchanged) pagination marker while truncated'); + } + + marker = nextMarker; + } if (prefixes.length === 0) { throw {