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..8f6aba8 100644 --- a/packages/oc-gs-storage-adapter/src/index.ts +++ b/packages/oc-gs-storage-adapter/src/index.ts @@ -118,19 +118,48 @@ 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 requestPageToken = pageToken; + const options: { + prefix: string; + autoPaginate: false; + pageToken?: string; + } = { + prefix: normalisedPath, + autoPaginate: false + }; + if (requestPageToken) { + options.pageToken = requestPageToken; + } + + 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 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); - const files = results[0]; - if (files.length === 0) { + 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 +287,41 @@ 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 requestPageToken = pageToken; + const options: { + prefix: string; + autoPaginate: false; + pageToken?: string; + } = { + prefix: normalisedPath, + autoPaginate: false + }; + if (requestPageToken) { + options.pageToken = requestPageToken; + } + + 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 }); + } + + 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(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..0f914cd 100644 --- a/packages/oc-s3-storage-adapter/src/index.ts +++ b/packages/oc-s3-storage-adapter/src/index.ts @@ -185,24 +185,48 @@ 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; + + while (true) { + 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.IsTruncated) { + break; + } + + const nextMarker = + data.NextMarker ?? + data.Contents?.[data.Contents.length - 1]?.Key ?? + data.CommonPrefixes?.[data.CommonPrefixes.length - 1]?.Prefix; - if (data.CommonPrefixes?.length === 0) { + if (!nextMarker || nextMarker === marker) { + throw new Error('S3 listObjects returned no (or unchanged) pagination marker while truncated'); + } + + marker = nextMarker; + } + + 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 +305,35 @@ 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; + + while (true) { + 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); + } + } + + if (!data.IsTruncated) { + break; + } + + 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))); };