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/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/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/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/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; 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..c857fa30 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 ( @@ -304,6 +365,7 @@ export const getFolderShareLinks = async ( email, [], shareLinkIds, + { pageSize: null, cursor: undefined }, ); - return shareLinks; + return shareLinks.items; }; 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/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..9c7d9a89 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, @@ -306,6 +376,7 @@ export const getRecordShareLinks = async ( email, [], shareLinkIds, + { pageSize: null, cursor: undefined }, ); - return shareLinks; + return shareLinks.items; }; 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..9b4788d4 100644 --- a/packages/api/src/routes/index.ts +++ b/packages/api/src/routes/index.ts @@ -9,9 +9,9 @@ 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 { folderController, foldersController } from "../folder"; import { shareLinkController } from "../share_link"; import { storagePurchaseController } from "../storage_purchase"; @@ -30,10 +30,10 @@ 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); +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); 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) {