From 1db1e1321db50bdef35330c2758dcd87151bd008 Mon Sep 17 00:00:00 2001 From: Daniel Salazar Date: Sat, 10 Oct 2026 18:51:07 -0400 Subject: [PATCH] refactor(stores): one keyset page helper for store listings Thirteen store listings each cut a LIMIT n+1 read to a page and sealed the last row's position, and each opened an id cursor its own way. They now use keysetPage() and openIdCursor() from util/pagination. Cursor payloads and sealing are unchanged; a cursor without a usable id still fails with 400 in ShareStore and still restarts the listing elsewhere (strict option). --- .../stores/events/DurableSubscriptionStore.ts | 18 ++-- .../stores/events/EventHandlerStore.ts | 14 +-- .../stores/events/KvShareHandleStore.ts | 25 ++---- src/backend/stores/fs/FSEntryStore.ts | 86 +++++++++---------- .../stores/permission/PermissionStore.ts | 23 ++--- src/backend/stores/share/ShareStore.js | 73 +++++----------- src/backend/stores/team/TeamStore.ts | 86 ++++++------------- src/backend/util/pagination.test.ts | 63 ++++++++++++++ src/backend/util/pagination.ts | 49 +++++++++++ 9 files changed, 228 insertions(+), 209 deletions(-) diff --git a/src/backend/stores/events/DurableSubscriptionStore.ts b/src/backend/stores/events/DurableSubscriptionStore.ts index 40f96caf9..1e7928443 100644 --- a/src/backend/stores/events/DurableSubscriptionStore.ts +++ b/src/backend/stores/events/DurableSubscriptionStore.ts @@ -26,8 +26,8 @@ import { HttpError } from '../../core/http/HttpError.js'; import type { DeliveryClass } from '../../services/events/registry.js'; import type { FsOp } from '../../services/events/subjects.js'; import { - openCursor, - sealCursor, + keysetPage, + openIdCursor, type PageResult, } from '../../util/pagination.js'; import { PuterStore } from '../types.js'; @@ -587,9 +587,7 @@ export class DurableSubscriptionStore extends PuterStore { ), DURABLE_LIST_LIMIT_CAP, ); - const after = asNumber( - openCursor(options.cursor, this.config.jwt_secret_v2)?.id, - ); + const after = openIdCursor(options.cursor, this.config.jwt_secret_v2); // The scope half of the predicate is what the total counts over; the // cursor half only positions one page inside it. @@ -613,15 +611,11 @@ export class DurableSubscriptionStore extends PuterStore { [...params, limit + 1], ); - const page = rows.slice(0, limit); + const page = keysetPage(rows, limit, this.config.jwt_secret_v2); const result: PageResult = { - items: page.map(toRow), + items: page.rows.map(toRow), }; - if (rows.length > limit) - result.cursor = sealCursor( - { id: Number(page[page.length - 1].id) }, - this.config.jwt_secret_v2, - ); + if (page.cursor) result.cursor = page.cursor; if (options.includeTotal) { const [count] = await this.clients.db.read( diff --git a/src/backend/stores/events/EventHandlerStore.ts b/src/backend/stores/events/EventHandlerStore.ts index 4ec68bbd1..b0cdb497a 100644 --- a/src/backend/stores/events/EventHandlerStore.ts +++ b/src/backend/stores/events/EventHandlerStore.ts @@ -25,7 +25,7 @@ import { import { HttpError } from '../../core/http/HttpError.js'; import { decodeCursor, - encodeCursor, + keysetPage, type PageResult, } from '../../util/pagination.js'; import { isUniqueViolation } from '../../util/dbError.js'; @@ -477,9 +477,12 @@ export class EventHandlerStore extends PuterStore { [...params, limit + 1], ); - const page = rows.slice(0, limit); + // An app uid rather than a sequence position, so it needn't be sealed. + const page = keysetPage(rows, limit, undefined, (last) => ({ + appUid: last.app_uid, + })); const result: PageResult = { - items: page.map((row) => ({ + items: page.rows.map((row) => ({ appUid: String(row.app_uid), appName: String(row.app_name), appTitle: String(row.app_title), @@ -488,10 +491,7 @@ export class EventHandlerStore extends PuterStore { updatedAt: Number(row.updated_at) || 0, })), }; - if (rows.length > limit) - result.cursor = encodeCursor({ - appUid: page[page.length - 1]!.app_uid, - }); + if (page.cursor) result.cursor = page.cursor; return result; } diff --git a/src/backend/stores/events/KvShareHandleStore.ts b/src/backend/stores/events/KvShareHandleStore.ts index e17da439e..9db58d99e 100644 --- a/src/backend/stores/events/KvShareHandleStore.ts +++ b/src/backend/stores/events/KvShareHandleStore.ts @@ -19,8 +19,8 @@ import { mintKvHandleId } from '../../services/events/kvShares.js'; import { - openCursor, - sealCursor, + keysetPage, + openIdCursor, type PageResult, } from '../../util/pagination.js'; import { PuterStore } from '../types.js'; @@ -78,11 +78,6 @@ export interface FindLiveKvShareHandleInput { const nowSeconds = (): number => Math.floor(Date.now() / 1000); -const asNumber = (value: unknown): number | null => { - const parsed = Number(value); - return Number.isFinite(parsed) ? parsed : null; -}; - const toRow = (row: Record): KvShareHandle => ({ handle: String(row.handle), ownerUserId: Number(row.owner_user_id), @@ -254,9 +249,7 @@ export class KvShareHandleStore extends PuterStore { ), KV_HANDLE_LIST_LIMIT_CAP, ); - const after = asNumber( - openCursor(options.cursor, this.config.jwt_secret_v2)?.id, - ); + const after = openIdCursor(options.cursor, this.config.jwt_secret_v2); const filter = ['`owner_user_id` = ?']; const filterParams: unknown[] = [ownerUserId]; @@ -278,13 +271,11 @@ export class KvShareHandleStore extends PuterStore { [...params, limit + 1], ); - const page = rows.slice(0, limit); - const result: PageResult = { items: page.map(toRow) }; - if (rows.length > limit) - result.cursor = sealCursor( - { id: Number(page[page.length - 1].id) }, - this.config.jwt_secret_v2, - ); + const page = keysetPage(rows, limit, this.config.jwt_secret_v2); + const result: PageResult = { + items: page.rows.map(toRow), + }; + if (page.cursor) result.cursor = page.cursor; if (options.includeTotal) { const [count] = await this.clients.db.read( diff --git a/src/backend/stores/fs/FSEntryStore.ts b/src/backend/stores/fs/FSEntryStore.ts index c31c7ec3a..387eda9b7 100644 --- a/src/backend/stores/fs/FSEntryStore.ts +++ b/src/backend/stores/fs/FSEntryStore.ts @@ -25,9 +25,9 @@ import { HttpError } from '../../core/http/HttpError.js'; import type { LayerInstances } from '../../types.js'; import { runWithConcurrencyLimit } from '../../util/concurrency.js'; import { + keysetPage, normalizeLimit, openCursor, - sealCursor, } from '../../util/pagination.js'; import type { puterStores } from '../index.js'; import { PuterStore } from '../types.js'; @@ -2679,33 +2679,31 @@ export class FSEntryStore extends PuterStore { params, )) as unknown as FSEntryRow[]; - const hasMore = rows.length > limit; - const pageRows = hasMore ? rows.slice(0, limit) : rows; - const entries = await this.#finalizeChildEntries(pageRows); - - let cursor: string | undefined; - if (hasMore) { - const last = pageRows[pageRows.length - 1]!; - const v = (() => { - switch (sortBy) { - case 'modified': - return last.modified ?? 0; - case 'size': - return last.size ?? -1; - case 'type': - return last.is_dir; - case 'name': - default: - return last.name; - } - })(); - cursor = sealCursor( - { v, id: Number(last.id), s: sortBy, o: sortOrder }, - this.config.jwt_secret_v2, - ); - } - - return { entries, ...(cursor ? { cursor } : {}) }; + const page = keysetPage( + rows, + limit, + this.config.jwt_secret_v2, + (last) => ({ + v: (() => { + switch (sortBy) { + case 'modified': + return last.modified ?? 0; + case 'size': + return last.size ?? -1; + case 'type': + return last.is_dir; + case 'name': + default: + return last.name; + } + })(), + id: Number(last.id), + s: sortBy, + o: sortOrder, + }), + ); + const entries = await this.#finalizeChildEntries(page.rows); + return { entries, ...(page.cursor ? { cursor: page.cursor } : {}) }; } async countChildren(parentUid: string): Promise { @@ -2882,25 +2880,19 @@ export class FSEntryStore extends PuterStore { params, )) as unknown as FSEntryRow[]; - const hasMore = rows.length > limit; - const pageRows = hasMore ? rows.slice(0, limit) : rows; - const entries = await this.#finalizeChildEntries(pageRows); - - let cursor: string | undefined; - if (hasMore) { - const last = pageRows[pageRows.length - 1]!; - cursor = sealCursor( - { - v: descendantSortValue(last, sortBy), - ...(tiebreak ? { id: Number(last.id) } : {}), - s: sortBy, - o: sortOrder, - }, - this.config.jwt_secret_v2, - ); - } - - return { entries, ...(cursor ? { cursor } : {}) }; + const page = keysetPage( + rows, + limit, + this.config.jwt_secret_v2, + (last) => ({ + v: descendantSortValue(last, sortBy), + ...(tiebreak ? { id: Number(last.id) } : {}), + s: sortBy, + o: sortOrder, + }), + ); + const entries = await this.#finalizeChildEntries(page.rows); + return { entries, ...(page.cursor ? { cursor: page.cursor } : {}) }; } async countDescendantsToDepth( diff --git a/src/backend/stores/permission/PermissionStore.ts b/src/backend/stores/permission/PermissionStore.ts index c53eb2cd6..34f429f8a 100644 --- a/src/backend/stores/permission/PermissionStore.ts +++ b/src/backend/stores/permission/PermissionStore.ts @@ -28,7 +28,7 @@ import { PERMISSION_SCAN_CACHE_TTL_SECONDS, } from '../../services/permission/consts'; import { kv } from '../../util/kvSingleton'; -import { openCursor, sealCursor } from '../../util/pagination'; +import { keysetPage, openIdCursor } from '../../util/pagination'; import type { UserRow } from '../user/UserStore'; // Short TTLs: FK CASCADE on user/app delete + PermissionService rewriters @@ -667,12 +667,10 @@ export class PermissionStore extends PuterStore { Math.max(1, Math.floor(Number(opts.limit) || AUDIT_PAGE_SIZE)), MAX_AUDIT_PAGE_SIZE, ); - const decoded = openCursor( - opts.cursor, - this.config.jwt_secret_v2, - 'audit cursor', - ); - const beforeId = Number(decoded?.id ?? 0) || null; + const beforeId = + openIdCursor(opts.cursor, this.config.jwt_secret_v2, { + label: 'audit cursor', + }) || null; const { sql, params } = this.#auditWhere(filter); const rows = await this.clients.db.read( @@ -683,15 +681,10 @@ export class PermissionStore extends PuterStore { [...params, ...(beforeId === null ? [] : [beforeId]), size + 1], ); - const hasMore = rows.length > size; - const items = rows.slice(0, size).map((row) => this.#auditRow(row)); - const last = items[items.length - 1]; + const page = keysetPage(rows, size, this.config.jwt_secret_v2); return { - items, - cursor: - hasMore && last - ? sealCursor({ id: last.id }, this.config.jwt_secret_v2) - : undefined, + items: page.rows.map((row) => this.#auditRow(row)), + cursor: page.cursor, }; } diff --git a/src/backend/stores/share/ShareStore.js b/src/backend/stores/share/ShareStore.js index 8e8dc2edb..a5d436d83 100644 --- a/src/backend/stores/share/ShareStore.js +++ b/src/backend/stores/share/ShareStore.js @@ -19,12 +19,7 @@ import { v4 as uuidv4 } from 'uuid'; import { HttpError } from '../../core/http/HttpError.js'; -import { - decodeCursor, - encodeCursor, - openCursor, - sealCursor, -} from '../../util/pagination'; +import { decodeCursor, keysetPage, openIdCursor } from '../../util/pagination'; import { PuterStore } from '../types'; /** Default page size for the keyset listings. */ @@ -110,17 +105,10 @@ export class ShareStore extends PuterStore { [...holderParams, afterId, size + 1], ); - const hasMore = rows.length > size; - const items = (hasMore ? rows.slice(0, size) : rows).map((r) => - this.#normalizeRow(r), - ); - const last = items[items.length - 1]; + const page = keysetPage(rows, size, this.config.jwt_secret_v2); return { - items, - cursor: - hasMore && last - ? sealCursor({ id: last.id }, this.config.jwt_secret_v2) - : undefined, + items: page.rows.map((r) => this.#normalizeRow(r)), + cursor: page.cursor, }; } @@ -184,15 +172,10 @@ export class ShareStore extends PuterStore { const merged = [...issued, ...delegated, ...unrecorded].sort( (a, b) => Number(a.id) - Number(b.id), ); - const hasMore = merged.length > size; - const items = merged.slice(0, size).map((r) => this.#normalizeRow(r)); - const last = items[items.length - 1]; + const page = keysetPage(merged, size, this.config.jwt_secret_v2); return { - items, - cursor: - hasMore && last - ? sealCursor({ id: last.id }, this.config.jwt_secret_v2) - : undefined, + items: page.rows.map((r) => this.#normalizeRow(r)), + cursor: page.cursor, }; } @@ -277,18 +260,16 @@ export class ShareStore extends PuterStore { ], ); - const hasMore = rows.length > size; - const items = rows.slice(0, size).map((row) => ({ - appUid: row.app_uid === '' ? null : String(row.app_uid), - count: Number(row.count), + // An app uid rather than a sequence position, so it needn't be sealed. + const page = keysetPage(rows, size, undefined, (last) => ({ + appUid: String(last.app_uid), })); - const last = items[items.length - 1]; return { - items, - cursor: - hasMore && last - ? encodeCursor({ appUid: last.appUid ?? '' }) - : undefined, + items: page.rows.map((row) => ({ + appUid: row.app_uid === '' ? null : String(row.app_uid), + count: Number(row.count), + })), + cursor: page.cursor, }; } @@ -1252,25 +1233,17 @@ export class ShareStore extends PuterStore { } /** - * The id a keyset page resumes after; 0 for the first page. A cursor that - * decodes but names no usable id — another endpoint's cursor, say — is - * refused rather than read as page one, which would silently restart a - * client's iteration from the top. + * The id a keyset page resumes after; 0 for the first page. A cursor + * without a usable id is refused rather than read as page one, which would + * silently restart a client's iteration from the top. */ #afterId(cursor) { - const decoded = openCursor( - cursor, - this.config.jwt_secret_v2, - 'share cursor', + return ( + openIdCursor(cursor, this.config.jwt_secret_v2, { + label: 'share cursor', + strict: true, + }) ?? 0 ); - if (decoded === undefined) return 0; - const id = Number(decoded.id); - if (!Number.isInteger(id) || id < 0) { - throw new HttpError(400, 'invalid share cursor', { - legacyCode: 'bad_request', - }); - } - return id; } /** diff --git a/src/backend/stores/team/TeamStore.ts b/src/backend/stores/team/TeamStore.ts index 34000db0e..dbad87890 100644 --- a/src/backend/stores/team/TeamStore.ts +++ b/src/backend/stores/team/TeamStore.ts @@ -19,9 +19,9 @@ import { v4 as uuidv4 } from 'uuid'; import { + keysetPage, normalizeLimit, - openCursor, - sealCursor, + openIdCursor, type PageResult, } from '../../util/pagination.js'; import { PuterStore } from '../types'; @@ -440,12 +440,9 @@ export class TeamStore extends PuterStore { const limit = normalizeLimit(opts.limit, { cap: MEMBER_PAGE_CAP }) ?? MEMBER_PAGE_SIZE; - const page = openCursor( - opts.cursor, - this.config.jwt_secret_v2, - 'team member cursor', - ); - const after = typeof page?.id === 'number' ? page.id : null; + const after = openIdCursor(opts.cursor, this.config.jwt_secret_v2, { + label: 'team member cursor', + }); // One row past the limit is how we know a further page exists. const rows = (await this.clients.db.read( @@ -462,15 +459,8 @@ export class TeamStore extends PuterStore { : [teamUid, TEAM_KIND, after, limit + 1], )) as unknown as TeamMemberRow[]; - const items = rows.slice(0, limit); - const cursor = - rows.length > limit - ? sealCursor( - { id: items[items.length - 1].id }, - this.config.jwt_secret_v2, - ) - : undefined; - return { items, cursor }; + const page = keysetPage(rows, limit, this.config.jwt_secret_v2); + return { items: page.rows, cursor: page.cursor }; } /** @@ -485,12 +475,9 @@ export class TeamStore extends PuterStore { const limit = normalizeLimit(opts.limit, { cap: MEMBER_PAGE_CAP }) ?? MEMBER_PAGE_SIZE; - const page = openCursor( - opts.cursor, - this.config.jwt_secret_v2, - 'team directory cursor', - ); - const after = typeof page?.id === 'number' ? page.id : null; + const after = openIdCursor(opts.cursor, this.config.jwt_secret_v2, { + label: 'team directory cursor', + }); const rows = (await this.clients.db.read( 'SELECT ug.`id`, u.`username`, u.`uuid` FROM `jct_user_group` ug ' + @@ -505,15 +492,8 @@ export class TeamStore extends PuterStore { : [teamUid, TEAM_KIND, after, limit + 1], )) as unknown as { id: number; username: string; uuid: string }[]; - const items = rows.slice(0, limit); - const cursor = - rows.length > limit - ? sealCursor( - { id: items[items.length - 1].id }, - this.config.jwt_secret_v2, - ) - : undefined; - return { items, cursor }; + const page = keysetPage(rows, limit, this.config.jwt_secret_v2); + return { items: page.rows, cursor: page.cursor }; } /** Internal ids of this user's live teams, for the share listing. */ @@ -633,12 +613,9 @@ export class TeamStore extends PuterStore { const limit = normalizeLimit(opts.limit, { cap: MEMBER_PAGE_CAP }) ?? MEMBER_PAGE_SIZE; - const page = openCursor( - opts.cursor, - this.config.jwt_secret_v2, - 'team cursor', - ); - const after = typeof page?.id === 'number' ? page.id : null; + const after = openIdCursor(opts.cursor, this.config.jwt_secret_v2, { + label: 'team cursor', + }); const open = opts.openOnly ? ' AND g.`directory_enabled` = 1' : ''; const rows = (await this.clients.db.read( @@ -653,14 +630,11 @@ export class TeamStore extends PuterStore { : [userId, TEAM_KIND, after, limit + 1], )) as unknown as TeamRow[]; - const items = rows.slice(0, limit); - const cursor = - rows.length > limit - ? sealCursor( - { id: items[items.length - 1].id }, - this.config.jwt_secret_v2, - ) - : undefined; + const { rows: items, cursor } = keysetPage( + rows, + limit, + this.config.jwt_secret_v2, + ); let total: number | undefined; if (opts.includeTotal) { @@ -903,12 +877,9 @@ export class TeamStore extends PuterStore { const limit = normalizeLimit(opts.limit, { cap: AUDIT_PAGE_CAP }) ?? AUDIT_PAGE_SIZE; - const page = openCursor( - opts.cursor, - this.config.jwt_secret_v2, - 'team audit cursor', - ); - const before = typeof page?.id === 'number' ? page.id : null; + const before = openIdCursor(opts.cursor, this.config.jwt_secret_v2, { + label: 'team audit cursor', + }); // Unix seconds in SQL: the mysql driver reads a stored UTC datetime // as local, which shifts every row away from the sign-ins. @@ -928,15 +899,8 @@ export class TeamStore extends PuterStore { : [...params, before, limit + 1], )) as unknown as TeamAuditRow[]; - const items = rows.slice(0, limit); - const cursor = - rows.length > limit - ? sealCursor( - { id: items[items.length - 1].id }, - this.config.jwt_secret_v2, - ) - : undefined; - return { items, cursor }; + const page = keysetPage(rows, limit, this.config.jwt_secret_v2); + return { items: page.rows, cursor: page.cursor }; } /** Removes a member, returning whether a row was there to remove. */ diff --git a/src/backend/util/pagination.test.ts b/src/backend/util/pagination.test.ts index 0801c971d..181f72668 100644 --- a/src/backend/util/pagination.test.ts +++ b/src/backend/util/pagination.test.ts @@ -2,9 +2,11 @@ import { describe, expect, it } from 'vitest'; import { decodeCursor, encodeCursor, + keysetPage, normalizeLimit, normalizeOffset, openCursor, + openIdCursor, sealCursor, } from './pagination'; import { HttpError } from '../core/http'; @@ -139,4 +141,65 @@ describe('pagination util', () => { expect(normalizeOffset(5000, { cap: 5000 })).toBe(5000); }); }); + + describe('openIdCursor', () => { + const secret = 'keyset-secret'; + + it('returns null without a cursor', () => { + expect(openIdCursor(undefined, secret)).toBeNull(); + expect(openIdCursor('', secret)).toBeNull(); + }); + + it('reads the id from a sealed or plain cursor', () => { + expect(openIdCursor(sealCursor({ id: 41 }, secret), secret)).toBe( + 41, + ); + expect(openIdCursor(encodeCursor({ id: 7 }), secret)).toBe(7); + }); + + it('reads a cursor without a usable id as the first page', () => { + expect( + openIdCursor(encodeCursor({ appUid: 'app-1' }), secret), + ).toBeNull(); + expect(openIdCursor(encodeCursor({ id: -3 }), secret)).toBeNull(); + }); + + it('refuses a cursor without a usable id when strict', () => { + expect(() => + openIdCursor(encodeCursor({ id: 'x' }), secret, { + label: 'share cursor', + strict: true, + }), + ).toThrowError('invalid share cursor'); + }); + + it('refuses an unreadable cursor either way', () => { + expect(() => openIdCursor('!!!', secret)).toThrowError(HttpError); + }); + }); + + describe('keysetPage', () => { + const secret = 'keyset-secret'; + const rows = [{ id: 3 }, { id: 5 }, { id: 9 }]; + + it('returns every row and no cursor when nothing follows', () => { + expect(keysetPage(rows, 3, secret)).toEqual({ rows }); + }); + + it('cuts to the limit and points the cursor at the last row kept', () => { + const page = keysetPage(rows, 2, secret); + expect(page.rows).toEqual([{ id: 3 }, { id: 5 }]); + expect(openIdCursor(page.cursor, secret)).toBe(5); + }); + + it('takes a custom position, plain without a secret', () => { + const page = keysetPage( + [{ app: 'a' }, { app: 'b' }], + 1, + undefined, + (last) => ({ appUid: last.app }), + ); + expect(decodeCursor(page.cursor)).toEqual({ appUid: 'a' }); + }); + }); }); diff --git a/src/backend/util/pagination.ts b/src/backend/util/pagination.ts index ccf9675c8..69c4a1757 100644 --- a/src/backend/util/pagination.ts +++ b/src/backend/util/pagination.ts @@ -190,3 +190,52 @@ export const normalizeOffset = ( } return floored; }; + +// -- Keyset pages ----------------------------------------------------- + +/** + * The id a keyset page resumes from, or null on the first page. A cursor that + * opens without a usable id (another listing's, say) throws when `strict`, and + * otherwise reads as the first page. + */ +export const openIdCursor = ( + cursor: string | Record | null | undefined, + secret: string | undefined, + { + label = 'cursor', + strict = false, + }: { label?: string; strict?: boolean } = {}, +): number | null => { + const decoded = openCursor(cursor, secret, label); + if (decoded === undefined) return null; + const id = Number(decoded.id); + if (Number.isSafeInteger(id) && id >= 0) return id; + if (strict) { + throw new HttpError(400, `invalid ${label}`, { + legacyCode: 'bad_request', + }); + } + return null; +}; + +/** + * One page of `rows`, fetched with `LIMIT limit + 1`: the first `limit` rows, + * and a cursor at the last of them when the extra row says another page + * follows. `position` is the cursor payload; it defaults to the row's `id`. + * Without `secret` the cursor is a plain `encodeCursor` one. + */ +export const keysetPage = ( + rows: R[], + limit: number, + secret: string | undefined, + position: (last: R) => Record = (last) => ({ + id: Number((last as { id?: unknown }).id), + }), +): { rows: R[]; cursor?: string } => { + if (rows.length <= limit) return { rows }; + const page = rows.slice(0, limit); + return { + rows: page, + cursor: sealCursor(position(page[page.length - 1]!), secret), + }; +};