diff --git a/lib/__tests__/jmap-contact-client.test.ts b/lib/__tests__/jmap-contact-client.test.ts index 7a054a6..0c1214b 100644 --- a/lib/__tests__/jmap-contact-client.test.ts +++ b/lib/__tests__/jmap-contact-client.test.ts @@ -120,27 +120,45 @@ describe('JMAPClient contact methods', () => { }); describe('getContacts', () => { - it('should return contacts from server', async () => { + function setupClientWithMaxObjects(max: number) { const client = createClient(); - mockFetch({ - methodResponses: [ - ['ContactCard/query', { ids: ['contact-1'] }, '0'], - ['ContactCard/get', { list: [mockContact] }, '1'], - ], + Object.assign(client, { + capabilities: { + 'urn:ietf:params:jmap:contacts': {}, + 'urn:ietf:params:jmap:core': { maxObjectsInGet: max }, + }, + }); + return client; + } + + it('should return contacts from server (single batch)', async () => { + const client = setupClientWithMaxObjects(500); + const fetchSpy = vi.spyOn(globalThis, 'fetch'); + + // Request 1: query IDs + mockFetchOnce(fetchSpy, { + methodResponses: [['ContactCard/query', { ids: ['contact-1'] }, '0']], + }); + // Request 2: get by IDs + mockFetchOnce(fetchSpy, { + methodResponses: [['ContactCard/get', { list: [mockContact] }, '0']], }); const result = await client.getContacts(); expect(result).toHaveLength(1); expect(result[0].id).toBe('contact-1'); + expect(fetchSpy).toHaveBeenCalledTimes(2); }); it('should filter by addressBookId when provided', async () => { - const client = createClient(); - const fetchSpy = mockFetch({ - methodResponses: [ - ['ContactCard/query', { ids: ['contact-1'] }, '0'], - ['ContactCard/get', { list: [mockContact] }, '1'], - ], + const client = setupClientWithMaxObjects(500); + const fetchSpy = vi.spyOn(globalThis, 'fetch'); + + mockFetchOnce(fetchSpy, { + methodResponses: [['ContactCard/query', { ids: ['contact-1'] }, '0']], + }); + mockFetchOnce(fetchSpy, { + methodResponses: [['ContactCard/get', { list: [mockContact] }, '0']], }); await client.getContacts('ab-1'); @@ -152,10 +170,7 @@ describe('JMAPClient contact methods', () => { it('should not include filter when no addressBookId', async () => { const client = createClient(); const fetchSpy = mockFetch({ - methodResponses: [ - ['ContactCard/query', { ids: [] }, '0'], - ['ContactCard/get', { list: [] }, '1'], - ], + methodResponses: [['ContactCard/query', { ids: [] }, '0']], }); await client.getContacts(); @@ -164,13 +179,10 @@ describe('JMAPClient contact methods', () => { expect(body.methodCalls[0][1].filter).toBeUndefined(); }); - it('should return empty array when no contacts', async () => { + it('should return empty array when zero contacts', async () => { const client = createClient(); mockFetch({ - methodResponses: [ - ['ContactCard/query', { ids: [] }, '0'], - ['ContactCard/get', { list: [] }, '1'], - ], + methodResponses: [['ContactCard/query', { ids: [] }, '0']], }); const result = await client.getContacts(); @@ -185,17 +197,88 @@ describe('JMAPClient contact methods', () => { expect(result).toEqual([]); }); - it('should return empty array for unexpected response at index 1', async () => { + it('should return empty array for unexpected query response', async () => { const client = createClient(); mockFetch({ + methodResponses: [['SomethingElse', {}, '0']], + }); + + const result = await client.getContacts(); + expect(result).toEqual([]); + }); + + it('should handle exact boundary (ids.length === maxObjectsInGet)', async () => { + const client = setupClientWithMaxObjects(2); + const fetchSpy = vi.spyOn(globalThis, 'fetch'); + + // Query returns exactly 2 IDs (== maxObjectsInGet) + mockFetchOnce(fetchSpy, { + methodResponses: [['ContactCard/query', { ids: ['c-1', 'c-2'] }, '0']], + }); + // Single batch get — all IDs fit in one call + mockFetchOnce(fetchSpy, { + methodResponses: [['ContactCard/get', { list: [ + { ...mockContact, id: 'c-1' }, + { ...mockContact, id: 'c-2' }, + ] }, '0']], + }); + + const result = await client.getContacts(); + expect(result).toHaveLength(2); + // Should be exactly 2 HTTP requests: query + one get + expect(fetchSpy).toHaveBeenCalledTimes(2); + }); + + it('should batch into a single JMAP request when over maxObjectsInGet', async () => { + const client = setupClientWithMaxObjects(2); + const fetchSpy = vi.spyOn(globalThis, 'fetch'); + + // Query returns 3 IDs (> maxObjectsInGet of 2) + mockFetchOnce(fetchSpy, { + methodResponses: [['ContactCard/query', { ids: ['c-1', 'c-2', 'c-3'] }, '0']], + }); + // Single JMAP request with 2 method calls (batch of 2 + batch of 1) + mockFetchOnce(fetchSpy, { methodResponses: [ - ['ContactCard/query', { ids: [] }, '0'], - ['SomethingElse', {}, '1'], + ['ContactCard/get', { list: [ + { ...mockContact, id: 'c-1' }, + { ...mockContact, id: 'c-2' }, + ] }, '0'], + ['ContactCard/get', { list: [ + { ...mockContact, id: 'c-3' }, + ] }, '1'], ], }); const result = await client.getContacts(); - expect(result).toEqual([]); + expect(result).toHaveLength(3); + // Only 2 HTTP roundtrips total: query + one packed get request + expect(fetchSpy).toHaveBeenCalledTimes(2); + + // Verify the second request contained 2 method calls + const body = JSON.parse(fetchSpy.mock.calls[1][1]?.body as string); + expect(body.methodCalls).toHaveLength(2); + expect(body.methodCalls[0][1].ids).toEqual(['c-1', 'c-2']); + expect(body.methodCalls[1][1].ids).toEqual(['c-3']); + }); + + it('should pass IDs directly instead of using back-reference', async () => { + const client = setupClientWithMaxObjects(500); + const fetchSpy = vi.spyOn(globalThis, 'fetch'); + + mockFetchOnce(fetchSpy, { + methodResponses: [['ContactCard/query', { ids: ['contact-1'] }, '0']], + }); + mockFetchOnce(fetchSpy, { + methodResponses: [['ContactCard/get', { list: [mockContact] }, '0']], + }); + + await client.getContacts(); + + // Second request should pass IDs directly, not use #ids back-reference + const body = JSON.parse(fetchSpy.mock.calls[1][1]?.body as string); + expect(body.methodCalls[0][1].ids).toEqual(['contact-1']); + expect(body.methodCalls[0][1]['#ids']).toBeUndefined(); }); }); diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index 43ff79e..bd4c1d6 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -1414,6 +1414,11 @@ export class JMAPClient { return coreCapability?.maxCallsInRequest || 50; } + getMaxObjectsInGet(): number { + const coreCapability = this.capabilities["urn:ietf:params:jmap:core"] as { maxObjectsInGet?: number } | undefined; + return coreCapability?.maxObjectsInGet || 500; + } + getEventSourceUrl(): string | null { if (!this.session) return null; @@ -1708,23 +1713,44 @@ export class JMAPClient { async getContacts(addressBookId?: string): Promise { try { const accountId = this.getContactsAccountId(); + const maxBatchSize = this.getMaxObjectsInGet(); + // TODO: paginate query for >1000 contacts (limit caps results silently) const queryArgs: Record = { accountId, limit: 1000 }; if (addressBookId) { queryArgs.filter = { inAddressBook: addressBookId }; } - const response = await this.request([ + // Step 1: Query all matching contact IDs + const queryResponse = await this.request([ ["ContactCard/query", queryArgs, "0"], - ["ContactCard/get", { - accountId, - "#ids": { resultOf: "0", name: "ContactCard/query", path: "/ids" }, - }, "1"], ], this.contactUsing()); - if (response.methodResponses?.[1]?.[0] === "ContactCard/get") { - return (response.methodResponses[1][1].list || []) as ContactCard[]; + if (queryResponse.methodResponses?.[0]?.[0] !== "ContactCard/query") { + return []; } - return []; + + const allIds = (queryResponse.methodResponses[0][1].ids || []) as string[]; + if (allIds.length === 0) { + return []; + } + + // Step 2: Fetch contacts, batching to respect maxObjectsInGet. + // All batches are packed into a single JMAP request (multiple method + // calls), so this is always one HTTP roundtrip regardless of count. + const calls: [string, Record, string][] = []; + for (let i = 0; i < allIds.length; i += maxBatchSize) { + const batchIds = allIds.slice(i, i + maxBatchSize); + calls.push(["ContactCard/get", { accountId, ids: batchIds }, String(calls.length)]); + } + + const response = await this.request(calls, this.contactUsing()); + const allContacts: ContactCard[] = []; + for (const [method, result] of response.methodResponses || []) { + if (method === "ContactCard/get") { + allContacts.push(...((result as { list?: ContactCard[] }).list || [])); + } + } + return allContacts; } catch (error) { console.error('Failed to get contacts:', error); return [];