Sync contacts by what changed, and hold fewer calendar windows

A pushed contact change, and every edit or import made here, reloaded the
whole address book. The store now keeps the state its cards were read at
and asks ContactCard/changes what changed since, fetching only those cards,
split to maxObjectsInGet. A server that cannot say falls back to the full
load.

The calendar held every week or month the reader had visited, queried each
of them again on any event change, and walked them all on every render. It
now holds the four most recently shown; a change reloads those in place,
without emptying the view first, and a window dropped is loaded again when
it is next shown. Shared calendars' events are fetched from every account
at once, and instancesIn builds the added-shares set once.

The mock keeps a ContactCard change log, answers ContactCard/changes, and
announces a ContactCard/set, as Stalwart does.

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, unknown>, string];
+let calls: Call[];
+let reply: (name: string, args: Record<string, unknown>) => 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<string, string[]> = {};
+    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"]]);
+  });
+});

web/src/store/calendar.ts

@@ -498,7 +498,8 @@ export const useCalendar = create<CalendarState>((set, get) => ({
     const accounts = [...new Set(shared.map((c) => c.accountId))];
     const ids: string[] = [];
     const events: Record<string, CalendarEvent> = {};
-    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<CalendarState>((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<CalendarState>((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<CalendarState>((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<CalendarState>((set, get) => ({
        hiding one is remembered under the same account-qualified key. */
     const sharedKeys = new Set<string>();
     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<CalendarState>((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<Id, Calendar> = {};
       for (const c of sharedCalendars) {
         if (c.accountId !== accountId) continue;
@@ -1000,11 +1006,16 @@ export const useCalendar = create<CalendarState>((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<CalendarState>((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<string, Id[]>, key: string, ids: Id[]): Record<string, Id[]> {
+  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<T>(map: Record<string, T>, held: Record<string, unknown>): Record<string, T> {
+  return Object.fromEntries(Object.entries(map).filter(([k]) => k in held));
+}
+
 /**
  * Whether an event is part of a series.
  *

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<Id, AddressBook>;
   cards: Record<Id, ContactCard>;
+  /** 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<void>;
   loadBooks(): Promise<void>;
   loadAll(): Promise<void>;
+  /**
+   * Bring `cards` up to date with what changed on the server, or load them all
+   * when that cannot be worked out.
+   */
+  syncCards(): Promise<void>;
   /** Books and cards from accounts that shared with the reader. */
   loadShared(): Promise<void>;
   select(selection: BookSelection): void;
@@ -247,6 +254,7 @@ export const useContacts = create<ContactsState>((set, get) => ({
   available: false,
   books: {},
   cards: {},
+  cardState: null,
   loaded: false,
   loading: false,
   error: null,
@@ -264,7 +272,7 @@ export const useContacts = create<ContactsState>((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<ContactsState>((set, get) => ({
       const cards: Record<Id, ContactCard> = {};
       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<ContactsState>((set, get) => ({
         const q = res.get("q")?.[0] as unknown as QueryResponse;
         const g = res.get("g")?.[0] as unknown as GetResponse<ContactCard>;
         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<Id>();
+      const destroyed = new Set<Id>();
+      let since = cardState;
+      for (let guard = 0; guard < 50; guard++) {
+        const ch = await client.call<ChangesResponse>("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<GetResponse<ContactCard>>("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<ContactsState>((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<ContactsState>((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<ContactsState>((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<ContactsState>((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<ContactsState>((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();
   },
 }));