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

@@ -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']);
   });
 });

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);

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<void> {
-  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<number> {
+  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<void> {
-  // 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<void> {
+  // 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]);
 }