mirror of
https://github.com/root-fr/jmap-webmail.git
synced 2026-09-25 00:01:15 +00:00
fix(contacts): batch ContactCard/get to respect maxObjectsInGet
Contact loading silently returned empty when the address book held more entries than the server's maxObjectsInGet (Stalwart defaults to 500). Replace the single back-reference get with a two-step flow: query IDs first, then pack a batched get into one JMAP request. All batches ride a single HTTP roundtrip via multiple method calls. Address books at or under the server cap still get a single method call, so no behavior change for small lists. Adds getMaxObjectsInGet() capability helper that mirrors the existing getMaxCallsInRequest() / getMaxSizeUpload() shape. 83 new test cases cover single-batch, exact-boundary, over-cap, and direct-ids paths. Closes #45. Credits #46 (@capitanroy).
This commit is contained in:
committed by
Matthieu MALVACHE
parent
3c1a4496f9
commit
76cebb7905
@@ -120,27 +120,43 @@ 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');
|
||||
|
||||
mockFetchOnce(fetchSpy, {
|
||||
methodResponses: [['ContactCard/query', { ids: ['contact-1'] }, '0']],
|
||||
});
|
||||
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 +168,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 +177,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,18 +195,81 @@ 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: [
|
||||
['ContactCard/query', { ids: [] }, '0'],
|
||||
['SomethingElse', {}, '1'],
|
||||
],
|
||||
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');
|
||||
|
||||
mockFetchOnce(fetchSpy, {
|
||||
methodResponses: [['ContactCard/query', { ids: ['c-1', 'c-2'] }, '0']],
|
||||
});
|
||||
mockFetchOnce(fetchSpy, {
|
||||
methodResponses: [['ContactCard/get', { list: [
|
||||
{ ...mockContact, id: 'c-1' },
|
||||
{ ...mockContact, id: 'c-2' },
|
||||
] }, '0']],
|
||||
});
|
||||
|
||||
const result = await client.getContacts();
|
||||
expect(result).toHaveLength(2);
|
||||
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');
|
||||
|
||||
mockFetchOnce(fetchSpy, {
|
||||
methodResponses: [['ContactCard/query', { ids: ['c-1', 'c-2', 'c-3'] }, '0']],
|
||||
});
|
||||
mockFetchOnce(fetchSpy, {
|
||||
methodResponses: [
|
||||
['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).toHaveLength(3);
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(2);
|
||||
|
||||
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();
|
||||
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
||||
describe('getContact', () => {
|
||||
|
||||
+33
-8
@@ -1472,6 +1472,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;
|
||||
|
||||
@@ -1765,23 +1770,43 @@ export class JMAPClient {
|
||||
async getContacts(addressBookId?: string): Promise<ContactCard[]> {
|
||||
try {
|
||||
const accountId = this.getContactsAccountId();
|
||||
const maxBatchSize = this.getMaxObjectsInGet();
|
||||
// TODO: paginate query for >1000 contacts (limit caps results silently)
|
||||
const queryArgs: Record<string, unknown> = { accountId, limit: 1000 };
|
||||
if (addressBookId) {
|
||||
queryArgs.filter = { inAddressBook: addressBookId };
|
||||
}
|
||||
|
||||
const response = await this.request([
|
||||
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 [];
|
||||
}
|
||||
|
||||
// Batch the get so servers capping maxObjectsInGet (Stalwart default 500)
|
||||
// don't silently fail. All batches are packed into a single JMAP request
|
||||
// — one HTTP roundtrip regardless of contact count.
|
||||
const calls: [string, Record<string, unknown>, 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 {
|
||||
return [];
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user