diff --git a/web/src/store/__tests__/list-refresh.test.ts b/web/src/store/__tests__/list-refresh.test.ts new file mode 100644 index 0000000..dea9c19 --- /dev/null +++ b/web/src/store/__tests__/list-refresh.test.ts @@ -0,0 +1,191 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { CAP, client } from "@/jmap/client"; +import { useMail } from "@/store/mail"; +import type { JmapSession } from "@/jmap/types"; + +/** + * What a list refresh and an open thread cost the server. + * + * A refresh fetches everything already on screen, and in conversation mode + * every message of every listed thread. Both used to go to Email/get in one + * call whatever their number, and Stalwart refuses a whole call over + * `maxObjectsInGet` -- so past a few pages, or with long threads, the refresh + * failed and the list silently stopped updating. + */ + +const MAX = 500; +const INBOX = "mbInbox"; + +type Call = [string, Record, string]; + +/** + * A mailbox of `count` messages. With `threadSize` above 1, each listed + * message leads a thread of that many, whose other members are not in the + * list. Enforces MAX on every /get, back-referenced ids included, as the mock + * server and Stalwart do. + */ +function server(count: number, threadSize = 1) { + const listed = Array.from({ length: count }, (_, i) => `e${i}`); + const members = (lead: string) => [lead, ...Array.from({ length: threadSize - 1 }, (_, j) => `${lead}m${j}`)]; + const calls: Call[] = []; + const fetchMock = vi.fn(async (_url: string, init: RequestInit) => { + const body = JSON.parse(init.body as string) as { methodCalls: Call[] }; + const responses: Call[] = []; + const resolve = (args: Record): Record => { + const ref = args["#ids"] as { resultOf: string; path: string } | undefined; + if (!ref) return args; + const from = responses.find((r) => r[2] === ref.resultOf)![1]; + const ids = + ref.path === "/ids" ? (from.ids as string[]) + : ref.path === "/list/*/threadId" ? (from.list as { threadId: string }[]).map((e) => e.threadId) + : (from.list as { emailIds: string[] }[]).flatMap((t) => t.emailIds); + const { "#ids": _drop, ...rest } = args; + return { ...rest, ids }; + }; + for (const [name, raw, id] of body.methodCalls) { + const args = resolve(raw); + calls.push([name, args, id]); + const ids = args.ids as string[] | undefined; + if (name.endsWith("/get") && ids && ids.length > MAX) { + responses.push(["error", { type: "requestTooLarge" }, id]); + continue; + } + if (name === "Email/query") { + const position = args.position as number; + const limit = args.limit as number; + responses.push([name, { accountId: "a1", queryState: "q1", canCalculateChanges: false, position, ids: listed.slice(position, position + limit), total: listed.length }, id]); + } else if (name === "Email/get") { + const list = ids!.map((e) => ({ id: e, threadId: `t${e.replace(/m\d+$/, "")}`, mailboxIds: { [INBOX]: true }, keywords: {}, receivedAt: "2026-09-16T00:00:00Z" })); + responses.push([name, { accountId: "a1", state: "s1", list, notFound: [] }, id]); + } else if (name === "Thread/get") { + const list = ids!.map((t) => ({ id: t, emailIds: members(t.slice(1)) })); + responses.push([name, { accountId: "a1", state: "s1", list, notFound: [] }, id]); + } else { + responses.push([name, { accountId: "a1", state: "s1", list: [], notFound: [] }, id]); + } + } + return { ok: true, status: 200, json: async () => ({ methodResponses: responses, sessionState: "1" }) } as Response; + }); + vi.stubGlobal("fetch", fetchMock); + return { calls, listed }; +} + +const getSizes = (calls: Call[]) => calls.filter(([n]) => n === "Email/get").map(([, a]) => (a.ids as string[]).length); + +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: { [INBOX]: { id: INBOX, role: "inbox", name: "Inbox" } } as never, + list: null, + emails: {}, + fullIds: {}, + threads: {}, + emailState: "s1", + loadingThreads: {}, + openThreadId: null, + }); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +const listOf = (ids: string[], collapseThreads = false) => ({ + key: "k", + filter: { inMailbox: INBOX }, + sort: [], + collapseThreads, + mailboxId: INBOX, + ids, + total: ids.length, + queryState: "q0", + loading: false, + loadingMore: false, + error: null, + exhausted: false, +}); + +describe("refreshList", () => { + it("refreshes more rows than one Email/get may carry, in pages the server takes", async () => { + const { calls, listed } = server(1300); + useMail.setState({ list: listOf(listed.slice(0, 1200)) as never }); + await useMail.getState().refreshList(); + const sizes = getSizes(calls); + expect(sizes.every((n) => n <= MAX)).toBe(true); + expect(calls.some(([n]) => n === "error")).toBe(false); + const list = useMail.getState().list!; + expect(list.ids).toEqual(listed.slice(0, 1200)); + expect(list.total).toBe(1300); + expect(list.error).toBeNull(); + }); + + it("stops at the end of a folder that shrank", async () => { + const { listed } = server(700); + useMail.setState({ list: listOf([...listed, ...Array.from({ length: 300 }, (_, i) => `gone${i}`)]) as never }); + await useMail.getState().refreshList(); + const list = useMail.getState().list!; + expect(list.ids).toEqual(listed); + expect(list.exhausted).toBe(true); + }); + + it("fetches long threads' other messages within the limit", async () => { + // 50 listed threads of 20 messages: 950 members that are not in the list. + const { calls, listed } = server(50, 20); + await useMail.getState().query({ key: "", filter: { inMailbox: INBOX }, sort: [], collapseThreads: true, mailboxId: INBOX }); + const list = useMail.getState().list!; + expect(list.error).toBeNull(); + expect(list.ids).toEqual(listed); + expect(getSizes(calls).every((n) => n <= MAX)).toBe(true); + const emails = useMail.getState().emails; + expect(Object.keys(emails)).toHaveLength(50 * 20); + // A second refresh does not fetch the members it already holds. + calls.length = 0; + await useMail.getState().refreshList(); + expect(getSizes(calls)).toEqual([50]); + }); +}); + +describe("loadThread", () => { + it("fetches no bodies for messages already held in full", async () => { + const { calls } = server(1, 3); + useMail.setState({ + emails: { e0: { id: "e0" }, e0m0: { id: "e0m0" }, e0m1: { id: "e0m1" } } as never, + fullIds: { e0: true, e0m0: true, e0m1: true }, + }); + const before = useMail.getState().emails.e0; + const got = await useMail.getState().loadThread("te0"); + expect(got.map((e) => e.id)).toEqual(["e0", "e0m0", "e0m1"]); + expect(calls.map(([n]) => n)).toEqual(["Thread/get"]); + // The same object, so nothing derived from it has to be rebuilt. + expect(useMail.getState().emails.e0).toBe(before); + }); + + it("fetches in full only the message it does not have", async () => { + const { calls } = server(1, 3); + useMail.setState({ + emails: { e0: { id: "e0" }, e0m0: { id: "e0m0" }, e0m1: { id: "e0m1" } } as never, + fullIds: { e0: true, e0m0: true }, + }); + await useMail.getState().loadThread("te0"); + const gets = calls.filter(([n]) => n === "Email/get"); + expect(gets).toHaveLength(1); + expect(gets[0]![1].ids).toEqual(["e0m1"]); + expect(gets[0]![1].fetchHTMLBodyValues).toBe(true); + expect(useMail.getState().fullIds.e0m1).toBe(true); + expect(useMail.getState().loadingThreads).toEqual({}); + }); + + it("splits a thread longer than one Email/get may carry", async () => { + const { calls } = server(1, 1200); + const got = await useMail.getState().loadThread("te0"); + expect(got).toHaveLength(1200); + expect(getSizes(calls).every((n) => n <= MAX)).toBe(true); + }); +}); diff --git a/web/src/store/mail/index.ts b/web/src/store/mail/index.ts index eeb13f1..4f2bd1e 100644 --- a/web/src/store/mail/index.ts +++ b/web/src/store/mail/index.ts @@ -206,8 +206,24 @@ export const useMail = create((set, get) => ({ const l = get().list; if (!accountId || !l) return; try { - const limit = Math.max(settings().pageSize, l.ids.length); - const { ids, total, queryState } = await runQuery(accountId, l, 0, limit); + /* + * Everything already on screen is fetched again, which past a few pages + * is more than one Email/get may carry: Stalwart refuses the whole call + * over `maxObjectsInGet`, and a refused refresh left the list silently + * stale. So it goes in pages the server will take. + */ + const want = Math.max(settings().pageSize, l.ids.length); + const ids: Id[] = []; + const seen = new Set(); + let total = 0; + let queryState = ""; + while (ids.length < want) { + const page = await runQuery(accountId, l, ids.length, want - ids.length); + total = page.total; + queryState ||= page.queryState; + for (const id of page.ids) if (!seen.has(id)) { seen.add(id); ids.push(id); } + if (page.ids.length < page.limit || ids.length >= total) break; + } const cur = get().list; if (!cur || cur.key !== l.key) return; set({ list: { ...cur, ids, total, queryState, loading: false, error: null, exhausted: ids.length >= total } }); @@ -255,34 +271,31 @@ export const useMail = create((set, get) => ({ if (!accountId) return []; set((s) => ({ loadingThreads: { ...s.loadingThreads, [threadId]: true } })); try { - const res = await client.chain([ - ["Thread/get", { accountId, ids: [threadId] }, "t"], - [ - "Email/get", - { - accountId, - "#ids": { resultOf: "t", name: "Thread/get", path: "/list/*/emailIds" }, - properties: FULL_PROPS, - fetchHTMLBodyValues: true, - fetchTextBodyValues: true, - maxBodyValueBytes: 2 * 1024 * 1024, - bodyProperties: BODY_PROPS, - }, - "e", - ], - ]); - const thread = (res.get("t")?.[0] as unknown as GetResponse).list[0]; - const emailsRes = res.get("e")?.[0] as unknown as GetResponse; - if (!thread) return []; + /* + * Bodies are fetched only for messages not already held in full. + * + * This runs on every push that touches mail, the open thread's own + * mark-as-read included, and it used to fetch every message in the + * thread in full each time -- up to 2 MB of body apiece, and new + * attachment objects that made the reading pane rebuild what it had + * already rendered. A body cannot change under an id (RFC 8621), and + * keywords and mailboxes come in with the list refresh, so a message + * held in full needs nothing more. getEmails also splits the fetch to + * `maxObjectsInGet`, which a long thread could exceed. + */ + const res = await client.call>("Thread/get", { accountId, ids: [threadId] }); + const thread = res.list[0]; + if (!thread) { + set((s) => { + const { [threadId]: _drop, ...rest } = s.loadingThreads; + return { loadingThreads: rest }; + }); + return []; + } + await get().getEmails(thread.emailIds, true); set((s) => { - const next = { ...s.emails }; - const nextFull = { ...s.fullIds }; - for (const e of emailsRes.list) { - next[e.id] = { ...next[e.id], ...e }; - nextFull[e.id] = true; - } const { [threadId]: _drop, ...rest } = s.loadingThreads; - return { emails: next, fullIds: nextFull, threads: { ...s.threads, [threadId]: thread }, loadingThreads: rest }; + return { threads: { ...s.threads, [threadId]: thread }, loadingThreads: rest }; }); return get().threadEmails(threadId); } catch (err) { @@ -1057,29 +1070,54 @@ function isUnsupportedSort(err: unknown): boolean { return type === "unsupportedSort" || /unsupportedSort/i.test(message); } -async function runQueryOnce(accountId: Id, q: ListQuery, position: number, limit: number) { +async function runQueryOnce(accountId: Id, q: ListQuery, position: number, requested: number) { + // The ids are back-referenced into Email/get, which may carry no more than this. + const limit = Math.min(requested, client.maxObjectsInGet); const calls: Array<[string, Record, string]> = [ ["Email/query", { accountId, filter: q.filter, sort: q.sort, collapseThreads: q.collapseThreads, position, limit, calculateTotal: true }, "q"], ["Email/get", { accountId, "#ids": { resultOf: "q", name: "Email/query", path: "/ids" }, properties: LIST_PROPS }, "e"], ]; if (q.collapseThreads) { calls.push(["Thread/get", { accountId, "#ids": { resultOf: "e", name: "Email/get", path: "/list/*/threadId" } }, "t"]); - calls.push(["Email/get", { accountId, "#ids": { resultOf: "t", name: "Thread/get", path: "/list/*/emailIds" }, properties: LIST_PROPS }, "te"]); } const res = await client.chain(calls); const query = res.get("q")?.[0] as unknown as QueryResponse; const emailsRes = res.get("e")?.[0] as unknown as GetResponse; const threadsRes = res.get("t")?.[0] as unknown as GetResponse | undefined; - const threadEmails = res.get("te")?.[0] as unknown as GetResponse | undefined; + /* + * The other messages in each listed thread, for its count and unread state. + * These used to come back-referenced from Thread/get in the same request, + * with no bound: fifty long conversations could carry more ids than one + * Email/get may, and the server refused the whole page. Fetched separately + * instead, split to the limit, and only those not already held -- a cached + * one is kept current by Email/changes. When there is no state to follow + * changes from, every member is fetched, since nothing else will update it. + */ + const following = useMail.getState().emailState !== null; useMail.setState((s) => { const emails = { ...s.emails }; for (const e of emailsRes.list) emails[e.id] = { ...emails[e.id], ...e }; - for (const e of threadEmails?.list ?? []) emails[e.id] = { ...emails[e.id], ...e }; const threads = { ...s.threads }; for (const t of threadsRes?.list ?? []) threads[t.id] = t; return { emails, threads, emailState: s.emailState ?? emailsRes.state }; }); - return { ids: query.ids, total: query.total ?? query.ids.length, queryState: query.queryState }; + const members = (threadsRes?.list ?? []).flatMap((t) => t.emailIds); + if (members.length) await refreshEmails(accountId, following ? members.filter((id) => !useMail.getState().emails[id]) : members); + return { ids: query.ids, total: query.total ?? query.ids.length, queryState: query.queryState, limit }; +} + +/** Fetch list properties for `ids`, split to `maxObjectsInGet`, and merge them in. */ +async function refreshEmails(accountId: Id, ids: Id[]): Promise { + const unique = [...new Set(ids)]; + if (!unique.length) return; + const results = await Promise.all( + chunk(unique, client.maxObjectsInGet).map((part) => client.call>("Email/get", { accountId, ids: part, properties: LIST_PROPS })), + ); + useMail.setState((s) => { + const emails = { ...s.emails }; + for (const r of results) for (const e of r.list) emails[e.id] = { ...emails[e.id], ...e }; + return { emails }; + }); } /** diff --git a/web/src/views/mail/MailView.tsx b/web/src/views/mail/MailView.tsx index e40ba3d..aacecfc 100644 --- a/web/src/views/mail/MailView.tsx +++ b/web/src/views/mail/MailView.tsx @@ -79,8 +79,25 @@ export function MailView({ mailboxId, threadId, search }: { mailboxId?: string; [settings.listSortScope, settings.listSortPreset, settings.listSortLevels], ); + /* + * What the list query reads from the folder map: names, roles and places in + * the tree. `mailboxes` itself is replaced on every reload -- every push that + * touches mail reloads it for the counts -- and depending on it directly made + * each reload build a new query, which `query()` answered with a second full + * refresh of the list. + */ + const folderShape = useMemo( + () => + Object.values(mailboxes) + .map((m) => `${m.id}\u0000${m.name}\u0000${m.role ?? ""}\u0000${m.parentId ?? ""}`) + .sort() + .join("\u0001"), + [mailboxes], + ); + // Build & run the list query const listQuery = useMemo(() => { + const mailboxes = useMail.getState().mailboxes; if (search) { if (!q) return null; const parsed = parseQuery(q); @@ -94,7 +111,7 @@ export function MailView({ mailboxId, threadId, search }: { mailboxId?: string; // are outgoing, and collapsing them into their threads hides them. const isDraftsOrSent = mb?.role === "drafts" || mb?.role === "sent" || mailboxId === scheduledId; return { key: "", filter: { inMailbox: mailboxId }, sort: sortForFolder(mailboxId), collapseThreads: settings.conversationMode && !isDraftsOrSent, mailboxId }; - }, [search, q, mailboxId, mailboxes, settings.conversationMode, scheduledId]); + }, [search, q, mailboxId, folderShape, settings.conversationMode, scheduledId]); useEffect(() => { if (listQuery && mailboxesLoaded) void query(listQuery);