diff --git a/src/api/files-trash.ts b/src/api/files-trash.ts index 1c03f28b..9a4028ae 100644 --- a/src/api/files-trash.ts +++ b/src/api/files-trash.ts @@ -7,7 +7,7 @@ import { import { JMAPSetError } from './jmap-result'; import { getUniqueName } from '../lib/filenode-name'; import { - TRASH_FOLDER_NAME, findTrashFolder, restoreTarget, trashBatchName, trashSubtreeIds, trashedItems, + TRASH_FOLDER_NAME, findTrashFolder, restoreTarget, trashBatchName, trashPlaces, trashSubtreeIds, trashedItems, type TrashedItem, } from '../lib/files-trash'; @@ -185,7 +185,8 @@ export async function restoreFromTrash(items: TrashedItem[], drive?: TrashDrive) const nodes = await getAllFileNodes(); assertDrive(drive); const trash = findTrashFolder(nodes); - const hidden = trashSubtreeIds(nodes, trash?.id); + const places = trashPlaces(nodes); + const { hidden } = places; // The items as the trash holds them now: their name and trash folder may // have changed on another device. const current = new Map(trashedItems(nodes, trash).map((item) => [item.node.id, item])); @@ -212,7 +213,7 @@ export async function restoreFromTrash(items: TrashedItem[], drive?: TrashDrive) const updates: Record>> = {}; const targets = new Map(); for (const item of pending) { - const target = restoreTarget(item, nodes); + const target = restoreTarget(item, places); const used = namesIn(target); const name = getUniqueName(item.node.name, used); used.add(name); diff --git a/src/lib/__tests__/files-trash.test.ts b/src/lib/__tests__/files-trash.test.ts index 2afbf50e..b49c86ba 100644 --- a/src/lib/__tests__/files-trash.test.ts +++ b/src/lib/__tests__/files-trash.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect } from 'vitest'; import type { FileNode } from '../../api/types'; import { findTrashFolder, parseTrashBatchName, restoreTarget, takesTrashName, trashBatchName, trashOriginPath, - trashSubtreeIds, trashedItems, withoutTrash, + trashPlaces, trashSubtreeIds, trashedItems, withoutTrash, } from '../files-trash'; // The webmail's trash convention (its lib/files-trash.ts), read the same way here. @@ -107,4 +107,23 @@ describe('trashOriginPath', () => { expect(trashOriginPath({ fromParentId: null }, drive)).toBe('/'); expect(trashOriginPath({ fromParentId: 'old' }, drive)).toBeNull(); }); + + it('reads the listing once for all the rows of the trash, not once per row', () => { + // A listing that counts how often it is gone through. + let reads = 0; + const counted = new Proxy([...drive], { + get(target, key, receiver) { + if (key === Symbol.iterator || key === 'find' || key === 'filter' || key === 'map' || key === 'some') reads++; + return Reflect.get(target, key, receiver); + }, + }); + const places = trashPlaces(counted); + const before = reads; + const rows = trashedItems(drive); + const paths = rows.map((item) => trashOriginPath(item, places)); + expect(paths).toEqual(['/Documents/Taxes', '/Documents/Taxes', '/', '/']); + expect(reads).toBe(before); + expect(rows.map((item) => restoreTarget(item, places))).toEqual(['tax', 'tax', null, null]); + expect(reads).toBe(before); + }); }); diff --git a/src/lib/files-trash.ts b/src/lib/files-trash.ts index 43046bbc..dad0ee40 100644 --- a/src/lib/files-trash.ts +++ b/src/lib/files-trash.ts @@ -55,14 +55,22 @@ export function parseTrashBatchName(name: string): { deletedAt: number; parentId export function trashSubtreeIds(nodes: FileNode[], trashId: string | undefined): Set { const ids = new Set(); if (!trashId) return ids; + // One pass over the drive, then down the tree: a pass per level was slow + // for a deep trash in a large drive. + const children = new Map(); + for (const n of nodes) { + if (!n.parentId) continue; + const list = children.get(n.parentId); + if (list) list.push(n.id); + else children.set(n.parentId, [n.id]); + } + const queue = [trashId]; ids.add(trashId); - let grew = true; - while (grew) { - grew = false; - for (const n of nodes) { - if (n.parentId && ids.has(n.parentId) && !ids.has(n.id)) { - ids.add(n.id); - grew = true; + for (let i = 0; i < queue.length; i++) { + for (const child of children.get(queue[i]) ?? []) { + if (!ids.has(child)) { + ids.add(child); + queue.push(child); } } } @@ -102,26 +110,46 @@ export function trashedItems(nodes: FileNode[], trash: FileNode | undefined = fi return items; } +/** + * What restoreTarget and trashOriginPath look up in a listing, worked out + * once for it: every row of the trash asks, and each time anew it cost a + * walk of the whole drive per row. + */ +export interface TrashPlaces { + byId: Map; + /** The trash and everything in it. */ + hidden: Set; +} + +export function trashPlaces(nodes: FileNode[]): TrashPlaces { + return { byId: new Map(nodes.map((n) => [n.id, n])), hidden: trashSubtreeIds(nodes, findTrashFolder(nodes)?.id) }; +} + +function placesOf(from: FileNode[] | TrashPlaces): TrashPlaces { + return Array.isArray(from) ? trashPlaces(from) : from; +} + /** * Where a restored item goes: the folder it came from while that is still a * folder of the user's own drive outside the trash, the root otherwise. */ -export function restoreTarget(item: Pick, nodes: FileNode[]): string | null { - const from = item.fromParentId; - if (from === null) return null; - const hidden = trashSubtreeIds(nodes, findTrashFolder(nodes)?.id); - const folder = nodes.find((n) => n.id === from); - return folder && isFolder(folder) && !folder.isShared && !hidden.has(folder.id) ? from : null; +export function restoreTarget(item: Pick, from: FileNode[] | TrashPlaces): string | null { + const parentId = item.fromParentId; + if (parentId === null) return null; + const { byId, hidden } = placesOf(from); + const folder = byId.get(parentId); + return folder && isFolder(folder) && !folder.isShared && !hidden.has(folder.id) ? parentId : null; } /** * The path of the folder a deleted item came from ("/Documents/Taxes", "/" * for the root); null when that folder is gone. */ -export function trashOriginPath(item: Pick, nodes: FileNode[]): string | null { +export function trashOriginPath(item: Pick, from: FileNode[] | TrashPlaces): string | null { if (item.fromParentId === null) return '/'; - if (restoreTarget(item, nodes) === null) return null; - const byId = new Map(nodes.map((n) => [n.id, n])); + const places = placesOf(from); + if (restoreTarget(item, places) === null) return null; + const { byId } = places; const names: string[] = []; const seen = new Set(); for (let cur = byId.get(item.fromParentId); cur && !seen.has(cur.id); cur = cur.parentId ? byId.get(cur.parentId) : undefined) { diff --git a/src/screens/FilesScreen.tsx b/src/screens/FilesScreen.tsx index 68df05bb..f3e764cd 100644 --- a/src/screens/FilesScreen.tsx +++ b/src/screens/FilesScreen.tsx @@ -56,7 +56,7 @@ import { type TrashMoveResult, } from '../api/files-trash'; import { - TRASH_FOLDER_NAME, takesTrashName, trashOriginPath, trashedItems, withoutTrash, type TrashedItem, + TRASH_FOLDER_NAME, takesTrashName, trashOriginPath, trashPlaces, trashedItems, withoutTrash, type TrashedItem, } from '../lib/files-trash'; import { toast } from '../stores/toast-store'; import { jmapClient } from '../api/jmap-client'; @@ -281,6 +281,8 @@ export default function FilesScreen() { // but the trash's own, hidden files shown or not. const driveNodes = useMemo(() => withoutTrash(allNodes), [allNodes]); const trashItems = useMemo(() => trashedItems(allNodes), [allNodes]); + // Looked up by every row of the trash; worked out once per listing. + const trashPlacesNow = useMemo(() => trashPlaces(allNodes), [allNodes]); // Inside a folder another account shared with us, new folders and uploads // go to that account, and only where its rights allow them. Nothing is // added to the trash other than by deleting. @@ -1076,7 +1078,7 @@ export default function FilesScreen() { // Where a deleted item came from, on its row in the trash. const trashOrigin = (item: TrashedItem): string | null => { - const from = trashOriginPath(item, allNodes); + const from = trashOriginPath(item, trashPlacesNow); if (from === null) return null; return t('files.deleted_from', 'Deleted from {folder}', { folder: from === '/' ? t('files.my_files', 'My files') : from,