Merge pull request #20 from LINUXexpert-org/empty-trash-batching

Empty a folder in batches the server will accept

server/src/mock/index.ts

@@ -254,6 +254,30 @@ class MethodError extends Error {
   }
 }
 
+const MAX_OBJECTS = 500;
+
+/**
+ * Stalwart refuses a whole method call that carries more objects than it will
+ * process at once - it does not quietly handle the first 500. Enforce the same
+ * ceiling the session advertises, so an unbatched client fails here too.
+ */
+function enforceLimits(name: string, args: Obj): void {
+  const tooLarge = () => {
+    throw new MethodError("requestTooLarge", "The number of ids requested by the client exceeds the maximum number the server is willing to process in a single method call.");
+  };
+  if (name.endsWith("/get")) {
+    const ids = args.ids as unknown[] | null | undefined;
+    if (Array.isArray(ids) && ids.length > MAX_OBJECTS) tooLarge();
+  }
+  if (name.endsWith("/set")) {
+    const n =
+      Object.keys((args.create as Obj) ?? {}).length +
+      Object.keys((args.update as Obj) ?? {}).length +
+      ((args.destroy as unknown[] | undefined)?.length ?? 0);
+    if (n > MAX_OBJECTS) tooLarge();
+  }
+}
+
 const setResp = (extra: Obj = {}): Obj => ({ accountId: ACCOUNT, oldState: "1", newState: nextState(), created: {}, updated: {}, destroyed: [], ...extra });
 
 function genericGet(list: Obj[]) {
@@ -520,7 +544,7 @@ function readBody(req: IncomingMessage): Promise<Buffer> {
 }
 
 const session = () => ({
-  capabilities: { "urn:ietf:params:jmap:core": { maxSizeUpload: 50000000, maxConcurrentUpload: 4, maxSizeRequest: 10000000, maxConcurrentRequests: 4, maxCallsInRequest: 16, maxObjectsInGet: 500, maxObjectsInSet: 500, collationAlgorithms: ["i;ascii-casemap"] }, "urn:ietf:params:jmap:mail": {}, "urn:ietf:params:jmap:submission": {}, "urn:ietf:params:jmap:vacationresponse": {}, "urn:ietf:params:jmap:sieve": { implementation: "mock" }, "urn:ietf:params:jmap:calendars": {}, "urn:ietf:params:jmap:calendars:parse": {}, "urn:ietf:params:jmap:contacts": {}, "urn:ietf:params:jmap:contacts:parse": {}, "urn:ietf:params:jmap:principals": {}, "urn:ietf:params:jmap:principals:availability": {}, "urn:ietf:params:jmap:quota": {}, "urn:ietf:params:jmap:blob": {}, "urn:ietf:params:jmap:filenode": {}, ...(LEGACY ? {} : { "urn:stalwart:jmap": {} }) },
+  capabilities: { "urn:ietf:params:jmap:core": { maxSizeUpload: 50000000, maxConcurrentUpload: 4, maxSizeRequest: 10000000, maxConcurrentRequests: 4, maxCallsInRequest: 16, maxObjectsInGet: MAX_OBJECTS, maxObjectsInSet: MAX_OBJECTS, collationAlgorithms: ["i;ascii-casemap"] }, "urn:ietf:params:jmap:mail": {}, "urn:ietf:params:jmap:submission": {}, "urn:ietf:params:jmap:vacationresponse": {}, "urn:ietf:params:jmap:sieve": { implementation: "mock" }, "urn:ietf:params:jmap:calendars": {}, "urn:ietf:params:jmap:calendars:parse": {}, "urn:ietf:params:jmap:contacts": {}, "urn:ietf:params:jmap:contacts:parse": {}, "urn:ietf:params:jmap:principals": {}, "urn:ietf:params:jmap:principals:availability": {}, "urn:ietf:params:jmap:quota": {}, "urn:ietf:params:jmap:blob": {}, "urn:ietf:params:jmap:filenode": {}, ...(LEGACY ? {} : { "urn:stalwart:jmap": {} }) },
   accounts: { [ACCOUNT]: { name: USER, isPersonal: true, isReadOnly: false, accountCapabilities: { "urn:ietf:params:jmap:mail": {}, "urn:ietf:params:jmap:submission": {}, "urn:ietf:params:jmap:vacationresponse": {}, "urn:ietf:params:jmap:sieve": {}, "urn:ietf:params:jmap:calendars": {}, "urn:ietf:params:jmap:contacts": {}, "urn:ietf:params:jmap:principals": {}, "urn:ietf:params:jmap:quota": {}, "urn:ietf:params:jmap:filenode": {} } } },
   primaryAccounts: { ...Object.fromEntries(["mail", "submission", "vacationresponse", "sieve", "calendars", "contacts", "principals", "quota", "filenode", "blob"].map((c) => [`urn:ietf:params:jmap:${c}`, ACCOUNT])), ...(LEGACY ? {} : { "urn:stalwart:jmap": ACCOUNT }) },
   username: USER,
@@ -596,6 +620,7 @@ export const server = createServer(async (req, res) => {
       if (!h || (LEGACY && name.startsWith("x:"))) { responses.push(["error", { type: "unknownMethod" }, id]); continue; }
       try {
         const args = resolveRefs(rawArgs, responses);
+        enforceLimits(name, args);
         const r = h(args);
         responses.push([name, r as Obj, id]);
         if (name.endsWith("/set") || name.endsWith("/import")) touched.add(name.split("/")[0]!);

web/src/jmap/client.ts

@@ -104,6 +104,11 @@ export class JmapClient {
     return core?.maxObjectsInGet ?? 500;
   }
 
+  get maxObjectsInSet(): number {
+    const core = this.session?.capabilities[CAP.core] as { maxObjectsInSet?: number } | undefined;
+    return core?.maxObjectsInSet ?? 500;
+  }
+
   get maxSizeUpload(): number {
     const core = this.session?.capabilities[CAP.core] as { maxSizeUpload?: number } | undefined;
     return core?.maxSizeUpload ?? 50_000_000;
@@ -358,7 +363,7 @@ export function ref(resultOf: string, name: string, path: string): ResultRef {
   return { resultOf, name, path };
 }
 
-/** Chunk ids for /get calls to respect maxObjectsInGet. */
+/** Chunk ids so a /get or /set call stays under the server's per-call maximum. */
 export function chunk<T>(arr: T[], size: number): T[][] {
   const out: T[][] = [];
   for (let i = 0; i < arr.length; i += size) out.push(arr.slice(i, i + size));

web/src/store/__tests__/empty-mailbox.test.ts

@@ -0,0 +1,110 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import { CAP, client } from "@/jmap/client";
+import { useMail } from "@/store/mail";
+import { useToasts } from "@/ui/toast";
+import type { JmapSession } from "@/jmap/types";
+
+/**
+ * Emptying a full folder used to back-reference one Email/query straight into
+ * one Email/set, so every id in the folder arrived in a single call. Stalwart
+ * refuses the whole call over `maxObjectsInSet` with `requestTooLarge` — a
+ * Deleted Items with 5192 messages in it could not be emptied at all.
+ */
+
+const TRASH = "mbTrash";
+const MAX = 500;
+
+interface Call {
+  name: string;
+  args: Record<string, unknown>;
+  id: string;
+}
+
+/** A server that holds `count` messages and enforces MAX objects per call. */
+function server(count: number, opts: { refuseDestroy?: boolean } = {}) {
+  const live = new Set(Array.from({ length: count }, (_, i) => `e${i}`));
+  const destroyBatches: number[] = [];
+  const fetchMock = vi.fn(async (_url: string, init: RequestInit) => {
+    const body = JSON.parse(init.body as string) as { methodCalls: [string, Record<string, unknown>, string][] };
+    const methodResponses = body.methodCalls.map(([name, args, id]: [string, Record<string, unknown>, string]) => {
+      if (name === "Email/query" && (args.filter as { inMailbox?: string })?.inMailbox === TRASH) {
+        const limit = Math.min((args.limit as number) ?? 50, MAX);
+        return [name, { accountId: "a1", queryState: "q", canCalculateChanges: false, position: 0, ids: [...live].slice(0, limit), total: live.size }, id];
+      }
+      if (name === "Email/set" && Array.isArray(args.destroy)) {
+        const destroy = args.destroy as string[];
+        destroyBatches.push(destroy.length);
+        if (destroy.length > MAX) return ["error", { type: "requestTooLarge", description: "The number of ids requested by the client exceeds the maximum number the server is willing to process in a single method call." }, id];
+        if (opts.refuseDestroy) return [name, { accountId: "a1", oldState: "1", newState: "2", destroyed: [], notDestroyed: Object.fromEntries(destroy.map((x) => [x, { type: "forbidden", description: "no" }])) }, id];
+        for (const x of destroy) live.delete(x);
+        return [name, { accountId: "a1", oldState: "1", newState: "2", destroyed: destroy, notDestroyed: {} }, id];
+      }
+      // Everything the store refreshes afterwards; shape fits get and query.
+      return [name, { accountId: "a1", state: "1", list: [], notFound: [], ids: [], total: 0, queryState: "q", position: 0, canCalculateChanges: false }, id];
+    }) as unknown as Call[];
+    return { ok: true, status: 200, json: async () => ({ methodResponses, sessionState: "1" }) } as Response;
+  });
+  vi.stubGlobal("fetch", fetchMock);
+  return { live, destroyBatches, fetchMock };
+}
+
+const messages = () => useToasts.getState().toasts.map((t) => t.message);
+
+beforeEach(() => {
+  client.session = {
+    capabilities: { [CAP.core]: { maxObjectsInGet: MAX, maxObjectsInSet: MAX }, [CAP.mail]: {} },
+    accounts: {},
+    primaryAccounts: {},
+    state: "s1",
+  } as unknown as JmapSession;
+  useMail.setState({ accountId: "a1", mailboxes: { [TRASH]: { id: TRASH, role: "trash", name: "Deleted Items" } } as never, list: null, emails: {} });
+  useToasts.setState({ toasts: [] });
+});
+
+afterEach(() => {
+  vi.unstubAllGlobals();
+  vi.restoreAllMocks();
+});
+
+describe("emptyMailbox", () => {
+  it("deletes a folder larger than maxObjectsInSet, one accepted batch at a time", async () => {
+    const s = server(5192);
+    await useMail.getState().emptyMailbox(TRASH);
+    expect(Math.max(...s.destroyBatches)).toBeLessThanOrEqual(MAX);
+    expect(s.live.size).toBe(0);
+    expect(messages()).toContain("Deleted 5192 messages");
+  });
+
+  it("needs no batching when the folder already fits in one call", async () => {
+    const s = server(12);
+    await useMail.getState().emptyMailbox(TRASH);
+    expect(s.destroyBatches).toEqual([12]);
+    expect(messages()).toContain("Deleted 12 messages");
+  });
+
+  it("refuses any folder that is not Deleted Items", async () => {
+    const s = server(5192);
+    useMail.setState({ mailboxes: { ...useMail.getState().mailboxes, mbJunk: { id: "mbJunk", role: "junk", name: "Junk" } } as never });
+    await useMail.getState().emptyMailbox("mbJunk");
+    expect(s.destroyBatches).toEqual([]);
+    expect(s.live.size).toBe(5192);
+    expect(messages()).toContain("Only Deleted Items can be emptied.");
+  });
+
+  it("stops instead of looping when the server destroys nothing", async () => {
+    const s = server(5192, { refuseDestroy: true });
+    await useMail.getState().emptyMailbox(TRASH);
+    expect(s.destroyBatches).toHaveLength(1);
+    expect(messages().some((m) => m.startsWith("Could not empty folder"))).toBe(true);
+  });
+});
+
+describe("destroy", () => {
+  it("splits a selection bigger than maxObjectsInSet across calls", async () => {
+    const s = server(1200);
+    await useMail.getState().destroy(Array.from({ length: 1200 }, (_, i) => `e${i}`));
+    expect(s.destroyBatches).toEqual([MAX, MAX, 200]);
+    expect(s.live.size).toBe(0);
+    expect(messages()).toContain("1200 messages deleted forever");
+  });
+});

web/src/store/mail.ts

@@ -11,6 +11,7 @@ import type {
   MailboxRole,
   QueryResponse,
   Quota,
+  SetError,
   SetResponse,
   Thread,
   VacationResponse,
@@ -517,8 +518,8 @@ export const useMail = create<MailState>((set, get) => ({
       return { emails: next, selected: {} };
     });
     try {
-      const res = await client.call<SetResponse>("Email/set", { accountId, destroy: ids });
-      const failed = Object.keys(res.notDestroyed ?? {});
+      const { notDestroyed } = await destroyEmails(accountId, ids);
+      const failed = Object.keys(notDestroyed);
       if (failed.length) toast.error(`${failed.length} message(s) could not be deleted`);
       else toast.show(`${ids.length === 1 ? "Message" : `${ids.length} messages`} deleted forever`);
       void get().loadMailboxes();
@@ -555,19 +556,44 @@ export const useMail = create<MailState>((set, get) => ({
   async emptyMailbox(mailboxId) {
     const accountId = get().accountId;
     if (!accountId) return;
+    // Emptying is permanent and covers the whole folder at once, so it is
+    // offered for Deleted Items alone. The menus hide it elsewhere; this is
+    // the guard that makes that true of the action itself.
+    if (mailboxId !== get().roleId("trash")) {
+      toast.error("Only Deleted Items can be emptied.");
+      return;
+    }
+    // A folder can hold far more messages than the server will destroy in one
+    // call, so walk it a page at a time instead of back-referencing one huge
+    // query into one Email/set. Each pass re-runs the filter, so the next page
+    // is simply whatever is still in the folder.
+    const page = client.maxObjectsInSet;
+    let deleted = 0;
+    let progress: number | null = null;
     try {
-      const res = await client.chain([
-        ["Email/query", { accountId, filter: { inMailbox: mailboxId }, limit: 5000 }, "q"],
-        ["Email/set", { accountId, "#destroy": { resultOf: "q", name: "Email/query", path: "/ids" } }, "s"],
-      ]);
-      const s = res.get("s")?.[0] as unknown as SetResponse;
-      const n = s.destroyed?.length ?? 0;
-      toast.show(`Deleted ${n} message${n === 1 ? "" : "s"}`);
+      for (;;) {
+        const q = await client.call<QueryResponse>("Email/query", { accountId, filter: { inMailbox: mailboxId }, limit: page });
+        if (!q.ids.length) break;
+        if (progress === null && (q.total ?? q.ids.length) > page) {
+          progress = toast.show("Emptying folder…", { duration: 0 });
+        }
+        const { destroyed, notDestroyed } = await destroyEmails(accountId, q.ids);
+        deleted += destroyed.length;
+        // Nothing went through: the rest is undeletable, and looping again
+        // would ask for the same ids forever.
+        if (!destroyed.length) {
+          const [, err] = Object.entries(notDestroyed)[0] ?? [];
+          throw new Error(err ? setErrorMessage(err) : "the server refused to delete these messages");
+        }
+      }
+      toast.show(`Deleted ${deleted} message${deleted === 1 ? "" : "s"}`);
       set({ list: get().list ? { ...get().list!, ids: get().list!.mailboxId === mailboxId ? [] : get().list!.ids, total: 0 } : null });
+    } catch (err) {
+      toast.error(`Could not empty folder: ${(err as Error).message}${deleted ? ` (${deleted} deleted first)` : ""}`);
+    } finally {
+      if (progress !== null) toast.dismiss(progress);
       void get().loadMailboxes();
       void get().refreshList();
-    } catch (err) {
-      toast.error(`Could not empty folder: ${(err as Error).message}`);
     }
   },
 
@@ -590,32 +616,40 @@ export const useMail = create<MailState>((set, get) => ({
     const accountId = get().accountId;
     if (!accountId) return;
     const boxes = includeChildren ? get().descendantMailboxIds(mailboxId) : [mailboxId];
+    // The ids the query returns are all we need; asking Email/get to echo them
+    // back only risks blowing past maxObjectsInGet on a very full folder.
+    const page = client.maxObjectsInSet;
     const unreadIn = async (filter: EmailFilter): Promise<Id[]> => {
-      const res = await client.chain([
-        ["Email/query", { accountId, filter, limit: 5000 }, "q"],
-        ["Email/get", { accountId, "#ids": { resultOf: "q", name: "Email/query", path: "/ids" }, properties: ["id"] }, "g"],
-      ]);
-      return ((res.get("g")?.[0] as unknown as GetResponse<Email>).list ?? []).map((e) => e.id);
+      const res = await client.call<QueryResponse>("Email/query", { accountId, filter, limit: page });
+      return res.ids;
     };
+    const nextUnread = async (): Promise<Id[]> => {
+      if (boxes.length === 1) return unreadIn({ inMailbox: boxes[0]!, notKeyword: "$seen" });
       try {
-      let ids: Id[];
-      if (boxes.length === 1) {
-        ids = await unreadIn({ inMailbox: boxes[0]!, notKeyword: "$seen" });
-      } else {
-        try {
-          ids = await unreadIn({ operator: "AND", conditions: [{ notKeyword: "$seen" }, { operator: "OR", conditions: boxes.map((id) => ({ inMailbox: id })) }] });
+        return await unreadIn({ operator: "AND", conditions: [{ notKeyword: "$seen" }, { operator: "OR", conditions: boxes.map((id) => ({ inMailbox: id })) }] });
       } catch {
         // Server without filter-operator support: one query per folder.
         const per = await Promise.all(boxes.map((id) => unreadIn({ inMailbox: id, notKeyword: "$seen" }).catch(() => [] as Id[])));
-          ids = [...new Set(per.flat())];
+        return [...new Set(per.flat())];
       }
+    };
+    try {
+      // One page per pass; the ones just marked drop out of the filter, so a
+      // repeated head id means the last pass changed nothing and we stop.
+      let marked = 0;
+      let lastHead: Id | null = null;
+      for (;;) {
+        const ids = await nextUnread();
+        if (!ids.length || ids[0] === lastHead) break;
+        lastHead = ids[0]!;
+        await get().markRead(ids, true);
+        marked += ids.length;
       }
-      if (!ids.length) {
+      if (!marked) {
         toast.show("Nothing unread here");
         return;
       }
-      await get().markRead(ids, true);
-      toast.success(`Marked ${ids.length} message${ids.length === 1 ? "" : "s"} as read${includeChildren && boxes.length > 1 ? ` in ${boxes.length} folders` : ""}`);
+      toast.success(`Marked ${marked} message${marked === 1 ? "" : "s"} as read${includeChildren && boxes.length > 1 ? ` in ${boxes.length} folders` : ""}`);
       void get().loadMailboxes();
     } catch (err) {
       toast.error(`Could not mark as read: ${(err as Error).message}`);
@@ -860,9 +894,26 @@ async function runQuery(accountId: Id, q: ListQuery, position: number, limit: nu
   return { ids: query.ids, total: query.total ?? query.ids.length, queryState: query.queryState };
 }
 
+/**
+ * Destroy emails in batches the server will accept.
+ *
+ * Handing Email/set more ids than `maxObjectsInSet` fails the whole call with
+ * requestTooLarge — nothing is deleted — so split first and merge the results.
+ */
+async function destroyEmails(accountId: Id, ids: Id[]): Promise<{ destroyed: Id[]; notDestroyed: Record<Id, SetError> }> {
+  const destroyed: Id[] = [];
+  const notDestroyed: Record<Id, SetError> = {};
+  for (const part of chunk(ids, client.maxObjectsInSet)) {
+    const res = await client.call<SetResponse>("Email/set", { accountId, destroy: part });
+    destroyed.push(...(res.destroyed ?? []));
+    Object.assign(notDestroyed, res.notDestroyed ?? {});
+  }
+  return { destroyed, notDestroyed };
+}
+
 async function setEmails(accountId: Id, update: Record<Id, Record<string, unknown>>) {
   const ids = Object.keys(update);
-  for (const part of chunk(ids, 400)) {
+  for (const part of chunk(ids, client.maxObjectsInSet)) {
     const sub: Record<Id, Record<string, unknown>> = {};
     for (const id of part) sub[id] = update[id]!;
     const res = await client.call<SetResponse>("Email/set", { accountId, update: sub });

web/src/views/mail/MailboxTree.tsx

@@ -261,7 +261,7 @@ function MailboxMenu({ mailbox: m, onCreateChild, onShare }: { mailbox: Mailbox;
       <MenuItem icon={m.isSubscribed ? <EyeOff size={16} /> : <Eye size={16} />} label={m.isSubscribed ? "Hide from list" : "Show in list"} onClick={() => void useMail.getState().updateMailbox(m.id, { isSubscribed: !m.isSubscribed })} disabled={m.role === "inbox"} />
       <MenuItem icon={<Share2 size={16} />} label="Share…" onClick={onShare} />
       <MenuSep />
-      {(m.role === "trash" || m.role === "junk") && <MenuItem icon={<Eraser size={16} />} label="Empty folder" onClick={() => void empty()} danger />}
+      {m.role === "trash" && <MenuItem icon={<Eraser size={16} />} label="Empty folder" onClick={() => void empty()} danger />}
       <MenuItem icon={<Trash2 size={16} />} label="Delete folder" onClick={() => void remove()} danger disabled={isSpecial || !m.myRights.mayDelete} />
     </>
   );

web/src/views/mail/MessageList.tsx

@@ -74,6 +74,8 @@ export function MessageList({ title, list, openThreadId, focusId, setFocusId, on
   const selCount = Object.keys(selected).length;
   const mailbox = mailboxId ? mailboxes[mailboxId] : undefined;
   const isTrashOrJunk = mailbox?.role === "trash" || mailbox?.role === "junk";
+  // Emptying in one action is for Deleted Items only; Junk is cleared by hand.
+  const isTrash = mailbox?.role === "trash";
   const isDrafts = mailbox?.role === "drafts";
 
   const rowHeight = twoLine ? (settings.density === "compact" ? 56 : settings.density === "comfortable" ? 78 : 66) : settings.density === "compact" ? 36 : settings.density === "comfortable" ? 52 : 44;
@@ -198,7 +200,7 @@ export function MessageList({ title, list, openThreadId, focusId, setFocusId, on
               <MenuSep />
               <MenuItem icon={<CheckSquare size={16} />} label="Select all" onClick={selectAll} />
               <MenuItem icon={<MailOpen size={16} />} label="Mark all as read" onClick={() => mailboxId && void useMail.getState().markMailboxRead(mailboxId)} disabled={!mailboxId} />
-              {isTrashOrJunk && (
+              {isTrash && (
                 <>
                   <MenuSep />
                   <MenuItem