inbuxa/ihasmail-inbuxa
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases 1 Wiki Activity

Merge pull request #215 from Coffey-Labs/fix/ical-import-batching

Browse Source 
Import an iCal file in batches the server will take
This commit is contained in:
Coffey Labs
2026-09-02 06:43:20 -07:00
committed by GitHub
parent b0edce1d9b 17bd548524
commit 6b8ac393c9
2 changed files with 117 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files
web/src/store/__tests__/ics-import.test.ts
+84 -6
View File
@@ -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<string, Record<string, unknown>>; 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<string, unknown> } = {}) {
function server(parsed: unknown, opts: { notCreated?: Record<string, unknown>; 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, unknown>, string][] };
@@ -44,8 +48,14 @@ function server(parsed: unknown, opts: { notCreated?: Record<string, unknown> }
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<string, Record<string, unknown>>, 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<string, unknown> }
}
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("[email protected]");
expect(uids).toContain("[email protected]");
});
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/);
});
});
web/src/store/calendar.ts
+33 -14
View File
@@ -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<CalendarState>((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<CalendarState>((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<SetResponse<CalendarEvent>>("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<string, unknown> = {};
for (const k of part) sub[k] = create[k];
const res = await client.call<SetResponse<CalendarEvent>>("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;
},