From a69d8a71e93d2250f3adf4cd40769a3b3c901560 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 23 Sep 2026 18:09:35 +0200 Subject: [PATCH] fix: load attachment chips lazily instead of in the message list #1089 --- .../email/__tests__/attachment-chips.test.tsx | 48 +++++- components/email/attachment-chips.tsx | 37 ++++- components/email/email-list.tsx | 4 + components/email/thread-list-item.tsx | 19 ++- components/mail/mail-app.tsx | 37 ++++- lib/__tests__/list-attachments.test.ts | 152 ++++++++++++++++++ lib/jmap/client-interface.ts | 8 +- lib/jmap/client.ts | 43 ++++- lib/list-attachments.ts | 141 ++++++++++++++++ 9 files changed, 473 insertions(+), 16 deletions(-) create mode 100644 lib/__tests__/list-attachments.test.ts create mode 100644 lib/list-attachments.ts diff --git a/components/email/__tests__/attachment-chips.test.tsx b/components/email/__tests__/attachment-chips.test.tsx index f0d8dab8a..535d3585d 100644 --- a/components/email/__tests__/attachment-chips.test.tsx +++ b/components/email/__tests__/attachment-chips.test.tsx @@ -1,6 +1,7 @@ -import { describe, it, expect } from "vitest"; -import { realAttachments } from "../attachment-chips"; -import type { Attachment } from "@/lib/jmap/types"; +import { describe, it, expect, vi } from "vitest"; +import { render, screen, act } from "@testing-library/react"; +import { realAttachments, ListAttachmentChips } from "../attachment-chips"; +import type { Attachment, Email } from "@/lib/jmap/types"; function att(over: Partial): Attachment { return { partId: "1", blobId: "b1", size: 100, type: "application/pdf", name: "doc.pdf", ...over }; @@ -40,3 +41,44 @@ describe("realAttachments", () => { expect(realAttachments([a, b])).toEqual([a, b]); }); }); + +// #1089: list requests no longer carry `attachments`; rows load their own. +describe("ListAttachmentChips", () => { + const row = (over: Partial) => ({ id: "m1", hasAttachment: true, ...over }) as Email; + + it("uses attachments the email already carries without loading", () => { + const load = vi.fn(() => () => {}); + render(); + expect(screen.getByTitle("have.pdf")).toBeTruthy(); + expect(load).not.toHaveBeenCalled(); + }); + + it("does not load for a row without a paperclip", () => { + const load = vi.fn(() => () => {}); + render(); + expect(load).not.toHaveBeenCalled(); + }); + + it("loads the parts of a row with a paperclip and shows them", () => { + let deliver!: (a: Attachment[]) => void; + const cancel = vi.fn(); + const load = vi.fn((_email: Email, onLoad: (a: Attachment[]) => void) => { deliver = onLoad; return cancel; }); + const { unmount } = render(); + expect(load).toHaveBeenCalledTimes(1); + expect(screen.queryByRole("button")).toBeNull(); + act(() => deliver([att({ name: "late.pdf" })])); + expect(screen.getByTitle("late.pdf")).toBeTruthy(); + unmount(); + expect(cancel).toHaveBeenCalled(); + }); + + it("does not show one message's parts on another", () => { + let deliver!: (a: Attachment[]) => void; + const load = vi.fn((_email: Email, onLoad: (a: Attachment[]) => void) => { deliver = onLoad; return () => {}; }); + const { rerender } = render(); + act(() => deliver([att({ name: "first.pdf" })])); + rerender(); + expect(screen.queryByTitle("first.pdf")).toBeNull(); + expect(load).toHaveBeenCalledTimes(2); + }); +}); diff --git a/components/email/attachment-chips.tsx b/components/email/attachment-chips.tsx index 0f4a1c959..c40dda359 100644 --- a/components/email/attachment-chips.tsx +++ b/components/email/attachment-chips.tsx @@ -1,8 +1,10 @@ "use client"; +import { useEffect, useState } from "react"; import { FileText, FileSpreadsheet, FileImage, FileArchive, File as FileIcon } from "@/components/icons"; import { cn } from "@/lib/utils"; -import type { Attachment } from "@/lib/jmap/types"; +import type { Attachment, Email } from "@/lib/jmap/types"; +import type { LoadListAttachments } from "@/lib/list-attachments"; /** * A message can carry a dozen inline images — signature logos, tracking @@ -83,3 +85,36 @@ export function AttachmentChips({ attachments, onOpen, max = 2, className }: Att ); } + +/** + * Attachment parts for a list row. List requests no longer carry + * `attachments` (#1089), so a row with a paperclip loads its own once it is + * mounted; an email that already has them (thread expansion, demo data) is + * used as is. + */ +export function useListAttachments(email: Email, load?: LoadListAttachments): Attachment[] | undefined { + const [loaded, setLoaded] = useState<{ id: string; attachments: Attachment[] } | null>(null); + const needsLoad = !!load && !!email.hasAttachment && !email.attachments; + + useEffect(() => { + if (!needsLoad || !load) return; + const id = email.id; + return load(email, (attachments) => setLoaded({ id, attachments })); + // The row's email object is replaced on every keyword change; only a + // different message needs a different answer. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [needsLoad, load, email.id]); + + if (email.attachments) return email.attachments; + return loaded?.id === email.id ? loaded.attachments : undefined; +} + +interface ListAttachmentChipsProps extends Omit { + email: Email; + load?: LoadListAttachments; +} + +export function ListAttachmentChips({ email, load, ...chipProps }: ListAttachmentChipsProps) { + const attachments = useListAttachments(email, load); + return ; +} diff --git a/components/email/email-list.tsx b/components/email/email-list.tsx index 3fd133b7d..87d73e2f0 100644 --- a/components/email/email-list.tsx +++ b/components/email/email-list.tsx @@ -3,6 +3,7 @@ import { Email, ThreadGroup } from "@/lib/jmap/types"; import { ThreadListItem } from "./thread-list-item"; import type { Attachment } from "@/lib/jmap/types"; +import type { LoadListAttachments } from "@/lib/list-attachments"; import { EmailContextMenu } from "./email-context-menu"; import { cn } from "@/lib/utils"; import { Trash2, Mail, MailX, MailOpen, Loader2, SearchX, AlertTriangle, CalendarClock, ShieldCheck } from "@/components/icons"; @@ -48,6 +49,7 @@ interface EmailListProps { onMarkAsSpam?: (email: Email) => void; onUndoSpam?: (email: Email) => void; onOpenAttachment?: (email: Email, attachment: Attachment) => void; + loadAttachments?: LoadListAttachments; onEditDraft?: (email: Email) => void; isScheduledView?: boolean; onLoadMoreScheduled?: () => void; @@ -78,6 +80,7 @@ export function EmailList({ onMarkAsSpam, onUndoSpam, onOpenAttachment, + loadAttachments, onMoveToMailbox, onEditDraft, isScheduledView = false, @@ -593,6 +596,7 @@ export function EmailList({ onMarkAsSpam={onMarkAsSpam ? (email) => onMarkAsSpam(email) : undefined} onUndoSpam={onUndoSpam ? (email) => onUndoSpam(email) : undefined} onOpenAttachment={onOpenAttachment} + loadAttachments={loadAttachments} /> ); diff --git a/components/email/thread-list-item.tsx b/components/email/thread-list-item.tsx index b3e045e91..d3fef0ea1 100644 --- a/components/email/thread-list-item.tsx +++ b/components/email/thread-list-item.tsx @@ -4,8 +4,9 @@ import React, { useCallback } from "react"; import { formatDate, formatDateTime, stripInvisibleLeading } from "@/lib/utils"; import { Email, ThreadGroup } from "@/lib/jmap/types"; import { cn } from "@/lib/utils"; -import { AttachmentChips } from "./attachment-chips"; +import { ListAttachmentChips } from "./attachment-chips"; import type { Attachment } from "@/lib/jmap/types"; +import type { LoadListAttachments } from "@/lib/list-attachments"; import { SelectableAvatar } from "@/components/email/selectable-avatar"; import { Paperclip, Star, Pin, Circle, ChevronRight, ChevronDown, Loader2, MessageSquare, CheckSquare, Square, Reply, Forward, CalendarClock, Folder, Archive, Trash2, MailOpen, ShieldAlert } from "@/components/icons"; import type { AppIcon } from "@/components/icons"; @@ -125,6 +126,7 @@ interface ThreadListItemProps { onMarkAsSpam?: (email: Email) => void; onUndoSpam?: (email: Email) => void; onOpenAttachment?: (email: Email, attachment: Attachment) => void; + loadAttachments?: LoadListAttachments; } interface SingleEmailItemProps { @@ -143,6 +145,7 @@ interface SingleEmailItemProps { onMarkAsSpam?: () => void; onUndoSpam?: () => void; onOpenAttachment?: (attachment: Attachment) => void; + loadAttachments?: LoadListAttachments; } // Visual + behaviour metadata for each mobile swipe action. Keyed by the @@ -156,7 +159,7 @@ const SWIPE_ACTION_META: Record, { icon: AppIcon; b }; const SingleEmailItem = React.forwardRef( - function SingleEmailItem({ email, selected, onClick, onDoubleClick, onContextMenu, showPreview, rowTint, onToggleStar, onMarkAsRead, onDelete, onArchive, onSetTag, onMarkAsSpam, onUndoSpam, onOpenAttachment }, ref) { + function SingleEmailItem({ email, selected, onClick, onDoubleClick, onContextMenu, showPreview, rowTint, onToggleStar, onMarkAsRead, onDelete, onArchive, onSetTag, onMarkAsSpam, onUndoSpam, onOpenAttachment, loadAttachments }, ref) { const t = useTranslations('email_viewer'); const tBatch = useTranslations('email_list.batch_actions'); const tStatus = useTranslations('email_list'); @@ -563,8 +566,9 @@ const SingleEmailItem = React.forwardRef(

)} {onOpenAttachment && ( - @@ -598,6 +602,7 @@ const SingleEmailItem = React.forwardRef( export const ThreadListItem = React.forwardRef( function ThreadListItem({ onOpenAttachment, + loadAttachments, thread, isExpanded, selectedEmailId, @@ -709,6 +714,7 @@ export const ThreadListItem = React.forwardRef onMarkAsSpam(latestEmail) : undefined} onUndoSpam={onUndoSpam ? () => onUndoSpam(latestEmail) : undefined} onOpenAttachment={onOpenAttachment ? (a) => onOpenAttachment(latestEmail, a) : undefined} + loadAttachments={loadAttachments} /> ); } @@ -1065,8 +1071,9 @@ export const ThreadListItem = React.forwardRef )} {onOpenAttachment && ( - onOpenAttachment(latestEmail, a)} className="mt-1.5" /> diff --git a/components/mail/mail-app.tsx b/components/mail/mail-app.tsx index c7fa32100..056e0c569 100644 --- a/components/mail/mail-app.tsx +++ b/components/mail/mail-app.tsx @@ -1,6 +1,6 @@ "use client"; -import { useEffect, useState, useRef, useMemo, useCallback, useId } from "react"; +import { useEffect, useLayoutEffect, useState, useRef, useMemo, useCallback, useId } from "react"; import { usePathname } from "next/navigation"; import { useTranslations } from "next-intl"; import { Sidebar } from "@/components/layout/sidebar"; @@ -72,6 +72,7 @@ import { isFilterEmpty, activeFilterCount } from "@/lib/jmap/search-utils"; import { SearchBox, type ContactSearchField } from "@/components/search/search-box"; import type { ContactSuggestion } from "@/lib/search-suggestions"; import type { Attachment } from "@/lib/jmap/types"; +import { requestListAttachments, type ListAttachmentSource, type LoadListAttachments } from "@/lib/list-attachments"; import { useSearchHistoryStore } from "@/stores/search-history-store"; import { WelcomeBanner } from "@/components/ui/welcome-banner"; import { NavigationRail } from "@/components/layout/navigation-rail"; @@ -3055,6 +3056,39 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { } }, [resolveBlobSource]); + // Where a list row's attachment parts are fetched from: the same routing + // the viewer uses to open that row (source stamps first, then the account + // being browsed, then a shared folder's owner). Read through a ref so the + // loader stays stable and rows do not re-request on every folder count + // change. A layout effect, because the rows ask from their own (passive) + // effects in the same commit. + const listRowSourceRef = useRef<(email: Email) => { source: ListAttachmentSource; accountId?: string } | null>(() => null); + useLayoutEffect(() => { + listRowSourceRef.current = (email) => { + const auth = useAuthStore.getState(); + if (email.sourceClientAccountId) { + const source = auth.getClientForAccount(email.sourceClientAccountId); + return source ? { source, accountId: email.sourceAccountId } : null; + } + const source = (viewingAccountId ? auth.getClientForAccount(viewingAccountId) : undefined) ?? client; + if (!source) return null; + const accountId = email.sourceAccountId + ?? resolveUnstampedEmailAccountId({ + mailboxes: viewMailboxes, + selectedMailbox, + searchActive: !!searchQuery || !isFilterEmpty(searchFilters), + searchMailboxId, + }); + return { source, accountId }; + }; + }, [client, viewingAccountId, viewMailboxes, selectedMailbox, searchQuery, searchFilters, searchMailboxId]); + + const loadListAttachments = useCallback((email, onLoad) => { + const target = listRowSourceRef.current(email); + if (!target) return () => {}; + return requestListAttachments(target.source, target.accountId, email.id, onLoad); + }, []); + const handleDownloadAttachment = async (blobId: string, name: string, type?: string, forceDownload?: boolean) => { const { blobClient, accountId, clientAccountId } = resolveBlobSource(selectedEmail); if (!blobClient) return; @@ -4111,6 +4145,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { await handleUndoSpam(email); }} onOpenAttachment={handleOpenListAttachment} + loadAttachments={loadListAttachments} onEditDraft={(email) => { handleEditDraft(email); }} diff --git a/lib/__tests__/list-attachments.test.ts b/lib/__tests__/list-attachments.test.ts new file mode 100644 index 000000000..0ca055298 --- /dev/null +++ b/lib/__tests__/list-attachments.test.ts @@ -0,0 +1,152 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { JMAPClient } from '../jmap/client'; +import { requestListAttachments, type ListAttachmentSource } from '../list-attachments'; +import type { Attachment } from '../jmap/types'; + +type Call = [string, Record, string]; + +const pdf = (blobId: string): Attachment => ({ partId: '2', blobId, size: 10, name: `${blobId}.pdf`, type: 'application/pdf' }); + +function recordingClient(maxObjectsInGet = 50) { + const client = new JMAPClient('https://jmap.example.com', 'user@example.com', 'pass'); + Object.assign(client, { accountId: 'me', capabilities: { 'urn:ietf:params:jmap:core': { maxObjectsInGet } } }); + const gets: Record[] = []; + vi.spyOn(client as unknown as { request: (c: Call[]) => Promise }, 'request') + .mockImplementation(async (calls: Call[]) => ({ + methodResponses: calls.map(([method, args, id]) => { + if (method === 'Email/query') return ['Email/query', { ids: ['m1'], total: 1, position: 0 }, id]; + if (method === 'Email/get') { + gets.push(args); + const ids = (args.ids as string[] | undefined) ?? ['m1']; + return ['Email/get', { + list: ids.filter(m => m !== 'gone').map(m => ({ id: m, attachments: [pdf(`b-${m}`)] })), + }, id]; + } + return [method, {}, id]; + }), + })); + return { client, gets }; +} + +// #1089: asking for `attachments` makes Stalwart read every message's raw +// blob, so a list page must not ask for it. +describe('list requests', () => { + it('do not ask for attachments but keep blobId for drag-out', async () => { + const { client, gets } = recordingClient(); + await client.getSomeEmails(['m1']); + await client.getEmails('inbox'); + expect(gets.length).toBeGreaterThanOrEqual(2); + for (const args of gets) { + expect(args.properties).not.toContain('attachments'); + expect(args.properties).toContain('blobId'); + expect(args.properties).toContain('hasAttachment'); + } + }); +}); + +describe('getEmailAttachments', () => { + it('asks only for attachment parts, in server-sized batches', async () => { + const { client, gets } = recordingClient(2); + const found = await client.getEmailAttachments(['a', 'b', 'c'], 'owner'); + expect(gets.map(g => g.ids)).toEqual([['a', 'b'], ['c']]); + expect(gets[0]).toMatchObject({ accountId: 'owner', properties: ['id', 'attachments'] }); + expect(gets[0].bodyProperties).toEqual(expect.arrayContaining(['partId', 'blobId', 'name', 'type', 'cid', 'disposition'])); + expect(found.get('c')).toEqual([pdf('b-c')]); + }); + + it('leaves out ids the server did not return', async () => { + const { client } = recordingClient(); + const found = await client.getEmailAttachments(['a', 'gone']); + expect([...found.keys()]).toEqual(['a']); + }); +}); + +describe('requestListAttachments', () => { + beforeEach(() => { vi.useFakeTimers(); }); + afterEach(() => { vi.useRealTimers(); }); + + function fakeSource(fail = false) { + const getEmailAttachments = vi.fn(async (ids: string[]) => { + if (fail) throw new Error('boom'); + return new Map(ids.filter(id => id !== 'gone').map(id => [id, [pdf(id)]])); + }); + return { getEmailAttachments } satisfies ListAttachmentSource; + } + + it('coalesces rows that mount together into one request', async () => { + const source = fakeSource(); + const a = vi.fn(), b = vi.fn(); + requestListAttachments(source, undefined, 'a', a); + requestListAttachments(source, undefined, 'b', b); + await vi.runAllTimersAsync(); + expect(source.getEmailAttachments).toHaveBeenCalledTimes(1); + expect(source.getEmailAttachments).toHaveBeenCalledWith(['a', 'b'], undefined); + expect(a).toHaveBeenCalledWith([pdf('a')]); + expect(b).toHaveBeenCalledWith([pdf('b')]); + }); + + it('answers a second mount from the cache, synchronously', async () => { + const source = fakeSource(); + requestListAttachments(source, undefined, 'a', vi.fn()); + await vi.runAllTimersAsync(); + const again = vi.fn(); + requestListAttachments(source, undefined, 'a', again); + expect(again).toHaveBeenCalledWith([pdf('a')]); + expect(source.getEmailAttachments).toHaveBeenCalledTimes(1); + }); + + it('drops a row scrolled away before the request goes out', async () => { + const source = fakeSource(); + const cancel = requestListAttachments(source, undefined, 'a', vi.fn()); + requestListAttachments(source, undefined, 'b', vi.fn()); + cancel(); + await vi.runAllTimersAsync(); + expect(source.getEmailAttachments).toHaveBeenCalledWith(['b'], undefined); + }); + + it('joins a request already in flight instead of asking again', async () => { + let release!: () => void; + const source: ListAttachmentSource & { getEmailAttachments: ReturnType } = { + getEmailAttachments: vi.fn(() => new Promise>((resolve) => { + release = () => resolve(new Map([['a', [pdf('a')]]])); + })), + }; + const first = vi.fn(), second = vi.fn(); + requestListAttachments(source, undefined, 'a', first); + await vi.advanceTimersByTimeAsync(100); + requestListAttachments(source, undefined, 'a', second); + await vi.advanceTimersByTimeAsync(100); + release(); + await vi.runAllTimersAsync(); + expect(source.getEmailAttachments).toHaveBeenCalledTimes(1); + expect(second).toHaveBeenCalledWith([pdf('a')]); + }); + + it('keeps accounts apart', async () => { + const source = fakeSource(); + const own = vi.fn(), shared = vi.fn(); + requestListAttachments(source, undefined, 'a', own); + requestListAttachments(source, 'owner', 'a', shared); + await vi.runAllTimersAsync(); + expect(source.getEmailAttachments).toHaveBeenCalledWith(['a'], undefined); + expect(source.getEmailAttachments).toHaveBeenCalledWith(['a'], 'owner'); + }); + + it('does not cache a failure, so the next mount retries', async () => { + const source = fakeSource(true); + const onLoad = vi.fn(); + requestListAttachments(source, undefined, 'a', onLoad); + await vi.runAllTimersAsync(); + expect(onLoad).not.toHaveBeenCalled(); + requestListAttachments(source, undefined, 'a', onLoad); + await vi.runAllTimersAsync(); + expect(source.getEmailAttachments).toHaveBeenCalledTimes(2); + }); + + it('does nothing for a client without lazy attachments', () => { + const onLoad = vi.fn(); + const cancel = requestListAttachments({}, undefined, 'a', onLoad); + cancel(); + expect(onLoad).not.toHaveBeenCalled(); + }); +}); diff --git a/lib/jmap/client-interface.ts b/lib/jmap/client-interface.ts index 225ad097b..74eac7dbd 100644 --- a/lib/jmap/client-interface.ts +++ b/lib/jmap/client-interface.ts @@ -1,4 +1,4 @@ -import type { Email, Mailbox, MailboxRights, StateChange, AccountStates, CollectionChanges, ShareNotification, BusyPeriod, CalendarParticipantIdentity, CalendarEventNotification, Thread, Identity, EmailAddress, ContactCard, AddressBook, AddressBookRights, VacationResponse, Calendar, CreateCalendarOptions, CalendarRights, CalendarEvent, CalendarEventFilter, CalendarTask, FileNode, FileNodeRights, Principal, PushSubscription, EmailPushConfig, ScheduledEmail, SendEmailResult, SharedAccount } from "./types"; +import type { Attachment, Email, Mailbox, MailboxRights, StateChange, AccountStates, CollectionChanges, ShareNotification, BusyPeriod, CalendarParticipantIdentity, CalendarEventNotification, Thread, Identity, EmailAddress, ContactCard, AddressBook, AddressBookRights, VacationResponse, Calendar, CreateCalendarOptions, CalendarRights, CalendarEvent, CalendarEventFilter, CalendarTask, FileNode, FileNodeRights, Principal, PushSubscription, EmailPushConfig, ScheduledEmail, SendEmailResult, SharedAccount } from "./types"; import type { SieveScript, SieveCapabilities } from "./sieve-types"; import type { FileNameRules } from "@/lib/file-name-rules"; import type { SortLevel } from "@/lib/message-list-order"; @@ -184,6 +184,12 @@ export interface IJMAPClient { getEmailsInMailbox(mailboxId: string): Promise; getEmail(emailId: string, accountId?: string): Promise; getSomeEmails(emailsId: string[], accountId?: string): Promise + /** + * Attachment parts per email id for list-row chips, fetched lazily because + * the server reads each full raw message to answer it (#1089). Optional: + * clients whose list emails already carry `attachments` need not implement it. + */ + getEmailAttachments?(emailIds: string[], accountId?: string): Promise>; /** * Total / unread message counts per tag id. `accountId` scopes the count to * a group/shared account reached through this client (defaults to the diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index 16810a289..525df1f72 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -1,5 +1,5 @@ import { generateUUID } from '@/lib/utils'; -import type { Email, Mailbox, MailboxRights, StateChange, AccountStates, CollectionChanges, ShareNotification, BusyPeriod, CalendarParticipantIdentity, CalendarEventNotification, Thread, Identity, EmailAddress, ContactCard, AddressBook, AddressBookRights, VacationResponse, Calendar, CalendarComponentType, CalendarRights, CalendarEvent, CalendarEventFilter, CalendarTask, CreateCalendarOptions, FileNode, FileNodeFilter, FileNodeRights, Principal, PushSubscription, EmailPushConfig, EmailSubmission, ScheduledEmail, SendEmailResult, SharedAccount } from "./types"; +import type { Attachment, Email, Mailbox, MailboxRights, StateChange, AccountStates, CollectionChanges, ShareNotification, BusyPeriod, CalendarParticipantIdentity, CalendarEventNotification, Thread, Identity, EmailAddress, ContactCard, AddressBook, AddressBookRights, VacationResponse, Calendar, CalendarComponentType, CalendarRights, CalendarEvent, CalendarEventFilter, CalendarTask, CreateCalendarOptions, FileNode, FileNodeFilter, FileNodeRights, Principal, PushSubscription, EmailPushConfig, EmailSubmission, ScheduledEmail, SendEmailResult, SharedAccount } from "./types"; import type { SieveScript, SieveCapabilities } from "./sieve-types"; import type { CalendarEventUpdateOptions, IJMAPClient, KeywordDiscoveryResult, KeywordInfo, KeywordMigration } from "./client-interface"; import { attachSearchSnippets, filterHasSnippetTerms, snippetFilterFor, type SearchSnippetResult } from "@/lib/search-snippet"; @@ -271,13 +271,20 @@ const EMAIL_LIST_PROPERTIES = [ "subject", "preview", "hasAttachment", - // Attachment metadata (name / type / blobId) so list rows can offer the - // files directly. hasAttachment alone only supports a paperclip icon. - "attachments", + // No "attachments" here: Stalwart answers it (like bodyStructure/textBody) + // by reading and parsing every message's full raw blob, so a 300-row page of + // mail with large files took seconds instead of milliseconds (#1089). List + // rows fetch their attachment chips lazily via getEmailAttachments. // Needed so list rows can serve drag-out to the file system as .eml. + // Served from stored metadata, so it costs nothing. "blobId", ] as const; +// Body-part fields an attachment chip needs (see components/email/attachment-chips.tsx). +const LIST_ATTACHMENT_BODY_PROPERTIES = [ + "partId", "blobId", "size", "name", "type", "charset", "cid", "disposition", +] as const; + /** * How many messages `discoverKeywords` walks before it gives up and reports an * incomplete scan. A keyword only has to sit on one message to be worth @@ -924,6 +931,34 @@ export class JMAPClient implements IJMAPClient { return []; } } + + /** + * Attachment parts of the given emails, keyed by email id, for list-row + * chips. Kept out of the list request on purpose: the server has to read + * each message's whole raw blob to answer it (#1089), so callers should + * ask only for the rows that are on screen and have `hasAttachment`. + * Ids the server does not return are absent from the map. + */ + async getEmailAttachments(emailIds: string[], accountId?: string): Promise> { + const targetAccountId = accountId || this.accountId; + const result = new Map(); + for (const batchIds of batched(emailIds, this.getMaxObjectsInGet())) { + const response = await this.request([ + ["Email/get", { + accountId: targetAccountId, + ids: batchIds, + properties: ["id", "attachments"], + bodyProperties: [...LIST_ATTACHMENT_BODY_PROPERTIES], + }, "0"], + ]); + if (response.methodResponses?.[0]?.[0] !== "Email/get") { + throw new Error("Email/get for list attachments failed"); + } + const list = (response.methodResponses[0][1]?.list || []) as Pick[]; + for (const email of list) result.set(email.id, email.attachments ?? []); + } + return result; + } /** Upgrade an existing basic-auth client to bearer-token auth (e.g. after TOTP token exchange). */ upgradeToBearer(accessToken: string, onRefresh?: () => Promise): void { this.authMode = 'bearer'; diff --git a/lib/list-attachments.ts b/lib/list-attachments.ts new file mode 100644 index 000000000..1c6cbc2d2 --- /dev/null +++ b/lib/list-attachments.ts @@ -0,0 +1,141 @@ +import type { Attachment, Email } from "@/lib/jmap/types"; +import { debug } from "@/lib/debug"; + +/** + * Lazy attachment metadata for message-list rows (#1089). + * + * Asking the server for `attachments` in the list request made Stalwart read + * and parse the full raw blob of every message on the page, attachment or + * not - seconds for a few hundred rows of real-world mail. Rows now ask for + * their own chips once they are on screen, and only when `hasAttachment` is + * set. Requests from rows that mount together (one list render, one scroll + * step) are coalesced into a single Email/get per client and account, and + * the answer is cached: an email's parts never change. + */ + +export interface ListAttachmentSource { + getEmailAttachments?(emailIds: string[], accountId?: string): Promise>; +} + +type Listener = (attachments: Attachment[]) => void; + +/** Loader handed to list rows: starts a lazy fetch, returns its cancel. */ +export type LoadListAttachments = (email: Email, onLoad: Listener) => () => void; + +interface Queue { + /** Waiting for the next flush, by email id. */ + pending: Map>; + /** Sent, answer not back yet - a row mounting now joins instead of re-asking. */ + inFlight: Map>; + timer: ReturnType | null; +} + +// Long enough to collect every row of one render or scroll step, short +// enough that nobody sees the chips arrive late. +const FLUSH_DELAY_MS = 50; +const CACHE_LIMIT = 2000; + +const queues = new WeakMap>(); +const caches = new WeakMap>(); + +const accountKey = (accountId: string | undefined) => accountId ?? ""; +const cacheKey = (accountId: string | undefined, emailId: string) => `${accountKey(accountId)}\u0000${emailId}`; + +function cacheFor(source: ListAttachmentSource): Map { + let cache = caches.get(source); + if (!cache) { + cache = new Map(); + caches.set(source, cache); + } + return cache; +} + +function remember(cache: Map, key: string, attachments: Attachment[]) { + cache.delete(key); + cache.set(key, attachments); + if (cache.size > CACHE_LIMIT) { + const oldest = cache.keys().next().value; + if (oldest !== undefined) cache.delete(oldest); + } +} + +function queueFor(source: ListAttachmentSource, accountId: string | undefined): Queue { + let perAccount = queues.get(source); + if (!perAccount) { + perAccount = new Map(); + queues.set(source, perAccount); + } + let queue = perAccount.get(accountKey(accountId)); + if (!queue) { + queue = { pending: new Map(), inFlight: new Map(), timer: null }; + perAccount.set(accountKey(accountId), queue); + } + return queue; +} + +function flush(source: ListAttachmentSource, accountId: string | undefined, queue: Queue) { + queue.timer = null; + const batch = new Map>(); + for (const [id, listeners] of queue.pending) { + if (listeners.size === 0) continue; + batch.set(id, listeners); + queue.inFlight.set(id, listeners); + } + queue.pending.clear(); + if (batch.size === 0 || !source.getEmailAttachments) return; + + const cache = cacheFor(source); + source.getEmailAttachments([...batch.keys()], accountId) + .then((found) => { + for (const [id, listeners] of batch) { + // An id the server did not return is gone; remember that too so a + // stale row does not ask again on every remount. + const attachments = found.get(id) ?? []; + remember(cache, cacheKey(accountId, id), attachments); + for (const listener of listeners) listener(attachments); + } + }) + .catch((error) => { + // Not cached: the next mount of the row tries again. + debug.warn("email", "Failed to load list attachments:", error); + }) + .finally(() => { + for (const id of batch.keys()) queue.inFlight.delete(id); + }); +} + +/** + * Ask for one email's attachment parts. `onLoad` runs once with the parts + * (synchronously when cached). Returns a cancel that drops the request if the + * row goes away first - a row scrolled past before the flush costs nothing. + */ +export function requestListAttachments( + source: ListAttachmentSource, + accountId: string | undefined, + emailId: string, + onLoad: Listener, +): () => void { + if (!source.getEmailAttachments) return () => {}; + + const cached = cacheFor(source).get(cacheKey(accountId, emailId)); + if (cached) { + onLoad(cached); + return () => {}; + } + + const queue = queueFor(source, accountId); + let listeners = queue.inFlight.get(emailId) ?? queue.pending.get(emailId); + if (!listeners) { + listeners = new Set(); + queue.pending.set(emailId, listeners); + } + listeners.add(onLoad); + if (queue.pending.has(emailId) && !queue.timer) { + queue.timer = setTimeout(() => flush(source, accountId, queue), FLUSH_DELAY_MS); + } + + const joined = listeners; + return () => { + joined.delete(onLoad); + }; +}