diff --git a/server/src/mock/events.ts b/server/src/mock/events.ts index faafa69..bd83a7d 100644 --- a/server/src/mock/events.ts +++ b/server/src/mock/events.ts @@ -18,6 +18,18 @@ export function recordEmailChange(change: { created?: string[]; updated?: string if (emailChanges.length > 200) emailChanges.splice(0, emailChanges.length - 200); } +/** The same for contact cards, so `ContactCard/changes` can answer too. */ +export const cardChanges: Array<{ state: number; created: string[]; updated: string[]; destroyed: string[] }> = []; +/** Changes at or below this state have been dropped from the log, so a client that far behind cannot be answered. */ +export const cardLog = { floor: 0 }; +export function recordCardChange(change: { created?: string[]; updated?: string[]; destroyed?: string[] }) { + cardChanges.push({ state: state.n, created: change.created ?? [], updated: change.updated ?? [], destroyed: change.destroyed ?? [] }); + if (cardChanges.length > 200) { + const dropped = cardChanges.splice(0, cardChanges.length - 200); + cardLog.floor = dropped[dropped.length - 1]!.state; + } +} + export function broadcast(types: string[]) { const payload = `event: state\ndata: ${JSON.stringify({ "@type": "StateChange", changed: { [ACCOUNT]: Object.fromEntries(types.map((t) => [t, String(state.n)])) } })}\n\n`; for (const c of sseClients) c.write(payload); diff --git a/server/src/mock/handlers.ts b/server/src/mock/handlers.ts index 6702a8e..c7a637f 100644 --- a/server/src/mock/handlers.ts +++ b/server/src/mock/handlers.ts @@ -1,5 +1,5 @@ import { checkOtp } from "./auth.js"; -import { emailChanges, recordEmailChange, broadcast } from "./events.js"; +import { cardChanges, cardLog, emailChanges, recordCardChange, recordEmailChange, broadcast } from "./events.js"; import { randomUUID } from "node:crypto"; import { eventGetView, expandOccurrences, occurrenceAt, occurrenceView, parseSyntheticId, splitOccurrencePatch, syntheticId, type Occurrence } from "./recurrence.js"; import { holdUntilOf, undoStatusOf } from "./futurerelease.js"; @@ -428,7 +428,29 @@ export const handlers: Record = { // An empty `properties` list returns `id` alone, which `pick` already does. // 0.16.22 made Stalwart agree; through 0.16.21 it returned every property. "ContactCard/get": (a) => genericGet(a.accountId === SHARED_ACCOUNT ? sharedCards : cards)(a), - "ContactCard/set": genericSet(cards, "cc"), + /* + * Recorded and announced like Email/set, so the client's incremental sync + * (`ContactCard/changes`, then fetching what it names) runs here too. A + * state older than the log's window cannot be answered, as on a real server. + */ + "ContactCard/set": (a) => { + const r = genericSet(cards, "cc")(a); + nextState(); + recordCardChange({ + created: Object.values((r.created ?? {}) as Record).map((x) => x.id), + updated: Object.keys((r.updated ?? {}) as Obj), + destroyed: (r.destroyed as string[] | undefined) ?? [], + }); + broadcast(["ContactCard"]); + return r; + }, + "ContactCard/changes": (a) => { + const since = Number(a.sinceState ?? 0); + if (since < cardLog.floor) throw new MethodError("cannotCalculateChanges", "That state is too old to answer from."); + const relevant = cardChanges.filter((c) => c.state > since); + const pick = (k: "created" | "updated" | "destroyed") => [...new Set(relevant.flatMap((c) => c[k]))]; + return { accountId: a.accountId ?? ACCOUNT, oldState: String(a.sinceState ?? "1"), newState: String(state.n), hasMoreChanges: false, created: pick("created"), updated: pick("updated"), destroyed: pick("destroyed") }; + }, "ContactCard/parse": (a) => { const parsed: Obj = {}; for (const b of a.blobIds as string[]) { const t = blobs.get(b)?.data.toString() ?? ""; const fn = /^FN:(.*)$/m.exec(t)?.[1]?.trim() ?? "Imported"; const em = /^EMAIL[^:]*:(.*)$/m.exec(t)?.[1]?.trim(); parsed[b] = [{ "@type": "Card", version: "1.0", uid: randomUUID(), kind: "individual", name: { full: fn }, emails: em ? { e1: { address: em } } : undefined }]; } return { accountId: ACCOUNT, parsed, notParsable: [] }; }, "FileNode/query": (a) => { const f = (a.filter as Obj) ?? {}; diff --git a/web/src/store/__tests__/contacts-calendar-sync.test.ts b/web/src/store/__tests__/contacts-calendar-sync.test.ts new file mode 100644 index 0000000..c5d91f4 --- /dev/null +++ b/web/src/store/__tests__/contacts-calendar-sync.test.ts @@ -0,0 +1,150 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { CAP, client } from "@/jmap/client"; +import type { JmapSession } from "@/jmap/types"; +import { useContacts } from "@/store/contacts"; +import { keepRecent, RANGES_KEPT, useCalendar } from "@/store/calendar"; + +/** + * What a change to one contact or one event costs. + * + * A pushed ContactCard change used to reload the whole address book, and a + * CalendarEvent change queried every window the reader had ever visited again. + */ + +type Call = [string, Record, string]; +let calls: Call[]; +let reply: (name: string, args: Record) => unknown; + +beforeEach(() => { + calls = []; + client.session = { + capabilities: { [CAP.core]: { maxCallsInRequest: 16, maxObjectsInGet: 2 }, [CAP.contacts]: {}, [CAP.calendars]: {} }, + accounts: {}, + primaryAccounts: {}, + state: "s", + } as unknown as JmapSession; + vi.stubGlobal("fetch", vi.fn(async (_url: string, init: RequestInit) => { + const { methodCalls } = JSON.parse(init.body as string) as { methodCalls: Call[] }; + const methodResponses = methodCalls.map(([name, args, id]) => { + calls.push([name, args, id]); + const out = reply(name, args); + return out instanceof Error ? ["error", { type: out.message }, id] : [name, out, id]; + }); + return { ok: true, status: 200, json: async () => ({ methodResponses, sessionState: "s" }) } as Response; + })); +}); + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +const card = (id: string, name: string) => ({ id, addressBookIds: { b1: true }, name: { full: name } }); + +describe("syncCards", () => { + beforeEach(() => { + useContacts.setState({ + accountId: "a1", + available: true, + loaded: true, + loading: false, + cardState: "10", + cards: { c1: card("c1", "Ann"), c2: card("c2", "Bob"), c3: card("c3", "Cy") } as never, + }); + }); + + it("fetches only what changed, in batches the server takes", async () => { + reply = (name, args) => { + if (name === "ContactCard/changes") return { oldState: "10", newState: "12", hasMoreChanges: false, created: ["c4", "c5", "c6"], updated: ["c1"], destroyed: ["c2"] }; + if (name === "ContactCard/get") return { state: "12", list: (args.ids as string[]).map((id) => card(id, `new ${id}`)), notFound: [] }; + throw new Error(`unexpected ${name}`); + }; + await useContacts.getState().syncCards(); + const names = calls.map(([n]) => n); + expect(names).not.toContain("ContactCard/query"); + const gets = calls.filter(([n]) => n === "ContactCard/get").map(([, a]) => a.ids as string[]); + expect(gets.every((ids) => ids.length <= 2)).toBe(true); + expect(gets.flat().sort()).toEqual(["c1", "c4", "c5", "c6"]); + const s = useContacts.getState(); + expect(Object.keys(s.cards).sort()).toEqual(["c1", "c3", "c4", "c5", "c6"]); + expect(s.cards.c1!.name!.full).toBe("new c1"); + expect(s.cards.c3!.name!.full).toBe("Cy"); + expect(s.cardState).toBe("12"); + }); + + it("follows changes across pages", async () => { + reply = (name, args) => { + if (name === "ContactCard/changes") { + return args.sinceState === "10" + ? { oldState: "10", newState: "11", hasMoreChanges: true, created: [], updated: ["c1"], destroyed: [] } + : { oldState: "11", newState: "13", hasMoreChanges: false, created: [], updated: [], destroyed: ["c1"] }; + } + return { state: "13", list: [], notFound: [] }; + }; + await useContacts.getState().syncCards(); + // Updated on the first page and destroyed on the second: gone, and not fetched. + expect(useContacts.getState().cards.c1).toBeUndefined(); + expect(calls.filter(([n]) => n === "ContactCard/get")).toHaveLength(0); + expect(useContacts.getState().cardState).toBe("13"); + }); + + it("reloads everything when the server cannot say what changed", async () => { + reply = (name) => { + if (name === "ContactCard/changes") return new Error("cannotCalculateChanges"); + if (name === "ContactCard/query") return { ids: ["c9"], total: 1, position: 0, queryState: "q" }; + if (name === "ContactCard/get") return { state: "20", list: [card("c9", "Zed")], notFound: [] }; + throw new Error(`unexpected ${name}`); + }; + await useContacts.getState().syncCards(); + const s = useContacts.getState(); + expect(Object.keys(s.cards)).toEqual(["c9"]); + expect(s.cardState).toBe("20"); + }); + + it("records the state a full load was read at", async () => { + useContacts.setState({ cardState: null, loaded: false }); + reply = (name) => { + if (name === "ContactCard/query") return { ids: ["c1"], total: 1, position: 0, queryState: "q" }; + return { state: "30", list: [card("c1", "Ann")], notFound: [] }; + }; + await useContacts.getState().loadAll(); + expect(useContacts.getState().cardState).toBe("30"); + }); +}); + +describe("calendar windows", () => { + const day = 86_400_000; + const windowAt = (n: number) => [new Date(n * 7 * day), new Date((n + 1) * 7 * day)] as const; + + beforeEach(() => { + useCalendar.setState({ accountId: "a1", available: true, ranges: {}, sharedRanges: {}, events: {}, sharedCalendars: [] }); + reply = (name) => (name === "CalendarEvent/query" ? { ids: [], total: 0, position: 0, queryState: "q" } : { state: "1", list: [], notFound: [] }); + }); + + it("keeps only the most recent few", () => { + let ranges: Record = {}; + for (let i = 0; i < RANGES_KEPT + 3; i++) ranges = keepRecent(ranges, `k${i}`, []); + expect(Object.keys(ranges)).toEqual(Array.from({ length: RANGES_KEPT }, (_, i) => `k${i + 3}`)); + // Seeing one again moves it to the back of the queue. + ranges = keepRecent(ranges, "k3", ["e"]); + expect(Object.keys(ranges).at(-1)).toBe("k3"); + expect(ranges.k3).toEqual(["e"]); + }); + + it("queries only the windows it holds when an event changes", async () => { + const store = useCalendar.getState(); + for (let i = 0; i < 10; i++) { + const [a, b] = windowAt(i); + await store.loadRange(a, b); + } + expect(Object.keys(useCalendar.getState().ranges)).toHaveLength(RANGES_KEPT); + calls = []; + useCalendar.getState().applyChanges(new Set(["CalendarEvent"])); + await vi.waitFor(() => expect(calls.filter(([n]) => n === "CalendarEvent/query")).toHaveLength(RANGES_KEPT)); + }); + + it("does not empty the windows while they reload", () => { + useCalendar.setState({ ranges: { [`${7 * day}|${14 * day}`]: ["e1"] } }); + useCalendar.getState().invalidate(); + expect(Object.values(useCalendar.getState().ranges)).toEqual([["e1"]]); + }); +}); diff --git a/web/src/store/calendar.ts b/web/src/store/calendar.ts index b2e1052..725ceeb 100644 --- a/web/src/store/calendar.ts +++ b/web/src/store/calendar.ts @@ -498,7 +498,8 @@ export const useCalendar = create((set, get) => ({ const accounts = [...new Set(shared.map((c) => c.accountId))]; const ids: string[] = []; const events: Record = {}; - for (const accountId of accounts) { + // Every account at once, rather than one waiting on the last. + await Promise.all(accounts.map(async (accountId) => { try { const res = await client.chain([ ["CalendarEvent/query", { accountId, filter: { after: toLocalDateTime(start), before: toLocalDateTime(end) }, timeZone: tz, sort: [{ property: "start", isAscending: true }], expandRecurrences: true, limit: 2000 }, "q"], @@ -512,10 +513,9 @@ export const useCalendar = create((set, get) => ({ } } catch { // One account refusing must not empty the calendar of the others. - continue; } - } - set((s) => ({ sharedEvents: { ...s.sharedEvents, ...events }, sharedRanges: { ...s.sharedRanges, [key]: ids } })); + })); + set((s) => ({ sharedEvents: { ...s.sharedEvents, ...events }, sharedRanges: key in s.ranges ? { ...s.sharedRanges, [key]: ids } : s.sharedRanges })); }, async loadCalendars() { @@ -535,7 +535,12 @@ export const useCalendar = create((set, get) => ({ const accountId = get().accountId; if (!accountId) return; const key = `${start.getTime()}|${end.getTime()}`; - if (!force && get().ranges[key]) return; + const held = get().ranges[key]; + if (!force && held) { + // Shown again, so the last to be dropped. + set((s) => ({ ranges: keepRecent(s.ranges, key, held) })); + return; + } set({ loading: true }); const tz = settings().timeZone ?? browserTimeZone; try { @@ -560,7 +565,8 @@ export const useCalendar = create((set, get) => ({ set((s) => { const events = { ...s.events }; for (const e of g.list) events[e.id] = e; - return { events, ranges: { ...s.ranges, [key]: q.ids }, loading: false, error: null }; + const ranges = keepRecent(s.ranges, key, q.ids); + return { events, ranges, sharedRanges: onlyKeys(s.sharedRanges, ranges), loading: false, error: null }; }); void get().loadSharedRange(start, end); } catch (err) { @@ -665,6 +671,7 @@ export const useCalendar = create((set, get) => ({ hiding one is remembered under the same account-qualified key. */ const sharedKeys = new Set(); for (const list of Object.values(sharedRanges)) for (const k of list) sharedKeys.add(k); + const added = new Set(settings().addedShares); for (const k of sharedKeys) { const e = sharedEvents[k]; if (!e) continue; @@ -676,7 +683,6 @@ export const useCalendar = create((set, get) => ({ an account linked for its files offered its calendar too. `isSubscribed` is the only thing separating "shared with me" from "reachable", so nothing unsubscribed is drawn. */ - const added = new Set(settings().addedShares); const theirs: Record = {}; for (const c of sharedCalendars) { if (c.accountId !== accountId) continue; @@ -1000,11 +1006,16 @@ export const useCalendar = create((set, get) => ({ if (types.has("CalendarEvent")) get().invalidate(); }, + /* + * Load the windows held again, after something changed. + * + * Only the few most recently shown are held (see `keepRecent`), so this is a + * handful of queries rather than one for every month ever looked at. They + * are replaced where they are, not emptied first: clearing them made the + * calendar go blank until the answers came back. + */ invalidate() { - // Force reload of all ranges currently cached. - const keys = Object.keys(get().ranges); - set({ ranges: {} }); - for (const k of keys) { + for (const k of Object.keys(get().ranges)) { const [s, e] = k.split("|").map(Number) as [number, number]; void get().loadRange(new Date(s), new Date(e), true); } @@ -1015,6 +1026,27 @@ export const useCalendar = create((set, get) => ({ }, })); +/** + * How many loaded windows are held. + * + * Every week or month the reader visited used to stay, and each was queried + * again whenever any event changed, and walked by every render. A window + * dropped here is simply loaded again if the reader goes back to it. + */ +export const RANGES_KEPT = 4; + +/** `ranges` with `key` set and moved to the end, trimmed to the most recent `RANGES_KEPT`. */ +export function keepRecent(ranges: Record, key: string, ids: Id[]): Record { + const { [key]: _old, ...rest } = ranges; + const entries = [...Object.entries(rest), [key, ids] as [string, Id[]]]; + return Object.fromEntries(entries.slice(-RANGES_KEPT)); +} + +/** `map` restricted to the windows still held. */ +function onlyKeys(map: Record, held: Record): Record { + return Object.fromEntries(Object.entries(map).filter(([k]) => k in held)); +} + /** * Whether an event is part of a series. * diff --git a/web/src/store/contacts.ts b/web/src/store/contacts.ts index ecc6d62..853b971 100644 --- a/web/src/store/contacts.ts +++ b/web/src/store/contacts.ts @@ -1,7 +1,7 @@ import { create } from "zustand"; import { accountKey, loadRaw, saveJson } from "@/lib/storage"; -import { CAP, chunk, client, setErrorMessage } from "@/jmap/client"; -import type { AddressBook, ContactCard, EmailAddress, GetResponse, Id, Principal, QueryResponse, SetError, SetResponse } from "@/jmap/types"; +import { CAP, chunk, client, JmapMethodError, setErrorMessage } from "@/jmap/client"; +import type { AddressBook, ChangesResponse, ContactCard, EmailAddress, GetResponse, Id, Principal, QueryResponse, SetError, SetResponse } from "@/jmap/types"; import { contactDisplayName, contactEmails, sortKey } from "@/lib/contacts"; import { parseLdif, uidFromDn } from "@/lib/ldif"; import { cardFromLdif } from "@/lib/mozillaAb"; @@ -173,6 +173,8 @@ interface ContactsState { available: boolean; books: Record; cards: Record; + /** The server's ContactCard state `cards` was read at, for asking what changed since. */ + cardState: string | null; loaded: boolean; loading: boolean; error: string | null; @@ -189,6 +191,11 @@ interface ContactsState { init(): Promise; loadBooks(): Promise; loadAll(): Promise; + /** + * Bring `cards` up to date with what changed on the server, or load them all + * when that cannot be worked out. + */ + syncCards(): Promise; /** Books and cards from accounts that shared with the reader. */ loadShared(): Promise; select(selection: BookSelection): void; @@ -247,6 +254,7 @@ export const useContacts = create((set, get) => ({ available: false, books: {}, cards: {}, + cardState: null, loaded: false, loading: false, error: null, @@ -264,7 +272,7 @@ export const useContacts = create((set, get) => ({ // should move when the switcher does. const accountId = useSession.getState().ownAccountFor(CAP.contacts); const available = Boolean(accountId && client.hasCapability(CAP.contacts)); - if (accountId !== get().accountId) set({ accountId, books: {}, cards: {}, loaded: false, selection: { accountId: null, bookId: "all" } }); + if (accountId !== get().accountId) set({ accountId, books: {}, cards: {}, cardState: null, loaded: false, selection: { accountId: null, bookId: "all" } }); set({ available }); if (!available) return; await get().loadBooks(); @@ -408,6 +416,7 @@ export const useContacts = create((set, get) => ({ const cards: Record = {}; let position = 0; const limit = 500; + let cardState: string | null = null; for (let guard = 0; guard < 50; guard++) { const res = await client.chain([ ["ContactCard/query", { accountId, position, limit, calculateTotal: true }, "q"], @@ -416,15 +425,63 @@ export const useContacts = create((set, get) => ({ const q = res.get("q")?.[0] as unknown as QueryResponse; const g = res.get("g")?.[0] as unknown as GetResponse; for (const c of g.list) cards[c.id] = c; + // The first page's: a change made while the rest were being read is + // then reported again by the next sync, rather than missed. + cardState ??= g.state; position += q.ids.length; if (q.ids.length < limit || (q.total != null && position >= q.total)) break; } - set({ cards, loaded: true, loading: false, error: null }); + set({ cards, cardState, loaded: true, loading: false, error: null }); } catch (err) { set({ loading: false, error: (err as Error).message }); } }, + /* + * What changed, rather than everything again. + * + * Every push that touched a card, and every edit made here, used to reload + * the whole address book -- up to fifty pages of five hundred cards with all + * their properties -- to pick up one change. ContactCard/changes names what + * changed since the state the cards were read at, and only those are fetched. + * A server that cannot say (`cannotCalculateChanges`), or any other failure, + * falls back to the full load, which is what happened before. + */ + async syncCards() { + const { accountId, cardState, loaded } = get(); + if (!accountId || !loaded || !cardState) return get().loadAll(); + try { + const changed = new Set(); + const destroyed = new Set(); + let since = cardState; + for (let guard = 0; guard < 50; guard++) { + const ch = await client.call("ContactCard/changes", { accountId, sinceState: since, maxChanges: 500 }); + for (const id of [...ch.created, ...ch.updated]) { changed.add(id); destroyed.delete(id); } + for (const id of ch.destroyed) { destroyed.add(id); changed.delete(id); } + since = ch.newState; + if (!ch.hasMoreChanges) break; + } + const fetched = await Promise.all( + chunk([...changed], client.maxObjectsInGet).map((part) => client.call>("ContactCard/get", { accountId, ids: part })), + ); + if (get().accountId !== accountId) return; + set((s) => { + const cards = { ...s.cards }; + for (const id of destroyed) delete cards[id]; + for (const r of fetched) { + for (const c of r.list) cards[c.id] = c; + // An id listed as changed but gone by the time it was asked for. + for (const id of r.notFound ?? []) delete cards[id]; + } + return { cards, cardState: since, error: null }; + }); + } catch (err) { + if (!(err instanceof JmapMethodError) || err.type !== "cannotCalculateChanges") console.warn("[ihasmail] contact sync failed, reloading:", err); + set({ cardState: null }); + await get().loadAll(); + } + }, + async getCard(id) { const accountId = get().accountId; if (!accountId) return null; @@ -546,7 +603,7 @@ export const useContacts = create((set, get) => ({ refused ??= Object.values(res.notDestroyed ?? {})[0] ?? Object.values(res.notUpdated ?? {})[0]; } } finally { - await get().loadAll(); + await get().syncCards(); } return { destroyed: gone.length, unfiled, refused }; }, @@ -574,7 +631,7 @@ export const useContacts = create((set, get) => ({ const err = res.notDestroyed?.[id]; if (err) throw new Error(setErrorMessage(err)); await get().loadBooks(); - await get().loadAll(); + await get().syncCards(); }, async importVCard(text, addressBookId) { @@ -623,7 +680,7 @@ export const useContacts = create((set, get) => ({ matched on it rather than guessed at. */ return { created, updated, alike: 0 }; } finally { - await get().loadAll(); + await get().syncCards(); } }, @@ -691,7 +748,7 @@ export const useContacts = create((set, get) => ({ if (!created && !updated) throw new Error(refused ? setErrorMessage(refused) : "the server did not accept any of its contacts"); return { created, updated, alike }; } finally { - await get().loadAll(); + await get().syncCards(); } }, @@ -786,7 +843,7 @@ export const useContacts = create((set, get) => ({ applyChanges(types) { if (types.has("AddressBook")) { void get().loadBooks(); void get().loadShared(); } - if (types.has("ContactCard") && get().loaded) void get().loadAll(); + if (types.has("ContactCard") && get().loaded) void get().syncCards(); }, }));