diff --git a/src/sync/backends/jmap/contacts.ts b/src/sync/backends/jmap/contacts.ts index a31017b..990cd56 100644 --- a/src/sync/backends/jmap/contacts.ts +++ b/src/sync/backends/jmap/contacts.ts @@ -673,6 +673,29 @@ export async function reconcileContacts({ transport, account, handlers, useWebSo return syncContacts({ transport, account, handlers, useWebSocket }); } +/** + * Reconcile the local cache for a small, known set of cards after a + * single-contact mutation (whitelist, add, edit), instead of re-pulling + * the entire address book the way `reconcileContacts` does. Cost is + * O(books) + O(ids) rather than O(all contacts), so a whitelist stays + * fast no matter how many contacts the account has. + * + * Address books are synced first (they are few) so a card filed in a + * book that was created in the same operation — e.g. "Trusted senders" — + * resolves locally; then only the named card ids are fetched + upserted. + */ +export async function reconcileContactCards({ + transport, account, handlers, ids, useWebSocket = false, +}) { + await syncAddressBooks({ transport, account, handlers, useWebSocket }); + const list = (ids ?? []).filter(Boolean); + if (list.length === 0) return { fetched: 0 }; + const fetched = await fetchAndPersistContactCards({ + transport, account, handlers, ids: list, useWebSocket, + }); + return { fetched }; +} + /** * Resolve the remote id of the account's primary address book for new * contacts: the one flagged `isDefault`, else the first book that is diff --git a/src/sync/backends/jmap/outbox.ts b/src/sync/backends/jmap/outbox.ts index bd4896d..ebf57db 100644 --- a/src/sync/backends/jmap/outbox.ts +++ b/src/sync/backends/jmap/outbox.ts @@ -52,7 +52,7 @@ import { createContactCard, updateContactCard, deleteContactCard, - reconcileContacts, + reconcileContactCards, } from './contacts'; import { base64ToBytes, extractDataUriImages } from '../../../utils/inline-images'; @@ -195,11 +195,12 @@ async function runWhitelistSender({ transport, account, handlers, request, useWe if (!result.ok) { return { ok: false, error: result.error ?? { type: 'serverFail' } }; } - // Pull the new cards (and their book) into the local cache once, after - // all trusted cards exist, so they show up in the contacts view without - // waiting for a StateChange push. Best-effort: the trust already took + // Pull only the newly-trusted card(s) into the local cache so they show + // up in the contacts view without waiting for a StateChange push. + // Targeted (not a full address-book resync) so a whitelist stays fast + // regardless of contact count. Best-effort: the trust already took // effect server-side, so a reconcile failure must not fail the mutation. - await reconcileContactsQuietly({ transport, account, handlers, useWebSocket }); + await reconcileContactCardsQuietly({ transport, account, handlers, ids: result.ids, useWebSocket }); return { ok: true }; } @@ -224,7 +225,9 @@ async function runCreateContact({ transport, account, handlers, request, useWebS if (!result.ok) { return { ok: false, error: result.error ?? { type: 'serverFail' } }; } - await reconcileContactsQuietly({ transport, account, handlers, useWebSocket }); + await reconcileContactCardsQuietly({ + transport, account, handlers, ids: result.id ? [result.id] : [], useWebSocket, + }); return { ok: true }; } @@ -245,7 +248,9 @@ async function runUpdateContact({ transport, account, handlers, request, useWebS if (!result.ok) { return { ok: false, error: result.error ?? { type: 'serverFail' } }; } - await reconcileContactsQuietly({ transport, account, handlers, useWebSocket }); + await reconcileContactCardsQuietly({ + transport, account, handlers, ids: request?.remoteId ? [request.remoteId] : [], useWebSocket, + }); return { ok: true }; } @@ -272,9 +277,9 @@ async function runDeleteContact({ transport, account, handlers, request, useWebS return { ok: true }; } -async function reconcileContactsQuietly({ transport, account, handlers, useWebSocket }) { +async function reconcileContactCardsQuietly({ transport, account, handlers, ids, useWebSocket }) { try { - await reconcileContacts({ transport, account, handlers, useWebSocket }); + await reconcileContactCards({ transport, account, handlers, ids, useWebSocket }); } catch { // Cache will catch up on the next StateChange push or periodic sync. } diff --git a/tests/unit/sync/jmap-contacts.test.ts b/tests/unit/sync/jmap-contacts.test.ts index b268553..9693ad7 100644 --- a/tests/unit/sync/jmap-contacts.test.ts +++ b/tests/unit/sync/jmap-contacts.test.ts @@ -13,8 +13,19 @@ import { updateContactCard, deleteContactCard, } from '../../../src/sync/backends/jmap/contacts'; +import { processMutationRow } from '../../../src/sync/backends/jmap/outbox'; import { MockTransport } from './_mock-transport'; +function jmapCalls(transport, method) { + const out = []; + for (const req of transport.requests) { + for (const [m, params] of req.methodCalls) { + if (m === method) out.push(params); + } + } + return out; +} + function countMethod(transport, name) { return transport.requests.filter( (r) => r.methodCalls.some(([m]) => m === name), @@ -507,3 +518,121 @@ describe('syncContactCardChanges', () => { expect(Number(destroyed.is_deleted)).toBe(1); }); }); + +describe('whitelist reconcile cost is independent of contact count', () => { + async function seedLocalContacts(n) { + await handlers[DB_RPC.ADDRESSBOOK_UPSERT_MANY]({ + accountId: account.id, + serviceKind: SERVICE_KIND.JMAP_CONTACTS, + addressbooks: [ + { remoteId: 'book-default', name: 'Default', isDefault: true }, + { remoteId: 'book-trusted', name: 'Trusted senders', isDefault: false }, + ], + }); + const books = await handlers[DB_RPC.ADDRESSBOOK_LIST]({ accountId: account.id }); + const defaultLocal = books.find((b) => b.remote_id === 'book-default').id; + const contacts = Array.from({ length: n }, (_, i) => ({ + addressbookId: defaultLocal, + remoteId: `seed-${i}`, + uid: null, + etag: null, + fullName: `Seed ${i}`, + displayName: `Seed ${i}`, + givenName: null, + familyName: null, + organization: null, + vcardText: null, + vcardVersion: null, + rawJson: '{}', + isDeleted: false, + emails: [{ position: 0, email: `seed-${i}@x.invalid`, label: null, isPreferred: true }], + })); + await handlers[DB_RPC.CONTACT_UPSERT_MANY]({ accountId: account.id, contacts }); + } + + function countContacts() { + return engine.get( + 'SELECT COUNT(*) AS n FROM contacts WHERE account_id = ? AND is_deleted = 0', + [account.id], + ); + } + + it('batch-whitelisting 200 mails from 150 senders on a 1099-contact book fetches only the 150 new cards', async () => { + await seedLocalContacts(1099); + expect((await countContacts()).n).toBe(1099); + + // 200 junk messages multi-selected, from 150 distinct senders (50 + // repeats), as the store's whitelistSenders would hand them off. + const senders = Array.from({ length: 200 }, (_, i) => ({ + email: `batch-${i % 150}@junk.invalid`, + name: `Sender ${i % 150}`, + })); + expect(new Set(senders.map((s) => s.email)).size).toBe(150); + + // The "server" holds 1099 cards, but the batch whitelist must never + // pull them: a full ContactCard/query (position/limit) would be the + // old O(contacts) reconcile; the existence check uses a {email} + // (OR-of-emails) filter. + const transport = new MockTransport(); + let fullListQueried = false; + let setCalls = 0; + let createdInOneCall = 0; + transport.handle('ContactCard/query', (params) => { + if (params.position != null || params.limit != null) { + fullListQueried = true; + return { ids: Array.from({ length: 1099 }, (_, i) => `seed-${i}`), total: 1099, state: 's' }; + } + return { ids: [], total: 0 }; // existence check: none of the senders carded yet + }); + transport.handle('AddressBook/get', () => ({ + list: [ + { id: 'book-default', name: 'Default', isDefault: true }, + { id: 'book-trusted', name: 'Trusted senders', isDefault: false }, + ], + })); + transport.handle('ContactCard/set', (params) => { + setCalls += 1; + const created = {}; + const keys = Object.keys(params.create ?? {}); + createdInOneCall = Math.max(createdInOneCall, keys.length); + for (const key of keys) created[key] = { id: key }; // distinct ids + return { created }; + }); + transport.handle('ContactCard/get', (params) => ({ + list: (params.ids ?? []).map((id) => ({ + '@type': 'Card', + id, + kind: 'individual', + name: { full: `Trusted ${id}` }, + emails: { e1: { '@type': 'EmailAddress', address: `wl-${id}@junk.invalid` } }, + addressBookIds: { 'book-trusted': true }, + })), + })); + + const result = await processMutationRow({ + transport, + account, + handlers, + row: { + mutation_type: 'whitelistSender', + request_json: JSON.stringify({ senders }), + }, + }); + expect(result.ok).toBe(true); + + // All 150 unique senders trusted in a single batched ContactCard/set + // (200 mails de-duped to 150 cards, one call, not a per-sender loop). + expect(setCalls).toBe(1); + expect(createdInOneCall).toBe(150); + + // The 1099-contact book is never re-pulled... + expect(fullListQueried).toBe(false); + // ...only the 150 new trusted cards are fetched. + const idsFetched = jmapCalls(transport, 'ContactCard/get') + .reduce((sum, p) => sum + (p.ids?.length ?? 0), 0); + expect(idsFetched).toBe(150); + + // The 1099 existing contacts are untouched; exactly 150 were added. + expect((await countContacts()).n).toBe(1249); + }); +});