From 8715d0de28822248f600fe68c0b89b6c62432900 Mon Sep 17 00:00:00 2001 From: Liam Lloyd-Tucker Date: Wed, 8 Jul 2026 15:52:56 -0700 Subject: [PATCH 1/4] Paginate GET /records response Our API design standard says that when endpoints return multiple objects, the results should be paginated. This commit updates the GET /records endpoint to make it compliant with that standard. It leaves current behavior in place for the deprecated GET /record path, because clients are actively using this path. Once clients switch to the now-paginated GET /records, we can remove the GET /record path. --- packages/api/docs/src/paths/record.yaml | 15 + .../api/src/record/controller/controller.ts | 31 ++ ...ords.test.ts => get_record_legacy.test.ts} | 78 ++-- .../controller/get_records_page.test.ts | 311 +++++++++++++ packages/api/src/record/index.ts | 2 +- packages/api/src/record/models.ts | 9 + .../api/src/record/queries/get_records.sql | 408 ++++++++++-------- packages/api/src/record/service.ts | 80 +++- packages/api/src/record/validators.test.ts | 76 +++- packages/api/src/record/validators.ts | 27 ++ packages/api/src/routes/index.ts | 4 +- 11 files changed, 805 insertions(+), 236 deletions(-) rename packages/api/src/record/controller/{get_records.test.ts => get_record_legacy.test.ts} (89%) create mode 100644 packages/api/src/record/controller/get_records_page.test.ts diff --git a/packages/api/docs/src/paths/record.yaml b/packages/api/docs/src/paths/record.yaml index 96c296bf..8654f82a 100644 --- a/packages/api/docs/src/paths/record.yaml +++ b/packages/api/docs/src/paths/record.yaml @@ -20,6 +20,19 @@ records: At least one of recordIds or archiveId must be provided. schema: type: string + - name: cursor + description: | + The id of the record right before the first record you want returned. + In most cases, this will be that of the last record on the previous page. + in: query + required: false + schema: + type: string + - name: pageSize + in: query + required: true + schema: + type: integer security: - {} - bearerHttpAuthentication: [] @@ -39,6 +52,8 @@ records: type: array items: $ref: "../models/record.yaml#/record" + pagination: + $ref: "../models/pagination_metadata.yaml#/pagination" "400": $ref: "../errors.yaml#/400" "401": diff --git a/packages/api/src/record/controller/controller.ts b/packages/api/src/record/controller/controller.ts index 576073ed..af40c063 100644 --- a/packages/api/src/record/controller/controller.ts +++ b/packages/api/src/record/controller/controller.ts @@ -12,12 +12,14 @@ import { } from "../../middleware"; import { getRecords, + getRecordsPage, patchRecord, getRecordShareLinks, createRecordCopy, } from "../service"; import { validateGetRecordQuery, + validateGetRecordsPageQuery, validatePatchRecordRequest, validateSingleRecordParams, validateCreateRecordCopyRequest, @@ -130,3 +132,32 @@ recordController.post( } }, ); + +export const recordsController = Router(); + +recordsController.get( + "/", + extractUserEmailFromAuthToken, + extractShareTokenFromHeaders, + async (req: Request, res: Response, next: NextFunction) => { + try { + validateOptionalAuthenticationValues(req.body); + validateGetRecordsPageQuery(req.query); + const response = await getRecordsPage({ + recordIds: req.query.recordIds, + archiveId: req.query.archiveId, + accountEmail: req.body.emailFromAuthToken, + shareToken: req.body.shareToken, + pageSize: req.query.pageSize, + cursor: req.query.cursor, + }); + res.status(HTTP_STATUS.SUCCESSFUL.OK).send(response); + } catch (err) { + next(err); + } + }, +); + +// Handles all other /records routes (e.g. PATCH/:recordId, /:recordId/copies) +// that are identical to the deprecated /record alias. +recordsController.use(recordController); diff --git a/packages/api/src/record/controller/get_records.test.ts b/packages/api/src/record/controller/get_record_legacy.test.ts similarity index 89% rename from packages/api/src/record/controller/get_records.test.ts rename to packages/api/src/record/controller/get_record_legacy.test.ts index de42dd1f..15663c4f 100644 --- a/packages/api/src/record/controller/get_records.test.ts +++ b/packages/api/src/record/controller/get_record_legacy.test.ts @@ -61,7 +61,7 @@ const clearDatabase = async (): Promise => { ); }; -describe("GET /records", () => { +describe("GET /record (deprecated alias, no pagination)", () => { beforeEach(async () => { mockExtractUserEmailFromAuthToken("test@permanent.org"); mockExtractShareTokenFromHeaders(); @@ -78,29 +78,29 @@ describe("GET /records", () => { const agent = request(app); test("expect request to have an email from auth token if an auth token exists", async () => { mockExtractUserEmailFromAuthToken("not an email"); - await agent.get("/api/v2/records?recordIds[]=10001").expect(400); + await agent.get("/api/v2/record?recordIds[]=10001").expect(400); }); test("expect request to have a share token from the headers if such a token exists", async () => { mockExtractShareTokenFromHeaders("2849c711-e72e-41b5-bb49-b0b86a052668"); await agent - .get("/api/v2/records?recordIds[]=10001") + .get("/api/v2/record?recordIds[]=10001") .set("X-Permanent-Share-Token", "2849c711-e72e-41b5-bb49-b0b86a052668") .expect(200); expect(extractShareTokenFromHeaders).toHaveBeenCalled(); }); test("expect an empty query to cause a 400 error", async () => { - await agent.get("/api/v2/records").expect(400); + await agent.get("/api/v2/record").expect(400); }); test("expect a non-array record ID to cause a 400 error", async () => { - await agent.get("/api/v2/records?recordIds=1").expect(400); + await agent.get("/api/v2/record?recordIds=1").expect(400); }); test("expect an empty array to cause a 400 error", async () => { - await agent.get("/api/v2/records?recordIds[]").expect(400); + await agent.get("/api/v2/record?recordIds[]").expect(400); }); test("expect return a public record when not logged in", async () => { mockExtractUserEmailFromAuthToken(); const response = await agent - .get("/api/v2/records?recordIds[]=10001") + .get("/api/v2/record?recordIds[]=10001") .expect(200); const { body: records } = response as { body: ArchiveRecord[] }; expect(records.length).toEqual(1); @@ -108,7 +108,7 @@ describe("GET /records", () => { }); test("expect to return a record", async () => { const response = await agent - .get("/api/v2/records?recordIds[]=10001") + .get("/api/v2/record?recordIds[]=10001") .expect(200); const { body: records } = response as { body: ArchiveRecord[] }; expect(records.length).toEqual(1); @@ -116,14 +116,14 @@ describe("GET /records", () => { }); test("expect to return multiple records", async () => { const response = await agent - .get("/api/v2/records?recordIds[]=10001&recordIds[]=10002") + .get("/api/v2/record?recordIds[]=10001&recordIds[]=10002") .expect(200); const { body: records } = response as { body: ArchiveRecord[] }; expect(records.length).toEqual(2); }); test("expect to return multiple records in the order of the request", async () => { const response = await agent - .get("/api/v2/records?recordIds[]=10002&recordIds[]=10001") + .get("/api/v2/record?recordIds[]=10002&recordIds[]=10001") .expect(200); const { body: records } = response as { body: ArchiveRecord[] }; expect(records.length).toEqual(2); @@ -132,21 +132,21 @@ describe("GET /records", () => { }); test("expect an empty response if the logged-in user does not own the record", async () => { const response = await agent - .get("/api/v2/records?recordIds[]=10007") + .get("/api/v2/record?recordIds[]=10007") .expect(200); const { body: records } = response as { body: ArchiveRecord[] }; expect(records.length).toEqual(0); }); test("expect an empty response if the record is deleted", async () => { const response = await agent - .get("/api/v2/records?recordIds[]=10004") + .get("/api/v2/record?recordIds[]=10004") .expect(200); const { body: records } = response as { body: ArchiveRecord[] }; expect(records.length).toEqual(0); }); test("expect to return a public record not owned by logged-in user", async () => { const response = await agent - .get("/api/v2/records?recordIds[]=10005") + .get("/api/v2/record?recordIds[]=10005") .expect(200); const { body: records } = response as { body: ArchiveRecord[] }; expect(records.length).toEqual(1); @@ -155,7 +155,7 @@ describe("GET /records", () => { test("expect return a public record when not logged in", async () => { mockExtractUserEmailFromAuthToken(); const response = await agent - .get("/api/v2/records?recordIds[]=10001") + .get("/api/v2/record?recordIds[]=10001") .expect(200); const { body: records } = response as { body: ArchiveRecord[] }; expect(records.length).toEqual(1); @@ -165,7 +165,7 @@ describe("GET /records", () => { mockExtractUserEmailFromAuthToken(); mockExtractShareTokenFromHeaders("2849c711-e72e-41b5-bb49-b0b86a052668"); const response = await agent - .get("/api/v2/records?recordIds[]=10002") + .get("/api/v2/record?recordIds[]=10002") .set("X-Permanent-Share-Token", "2849c711-e72e-41b5-bb49-b0b86a052668") .expect(200); const { body: records } = response as { body: ArchiveRecord[] }; @@ -176,7 +176,7 @@ describe("GET /records", () => { mockExtractUserEmailFromAuthToken(); mockExtractShareTokenFromHeaders("17e86544-30b3-4039-9f50-56681bcf3085"); const response = await agent - .get("/api/v2/records?recordIds[]=10002") + .get("/api/v2/record?recordIds[]=10002") .set("X-Permanent-Share-Token", "17e86544-30b3-4039-9f50-56681bcf3085") .expect(200); const { body: records } = response as { body: ArchiveRecord[] }; @@ -186,7 +186,7 @@ describe("GET /records", () => { mockExtractUserEmailFromAuthToken(); mockExtractShareTokenFromHeaders("1753eb10-ca46-4964-890b-0d4cdca1a783"); const response = await agent - .get("/api/v2/records?recordIds[]=10002") + .get("/api/v2/record?recordIds[]=10002") .set("X-Permanent-Share-Token", "1753eb10-ca46-4964-890b-0d4cdca1a783") .expect(200); const { body: records } = response as { body: ArchiveRecord[] }; @@ -196,7 +196,7 @@ describe("GET /records", () => { mockExtractUserEmailFromAuthToken(); mockExtractShareTokenFromHeaders("5b23ec69-3e37-4b83-9147-acf55d4654b5"); const response = await agent - .get("/api/v2/records?recordIds[]=10002") + .get("/api/v2/record?recordIds[]=10002") .set("X-Permanent-Share-Token", "5b23ec69-3e37-4b83-9147-acf55d4654b5") .expect(200); const { body: records } = response as { body: ArchiveRecord[] }; @@ -207,7 +207,7 @@ describe("GET /records", () => { mockExtractUserEmailFromAuthToken(); mockExtractShareTokenFromHeaders("85018ca8-881e-4cb7-9a22-24f3e015f797"); const response = await agent - .get("/api/v2/records?recordIds[]=10002") + .get("/api/v2/record?recordIds[]=10002") .set("X-Permanent-Share-Token", "85018ca8-881e-4cb7-9a22-24f3e015f797") .expect(200); const { body: records } = response as { body: ArchiveRecord[] }; @@ -217,7 +217,7 @@ describe("GET /records", () => { mockExtractUserEmailFromAuthToken(); mockExtractShareTokenFromHeaders("fbff79db-3814-4a1e-86be-ae1326cd56a3"); const response = await agent - .get("/api/v2/records?recordIds[]=10002") + .get("/api/v2/record?recordIds[]=10002") .set("X-Permanent-Share-Token", "fbff79db-3814-4a1e-86be-ae1326cd56a3") .expect(200); const { body: records } = response as { body: ArchiveRecord[] }; @@ -226,7 +226,7 @@ describe("GET /records", () => { test("expect non-manager viewer to not receive pendingShares", async () => { mockExtractUserEmailFromAuthToken("test+1@permanent.org"); const response = await agent - .get("/api/v2/records?recordIds[]=10008") + .get("/api/v2/record?recordIds[]=10008") .expect(200); const { body: records } = response as { body: ArchiveRecord[] }; expect(records.length).toEqual(1); @@ -235,7 +235,7 @@ describe("GET /records", () => { test("expect unauthenticated viewer of public record to not receive pendingShares", async () => { mockExtractUserEmailFromAuthToken(); const response = await agent - .get("/api/v2/records?recordIds[]=10008") + .get("/api/v2/record?recordIds[]=10008") .expect(200); const { body: records } = response as { body: ArchiveRecord[] }; expect(records.length).toEqual(1); @@ -244,7 +244,7 @@ describe("GET /records", () => { test("expect not to return a private record when not logged in", async () => { mockExtractUserEmailFromAuthToken(); const response = await agent - .get("/api/v2/records?recordIds[]=10002") + .get("/api/v2/record?recordIds[]=10002") .expect(200); const { body: records } = response as { body: ArchiveRecord[] }; expect(records.length).toEqual(0); @@ -254,7 +254,7 @@ describe("GET /records", () => { // will all have equivalent entries in the access table. So we don't need to // test that separately. const response = await agent - .get("/api/v2/records?recordIds[]=10006") + .get("/api/v2/record?recordIds[]=10006") .expect(200); const { body: records } = response as { body: ArchiveRecord[] }; expect(records.length).toEqual(1); @@ -262,7 +262,7 @@ describe("GET /records", () => { }); test("expect to receive a whole record", async () => { const response = await agent - .get("/api/v2/records?recordIds[]=10008") + .get("/api/v2/record?recordIds[]=10008") .expect(200); const { body: records } = response as { body: ArchiveRecord[] }; const [record] = records; @@ -409,7 +409,7 @@ describe("GET /records", () => { }); test("expect to not return deleted files", async () => { const response = await agent - .get("/api/v2/records?recordIds[]=10009") + .get("/api/v2/record?recordIds[]=10009") .expect(200); const { body: [record], @@ -418,28 +418,28 @@ describe("GET /records", () => { }); test("expect to not return a record in a deleted archive", async () => { const response = await agent - .get("/api/v2/records?recordIds[]=10010") + .get("/api/v2/record?recordIds[]=10010") .expect(200); const { body: records } = response as { body: ArchiveRecord[] }; expect(records.length).toEqual(0); }); test("expect to not return a record for a pending archive member", async () => { const response = await agent - .get("/api/v2/records?recordIds[]=10011") + .get("/api/v2/record?recordIds[]=10011") .expect(200); const { body: records } = response as { body: ArchiveRecord[] }; expect(records.length).toEqual(0); }); test("expect to not return a record with a deleted folder_link", async () => { const response = await agent - .get("/api/v2/records?recordIds[]=10012") + .get("/api/v2/record?recordIds[]=10012") .expect(200); const { body: records } = response as { body: ArchiveRecord[] }; expect(records.length).toEqual(0); }); test("expect to not return a record with deleted access", async () => { const response = await agent - .get("/api/v2/records?recordIds[]=10013") + .get("/api/v2/record?recordIds[]=10013") .expect(200); const { body: records } = response as { body: ArchiveRecord[] }; expect(records.length).toEqual(0); @@ -447,14 +447,14 @@ describe("GET /records", () => { test("expect to not return a record shared with a deleted membership", async () => { mockExtractUserEmailFromAuthToken("test+2@permanent.org"); const response = await agent - .get("/api/v2/records?recordIds[]=10002") + .get("/api/v2/record?recordIds[]=10002") .expect(200); const { body: records } = response as { body: ArchiveRecord[] }; expect(records.length).toEqual(0); }); test("expect to not return a record with a deleted parent folder", async () => { const response = await agent - .get("/api/v2/records?recordIds[]=10014") + .get("/api/v2/record?recordIds[]=10014") .expect(200); const { body: records } = response as { body: ArchiveRecord[] }; expect(records.length).toEqual(0); @@ -465,11 +465,11 @@ describe("GET /records", () => { throw testError; }); - await agent.get("/api/v2/records?recordIds[]=10014").expect(500); + await agent.get("/api/v2/record?recordIds[]=10014").expect(500); expect(logger.error).toHaveBeenCalledWith(testError); }); test("expect to return records filtered by archiveId", async () => { - const response = await agent.get("/api/v2/records?archiveId=1").expect(200); + const response = await agent.get("/api/v2/record?archiveId=1").expect(200); const { body: records } = response as { body: ArchiveRecord[] }; const recordIds = records.map((record) => record.recordId); expect(recordIds).toHaveLength(5); @@ -479,7 +479,7 @@ describe("GET /records", () => { }); test("expect archiveId and recordIds to act as an AND filter", async () => { const response = await agent - .get("/api/v2/records?archiveId=1&recordIds[]=10001") + .get("/api/v2/record?archiveId=1&recordIds[]=10001") .expect(200); const { body: records } = response as { body: ArchiveRecord[] }; expect(records.length).toEqual(1); @@ -487,24 +487,24 @@ describe("GET /records", () => { }); test("expect archiveId with no matching records to return empty array", async () => { const response = await agent - .get("/api/v2/records?archiveId=9999") + .get("/api/v2/record?archiveId=9999") .expect(200); const { body: records } = response as { body: ArchiveRecord[] }; expect(records.length).toEqual(0); }); test("expect archiveId for non-owned archive to return only public records", async () => { - const response = await agent.get("/api/v2/records?archiveId=2").expect(200); + const response = await agent.get("/api/v2/record?archiveId=2").expect(200); const { body: records } = response as { body: ArchiveRecord[] }; const recordIds = records.map((record) => record.recordId); expect(recordIds).toHaveLength(2); expect(recordIds).toEqual(expect.arrayContaining(["10005", "10006"])); }); test("expect archiveId query without recordIds still requires archiveId", async () => { - await agent.get("/api/v2/records").expect(400); + await agent.get("/api/v2/record").expect(400); }); test("expect return records by archiveId for a public record when not logged in", async () => { mockExtractUserEmailFromAuthToken(); - const response = await agent.get("/api/v2/records?archiveId=1").expect(200); + const response = await agent.get("/api/v2/record?archiveId=1").expect(200); const { body: records } = response as { body: ArchiveRecord[] }; const recordIds = records.map((record) => record.recordId); expect(recordIds).toHaveLength(2); diff --git a/packages/api/src/record/controller/get_records_page.test.ts b/packages/api/src/record/controller/get_records_page.test.ts new file mode 100644 index 00000000..595daef3 --- /dev/null +++ b/packages/api/src/record/controller/get_records_page.test.ts @@ -0,0 +1,311 @@ +import { logger } from "@stela/logger"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +import request from "supertest"; +import { app } from "../../app"; +import { db } from "../../database"; +import type { ArchiveRecord, GetRecordsResponse } from "../models"; +import { + mockExtractShareTokenFromHeaders, + mockExtractUserEmailFromAuthToken, +} from "../../../test/middleware_mocks"; + +vi.mock("../../database"); +vi.mock("../../middleware"); +vi.mock("@stela/logger"); + +const setupDatabase = async (): Promise => { + await db.sql("record.fixtures.create_test_accounts"); + await db.sql("record.fixtures.create_test_archives"); + await db.sql("record.fixtures.create_test_account_archives"); + await db.sql("record.fixtures.create_test_locations"); + await db.sql("record.fixtures.create_test_records"); + await db.sql("record.fixtures.create_complete_test_record"); + await db.sql("record.fixtures.create_test_folders"); + await db.sql("record.fixtures.create_test_folder_links"); + await db.sql("record.fixtures.create_test_accesses"); + await db.sql("record.fixtures.create_test_files"); + await db.sql("record.fixtures.create_complete_test_files"); + await db.sql("record.fixtures.create_test_record_files"); + await db.sql("record.fixtures.create_test_tags"); + await db.sql("record.fixtures.create_test_tag_links"); + await db.sql("record.fixtures.create_test_shares"); + await db.sql("record.fixtures.create_test_profile_items"); + await db.sql("record.fixtures.create_complete_test_folder_links"); + await db.sql("record.fixtures.create_test_shareby_urls"); + await db.sql("record.fixtures.create_test_invite_shares"); + await db.sql("record.fixtures.create_test_account_space"); + await db.sql("record.fixtures.create_test_archive_nbr"); +}; + +const clearDatabase = async (): Promise => { + await db.query( + `TRUNCATE + account, + archive, + account_archive, + record, + folder, + folder_link, + locn, + access, + tag, + tag_link, + share, + shareby_url, + profile_item, + invite, + invite_share, + archive_nbr, + file CASCADE`, + ); +}; + +describe("GET /records", () => { + beforeEach(async () => { + mockExtractUserEmailFromAuthToken("test@permanent.org"); + mockExtractShareTokenFromHeaders(); + await clearDatabase(); + await setupDatabase(); + }); + + afterEach(async () => { + await clearDatabase(); + vi.restoreAllMocks(); + vi.clearAllMocks(); + }); + + const agent = request(app); + + test("expect a missing pageSize to cause a 400 error", async () => { + await agent.get("/api/v2/records?archiveId=1").expect(400); + }); + + test("expect a query with neither recordIds nor archiveId to cause a 400 error", async () => { + await agent.get("/api/v2/records?pageSize=100").expect(400); + }); + + test("expect a response with an items array and pagination metadata", async () => { + const response = await agent + .get("/api/v2/records?recordIds[]=10001&pageSize=100") + .expect(200); + const { body } = response as { body: GetRecordsResponse }; + expect(body.items).toHaveLength(1); + expect(body.items[0]?.recordId).toEqual("10001"); + expect(body.pagination).toBeDefined(); + }); + + test("expect no more than pageSize items to be returned", async () => { + const response = await agent + .get("/api/v2/records?archiveId=1&pageSize=2") + .expect(200); + const { body } = response as { body: GetRecordsResponse }; + expect(body.items).toHaveLength(2); + }); + + test("expect to page through all matching records via cursor, in ascending folderLinkId order", async () => { + const firstResponse = await agent + .get("/api/v2/records?archiveId=1&pageSize=2") + .expect(200); + const { body: firstPage } = firstResponse as { body: GetRecordsResponse }; + expect(firstPage.items.map((record) => record.recordId)).toEqual([ + "10001", + "10002", + ]); + expect(firstPage.pagination.totalPages).toEqual(3); + expect(firstPage.pagination.nextCursor).toBeDefined(); + + const secondResponse = await agent + .get( + `/api/v2/records?archiveId=1&pageSize=2&cursor=${firstPage.pagination.nextCursor}`, + ) + .expect(200); + const { body: secondPage } = secondResponse as { body: GetRecordsResponse }; + expect(secondPage.items.map((record) => record.recordId)).toEqual([ + "10003", + "10008", + ]); + expect(secondPage.pagination.totalPages).toEqual(3); + + const thirdResponse = await agent + .get( + `/api/v2/records?archiveId=1&pageSize=2&cursor=${secondPage.pagination.nextCursor}`, + ) + .expect(200); + const { body: thirdPage } = thirdResponse as { body: GetRecordsResponse }; + expect(thirdPage.items.map((record) => record.recordId)).toEqual(["10009"]); + expect(thirdPage.pagination.totalPages).toEqual(3); + }); + + test("expect pagination.nextPage to link to the next page with the same filters", async () => { + const response = await agent + .get("/api/v2/records?archiveId=1&pageSize=2") + .expect(200); + const { body } = response as { body: GetRecordsResponse }; + expect(body.pagination.nextPage).toEqual( + `https://${process.env["SITE_URL"] ?? ""}/api/v2/records?archiveId=1&pageSize=2&cursor=${ + body.pagination.nextCursor ?? "" + }`, + ); + }); + + test("expect no items when querying with a cursor past the last item", async () => { + const response = await agent + .get("/api/v2/records?archiveId=1&pageSize=100") + .expect(200); + const { + body, + body: { + pagination: { nextCursor }, + }, + } = response as { body: GetRecordsResponse }; + expect(body.items).toHaveLength(5); + expect(nextCursor).toBeDefined(); + + const nextResponse = await agent + .get(`/api/v2/records?archiveId=1&pageSize=100&cursor=${nextCursor}`) + .expect(200); + const { body: nextBody } = nextResponse as { body: GetRecordsResponse }; + expect(nextBody.items).toHaveLength(0); + }); + + test("expect archiveId and recordIds to act as an AND filter", async () => { + const response = await agent + .get("/api/v2/records?archiveId=1&recordIds[]=10001&pageSize=100") + .expect(200); + const { body } = response as { body: GetRecordsResponse }; + expect(body.items).toHaveLength(1); + expect(body.items[0]?.recordId).toEqual("10001"); + }); + + test("expect archiveId with no matching records to return an empty items array", async () => { + const response = await agent + .get("/api/v2/records?archiveId=9999&pageSize=100") + .expect(200); + const { body } = response as { body: GetRecordsResponse }; + expect(body.items).toHaveLength(0); + expect(body.pagination.totalPages).toEqual(0); + }); + + test("expect return a public record when not logged in", async () => { + mockExtractUserEmailFromAuthToken(); + const response = await agent + .get("/api/v2/records?recordIds[]=10001&pageSize=100") + .expect(200); + const { body } = response as { body: GetRecordsResponse }; + expect(body.items).toHaveLength(1); + expect(body.items[0]?.recordId).toEqual("10001"); + }); + + test("expect not to return a private record when not logged in", async () => { + mockExtractUserEmailFromAuthToken(); + const response = await agent + .get("/api/v2/records?recordIds[]=10002&pageSize=100") + .expect(200); + const { body } = response as { body: GetRecordsResponse }; + expect(body.items).toHaveLength(0); + }); + + test("expect return a private record when not logged in but providing a valid unlisted share token", async () => { + mockExtractUserEmailFromAuthToken(); + mockExtractShareTokenFromHeaders("2849c711-e72e-41b5-bb49-b0b86a052668"); + const response = await agent + .get("/api/v2/records?recordIds[]=10002&pageSize=100") + .set("X-Permanent-Share-Token", "2849c711-e72e-41b5-bb49-b0b86a052668") + .expect(200); + const { body } = response as { body: GetRecordsResponse }; + expect(body.items).toHaveLength(1); + expect(body.items[0]?.recordId).toEqual("10002"); + }); + + test("expect not to return a private record when share token provided is not unlisted", async () => { + mockExtractUserEmailFromAuthToken(); + mockExtractShareTokenFromHeaders("17e86544-30b3-4039-9f50-56681bcf3085"); + const response = await agent + .get("/api/v2/records?recordIds[]=10002&pageSize=100") + .set("X-Permanent-Share-Token", "17e86544-30b3-4039-9f50-56681bcf3085") + .expect(200); + const { body } = response as { body: GetRecordsResponse }; + expect(body.items).toHaveLength(0); + }); + + test("expect not to return a private record when share token provided is expired", async () => { + mockExtractUserEmailFromAuthToken(); + mockExtractShareTokenFromHeaders("1753eb10-ca46-4964-890b-0d4cdca1a783"); + const response = await agent + .get("/api/v2/records?recordIds[]=10002&pageSize=100") + .set("X-Permanent-Share-Token", "1753eb10-ca46-4964-890b-0d4cdca1a783") + .expect(200); + const { body } = response as { body: GetRecordsResponse }; + expect(body.items).toHaveLength(0); + }); + + test("expect return a private record when not logged in but providing a valid share token for an ancestor folder", async () => { + mockExtractUserEmailFromAuthToken(); + mockExtractShareTokenFromHeaders("5b23ec69-3e37-4b83-9147-acf55d4654b5"); + const response = await agent + .get("/api/v2/records?recordIds[]=10002&pageSize=100") + .set("X-Permanent-Share-Token", "5b23ec69-3e37-4b83-9147-acf55d4654b5") + .expect(200); + const { body } = response as { body: GetRecordsResponse }; + expect(body.items).toHaveLength(1); + expect(body.items[0]?.recordId).toEqual("10002"); + }); + + test("expect to return a private record shared with the logged in account", async () => { + const response = await agent + .get("/api/v2/records?recordIds[]=10006&pageSize=100") + .expect(200); + const { body } = response as { body: GetRecordsResponse }; + expect(body.items).toHaveLength(1); + expect(body.items[0]?.recordId).toEqual("10006"); + }); + + test("expect to not return a record in a deleted archive", async () => { + const response = await agent + .get("/api/v2/records?recordIds[]=10010&pageSize=100") + .expect(200); + const { body } = response as { body: GetRecordsResponse }; + expect(body.items).toHaveLength(0); + }); + + test("expect to not return a record with a deleted folder_link", async () => { + const response = await agent + .get("/api/v2/records?recordIds[]=10012&pageSize=100") + .expect(200); + const { body } = response as { body: GetRecordsResponse }; + expect(body.items).toHaveLength(0); + }); + + test("expect to not return a record with deleted access", async () => { + const response = await agent + .get("/api/v2/records?recordIds[]=10013&pageSize=100") + .expect(200); + const { body } = response as { body: GetRecordsResponse }; + expect(body.items).toHaveLength(0); + }); + + test("expect to return records filtered by archiveId for a public record when not logged in", async () => { + mockExtractUserEmailFromAuthToken(); + const response = await agent + .get("/api/v2/records?archiveId=1&pageSize=100") + .expect(200); + const { body } = response as { body: GetRecordsResponse }; + const recordIds = body.items.map( + (record: ArchiveRecord) => record.recordId, + ); + expect(recordIds).toHaveLength(2); + expect(recordIds).toEqual(expect.arrayContaining(["10001", "10008"])); + }); + + test("expect to log error and return 500 if database lookup fails", async () => { + const testError = new Error("test error"); + vi.spyOn(db, "sql").mockImplementation(async () => { + throw testError; + }); + + await agent + .get("/api/v2/records?recordIds[]=10001&pageSize=100") + .expect(500); + expect(logger.error).toHaveBeenCalledWith(testError); + }); +}); diff --git a/packages/api/src/record/index.ts b/packages/api/src/record/index.ts index 75405087..2287ec95 100644 --- a/packages/api/src/record/index.ts +++ b/packages/api/src/record/index.ts @@ -1 +1 @@ -export { recordController } from "./controller/controller"; +export { recordController, recordsController } from "./controller/controller"; diff --git a/packages/api/src/record/models.ts b/packages/api/src/record/models.ts index dcea4d76..bfcff0c1 100644 --- a/packages/api/src/record/models.ts +++ b/packages/api/src/record/models.ts @@ -113,6 +113,15 @@ export interface ArchiveFile { updatedAt: Date; } +export interface GetRecordsResponse { + items: ArchiveRecord[]; + pagination: { + nextCursor: string | undefined; + nextPage: string | undefined; + totalPages: number; + }; +} + export interface PatchRecordRequest { emailFromAuthToken: string; locationId?: bigint | null; diff --git a/packages/api/src/record/queries/get_records.sql b/packages/api/src/record/queries/get_records.sql index 3a7489ed..97c39ea0 100644 --- a/packages/api/src/record/queries/get_records.sql +++ b/packages/api/src/record/queries/get_records.sql @@ -176,194 +176,226 @@ aggregated_ancestor_unrestricted_share_tokens AS ( FROM ancestor_unrestricted_share_tokens GROUP BY recordid -) +), -SELECT DISTINCT ON (record.recordid) - record.recordid AS id, - record.recordid AS "recordId", - record.displayname AS "displayName", - record.archiveid AS "archiveId", - record.archivenbr AS "archiveNumber", - record.description, - record.publicdt AS "publicAt", - record.downloadname AS "downloadName", - record.uploadfilename AS "uploadFileName", - record.uploadaccountid AS "uploadAccountId", - record.uploadpayeraccountid AS "uploadPayerAccountId", - record.size, - record.displaydt AS "displayDate", - record.displaytime AS "displayTime", - record.derivedcreateddt AS "fileCreatedAt", - record.imageratio AS "imageRatio", - record.thumburl200 AS "thumbUrl200", - record.thumburl500 AS "thumbUrl500", - record.thumburl1000 AS "thumbUrl1000", - record.thumburl2000 AS "thumbUrl2000", - record.status, - record.type, - record.createddt AS "createdAt", - record.updateddt AS "updatedAt", - record.alttext AS "altText", - aggregated_files.files, - folder_link.folder_linkid AS "folderLinkId", - folder_link.type AS "folderLinkType", - folder_link.parentfolderid AS "parentFolderId", - folder_link.parentfolder_linkid AS "parentFolderLinkId", - parent_folder.archivenbr AS "parentFolderArchiveNumber", - aggregated_tags.tags, - archive.archivenbr AS "archiveArchiveNumber", - aggregated_shares.shares_as_json AS shares, - CASE - WHEN - EXISTS ( - SELECT 1 - FROM account_archive - INNER JOIN account - ON - account_archive.accountid = account.accountid - AND account.primaryemail = :accountEmail - WHERE - account_archive.archiveid = record.archiveid - AND account_archive.status = 'status.generic.ok' - AND account_archive.accessrole IN ( - 'access.role.owner', 'access.role.manager' - ) - ) - THEN aggregated_pending_shares.pending_shares_as_json - END AS "pendingShares", - JSON_BUILD_OBJECT( - '200', - record.thumburl200, - '500', - record.thumburl500, - '1000', - record.thumburl1000, - '2000', - record.thumburl2000, - '256', - record.thumbnail256 - ) AS "thumbnailUrls", - JSON_BUILD_OBJECT( - 'id', - locn.locnid::TEXT, - 'name', - locn.name, - 'sublocation', - locn.sublocation, - 'city', - locn.city, - 'state', - locn.adminonename, - 'postalCode', - locn.postalcode, - 'country', - locn.country, - 'latitude', - locn.latitude, - 'longitude', - locn.longitude, - 'altitudeMeters', - locn.altitudemeters, - 'precision', - locn.locationprecision, - 'streetNumber', - locn.streetnumber, - 'streetName', - locn.streetname, - 'locality', - locn.locality, - 'county', - locn.admintwoname, - 'countryCode', - locn.countrycode, - 'displayName', - locn.displayname - ) AS location, - JSON_BUILD_OBJECT( - 'id', - archive.archiveid::TEXT, - 'archiveNumber', - archive.archivenbr, - 'name', - profile_item.string1 - ) AS archive -FROM - record -INNER JOIN - archive - ON - record.archiveid = archive.archiveid - AND archive.status != 'status.generic.deleted' - AND archive.status IS NOT NULL -LEFT JOIN - profile_item - ON - archive.archiveid = profile_item.archiveid - AND profile_item.fieldnameui = 'profile.basic' - AND profile_item.status = 'status.generic.ok' - AND profile_item.string1 IS NOT NULL -INNER JOIN - account_archive AS record_account_archive - ON - record.archiveid = record_account_archive.archiveid - AND record_account_archive.status = 'status.generic.ok' -INNER JOIN - account AS record_account - ON record_account_archive.accountid = record_account.accountid -INNER JOIN - aggregated_files - ON record.recordid = aggregated_files.recordid -LEFT JOIN - aggregated_tags - ON record.recordid = aggregated_tags.refid -LEFT JOIN - locn - ON record.locnid = locn.locnid -INNER JOIN - folder_link - ON - record.recordid = folder_link.recordid - AND folder_link.status != 'status.generic.deleted' -INNER JOIN - folder AS parent_folder - ON - folder_link.parentfolderid = parent_folder.folderid - AND parent_folder.status != 'status.generic.deleted' -LEFT JOIN - access - ON - folder_link.folder_linkid = access.folder_linkid - AND access.status = 'status.generic.ok' -LEFT JOIN - account_archive AS share_account_archive - ON - access.archiveid = share_account_archive.archiveid - AND share_account_archive.status = 'status.generic.ok' -LEFT JOIN - account AS share_account - ON share_account_archive.accountid = share_account.accountid -LEFT JOIN - aggregated_shares - ON folder_link.folder_linkid = aggregated_shares.folder_linkid -LEFT JOIN - aggregated_pending_shares - ON folder_link.folder_linkid = aggregated_pending_shares.folder_linkid -LEFT JOIN - aggregated_ancestor_unrestricted_share_tokens - ON record.recordid = aggregated_ancestor_unrestricted_share_tokens.recordid -INNER JOIN - candidate_records - ON record.recordid = candidate_records.recordid -WHERE - ( - record_account.primaryemail = :accountEmail - OR share_account.primaryemail = :accountEmail - OR (record.publicdt IS NOT NULL AND record.publicdt <= NOW()) - OR ( - :shareToken::TEXT IS NOT NULL - AND :shareToken = ANY( - aggregated_ancestor_unrestricted_share_tokens.tokens +all_records AS ( + SELECT DISTINCT ON (record.recordid) + record.recordid AS id, + record.recordid AS "recordId", + record.displayname AS "displayName", + record.archiveid AS "archiveId", + record.archivenbr AS "archiveNumber", + record.description, + record.publicdt AS "publicAt", + record.downloadname AS "downloadName", + record.uploadfilename AS "uploadFileName", + record.uploadaccountid AS "uploadAccountId", + record.uploadpayeraccountid AS "uploadPayerAccountId", + record.size, + record.displaydt AS "displayDate", + record.displaytime AS "displayTime", + record.derivedcreateddt AS "fileCreatedAt", + record.imageratio AS "imageRatio", + record.thumburl200 AS "thumbUrl200", + record.thumburl500 AS "thumbUrl500", + record.thumburl1000 AS "thumbUrl1000", + record.thumburl2000 AS "thumbUrl2000", + record.status, + record.type, + record.createddt AS "createdAt", + record.updateddt AS "updatedAt", + record.alttext AS "altText", + aggregated_files.files, + folder_link.folder_linkid AS "folderLinkId", + folder_link.type AS "folderLinkType", + folder_link.parentfolderid AS "parentFolderId", + folder_link.parentfolder_linkid AS "parentFolderLinkId", + parent_folder.archivenbr AS "parentFolderArchiveNumber", + aggregated_tags.tags, + archive.archivenbr AS "archiveArchiveNumber", + aggregated_shares.shares_as_json AS shares, + CASE + WHEN + EXISTS ( + SELECT 1 + FROM account_archive + INNER JOIN account + ON + account_archive.accountid = account.accountid + AND account.primaryemail = :accountEmail + WHERE + account_archive.archiveid = record.archiveid + AND account_archive.status = 'status.generic.ok' + AND account_archive.accessrole IN ( + 'access.role.owner', 'access.role.manager' + ) + ) + THEN aggregated_pending_shares.pending_shares_as_json + END AS "pendingShares", + JSON_BUILD_OBJECT( + '200', + record.thumburl200, + '500', + record.thumburl500, + '1000', + record.thumburl1000, + '2000', + record.thumburl2000, + '256', + record.thumbnail256 + ) AS "thumbnailUrls", + JSON_BUILD_OBJECT( + 'id', + locn.locnid::TEXT, + 'name', + locn.name, + 'sublocation', + locn.sublocation, + 'city', + locn.city, + 'state', + locn.adminonename, + 'postalCode', + locn.postalcode, + 'country', + locn.country, + 'latitude', + locn.latitude, + 'longitude', + locn.longitude, + 'altitudeMeters', + locn.altitudemeters, + 'precision', + locn.locationprecision, + 'streetNumber', + locn.streetnumber, + 'streetName', + locn.streetname, + 'locality', + locn.locality, + 'county', + locn.admintwoname, + 'countryCode', + locn.countrycode, + 'displayName', + locn.displayname + ) AS location, + JSON_BUILD_OBJECT( + 'id', + archive.archiveid::TEXT, + 'archiveNumber', + archive.archivenbr, + 'name', + profile_item.string1 + ) AS archive + FROM + record + INNER JOIN + archive + ON + record.archiveid = archive.archiveid + AND archive.status != 'status.generic.deleted' + AND archive.status IS NOT NULL + LEFT JOIN + profile_item + ON + archive.archiveid = profile_item.archiveid + AND profile_item.fieldnameui = 'profile.basic' + AND profile_item.status = 'status.generic.ok' + AND profile_item.string1 IS NOT NULL + INNER JOIN + account_archive AS record_account_archive + ON + record.archiveid = record_account_archive.archiveid + AND record_account_archive.status = 'status.generic.ok' + INNER JOIN + account AS record_account + ON record_account_archive.accountid = record_account.accountid + INNER JOIN + aggregated_files + ON record.recordid = aggregated_files.recordid + LEFT JOIN + aggregated_tags + ON record.recordid = aggregated_tags.refid + LEFT JOIN + locn + ON record.locnid = locn.locnid + INNER JOIN + folder_link + ON + record.recordid = folder_link.recordid + AND folder_link.status != 'status.generic.deleted' + INNER JOIN + folder AS parent_folder + ON + folder_link.parentfolderid = parent_folder.folderid + AND parent_folder.status != 'status.generic.deleted' + LEFT JOIN + access + ON + folder_link.folder_linkid = access.folder_linkid + AND access.status = 'status.generic.ok' + LEFT JOIN + account_archive AS share_account_archive + ON + access.archiveid = share_account_archive.archiveid + AND share_account_archive.status = 'status.generic.ok' + LEFT JOIN + account AS share_account + ON share_account_archive.accountid = share_account.accountid + LEFT JOIN + aggregated_shares + ON folder_link.folder_linkid = aggregated_shares.folder_linkid + LEFT JOIN + aggregated_pending_shares + ON folder_link.folder_linkid = aggregated_pending_shares.folder_linkid + LEFT JOIN + aggregated_ancestor_unrestricted_share_tokens + ON record.recordid = aggregated_ancestor_unrestricted_share_tokens.recordid + INNER JOIN + candidate_records + ON record.recordid = candidate_records.recordid + WHERE + ( + record_account.primaryemail = :accountEmail + OR share_account.primaryemail = :accountEmail + OR (record.publicdt IS NOT NULL AND record.publicdt <= NOW()) + OR ( + :shareToken::TEXT IS NOT NULL + AND :shareToken = ANY( + aggregated_ancestor_unrestricted_share_tokens.tokens + ) ) ) - ) - AND record.status != 'status.generic.deleted'; + AND record.status != 'status.generic.deleted' + ORDER BY record.recordid ASC +), + +ranked_records AS ( + SELECT + *, + ROW_NUMBER() OVER (ORDER BY id::BIGINT ASC) AS rank + FROM all_records +), + +cursor AS ( + SELECT rank + FROM + ranked_records + WHERE + id = :cursor +), + +total_pages AS ( + SELECT CEILING(COUNT(*)::FLOAT / :pageSize) AS total_pages + FROM all_records +) + +SELECT + ranked_records.*, + (SELECT total_pages.total_pages FROM total_pages) AS "totalPages" +FROM ranked_records +WHERE + ranked_records.rank > COALESCE((SELECT cursor.rank FROM cursor), 0) +ORDER BY ranked_records.rank ASC +LIMIT :pageSize; diff --git a/packages/api/src/record/service.ts b/packages/api/src/record/service.ts index 80d5ca7b..aa1d3d8f 100644 --- a/packages/api/src/record/service.ts +++ b/packages/api/src/record/service.ts @@ -6,6 +6,7 @@ import type { ArchiveRecord, ArchiveRecordRow, CreateRecordCopyRequest, + GetRecordsResponse, PatchRecordRequest, } from "./models"; import { @@ -20,6 +21,12 @@ import { getFolders } from "../folder/service"; import { type Folder, PrettyFolderType } from "../folder/models"; import { insertLocation, updateLocation } from "../location/service"; +const mapRecordRow = (row: ArchiveRecordRow): ArchiveRecord => ({ + ...row, + size: +(row.size ?? 0), + imageRatio: +(row.imageRatio ?? 0), +}); + export const getRecords = async (requestQuery: { recordIds: string[] | undefined; archiveId?: string | undefined; @@ -32,16 +39,14 @@ export const getRecords = async (requestQuery: { archiveId: requestQuery.archiveId ?? null, accountEmail: requestQuery.accountEmail, shareToken: requestQuery.shareToken, + pageSize: null, + cursor: undefined, }) .catch((err: unknown) => { logger.error(err); throw new createError.InternalServerError("failed to retrieve records"); }); - const records = record.rows.map((row: ArchiveRecordRow) => ({ - ...row, - size: +(row.size ?? 0), - imageRatio: +(row.imageRatio ?? 0), - })); + const records = record.rows.map(mapRecordRow); if (requestQuery.recordIds !== undefined) { // Our API contract remains that the order in which this endpoint returns @@ -63,6 +68,71 @@ export const getRecords = async (requestQuery: { return records; }; +const buildRecordsNextPageUrl = ( + requestQuery: { + recordIds: string[] | undefined; + archiveId?: string | undefined; + pageSize: number; + }, + nextCursor: string, +): string => { + const params = new URLSearchParams(); + requestQuery.recordIds?.forEach((recordId) => { + params.append("recordIds[]", recordId); + }); + if (requestQuery.archiveId !== undefined) { + params.set("archiveId", requestQuery.archiveId); + } + params.set("pageSize", String(requestQuery.pageSize)); + params.set("cursor", nextCursor); + return `https://${process.env["SITE_URL"] ?? ""}/api/v2/records?${params.toString()}`; +}; + +export const getRecordsPage = async (requestQuery: { + recordIds: string[] | undefined; + archiveId?: string | undefined; + accountEmail: string | undefined; + shareToken?: string | undefined; + pageSize: number; + cursor: string | undefined; +}): Promise => { + const result = await db + .sql( + "record.queries.get_records", + { + recordIds: requestQuery.recordIds ?? null, + archiveId: requestQuery.archiveId ?? null, + accountEmail: requestQuery.accountEmail, + shareToken: requestQuery.shareToken, + pageSize: requestQuery.pageSize, + cursor: requestQuery.cursor, + }, + ) + .catch((err: unknown) => { + logger.error(err); + throw new createError.InternalServerError("failed to retrieve records"); + }); + + const items = result.rows.map((row) => { + const { rank: _rank, totalPages: _totalPages, ...recordRow } = row; + return mapRecordRow(recordRow); + }); + + const nextCursor = items[items.length - 1]?.id; + + return { + items, + pagination: { + nextCursor, + nextPage: + nextCursor === undefined + ? undefined + : buildRecordsNextPageUrl(requestQuery, nextCursor), + totalPages: result.rows[0]?.totalPages ?? 0, + }, + }; +}; + const validateCanPatchRecord = async ( recordId: string, emailFromAuthToken: string, diff --git a/packages/api/src/record/validators.test.ts b/packages/api/src/record/validators.test.ts index 4e85ffcb..be4d24f1 100644 --- a/packages/api/src/record/validators.test.ts +++ b/packages/api/src/record/validators.test.ts @@ -1,5 +1,8 @@ -import { validatePatchRecordRequest } from "./validators"; import { describe, expect, test } from "vitest"; +import { + validatePatchRecordRequest, + validateGetRecordsPageQuery, +} from "./validators"; describe("validatePatchRecordRequest", () => { test("should find no errors in a valid request", () => { @@ -185,3 +188,74 @@ describe("validatePatchRecordRequest", () => { } }); }); + +describe("validateGetRecordsPageQuery", () => { + test("should find no errors in a valid request", () => { + let error = null; + try { + validateGetRecordsPageQuery({ + archiveId: "1", + pageSize: 10, + }); + } catch (err) { + error = err; + } finally { + expect(error).toBeNull(); + } + }); + + test("should find no errors when a cursor is provided", () => { + let error = null; + try { + validateGetRecordsPageQuery({ + recordIds: ["1", "2"], + pageSize: 10, + cursor: "5", + }); + } catch (err) { + error = err; + } finally { + expect(error).toBeNull(); + } + }); + + test("should raise an error if pageSize is missing", () => { + let error = null; + try { + validateGetRecordsPageQuery({ + archiveId: "1", + }); + } catch (err) { + error = err; + } finally { + expect(error).not.toBeNull(); + } + }); + + test("should raise an error if pageSize is not an integer", () => { + let error = null; + try { + validateGetRecordsPageQuery({ + archiveId: "1", + pageSize: 1.5, + }); + } catch (err) { + error = err; + } finally { + expect(error).not.toBeNull(); + } + }); + + test("should raise an error if neither recordIds nor archiveId is provided", () => { + let error = null; + try { + validateGetRecordsPageQuery({ + pageSize: 10, + }); + } catch (err) { + error = err; + } finally { + expect(error).not.toBeNull(); + } + }); +}); diff --git a/packages/api/src/record/validators.ts b/packages/api/src/record/validators.ts index db6f05f5..e6ae29cf 100644 --- a/packages/api/src/record/validators.ts +++ b/packages/api/src/record/validators.ts @@ -2,6 +2,7 @@ import Joi from "joi"; import { parse as parseEDTF } from "@edtf-ts/core"; import type { CreateRecordCopyRequest, PatchRecordRequest } from "./models"; import { fieldsFromUserAuthentication } from "../validators"; +import { paginationFields } from "../validators/shared"; import { locationInputSchema } from "../location/validators"; import { EDTF_LEVEL_2 } from "../constants"; @@ -21,6 +22,32 @@ export const validateGetRecordQuery: ( throw validation.error; } }; + +export const validateGetRecordsPageQuery: (data: unknown) => asserts data is { + recordIds?: string[]; + archiveId?: string; + cursor?: string; + pageSize: number; +} = ( + data: unknown, +): asserts data is { + recordIds?: string[]; + archiveId?: string; + cursor?: string; + pageSize: number; +} => { + const validation = Joi.object() + .keys({ + recordIds: Joi.array().items(Joi.string().required()), + archiveId: Joi.string(), + ...paginationFields, + }) + .or("recordIds", "archiveId") + .validate(data); + if (validation.error !== undefined) { + throw validation.error; + } +}; export const validateSingleRecordParams: ( data: unknown, ) => asserts data is { recordId: string } = ( diff --git a/packages/api/src/routes/index.ts b/packages/api/src/routes/index.ts index 6a3f2e35..fff4a191 100644 --- a/packages/api/src/routes/index.ts +++ b/packages/api/src/routes/index.ts @@ -9,7 +9,7 @@ import { storageController } from "../storage"; import { eventController } from "../event"; import { promoController } from "../promo"; import { idpUserController } from "../idpuser"; -import { recordController } from "../record"; +import { recordController, recordsController } from "../record"; import { featureController } from "../feature_flag"; import { folderController } from "../folder"; import { shareLinkController } from "../share_link"; @@ -30,7 +30,7 @@ apiRoutes.use("/events", eventController); apiRoutes.use("/event", eventController); // Deprecated path maintained to avoid breaking changes apiRoutes.use("/promo", promoController); apiRoutes.use("/idpuser", idpUserController); -apiRoutes.use("/records", recordController); +apiRoutes.use("/records", recordsController); apiRoutes.use("/record", recordController); // Deprecated path maintained to avoid breaking changes apiRoutes.use("/feature-flags", featureController); apiRoutes.use("/folders", folderController); From e699e573f9cc070633c9da7e106d042fb87f952d Mon Sep 17 00:00:00 2001 From: Liam Lloyd-Tucker Date: Wed, 8 Jul 2026 17:08:17 -0700 Subject: [PATCH 2/4] Paginate GET /folders response Our API design standard says that when endpoints return multiple objects, the results should be paginated. This commit updates the GET /folders endpoint to make it compliant with that standard. It leaves current behavior in place for the deprecated GET /folder path, because clients are actively using this path. Once clients switch to the now-paginated GET /folders, we can remove the GET /folder path. --- packages/api/docs/src/paths/folder.yaml | 15 + .../api/src/folder/controller/controller.ts | 30 ++ ...lder.test.ts => get_folder_legacy.test.ts} | 48 +-- .../controller/get_folders_page.test.ts | 212 +++++++++ .../folder/controller/patch_folder.test.ts | 4 + packages/api/src/folder/index.ts | 2 +- packages/api/src/folder/models.ts | 9 + .../api/src/folder/queries/get_folders.sql | 406 ++++++++++-------- packages/api/src/folder/service.ts | 85 +++- packages/api/src/folder/validators.test.ts | 72 ++++ packages/api/src/folder/validators.ts | 23 + packages/api/src/routes/index.ts | 4 +- 12 files changed, 684 insertions(+), 226 deletions(-) rename packages/api/src/folder/controller/{get_folder.test.ts => get_folder_legacy.test.ts} (92%) create mode 100644 packages/api/src/folder/controller/get_folders_page.test.ts diff --git a/packages/api/docs/src/paths/folder.yaml b/packages/api/docs/src/paths/folder.yaml index af1e9af5..f7a11f89 100644 --- a/packages/api/docs/src/paths/folder.yaml +++ b/packages/api/docs/src/paths/folder.yaml @@ -7,6 +7,19 @@ folders: type: array items: type: string + - name: cursor + description: | + The id of the folder right before the first folder you want returned. + In most cases, this will be that of the last folder on the previous page. + in: query + required: false + schema: + type: string + - name: pageSize + in: query + required: true + schema: + type: integer get: summary: Get folders by their IDs security: @@ -28,6 +41,8 @@ folders: type: array items: $ref: "../models/folder.yaml#/folder" + pagination: + $ref: "../models/pagination_metadata.yaml#/pagination" "400": $ref: "../errors.yaml#/400" "401": diff --git a/packages/api/src/folder/controller/controller.ts b/packages/api/src/folder/controller/controller.ts index 18b7c742..a2fa9d31 100644 --- a/packages/api/src/folder/controller/controller.ts +++ b/packages/api/src/folder/controller/controller.ts @@ -12,6 +12,7 @@ import { import { patchFolder, getFolders, + getFoldersPage, getFolderChildren, getFolderShareLinks, } from "../service"; @@ -19,6 +20,7 @@ import { validatePatchFolderRequest, validateFolderRequest, validateGetFoldersQuery, + validateGetFoldersPageQuery, } from "../validators"; import { validateOptionalAuthenticationValues, @@ -114,3 +116,31 @@ folderController.get( } }, ); + +export const foldersController = Router(); + +foldersController.get( + "/", + extractUserEmailFromAuthToken, + extractShareTokenFromHeaders, + async (req: Request, res: Response, next: NextFunction) => { + try { + validateOptionalAuthenticationValues(req.body); + validateGetFoldersPageQuery(req.query); + const response = await getFoldersPage({ + folderIds: req.query.folderIds, + email: req.body.emailFromAuthToken, + shareToken: req.body.shareToken, + pageSize: req.query.pageSize, + cursor: req.query.cursor, + }); + res.status(HTTP_STATUS.SUCCESSFUL.OK).send(response); + } catch (err) { + next(err); + } + }, +); + +// Handles all other /folders routes (e.g. PATCH /:folderId, /:folderId/children) +// that are identical to the deprecated /folder alias. +foldersController.use(folderController); diff --git a/packages/api/src/folder/controller/get_folder.test.ts b/packages/api/src/folder/controller/get_folder_legacy.test.ts similarity index 92% rename from packages/api/src/folder/controller/get_folder.test.ts rename to packages/api/src/folder/controller/get_folder_legacy.test.ts index 62f1080b..9df69614 100644 --- a/packages/api/src/folder/controller/get_folder.test.ts +++ b/packages/api/src/folder/controller/get_folder_legacy.test.ts @@ -18,7 +18,7 @@ vi.mock("../../middleware"); vi.mock("@stela/logger"); const testEmail = "test@permanent.org"; -describe("GET /folder", () => { +describe("GET /folder (deprecated alias, no pagination)", () => { const agent = request(app); beforeEach(async () => { @@ -35,28 +35,28 @@ describe("GET /folder", () => { }); test("should return 200 code for successful call", async () => { - await agent.get("/api/v2/folders?folderIds[]=1").expect(200); + await agent.get("/api/v2/folder?folderIds[]=1").expect(200); }); test("should call extractUserEmailFromAuthToken middleware", async () => { - await agent.get("/api/v2/folders?folderIds[]=1").expect(200); + await agent.get("/api/v2/folder?folderIds[]=1").expect(200); expect(extractUserEmailFromAuthToken).toHaveBeenCalled(); }); test("should call extractShareTokenFromHeaders middleware", async () => { - await agent.get("/api/v2/folders?folderIds[]=1").expect(200); + await agent.get("/api/v2/folder?folderIds[]=1").expect(200); expect(extractShareTokenFromHeaders).toHaveBeenCalled(); }); test("should return 400 code if the header values are improper", async () => { mockExtractUserEmailFromAuthToken("not_an_email"); - await agent.get("/api/v2/folders?folderIds[]=1").expect(400); + await agent.get("/api/v2/folder?folderIds[]=1").expect(400); }); test("should return a public folder if the user is not authenticated", async () => { mockExtractUserEmailFromAuthToken(); const response = await agent - .get("/api/v2/folders?folderIds[]=1") + .get("/api/v2/folder?folderIds[]=1") .expect(200); const { body: { items: folders }, @@ -68,7 +68,7 @@ describe("GET /folder", () => { test("should not return a private folder if the user is not authenticated", async () => { mockExtractUserEmailFromAuthToken(); const response = await agent - .get("/api/v2/folders?folderIds[]=2") + .get("/api/v2/folder?folderIds[]=2") .expect(200); const { body: { items: folders }, @@ -78,7 +78,7 @@ describe("GET /folder", () => { test("should return a private folder if the user is authenticated", async () => { const response = await agent - .get("/api/v2/folders?folderIds[]=2") + .get("/api/v2/folder?folderIds[]=2") .expect(200); const { body: { items: folders }, @@ -91,7 +91,7 @@ describe("GET /folder", () => { mockExtractUserEmailFromAuthToken("test+3@permanent.org"); const response = await agent - .get("/api/v2/folders?folderIds[]=2") + .get("/api/v2/folder?folderIds[]=2") .expect(200); const { body: { items: folders }, @@ -103,7 +103,7 @@ describe("GET /folder", () => { mockExtractUserEmailFromAuthToken(); mockExtractShareTokenFromHeaders("c0f523e4-48d8-4c39-8cda-5e95161532e4"); const response = await agent - .get("/api/v2/folders?folderIds[]=2") + .get("/api/v2/folder?folderIds[]=2") .set("X-Permanent-Share-Token", "c0f523e4-48d8-4c39-8cda-5e95161532e4") .expect(200); const { @@ -117,7 +117,7 @@ describe("GET /folder", () => { mockExtractUserEmailFromAuthToken(); mockExtractShareTokenFromHeaders("56f7c246-e4ec-41f3-b117-6df4c9377075"); const response = await agent - .get("/api/v2/folders?folderIds[]=2") + .get("/api/v2/folder?folderIds[]=2") .set("X-Permanent-Share-Token", "56f7c246-e4ec-41f3-b117-6df4c9377075") .expect(200); const { @@ -131,7 +131,7 @@ describe("GET /folder", () => { mockExtractUserEmailFromAuthToken(); mockExtractShareTokenFromHeaders("7d6412af-5abe-4acb-808a-64e9ce3b7535"); const response = await agent - .get("/api/v2/folders?folderIds[]=2") + .get("/api/v2/folder?folderIds[]=2") .set("X-Permanent-Share-Token", "7d6412af-5abe-4acb-808a-64e9ce3b7535") .expect(200); const { @@ -144,7 +144,7 @@ describe("GET /folder", () => { mockExtractUserEmailFromAuthToken(); mockExtractShareTokenFromHeaders("9cc057f0-d3e8-41df-94d6-9b315b4921af"); const response = await agent - .get("/api/v2/folders?folderIds[]=2") + .get("/api/v2/folder?folderIds[]=2") .set("X-Permanent-Share-Token", "9cc057f0-d3e8-41df-94d6-9b315b4921af") .expect(200); const { @@ -156,7 +156,7 @@ describe("GET /folder", () => { test("should return a private folder if the folder is shared with the caller", async () => { mockExtractUserEmailFromAuthToken("test+2@permanent.org"); const response = await agent - .get("/api/v2/folders?folderIds[]=2") + .get("/api/v2/folder?folderIds[]=2") .expect(200); const { body: { items: folders }, @@ -168,7 +168,7 @@ describe("GET /folder", () => { test("should not return a private folder if caller access relies on a deleted share", async () => { mockExtractUserEmailFromAuthToken("test+3@permanent.org"); const response = await agent - .get("/api/v2/folders?folderIds[]=2") + .get("/api/v2/folder?folderIds[]=2") .expect(200); const { body: { items: folders }, @@ -179,7 +179,7 @@ describe("GET /folder", () => { test("should not return a private folder if caller access relies on a share to an archive caller can no longer access", async () => { mockExtractUserEmailFromAuthToken("test+4@permanent.org"); const response = await agent - .get("/api/v2/folders?folderIds[]=2") + .get("/api/v2/folder?folderIds[]=2") .expect(200); const { body: { items: folders }, @@ -189,7 +189,7 @@ describe("GET /folder", () => { test("should return all folder data", async () => { const response = await agent - .get("/api/v2/folders?folderIds[]=2") + .get("/api/v2/folder?folderIds[]=2") .expect(200); const { body: { items: folders }, @@ -315,7 +315,7 @@ describe("GET /folder", () => { test("should not return parent object for a root level folder", async () => { const response = await agent - .get("/api/v2/folders?folderIds[]=7") + .get("/api/v2/folder?folderIds[]=7") .expect(200); const { body: { items: folders }, @@ -326,7 +326,7 @@ describe("GET /folder", () => { test("should not return pendingShares for non-manager viewer", async () => { mockExtractUserEmailFromAuthToken("test+1@permanent.org"); const response = await agent - .get("/api/v2/folders?folderIds[]=2") + .get("/api/v2/folder?folderIds[]=2") .expect(200); const { body: { items: folders }, @@ -337,7 +337,7 @@ describe("GET /folder", () => { test("should retrieve multiple folders if requested", async () => { const response = await agent - .get("/api/v2/folders?folderIds[]=2&folderIds[]=1") + .get("/api/v2/folder?folderIds[]=2&folderIds[]=1") .expect(200); const { body: { items: folders }, @@ -347,7 +347,7 @@ describe("GET /folder", () => { test("should not retrieve a deleted folder", async () => { const response = await agent - .get("/api/v2/folders?folderIds[]=4") + .get("/api/v2/folder?folderIds[]=4") .expect(200); const { body: { items: folders }, @@ -357,7 +357,7 @@ describe("GET /folder", () => { test("should not retrieve a folder with a deleted folder_link", async () => { const response = await agent - .get("/api/v2/folders?folderIds[]=3") + .get("/api/v2/folder?folderIds[]=3") .expect(200); const { body: { items: folders }, @@ -367,7 +367,7 @@ describe("GET /folder", () => { test("should omit size from a folder with a deleted folder_size", async () => { const response = await agent - .get("/api/v2/folders?folderIds[]=7") + .get("/api/v2/folder?folderIds[]=7") .expect(200); const { body: { items: folders }, @@ -378,6 +378,6 @@ describe("GET /folder", () => { test("should throw a 500 error if database call fails", async () => { vi.spyOn(db, "sql").mockRejectedValue(new Error("test error")); - await agent.get("/api/v2/folders?folderIds[]=2").expect(500); + await agent.get("/api/v2/folder?folderIds[]=2").expect(500); }); }); diff --git a/packages/api/src/folder/controller/get_folders_page.test.ts b/packages/api/src/folder/controller/get_folders_page.test.ts new file mode 100644 index 00000000..10d42e72 --- /dev/null +++ b/packages/api/src/folder/controller/get_folders_page.test.ts @@ -0,0 +1,212 @@ +import request from "supertest"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +import { app } from "../../app"; +import { db } from "../../database"; +import { + extractShareTokenFromHeaders, + extractUserEmailFromAuthToken, +} from "../../middleware"; +import type { GetFoldersResponse } from "../models"; +import { + mockExtractUserEmailFromAuthToken, + mockExtractShareTokenFromHeaders, +} from "../../../test/middleware_mocks"; +import { loadFixtures, clearDatabase } from "./utils_test"; + +vi.mock("../../database"); +vi.mock("../../middleware"); +vi.mock("@stela/logger"); + +const testEmail = "test@permanent.org"; +describe("GET /folders", () => { + const agent = request(app); + + beforeEach(async () => { + mockExtractUserEmailFromAuthToken(testEmail); + mockExtractShareTokenFromHeaders(); + + await loadFixtures(); + }); + + afterEach(async () => { + await clearDatabase(); + vi.restoreAllMocks(); + vi.clearAllMocks(); + }); + + test("expect a missing pageSize to cause a 400 error", async () => { + await agent.get("/api/v2/folders?folderIds[]=1").expect(400); + }); + + test("expect a missing folderIds to cause a 400 error", async () => { + await agent.get("/api/v2/folders?pageSize=100").expect(400); + }); + + test("should return 200 code for successful call", async () => { + await agent.get("/api/v2/folders?folderIds[]=1&pageSize=100").expect(200); + }); + + test("should call extractUserEmailFromAuthToken middleware", async () => { + await agent.get("/api/v2/folders?folderIds[]=1&pageSize=100").expect(200); + expect(extractUserEmailFromAuthToken).toHaveBeenCalled(); + }); + + test("should call extractShareTokenFromHeaders middleware", async () => { + await agent.get("/api/v2/folders?folderIds[]=1&pageSize=100").expect(200); + expect(extractShareTokenFromHeaders).toHaveBeenCalled(); + }); + + test("expect a response with an items array and pagination metadata", async () => { + const response = await agent + .get("/api/v2/folders?folderIds[]=1&pageSize=100") + .expect(200); + const { body } = response as { body: GetFoldersResponse }; + expect(body.items).toHaveLength(1); + expect(body.items[0]?.folderId).toEqual("1"); + expect(body.pagination).toBeDefined(); + }); + + test("expect no more than pageSize items to be returned", async () => { + const response = await agent + .get( + "/api/v2/folders?folderIds[]=1&folderIds[]=2&folderIds[]=12&pageSize=2", + ) + .expect(200); + const { body } = response as { body: GetFoldersResponse }; + expect(body.items).toHaveLength(2); + }); + + test("expect to page through all matching folders via cursor, in ascending folderId order", async () => { + const firstResponse = await agent + .get( + "/api/v2/folders?folderIds[]=1&folderIds[]=2&folderIds[]=12&folderIds[]=13&folderIds[]=100&pageSize=2", + ) + .expect(200); + const { body: firstPage } = firstResponse as { body: GetFoldersResponse }; + expect(firstPage.items.map((folder) => folder.folderId)).toEqual([ + "1", + "2", + ]); + expect(firstPage.pagination.totalPages).toEqual(3); + expect(firstPage.pagination.nextCursor).toBeDefined(); + + const secondResponse = await agent + .get( + `/api/v2/folders?folderIds[]=1&folderIds[]=2&folderIds[]=12&folderIds[]=13&folderIds[]=100&pageSize=2&cursor=${firstPage.pagination.nextCursor}`, + ) + .expect(200); + const { body: secondPage } = secondResponse as { + body: GetFoldersResponse; + }; + expect(secondPage.items.map((folder) => folder.folderId)).toEqual([ + "12", + "13", + ]); + expect(secondPage.pagination.totalPages).toEqual(3); + + const thirdResponse = await agent + .get( + `/api/v2/folders?folderIds[]=1&folderIds[]=2&folderIds[]=12&folderIds[]=13&folderIds[]=100&pageSize=2&cursor=${secondPage.pagination.nextCursor}`, + ) + .expect(200); + const { body: thirdPage } = thirdResponse as { body: GetFoldersResponse }; + expect(thirdPage.items.map((folder) => folder.folderId)).toEqual(["100"]); + expect(thirdPage.pagination.totalPages).toEqual(3); + }); + + test("expect pagination.nextPage to link to the next page with the same filters", async () => { + const response = await agent + .get( + "/api/v2/folders?folderIds[]=1&folderIds[]=2&folderIds[]=12&pageSize=2", + ) + .expect(200); + const { body } = response as { body: GetFoldersResponse }; + expect(body.pagination.nextPage).toEqual( + `https://${process.env["SITE_URL"] ?? ""}/api/v2/folders?folderIds%5B%5D=1&folderIds%5B%5D=2&folderIds%5B%5D=12&pageSize=2&cursor=${ + body.pagination.nextCursor ?? "" + }`, + ); + }); + + test("expect no items when querying with a cursor past the last item", async () => { + const response = await agent + .get("/api/v2/folders?folderIds[]=1&folderIds[]=2&pageSize=100") + .expect(200); + const { + body, + body: { + pagination: { nextCursor }, + }, + } = response as { body: GetFoldersResponse }; + expect(body.items).toHaveLength(2); + expect(nextCursor).toBeDefined(); + + const nextResponse = await agent + .get( + `/api/v2/folders?folderIds[]=1&folderIds[]=2&pageSize=100&cursor=${nextCursor}`, + ) + .expect(200); + const { body: nextBody } = nextResponse as { body: GetFoldersResponse }; + expect(nextBody.items).toHaveLength(0); + }); + + test("should return a public folder if the user is not authenticated", async () => { + mockExtractUserEmailFromAuthToken(); + const response = await agent + .get("/api/v2/folders?folderIds[]=1&pageSize=100") + .expect(200); + const { body } = response as { body: GetFoldersResponse }; + expect(body.items).toHaveLength(1); + expect(body.items[0]?.folderId).toEqual("1"); + }); + + test("should not return a private folder if the user is not authenticated", async () => { + mockExtractUserEmailFromAuthToken(); + const response = await agent + .get("/api/v2/folders?folderIds[]=2&pageSize=100") + .expect(200); + const { body } = response as { body: GetFoldersResponse }; + expect(body.items).toHaveLength(0); + }); + + test("should return a private folder if the user has a share token", async () => { + mockExtractUserEmailFromAuthToken(); + mockExtractShareTokenFromHeaders("c0f523e4-48d8-4c39-8cda-5e95161532e4"); + const response = await agent + .get("/api/v2/folders?folderIds[]=2&pageSize=100") + .set("X-Permanent-Share-Token", "c0f523e4-48d8-4c39-8cda-5e95161532e4") + .expect(200); + const { body } = response as { body: GetFoldersResponse }; + expect(body.items).toHaveLength(1); + expect(body.items[0]?.folderId).toEqual("2"); + }); + + test("should not retrieve a deleted folder", async () => { + const response = await agent + .get("/api/v2/folders?folderIds[]=4&pageSize=100") + .expect(200); + const { body } = response as { body: GetFoldersResponse }; + expect(body.items).toHaveLength(0); + }); + + test("should not retrieve a folder with a deleted folder_link", async () => { + const response = await agent + .get("/api/v2/folders?folderIds[]=3&pageSize=100") + .expect(200); + const { body } = response as { body: GetFoldersResponse }; + expect(body.items).toHaveLength(0); + }); + + test("should retrieve multiple folders if requested", async () => { + const response = await agent + .get("/api/v2/folders?folderIds[]=2&folderIds[]=1&pageSize=100") + .expect(200); + const { body } = response as { body: GetFoldersResponse }; + expect(body.items).toHaveLength(2); + }); + + test("should throw a 500 error if database call fails", async () => { + vi.spyOn(db, "sql").mockRejectedValue(new Error("test error")); + await agent.get("/api/v2/folders?folderIds[]=2&pageSize=100").expect(500); + }); +}); diff --git a/packages/api/src/folder/controller/patch_folder.test.ts b/packages/api/src/folder/controller/patch_folder.test.ts index f6d474a6..7c3d3247 100644 --- a/packages/api/src/folder/controller/patch_folder.test.ts +++ b/packages/api/src/folder/controller/patch_folder.test.ts @@ -186,6 +186,8 @@ describe("patch folder", () => { folderIds: ["1"], email: "test@permanent.org", shareToken: undefined, + pageSize: null, + cursor: undefined, }, { reject: testError }, ); @@ -229,6 +231,8 @@ describe("patch folder", () => { folderIds: ["1"], email: "test@permanent.org", shareToken: undefined, + pageSize: null, + cursor: undefined, }, { resolve: { rows: [] } }, ); diff --git a/packages/api/src/folder/index.ts b/packages/api/src/folder/index.ts index 9e31c1c1..81fbbfeb 100644 --- a/packages/api/src/folder/index.ts +++ b/packages/api/src/folder/index.ts @@ -1 +1 @@ -export { folderController } from "./controller/controller"; +export { folderController, foldersController } from "./controller/controller"; diff --git a/packages/api/src/folder/models.ts b/packages/api/src/folder/models.ts index 73b0a927..cee4b491 100644 --- a/packages/api/src/folder/models.ts +++ b/packages/api/src/folder/models.ts @@ -16,6 +16,15 @@ export interface GetFolderChildrenResponse { }; } +export interface GetFoldersResponse { + items: Folder[]; + pagination: { + nextCursor: string | undefined; + nextPage: string | undefined; + totalPages: number; + }; +} + export interface FolderRow { folderId: string; id: string; diff --git a/packages/api/src/folder/queries/get_folders.sql b/packages/api/src/folder/queries/get_folders.sql index 72edc71c..771c15e5 100644 --- a/packages/api/src/folder/queries/get_folders.sql +++ b/packages/api/src/folder/queries/get_folders.sql @@ -209,196 +209,228 @@ account_by_share AS ( account.primaryemail = :email AND access.status != 'status.generic.deleted' AND account_archive.status = 'status.generic.ok' -) +), -SELECT - folder.folderid AS "folderId", - folder.folderid AS id, - folder.archivenbr AS "archiveNumber", - folder_size.allfilesizedeep AS size, - aggregated_shares.folder_shares AS shares, - aggregated_tags.tags, - folder.createddt AS "createdAt", - folder.updateddt AS "updatedAt", - folder.description, - folder.displaydt AS "displayTimestamp", - folder.displayenddt AS "displayEndTimestamp", - folder.displaytime AS "displayTime", - folder.displayname AS "displayName", - folder.downloadname AS "downloadName", - folder.imageratio AS "imageRatio", - folder.publicdt AS "publicAt", - folder.sort, - folder.type, - folder.status, - folder.view, - folder_link.folder_linkid AS "folderLinkId", - CASE - WHEN - EXISTS ( - SELECT 1 - FROM account_archive - INNER JOIN account - ON - account_archive.accountid = account.accountid - AND account.primaryemail = :email - WHERE - account_archive.archiveid = folder.archiveid - AND account_archive.status = 'status.generic.ok' - AND account_archive.accessrole IN ( - 'access.role.owner', 'access.role.manager' +all_folders AS ( + SELECT + folder.folderid AS "folderId", + folder.folderid AS id, + folder.archivenbr AS "archiveNumber", + folder_size.allfilesizedeep AS size, + aggregated_shares.folder_shares AS shares, + aggregated_tags.tags, + folder.createddt AS "createdAt", + folder.updateddt AS "updatedAt", + folder.description, + folder.displaydt AS "displayTimestamp", + folder.displayenddt AS "displayEndTimestamp", + folder.displaytime AS "displayTime", + folder.displayname AS "displayName", + folder.downloadname AS "downloadName", + folder.imageratio AS "imageRatio", + folder.publicdt AS "publicAt", + folder.sort, + folder.type, + folder.status, + folder.view, + folder_link.folder_linkid AS "folderLinkId", + CASE + WHEN + EXISTS ( + SELECT 1 + FROM account_archive + INNER JOIN account + ON + account_archive.accountid = account.accountid + AND account.primaryemail = :email + WHERE + account_archive.archiveid = folder.archiveid + AND account_archive.status = 'status.generic.ok' + AND account_archive.accessrole IN ( + 'access.role.owner', 'access.role.manager' + ) + ) + THEN aggregated_pending_shares.pending_shares_as_json + END AS "pendingShares", + JSON_BUILD_OBJECT( + 'id', + locn.locnid::text, + 'name', + locn.name, + 'sublocation', + locn.sublocation, + 'city', + locn.city, + 'state', + locn.adminonename, + 'postalCode', + locn.postalcode, + 'country', + locn.country, + 'latitude', + locn.latitude, + 'longitude', + locn.longitude, + 'altitudeMeters', + locn.altitudemeters, + 'precision', + locn.locationprecision, + 'streetNumber', + locn.streetnumber, + 'streetName', + locn.streetname, + 'locality', + locn.locality, + 'county', + locn.admintwoname, + 'countryCode', + locn.countrycode, + 'displayName', + locn.displayname + ) AS location, + CASE + WHEN folder_link.parentfolderid IS NOT NULL + THEN + JSON_BUILD_OBJECT( + 'id', + folder_link.parentfolderid::text, + 'folderLinkId', + folder_link.parentfolder_linkid::text ) + END AS "parentFolder", + JSON_BUILD_OBJECT( + 'id', + folder.archiveid::text, + 'name', + profile_item.string1 + ) AS archive, + JSONB_BUILD_OBJECT( + 'names', + aggregated_path.name_path, + 'folderLinkIds', + aggregated_path.folder_link_id_path, + 'archiveNumbers', + aggregated_path.archive_number_path + ) AS paths, + JSON_BUILD_OBJECT( + '200', + folder.thumburl200, + '500', + folder.thumburl500, + '1000', + folder.thumburl1000, + '2000', + folder.thumburl2000, + '256', + folder.thumbnail256 + ) AS "thumbnailUrls" + FROM + folder + INNER JOIN + archive + ON + folder.archiveid = archive.archiveid + AND archive.status != 'status.generic.deleted' + AND archive.status IS NOT NULL + LEFT JOIN + profile_item + ON + archive.archiveid = profile_item.archiveid + AND profile_item.fieldnameui = 'profile.basic' + AND profile_item.status = 'status.generic.ok' + AND profile_item.string1 IS NOT NULL + INNER JOIN + folder_link + ON + folder.folderid = folder_link.folderid + AND folder_link.status != 'status.generic.deleted' + LEFT JOIN + folder_size + ON + folder.folderid = folder_size.folderid + AND folder_size.status != 'status.generic.deleted' + INNER JOIN + aggregated_path + ON + folder.folderid = aggregated_path.folderid + LEFT JOIN + account_by_archive + ON + folder.archiveid = account_by_archive.archiveid + LEFT JOIN + aggregated_shares + ON + folder_link.folder_linkid = aggregated_shares.folder_linkid + LEFT JOIN + aggregated_pending_shares + ON + folder_link.folder_linkid = aggregated_pending_shares.folder_linkid + LEFT JOIN + aggregated_tags + ON + folder.folderid = aggregated_tags.refid + LEFT JOIN + locn + ON + folder.locnid = locn.locnid + LEFT JOIN + account_by_share + ON + folder_link.folder_linkid = account_by_share.folder_linkid + LEFT JOIN + aggregated_ancestor_unrestricted_share_tokens + ON folder.folderid = aggregated_ancestor_unrestricted_share_tokens.folderid + WHERE + folder.folderid = ANY(:folderIds) + AND ( + ( + folder.publicdt IS NOT NULL + AND folder.publicdt <= NOW() + ) + OR ( + account_by_archive.primaryemail = :email + AND account_by_archive.primaryemail IS NOT NULL ) - THEN aggregated_pending_shares.pending_shares_as_json - END AS "pendingShares", - JSON_BUILD_OBJECT( - 'id', - locn.locnid::text, - 'name', - locn.name, - 'sublocation', - locn.sublocation, - 'city', - locn.city, - 'state', - locn.adminonename, - 'postalCode', - locn.postalcode, - 'country', - locn.country, - 'latitude', - locn.latitude, - 'longitude', - locn.longitude, - 'altitudeMeters', - locn.altitudemeters, - 'precision', - locn.locationprecision, - 'streetNumber', - locn.streetnumber, - 'streetName', - locn.streetname, - 'locality', - locn.locality, - 'county', - locn.admintwoname, - 'countryCode', - locn.countrycode, - 'displayName', - locn.displayname - ) AS location, - CASE - WHEN folder_link.parentfolderid IS NOT NULL - THEN - JSON_BUILD_OBJECT( - 'id', - folder_link.parentfolderid::text, - 'folderLinkId', - folder_link.parentfolder_linkid::text + OR ( + :shareToken::text IS NOT NULL + AND :shareToken = ANY( + aggregated_ancestor_unrestricted_share_tokens.tokens ) - END AS "parentFolder", - JSON_BUILD_OBJECT( - 'id', - folder.archiveid::text, - 'name', - profile_item.string1 - ) AS archive, - JSONB_BUILD_OBJECT( - 'names', - aggregated_path.name_path, - 'folderLinkIds', - aggregated_path.folder_link_id_path, - 'archiveNumbers', - aggregated_path.archive_number_path - ) AS paths, - JSON_BUILD_OBJECT( - '200', - folder.thumburl200, - '500', - folder.thumburl500, - '1000', - folder.thumburl1000, - '2000', - folder.thumburl2000, - '256', - folder.thumbnail256 - ) AS "thumbnailUrls" -FROM - folder -INNER JOIN - archive - ON - folder.archiveid = archive.archiveid - AND archive.status != 'status.generic.deleted' - AND archive.status IS NOT NULL -LEFT JOIN - profile_item - ON - archive.archiveid = profile_item.archiveid - AND profile_item.fieldnameui = 'profile.basic' - AND profile_item.status = 'status.generic.ok' - AND profile_item.string1 IS NOT NULL -INNER JOIN - folder_link - ON - folder.folderid = folder_link.folderid - AND folder_link.status != 'status.generic.deleted' -LEFT JOIN - folder_size - ON - folder.folderid = folder_size.folderid - AND folder_size.status != 'status.generic.deleted' -INNER JOIN - aggregated_path - ON - folder.folderid = aggregated_path.folderid -LEFT JOIN - account_by_archive - ON - folder.archiveid = account_by_archive.archiveid -LEFT JOIN - aggregated_shares - ON - folder_link.folder_linkid = aggregated_shares.folder_linkid -LEFT JOIN - aggregated_pending_shares - ON - folder_link.folder_linkid = aggregated_pending_shares.folder_linkid -LEFT JOIN - aggregated_tags - ON - folder.folderid = aggregated_tags.refid -LEFT JOIN - locn - ON - folder.locnid = locn.locnid -LEFT JOIN - account_by_share - ON - folder_link.folder_linkid = account_by_share.folder_linkid -LEFT JOIN - aggregated_ancestor_unrestricted_share_tokens - ON folder.folderid = aggregated_ancestor_unrestricted_share_tokens.folderid -WHERE - folder.folderid = ANY(:folderIds) - AND ( - ( - folder.publicdt IS NOT NULL - AND folder.publicdt <= NOW() - ) - OR ( - account_by_archive.primaryemail = :email - AND account_by_archive.primaryemail IS NOT NULL - ) - OR ( - :shareToken::text IS NOT NULL - AND :shareToken = ANY( - aggregated_ancestor_unrestricted_share_tokens.tokens + ) + OR ( + account_by_share.primaryemail = :email + AND account_by_share.primaryemail IS NOT NULL ) ) - OR ( - account_by_share.primaryemail = :email - AND account_by_share.primaryemail IS NOT NULL - ) - ) - AND folder.status != 'status.generic.deleted'; + AND folder.status != 'status.generic.deleted' + ORDER BY folder.folderid ASC +), + +ranked_folders AS ( + SELECT + *, + ROW_NUMBER() OVER (ORDER BY "folderId"::bigint ASC) AS rank + FROM all_folders +), + +cursor AS ( + SELECT rank + FROM + ranked_folders + WHERE + "folderId" = :cursor +), + +total_pages AS ( + SELECT CEILING(COUNT(*)::float / :pageSize) AS total_pages + FROM all_folders +) + +SELECT + ranked_folders.*, + (SELECT total_pages.total_pages FROM total_pages) AS "totalPages" +FROM ranked_folders +WHERE + ranked_folders.rank > COALESCE((SELECT cursor.rank FROM cursor), 0) +ORDER BY ranked_folders.rank ASC +LIMIT :pageSize; diff --git a/packages/api/src/folder/service.ts b/packages/api/src/folder/service.ts index d269dc64..e08d66b0 100644 --- a/packages/api/src/folder/service.ts +++ b/packages/api/src/folder/service.ts @@ -7,6 +7,7 @@ import type { Folder, PatchFolderRequest, GetFolderChildrenResponse, + GetFoldersResponse, FolderChildItem, } from "./models"; import { @@ -96,6 +97,16 @@ export const prettifyFolderView = (view: FolderView): PrettyFolderView => { } }; +const mapFolderRow = (row: FolderRow): Folder => ({ + ...row, + size: row.size === null ? null : +row.size, + imageRatio: +(row.imageRatio ?? 0), + sort: prettifyFolderSortType(row.sort), + type: prettifyFolderType(row.type), + status: prettifyFolderStatus(row.status), + view: prettifyFolderView(row.view), +}); + export const getFolders = async ( folderIds: string[], email?: string, @@ -106,24 +117,74 @@ export const getFolders = async ( folderIds, email, shareToken, + pageSize: null, + cursor: undefined, }) .catch((err: unknown) => { logger.error(err); throw new createError.InternalServerError("Failed to retrieve folders"); }); - const folders = result.rows.map( - (row: FolderRow): Folder => ({ - ...row, - size: row.size === null ? null : +row.size, - imageRatio: +(row.imageRatio ?? 0), - sort: prettifyFolderSortType(row.sort), - type: prettifyFolderType(row.type), - status: prettifyFolderStatus(row.status), - view: prettifyFolderView(row.view), - }), - ); - return folders; + return result.rows.map(mapFolderRow); +}; + +const buildFoldersNextPageUrl = ( + requestQuery: { + folderIds: string[]; + pageSize: number; + }, + nextCursor: string, +): string => { + const params = new URLSearchParams(); + requestQuery.folderIds.forEach((folderId) => { + params.append("folderIds[]", folderId); + }); + params.set("pageSize", String(requestQuery.pageSize)); + params.set("cursor", nextCursor); + return `https://${process.env["SITE_URL"] ?? ""}/api/v2/folders?${params.toString()}`; +}; + +export const getFoldersPage = async (requestQuery: { + folderIds: string[]; + email: string | undefined; + shareToken: string | undefined; + pageSize: number; + cursor: string | undefined; +}): Promise => { + const result = await db + .sql( + "folder.queries.get_folders", + { + folderIds: requestQuery.folderIds, + email: requestQuery.email, + shareToken: requestQuery.shareToken, + pageSize: requestQuery.pageSize, + cursor: requestQuery.cursor, + }, + ) + .catch((err: unknown) => { + logger.error(err); + throw new createError.InternalServerError("Failed to retrieve folders"); + }); + + const items = result.rows.map((row) => { + const { rank: _rank, totalPages: _totalPages, ...folderRow } = row; + return mapFolderRow(folderRow); + }); + + const nextCursor = items[items.length - 1]?.folderId; + + return { + items, + pagination: { + nextCursor, + nextPage: + nextCursor === undefined + ? undefined + : buildFoldersNextPageUrl(requestQuery, nextCursor), + totalPages: result.rows[0]?.totalPages ?? 0, + }, + }; }; export const getFolderChildren = async ( diff --git a/packages/api/src/folder/validators.test.ts b/packages/api/src/folder/validators.test.ts index e1b58892..45d6a578 100644 --- a/packages/api/src/folder/validators.test.ts +++ b/packages/api/src/folder/validators.test.ts @@ -1,6 +1,7 @@ import { validatePatchFolderRequest, validateGetFoldersQuery, + validateGetFoldersPageQuery, } from "./validators"; import { describe, expect, test } from "vitest"; @@ -197,3 +198,74 @@ describe("validateGetFoldersQuery", () => { } }); }); + +describe("validateGetFoldersPageQuery", () => { + test("should find no errors in a valid request", () => { + let error = null; + try { + validateGetFoldersPageQuery({ + folderIds: ["1", "2", "3"], + pageSize: 10, + }); + } catch (err) { + error = err; + } finally { + expect(error).toBeNull(); + } + }); + + test("should find no errors when a cursor is provided", () => { + let error = null; + try { + validateGetFoldersPageQuery({ + folderIds: ["1", "2"], + pageSize: 10, + cursor: "5", + }); + } catch (err) { + error = err; + } finally { + expect(error).toBeNull(); + } + }); + + test("should raise an error if pageSize is missing", () => { + let error = null; + try { + validateGetFoldersPageQuery({ + folderIds: ["1"], + }); + } catch (err) { + error = err; + } finally { + expect(error).not.toBeNull(); + } + }); + + test("should raise an error if pageSize is not an integer", () => { + let error = null; + try { + validateGetFoldersPageQuery({ + folderIds: ["1"], + pageSize: 1.5, + }); + } catch (err) { + error = err; + } finally { + expect(error).not.toBeNull(); + } + }); + + test("should raise an error if folderIds is missing", () => { + let error = null; + try { + validateGetFoldersPageQuery({ + pageSize: 10, + }); + } catch (err) { + error = err; + } finally { + expect(error).not.toBeNull(); + } + }); +}); diff --git a/packages/api/src/folder/validators.ts b/packages/api/src/folder/validators.ts index e2ed4059..53829f87 100644 --- a/packages/api/src/folder/validators.ts +++ b/packages/api/src/folder/validators.ts @@ -2,6 +2,7 @@ import Joi from "joi"; import { parse as parseEDTF } from "@edtf-ts/core"; import type { PatchFolderRequest } from "./models"; import { fieldsFromUserAuthentication } from "../validators"; +import { paginationFields } from "../validators/shared"; import { locationInputSchema } from "../location/validators"; import { EDTF_LEVEL_2 } from "../constants"; @@ -65,3 +66,25 @@ export const validateGetFoldersQuery: ( throw validation.error; } }; + +export const validateGetFoldersPageQuery: (data: unknown) => asserts data is { + folderIds: string[]; + cursor?: string; + pageSize: number; +} = ( + data: unknown, +): asserts data is { + folderIds: string[]; + cursor?: string; + pageSize: number; +} => { + const validation = Joi.object() + .keys({ + folderIds: Joi.array().items(Joi.string().required()).required(), + ...paginationFields, + }) + .validate(data); + if (validation.error !== undefined) { + throw validation.error; + } +}; diff --git a/packages/api/src/routes/index.ts b/packages/api/src/routes/index.ts index fff4a191..9b4788d4 100644 --- a/packages/api/src/routes/index.ts +++ b/packages/api/src/routes/index.ts @@ -11,7 +11,7 @@ import { promoController } from "../promo"; import { idpUserController } from "../idpuser"; import { recordController, recordsController } from "../record"; import { featureController } from "../feature_flag"; -import { folderController } from "../folder"; +import { folderController, foldersController } from "../folder"; import { shareLinkController } from "../share_link"; import { storagePurchaseController } from "../storage_purchase"; @@ -33,7 +33,7 @@ apiRoutes.use("/idpuser", idpUserController); apiRoutes.use("/records", recordsController); apiRoutes.use("/record", recordController); // Deprecated path maintained to avoid breaking changes apiRoutes.use("/feature-flags", featureController); -apiRoutes.use("/folders", folderController); +apiRoutes.use("/folders", foldersController); apiRoutes.use("/folder", folderController); // Deprecated path maintained to avoid breaking changes apiRoutes.use("/share-links", shareLinkController); apiRoutes.use("/storage-purchases", storagePurchaseController); From 189a5b6fe0b165e128db68fbbe35a5631a2e72fe Mon Sep 17 00:00:00 2001 From: Liam Lloyd-Tucker Date: Thu, 9 Jul 2026 10:59:52 -0700 Subject: [PATCH 3/4] Paginate GET /archives/:archiveId/folders/shared response Our API design principles say that endpoints that return lists of objects should paginate those lists. Currently, GET /archives/:archiveId/folders/shared returns an unpaginated response. This commit paginates it. No client currently uses this endpoint, so this should be a safe change to make. --- packages/api/docs/src/paths/archive.yaml | 15 ++++ .../api/src/archive/controller/controller.ts | 10 ++- .../controller/get_shared_folders.test.ts | 82 ++++++++++++++++--- .../fixtures/create_test_folder_links.sql | 12 +++ .../archive/fixtures/create_test_folders.sql | 12 +++ .../archive/fixtures/create_test_shares.sql | 12 +++ packages/api/src/archive/models.ts | 10 +++ .../archive/queries/get_shared_folders.sql | 41 ++++++++-- .../src/archive/service/get_shared_folders.ts | 53 ++++++++++-- packages/api/src/archive/validators.ts | 16 ++++ 10 files changed, 236 insertions(+), 27 deletions(-) diff --git a/packages/api/docs/src/paths/archive.yaml b/packages/api/docs/src/paths/archive.yaml index ef37d9b6..eb72f985 100644 --- a/packages/api/docs/src/paths/archive.yaml +++ b/packages/api/docs/src/paths/archive.yaml @@ -113,6 +113,19 @@ archives-id-folders-shared: required: true schema: type: string + - name: cursor + description: | + The id of the folder right before the first folder you want returned. + In most cases, this will be that of the last folder on the previous page. + in: query + required: false + schema: + type: string + - name: pageSize + in: query + required: true + schema: + type: integer get: summary: Get all the top-level shared folders in an archive security: @@ -132,6 +145,8 @@ archives-id-folders-shared: type: array items: $ref: "../models/folder.yaml#/folder" + pagination: + $ref: "../models/pagination_metadata.yaml#/pagination" "400": $ref: "../errors.yaml#/400" "401": diff --git a/packages/api/src/archive/controller/controller.ts b/packages/api/src/archive/controller/controller.ts index 30f8d13b..e1ad2a0e 100644 --- a/packages/api/src/archive/controller/controller.ts +++ b/packages/api/src/archive/controller/controller.ts @@ -11,6 +11,7 @@ import { validateBodyFromAuthentication, validateSearchQuery, validatePatchArchiveBody, + validateGetSharedFoldersQuery, } from "../validators"; import { archiveService } from "../service"; import { HTTP_STATUS } from "@pdc/http-status-codes"; @@ -171,11 +172,16 @@ archiveController.get( try { validateArchiveIdFromParams(req.params); validateBodyFromAuthentication(req.body); - const folders = await archiveService.getSharedFolders( + validateGetSharedFoldersQuery(req.query); + const response = await archiveService.getSharedFolders( req.params.archiveId, req.body.emailFromAuthToken, + { + pageSize: req.query.pageSize, + cursor: req.query.cursor, + }, ); - res.json({ items: folders }); + res.json(response); } catch (err) { next(err); } diff --git a/packages/api/src/archive/controller/get_shared_folders.test.ts b/packages/api/src/archive/controller/get_shared_folders.test.ts index eb868e6f..bd1d324e 100644 --- a/packages/api/src/archive/controller/get_shared_folders.test.ts +++ b/packages/api/src/archive/controller/get_shared_folders.test.ts @@ -6,7 +6,7 @@ import { logger } from "@stela/logger"; import { app } from "../../app"; import { verifyUserAuthentication } from "../../middleware"; import { db } from "../../database"; -import type { Folder } from "../../folder/models"; +import type { GetSharedFoldersResponse } from "../models"; import { mockVerifyUserAuthentication } from "../../../test/middleware_mocks"; vi.mock("../../database"); @@ -46,14 +46,13 @@ describe("getSharedFolders", () => { test("should return shared folders for an archive", async () => { const response = await agent - .get(`/api/v2/archive/2/folders/shared`) + .get(`/api/v2/archive/2/folders/shared?pageSize=100`) .expect(200); const { body: { items: folders }, - } = response as { body: { items: Folder[] } }; - expect(folders.length).toBe(1); - expect(folders[0]?.folderId).toBe("3"); + } = response as { body: GetSharedFoldersResponse }; + expect(folders.map((folder) => folder.folderId)).toEqual(["3", "200"]); }); test("should return 401 when not authenticated", async () => { @@ -63,18 +62,28 @@ describe("getSharedFolders", () => { }, ); - await agent.get(`/api/v2/archive/2/folders/shared`).expect(401); + await agent + .get(`/api/v2/archive/2/folders/shared?pageSize=100`) + .expect(401); }); test("should return 400 if the header data is missing", async () => { mockVerifyUserAuthentication(); + await agent + .get(`/api/v2/archive/2/folders/shared?pageSize=100`) + .expect(400); + }); + + test("should return 400 if pageSize is missing", async () => { await agent.get(`/api/v2/archive/2/folders/shared`).expect(400); }); test("should return 500 if database query fails", async () => { const testError = new Error("error: database connection lost"); vi.spyOn(db, "sql").mockRejectedValueOnce(testError); - await agent.get(`/api/v2/archive/2/folders/shared`).expect(500); + await agent + .get(`/api/v2/archive/2/folders/shared?pageSize=100`) + .expect(500); expect(logger.error).toHaveBeenCalledWith(testError); }); @@ -84,34 +93,81 @@ describe("getSharedFolders", () => { "553f3cb8-b753-43ce-83af-4443a404741b", ); const response = await agent - .get(`/api/v2/archive/2/folders/shared`) + .get(`/api/v2/archive/2/folders/shared?pageSize=100`) .expect(200); const { body: { items: folders }, - } = response as { body: { items: Folder[] } }; + } = response as { body: GetSharedFoldersResponse }; expect(folders.length).toBe(0); }); test("should not return folders where the share has been deleted", async () => { const response = await agent - .get(`/api/v2/archive/1/folders/shared`) + .get(`/api/v2/archive/1/folders/shared?pageSize=100`) .expect(200); const { body: { items: folders }, - } = response as { body: { items: Folder[] } }; + } = response as { body: GetSharedFoldersResponse }; expect(folders.length).toBe(0); }); test("should not return folders where archive membership has been deleted", async () => { const response = await agent - .get(`/api/v2/archive/3/folders/shared`) + .get(`/api/v2/archive/3/folders/shared?pageSize=100`) .expect(200); const { body: { items: folders }, - } = response as { body: { items: Folder[] } }; + } = response as { body: GetSharedFoldersResponse }; expect(folders.length).toBe(0); }); + + test("expect no more than pageSize items to be returned", async () => { + const response = await agent + .get(`/api/v2/archive/2/folders/shared?pageSize=1`) + .expect(200); + + const { body } = response as { body: GetSharedFoldersResponse }; + expect(body.items).toHaveLength(1); + expect(body.items[0]?.folderId).toEqual("3"); + expect(body.pagination.totalPages).toEqual(2); + expect(body.pagination.nextCursor).toEqual("3"); + }); + + test("expect to page through all shared folders via cursor, in ascending folderId order", async () => { + const firstResponse = await agent + .get(`/api/v2/archive/2/folders/shared?pageSize=1`) + .expect(200); + const { body: firstPage } = firstResponse as { + body: GetSharedFoldersResponse; + }; + expect(firstPage.items.map((folder) => folder.folderId)).toEqual(["3"]); + expect(firstPage.pagination.totalPages).toEqual(2); + expect(firstPage.pagination.nextCursor).toBeDefined(); + + const secondResponse = await agent + .get( + `/api/v2/archive/2/folders/shared?pageSize=1&cursor=${firstPage.pagination.nextCursor}`, + ) + .expect(200); + const { body: secondPage } = secondResponse as { + body: GetSharedFoldersResponse; + }; + expect(secondPage.items.map((folder) => folder.folderId)).toEqual(["200"]); + expect(secondPage.pagination.totalPages).toEqual(2); + }); + + test("expect pagination.nextPage to link to the next page with the same filters", async () => { + const response = await agent + .get(`/api/v2/archive/2/folders/shared?pageSize=1`) + .expect(200); + const { body } = response as { body: GetSharedFoldersResponse }; + expect(body.pagination.nextPage).toEqual( + `https://${process.env["SITE_URL"] ?? ""}/api/v2/archives/2/folders/shared?pageSize=1&cursor=${ + body.pagination.nextCursor ?? "" + }`, + ); + }); }); diff --git a/packages/api/src/archive/fixtures/create_test_folder_links.sql b/packages/api/src/archive/fixtures/create_test_folder_links.sql index adf640a6..150c7510 100644 --- a/packages/api/src/archive/fixtures/create_test_folder_links.sql +++ b/packages/api/src/archive/fixtures/create_test_folder_links.sql @@ -47,4 +47,16 @@ VALUES CURRENT_TIMESTAMP, CURRENT_TIMESTAMP, 'access.role.owner' +), +( + 10, + 200, + 2, + 11, + 0, + 'status.generic.ok', + 'type.folder_link.folder', + CURRENT_TIMESTAMP, + CURRENT_TIMESTAMP, + 'access.role.owner' ); diff --git a/packages/api/src/archive/fixtures/create_test_folders.sql b/packages/api/src/archive/fixtures/create_test_folders.sql index 272cddc0..357cbc80 100644 --- a/packages/api/src/archive/fixtures/create_test_folders.sql +++ b/packages/api/src/archive/fixtures/create_test_folders.sql @@ -203,4 +203,16 @@ VALUES CURRENT_TIMESTAMP, 'type.folder.root.root', NULL +), +( + 200, + 2, + NULL, + 'Second Shared Folder', + 'Second Shared Folder', + NULL, + 'status.generic.ok', + CURRENT_TIMESTAMP, + 'type.folder.private', + NULL ); diff --git a/packages/api/src/archive/fixtures/create_test_shares.sql b/packages/api/src/archive/fixtures/create_test_shares.sql index 8d9bca3c..aec8b93d 100644 --- a/packages/api/src/archive/fixtures/create_test_shares.sql +++ b/packages/api/src/archive/fixtures/create_test_shares.sql @@ -47,4 +47,16 @@ VALUES 0, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP +), +( + 10, + 10, + 2, + 'access.role.viewer', + 'status.generic.ok', + 'type.share.folder', + NULL, + 0, + CURRENT_TIMESTAMP, + CURRENT_TIMESTAMP ); diff --git a/packages/api/src/archive/models.ts b/packages/api/src/archive/models.ts index a08dd386..e67e1290 100644 --- a/packages/api/src/archive/models.ts +++ b/packages/api/src/archive/models.ts @@ -1,4 +1,5 @@ import { ArchiveMembershipRole } from "../access/models"; +import type { Folder } from "../folder/models"; export { ArchiveMembershipRole }; @@ -79,3 +80,12 @@ export interface GetArchivesResponse { totalPages: number; }; } + +export interface GetSharedFoldersResponse { + items: Folder[]; + pagination: { + nextCursor: string | undefined; + nextPage: string | undefined; + totalPages: number; + }; +} diff --git a/packages/api/src/archive/queries/get_shared_folders.sql b/packages/api/src/archive/queries/get_shared_folders.sql index fafe7dc9..53cded0b 100644 --- a/packages/api/src/archive/queries/get_shared_folders.sql +++ b/packages/api/src/archive/queries/get_shared_folders.sql @@ -6,10 +6,41 @@ WITH archive_access AS ( account.primaryemail = :email AND account_archive.archiveid = :archiveId AND account_archive.status = 'status.generic.ok' +), + +all_shared_folders AS ( + SELECT DISTINCT folder_link.folderid AS "folderId" + FROM folder_link + INNER JOIN share ON folder_link.folder_linkid = share.folder_linkid + INNER JOIN archive_access ON folder_link.archiveid = archive_access.archiveid + WHERE share.status = 'status.generic.ok' +), + +ranked_shared_folders AS ( + SELECT + "folderId", + ROW_NUMBER() OVER (ORDER BY "folderId"::BIGINT ASC) AS rank + FROM all_shared_folders +), + +cursor AS ( + SELECT rank + FROM + ranked_shared_folders + WHERE + "folderId" = :cursor +), + +total_pages AS ( + SELECT CEILING(COUNT(*)::FLOAT / :pageSize) AS total_pages + FROM all_shared_folders ) -SELECT folder_link.folderid AS "folderId" -FROM folder_link -INNER JOIN share ON folder_link.folder_linkid = share.folder_linkid -INNER JOIN archive_access ON folder_link.archiveid = archive_access.archiveid -WHERE share.status = 'status.generic.ok'; +SELECT + ranked_shared_folders."folderId", + (SELECT total_pages.total_pages FROM total_pages) AS "totalPages" +FROM ranked_shared_folders +WHERE + ranked_shared_folders.rank > COALESCE((SELECT cursor.rank FROM cursor), 0) +ORDER BY ranked_shared_folders.rank ASC +LIMIT :pageSize; diff --git a/packages/api/src/archive/service/get_shared_folders.ts b/packages/api/src/archive/service/get_shared_folders.ts index 01236d7f..fce50b84 100644 --- a/packages/api/src/archive/service/get_shared_folders.ts +++ b/packages/api/src/archive/service/get_shared_folders.ts @@ -2,17 +2,39 @@ import { logger } from "@stela/logger"; import createError from "http-errors"; import { db } from "../../database"; import { getFolders } from "../../folder/service"; -import type { Folder } from "../../folder/models"; +import type { GetSharedFoldersResponse } from "../models"; + +const buildSharedFoldersNextPageUrl = ( + archiveId: string, + pageSize: number, + nextCursor: string, +): string => { + const params = new URLSearchParams(); + params.set("pageSize", String(pageSize)); + params.set("cursor", nextCursor); + return `https://${ + process.env["SITE_URL"] ?? "" + }/api/v2/archives/${archiveId}/folders/shared?${params.toString()}`; +}; export const getSharedFolders = async ( archiveId: string, email: string, -): Promise => { + pagination: { + pageSize: number; + cursor: string | undefined; + }, +): Promise => { const result = await db - .sql<{ folderId: string }>("archive.queries.get_shared_folders", { - archiveId, - email, - }) + .sql<{ folderId: string; totalPages: number }>( + "archive.queries.get_shared_folders", + { + archiveId, + email, + pageSize: pagination.pageSize, + cursor: pagination.cursor, + }, + ) .catch((err: unknown) => { logger.error(err); throw new createError.InternalServerError( @@ -21,6 +43,23 @@ export const getSharedFolders = async ( }); const folderIds = result.rows.map((row) => row.folderId); + const items = await getFolders(folderIds, email); + + const nextCursor = items[items.length - 1]?.folderId; - return await getFolders(folderIds, email); + return { + items, + pagination: { + nextCursor, + nextPage: + nextCursor === undefined + ? undefined + : buildSharedFoldersNextPageUrl( + archiveId, + pagination.pageSize, + nextCursor, + ), + totalPages: result.rows[0]?.totalPages ?? 0, + }, + }; }; diff --git a/packages/api/src/archive/validators.ts b/packages/api/src/archive/validators.ts index 3277fe31..3798f20b 100644 --- a/packages/api/src/archive/validators.ts +++ b/packages/api/src/archive/validators.ts @@ -3,6 +3,7 @@ import { validateBodyFromAuthentication, fieldsFromUserAuthentication, } from "../validators"; +import { paginationFields } from "../validators/shared"; import { ArchiveMembershipRole, type MilestoneSortOrder } from "./models"; export { validateBodyFromAuthentication }; @@ -62,6 +63,21 @@ export const validateSearchQuery: (data: unknown) => asserts data is { } }; +export const validateGetSharedFoldersQuery: (data: unknown) => asserts data is { + pageSize: number; + cursor?: string; +} = ( + data: unknown, +): asserts data is { + pageSize: number; + cursor?: string; +} => { + const validation = Joi.object().keys(paginationFields).validate(data); + if (validation.error !== undefined) { + throw validation.error; + } +}; + export const validatePatchArchiveBody: (data: unknown) => asserts data is { emailFromAuthToken: string; milestoneSortOrder: MilestoneSortOrder; From 24c0a193792a6c2aca85b558dbbf76b240bd31c9 Mon Sep 17 00:00:00 2001 From: Liam Lloyd-Tucker Date: Thu, 9 Jul 2026 13:37:14 -0700 Subject: [PATCH 4/4] Add pagination to GET /share-links Our API design principles say that endpoints that return lists of objects should be paginated. This commit updates GET /share-links to comply with that. Unlike other paginated endpoints, makes pageSize optional for backward compatibility. pageSize defaults to 10, which is greater than the number of share links requested in any existing call to this endpoint. --- packages/api/docs/src/paths/share_link.yaml | 16 +++ packages/api/src/folder/service.ts | 3 +- packages/api/src/record/service.ts | 3 +- .../api/src/share_link/controller.test.ts | 78 ++++++++++- packages/api/src/share_link/controller.ts | 8 +- packages/api/src/share_link/models.ts | 9 ++ .../share_link/queries/get_share_links.sql | 121 +++++++++++------- packages/api/src/share_link/service.ts | 74 +++++++++-- .../api/src/share_link/validators.test.ts | 57 +++++++++ packages/api/src/share_link/validators.ts | 7 + 10 files changed, 318 insertions(+), 58 deletions(-) diff --git a/packages/api/docs/src/paths/share_link.yaml b/packages/api/docs/src/paths/share_link.yaml index 53d89b6e..f6585a44 100644 --- a/packages/api/docs/src/paths/share_link.yaml +++ b/packages/api/docs/src/paths/share_link.yaml @@ -87,6 +87,20 @@ share-link: type: array items: type: string + - name: cursor + description: | + The id of the share link right before the first share link you want returned. + In most cases, this will be that of the last share link on the previous page. + in: query + required: false + schema: + type: string + - name: pageSize + description: Defaults to 10 if not provided. + in: query + required: false + schema: + type: integer summary: Retrieve a share link by ID or token security: - {} @@ -107,6 +121,8 @@ share-link: type: array items: $ref: "../models/share_link.yaml#/shareLink" + pagination: + $ref: "../models/pagination_metadata.yaml#/pagination" "400": $ref: "../errors.yaml#/400" "401": diff --git a/packages/api/src/folder/service.ts b/packages/api/src/folder/service.ts index e08d66b0..c857fa30 100644 --- a/packages/api/src/folder/service.ts +++ b/packages/api/src/folder/service.ts @@ -365,6 +365,7 @@ export const getFolderShareLinks = async ( email, [], shareLinkIds, + { pageSize: null, cursor: undefined }, ); - return shareLinks; + return shareLinks.items; }; diff --git a/packages/api/src/record/service.ts b/packages/api/src/record/service.ts index aa1d3d8f..9c7d9a89 100644 --- a/packages/api/src/record/service.ts +++ b/packages/api/src/record/service.ts @@ -376,6 +376,7 @@ export const getRecordShareLinks = async ( email, [], shareLinkIds, + { pageSize: null, cursor: undefined }, ); - return shareLinks; + return shareLinks.items; }; diff --git a/packages/api/src/share_link/controller.test.ts b/packages/api/src/share_link/controller.test.ts index 73434fbc..5a549f4f 100644 --- a/packages/api/src/share_link/controller.test.ts +++ b/packages/api/src/share_link/controller.test.ts @@ -10,7 +10,7 @@ import { verifyUserAuthentication, } from "../middleware"; import { db } from "../database"; -import type { ShareLink } from "./models"; +import type { GetShareLinksResponse, ShareLink } from "./models"; import { mockVerifyUserAuthentication, mockExtractUserEmailFromAuthToken, @@ -706,6 +706,82 @@ describe("GET /share-links", () => { ); await agent.get("/api/v2/share-links?shareLinkIds[]=1000").expect(500); }); + + test("should default to a pageSize of 10 if not provided", async () => { + const response = await agent + .get( + "/api/v2/share-links?shareLinkIds[]=1000&shareLinkIds[]=1001&shareLinkIds[]=1002", + ) + .expect(200); + const { body } = response as { body: GetShareLinksResponse }; + expect(body.items).toHaveLength(3); + expect(body.pagination.totalPages).toEqual(1); + }); + + test("should limit results to the provided pageSize", async () => { + const response = await agent + .get( + "/api/v2/share-links?shareLinkIds[]=1000&shareLinkIds[]=1001&shareLinkIds[]=1002&pageSize=1", + ) + .expect(200); + const { body } = response as { body: GetShareLinksResponse }; + expect(body.items).toHaveLength(1); + expect(body.items[0]?.id).toEqual("1000"); + expect(body.pagination.totalPages).toEqual(3); + expect(body.pagination.nextCursor).toEqual("1000"); + }); + + test("should page through all matching share links via cursor, in ascending id order", async () => { + const firstResponse = await agent + .get( + "/api/v2/share-links?shareLinkIds[]=1000&shareLinkIds[]=1001&shareLinkIds[]=1002&pageSize=2", + ) + .expect(200); + const { body: firstPage } = firstResponse as { + body: GetShareLinksResponse; + }; + expect(firstPage.items.map((shareLink) => shareLink.id)).toEqual([ + "1000", + "1001", + ]); + expect(firstPage.pagination.totalPages).toEqual(2); + expect(firstPage.pagination.nextCursor).toBeDefined(); + + const secondResponse = await agent + .get( + `/api/v2/share-links?shareLinkIds[]=1000&shareLinkIds[]=1001&shareLinkIds[]=1002&pageSize=2&cursor=${ + firstPage.pagination.nextCursor ?? "" + }`, + ) + .expect(200); + const { body: secondPage } = secondResponse as { + body: GetShareLinksResponse; + }; + expect(secondPage.items.map((shareLink) => shareLink.id)).toEqual(["1002"]); + expect(secondPage.pagination.totalPages).toEqual(2); + }); + + test("should return pagination.nextPage linking to the next page with the same filters", async () => { + const response = await agent + .get( + "/api/v2/share-links?shareLinkIds[]=1000&shareLinkIds[]=1001&shareLinkIds[]=1002&pageSize=1", + ) + .expect(200); + const { body } = response as { body: GetShareLinksResponse }; + expect(body.pagination.nextPage).toEqual( + `https://${ + process.env["SITE_URL"] ?? "" + }/api/v2/share-links?shareLinkIds%5B%5D=1000&shareLinkIds%5B%5D=1001&shareLinkIds%5B%5D=1002&pageSize=1&cursor=${ + body.pagination.nextCursor ?? "" + }`, + ); + }); + + test("should return 400 if pageSize is not a positive integer", async () => { + await agent + .get("/api/v2/share-links?shareLinkIds[]=1000&pageSize=0") + .expect(400); + }); }); describe("DELETE /share-links", () => { diff --git a/packages/api/src/share_link/controller.ts b/packages/api/src/share_link/controller.ts index a36ed9d4..3652d832 100644 --- a/packages/api/src/share_link/controller.ts +++ b/packages/api/src/share_link/controller.ts @@ -65,12 +65,16 @@ shareLinkController.get( "Accessing share links by ID requires authentication", ); } - const shareLinks = await shareLinkService.getShareLinks( + const response = await shareLinkService.getShareLinks( req.body.emailFromAuthToken, req.query.shareTokens, req.query.shareLinkIds, + { + pageSize: req.query.pageSize, + cursor: req.query.cursor, + }, ); - res.status(HTTP_STATUS.SUCCESSFUL.OK).json({ items: shareLinks }); + res.status(HTTP_STATUS.SUCCESSFUL.OK).json(response); } catch (err) { next(err); } diff --git a/packages/api/src/share_link/models.ts b/packages/api/src/share_link/models.ts index 1a857a81..5ae1603a 100644 --- a/packages/api/src/share_link/models.ts +++ b/packages/api/src/share_link/models.ts @@ -87,3 +87,12 @@ export interface ShareLinkRow { createdAt: Date; updatedAt: Date; } + +export interface GetShareLinksResponse { + items: ShareLink[]; + pagination: { + nextCursor: string | undefined; + nextPage: string | undefined; + totalPages: number; + }; +} diff --git a/packages/api/src/share_link/queries/get_share_links.sql b/packages/api/src/share_link/queries/get_share_links.sql index d6cef15e..0f372465 100644 --- a/packages/api/src/share_link/queries/get_share_links.sql +++ b/packages/api/src/share_link/queries/get_share_links.sql @@ -1,47 +1,78 @@ +WITH all_share_links AS ( + SELECT + shareby_url.shareby_urlid AS id, + shareby_url.urltoken AS token, + shareby_url.uses AS "usesExpended", + shareby_url.expiresdt AS "expirationTimestamp", + shareby_url.createddt AS "createdAt", + shareby_url.updateddt AS "updatedAt", + SUBSTRING( + shareby_url.defaultaccessrole FROM (LENGTH('access.role.') + 1) + ) AS "permissionsLevel", + CASE + WHEN shareby_url.maxuses = 0 THEN NULL + ELSE shareby_url.maxuses + END AS "maxUses", + COALESCE(folder_link.recordid, folder_link.folderid) AS "itemId", + CASE + WHEN folder_link.recordid IS NOT NULL THEN 'record' + ELSE 'folder' + END AS "itemType", + CASE + WHEN shareby_url.unrestricted THEN 'none' + WHEN shareby_url.autoapprovetoggle = 1 THEN 'account' + ELSE 'approval' + END AS "accessRestrictions", + JSON_BUILD_OBJECT( + 'id', + shareby_url.byaccountid::text, + 'name', + account.fullname + ) AS "creatorAccount" + FROM + shareby_url + INNER JOIN + account + ON shareby_url.byaccountid = account.accountid + INNER JOIN + folder_link + ON shareby_url.folder_linkid = folder_link.folder_linkid + WHERE + ( + shareby_url.shareby_urlid::text = ANY(:shareLinkIds) + OR shareby_url.urltoken = ANY(:shareTokens) + ) + AND ( + account.primaryemail = :email + OR :email IS NULL + ) +), + +ranked_share_links AS ( + SELECT + *, + ROW_NUMBER() OVER (ORDER BY id::bigint ASC) AS rank + FROM all_share_links +), + +cursor AS ( + SELECT rank + FROM + ranked_share_links + WHERE + id = :cursor +), + +total_pages AS ( + SELECT CEILING(COUNT(*)::float / :pageSize) AS total_pages + FROM all_share_links +) + SELECT - shareby_url.shareby_urlid AS id, - shareby_url.urltoken AS token, - shareby_url.uses AS "usesExpended", - shareby_url.expiresdt AS "expirationTimestamp", - shareby_url.createddt AS "createdAt", - shareby_url.updateddt AS "updatedAt", - SUBSTRING( - shareby_url.defaultaccessrole FROM (LENGTH('access.role.') + 1) - ) AS "permissionsLevel", - CASE - WHEN shareby_url.maxuses = 0 THEN NULL - ELSE shareby_url.maxuses - END AS "maxUses", - COALESCE(folder_link.recordid, folder_link.folderid) AS "itemId", - CASE - WHEN folder_link.recordid IS NOT NULL THEN 'record' - ELSE 'folder' - END AS "itemType", - CASE - WHEN shareby_url.unrestricted THEN 'none' - WHEN shareby_url.autoapprovetoggle = 1 THEN 'account' - ELSE 'approval' - END AS "accessRestrictions", - JSON_BUILD_OBJECT( - 'id', - shareby_url.byaccountid::text, - 'name', - account.fullname - ) AS "creatorAccount" -FROM - shareby_url -INNER JOIN - account - ON shareby_url.byaccountid = account.accountid -INNER JOIN - folder_link - ON shareby_url.folder_linkid = folder_link.folder_linkid + ranked_share_links.*, + (SELECT total_pages.total_pages FROM total_pages) AS "totalPages" +FROM ranked_share_links WHERE - ( - shareby_url.shareby_urlid::text = ANY(:shareLinkIds) - OR shareby_url.urltoken = ANY(:shareTokens) - ) - AND ( - account.primaryemail = :email - OR :email IS NULL - ); + ranked_share_links.rank > COALESCE((SELECT cursor.rank FROM cursor), 0) +ORDER BY ranked_share_links.rank ASC +LIMIT :pageSize; diff --git a/packages/api/src/share_link/service.ts b/packages/api/src/share_link/service.ts index 1a315551..c4e0a76f 100644 --- a/packages/api/src/share_link/service.ts +++ b/packages/api/src/share_link/service.ts @@ -8,6 +8,7 @@ import type { ShareLinkRow, CreateShareLinkDatabaseParams, UpdateShareLinkDatabaseParams, + GetShareLinksResponse, } from "./models"; import { db } from "../database"; import { @@ -19,6 +20,7 @@ import { AccessRole } from "../access/models"; const APPROVAL_NOT_REQUIRED = 1; const APPROVAL_REQUIRED = 0; +const DEFAULT_PAGE_SIZE = 10; const createShareLinkRequestParamsToDatabaseParams = ( data: CreateShareLinkRequest, @@ -150,6 +152,8 @@ const updateShareLink = async ( shareLinkIds: [shareLinkId], shareTokens: [], email: data.emailFromAuthToken, + pageSize: null, + cursor: undefined, }) .catch((err: unknown) => { logger.error(err); @@ -209,22 +213,76 @@ const updateShareLink = async ( }; }; +const buildShareLinksNextPageUrl = ( + requestQuery: { + shareTokens: string[] | undefined; + shareLinkIds: string[] | undefined; + pageSize: number; + }, + nextCursor: string, +): string => { + const params = new URLSearchParams(); + requestQuery.shareTokens?.forEach((shareToken) => { + params.append("shareTokens[]", shareToken); + }); + requestQuery.shareLinkIds?.forEach((shareLinkId) => { + params.append("shareLinkIds[]", shareLinkId); + }); + params.set("pageSize", String(requestQuery.pageSize)); + params.set("cursor", nextCursor); + return `https://${ + process.env["SITE_URL"] ?? "" + }/api/v2/share-links?${params.toString()}`; +}; + const getShareLinks = async ( email: string | undefined, shareTokens: string[] | undefined, shareLinkIds: string[] | undefined, -): Promise => { - const shareLinks = await db - .sql("share_link.queries.get_share_links", { - email, - shareTokens, - shareLinkIds, - }) + pagination: { + pageSize: number | null | undefined; + cursor: string | undefined; + }, +): Promise => { + const pageSize = + pagination.pageSize === undefined ? DEFAULT_PAGE_SIZE : pagination.pageSize; + const result = await db + .sql( + "share_link.queries.get_share_links", + { + email, + shareTokens, + shareLinkIds, + pageSize, + cursor: pagination.cursor, + }, + ) .catch((err: unknown) => { logger.error(err); throw new Error("Failed to get share links"); }); - return shareLinks.rows; + + const items = result.rows.map((row) => { + const { rank: _rank, totalPages: _totalPages, ...shareLink } = row; + return shareLink; + }); + + const nextCursor = items[items.length - 1]?.id; + + return { + items, + pagination: { + nextCursor, + nextPage: + nextCursor === undefined || pageSize === null + ? undefined + : buildShareLinksNextPageUrl( + { shareTokens, shareLinkIds, pageSize }, + nextCursor, + ), + totalPages: result.rows[0]?.totalPages ?? 0, + }, + }; }; const deleteShareLink = async ( diff --git a/packages/api/src/share_link/validators.test.ts b/packages/api/src/share_link/validators.test.ts index 2d6ad989..40d8b286 100644 --- a/packages/api/src/share_link/validators.test.ts +++ b/packages/api/src/share_link/validators.test.ts @@ -937,6 +937,63 @@ describe("validateUpdateShareLinkRequest", () => { expect(error).not.toBeNull(); } }); + + test("should allow pageSize and cursor to be provided", () => { + let error = null; + try { + validateGetShareLinksParameters({ + shareLinkIds: ["1"], + pageSize: 5, + cursor: "1", + }); + } catch (err) { + error = err; + } finally { + expect(error).toBeNull(); + } + }); + + test("should error if pageSize is not an integer", () => { + let error = null; + try { + validateGetShareLinksParameters({ + shareLinkIds: ["1"], + pageSize: 1.5, + }); + } catch (err) { + error = err; + } finally { + expect(error).not.toBeNull(); + } + }); + + test("should error if pageSize is less than 1", () => { + let error = null; + try { + validateGetShareLinksParameters({ + shareLinkIds: ["1"], + pageSize: 0, + }); + } catch (err) { + error = err; + } finally { + expect(error).not.toBeNull(); + } + }); + + test("should error if cursor is not a string", () => { + let error = null; + try { + validateGetShareLinksParameters({ + shareLinkIds: ["1"], + cursor: 1, + }); + } catch (err) { + error = err; + } finally { + expect(error).not.toBeNull(); + } + }); }); describe("validateShareLinkParameters", () => { diff --git a/packages/api/src/share_link/validators.ts b/packages/api/src/share_link/validators.ts index d6607f12..c2c6470a 100644 --- a/packages/api/src/share_link/validators.ts +++ b/packages/api/src/share_link/validators.ts @@ -3,6 +3,7 @@ import type { CreateShareLinkRequest, UpdateShareLinkRequest } from "./models"; import { fieldsFromUserAuthentication } from "../validators"; const MINIMUM_MAX_USES = 1; +const MINIMUM_PAGE_SIZE = 1; export const validateCreateShareLinkRequest: ( data: unknown, @@ -116,16 +117,22 @@ export const validateGetShareLinksParameters: ( ) => asserts data is { shareTokens: string[] | undefined; shareLinkIds: string[] | undefined; + cursor: string | undefined; + pageSize: number | undefined; } = ( data: unknown, ): asserts data is { shareTokens: string[] | undefined; shareLinkIds: string[] | undefined; + cursor: string | undefined; + pageSize: number | undefined; } => { const validation = Joi.object() .keys({ shareTokens: Joi.array().items(Joi.string()).optional(), shareLinkIds: Joi.array().items(Joi.string()).optional(), + cursor: Joi.string().optional(), + pageSize: Joi.number().integer().min(MINIMUM_PAGE_SIZE).optional(), }) .validate(data); if (validation.error !== undefined) {