From db2c0aff3db3ca6533c3e3bfafeb5e75fdeca0c2 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 15 Oct 2026 00:14:57 +0200 Subject: [PATCH] perf: load conversation sizes with the folder page instead of in a second request --- app/api/dev-jmap/[...path]/route.ts | 32 +++++++-- lib/__tests__/dev-jmap-mock.test.ts | 22 ++++++ lib/__tests__/jmap-list-thread-counts.test.ts | 72 +++++++++++++++++++ lib/jmap/client-interface.ts | 6 +- lib/jmap/client.ts | 25 ++++++- .../email-store-thread-counts.test.ts | 63 ++++++++++++++++ stores/email-store.ts | 36 ++++++++-- 7 files changed, 243 insertions(+), 13 deletions(-) create mode 100644 lib/__tests__/jmap-list-thread-counts.test.ts create mode 100644 stores/__tests__/email-store-thread-counts.test.ts diff --git a/app/api/dev-jmap/[...path]/route.ts b/app/api/dev-jmap/[...path]/route.ts index c976f7fea..d6b957b61 100644 --- a/app/api/dev-jmap/[...path]/route.ts +++ b/app/api/dev-jmap/[...path]/route.ts @@ -2481,6 +2481,24 @@ const METHOD_HANDLERS: Record = { // Resolve back-references between method calls // --------------------------------------------------------------------------- +/** A JSON pointer with "*" wildcard steps, as JMAP result references use. */ +function resolvePointer(value: unknown, path: string): unknown { + const steps = path.replace(/^\//, '').split('/').filter((step) => step.length > 0); + let current: unknown[] = [value]; + for (const step of steps) { + const next: unknown[] = []; + for (const item of current) { + if (step === '*') { + if (Array.isArray(item)) next.push(...item); + } else if (item && typeof item === 'object') { + next.push((item as Record)[step]); + } + } + current = next; + } + return steps.includes('*') ? current : current[0]; +} + function resolveBackReferences( methodCalls: Array<[string, MethodArgs, string]>, responses: MethodResult[], @@ -2489,15 +2507,17 @@ function resolveBackReferences( const [method, args, callId] = call; const resolved = { ...args }; - // Handle #ids back-reference (used by Email/get after Email/query) - if (resolved['#ids']) { - const ref = resolved['#ids'] as { resultOf: string; name: string; path: string }; + // Every "#name" argument is a back-reference (RFC 8620 §3.7): the value at + // `path` in an earlier response replaces it under the plain name, where a + // "*" step collects across an array (e.g. /list/*/threadId). + for (const key of Object.keys(resolved)) { + if (!key.startsWith('#')) continue; + const ref = resolved[key] as { resultOf: string; name: string; path: string }; const refResponse = responses.find((r) => r[2] === ref.resultOf && r[0] === ref.name); if (refResponse) { - const path = ref.path.replace(/^\//, ''); - resolved.ids = refResponse[1][path] as string[]; + resolved[key.slice(1)] = resolvePointer(refResponse[1], ref.path); } - delete resolved['#ids']; + delete resolved[key]; } return [method, resolved, callId] as [string, MethodArgs, string]; diff --git a/lib/__tests__/dev-jmap-mock.test.ts b/lib/__tests__/dev-jmap-mock.test.ts index e3fc2d19f..784777e0d 100644 --- a/lib/__tests__/dev-jmap-mock.test.ts +++ b/lib/__tests__/dev-jmap-mock.test.ts @@ -90,6 +90,28 @@ describe('dev-jmap mock server', () => { }); }); + describe('POST /api - back-references', () => { + it('resolves a wildcard path across a list (RFC 8620 §3.7)', async () => { + const req = makeRequest('http://localhost:3000/api/dev-jmap/api', { + method: 'POST', + headers: { 'Content-Type': 'application/json', host: 'localhost:3000' }, + body: JSON.stringify({ + methodCalls: [ + ['Email/query', { accountId: 'dev-account-001', filter: { inMailbox: 'mb-inbox' }, limit: 3 }, '0'], + ['Email/get', { accountId: 'dev-account-001', '#ids': { resultOf: '0', name: 'Email/query', path: '/ids' }, properties: ['threadId'] }, '1'], + ['Thread/get', { accountId: 'dev-account-001', '#ids': { resultOf: '1', name: 'Email/get', path: '/list/*/threadId' } }, '2'], + ], + }), + }); + const res = await POST(req, { params: Promise.resolve({ path: ['api'] }) }); + const data = await res.json(); + const threadIds = data.methodResponses[1][1].list.map((e: { threadId: string }) => e.threadId); + expect(threadIds.length).toBeGreaterThan(0); + expect(data.methodResponses[2][0]).toBe('Thread/get'); + expect(data.methodResponses[2][1].list.map((t: { id: string }) => t.id).sort()).toEqual([...new Set(threadIds)].sort()); + }); + }); + describe('POST /api - Email/query', () => { it('should filter by mailbox', async () => { const req = makeRequest('http://localhost:3000/api/dev-jmap/api', { diff --git a/lib/__tests__/jmap-list-thread-counts.test.ts b/lib/__tests__/jmap-list-thread-counts.test.ts new file mode 100644 index 000000000..1f9f3d44c --- /dev/null +++ b/lib/__tests__/jmap-list-thread-counts.test.ts @@ -0,0 +1,72 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { JMAPClient } from '../jmap/client'; + +// A folder page brings its conversation sizes along: Thread/get rides on the +// list request through a wildcard back-reference (/list/*/threadId), so the +// list does not need a second round trip (and a second render) for them. + +type MethodCall = [string, Record, string]; + +const session = { + capabilities: { 'urn:ietf:params:jmap:core': {} }, + accounts: { 'acct-1': { name: 'test', isPersonal: true, accountCapabilities: { 'urn:ietf:params:jmap:mail': {} } } }, + primaryAccounts: { 'urn:ietf:params:jmap:mail': 'acct-1' }, + apiUrl: 'https://mail.example.com/jmap/api', + downloadUrl: 'https://mail.example.com/jmap/download/{accountId}/{blobId}/{name}', + uploadUrl: 'https://mail.example.com/jmap/upload/{accountId}/', + eventSourceUrl: 'https://mail.example.com/jmap/eventsource', +}; + +const json = (body: unknown) => new Response(JSON.stringify(body), { status: 200, headers: { 'Content-Type': 'application/json' } }); +const page = [ + { id: 'e1', threadId: 't1', keywords: {}, receivedAt: '2026-01-02T00:00:00Z', mailboxIds: {}, size: 1, hasAttachment: false }, + { id: 'e2', threadId: 't2', keywords: {}, receivedAt: '2026-01-01T00:00:00Z', mailboxIds: {}, size: 1, hasAttachment: false }, +]; + +describe('JMAPClient.getEmails thread counts', () => { + let fetchSpy: ReturnType; + const sent: MethodCall[][] = []; + + beforeEach(() => { + sent.length = 0; + fetchSpy = vi.spyOn(globalThis, 'fetch'); + vi.spyOn(console, 'error').mockImplementation(() => {}); + }); + afterEach(() => vi.restoreAllMocks()); + + async function client(threadAnswer: unknown) { + fetchSpy.mockResolvedValueOnce(json(session)); + const c = JMAPClient.withBearer('https://mail.example.com', 'token', 'user@test.com'); + await c.connect(); + fetchSpy.mockReset(); + fetchSpy.mockImplementation((async (_url: string, init: RequestInit) => { + const calls = JSON.parse(init.body as string).methodCalls as MethodCall[]; + sent.push(calls); + if (calls[0][2] === 'asc') { + return json({ methodResponses: [['Email/query', { ids: [] }, 'asc'], ['Email/query', { ids: [] }, 'desc'], ['Email/get', { list: [] }, 'get']] }); + } + return json({ methodResponses: [['Email/query', { ids: ['e1', 'e2'], total: 2 }, '0'], ['Email/get', { state: 's1', list: page }, '1'], threadAnswer] }); + }) as never); + return c; + } + + it('asks for the threads of the page in the same request and returns their sizes', async () => { + const c = await client(['Thread/get', { list: [{ id: 't1', emailIds: ['e1', 'x', 'y'] }, { id: 't2', emailIds: ['e2'] }] }, '2']); + + const result = await c.getEmails('inbox', undefined, 50, 0); + + const list = sent.find((calls) => calls[0][2] === '0')!; + expect(list[2]).toEqual(['Thread/get', { + accountId: 'acct-1', + '#ids': { resultOf: '1', name: 'Email/get', path: '/list/*/threadId' }, + }, '2']); + expect(result.threadCounts).toEqual({ t1: 3, t2: 1 }); + }); + + it('returns no counts when the server could not answer the chained call', async () => { + const c = await client(['error', { type: 'invalidResultReference' }, '2']); + const result = await c.getEmails('inbox', undefined, 50, 0); + expect(result.emails.map((e) => e.id)).toEqual(['e1', 'e2']); + expect(result.threadCounts).toBeUndefined(); + }); +}); diff --git a/lib/jmap/client-interface.ts b/lib/jmap/client-interface.ts index f1883149a..160d300d7 100644 --- a/lib/jmap/client-interface.ts +++ b/lib/jmap/client-interface.ts @@ -195,7 +195,11 @@ export interface IJMAPClient { // server-side after pinned-first; see lib/message-list-order.ts. // `state` is the Email collection state the page was read at (RFC 8620 // §5.1), when the client reports it; the store uses it for Email/changes. - getEmails(mailboxId?: string, accountId?: string, limit?: number, position?: number, hasKeyword?: string, pinnedFirst?: boolean, extraFilter?: Record, order?: SortLevel[]): Promise<{ emails: Email[]; hasMore: boolean; total: number; state?: string }>; + /** + * A page of a view. `threadCounts` (message count per thread id) comes with + * the page when the server answers the Thread/get chained onto it. + */ + getEmails(mailboxId?: string, accountId?: string, limit?: number, position?: number, hasKeyword?: string, pinnedFirst?: boolean, extraFilter?: Record, order?: SortLevel[]): Promise<{ emails: Email[]; hasMore: boolean; total: number; state?: string; threadCounts?: Record }>; /** * Sort properties the account's server advertises for Email/query * (`emailQuerySortOptions` in the mail capability, RFC 8621 §1.3), or null diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index 958945b72..47cb56c2d 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -674,6 +674,20 @@ const FILE_NODE_QUERY_PAGE = 5000; /** Safety bound on how many FileNode ids one listing pages through. */ const FILE_NODE_MAX_IDS = 200_000; +/** + * Message counts per thread id from the Thread/get chained onto a list + * request; undefined when the server did not answer it (one that cannot + * resolve a wildcard back-reference fails just that call). + */ +function threadCountsFrom(methodResponse: unknown): Record | undefined { + if (!Array.isArray(methodResponse) || methodResponse[0] !== "Thread/get") return undefined; + const list = (methodResponse[1] as { list?: Thread[] } | undefined)?.list; + if (!Array.isArray(list)) return undefined; + const counts: Record = {}; + for (const thread of list) counts[thread.id] = thread.emailIds?.length ?? 0; + return counts; +} + function computeHasMore(position: number, emailCount: number, total: number, limit: number): boolean { if (total > 0) return (position + emailCount) < total; return emailCount === limit; @@ -1991,7 +2005,7 @@ export class JMAPClient implements IJMAPClient { // Throws when the page cannot be read. Answering with an empty page made a // failed refresh empty the list, and callers could not tell it from an // empty folder. - async getEmails(mailboxId?: string, accountId?: string, limit: number = 50, position: number = 0, hasKeyword?: string, pinnedFirst?: boolean, extraFilter?: Record, order: SortLevel[] = []): Promise<{ emails: Email[], hasMore: boolean, total: number, state?: string }> { + async getEmails(mailboxId?: string, accountId?: string, limit: number = 50, position: number = 0, hasKeyword?: string, pinnedFirst?: boolean, extraFilter?: Record, order: SortLevel[] = []): Promise<{ emails: Email[], hasMore: boolean, total: number, state?: string, threadCounts?: Record }> { const targetAccountId = accountId || this.accountId; const simple: { inMailbox?: string; hasKeyword?: string } = {}; if (mailboxId) { @@ -2030,6 +2044,13 @@ export class JMAPClient implements IJMAPClient { "#ids": { resultOf: "0", name: "Email/query", path: "/ids" }, properties: [...EMAIL_LIST_PROPERTIES], }, "1"], + // The page's conversation sizes, off the thread ids the Email/get + // returns (RFC 8620 §3.7 wildcard path): no second round trip, and the + // list paints with its counts instead of re-rendering for them. + ["Thread/get", { + accountId: targetAccountId, + "#ids": { resultOf: "1", name: "Email/get", path: "/list/*/threadId" }, + }, "2"], ]); let response = await query(built.sort); @@ -2064,7 +2085,7 @@ export class JMAPClient implements IJMAPClient { // The Email collection state this page was read at; the store keeps // it so a later push can be applied with Email/changes. const state = typeof getResponse.state === 'string' ? getResponse.state : undefined; - return { emails, hasMore, total, state }; + return { emails, hasMore, total, state, threadCounts: threadCountsFrom(response.methodResponses?.[2]) }; } throw new Error(methodErrorMessage(response, 'Failed to get emails')); diff --git a/stores/__tests__/email-store-thread-counts.test.ts b/stores/__tests__/email-store-thread-counts.test.ts new file mode 100644 index 000000000..3742e1b8e --- /dev/null +++ b/stores/__tests__/email-store-thread-counts.test.ts @@ -0,0 +1,63 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { useEmailStore } from '../email-store'; +import { useAuthStore } from '../auth-store'; +import { useSettingsStore } from '@/stores/settings-store'; +import { DEFAULT_SEARCH_FILTERS } from '@/lib/jmap/search-utils'; +import type { Email, Mailbox } from '@/lib/jmap/types'; +import type { IJMAPClient } from '@/lib/jmap/client-interface'; + +// A folder page that brings its thread sizes along (Thread/get chained onto +// the list request) is stored with them, and only a page without them sends +// the separate Thread/get. + +const row = (id: string, threadId: string): Email => ({ + id, threadId, mailboxIds: { inbox: true }, keywords: {}, subject: id, + receivedAt: '2026-10-01T00:00:00Z', from: [], to: [], preview: '', size: 1, hasAttachment: false, +} as Email); + +function makeClient(threadCounts?: Record) { + return { + getEmails: vi.fn(async () => ({ emails: [row('e1', 't1'), row('e2', 't2')], hasMore: false, total: 2, state: 's1', threadCounts })), + getThreads: vi.fn(async () => [{ id: 't1', emailIds: ['e1', 'a'] }, { id: 't2', emailIds: ['e2'] }]), + getAccountId: () => 'me', + } as unknown as IJMAPClient; +} + +describe('thread sizes with the folder page', () => { + beforeEach(() => { + useSettingsStore.setState({ emailsPerPage: 50 } as never); + useEmailStore.setState({ + isUnifiedView: false, unifiedRole: null, crossView: null, viewingAccountId: null, selectedKeyword: null, + selectedMailbox: 'inbox', searchQuery: '', searchFilters: { ...DEFAULT_SEARCH_FILTERS }, + emails: [], threadEmailCounts: new Map(), + mailboxes: [{ id: 'inbox', name: 'Inbox', role: 'inbox', totalEmails: 2, unreadEmails: 0 } as Mailbox], + accountMailboxes: {}, + }); + }); + + function connect(client: IJMAPClient) { + useAuthStore.setState({ + activeAccountId: 'account-a', client, + getClientForAccount: (id: string) => (id === 'account-a' ? client : undefined) as never, + } as never); + } + + it('stores the counts the page brought and sends no Thread/get of its own', async () => { + const client = makeClient({ t1: 4, t2: 1 }); + connect(client); + + await useEmailStore.getState().fetchEmails(client, 'inbox'); + + expect(Object.fromEntries(useEmailStore.getState().threadEmailCounts)).toEqual({ t1: 4, t2: 1 }); + expect(client.getThreads).not.toHaveBeenCalled(); + }); + + it('asks for the counts separately when the page came without them', async () => { + const client = makeClient(undefined); + connect(client); + + await useEmailStore.getState().fetchEmails(client, 'inbox'); + await vi.waitFor(() => expect(client.getThreads).toHaveBeenCalled()); + await vi.waitFor(() => expect(useEmailStore.getState().threadEmailCounts.get('t1')).toBe(2)); + }); +}); diff --git a/stores/email-store.ts b/stores/email-store.ts index 2800316f3..9d7fd91c0 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -977,6 +977,27 @@ export function resolveEmailBlobContext( */ let shownFolderList: { key: string; mailboxId: string; pageSize: number } | null = null; +/** + * The per-thread message counts to store once a page arrived: the counts that + * page brought along (Thread/get chained onto its query) under the keys the + * list groups threads by, merged over `current`. Empty when it brought none, + * so the caller knows to ask for them separately. + */ +function threadCountsPatch( + emails: Email[], + counts: Record | undefined, + current: Map, +): { threadEmailCounts: Map } | Record { + if (!counts) return {}; + const merged = new Map(current); + for (const email of emails) { + if (!email.threadId) continue; + const count = counts[email.threadId]; + if (count !== undefined) merged.set(threadKeyFor(email), count); + } + return { threadEmailCounts: merged }; +} + function captureEmailListView(): () => boolean { const view = useEmailStore.getState(); const activeAccountId = useAuthStore.getState().activeAccountId; @@ -2580,6 +2601,7 @@ export const useEmailStore = create((set, get) => ({ mailboxId: targetMailboxId, } : null; + const threadCounts = threadCountsPatch(enrichedEmails, result.threadCounts, get().threadEmailCounts); set({ emails: annotateScheduledEmails(enrichedEmails, get().scheduledSubmissionByEmailId), hasMoreEmails: result.hasMore, @@ -2590,12 +2612,14 @@ export const useEmailStore = create((set, get) => ({ threadEmailsCache: new Map(), expandedThreadIds: new Set(), isLoadingThread: null, + ...threadCounts, isLoading: false }); shownFolderList = { key: listKey, mailboxId: targetMailboxId, pageSize: emailsPerPage }; rememberFolderList(listKey, get(), emailsPerPage); - // Fetch full thread counts in the background (non-blocking) - void get().fetchThreadEmailCounts(client); + // Only a server that did not answer the chained Thread/get needs a + // request of its own for the conversation sizes. + if (!('threadEmailCounts' in threadCounts)) void get().fetchThreadEmailCounts(client); } catch (error) { if (!isCurrentView()) return; console.error('Failed to fetch emails:', error); @@ -2819,14 +2843,18 @@ export const useEmailStore = create((set, get) => ({ const enrichedNewEmails = await emailHooks.onEmailsFetched.transform(newEmails); if (!isCurrentView()) return; + const threadCounts = threadCountsPatch( + enrichedNewEmails, 'threadCounts' in result ? result.threadCounts : undefined, get().threadEmailCounts, + ); set({ emails: [...currentEmails, ...enrichedNewEmails], hasMoreEmails: result.hasMore, totalEmails: result.total, + ...threadCounts, isLoadingMore: false }); - // Fetch full thread counts for newly loaded threads in the background - if (enrichedNewEmails.length > 0) { + // Thread counts for the new rows, unless the page brought them along. + if (enrichedNewEmails.length > 0 && !('threadEmailCounts' in threadCounts)) { void get().fetchThreadEmailCounts(client); } } catch (error) {