diff --git a/web/src/store/__tests__/ics-import.test.ts b/web/src/store/__tests__/ics-import.test.ts index e6062b0..049b661 100644 --- a/web/src/store/__tests__/ics-import.test.ts +++ b/web/src/store/__tests__/ics-import.test.ts @@ -6,9 +6,10 @@ import type { JmapSession, UploadResponse } from "@/jmap/types"; /** * Importing a file is not importing an invitation, and the difference is the * count: an emailed invite carries one event, an export carries a year of them. - * These pin the two things that follow from that -- one round trip rather than - * one per event, and nothing of where the events came from riding along into - * the calendar they land in. + * These pin the three things that follow from that -- as few round trips as the + * server will take, none of them over the ceiling it will refuse the whole call + * for, and nothing of where the events came from riding along into the calendar + * they land in. */ /** What the server hands back for a two-event file. Ids and the JMAP-only @@ -33,8 +34,11 @@ interface SetArgs { create?: Record>; sendSchedu * @param parsed what `CalendarEvent/parse` answers with; a bare object rather * than an array is the single-event shape, which Stalwart also returns. * @param notCreated refusals to hand back instead of creations. + * @param max the ceiling on objects in one call, refused the way Stalwart + * refuses it: the whole call, creating nothing. + * @param failOn which `/set` call (0-based) answers with an error instead. */ -function server(parsed: unknown, opts: { notCreated?: Record } = {}) { +function server(parsed: unknown, opts: { notCreated?: Record; max?: number; failOn?: number } = {}) { const sets: SetArgs[] = []; const fetchMock = vi.fn(async (_url: string, init: RequestInit) => { const body = JSON.parse(init.body as string) as { methodCalls: [string, Record, string][] }; @@ -44,8 +48,14 @@ function server(parsed: unknown, opts: { notCreated?: Record } return [name, { accountId: "a1", parsed: parsed === null ? {} : { [blobIds[0]!]: parsed }, notParsable: [] }, id]; } if (name === "CalendarEvent/set") { + const nth = sets.length; sets.push({ create: args.create as Record>, sendSchedulingMessages: args.sendSchedulingMessages as boolean }); const keys = Object.keys((args.create ?? {}) as object); + // Whole-call refusals, both of them: nothing in this call is created. + if (opts.max != null && keys.length > opts.max) { + return ["error", { type: "requestTooLarge", description: "The number of ids requested by the client exceeds the maximum number the server is willing to process in a single method call." }, id]; + } + if (opts.failOn === nth) return ["error", { type: "serverFail", description: "the roof fell in" }, id]; const notCreated = opts.notCreated ?? {}; return [name, { accountId: "a1", oldState: "1", newState: "2", @@ -62,13 +72,15 @@ function server(parsed: unknown, opts: { notCreated?: Record } } let uploaded: { type?: string; text: string } | null = null; +/** Two tests stand a mock in for it; put the store's own back afterwards. */ +const realInvalidate = useCalendar.getState().invalidate; beforeEach(() => { client.session = { capabilities: { [CAP.core]: { maxObjectsInGet: 500, maxObjectsInSet: 500 }, [CAP.calendars]: {} }, accounts: {}, primaryAccounts: {}, state: "s1", } as unknown as JmapSession; - useCalendar.setState({ accountId: "a1", available: true, calendars: {}, events: {}, ranges: {} }); + useCalendar.setState({ accountId: "a1", available: true, calendars: {}, events: {}, ranges: {}, invalidate: realInvalidate }); uploaded = null; // XHR, not fetch, so it is stubbed at the client rather than at the network. // jsdom's Blob has no `text()`, hence the reader. @@ -96,7 +108,7 @@ describe("importing an .ics file", () => { expect(uploaded?.text).toContain("BEGIN:VCALENDAR"); }); - it("creates every event in one call, not one call each", async () => { + it("creates every event in one call when the file fits in one, not one call each", async () => { const sets = server(PARSED); const n = await useCalendar.getState().importIcs("x", "cal1"); expect(n).toBe(2); @@ -159,3 +171,69 @@ describe("importing an .ics file", () => { await expect(useCalendar.getState().importIcs("x", "cal1")).resolves.toBe(1); }); }); + +/* + * A real export, rather than the two-event file above. + * + * `CalendarEvent/set` is refused whole over `maxObjectsInSet` -- the server + * does not take the first 500 and drop the rest, it creates nothing and answers + * `requestTooLarge` -- so a file large enough to cross the ceiling used to + * import no events at all. The server here refuses the same way, which is what + * makes these more than an assertion about call counts. + */ +describe("importing a file bigger than the server will take at once", () => { + const MAX = 500; + const many = (n: number) => + Array.from({ length: n }, (_, i) => ({ + "@type": "Event", uid: `uid-${i}@example.org`, title: `Event ${i}`, + start: "2026-09-02T09:00:00", duration: "PT1H", timeZone: "Etc/UTC", + })); + + it("splits it into calls the server will accept, and files all of it", async () => { + const sets = server(many(1200), { max: MAX }); + await expect(useCalendar.getState().importIcs("x", "cal1")).resolves.toBe(1200); + expect(sets.map((s) => Object.keys(s.create!).length)).toEqual([500, 500, 200]); + }); + + it("splits by what the session advertises, not by a number of its own", async () => { + client.session!.capabilities[CAP.core] = { maxObjectsInGet: 40, maxObjectsInSet: 40 }; + const sets = server(many(100), { max: 40 }); + await expect(useCalendar.getState().importIcs("x", "cal1")).resolves.toBe(100); + expect(sets.map((s) => Object.keys(s.create!).length)).toEqual([40, 40, 20]); + }); + + it("keeps every event distinct across the split", async () => { + const sets = server(many(600), { max: MAX }); + await useCalendar.getState().importIcs("x", "cal1"); + const uids = sets.flatMap((s) => Object.values(s.create!).map((e) => e.uid)); + expect(new Set(uids).size).toBe(600); + expect(uids).toContain("uid-0@example.org"); + expect(uids).toContain("uid-599@example.org"); + }); + + it("re-reads the calendar once, not once per batch", async () => { + server(many(1200), { max: MAX }); + const invalidate = vi.fn(); + useCalendar.setState({ invalidate }); + await useCalendar.getState().importIcs("x", "cal1"); + expect(invalidate).toHaveBeenCalledTimes(1); + }); + + it("says how much got in when a later batch fails, rather than only that it failed", async () => { + server(many(1200), { max: MAX, failOn: 2 }); + await expect(useCalendar.getState().importIcs("x", "cal1")).rejects.toThrow(/1000 of 1200/); + }); + + it("leaves what did get in visible when a later batch fails", async () => { + server(many(1200), { max: MAX, failOn: 2 }); + const invalidate = vi.fn(); + useCalendar.setState({ invalidate }); + await expect(useCalendar.getState().importIcs("x", "cal1")).rejects.toThrow(); + expect(invalidate).toHaveBeenCalledTimes(1); + }); + + it("passes the server's own words through when the very first batch fails", async () => { + server(many(1200), { max: MAX, failOn: 0 }); + await expect(useCalendar.getState().importIcs("x", "cal1")).rejects.toThrow(/roof fell in/); + }); +}); diff --git a/web/src/store/calendar.ts b/web/src/store/calendar.ts index 737933b..f25ecbe 100644 --- a/web/src/store/calendar.ts +++ b/web/src/store/calendar.ts @@ -1,6 +1,6 @@ import { create } from "zustand"; -import { CAP, client, setErrorMessage } from "@/jmap/client"; -import type { BusyPeriod, Calendar, CalendarEvent, EmailAddress, GetResponse, Id, JSCalendarParticipant, JSCalendarRecurrenceRule, ParticipantIdentity, QueryResponse, SetResponse } from "@/jmap/types"; +import { CAP, chunk, client, setErrorMessage } from "@/jmap/client"; +import type { BusyPeriod, Calendar, CalendarEvent, EmailAddress, GetResponse, Id, JSCalendarParticipant, JSCalendarRecurrenceRule, ParticipantIdentity, QueryResponse, SetError, SetResponse } from "@/jmap/types"; import { toUTCDate, toLocalDateTime, zonedToDate, parseDuration, DAY_MS, browserTimeZone } from "@/lib/dates"; import { t } from "@/lib/i18n"; import { useContacts } from "./contacts"; @@ -808,11 +808,16 @@ export const useCalendar = create((set, get) => ({ * a browser, and the one already in Stalwart handles what a hand-rolled * parser would not. * - * Every event goes out in one `CalendarEvent/set` rather than a call each. - * The round trips are the smaller half of the reason: `createEvent` - * invalidates on the way out, and invalidating re-fetches every cached range, - * so importing a year of events one at a time would refetch the calendar a - * few hundred times. + * The events go out `maxObjectsInSet` at a time -- the ceiling the session + * advertises, 500 where a server does not say. A call carrying more than that + * is refused whole with `requestTooLarge` and creates nothing, so a real + * export -- an 800 KB file is thousands of events -- imported nothing at all + * while this went out in a single call. + * + * Batches rather than a call per event, though: `createEvent` invalidates on + * the way out, and invalidating re-fetches every cached range, so importing a + * year of events one at a time would refetch the calendar a few hundred + * times. One invalidate here, after the last batch. * * No scheduling messages. Importing a file is filing something you already * have, and mailing its participants would be a surprise to everyone. @@ -830,15 +835,29 @@ export const useCalendar = create((set, get) => ({ // invented, and an event with no UID is not one anything can match to. create[`e${i}`] = { "@type": "Event", ...rest, uid: rest.uid || crypto.randomUUID(), calendarIds: { [calendarId]: true } }; }); - const res = await client.call>("CalendarEvent/set", { accountId, create, sendSchedulingMessages: false }); - get().invalidate(); - const created = Object.keys(res.created ?? {}).length; + const keys = Object.keys(create); + let created = 0; + let refused: SetError | undefined; + try { + for (const part of chunk(keys, client.maxObjectsInSet)) { + const sub: Record = {}; + for (const k of part) sub[k] = create[k]; + const res = await client.call>("CalendarEvent/set", { accountId, create: sub, sendSchedulingMessages: false }); + created += Object.keys(res.created ?? {}).length; + refused ??= Object.values(res.notCreated ?? {})[0]; + } + } catch (err) { + // A batch that failed with earlier ones already filed: those events are + // in the calendar, and an error saying only that the import failed sends + // someone looking for events that are already there. + if (!created) throw err; + throw new Error(`${created} of ${keys.length} events were imported before this happened: ${(err as Error).message}`); + } finally { + if (created) get().invalidate(); + } // Nothing at all got in: say why rather than report importing zero events // as though the file had been empty. - if (!created) { - const first = Object.values(res.notCreated ?? {})[0]; - throw new Error(first ? setErrorMessage(first) : "the server did not accept any of its events"); - } + if (!created) throw new Error(refused ? setErrorMessage(refused) : "the server did not accept any of its events"); return created; },