Import an iCal file in batches the server will take
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:
@@ -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/);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user