diff --git a/src/api/__tests__/email-truncated-body.test.ts b/src/api/__tests__/email-truncated-body.test.ts index c0af1d54..4db8812d 100644 --- a/src/api/__tests__/email-truncated-body.test.ts +++ b/src/api/__tests__/email-truncated-body.test.ts @@ -10,7 +10,7 @@ vi.mock('../jmap-client', () => ({ })); import { jmapClient } from '../jmap-client'; -import { getFullEmail, getFullEmails, getThreadEmails } from '../email'; +import { getFullEmail, getFullEmails } from '../email'; const mockRequest = jmapClient.request as ReturnType; @@ -83,13 +83,10 @@ describe('truncated body refetch (#884)', () => { warn.mockRestore(); }); - it('refetches a thread in one batched call', async () => { + it('refetches every truncated body of a batch in one call', async () => { mockRequest .mockResolvedValueOnce({ - methodResponses: [ - ['Thread/get', { list: [{ id: 't1', emailIds: ['e1', 'e2', 'e3'] }] }, '0'], - ['Email/get', { list: [report('e1', true), report('e2', false), report('e3', true)] }, '1'], - ], + methodResponses: [['Email/get', { list: [report('e1', true), report('e2', false), report('e3', true)] }, '0']], }) .mockResolvedValueOnce({ methodResponses: [['Email/get', { @@ -97,7 +94,7 @@ describe('truncated body refetch (#884)', () => { }, '0']], }); - const emails = await getThreadEmails('t1'); + const emails = await getFullEmails(['e1', 'e2', 'e3']); expect(mockRequest).toHaveBeenCalledTimes(2); expect(mockRequest.mock.calls[1][0][0][1].ids).toEqual(['e1', 'e3']); diff --git a/src/api/__tests__/email-viewer-fetch.test.ts b/src/api/__tests__/email-viewer-fetch.test.ts index 4e01cab0..d82fa244 100644 --- a/src/api/__tests__/email-viewer-fetch.test.ts +++ b/src/api/__tests__/email-viewer-fetch.test.ts @@ -10,7 +10,7 @@ vi.mock('../jmap-client', () => ({ })); import { jmapClient } from '../jmap-client'; -import { getEmailFlags, getFullEmailsWithState, getThreadHeaders } from '../email'; +import { getEmailFlags, getFullEmailsWithState, getThreadsHeaders } from '../email'; const mockRequest = jmapClient.request as ReturnType; @@ -19,21 +19,30 @@ beforeEach(() => { }); describe('viewer fetches', () => { - it('lists a conversation from headers only, in thread order, without bodies', async () => { + it('lists conversations from headers only, in thread order, without bodies, in one request', async () => { mockRequest.mockResolvedValue({ methodResponses: [ - ['Thread/get', { list: [{ id: 't1', emailIds: ['a', 'gone', 'b'] }] }, '0'], - ['Email/get', { state: 's9', list: [{ id: 'b', subject: 'Re: hi' }, { id: 'a', subject: 'hi' }] }, '1'], + ['Thread/get', { + list: [{ id: 't1', emailIds: ['a', 'gone', 'b'] }, { id: 't2', emailIds: ['c'] }], + notFound: ['t3'], + }, '0'], + ['Email/get', { + state: 's9', + list: [{ id: 'b', subject: 'Re: hi' }, { id: 'c', subject: 'other' }, { id: 'a', subject: 'hi' }], + }, '1'], ], }); - const res = await getThreadHeaders('t1', 'group-1'); + const res = await getThreadsHeaders(['t1', 't2', 't3', 't1'], 'group-1'); - expect(res.emailIds).toEqual(['a', 'b']); - expect(res.list.map((e) => e.id)).toEqual(['a', 'b']); + expect(res.threads.t1.emailIds).toEqual(['a', 'b']); + expect(res.threads.t1.list.map((e) => e.id)).toEqual(['a', 'b']); + expect(res.threads.t2.list.map((e) => e.id)).toEqual(['c']); + expect(res.notFound).toEqual(['t3']); expect(res.state).toBe('s9'); + expect(mockRequest).toHaveBeenCalledTimes(1); const [calls] = mockRequest.mock.calls[0]; - expect(calls[0]).toEqual(['Thread/get', { accountId: 'group-1', ids: ['t1'] }, '0']); + expect(calls[0]).toEqual(['Thread/get', { accountId: 'group-1', ids: ['t1', 't2', 't3'] }, '0']); const args = calls[1][1]; expect(args.accountId).toBe('group-1'); expect(args['#ids']).toEqual({ resultOf: '0', name: 'Thread/get', path: '/list/*/emailIds' }); diff --git a/src/api/__tests__/live-stalwart.test.ts b/src/api/__tests__/live-stalwart.test.ts index 14bc435c..dbdb939c 100644 --- a/src/api/__tests__/live-stalwart.test.ts +++ b/src/api/__tests__/live-stalwart.test.ts @@ -20,7 +20,7 @@ import { queryEmails, queryEmailPage, getFullEmails, - getThreadEmails, + getThreadsHeaders, createDraft, sendEmail, markAsSpam, @@ -152,7 +152,8 @@ live('live Stalwart', () => { const { ids } = await queryEmails(inbox, { limit: 20 }); const emails = await getFullEmails(ids); const reply = emails.find((e) => (e.subject ?? '').startsWith('Re: '))!; - const thread = await getThreadEmails(reply.threadId); + const { threads } = await getThreadsHeaders([reply.threadId]); + const thread = threads[reply.threadId].list; expect(thread.length).toBeGreaterThanOrEqual(2); const times = thread.map((e) => new Date(e.receivedAt).getTime()); expect([...times].sort((a, b) => a - b)).toEqual(times); diff --git a/src/api/email.ts b/src/api/email.ts index 37eb0347..a02943da 100644 --- a/src/api/email.ts +++ b/src/api/email.ts @@ -913,33 +913,44 @@ export async function getEmailFlags( // What a collapsed conversation card shows; `sentAt` is the date it prints. export const THREAD_HEADER_PROPERTIES = [...EMAIL_LIST_PROPERTIES, 'sentAt']; +/** A conversation's members, oldest first, without bodies. */ +export interface ThreadHeaders { + emailIds: string[]; + list: Email[]; +} + /** - * The messages of a thread without their bodies, oldest first, in one - * back-referenced Thread/get → Email/get. The viewer lists a conversation from - * these and downloads bodies only for the cards it opens. + * The messages of several threads without their bodies, oldest first, in one + * back-referenced Thread/get → Email/get. The viewer lists conversations from + * these and downloads bodies only for the cards it opens. Threads the server + * does not know are in `notFound`. */ -export async function getThreadHeaders( - threadId: string, +export async function getThreadsHeaders( + threadIds: string[], accountIdOverride?: string, -): Promise<{ emailIds: string[]; list: Email[]; state?: string }> { +): Promise<{ threads: Record; notFound: string[]; state?: string }> { const accountId = accountIdOverride ?? jmapClient.accountId; + const ids = Array.from(new Set(threadIds)); const res = await jmapClient.request([ - ['Thread/get', { accountId, ids: [threadId] }, '0'], + ['Thread/get', { accountId, ids }, '0'], ['Email/get', { accountId, '#ids': { resultOf: '0', name: 'Thread/get', path: '/list/*/emailIds' }, properties: THREAD_HEADER_PROPERTIES, }, '1'], ]); - const thread = (requireMethodResult(res, '0', 'Thread/get').list as Thread[] | undefined)?.[0]; - if (!thread) throw new Error(`Thread ${threadId} not found`); + const found = (requireMethodResult(res, '0', 'Thread/get').list as Thread[] | undefined) ?? []; const body = requireMethodResult(res, '1', 'Email/get'); const byId = new Map(((body.list as Email[]) ?? []).map((e) => [e.id, e])); - // Members the server did not return (no access to them) are left out. - const emailIds = thread.emailIds.filter((id) => byId.has(id)); + const threads: Record = {}; + for (const thread of found) { + // Members the server did not return (no access to them) are left out. + const emailIds = thread.emailIds.filter((id) => byId.has(id)); + threads[thread.id] = { emailIds, list: emailIds.map((id) => byId.get(id)!) }; + } return { - emailIds, - list: emailIds.map((id) => byId.get(id)!), + threads, + notFound: ids.filter((id) => !threads[id]), state: body.state as string | undefined, }; } @@ -996,28 +1007,6 @@ export async function getThreads(threadIds: string[], accountIdOverride?: string return out; } -/** - * Every message of a thread with full bodies, oldest first, via a - * back-referenced Thread/get → Email/get. Powers the conversation view. - */ -export async function getThreadEmails(threadId: string, accountIdOverride?: string): Promise { - const accountId = accountIdOverride ?? jmapClient.accountId; - const res = await jmapClient.request([ - ['Thread/get', { accountId, ids: [threadId] }, '0'], - ['Email/get', { - accountId, - '#ids': { resultOf: '0', name: 'Thread/get', path: '/list/*/emailIds' }, - ...FULL_BODY_ARGS, - }, '1'], - ]); - const thread = (requireMethodResult(res, '0', 'Thread/get').list as Thread[] | undefined)?.[0]; - const emails = (requireMethodResult(res, '1', 'Email/get').list as Email[]) ?? []; - await refetchTruncatedBodyValues(emails, accountId); - if (!thread) return emails; - const order = new Map(thread.emailIds.map((id, i) => [id, i])); - return emails.sort((a, b) => (order.get(a.id) ?? 0) - (order.get(b.id) ?? 0)); -} - // One `keywords/` pointer per changed keyword. Never send the whole // `keywords` map on update: the server replaces it, erasing every keyword the // local copy didn't have (a star or tag set elsewhere, or all of them on a diff --git a/src/lib/__tests__/email-detail-cache.test.ts b/src/lib/__tests__/email-detail-cache.test.ts index a4ec7790..414cffbb 100644 --- a/src/lib/__tests__/email-detail-cache.test.ts +++ b/src/lib/__tests__/email-detail-cache.test.ts @@ -7,11 +7,11 @@ vi.mock('../../api/jmap-client', () => ({ vi.mock('../../api/email', () => ({ getFullEmailsWithState: vi.fn(), getEmailFlags: vi.fn(), - getThreadHeaders: vi.fn(), + getThreadsHeaders: vi.fn(), })); import type { Email } from '../../api/types'; -import { getEmailFlags, getFullEmailsWithState, getThreadHeaders } from '../../api/email'; +import { getEmailFlags, getFullEmailsWithState, getThreadsHeaders } from '../../api/email'; import { useOfflineCacheStore } from '../../stores/offline-cache-store'; import { dispatchStateChange } from '../state-change-bus'; import { @@ -21,7 +21,12 @@ import { const fullGet = getFullEmailsWithState as ReturnType; const flagsGet = getEmailFlags as ReturnType; -const threadGet = getThreadHeaders as ReturnType; +const threadGet = getThreadsHeaders as ReturnType; + +// A Thread/get response carrying one conversation. +function threadResponse(threadId: string, list: Array & { id: string }>, state = 's1') { + return { threads: { [threadId]: { emailIds: list.map((e) => e.id), list } }, notFound: [], state }; +} function full(id: string, keywords: Record = {}): Email { return { @@ -234,7 +239,7 @@ describe('loading a conversation', () => { const headers = (ids: string[]) => ids.map((id) => ({ id, threadId: 't1', keywords: {}, mailboxIds: {} })); it('fetches member headers once for concurrent loads and keeps them', async () => { - threadGet.mockResolvedValue({ emailIds: ['m1', 'm2'], list: headers(['m1', 'm2']), state: 's1' }); + threadGet.mockResolvedValue(threadResponse('t1', headers(['m1', 'm2']))); const [a, b] = await Promise.all([loadThread('t1'), loadThread('t1')]); @@ -247,7 +252,7 @@ describe('loading a conversation', () => { }); it('refetches a conversation once the state moved on, keeping it if that fails', async () => { - threadGet.mockResolvedValueOnce({ emailIds: ['m1'], list: headers(['m1']), state: 's1' }); + threadGet.mockResolvedValueOnce(threadResponse('t1', headers(['m1']))); await loadThread('t1'); noteEmailState(undefined, 's2'); threadGet.mockRejectedValueOnce(new Error('offline')); @@ -261,11 +266,9 @@ describe('loading a conversation', () => { it('brings a member copy held from an earlier open up to date with the headers', async () => { fullGet.mockResolvedValue(fullResponse([full('m1', { $seen: true })], 's1')); const held = await loadDetail('m1'); - threadGet.mockResolvedValue({ - emailIds: ['m1'], - list: [{ id: 'm1', threadId: 't1', keywords: {}, mailboxIds: { inbox: true } }], - state: 's2', - }); + threadGet.mockResolvedValue( + threadResponse('t1', [{ id: 'm1', threadId: 't1', keywords: {}, mailboxIds: { inbox: true } }], 's2'), + ); await loadThread('t1'); @@ -273,8 +276,41 @@ describe('loading a conversation', () => { expect(peekDetail('m1')?.bodyValues).toBe(held.bodyValues); }); + it('asks for conversations wanted in the same tick in one request', async () => { + threadGet.mockResolvedValue({ + threads: { + tA: { emailIds: ['a1', 'a2'], list: headers(['a1', 'a2']) }, + tB: { emailIds: ['b1'], list: headers(['b1']) }, + }, + notFound: ['tC'], + state: 's1', + }); + + const [a, b, c] = await Promise.allSettled([loadThread('tA'), loadThread('tB'), loadThread('tC')]); + + expect(threadGet).toHaveBeenCalledTimes(1); + expect(threadGet).toHaveBeenCalledWith(['tA', 'tB', 'tC'], undefined); + expect(a.status === 'fulfilled' && a.value.ids).toEqual(['a1', 'a2']); + expect(b.status === 'fulfilled' && b.value.ids).toEqual(['b1']); + expect(c.status).toBe('rejected'); + }); + + it('asks each account separately', async () => { + threadGet.mockImplementation(async (ids: string[]) => ({ + threads: Object.fromEntries(ids.map((id) => [id, { emailIds: ['m'], list: headers(['m']) }])), + notFound: [], + state: 's1', + })); + + await Promise.all([loadThread('t1'), loadThread('t1', 'group')]); + + expect(threadGet).toHaveBeenCalledTimes(2); + expect(threadGet).toHaveBeenCalledWith(['t1'], undefined); + expect(threadGet).toHaveBeenCalledWith(['t1'], 'group'); + }); + it('applies local keyword changes to the conversation headers too', async () => { - threadGet.mockResolvedValue({ emailIds: ['m1'], list: headers(['m1']), state: 's1' }); + threadGet.mockResolvedValue(threadResponse('t1', headers(['m1']))); await loadThread('t1'); patchDetail('m1', undefined, { keywords: { $seen: true } }); @@ -286,7 +322,7 @@ describe('loading a conversation', () => { describe('prefetchMessage', () => { it('starts the body and the conversation so the viewer joins them', async () => { fullGet.mockResolvedValue(fullResponse([full('e1')], 's1')); - threadGet.mockResolvedValue({ emailIds: ['e1'], list: headers1(), state: 's1' }); + threadGet.mockResolvedValue(threadResponse('t-e1', headers1())); prefetchMessage({ id: 'e1', threadId: 't-e1', keywords: {}, mailboxIds: { inbox: true } }); await Promise.all([loadDetail('e1'), loadThread('t-e1')]); diff --git a/src/lib/email-detail-cache.ts b/src/lib/email-detail-cache.ts index 8a41dd0f..aea6259c 100644 --- a/src/lib/email-detail-cache.ts +++ b/src/lib/email-detail-cache.ts @@ -14,7 +14,7 @@ import { AppState } from 'react-native'; import type { Email } from '../api/types'; import { jmapClient } from '../api/jmap-client'; -import { getEmailFlags, getFullEmailsWithState, getThreadHeaders } from '../api/email'; +import { getEmailFlags, getFullEmailsWithState, getThreadsHeaders } from '../api/email'; import { useOfflineCacheStore } from '../stores/offline-cache-store'; import { useSettingsStore } from '../stores/settings-store'; import { onStateChangeType } from './state-change-bus'; @@ -214,6 +214,7 @@ export function clearEmailDetailCache(): void { rows.clear(); pendingDetails.clear(); pendingThreads.clear(); + threadBatches.clear(); latestStates.clear(); detailChars = 0; } @@ -421,7 +422,9 @@ export function loadDetail(id: string, accountId?: string, hint?: FlagsHint): Pr /** * A conversation's members without bodies, cache first. A held conversation * resolves at once while the account's Email state says it is current; it is - * otherwise refetched (one request) and, if that fails, still returned. + * otherwise refetched and, if that fails, still returned. Conversations asked + * for in the same tick (the pages on both sides of the open one) share one + * `Thread/get` request. */ export function loadThread(threadId: string, accountId?: string): Promise { wire(); @@ -431,20 +434,60 @@ export function loadThread(threadId: string, accountId?: string): Promise(); + pendingThreads.set(key, d.promise); + const done = () => { if (pendingThreads.get(key) === d.promise) pendingThreads.delete(key); }; + d.promise.then(done, done); + + const batchKey = accountKey(accountId); + let batch = threadBatches.get(batchKey); + if (!batch) { + batch = { accountId, waiting: new Map() }; + threadBatches.set(batchKey, batch); + const queued = batch; + void Promise.resolve().then(() => { + if (threadBatches.get(batchKey) === queued) threadBatches.delete(batchKey); + void fetchThreads(queued); + }); + } + batch.waiting.set(threadId, d); + return d.promise; +} + +interface ThreadBatch { + accountId: string | undefined; + waiting: Map>; +} + +// Conversations asked for in the current tick, per account, not yet requested. +const threadBatches = new Map(); + +async function fetchThreads({ accountId, waiting }: ThreadBatch): Promise { const gen = generation; - const p = getThreadHeaders(threadId, accountId) - .then((res) => { - if (gen !== generation) throw new Error('cleared'); - noteEmailState(accountId, res.state); + // What was held when the load started: shown again if the refetch fails. + const fallback = (threadId: string) => (gen === generation ? threads.get(keyOf(threadId, accountId)) : undefined); + try { + const res = await getThreadsHeaders(Array.from(waiting.keys()), accountId); + if (gen !== generation) throw new Error('cleared'); + noteEmailState(accountId, res.state); + for (const [threadId, d] of waiting) { + const found = res.threads[threadId]; + if (!found) { + const held = fallback(threadId); + if (held) d.resolve(held); + else d.reject(new Error(`Thread ${threadId} not found`)); + continue; + } const entry: ThreadEntry = { - ids: res.emailIds, - headers: new Map(res.list.map((e) => [e.id, e])), + ids: found.emailIds, + headers: new Map(found.list.map((e) => [e.id, e])), state: res.state, }; - putThread(key, entry); + putThread(keyOf(threadId, accountId), entry); // The headers were just read: a member's copy held from an earlier // open takes their keywords and folders, which may have changed since. - for (const header of res.list) { + for (const header of found.list) { const memberKey = keyOf(header.id, accountId); const held = details.get(memberKey); if (!held) continue; @@ -457,16 +500,16 @@ export function loadThread(threadId: string, accountId?: string): Promise { - if (held && gen === generation) return held as ThreadView; - throw err; - }) - .finally(() => { if (pendingThreads.get(key) === p) pendingThreads.delete(key); }); - pendingThreads.set(key, p); - return p; + d.resolve(entry); + } + emit(); + } catch (err) { + for (const [threadId, d] of waiting) { + const held = fallback(threadId); + if (held) d.resolve(held); + else d.reject(err); + } + } } /** Keep list rows the viewer may page over; see {@link peekRow}. */ diff --git a/src/screens/EmailThreadScreen.tsx b/src/screens/EmailThreadScreen.tsx index 2e46efa2..f510836f 100644 --- a/src/screens/EmailThreadScreen.tsx +++ b/src/screens/EmailThreadScreen.tsx @@ -207,7 +207,8 @@ function EmailViewer({ route, navigation }: Props) { }, [ownerAccountId]); // The conversation's members without bodies; the cards fetch bodies only - // when opened (ensureDetails). + // when opened (ensureDetails). The neighbouring pages ask in the same tick + // once they are let go, which the cache sends as one Thread/get. const ensureThread = React.useCallback((threadId: string): void => { void loadThread(threadId, ownerAccountId).catch((err) => { console.warn('[thread] fetch failed', err);