diff --git a/src/api/__tests__/files-trash.test.ts b/src/api/__tests__/files-trash.test.ts index 39056804..c3830df4 100644 --- a/src/api/__tests__/files-trash.test.ts +++ b/src/api/__tests__/files-trash.test.ts @@ -3,6 +3,8 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; vi.mock('../jmap-client', () => ({ jmapClient: { accountId: 'acc-1', + serverUrl: 'https://mail.example.org', + username: 'me@example.org', request: vi.fn(), hasCapability: vi.fn(() => true), hasAccountCapability: vi.fn(() => false), @@ -19,7 +21,7 @@ vi.mock('../jmap-client', () => ({ import { jmapClient } from '../jmap-client'; import type { FileNode } from '../types'; -import { emptyTrash, moveToTrash, purgeFromTrash, restoreFromTrash } from '../files-trash'; +import { emptyTrash, moveToTrash, purgeFromTrash, restoreFromTrash, undoMoveToTrash } from '../files-trash'; import { trashedItems } from '../../lib/files-trash'; // A Stalwart-like FileNode store for the user's own account: destroy without @@ -83,7 +85,15 @@ function serve(method: string, args: Record) { const listed = (): FileNode[] => store.map((n) => ({ ...n, type: n.type ?? '' }) as FileNode); const byId = (id: string) => store.find((n) => n.id === id); +// Who is signed in: another login, or another files account of it. +const client = jmapClient as unknown as { username: string; currentSession: { primaryAccounts: Record } }; +function signIn(username: string, filesAccount = 'acc-1') { + client.username = username; + client.currentSession.primaryAccounts['urn:ietf:params:jmap:filenode'] = filesAccount; +} + beforeEach(() => { + signIn('me@example.org'); nextId = 1; calls = []; refuseUpdate = new Set(); @@ -258,6 +268,63 @@ describe('restoreFromTrash', () => { }); }); +describe('undoMoveToTrash', () => { + it('puts the items back while the same drive is signed in', async () => { + const before = listed(); + const moved = await moveToTrash(before, before.filter((n) => n.id === 'a'), 1730112000007); + expect((await undoMoveToTrash(moved))?.done).toHaveLength(1); + expect(byId('a')).toMatchObject({ parentId: 'docs' }); + }); + + it('sends nothing once another login is signed in, whose ids may be the same', async () => { + const before = listed(); + const moved = await moveToTrash(before, before.filter((n) => n.id === 'a'), 1730112000007); + signIn('other@example.org'); + const sent = calls.length; + const requests = (jmapClient.request as ReturnType).mock.calls.length; + expect(await undoMoveToTrash(moved)).toBeNull(); + expect(calls.length).toBe(sent); + expect((jmapClient.request as ReturnType).mock.calls.length).toBe(requests); + expect(byId(byId('a')!.parentId!)!.name).toBe('1730112000007~docs'); + }); + + it('sends nothing once the login reads another files account', async () => { + const before = listed(); + const moved = await moveToTrash(before, before.filter((n) => n.id === 'a'), 1730112000007); + signIn('me@example.org', 'acc-3'); + const sent = calls.length; + expect(await undoMoveToTrash(moved)).toBeNull(); + expect(calls.length).toBe(sent); + }); + + it('stops before the restore when the account changes while the drive is read', async () => { + const before = listed(); + const moved = await moveToTrash(before, before.filter((n) => n.id === 'a'), 1730112000007); + const real = (jmapClient.request as ReturnType).getMockImplementation() as (c: any[]) => Promise; + (jmapClient.request as ReturnType).mockImplementation(async (methodCalls: any[]) => { + const out = await real(methodCalls); + if (methodCalls[0][0] === 'FileNode/get') signIn('other@example.org'); + return out; + }); + const sent = calls.length; + expect(await undoMoveToTrash(moved)).toBeNull(); + expect(calls.length).toBe(sent); + }); + + it('stops a delete whose account changes before its items are moved', async () => { + const before = listed(); + const real = (jmapClient.request as ReturnType).getMockImplementation() as (c: any[]) => Promise; + (jmapClient.request as ReturnType).mockImplementation(async (methodCalls: any[]) => { + const out = await real(methodCalls); + if (methodCalls[0][1].create) signIn('other@example.org'); + return out; + }); + await expect(moveToTrash(before, before.filter((n) => n.id === 'a'), 1730112000007)).rejects.toThrow('Another account'); + expect(calls.some((c) => c.update)).toBe(false); + expect(byId('a')!.parentId).toBe('docs'); + }); +}); + describe('purgeFromTrash and emptyTrash', () => { it('deletes items for good with what is inside, and their emptied trash folder', async () => { let nodes = listed(); diff --git a/src/api/files-trash.ts b/src/api/files-trash.ts index a51af3da..5c651c6f 100644 --- a/src/api/files-trash.ts +++ b/src/api/files-trash.ts @@ -1,6 +1,8 @@ import type { FileNode } from './types'; +import { jmapClient } from './jmap-client'; import { - createFolder, deleteFileNodes, destroyEmptyFolders, getAllFileNodes, isCrossAccountId, updateFileNodes, + createFolder, deleteFileNodes, destroyEmptyFolders, filesAccountId, getAllFileNodes, isCrossAccountId, + updateFileNodes, } from './files'; import { JMAPSetError } from './jmap-result'; import { getUniqueName } from '../lib/filenode-name'; @@ -22,6 +24,44 @@ export interface TrashWriteResult { refused: JMAPSetError[]; } +/** + * The drive a write went to: the login (server and user) and its files + * account. Every write resolves the account when it is sent, and node ids + * repeat across accounts and servers, so a write prepared for one drive + * must not go out once another is signed in. + */ +export interface TrashDrive { + server: string | null; + username: string | null; + accountId: string; +} + +export function currentTrashDrive(): TrashDrive { + return { server: jmapClient.serverUrl, username: jmapClient.username, accountId: filesAccountId() }; +} + +function isCurrentDrive(drive: TrashDrive): boolean { + const now = currentTrashDrive(); + return now.server === drive.server && now.username === drive.username && now.accountId === drive.accountId; +} + +/** Thrown before a write would go to another drive than the one it was prepared for. */ +export class TrashDriveChangedError extends Error { + constructor() { + super('Another account is signed in'); + this.name = 'TrashDriveChangedError'; + } +} + +function assertDrive(drive: TrashDrive | undefined): void { + if (drive && !isCurrentDrive(drive)) throw new TrashDriveChangedError(); +} + +/** A delete into the trash, with the drive it went to (for its undo). */ +export interface TrashMoveResult extends TrashWriteResult { + drive: TrashDrive; +} + /** The trash folder, created on the first delete. */ async function ensureTrashFolder(nodes: FileNode[]): Promise { const known = findTrashFolder(nodes); @@ -47,14 +87,15 @@ async function removeEmptyBatches(nodes: FileNode[], batchIds: Iterable, * Move nodes of the user's own drive into the trash: one trash folder per * folder they came from, named after the time and that folder, like the * webmail does. A node of a shared drive can't go there; delete it instead. - * Throws when nothing went. + * Stops once another account is signed in. Throws when nothing went. */ export async function moveToTrash( nodes: FileNode[], targets: FileNode[], deletedAt: number = Date.now(), -): Promise { - const result: TrashWriteResult = { done: [], refused: [] }; +): Promise { + const drive = currentTrashDrive(); + const result: TrashMoveResult = { done: [], refused: [], drive }; if (targets.length === 0) return result; if (targets.some((n) => n.isShared || isCrossAccountId(n.id))) { throw new JMAPSetError('forbidden', 'Files of a shared drive cannot be moved to the trash'); @@ -74,12 +115,15 @@ export async function moveToTrash( // The trash on screen may be gone (emptied on another device): read // the drive again and try once more. trash = await ensureTrashFolder(await getAllFileNodes()); + assertDrive(drive); return createFolder(name, trash.id); } }; for (const [parentId, group] of groups) { + if (!isCurrentDrive(drive)) break; const batch = await createBatch(parentId); + if (!isCurrentDrive(drive)) break; const update = await updateFileNodes(Object.fromEntries(group.map((n) => [n.id, { parentId: batch.id }]))); const moved = new Set(update.updated); result.refused.push(...Object.values(update.notUpdated)); @@ -92,6 +136,7 @@ export async function moveToTrash( } } if (result.done.length === 0) { + assertDrive(drive); throw result.refused[0] ?? new JMAPSetError('setError', 'Failed to move to the trash'); } return result; @@ -105,12 +150,16 @@ export async function moveToTrash( * The drive is read again first, and that is what counts: the listing on * screen (for an undo, the one from before the delete) misses a folder * trashed since and a name taken since. Items no longer in the trash are left - * where they are. Throws when nothing of what is still there went. + * where they are. Throws when nothing of what is still there went, and + * TrashDriveChangedError, with nothing sent, once `drive` is no longer the + * one signed in. */ -export async function restoreFromTrash(items: TrashedItem[]): Promise { +export async function restoreFromTrash(items: TrashedItem[], drive?: TrashDrive): Promise { const result: TrashWriteResult = { done: [], refused: [] }; if (items.length === 0) return result; + assertDrive(drive); const nodes = await getAllFileNodes(); + assertDrive(drive); const trash = findTrashFolder(nodes); const hidden = trashSubtreeIds(nodes, trash?.id); // The items as the trash holds them now: their name and trash folder may @@ -146,6 +195,7 @@ export async function restoreFromTrash(items: TrashedItem[]): Promise (i.batchId ? [i.batchId] : [])), restored); + if (!drive || isCurrentDrive(drive)) { + await removeEmptyBatches(nodes, result.done.flatMap((i) => (i.batchId ? [i.batchId] : [])), restored); + } return result; } +/** + * Undo of a delete into the trash (see restoreFromTrash). Null, and nothing + * sent, once another account or server is signed in than the one the delete + * went to: the same ids name other files there. + */ +export async function undoMoveToTrash(moved: Pick): Promise { + try { + return await restoreFromTrash(moved.done, moved.drive); + } catch (err) { + if (err instanceof TrashDriveChangedError) return null; + throw err; + } +} + /** * Delete items of the trash for good, with what is inside a deleted folder. * The drive is read again first and only what is still in the trash goes: a diff --git a/src/screens/FilesScreen.tsx b/src/screens/FilesScreen.tsx index 8edef477..de35b30b 100644 --- a/src/screens/FilesScreen.tsx +++ b/src/screens/FilesScreen.tsx @@ -51,7 +51,9 @@ import { getFileNameRules, getFileNodeDownloadUrl, getMaxSizeUpload, isFolder, isSymlink, listFolder, moveFileNode, renameFileNode, supportsSharing, uploadFileBatch, } from '../api/files'; -import { emptyTrash, moveToTrash, purgeFromTrash, restoreFromTrash } from '../api/files-trash'; +import { + emptyTrash, moveToTrash, purgeFromTrash, restoreFromTrash, undoMoveToTrash, type TrashMoveResult, +} from '../api/files-trash'; import { trashOriginPath, trashedItems, withoutTrash, type TrashedItem } from '../lib/files-trash'; import { toast } from '../stores/toast-store'; import { jmapClient } from '../api/jmap-client'; @@ -317,12 +319,16 @@ export default function FilesScreen() { // (another port, or Stalwart reinstalled there), which the account id does // not tell. Every new session reads the drive again. const driveRef = useRef(undefined); + const undoToastRef = useRef(null); useEffect(() => { const { action, scope } = driveChange(driveRef.current, activeAccountId, session); driveRef.current = scope; if (action === 'keep') return; if (action === 'reset') { uploadAbortRef.current?.abort(); + // The last delete's Undo belongs to the previous drive. + if (undoToastRef.current) toast.dismiss(undoToastRef.current); + undoToastRef.current = null; setAllNodes([]); setPath([]); setInTrash(false); @@ -745,11 +751,14 @@ export default function FilesScreen() { }, [uploading, currentParentId, allNodes, loadFiles, t]); // Undo of a delete: the items go back from the trash the way Restore puts - // them back, judged against the drive as it is by then. - const undoTrash = useCallback(async (items: TrashedItem[]) => { + // them back, judged against the drive as it is by then. Nothing happens + // once another account is signed in (see undoMoveToTrash). + const undoTrash = useCallback(async (moved: TrashMoveResult) => { + undoToastRef.current = null; try { - const { done } = await restoreFromTrash(items); - if (done.length > 0) toast.success(t('files.undo_success', 'Action undone')); + const result = await undoMoveToTrash(moved); + if (!result) return; + if (result.done.length > 0) toast.success(t('files.undo_success', 'Action undone')); } catch (e) { toast.error(t('files.undo_error', 'Failed to undo'), fileErrorMessage(e, t)); } @@ -763,14 +772,15 @@ export default function FilesScreen() { clearSelection(); const listing = allNodes; try { - const { done, refused } = await moveToTrash(listing, rows); + const moved = await moveToTrash(listing, rows); + const { done, refused } = moved; await loadFiles('refresh'); if (refused.length > 0) { Alert.alert(t('files.delete_error', 'Failed to delete'), fileErrorMessage(refused[0], t)); } - toast.success( + undoToastRef.current = toast.success( t('files.moved_to_trash', '{count, plural, one {Moved to trash} other {# items moved to trash}}', { count: done.length }), - { action: { label: t('files.undo', 'Undo'), onPress: () => void undoTrash(done) } }, + { action: { label: t('files.undo', 'Undo'), onPress: () => void undoTrash(moved) } }, ); } catch (e) { Alert.alert(t('files.delete_error', 'Failed to delete'), fileErrorMessage(e, t));