From 747bf31efb5dbb023538a46a37f6a5262297b67a Mon Sep 17 00:00:00 2001 From: Hannes Hertach Date: Wed, 9 Jul 2025 15:44:12 +0200 Subject: [PATCH 1/9] add pagination to list api --- src/handlers/get.js | 4 ++-- src/index.js | 2 +- src/routes/list.js | 13 +++++++++-- src/storage/object/list.js | 44 ++++++++++++++++++++++++++++++-------- 4 files changed, 49 insertions(+), 14 deletions(-) diff --git a/src/handlers/get.js b/src/handlers/get.js index 87916a3b..0f5f7c17 100644 --- a/src/handlers/get.js +++ b/src/handlers/get.js @@ -24,14 +24,14 @@ function getRobots() { return { body, status: 200 }; } -export default async function getHandler({ env, daCtx }) { +export default async function getHandler({ req, env, daCtx }) { const { path } = daCtx; if (path.startsWith('/favicon.ico')) return get404(); if (path.startsWith('/robots.txt')) return getRobots(); if (path.startsWith('/source')) return getSource({ env, daCtx }); - if (path.startsWith('/list')) return getList({ env, daCtx }); + if (path.startsWith('/list')) return getList({ req, env, daCtx }); if (path.startsWith('/config')) return getConfig({ env, daCtx }); if (path.startsWith('/versionlist')) return getVersionList({ env, daCtx }); if (path.startsWith('/versionsource')) return getVersionSource({ env, daCtx }); diff --git a/src/index.js b/src/index.js index ca62928d..47016bf3 100644 --- a/src/index.js +++ b/src/index.js @@ -36,7 +36,7 @@ export default { respObj = await headHandler({ env, daCtx }); break; case 'GET': - respObj = await getHandler({ env, daCtx }); + respObj = await getHandler({ req, env, daCtx }); break; case 'PUT': respObj = await postHandler({ req, env, daCtx }); diff --git a/src/routes/list.js b/src/routes/list.js index dc4ddfb0..0e3073a5 100644 --- a/src/routes/list.js +++ b/src/routes/list.js @@ -13,11 +13,20 @@ import listBuckets from '../storage/bucket/list.js'; import listObjects from '../storage/object/list.js'; import { getChildRules, hasPermission } from '../utils/auth.js'; -export default async function getList({ env, daCtx }) { +export default async function getList({ req, env, daCtx }) { if (!daCtx.org) return listBuckets(env, daCtx); if (!hasPermission(daCtx, daCtx.key, 'read')) return { status: 403 }; // Get the child rules of the current folder and store this in daCtx.aclCtx getChildRules(daCtx); - return /* await */ listObjects(env, daCtx); + + const { searchParams } = new URL(req.url); + const limit = Number.parseInt(searchParams.get('limit'), 10) ?? null; + const offset = Number.parseInt(searchParams.get('offset'), 10) ?? null; + + function numOrUndef(num) { + return Number.isNaN(num) ? undefined : num; + } + + return /* await */ listObjects(env, daCtx, numOrUndef(limit), numOrUndef(offset)); } diff --git a/src/storage/object/list.js b/src/storage/object/list.js index 9132fef2..5ae2446d 100644 --- a/src/storage/object/list.js +++ b/src/storage/object/list.js @@ -17,30 +17,56 @@ import { import getS3Config from '../utils/config.js'; import formatList from '../utils/list.js'; -function buildInput({ org, key, maxKeys }) { +function buildInput({ + org, key, maxKeys, continuationToken, +}) { const input = { Bucket: `${org}-content`, Prefix: key ? `${key}/` : null, Delimiter: '/', }; if (maxKeys) input.MaxKeys = maxKeys; + if (continuationToken) input.ContinuationToken = continuationToken; return input; } -export default async function listObjects(env, daCtx, maxKeys) { +async function scanFiles({ + daCtx, env, offset, limit, +}) { const config = getS3Config(env); const client = new S3Client(config); - const input = buildInput({ ...daCtx, maxKeys }); - const command = new ListObjectsV2Command(input); - try { + let continuationToken = null; + const visibleFiles = []; + + while (visibleFiles.length < offset + limit) { + const numKeys = Math.min(1000, (offset + limit) * 2); + + const input = buildInput({ ...daCtx, maxKeys: numKeys, continuationToken }); + const command = new ListObjectsV2Command(input); + const resp = await client.send(command); - // console.log(resp); - const body = formatList(resp, daCtx); + continuationToken = resp.NextContinuationToken; + visibleFiles.push(...formatList(resp, daCtx)); + + if (!continuationToken) break; + } + + return { + body: visibleFiles.slice(offset, offset + limit), + status: 200, + }; +} + +export default async function listObjects(env, daCtx, maxKeys = 1000, offset = 0) { + try { + const { body, status, contentType } = await scanFiles({ + daCtx, env, limit: maxKeys, offset, + }); return { body: JSON.stringify(body), - status: resp.$metadata.httpStatusCode, - contentType: resp.ContentType, + status, + contentType, }; } catch (e) { return { body: '', status: 404 }; From e68780af1dd7f6b88f50b2fbd60d0e217d05df62 Mon Sep 17 00:00:00 2001 From: Hannes Hertach Date: Thu, 10 Jul 2025 13:28:00 +0200 Subject: [PATCH 2/9] refactor pagination --- src/handlers/get.js | 4 +- src/routes/list-paginated.js | 32 ++++++++++ src/routes/list.js | 13 +--- src/storage/object/list.js | 51 ++++++++++++---- src/storage/utils/list.js | 111 +++++++++++++++++++++-------------- 5 files changed, 142 insertions(+), 69 deletions(-) create mode 100644 src/routes/list-paginated.js diff --git a/src/handlers/get.js b/src/handlers/get.js index 0f5f7c17..b8114faf 100644 --- a/src/handlers/get.js +++ b/src/handlers/get.js @@ -11,6 +11,7 @@ */ import { getSource } from '../routes/source.js'; import getList from '../routes/list.js'; +import getListV2 from '../routes/list-paginated.js'; import logout from '../routes/logout.js'; import { getConfig } from '../routes/config.js'; import { getVersionSource, getVersionList } from '../routes/version.js'; @@ -31,7 +32,8 @@ export default async function getHandler({ req, env, daCtx }) { if (path.startsWith('/robots.txt')) return getRobots(); if (path.startsWith('/source')) return getSource({ env, daCtx }); - if (path.startsWith('/list')) return getList({ req, env, daCtx }); + if (path.startsWith('/list-paginated')) return getListV2({ req, env, daCtx }); + if (path.startsWith('/list')) return getList({ env, daCtx }); if (path.startsWith('/config')) return getConfig({ env, daCtx }); if (path.startsWith('/versionlist')) return getVersionList({ env, daCtx }); if (path.startsWith('/versionsource')) return getVersionSource({ env, daCtx }); diff --git a/src/routes/list-paginated.js b/src/routes/list-paginated.js new file mode 100644 index 00000000..38d35723 --- /dev/null +++ b/src/routes/list-paginated.js @@ -0,0 +1,32 @@ +/* + * Copyright 2024 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ +import listBuckets from '../storage/bucket/list.js'; +import { listObjectsPaginated } from '../storage/object/list.js'; +import { getChildRules, hasPermission } from '../utils/auth.js'; + +export default async function getList({ req, env, daCtx }) { + if (!daCtx.org) return listBuckets(env, daCtx); + if (!hasPermission(daCtx, daCtx.key, 'read')) return { status: 403 }; + + // Get the child rules of the current folder and store this in daCtx.aclCtx + getChildRules(daCtx); + + const { searchParams } = new URL(req.url); + const limit = Number.parseInt(searchParams.get('limit'), 10) ?? null; + const offset = Number.parseInt(searchParams.get('offset'), 10) ?? null; + + function numOrUndef(num) { + return Number.isNaN(num) ? undefined : num; + } + + return /* await */ listObjectsPaginated(env, daCtx, numOrUndef(limit), numOrUndef(offset)); +} diff --git a/src/routes/list.js b/src/routes/list.js index 0e3073a5..dc4ddfb0 100644 --- a/src/routes/list.js +++ b/src/routes/list.js @@ -13,20 +13,11 @@ import listBuckets from '../storage/bucket/list.js'; import listObjects from '../storage/object/list.js'; import { getChildRules, hasPermission } from '../utils/auth.js'; -export default async function getList({ req, env, daCtx }) { +export default async function getList({ env, daCtx }) { if (!daCtx.org) return listBuckets(env, daCtx); if (!hasPermission(daCtx, daCtx.key, 'read')) return { status: 403 }; // Get the child rules of the current folder and store this in daCtx.aclCtx getChildRules(daCtx); - - const { searchParams } = new URL(req.url); - const limit = Number.parseInt(searchParams.get('limit'), 10) ?? null; - const offset = Number.parseInt(searchParams.get('offset'), 10) ?? null; - - function numOrUndef(num) { - return Number.isNaN(num) ? undefined : num; - } - - return /* await */ listObjects(env, daCtx, numOrUndef(limit), numOrUndef(offset)); + return /* await */ listObjects(env, daCtx); } diff --git a/src/storage/object/list.js b/src/storage/object/list.js index 5ae2446d..572b5d61 100644 --- a/src/storage/object/list.js +++ b/src/storage/object/list.js @@ -15,7 +15,9 @@ import { } from '@aws-sdk/client-s3'; import getS3Config from '../utils/config.js'; -import formatList from '../utils/list.js'; +import formatList, { formatPaginatedList } from '../utils/list.js'; + +const LIST_LIMIT = 5000; function buildInput({ org, key, maxKeys, continuationToken, @@ -40,33 +42,58 @@ async function scanFiles({ const visibleFiles = []; while (visibleFiles.length < offset + limit) { - const numKeys = Math.min(1000, (offset + limit) * 2); + const remainingKeys = offset + limit - visibleFiles.length; + // fetch 25 extra to account for some hidden files + const numKeysToFetch = Math.min(1000, remainingKeys + 25); - const input = buildInput({ ...daCtx, maxKeys: numKeys, continuationToken }); + const input = buildInput({ ...daCtx, maxKeys: numKeysToFetch, continuationToken }); const command = new ListObjectsV2Command(input); const resp = await client.send(command); continuationToken = resp.NextContinuationToken; - visibleFiles.push(...formatList(resp, daCtx)); + visibleFiles.push(...formatPaginatedList(resp, daCtx)); if (!continuationToken) break; } - return { - body: visibleFiles.slice(offset, offset + limit), - status: 200, - }; + return visibleFiles.slice(offset, offset + limit); } -export default async function listObjects(env, daCtx, maxKeys = 1000, offset = 0) { +export async function listObjectsPaginated(env, daCtx, maxKeys = 1000, offset = 0) { + if (offset + maxKeys > LIST_LIMIT) { + return { status: 400 }; + } + try { - const { body, status, contentType } = await scanFiles({ + const files = await scanFiles({ daCtx, env, limit: maxKeys, offset, }); + return { + body: JSON.stringify({ + offset, + limit: maxKeys, + data: files, + }), + status: 200, + }; + } catch (e) { + return { body: '', status: 404 }; + } +} + +export default async function listObjects(env, daCtx, maxKeys) { + const config = getS3Config(env); + const client = new S3Client(config); + + const input = buildInput({ ...daCtx, maxKeys }); + const command = new ListObjectsV2Command(input); + try { + const resp = await client.send(command); + const body = formatList(resp, daCtx); return { body: JSON.stringify(body), - status, - contentType, + status: resp.$metadata.httpStatusCode, + contentType: resp.ContentType, }; } catch (e) { return { body: '', status: 404 }; diff --git a/src/storage/utils/list.js b/src/storage/utils/list.js index 5d6e5ee9..fa21c128 100644 --- a/src/storage/utils/list.js +++ b/src/storage/utils/list.js @@ -13,6 +13,68 @@ import { ListObjectsV2Command, } from '@aws-sdk/client-s3'; +function mapPrefixes(CommonPrefixes, daCtx) { + return CommonPrefixes?.map((prefix) => { + const name = prefix.Prefix.slice(0, -1).split('/').pop(); + const splitName = name.split('.'); + + // Do not add any extension folders + if (splitName.length > 1) return null; + + const path = `/${daCtx.org}/${prefix.Prefix.slice(0, -1)}`; + + return { path, name }; + }).filter((x) => !!x) ?? []; +} + +function mapContents(Contents, folders, daCtx) { + return Contents?.map((content) => { + const itemName = content.Key.split('/').pop(); + const splitName = itemName.split('.'); + // file.jpg.props should not be a part of the list + // hidden files (.props) should not be a part of this list + if (splitName.length !== 2) return null; + + const [name, ext, props] = splitName; + + // Do not show any props sidecar files + if (props) return null; + + // See if the folder is already in the list + if (ext === 'props') { + if (folders.some((item) => item.name === name)) return null; + + // Remove props from the key so it can look like a folder + // eslint-disable-next-line no-param-reassign + content.Key = content.Key.replace('.props', ''); + } + + // Do not show any hidden files. + if (!name) return null; + const item = { path: `/${daCtx.org}/${content.Key}`, name }; + if (ext !== 'props') { + item.ext = ext; + item.lastModified = content.LastModified.getTime(); + } + + return item; + }).filter((x) => !!x) ?? []; +} + +// Performs the same as formatList, but doesn't sort +// This prevents bugs when sorting across pages of the paginated api response +// However, the order is slightly different to the formatList return value +export function formatPaginatedList(resp, daCtx) { + const { Contents } = resp; + + const combined = []; + + const files = mapContents(Contents, [], daCtx); + combined.push(...files); + + return combined; +} + export default function formatList(resp, daCtx) { function compare(a, b) { if (a.name < b.name) return -1; @@ -24,52 +86,11 @@ export default function formatList(resp, daCtx) { const combined = []; - if (CommonPrefixes) { - CommonPrefixes.forEach((prefix) => { - const name = prefix.Prefix.slice(0, -1).split('/').pop(); - const splitName = name.split('.'); + const folders = mapPrefixes(CommonPrefixes, daCtx); + combined.push(...folders); - // Do not add any extension folders - if (splitName.length > 1) return; - - const path = `/${daCtx.org}/${prefix.Prefix.slice(0, -1)}`; - combined.push({ path, name }); - }); - } - - if (Contents) { - Contents.forEach((content) => { - const itemName = content.Key.split('/').pop(); - const splitName = itemName.split('.'); - // file.jpg.props should not be a part of the list - // hidden files (.props) should not be a part of this list - if (splitName.length !== 2) return; - - const [name, ext, props] = splitName; - - // Do not show any props sidecar files - if (props) return; - - // See if the folder is already in the list - if (ext === 'props') { - if (combined.some((item) => item.name === name)) return; - - // Remove props from the key so it can look like a folder - // eslint-disable-next-line no-param-reassign - content.Key = content.Key.replace('.props', ''); - } - - // Do not show any hidden files. - if (!name) return; - const item = { path: `/${daCtx.org}/${content.Key}`, name }; - if (ext !== 'props') { - item.ext = ext; - item.lastModified = content.LastModified.getTime(); - } - - combined.push(item); - }); - } + const files = mapContents(Contents, folders, daCtx); + combined.push(...files); return combined.sort(compare); } From 7f310e8541588ae669921a804638b3b170a5698f Mon Sep 17 00:00:00 2001 From: Hannes Hertach Date: Thu, 10 Jul 2025 13:34:32 +0200 Subject: [PATCH 3/9] rename paginated list api --- src/handlers/get.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/handlers/get.js b/src/handlers/get.js index b8114faf..3f16c13a 100644 --- a/src/handlers/get.js +++ b/src/handlers/get.js @@ -11,7 +11,7 @@ */ import { getSource } from '../routes/source.js'; import getList from '../routes/list.js'; -import getListV2 from '../routes/list-paginated.js'; +import getListPaginated from '../routes/list-paginated.js'; import logout from '../routes/logout.js'; import { getConfig } from '../routes/config.js'; import { getVersionSource, getVersionList } from '../routes/version.js'; @@ -32,7 +32,7 @@ export default async function getHandler({ req, env, daCtx }) { if (path.startsWith('/robots.txt')) return getRobots(); if (path.startsWith('/source')) return getSource({ env, daCtx }); - if (path.startsWith('/list-paginated')) return getListV2({ req, env, daCtx }); + if (path.startsWith('/list-paginated')) return getListPaginated({ req, env, daCtx }); if (path.startsWith('/list')) return getList({ env, daCtx }); if (path.startsWith('/config')) return getConfig({ env, daCtx }); if (path.startsWith('/versionlist')) return getVersionList({ env, daCtx }); From bd4f1e04f3e52f3f35c95456b689236c001f5dc7 Mon Sep 17 00:00:00 2001 From: Hannes Hertach Date: Thu, 10 Jul 2025 16:15:43 +0200 Subject: [PATCH 4/9] add tests to paginated list --- test/storage/object/list.test.js | 127 ++++++++++++++++++++++++++++++- 1 file changed, 124 insertions(+), 3 deletions(-) diff --git a/test/storage/object/list.test.js b/test/storage/object/list.test.js index 032e11d7..f00046c1 100644 --- a/test/storage/object/list.test.js +++ b/test/storage/object/list.test.js @@ -16,9 +16,11 @@ import { mockClient } from 'aws-sdk-client-mock'; const s3Mock = mockClient(S3Client); -import listObjects from '../../../src/storage/object/list.js'; +import listObjects, {listObjectsPaginated} from '../../../src/storage/object/list.js'; const Contents = [ + { Key: 'wknd/abc1234.html', LastModified: new Date() }, + { Key: 'wknd/abc123.html', LastModified: new Date() }, { Key: 'wknd/index.html', LastModified: new Date() }, { Key: 'wknd/nav.html', LastModified: new Date() }, { Key: 'wknd/footer.html', LastModified: new Date() }, @@ -40,7 +42,7 @@ describe('List Objects', () => { const daCtx = { org: 'adobe', key: 'wknd' }; const resp = await listObjects({}, daCtx); const data = JSON.parse(resp.body); - assert.strictEqual(data.length, 3); + assert.strictEqual(data.length, Contents.length); assert(data.every((item) => item.ext && item.lastModified)); }); @@ -57,4 +59,123 @@ describe('List Objects', () => { const data = JSON.parse(resp.body); assert.strictEqual(data.length, 2, 'Should only return 2 items'); }); -}) + + it('sorts the results', async () => { + s3Mock.on(ListObjectsV2Command, { + Bucket: 'adobe-content', + Prefix: 'wknd/', + Delimiter: '/', + }).resolves({ $metadata: { httpStatusCode: 200 }, Contents }); + + const daCtx = { org: 'adobe', key: 'wknd' }; + const resp = await listObjects({}, daCtx); + const data = JSON.parse(resp.body); + + const firstIndex = data.findIndex((x) => x.name === 'abc123'); + const secondIndex = data.findIndex((x) => x.name === 'abc1234'); + assert.strictEqual(true, firstIndex < secondIndex); + }); +}); + +describe('list paginated objects', async () => { + it('correctly handles continuation token', async () => { + s3Mock.on(ListObjectsV2Command, { + Bucket: 'adobe-content', + Prefix: 'wknd/', + Delimiter: '/', + }).resolves({ + $metadata: { httpStatusCode: 200 }, + Contents: [Contents[0], Contents[1]], + NextContinuationToken: 'token' + }); + + s3Mock.on(ListObjectsV2Command, { + Bucket: 'adobe-content', + Prefix: 'wknd/', + Delimiter: '/', + ContinuationToken: 'token' + }).resolves({ $metadata: { httpStatusCode: 200 }, Contents: [Contents[2], Contents[3]] }); + + const daCtx = { org: 'adobe', key: 'wknd' }; + const resp = await listObjectsPaginated({}, daCtx); + const { data, limit, offset } = JSON.parse(resp.body); + assert.strictEqual(data.length, 4, 'Should return all items'); + assert.strictEqual(limit, 1000, 'Should use default limit if no limit passed'); + assert.strictEqual(offset, 0, 'Should use default offset if no limit passed'); + }); + + it('correctly passes limit and offset', async () => { + s3Mock.on(ListObjectsV2Command, { + Bucket: 'adobe-content', + Prefix: 'wknd/', + Delimiter: '/', + MaxKeys: 27, + }).resolves({ + $metadata: { httpStatusCode: 200 }, + Contents: Contents, + NextContinuationToken: 'token', + }); + + const daCtx = { org: 'adobe', key: 'wknd' }; + const resp = await listObjectsPaginated({}, daCtx, 2, 1); + const { data, limit, offset } = JSON.parse(resp.body); + assert.strictEqual(data.length, 2, 'Should return 2 items'); + assert.strictEqual(data[1].name, 'index', 'Should return correct items'); + assert.strictEqual(limit, 2, 'Should use default limit if no limit passed'); + assert.strictEqual(offset, 1, 'Should use default offset if no limit passed'); + }); + + it('fetches more until enough files are present', async () => { + s3Mock.on(ListObjectsV2Command, { + Bucket: 'adobe-content', + Prefix: 'wknd/', + Delimiter: '/', + MaxKeys: 29, + }).resolves({ + $metadata: { httpStatusCode: 200 }, + Contents: new Array(29).fill({ Key: '.ignored', LastModified: new Date() }), + NextContinuationToken: 'token', + }); + + s3Mock.on(ListObjectsV2Command, { + Bucket: 'adobe-content', + Prefix: 'wknd/', + Delimiter: '/', + MaxKeys: 29, + ContinuationToken: 'token', + }).resolves({ + $metadata: { httpStatusCode: 200 }, + Contents: Contents, + NextContinuationToken: 'token', + }); + + const daCtx = { org: 'adobe', key: 'wknd' }; + const resp = await listObjectsPaginated({}, daCtx, 4, 0); + const { data } = JSON.parse(resp.body); + assert.strictEqual(data.length, 4, 'Should return 4 items'); + }); + + it('doesn\'t sort the results', async () => { + s3Mock.on(ListObjectsV2Command, { + Bucket: 'adobe-content', + Prefix: 'wknd/', + Delimiter: '/', + }).resolves({ $metadata: { httpStatusCode: 200 }, Contents }); + + const daCtx = { org: 'adobe', key: 'wknd' }; + const resp = await listObjectsPaginated({}, daCtx); + const data = JSON.parse(resp.body).data; + + const firstIndex = data.findIndex((x) => x.name === 'abc1234'); + const secondIndex = data.findIndex((x) => x.name === 'abc123'); + assert.strictEqual(true, firstIndex < secondIndex); + }); + + it('enforces size limit', async () => { + const daCtx = { org: 'adobe', key: 'wknd' }; + const resp1 = await listObjectsPaginated({}, daCtx, 5001, 0); + assert.strictEqual(resp1.status, 400); + const resp2 = await listObjectsPaginated({}, daCtx, 500, 4501); + assert.strictEqual(resp2.status, 400); + }); +}); From 1e7ae07383add72aebbe774766607cf2fde636a8 Mon Sep 17 00:00:00 2001 From: Hannes Hertach Date: Thu, 10 Jul 2025 17:15:49 +0200 Subject: [PATCH 5/9] add test for paginated list route --- src/routes/list-paginated.js | 2 +- test/routes/list-paginated.test.js | 95 ++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 test/routes/list-paginated.test.js diff --git a/src/routes/list-paginated.js b/src/routes/list-paginated.js index 38d35723..df33ab2f 100644 --- a/src/routes/list-paginated.js +++ b/src/routes/list-paginated.js @@ -13,7 +13,7 @@ import listBuckets from '../storage/bucket/list.js'; import { listObjectsPaginated } from '../storage/object/list.js'; import { getChildRules, hasPermission } from '../utils/auth.js'; -export default async function getList({ req, env, daCtx }) { +export default async function getListPaginated({ req, env, daCtx }) { if (!daCtx.org) return listBuckets(env, daCtx); if (!hasPermission(daCtx, daCtx.key, 'read')) return { status: 403 }; diff --git a/test/routes/list-paginated.test.js b/test/routes/list-paginated.test.js new file mode 100644 index 00000000..b951e6b8 --- /dev/null +++ b/test/routes/list-paginated.test.js @@ -0,0 +1,95 @@ +/* + * Copyright 2025 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ +import assert from 'assert'; +import esmock from 'esmock'; + +describe('List Route', () => { + it('Test getListPaginated with permissions', async () => { + const loCalled = []; + const listObjectsPaginated = (e, c) => { + loCalled.push({ e, c }); + return {}; + } + + const ctx = { org: 'foo', key: 'q/q/q' }; + const hasPermission = (c, k, a) => { + if (k === 'q/q/q' && a === 'read') { + return false; + } + return true; + } + + const getListPaginated = await esmock( + '../../src/routes/list-paginated.js', { + '../../src/storage/object/list.js': { + listObjectsPaginated + }, + '../../src/utils/auth.js': { + hasPermission + } + } + ); + + const req = { + url: new URL('https://admin.da.live/list/foo/bar'), + } + + const resp = await getListPaginated({ req, env: {}, daCtx: ctx, aclCtx: {} }); + assert.strictEqual(403, resp.status); + assert.strictEqual(0, loCalled.length); + + const aclCtx = { pathLookup: new Map() }; + await getListPaginated({ req, env: {}, daCtx: { org: 'bar', key: 'q/q', users: [], aclCtx }}); + assert.strictEqual(1, loCalled.length); + assert.strictEqual('q/q', loCalled[0].c.key); + + const childRules = aclCtx.childRules; + assert.strictEqual(1, childRules.length); + assert(childRules[0].startsWith('/q/q/**='), 'Should have defined some child rule'); + }); + + it('parses request params', async () => { + const loCalled = []; + const listObjectsPaginated = (e, c, limit, offset) => { + console.log({offset, limit}) + loCalled.push({ offset, limit }); + return {}; + } + + const hasPermission = () => true; + + const getListPaginated = await esmock( + '../../src/routes/list-paginated.js', { + '../../src/storage/object/list.js': { + listObjectsPaginated + }, + '../../src/utils/auth.js': { + hasPermission, + getChildRules: () => {} + } + } + ); + + const ctx = { org: 'foo', }; + const reqs = [ + { url: 'https://admin.da.live/list/foo/bar?limit=12&offset=1' }, + { url: 'https://admin.da.live/list/foo/bar?limit=asdf&offset=17' }, + { url: 'https://admin.da.live/list/foo/bar?limit=12&offset=asdf' }, + ]; + await getListPaginated({ req: reqs[0], env: {}, daCtx: ctx, aclCtx: {} }); + assert.deepStrictEqual(loCalled[0], { limit: 12, offset: 1 }); + await getListPaginated({ req: reqs[1], env: {}, daCtx: ctx, aclCtx: {} }); + assert.deepStrictEqual(loCalled[1], { limit: undefined, offset: 17 }); + await getListPaginated({ req: reqs[2], env: {}, daCtx: ctx, aclCtx: {} }); + assert.deepStrictEqual(loCalled[2], { limit: 12, offset: undefined }); + }); +}); From 39b03f0fc6e24bada6aae8efe3fba4563062a032 Mon Sep 17 00:00:00 2001 From: Hannes Hertach Date: Thu, 10 Jul 2025 19:46:16 +0200 Subject: [PATCH 6/9] add test for mapping --- test/storage/utils.t.js | 65 ------------------------ test/storage/utils.test.js | 100 +++++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 65 deletions(-) delete mode 100644 test/storage/utils.t.js create mode 100644 test/storage/utils.test.js diff --git a/test/storage/utils.t.js b/test/storage/utils.t.js deleted file mode 100644 index 6148f275..00000000 --- a/test/storage/utils.t.js +++ /dev/null @@ -1,65 +0,0 @@ -/* eslint-env mocha */ -import assert from 'assert'; - -import { getDaCtx } from '../../src/utils/daCtx.js'; -import formatList from '../../src/storage/utils/list.js'; - -const MOCK = { - CommonPrefixes: [ - { Prefix: 'blog/' }, - { Prefix: 'da-aem-boilerplate/' }, - { Prefix: 'da/' }, - { Prefix: 'dac/' }, - { Prefix: 'milo/' }, - { Prefix: 'dark-alley.jpg/' }, - ], - Contents: [ - { - Key: 'blog.props', - }, - { - Key: 'da.props', - }, - { - Key: 'folder-only.props', - }, - { - Key: 'test.html', - }, - { - Key: 'dark-alley.jpg.props', - }, - { - Key: 'dark-alley.jpg', - } - ], -}; - -const daCtx = getDaCtx('/source/adobecom'); - -describe('Format object list', () => { - const list = formatList(MOCK, daCtx); - - it('should return a true folder / common prefix', () => { - assert.strictEqual(list[0].name, 'blog'); - }); - - it('should return a contents-based folder', () => { - const folderOnly = list.find((item) => { return item.name === 'folder-only' }); - assert.strictEqual(folderOnly.name, 'folder-only'); - }); - - it('should not return a props file of same folder name', () => { - const found = list.reduce((acc, item) => { - if (item.name === 'blog') acc.push(item); - return acc; - },[]); - - assert.strictEqual(found.length, 1); - }); - - it('should not have a filename props file in the list', () => { - const propsSidecar = list.find((item) => { return item.name === 'dark-alley.jpg.props' }); - assert.strictEqual(propsSidecar, undefined); - }); -}); diff --git a/test/storage/utils.test.js b/test/storage/utils.test.js new file mode 100644 index 00000000..17bf6cfa --- /dev/null +++ b/test/storage/utils.test.js @@ -0,0 +1,100 @@ +/* eslint-env mocha */ +import assert from 'assert'; + +import getDaCtx from '../../src/utils/daCtx.js'; +import formatList, { formatPaginatedList } from '../../src/storage/utils/list.js'; + +function getMock() { + return { + CommonPrefixes: [ + { Prefix: 'blog/' }, + { Prefix: 'da-aem-boilerplate/' }, + { Prefix: 'da/' }, + { Prefix: 'dac/' }, + { Prefix: 'milo/' }, + { Prefix: 'dark-alley.jpg/' }, + ], + Contents: [ + { + Key: 'blog.props', + LastModified: new Date(), + }, + { + Key: 'da.props', + LastModified: new Date(), + }, + { + Key: 'folder-only.props', + LastModified: new Date(), + }, + { + Key: 'test.html', + LastModified: new Date(), + }, + { + Key: 'dark-alley.jpg.props', + LastModified: new Date(), + }, + { + Key: 'dark-alley.jpg', + LastModified: new Date(), + } + ], + }; +} + +const daCtx = getDaCtx('/source/adobecom'); + +describe('Format object list', () => { + const list = formatList(getMock(), daCtx); + + it('should return a true folder / common prefix', () => { + assert.strictEqual(list[0].name, 'blog'); + }); + + it('should return a contents-based folder', () => { + const folderOnly = list.find((item) => { return item.name === 'folder-only' }); + assert.strictEqual(folderOnly.name, 'folder-only'); + }); + + it('should not return a props file of same folder name', () => { + const found = list.reduce((acc, item) => { + if (item.name === 'blog') acc.push(item); + return acc; + },[]); + + assert.strictEqual(found.length, 1); + }); + + it('should not have a filename props file in the list', () => { + const propsSidecar = list.find((item) => { return item.name === 'dark-alley.jpg.props' }); + assert.strictEqual(propsSidecar, undefined); + }); +}); + +describe('format paginated object list', () => { + const list = formatPaginatedList(getMock(), daCtx); + + it('should return a true folder / common prefix', () => { + assert.strictEqual(list[0].name, 'blog'); + }); + + it('should return a contents-based folder', () => { + const folderOnly = list.find((item) => { return item.name === 'folder-only' }); + assert.strictEqual(folderOnly.name, 'folder-only'); + }); + + it('should not return a props file of same folder name', () => { + const found = list.reduce((acc, item) => { + if (item.name === 'blog') acc.push(item); + return acc; + },[]); + + assert.strictEqual(found.length, 1); + }); + + it('should not have a filename props file in the list', () => { + const propsSidecar = list.find((item) => { return item.name === 'dark-alley.jpg.props' }); + assert.strictEqual(propsSidecar, undefined); + }); +}); From 198698a3f34b626e9d781062717ba103446f9e77 Mon Sep 17 00:00:00 2001 From: Hannes Hertach Date: Thu, 10 Jul 2025 19:48:48 +0200 Subject: [PATCH 7/9] fix test --- test/storage/utils.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/storage/utils.test.js b/test/storage/utils.test.js index 17bf6cfa..3c7a9ffc 100644 --- a/test/storage/utils.test.js +++ b/test/storage/utils.test.js @@ -43,7 +43,7 @@ function getMock() { }; } -const daCtx = getDaCtx('/source/adobecom'); +const daCtx = { url: 'https://admin.da.live/list/foo/bar' }; describe('Format object list', () => { const list = formatList(getMock(), daCtx); From 57d6a0a22f2fdbf717f3536671b9c72b5adf2ada Mon Sep 17 00:00:00 2001 From: Hannes Hertach Date: Mon, 21 Jul 2025 15:09:14 +0200 Subject: [PATCH 8/9] fix test coverage --- test/storage/object/delete.test.js | 10 --------- test/utils/auth.test.js | 34 +++++++++++++++++++++++------- 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/test/storage/object/delete.test.js b/test/storage/object/delete.test.js index c6259da0..2577e1be 100644 --- a/test/storage/object/delete.test.js +++ b/test/storage/object/delete.test.js @@ -252,11 +252,6 @@ describe('Object delete', () => { getSignedUrl: mockSignedUrl, } }, - { - import: { - fetch: async () => ({ status: 200 }), - } - } ); s3Mock.on(ListObjectsV2Command).resolves({ Contents: [{ Key: 'foo/bar.html' }] }); const resp = await deleteObjects(env, daCtx, {}); @@ -287,11 +282,6 @@ describe('Object delete', () => { getSignedUrl: mockSignedUrl, } }, - { - import: { - fetch: async () => ({ status: 200 }), - } - } ); s3Mock.on(ListObjectsV2Command).resolves({ Contents: [{ Key: 'foo/bar.html' }], NextContinuationToken: 'token' }); const resp = await deleteObjects(env, daCtx, {}); diff --git a/test/utils/auth.test.js b/test/utils/auth.test.js index 551ef7ce..cb296adb 100644 --- a/test/utils/auth.test.js +++ b/test/utils/auth.test.js @@ -29,7 +29,17 @@ import { const { setUser, getUsers, -} = await esmock('../../src/utils/auth.js', { jose, import: { fetch } }); +} = await esmock('../../src/utils/auth.js', { jose }); + +async function withMockedFetch(act) { + const savedFetch = globalThis.fetch; + globalThis.fetch = fetch; + try { + await act(); + } finally { + globalThis.fetch = savedFetch; + } +} describe('DA auth', () => { describe('get user', async () => { @@ -49,14 +59,18 @@ describe('DA auth', () => { }); it('authorized if email matches', async () => { - const users = await getUsers(reqs.site, env); - assert.strictEqual(users[0].email, 'aparker@geometrixx.info'); + await withMockedFetch(async () => { + const users = await getUsers(reqs.site, env); + assert.strictEqual(users[0].email, 'aparker@geometrixx.info'); + }); }); it('authorized with user if email matches and anonymous if present', async () => { - const users = await getUsers(reqs.siteMulti, env); - assert.strictEqual(users[0].email, 'anonymous') - assert.strictEqual(users[1].email, 'aparker@geometrixx.info'); + await withMockedFetch(async () => { + const users = await getUsers(reqs.siteMulti, env); + assert.strictEqual(users[0].email, 'anonymous') + assert.strictEqual(users[1].email, 'aparker@geometrixx.info'); + }) }); it('anonymous if ims fails', async () => { @@ -71,8 +85,12 @@ describe('DA auth', () => { 'Authorization': `Bearer aparker@geometrixx.info`, }); - const userValStr = await setUser('aparker@geometrixx.info', 100, headers, env); - const userValue = JSON.parse(userValStr); + let userValue; + + await withMockedFetch(async () => { + const userValStr = await setUser('aparker@geometrixx.info', 100, headers, env); + userValue = JSON.parse(userValStr); + }); assert.strictEqual('aparker@geometrixx.info', userValue.email); assert.strictEqual('123', userValue.ident); From 9284e612d0294349533152c5e6951dfaf05a1c48 Mon Sep 17 00:00:00 2001 From: Hannes Hertach Date: Tue, 22 Jul 2025 11:59:26 +0200 Subject: [PATCH 9/9] fix empty folders not showing --- src/routes/list-paginated.js | 4 +- src/storage/object/list.js | 11 ++- src/storage/utils/list.js | 40 ++++++---- test/storage/object/list.test.js | 4 +- test/storage/utils/list.test.js | 125 ++++++++++++++++++++++++++++++- 5 files changed, 159 insertions(+), 25 deletions(-) diff --git a/src/routes/list-paginated.js b/src/routes/list-paginated.js index df33ab2f..ea45bfe8 100644 --- a/src/routes/list-paginated.js +++ b/src/routes/list-paginated.js @@ -21,8 +21,8 @@ export default async function getListPaginated({ req, env, daCtx }) { getChildRules(daCtx); const { searchParams } = new URL(req.url); - const limit = Number.parseInt(searchParams.get('limit'), 10) ?? null; - const offset = Number.parseInt(searchParams.get('offset'), 10) ?? null; + const limit = Number.parseInt(searchParams.get('limit'), 10); + const offset = Number.parseInt(searchParams.get('offset'), 10); function numOrUndef(num) { return Number.isNaN(num) ? undefined : num; diff --git a/src/storage/object/list.js b/src/storage/object/list.js index 572b5d61..4414da4a 100644 --- a/src/storage/object/list.js +++ b/src/storage/object/list.js @@ -39,11 +39,13 @@ async function scanFiles({ const client = new S3Client(config); let continuationToken = null; - const visibleFiles = []; + let visibleFiles = []; + const fetchedItems = []; + const fetchedPrefixes = []; while (visibleFiles.length < offset + limit) { const remainingKeys = offset + limit - visibleFiles.length; - // fetch 25 extra to account for some hidden files + // fetch 25 extra to account for some hidden files (reduce likelihood of continuation token) const numKeysToFetch = Math.min(1000, remainingKeys + 25); const input = buildInput({ ...daCtx, maxKeys: numKeysToFetch, continuationToken }); @@ -51,7 +53,10 @@ async function scanFiles({ const resp = await client.send(command); continuationToken = resp.NextContinuationToken; - visibleFiles.push(...formatPaginatedList(resp, daCtx)); + + fetchedItems.push(...(resp.Contents ?? [])); + fetchedPrefixes.push(...(resp.CommonPrefixes ?? [])); + visibleFiles = formatPaginatedList(fetchedItems, fetchedPrefixes, daCtx); if (!continuationToken) break; } diff --git a/src/storage/utils/list.js b/src/storage/utils/list.js index 372e9654..1749931d 100644 --- a/src/storage/utils/list.js +++ b/src/storage/utils/list.js @@ -29,7 +29,8 @@ function mapPrefixes(CommonPrefixes, daCtx) { function mapContents(Contents, folders, daCtx) { return Contents?.map((content) => { - const itemName = content.Key.split('/').pop(); + let key = content.Key; + const itemName = key.split('/').pop(); const splitName = itemName.split('.'); // file.jpg.props should not be a part of the list // hidden files (.props) should not be a part of this list @@ -45,13 +46,12 @@ function mapContents(Contents, folders, daCtx) { if (folders.some((item) => item.name === name && !item.ext)) return null; // Remove props from the key so it can look like a folder - // eslint-disable-next-line no-param-reassign - content.Key = content.Key.replace('.props', ''); + key = key.replace('.props', ''); } // Do not show any hidden files. if (!name) return null; - const item = { path: `/${daCtx.org}/${content.Key}`, name }; + const item = { path: `/${daCtx.org}/${key}`, name }; if (ext !== 'props') { item.ext = ext; item.lastModified = content.LastModified.getTime(); @@ -61,18 +61,28 @@ function mapContents(Contents, folders, daCtx) { }).filter((x) => !!x) ?? []; } -// Performs the same as formatList, but doesn't sort +// Performs the same as formatList, but doesn't sort (returns exactly how it was +// sorted in the S3 client response) // This prevents bugs when sorting across pages of the paginated api response // However, the order is slightly different to the formatList return value -export function formatPaginatedList(resp, daCtx) { - const { Contents } = resp; +// for strings with the same prefix but different length +export function formatPaginatedList(items, prefixes, daCtx) { + function compare(a, b) { + const aN = a.name; + const bN = b.name; + if (aN.startsWith(bN) || bN.startsWith(aN)) return bN.length - aN.length; + if (aN < bN) return -1; + if (aN > bN) return 1; + return undefined; + } - const combined = []; + const folders = mapPrefixes(prefixes, daCtx); + const files = mapContents(items, folders, daCtx); - const files = mapContents(Contents, [], daCtx); - combined.push(...files); + const combined = []; + combined.push(...files, ...folders); - return combined; + return combined.sort(compare); } export default function formatList(resp, daCtx) { @@ -84,13 +94,11 @@ export default function formatList(resp, daCtx) { const { CommonPrefixes, Contents } = resp; - const combined = []; - const folders = mapPrefixes(CommonPrefixes, daCtx); - combined.push(...folders); - const files = mapContents(Contents, folders, daCtx); - combined.push(...files); + + const combined = []; + combined.push(...files, ...folders); return combined.sort(compare); } diff --git a/test/storage/object/list.test.js b/test/storage/object/list.test.js index f00046c1..529660ac 100644 --- a/test/storage/object/list.test.js +++ b/test/storage/object/list.test.js @@ -60,7 +60,7 @@ describe('List Objects', () => { assert.strictEqual(data.length, 2, 'Should only return 2 items'); }); - it('sorts the results', async () => { + it('sorts the results with shorter prefixes first', async () => { s3Mock.on(ListObjectsV2Command, { Bucket: 'adobe-content', Prefix: 'wknd/', @@ -155,7 +155,7 @@ describe('list paginated objects', async () => { assert.strictEqual(data.length, 4, 'Should return 4 items'); }); - it('doesn\'t sort the results', async () => { + it('sorts the results with longer prefixes first', async () => { s3Mock.on(ListObjectsV2Command, { Bucket: 'adobe-content', Prefix: 'wknd/', diff --git a/test/storage/utils/list.test.js b/test/storage/utils/list.test.js index d536ff28..3caa1939 100644 --- a/test/storage/utils/list.test.js +++ b/test/storage/utils/list.test.js @@ -59,7 +59,8 @@ const req = new Request('https://example.com/source/adobecom'); const daCtx = getDaCtx(req, {}); describe('Format object list', () => { - const list = formatList(getMock(), daCtx); + const mockInput = getMock(); + const list = formatList(mockInput, daCtx); it('should return a true folder / common prefix', () => { assert.strictEqual(list[0].name, 'blog'); @@ -194,6 +195,10 @@ describe('Format object list', () => { assert.strictEqual(result[1].name, 'beta'); assert.strictEqual(result[2].name, 'zebra'); }); + + it('formatting should not have side effects', () => { + assert.deepStrictEqual(mockInput, getMock()); + }); }); describe('listCommand', () => { @@ -303,7 +308,8 @@ describe('listCommand', () => { }); describe('format paginated object list', () => { - const list = formatPaginatedList(getMock(), daCtx); + const mockInput = getMock(); + const list = formatPaginatedList(mockInput.Contents, mockInput.CommonPrefixes, daCtx); it('should return a true folder / common prefix', () => { assert.strictEqual(list[0].name, 'blog'); @@ -327,4 +333,119 @@ describe('format paginated object list', () => { const propsSidecar = list.find((item) => { return item.name === 'dark-alley.jpg.props' }); assert.strictEqual(propsSidecar, undefined); }); + + it('should handle empty folders with sibling file names of same name', () => { + const filtered = list.filter((item) => { return item.name === 'empty-folder-with-sibling-file' }); + assert.strictEqual(filtered.length, 2); + }); + + it('should handle empty CommonPrefixes', () => { + const emptyMock = { Contents: getMock().Contents }; + const result = formatList(emptyMock, daCtx); + assert(Array.isArray(result)); + assert(result.length > 0); + }); + + it('should handle empty Contents', () => { + const emptyMock = { CommonPrefixes: getMock().CommonPrefixes }; + const result = formatList(emptyMock, daCtx); + assert(Array.isArray(result)); + assert(result.length > 0); + }); + + it('should handle both empty CommonPrefixes and Contents', () => { + const emptyMock = {}; + const result = formatList(emptyMock, daCtx); + assert(Array.isArray(result)); + assert.strictEqual(result.length, 0); + }); + + it('should filter out extension folders from CommonPrefixes', () => { + const mockWithExtensionFolder = { + CommonPrefixes: [ + { Prefix: 'file.jpg/' }, + { Prefix: 'normal-folder/' } + ] + }; + const result = formatList(mockWithExtensionFolder, daCtx); + const extensionFolder = result.find(item => item.name === 'file.jpg'); + assert.strictEqual(extensionFolder, undefined); + const normalFolder = result.find(item => item.name === 'normal-folder'); + assert(normalFolder); + }); + + it('should handle files with more than 2 dot separators', () => { + const mockWithComplexFile = { + Contents: [ + { + Key: 'file.name.with.multiple.dots', + LastModified: new Date('2025-01-01'), + } + ] + }; + const result = formatList(mockWithComplexFile, daCtx); + assert.strictEqual(result.length, 0); + }); + + it('should handle hidden files (starting with dot)', () => { + const mockWithHiddenFile = { + Contents: [ + { + Key: '.hidden-file', + LastModified: new Date('2025-01-01'), + } + ] + }; + const result = formatList(mockWithHiddenFile, daCtx); + assert.strictEqual(result.length, 0); + }); + + it('should handle files with props extension correctly', () => { + const mockWithProps = { + Contents: [ + { + Key: 'test.props', + LastModified: new Date('2025-01-01'), + } + ] + }; + const result = formatList(mockWithProps, daCtx); + const propsItem = result.find(item => item.name === 'test'); + assert(propsItem); + assert.strictEqual(propsItem.ext, undefined); + assert.strictEqual(propsItem.lastModified, undefined); + }); + + it('should not add props file if folder already exists', () => { + const mockWithBoth = { + CommonPrefixes: [{ Prefix: 'test/' }], + Contents: [ + { + Key: 'test.props', + LastModified: new Date('2025-01-01'), + } + ] + }; + const result = formatList(mockWithBoth, daCtx); + const testItems = result.filter(item => item.name === 'test'); + assert.strictEqual(testItems.length, 1); + }); + + it('should sort results alphabetically', () => { + const mockForSorting = { + Contents: [ + { Key: 'zebra.html', LastModified: new Date('2025-01-01') }, + { Key: 'alpha.html', LastModified: new Date('2025-01-01') }, + { Key: 'beta.html', LastModified: new Date('2025-01-01') } + ] + }; + const result = formatList(mockForSorting, daCtx); + assert.strictEqual(result[0].name, 'alpha'); + assert.strictEqual(result[1].name, 'beta'); + assert.strictEqual(result[2].name, 'zebra'); + }); + + it('formatting should not have side effects', () => { + assert.deepStrictEqual(mockInput, getMock()); + }); });