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

Import an iCal file in batches the server will take

Browse Source 
An 800 KB export imported nothing at all. Every event in the file went out
in a single CalendarEvent/set, and Stalwart refuses a method call carrying
more objects than maxObjectsInSet -- the whole call, with requestTooLarge,
creating none of it -- so the import failed at exactly the size that makes
importing worth doing. A two-event invitation was fine; a real calendar was
not.

The events now go out maxObjectsInSet at a time, which the client already
reads off the session and defaults to 500 where a server does not say. That
is the same ceiling and the same helper the mail store batches deletes and
flag changes by; nothing new had to be learned about the limit, and there is
no need to ask anyone to split an .ics by hand at an arbitrary line.

Still batches rather than a call per event: createEvent invalidates on the
way out and invalidating re-fetches every cached range, which is why the
import writes its own set calls in the first place. One invalidate, after
the last batch.

A batch that fails after earlier ones have been filed now says how many got
in -- "1000 of 1200 events were imported before this happened" -- and
re-reads the calendar so they are visible. Reporting only that the import
failed would send someone looking for events that are already there.

The mock enforced this ceiling all along, on both /get and /set; nothing had
exercised it with a file big enough to cross it.

Reported on #173.
This commit is contained in:
jcoffey-dev
2026-09-02 06:29:49 -07:00
parent b0edce1d9b
commit 17bd548524
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;
},