From be04fcb1795dbca284a5b93469bf2d52f386513f Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 29 Oct 2026 02:53:53 +0100 Subject: [PATCH] fix: delete permanently only what is still in the trash, and read the drive again on returning to Files --- src/api/__tests__/files-trash.test.ts | 41 +++++++++-- src/api/__tests__/live-stalwart-files.test.ts | 6 +- src/api/files-trash.ts | 33 ++++++--- src/screens/FilesScreen.tsx | 73 +++++++++++++++---- 4 files changed, 118 insertions(+), 35 deletions(-) diff --git a/src/api/__tests__/files-trash.test.ts b/src/api/__tests__/files-trash.test.ts index 6cdda50f..9b6ff70f 100644 --- a/src/api/__tests__/files-trash.test.ts +++ b/src/api/__tests__/files-trash.test.ts @@ -218,23 +218,48 @@ describe('purgeFromTrash and emptyTrash', () => { let nodes = listed(); await moveToTrash(nodes, nodes.filter((n) => n.id === 'docs'), 1730112000007); nodes = listed(); - await purgeFromTrash(nodes, trashedItems(nodes)); + expect(await purgeFromTrash(trashedItems(nodes))).toBe(1); expect(store.map((n) => n.id).sort()).toEqual(['n1', 'top']); expect(byId('n1')!.name).toBe('.Trash'); }); - it('empties the trash, everything in it included', async () => { + it('leaves a row on screen alone that was restored elsewhere meanwhile', async () => { + let nodes = listed(); + await moveToTrash(nodes, nodes.filter((n) => n.id === 'docs'), 1730112000007); + nodes = listed(); + const rows = trashedItems(nodes); + // Another device puts Docs back before the Delete permanently here. + byId('docs')!.parentId = null; + expect(await purgeFromTrash(rows)).toBe(0); + expect(store.map((n) => n.id).sort()).toEqual(['a', 'b', 'docs', 'n1', 'n2', 'top']); + expect(calls.some((c) => (c.destroy as string[] | undefined)?.includes('docs'))).toBe(false); + }); + + it('deletes only the rows still in the trash, and their trash folder as it is now', async () => { let nodes = listed(); await moveToTrash(nodes, nodes.filter((n) => n.parentId === 'docs'), 1730112000007); nodes = listed(); - await emptyTrash(nodes); + const rows = trashedItems(nodes); + byId('b')!.parentId = 'docs'; + expect(await purgeFromTrash(rows)).toBe(1); + expect(byId('a')).toBeUndefined(); + expect(byId('b')).toMatchObject({ parentId: 'docs' }); + expect(store.some((n) => n.name === '1730112000007~docs')).toBe(false); + }); + + it('empties the trash, everything in it included', async () => { + const nodes = listed(); + await moveToTrash(nodes, nodes.filter((n) => n.parentId === 'docs'), 1730112000007); + await emptyTrash(); expect(store.map((n) => n.id).sort()).toEqual(['docs', 'top']); }); - it('empties a trash made after the listing on screen', async () => { - const stale = listed(); - await moveToTrash(stale, stale.filter((n) => n.id === 'top'), 1730112000007); - await emptyTrash(stale); - expect(store.map((n) => n.id).sort()).toEqual(['a', 'b', 'docs']); + it('empties the trash as it is now, not a folder that was the trash on screen', async () => { + store.push({ id: 'T', parentId: null, name: '.Trash', blobId: null }, { id: 'old', parentId: 'T', name: 'old.txt', blobId: 'bo' }); + // Renamed elsewhere (WebDAV), and a new trash made since. + byId('T')!.name = 'Kept'; + store.push({ id: 'T2', parentId: null, name: '.Trash', blobId: null }, { id: 'new', parentId: 'T2', name: 'new.txt', blobId: 'bn' }); + await emptyTrash(); + expect(store.map((n) => n.id).sort()).toEqual(['T', 'a', 'b', 'docs', 'old', 'top']); }); }); diff --git a/src/api/__tests__/live-stalwart-files.test.ts b/src/api/__tests__/live-stalwart-files.test.ts index 6af6e804..85dd47b9 100644 --- a/src/api/__tests__/live-stalwart-files.test.ts +++ b/src/api/__tests__/live-stalwart-files.test.ts @@ -362,12 +362,12 @@ live('live Stalwart files trash', () => { list = await own(); expect(list.some((n) => n.id === batchId)).toBe(true); - await purgeFromTrash(list, trashedItems(list).filter((i) => i.node.id === x.id)); + await purgeFromTrash(trashedItems(list).filter((i) => i.node.id === x.id)); list = await own(); expect(list.some((n) => n.id === x.id)).toBe(false); expect(list.some((n) => n.id === batchId)).toBe(true); - await purgeFromTrash(list, trashedItems(list).filter((i) => i.node.id === y.id)); + await purgeFromTrash(trashedItems(list).filter((i) => i.node.id === y.id)); list = await own(); expect(list.some((n) => [x.id, y.id, batchId].includes(n.id))).toBe(false); }, 90_000); @@ -378,7 +378,7 @@ live('live Stalwart files trash', () => { const file = await upload('empty-me.txt', scratch); const list = await own(); await moveToTrash(list, [list.find((n) => n.id === file.id)!]); - await emptyTrash(await own()); + await emptyTrash(); const after = await own(); expect(findTrashFolder(after)).toBeUndefined(); expect(after.some((n) => n.id === file.id)).toBe(false); diff --git a/src/api/files-trash.ts b/src/api/files-trash.ts index 320fad7f..f7db1464 100644 --- a/src/api/files-trash.ts +++ b/src/api/files-trash.ts @@ -5,7 +5,8 @@ import { import { JMAPSetError } from './jmap-result'; import { getUniqueName } from '../lib/filenode-name'; import { - TRASH_FOLDER_NAME, findTrashFolder, restoreTarget, trashBatchName, trashSubtreeIds, type TrashedItem, + TRASH_FOLDER_NAME, findTrashFolder, restoreTarget, trashBatchName, trashSubtreeIds, trashedItems, + type TrashedItem, } from '../lib/files-trash'; /* @@ -148,21 +149,35 @@ export async function restoreFromTrash(nodes: FileNode[], items: TrashedItem[]): return result; } -/** Delete items of the trash for good, with what is inside a deleted folder. */ -export async function purgeFromTrash(nodes: FileNode[], items: TrashedItem[]): Promise { - if (items.length === 0) return; - const ids = new Set(items.map((item) => item.node.id)); +/** + * 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 + * row on screen may have been restored or moved out on another device since, + * and the destroy takes a folder's whole content wherever it sits. Returns + * how many items went. + */ +export async function purgeFromTrash(items: TrashedItem[]): Promise { + if (items.length === 0) return 0; + const nodes = await getAllFileNodes(); + const trash = findTrashFolder(nodes); + const inTrash = trashSubtreeIds(nodes, trash?.id); + const ids = new Set(items.map((item) => item.node.id).filter((id) => id !== trash?.id && inTrash.has(id))); + if (ids.size === 0) return 0; + // The trash folders the items sit in now, which need not be the ones on screen. + const batchIds = trashedItems(nodes, trash).flatMap((i) => (i.batchId && ids.has(i.node.id) ? [i.batchId] : [])); try { await deleteFileNodes([...ids]); } finally { // Also after a partial refusal: the server keeps a trash folder that still holds anything. - await removeEmptyBatches(nodes, items.flatMap((i) => (i.batchId ? [i.batchId] : [])), ids); + await removeEmptyBatches(nodes, batchIds, ids); } + return ids.size; } /** Delete everything in the trash for good. The next delete makes a new trash folder. */ -export async function emptyTrash(nodes: FileNode[]): Promise { - // Not in the listing on screen: another client may have made one since. - const trash = findTrashFolder(nodes) ?? findTrashFolder(await getAllFileNodes()); +export async function emptyTrash(): Promise { + // Read again rather than trust the listing on screen: another client may + // have made the trash since, or put another folder in its place. + const trash = findTrashFolder(await getAllFileNodes()); if (trash) await deleteFileNodes([trash.id]); } diff --git a/src/screens/FilesScreen.tsx b/src/screens/FilesScreen.tsx index 1b738ce9..e671c927 100644 --- a/src/screens/FilesScreen.tsx +++ b/src/screens/FilesScreen.tsx @@ -2,6 +2,7 @@ import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { ActivityIndicator, Alert, + AppState, FlatList, Image, Modal, @@ -22,6 +23,7 @@ import { MoreVertical, Pencil, Share2, Trash2, Upload, Users, X, Download, Search, ArrowUpDown, ArrowUp, ArrowDown, FolderInput, Copy, Check, Eye, ExternalLink, FileSymlink, RotateCcw, } from '../components/icons'; +import { useFocusEffect } from '@react-navigation/native'; import { File, Paths } from 'expo-file-system'; import * as Sharing from 'expo-sharing'; import JSZip from 'jszip'; @@ -277,25 +279,36 @@ export default function FilesScreen() { const headerName = inTrash ? trashLabel : path.length === 0 ? rootLabel : path[path.length - 1].name; const sharingEnabled = supportsSharing(); + // Only the latest read is shown: one started before an account switch or a + // write may finish after the next one. + const loadSeqRef = useRef(0); const loadFiles = useCallback( - async (mode: 'initial' | 'refresh' = 'initial') => { + async (mode: 'initial' | 'refresh' | 'quiet' = 'initial') => { + const seq = ++loadSeqRef.current; if (mode === 'refresh') setRefreshing(true); - else setLoading(true); - setError(null); + else if (mode === 'initial') setLoading(true); + if (mode !== 'quiet') setError(null); try { // One fetch across all accessible accounts: own files plus nodes other // principals shared with us (tagged isShared, ids namespaced). const nodes = await getAllFileNodesAcrossAccounts(); - setAllNodes(nodes); + if (seq === loadSeqRef.current) setAllNodes(nodes); } catch (e) { - setError(e instanceof Error ? e.message : t('files.download_error', 'Failed to load files')); + // A quiet read keeps what is on screen. + if (mode !== 'quiet' && seq === loadSeqRef.current) { + setError(e instanceof Error ? e.message : t('files.download_error', 'Failed to load files')); + } } finally { - setLoading(false); - setRefreshing(false); + if (seq === loadSeqRef.current) { + setLoading(false); + setRefreshing(false); + } } }, [t], ); + const loadFilesRef = useRef(loadFiles); + loadFilesRef.current = loadFiles; // Load on mount and reset the whole drive whenever the active account, or // the server behind it, changes: the node cache, folder stack and @@ -325,6 +338,32 @@ export default function FilesScreen() { void loadFiles(action === 'reload' ? 'refresh' : 'initial'); }, [activeAccountId, session, loadFiles]); + // Read the drive again when the tab comes back into view or the app to the + // foreground: another device may have restored, moved or emptied what the + // rows on screen show. The first focus is the mount, which reads already. + const focusedRef = useRef(false); + const seenFocusRef = useRef(false); + useFocusEffect( + useCallback(() => { + focusedRef.current = true; + if (seenFocusRef.current && driveRef.current?.account) void loadFilesRef.current('quiet'); + seenFocusRef.current = true; + return () => { + focusedRef.current = false; + }; + }, []), + ); + useEffect(() => { + let last = AppState.currentState; + const subscription = AppState.addEventListener('change', (next) => { + if (next === 'active' && last !== 'active' && focusedRef.current && driveRef.current?.account) { + void loadFilesRef.current('quiet'); + } + last = next; + }); + return () => subscription.remove(); + }, []); + // If we navigated into a folder that no longer exists (deleted, moved to // the trash on another device, or its share was revoked), cut the stack // back to the deepest ancestor that does. @@ -752,17 +791,21 @@ export default function FilesScreen() { clearSelection(); try { const fromTrash = targets.flatMap((r) => (r.trash ? [r.trash] : [])); - // The server removes folder descendants (onDestroyRemoveChildren). - if (fromTrash.length > 0) await purgeFromTrash(allNodes, fromTrash); + // The server removes folder descendants (onDestroyRemoveChildren). In + // the trash only what is still there goes (see purgeFromTrash). + let count = targets.length; + if (fromTrash.length > 0) count = await purgeFromTrash(fromTrash); else await deleteFileNodes(targets.map((r) => r.id)); - toast.success(t('files.deleted_forever', '{count, plural, one {Deleted permanently} other {# items deleted permanently}}', { - count: targets.length, - })); + if (count > 0) { + toast.success(t('files.deleted_forever', '{count, plural, one {Deleted permanently} other {# items deleted permanently}}', { + count, + })); + } } catch (e) { Alert.alert(t('files.delete_error', 'Failed to delete'), fileErrorMessage(e, t)); } await loadFiles('refresh'); - }, [confirmDelete, allNodes, clearSelection, loadFiles, t]); + }, [confirmDelete, clearSelection, loadFiles, t]); const restoreRows = useCallback(async (rows: FileRow[]) => { const items = rows.flatMap((r) => (r.trash ? [r.trash] : [])); @@ -784,13 +827,13 @@ export default function FilesScreen() { setConfirmEmptyTrash(false); clearSelection(); try { - await emptyTrash(allNodes); + await emptyTrash(); toast.success(t('files.trash_emptied', 'Trash emptied')); } catch (e) { Alert.alert(t('files.empty_trash_error', "Couldn't empty the trash"), fileErrorMessage(e, t)); } await loadFiles('refresh'); - }, [allNodes, clearSelection, loadFiles, t]); + }, [clearSelection, loadFiles, t]); const sortLabel = t(`files.${sortKey}`, sortKey); const SortDirIcon = sortDir === 'desc' ? ArrowDown : ArrowUp;