fix: slow file deletes from the trash lookups on every delete (#1390)

plugins/file-manager/src/backend/operation-routes.ts

@@ -1,5 +1,6 @@
 import type { Express, Request, Response } from "express";
 import type { PluginContext } from "@termix/plugin-sdk/backend";
+import type { SFTPWrapper } from "ssh2";
 import {
   execChannel,
   execWithSudo,
@@ -24,6 +25,8 @@ type FileOperationRoutesDeps = {
 
 const TRASH_RETENTION_DAYS_KEY = "trashRetentionDays";
 
+const TRASH_PURGE_INTERVAL_MS = 60 * 60 * 1000;
+
 export function registerFileOperationRoutes(
   app: Express,
   { ctx, sshSessions, verifySessionOwnership }: FileOperationRoutesDeps,
@@ -39,6 +42,22 @@ export function registerFileOperationRoutes(
     }
   };
 
+  // Deleting used to purge expired trash first, reading every trash entry
+  // before answering. Do it after the response, at most hourly per session.
+  const lastPurge = new WeakMap<SSHSession, number>();
+  const purgeExpiredTrash = (session: SSHSession, sftp: SFTPWrapper) => {
+    const now = Date.now();
+    if (now - (lastPurge.get(session) ?? 0) < TRASH_PURGE_INTERVAL_MS) return;
+    lastPurge.set(session, now);
+    void getTrashRetentionDays()
+      .then((days) => listTrash(sftp, days))
+      .catch((error) =>
+        ctx.log.warn(
+          `Could not purge expired trash: ${(error as Error).message}`,
+        ),
+      );
+  };
+
   async function ownedSession(
     req: Request,
     res: Response,
@@ -494,9 +513,8 @@ export function registerFileOperationRoutes(
     if (!permanent) {
       try {
         const sftp = await getSessionSftp(sshConn);
-        const retentionDays = await getTrashRetentionDays();
-        await listTrash(sftp, retentionDays);
         const item = await moveToTrash(sftp, itemPath);
+        purgeExpiredTrash(sshConn, sftp);
         ctx.log.info(
           `Item moved to trash: ${sessionId} (${itemPath}, trashId=${item.id})`,
         );

plugins/file-manager/src/backend/trash-service.ts

@@ -70,6 +70,8 @@ async function exists(sftp: SFTPWrapper, target: string) {
 }
 
 async function ensureDirectory(sftp: SFTPWrapper, target: string) {
+  // Usually there already: one round trip instead of one per segment.
+  if (await exists(sftp, target)) return;
   const normalized = target.replace(/\\/g, "/");
   const root = normalized.startsWith("/") ? "/" : "";
   const parts = normalized.split("/").filter(Boolean);
@@ -95,13 +97,32 @@ async function removeTree(sftp: SFTPWrapper, target: string): Promise<void> {
   await rmdir(sftp, target);
 }
 
-async function trashPaths(sftp: SFTPWrapper) {
+type TrashDirs = { root: string; files: string; info: string };
+
+// The trash folders of a channel do not move while it is open, and finding
+// them costs several round trips, which made every delete slow on a remote
+// host (#1390).
+const trashDirsByChannel = new WeakMap<SFTPWrapper, Promise<TrashDirs>>();
+
+function trashPaths(sftp: SFTPWrapper): Promise<TrashDirs> {
+  let dirs = trashDirsByChannel.get(sftp);
+  if (!dirs) {
+    dirs = findTrashPaths(sftp);
+    trashDirsByChannel.set(sftp, dirs);
+    dirs.catch(() => trashDirsByChannel.delete(sftp));
+  }
+  return dirs;
+}
+
+async function findTrashPaths(sftp: SFTPWrapper): Promise<TrashDirs> {
   const home = await call<string>((done) => sftp.realpath(".", done));
   const root = path.posix.join(home.replace(/\\/g, "/"), TRASH_DIR);
   const files = path.posix.join(root, "files");
   const info = path.posix.join(root, "info");
-  await ensureDirectory(sftp, files);
-  await ensureDirectory(sftp, info);
+  await Promise.all([
+    ensureDirectory(sftp, files),
+    ensureDirectory(sftp, info),
+  ]);
   return { root, files, info };
 }
 
@@ -124,7 +145,7 @@ function publicItem(item: StoredTrashItem): TrashItem {
 
 async function readStoredItem(
   sftp: SFTPWrapper,
-  dirs: { root: string; files: string; info: string },
+  dirs: TrashDirs,
   id: string,
 ): Promise<StoredTrashItem> {
   if (!ID_PATTERN.test(id)) throw new Error("Invalid trash item id");

plugins/file-manager/tests/backend/trash-service.test.ts

@@ -0,0 +1,95 @@
+import { describe, expect, it, vi } from "vitest";
+import type { SFTPWrapper } from "ssh2";
+import { listTrash, moveToTrash } from "../../src/backend/trash-service.js";
+
+/** An in-memory SFTP channel that counts every request. */
+function fakeSftp() {
+  const dirs = new Set(["/", "/home", "/home/me"]);
+  const files = new Map<string, Buffer>([["/home/me/a.txt", Buffer.from("a")]]);
+  const calls: string[] = [];
+  const stats = (target: string) =>
+    dirs.has(target) || files.has(target)
+      ? {
+          isDirectory: () => dirs.has(target),
+          size: files.get(target)?.length ?? 0,
+        }
+      : null;
+  const sftp = {
+    realpath: vi.fn((_p: string, done: (e?: Error, v?: string) => void) => {
+      calls.push("realpath");
+      done(undefined, "/home/me");
+    }),
+    stat: vi.fn((target: string, done: (e?: Error, v?: unknown) => void) => {
+      calls.push("stat");
+      const found = stats(target);
+      done(found ? undefined : new Error("ENOENT"), found ?? undefined);
+    }),
+    lstat: vi.fn((target: string, done: (e?: Error, v?: unknown) => void) => {
+      calls.push("lstat");
+      const found = stats(target);
+      done(found ? undefined : new Error("ENOENT"), found ?? undefined);
+    }),
+    mkdir: vi.fn((target: string, done: (e?: Error) => void) => {
+      calls.push("mkdir");
+      dirs.add(target);
+      done();
+    }),
+    rename: vi.fn((from: string, to: string, done: (e?: Error) => void) => {
+      calls.push("rename");
+      files.set(to, files.get(from)!);
+      files.delete(from);
+      done();
+    }),
+    writeFile: vi.fn(
+      (target: string, data: string, done: (e?: Error) => void) => {
+        calls.push("writeFile");
+        files.set(target, Buffer.from(data));
+        done();
+      },
+    ),
+    readdir: vi.fn((target: string, done: (e?: Error, v?: unknown) => void) => {
+      calls.push("readdir");
+      const prefix = `${target}/`;
+      done(
+        undefined,
+        [...files.keys()]
+          .filter((name) => name.startsWith(prefix))
+          .map((name) => ({ filename: name.slice(prefix.length) })),
+      );
+    }),
+    readFile: vi.fn((target: string, done: (e?: Error, v?: Buffer) => void) => {
+      calls.push("readFile");
+      done(undefined, files.get(target));
+    }),
+    unlink: vi.fn((target: string, done: (e?: Error) => void) => {
+      calls.push("unlink");
+      files.delete(target);
+      done();
+    }),
+  };
+  return { sftp: sftp as unknown as SFTPWrapper, calls, files };
+}
+
+describe("trash", () => {
+  it("finds the trash folders once per channel", async () => {
+    const { sftp, calls, files } = fakeSftp();
+    files.set("/home/me/b.txt", Buffer.from("b"));
+
+    await moveToTrash(sftp, "/home/me/a.txt");
+    const first = calls.length;
+    calls.length = 0;
+    await moveToTrash(sftp, "/home/me/b.txt");
+
+    expect(calls.filter((call) => call === "realpath")).toHaveLength(0);
+    expect(calls.filter((call) => call === "mkdir")).toHaveLength(0);
+    // lstat, rename and the metadata write.
+    expect(calls.length).toBeLessThan(first);
+    expect(calls).toEqual(["lstat", "rename", "writeFile"]);
+  });
+
+  it("lists what was moved", async () => {
+    const { sftp } = fakeSftp();
+    const item = await moveToTrash(sftp, "/home/me/a.txt");
+    expect(await listTrash(sftp, 7)).toEqual([item]);
+  });
+});