diff --git a/src/backend/controllers/fs/FSController.test.ts b/src/backend/controllers/fs/FSController.test.ts index 89db117e1..9e9b292f3 100644 --- a/src/backend/controllers/fs/FSController.test.ts +++ b/src/backend/controllers/fs/FSController.test.ts @@ -1822,6 +1822,44 @@ describe('FSController.readdirEntries recursive', () => { ).toEqual(['l1a', 'l1a/l2a', 'l1a/l2a/l3a', 'l1a/l2a/l3a/l4a', 'l1b']); }); + it('sorts a recursive listing, paging in the same order', async () => { + const { actor, base } = await makeTree(); + // A recursive `name` sort is path order, so descending walks the + // subtree backwards. + const descending = (await readdir(actor, { + path: base, + recursive: true, + depth: 10, + sortBy: 'name', + sortOrder: 'desc', + })) as { items: Array<{ path: string }> }; + expect( + descending.items.map((e) => e.path.slice(base.length + 1)), + ).toEqual(['l1b', 'l1a/l2a/l3a/l4a', 'l1a/l2a/l3a', 'l1a/l2a', 'l1a']); + + const seen: string[] = []; + let cursor: string | null | undefined = null; + do { + const page = (await readdir(actor, { + path: base, + recursive: true, + depth: 10, + sortOrder: 'desc', + limit: 2, + cursor, + })) as { items: Array<{ path: string }>; cursor?: string }; + seen.push(...page.items.map((e) => e.path.slice(base.length + 1))); + cursor = page.cursor; + } while (cursor); + expect(seen).toEqual([ + 'l1b', + 'l1a/l2a/l3a/l4a', + 'l1a/l2a/l3a', + 'l1a/l2a', + 'l1a', + ]); + }); + it('counts the subtree with includeTotal', async () => { const { actor, base } = await makeTree(); const page = (await readdir(actor, { diff --git a/src/backend/controllers/fs/FSController.ts b/src/backend/controllers/fs/FSController.ts index 9f20f1d52..f74ca872b 100644 --- a/src/backend/controllers/fs/FSController.ts +++ b/src/backend/controllers/fs/FSController.ts @@ -1140,8 +1140,8 @@ export class FSController extends PuterController { Object.prototype.hasOwnProperty.call(body, 'cursor') || includeTotal; - // Undocumented: `recursive` lists descendants (prefix scan) up to - // `depth` levels below the target. Always paginated; sorts by path. + // `recursive` lists descendants (prefix scan) up to `depth` levels + // below the target. Always paginated. const recursive = this.#toBoolean(body.recursive) === true; if (this.#isRootPathRef(body)) { @@ -1230,6 +1230,8 @@ export class FSController extends PuterController { ? body.cursor : undefined, maxDepth, + sortBy, + sortOrder, }, ); await this.#attachSuggestedApps(page.entries); diff --git a/src/backend/services/fs/FSService.ts b/src/backend/services/fs/FSService.ts index fbe5bb8a8..236703e1e 100644 --- a/src/backend/services/fs/FSService.ts +++ b/src/backend/services/fs/FSService.ts @@ -3067,12 +3067,19 @@ export class FSService extends PuterService { /** * Cursor-paginated nested listing: descendants of `path` up to `maxDepth` - * levels deep, ordered by path. Owner-scoped by `userId` + path prefix. + * levels deep. Owner-scoped by `userId` + path prefix. Sorts as asked; a + * `name` sort orders by full path so subtrees stay grouped. */ async listDirectoryTreePage( userId: number, path: string, - options: { limit?: number; cursor?: string | null; maxDepth: number }, + options: { + limit?: number; + cursor?: string | null; + maxDepth: number; + sortBy?: 'name' | 'modified' | 'type' | 'size' | null; + sortOrder?: 'asc' | 'desc' | null; + }, ): Promise<{ entries: FSEntry[]; cursor?: string }> { return this.stores.fsEntry.listDescendantsPage(userId, path, options); } diff --git a/src/backend/stores/fs/FSEntryStore.test.ts b/src/backend/stores/fs/FSEntryStore.test.ts index 3c24fe9ca..40e399222 100644 --- a/src/backend/stores/fs/FSEntryStore.test.ts +++ b/src/backend/stores/fs/FSEntryStore.test.ts @@ -24,6 +24,7 @@ import { configContainer } from '../../exports.js'; import { PuterServer } from '../../server.js'; import { setupTestServer } from '../../testUtil.js'; import type { IConfig } from '../../types'; +import { encodeCursor } from '../../util/pagination.js'; import { generateDefaultFsentries } from '../../util/userProvisioning.js'; import type { FSEntry, FSEntryCreateInput } from './FSEntry.js'; import { FSEntryStore } from './FSEntryStore.js'; @@ -1268,6 +1269,92 @@ describe('FSEntryStore listing and pagination', () => { expect(rootPage.statusCode).toBe(400); }); + it('sorts a descendant page by the requested field', async () => { + // Sizes: a=30, b=20, c=10, sub=null (a directory), sub/deep=5. The + // sort spans the whole subtree, so a nested entry sorts among the + // entries above it. + const bySize = await store.listDescendantsPage( + user.userId, + parent.path, + { maxDepth: 5, sortBy: 'size', sortOrder: 'desc' }, + ); + expect(bySize.entries.map((entry) => entry.name)).toEqual([ + 'a.txt', + 'b.txt', + 'c.txt', + 'deep.txt', + 'sub', + ]); + + // A name sort is path order: names alone would interleave depths. + const byName = await store.listDescendantsPage( + user.userId, + parent.path, + { maxDepth: 5, sortBy: 'name' }, + ); + expect(byName.entries.map((entry) => entry.name)).toEqual([ + 'a.txt', + 'b.txt', + 'c.txt', + 'sub', + 'deep.txt', + ]); + }); + + it('pages a sorted descendant listing without dupes or gaps', async () => { + const seen: string[] = []; + let cursor: string | undefined; + do { + const page = await store.listDescendantsPage( + user.userId, + parent.path, + { + maxDepth: 5, + limit: 2, + sortBy: 'size', + sortOrder: 'desc', + cursor, + }, + ); + seen.push(...page.entries.map((entry) => entry.name)); + cursor = page.cursor; + } while (cursor); + expect(seen).toEqual(['a.txt', 'b.txt', 'c.txt', 'deep.txt', 'sub']); + }); + + it('pins the sort to the descendant cursor', async () => { + const first = await store.listDescendantsPage( + user.userId, + parent.path, + { maxDepth: 5, limit: 1, sortBy: 'size' }, + ); + const mismatch = await caught(() => + store.listDescendantsPage(user.userId, parent.path, { + maxDepth: 5, + limit: 1, + sortBy: 'modified', + cursor: first.cursor, + }), + ); + expect(mismatch.statusCode).toBe(400); + expect(mismatch.message).toBe('cursor does not match requested sort'); + + // Cursors minted before the sort was honored carried only a path. + const legacy = await store.listDescendantsPage( + user.userId, + parent.path, + { + maxDepth: 5, + cursor: encodeCursor({ p: `${parent.path}/b.txt` }), + }, + ); + expect(legacy.entries.map((entry) => entry.name)).toEqual([ + 'c.txt', + 'sub', + 'deep.txt', + ]); + }); + it('counts descendants to a depth', async () => { await expect( store.countDescendantsToDepth(user.userId, parent.path, 1), diff --git a/src/backend/stores/fs/FSEntryStore.ts b/src/backend/stores/fs/FSEntryStore.ts index 99d5f75c1..da731e3f8 100644 --- a/src/backend/stores/fs/FSEntryStore.ts +++ b/src/backend/stores/fs/FSEntryStore.ts @@ -52,6 +52,48 @@ import type { ReadEntriesByPathsOptions, } from './types.js'; +// Sort fields a recursive listing accepts, mapped to the SQL expression they +// order by. `name` orders by full path: across depths, bare names interleave +// subtrees, while path keeps every child next to its parent. NULLs drop out of +// keyset comparisons, so nullable columns are coalesced here and in the seek. +const DESCENDANT_SORT_EXPRESSIONS = { + name: 'path', + modified: 'COALESCE(modified, 0)', + size: 'COALESCE(size, -1)', + type: 'is_dir', +} as const; + +type DescendantSortField = keyof typeof DESCENDANT_SORT_EXPRESSIONS; + +// Cursors are opaque to callers but arrive from the wire, so the sort they pin +// is validated rather than trusted. +const toDescendantSortField = (value: unknown): DescendantSortField => { + if ( + typeof value === 'string' && + Object.prototype.hasOwnProperty.call(DESCENDANT_SORT_EXPRESSIONS, value) + ) { + return value as DescendantSortField; + } + throw new HttpError(400, 'unsupported sort field', { + legacyCode: 'bad_request', + }); +}; + +// The value the next page seeks from — must match the sort expression above. +const descendantSortValue = (row: FSEntryRow, sortBy: DescendantSortField) => { + switch (sortBy) { + case 'modified': + return row.modified ?? 0; + case 'size': + return row.size ?? -1; + case 'type': + return row.is_dir; + case 'name': + default: + return row.path; + } +}; + const ENTRY_CACHE_TTL_SECONDS = 60; const BULK_QUERY_CHUNK_SIZE = 200; const DEFAULT_DB_CHUNK_CONCURRENCY = 4; @@ -2733,13 +2775,20 @@ export class FSEntryStore extends PuterStore { /** * Cursor-paginated descendants of a directory, limited to `maxDepth` levels * below the prefix. Prefix + user_id scan (same index as - * `listDescendantsByPath`) with a portable slash-count depth filter. Keyset - * pagination on `path ASC` — path is unique per user, so no id tiebreaker. + * `listDescendantsByPath`) with a portable slash-count depth filter. + * Keyset-paginated on the requested sort; the cursor pins it, so a later + * page can't switch sorts mid-sequence. */ async listDescendantsPage( userId: number, pathPrefix: string, - options: { limit?: number; cursor?: string | null; maxDepth: number }, + options: { + limit?: number; + cursor?: string | null; + maxDepth: number; + sortBy?: DescendantSortField | null; + sortOrder?: 'asc' | 'desc' | null; + }, ): Promise<{ entries: FSEntry[]; cursor?: string }> { const normalizedPrefix = this.#normalizePath(pathPrefix); if (normalizedPrefix === '/') { @@ -2752,20 +2801,66 @@ export class FSEntryStore extends PuterStore { this.#slashCount(normalizedPrefix) + Math.max(1, options.maxDepth); const limit = normalizeLimit(options.limit, { cap: 10_000 }) ?? 1000; - const payload = decodeCursor(options.cursor) as - | { p: string } + const rawPayload = decodeCursor(options.cursor) as + | { v?: unknown; id?: number; s?: string; o?: string; p?: string } | undefined; - const seek = payload ? 'AND path > ?' : ''; - const params: unknown[] = payload - ? [userId, likePattern, maxSlashes, payload.p, limit + 1] - : [userId, likePattern, maxSlashes, limit + 1]; + // Cursors minted before this listing honored the sort carried only the + // last path, always ascending. + const payload = + rawPayload && rawPayload.v === undefined + ? { v: rawPayload.p, s: 'name', o: 'asc' } + : rawPayload; + + const requestedSort = options.sortBy ?? null; + const requestedOrder = options.sortOrder ?? null; + if ( + payload && + ((requestedSort && payload.s !== requestedSort) || + (requestedOrder && payload.o !== requestedOrder)) + ) { + throw new HttpError(400, 'cursor does not match requested sort', { + legacyCode: 'bad_request', + }); + } + + const sortBy = toDescendantSortField( + requestedSort ?? payload?.s ?? 'name', + ); + const sortOrder = + (requestedOrder ?? payload?.o) === 'desc' ? 'desc' : 'asc'; + const sortExpr = DESCENDANT_SORT_EXPRESSIONS[sortBy]; + const dir = sortOrder === 'desc' ? 'DESC' : 'ASC'; + const cmp = sortOrder === 'desc' ? '<' : '>'; + + // A name sort orders on path, which is unique within one user's tree + // and so is already a total order; the other fields repeat across the + // subtree and fall back on `id`. + const tiebreak = sortBy !== 'name'; + const seekId = Number(payload?.id); + const seek = !payload + ? '' + : tiebreak && Number.isFinite(seekId) + ? `AND (${sortExpr} ${cmp} ? OR (${sortExpr} = ? AND id ${cmp} ?))` + : `AND ${sortExpr} ${cmp} ?`; + const seekParams = !payload + ? [] + : tiebreak && Number.isFinite(seekId) + ? [payload.v, payload.v, seekId] + : [payload.v]; + const params: unknown[] = [ + userId, + likePattern, + maxSlashes, + ...seekParams, + limit + 1, + ]; const rows = (await this.clients.db.read( `SELECT ${this.#selectFsentriesColumns()} FROM fsentries WHERE user_id = ? AND path LIKE ? ESCAPE '!' AND (LENGTH(path) - LENGTH(REPLACE(path, '/', ''))) <= ? ${seek} - ORDER BY path ASC + ORDER BY ${sortExpr} ${dir}${tiebreak ? `, id ${dir}` : ''} LIMIT ?`, params, )) as unknown as FSEntryRow[]; @@ -2777,7 +2872,12 @@ export class FSEntryStore extends PuterStore { let cursor: string | undefined; if (hasMore) { const last = pageRows[pageRows.length - 1]!; - cursor = encodeCursor({ p: last.path }); + cursor = encodeCursor({ + v: descendantSortValue(last, sortBy), + ...(tiebreak ? { id: Number(last.id) } : {}), + s: sortBy, + o: sortOrder, + }); } return { entries, ...(cursor ? { cursor } : {}) }; diff --git a/src/docs/src/FS/readdir.md b/src/docs/src/FS/readdir.md index 1ebadf595..6e1d737db 100755 --- a/src/docs/src/FS/readdir.md +++ b/src/docs/src/FS/readdir.md @@ -29,7 +29,7 @@ An object with the following properties: - `uid` (String) (optional) - The UID of the directory to read. - `limit` (Number) (optional) - Maximum number of entries to return. - `offset` (Number) (optional) - Skips the given number of entries. Prefer `cursor` for paging through large directories. -- `sortBy` (String) (optional) - Sort field: `name`, `modified`, `type`, or `size`. Default is `name`. +- `sortBy` (String) (optional) - Sort field: `name`, `modified`, `type`, or `size`. Default is `name`. With `recursive`, sorting by `name` orders by full path, so each directory's contents stay together; the other fields sort across the whole subtree. - `sortOrder` (String) (optional) - `asc` or `desc`. Default is `asc`. - `recursive` (Boolean) (optional) - If `true`, the contents of subdirectories are listed too. Defaults to `false`. - `depth` (Number) (optional) - How many levels to descend when `recursive` is `true`. Defaults to unlimited. diff --git a/src/puter-js/src/modules/FileSystem/types.js b/src/puter-js/src/modules/FileSystem/types.js index 765d1b04f..b6526e1b4 100644 --- a/src/puter-js/src/modules/FileSystem/types.js +++ b/src/puter-js/src/modules/FileSystem/types.js @@ -120,7 +120,8 @@ * large directories. * @property {string | null} [cursor] Opaque continuation cursor from a previous page. * @property {boolean} [includeTotal] Include a `total` count of every entry across all pages. - * @property {'name' | 'modified' | 'type' | 'size'} [sortBy] Sort field. Default is `name`. + * @property {'name' | 'modified' | 'type' | 'size'} [sortBy] Sort field. Default is `name`. With + * `recursive`, a `name` sort orders by full path. * @property {'asc' | 'desc'} [sortOrder] Sort direction. Default is `asc`. * @property {boolean} [recursive] Whether to also list the contents of subdirectories. Defaults to * `false`.