Merge pull request #383 from Coffey-Labs/perf/lazy-store-init

Ask shared accounts together, and about files only when Files opens

web/src/store/__tests__/shared-accounts-batched.test.ts

@@ -0,0 +1,81 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import { CAP, client } from "@/jmap/client";
+import type { JmapSession } from "@/jmap/types";
+import { useSession } from "@/store/session";
+import { useFiles } from "@/store/files";
+import { useContacts } from "@/store/contacts";
+import { useCalendar } from "@/store/calendar";
+
+/**
+ * What signing in costs for each account somebody has shared with the reader.
+ *
+ * The files, contacts and calendar stores each asked every shared account a
+ * question at sign-in, one account after another -- a request apiece, before
+ * the reader had opened any of those views. The questions now go out together,
+ * and Files does not ask at all until it is opened.
+ */
+
+const SHARED = ["s1", "s2", "s3"];
+
+const session = {
+  capabilities: { [CAP.core]: { maxCallsInRequest: 16, maxObjectsInGet: 500 }, [CAP.filenode]: {}, [CAP.contacts]: {}, [CAP.calendars]: {} },
+  accounts: {
+    own: { name: "[email protected]", isPersonal: true, accountCapabilities: { [CAP.filenode]: {}, [CAP.contacts]: {}, [CAP.calendars]: {} } },
+    ...Object.fromEntries(SHARED.map((id) => [id, { name: `${id}@example.com`, isPersonal: false, accountCapabilities: {} }])),
+  },
+  primaryAccounts: { [CAP.filenode]: "own", [CAP.contacts]: "own", [CAP.calendars]: "own" },
+  state: "s",
+} as unknown as JmapSession;
+
+type Call = [string, Record<string, unknown>, string];
+
+let requests: Call[][];
+
+beforeEach(() => {
+  requests = [];
+  vi.stubGlobal("fetch", vi.fn(async (_url: string, init: RequestInit) => {
+    const { methodCalls } = JSON.parse(init.body as string) as { methodCalls: Call[] };
+    requests.push(methodCalls);
+    const methodResponses = methodCalls.map(([name, args, id]) => {
+      const accountId = args.accountId as string;
+      if (name === "FileNode/query") return [name, { accountId, ids: accountId === "s2" ? ["f1"] : [], total: 0, position: 0, queryState: "q" }, id];
+      if (name === "Calendar/get") return [name, { accountId, state: "1", list: [{ id: `cal-${accountId}`, name: `Calendar of ${accountId}` }], notFound: [] }, id];
+      return [name, { accountId, state: "1", list: [], notFound: [] }, id];
+    });
+    return { ok: true, status: 200, json: async () => ({ methodResponses, sessionState: "s" }) } as Response;
+  }));
+  client.session = session;
+  useSession.setState({ status: "authenticated", session, accountId: "own" });
+});
+
+afterEach(() => {
+  vi.unstubAllGlobals();
+});
+
+describe("shared accounts", () => {
+  it("are not asked about files at sign-in", async () => {
+    await useFiles.getState().init();
+    expect(requests).toHaveLength(0);
+    expect(useFiles.getState().available).toBe(true);
+    expect(useFiles.getState().ownAccountId).toBe("own");
+  });
+
+  it("are asked about files together when Files wants to know", async () => {
+    await useFiles.getState().discoverShared();
+    expect(requests).toHaveLength(1);
+    expect(requests[0]!.map(([n, a]) => `${n} ${a.accountId}`)).toEqual(SHARED.map((id) => `FileNode/query ${id}`));
+    expect(useFiles.getState().sharedAccounts).toEqual([{ id: "s2", name: "[email protected]" }]);
+  });
+
+  it("are asked about address books in one request", async () => {
+    await useContacts.getState().loadShared();
+    expect(requests).toHaveLength(1);
+    expect(requests[0]!.filter(([n]) => n === "AddressBook/get")).toHaveLength(3);
+  });
+
+  it("are asked about calendars in one request, and listed in the session's order", async () => {
+    await useCalendar.getState().loadSharedCalendars();
+    expect(requests).toHaveLength(1);
+    expect(useCalendar.getState().sharedCalendars.map((c) => c.accountId)).toEqual(SHARED);
+  });
+});

web/src/store/calendar.ts

@@ -409,14 +409,13 @@ export const useCalendar = create<CalendarState>((set, get) => ({
     if (accountId !== get().accountId) set({ accountId, calendars: {}, events: {}, ranges: {} });
     set({ available });
     if (!available) return;
-    await get().loadCalendars();
+    // Side by side: none of the three waits on another, and together they share a request.
+    const identities = client.call<GetResponse<ParticipantIdentity>>("ParticipantIdentity/get", { accountId, ids: null }).then(
+      (res) => set({ identities: res.list }),
+      () => set({ identities: [] }),
+    );
     void get().loadSharedCalendars();
-    try {
-      const res = await client.call<GetResponse<ParticipantIdentity>>("ParticipantIdentity/get", { accountId, ids: null });
-      set({ identities: res.list });
-    } catch {
-      set({ identities: [] });
-    }
+    await Promise.all([get().loadCalendars(), identities]);
   },
 
   /*
@@ -434,15 +433,16 @@ export const useCalendar = create<CalendarState>((set, get) => ({
     const session = useSession.getState();
     const own = session.ownAccountFor(CAP.calendars);
     const accounts = Object.entries(session.session?.accounts ?? {}).filter(([id, a]) => a.isPersonal === false && id !== own);
-    const found: SharedCalendar[] = [];
-    for (const [accountId, account] of accounts) {
-      try {
-        const res = await client.call<GetResponse<Calendar>>("Calendar/get", { accountId, ids: null, properties: CALENDAR_PROPS });
-        for (const calendar of res.list) found.push({ accountId, accountName: account.name, calendar });
-      } catch {
-        continue;
-      }
-    }
+    // Every account at once, in one request, and listed in the session's order.
+    const answers = await Promise.all(
+      accounts.map(([accountId, account]) =>
+        client.call<GetResponse<Calendar>>("Calendar/get", { accountId, ids: null, properties: CALENDAR_PROPS }).then(
+          (res) => res.list.map((calendar): SharedCalendar => ({ accountId, accountName: account.name, calendar })),
+          (): SharedCalendar[] => [],
+        ),
+      ),
+    );
+    const found = answers.flat();
     set({ sharedCalendars: found });
     // Fill in whatever windows are already on screen.
     for (const key of Object.keys(get().ranges)) {

web/src/store/contacts.ts

@@ -294,7 +294,9 @@ export const useContacts = create<ContactsState>((set, get) => ({
     }
     const books: SharedBook[] = [];
     const cards: Record<string, ContactCard> = {};
-    for (const [accountId, account] of accounts) {
+    // Every account at once: calls made in one tick share a request, where a
+    // loop sent one after another for each account shared with the reader.
+    await Promise.all(accounts.map(async ([accountId, account]) => {
       try {
         const res = await client.call<GetResponse<AddressBook>>("AddressBook/get", { accountId, ids: null, properties: ADDRESS_BOOK_PROPS });
         for (const book of res.list) books.push({ accountId, accountName: account.name, book });
@@ -310,7 +312,7 @@ export const useContacts = create<ContactsState>((set, get) => ({
          */
         const added = new Set(useSettings.getState().settings.addedShares);
         const wanted = new Set(res.list.filter((b) => b.isSubscribed || added.has(sharedKey(accountId, b.id))).map((b) => b.id));
-        if (!wanted.size) continue;
+        if (!wanted.size) return;
         // One page. A shared book is a colleague's contacts, not an archive,
         // and the alternative is holding the reader's own list hostage to it.
         const cardsRes = await client.chain([
@@ -325,9 +327,11 @@ export const useContacts = create<ContactsState>((set, get) => ({
       } catch {
         // An account that refuses is one that shared nothing here. Not an
         // error to show: the reader did not ask for it and cannot act on it.
-        continue;
       }
-    }
+    }));
+    // Answers arrive in any order; list the books in the session's.
+    const order = new Map(accounts.map(([id], i) => [id, i]));
+    books.sort((a, b) => (order.get(a.accountId) ?? 0) - (order.get(b.accountId) ?? 0));
     set({ sharedBooks: books, sharedCards: cards, sharedLoaded: true });
   },
 

web/src/store/files.ts

@@ -50,7 +50,10 @@ interface FilesState {
    */
   draggingIds: Id[];
 
+  /** Whether Files is available and which account is the reader's. No round trip. */
   init(): Promise<void>;
+  /** Ask each shared account whether it holds files; see the note on it. */
+  discoverShared(): Promise<void>;
   /** Browse an account: the reader's own, or one shared with them. */
   openAccount(accountId: Id | null): void;
   loadChildren(parentId: Id | null): Promise<void>;
@@ -138,6 +141,17 @@ export const useFiles = create<FilesState>((set, get) => ({
     const session = useSession.getState();
     const ownAccountId = session.ownAccountFor(CAP.filenode);
     const available = Boolean(ownAccountId && client.hasCapability(CAP.filenode));
+    // Stay where the reader is if the session still offers that account;
+    // whether it still holds files is `discoverShared`'s to say.
+    const browsing = get().accountId;
+    const offered = Object.entries(session.session?.accounts ?? {}).some(([id, a]) => id === browsing && a.isPersonal === false);
+    if (!(browsing && (browsing === ownAccountId || offered))) set(emptyForAccount(ownAccountId));
+    set({ available, ownAccountId });
+  },
+
+  async discoverShared() {
+    const session = useSession.getState();
+    const ownAccountId = get().ownAccountId;
     /*
      * Which accounts hold shared files cannot be worked out from capabilities:
      * Stalwart advertises the whole set on a shared account -- mail, calendars,
@@ -151,23 +165,29 @@ export const useFiles = create<FilesState>((set, get) => ({
      * account whose calendar or contacts were the thing actually shared. An
      * account that shares no files does not belong in a list of shared files.
      */
+    /*
+     * Not at sign-in: only the Files view and the file picker list shared
+     * accounts, and each opening asks afresh. The questions go out together --
+     * calls made in one tick share a request -- rather than one account after
+     * another.
+     */
     const s = session.session;
     const candidates = Object.entries(s?.accounts ?? {}).filter(([, a]) => a.isPersonal === false);
-    const sharedAccounts: SharedAccount[] = [];
-    for (const [id, a] of candidates) {
-      try {
-        const res = await client.call<QueryResponse>("FileNode/query", { accountId: id, limit: 1 });
-        if (res.ids.length) sharedAccounts.push({ id, name: a.name });
-      } catch {
-        // Refused means nothing here is ours to see, which is the same answer.
-        continue;
-      }
-    }
+    const answers = await Promise.all(
+      candidates.map(([id, a]) =>
+        client.call<QueryResponse>("FileNode/query", { accountId: id, limit: 1 }).then(
+          (res): SharedAccount | null => (res.ids.length ? { id, name: a.name } : null),
+          // Refused means nothing here is ours to see, which is the same answer.
+          () => null,
+        ),
+      ),
+    );
+    const sharedAccounts = answers.filter((a): a is SharedAccount => a !== null);
     // Stay where the reader is if they are reading a share that still exists.
     const browsing = get().accountId;
     const keep = browsing && (browsing === ownAccountId || sharedAccounts.some((a) => a.id === browsing));
     if (!keep) set(emptyForAccount(ownAccountId));
-    set({ available, ownAccountId, sharedAccounts });
+    set({ sharedAccounts });
   },
 
   openAccount(accountId) {

web/src/views/compose/FilePicker.tsx

@@ -27,6 +27,11 @@ export function FilePicker({ onPick, onClose }: { onPick: (files: AttachableFile
   const [picked, setPicked] = useState<Record<string, FileNode>>({});
   const [returnTo] = useState(() => files.accountId);
 
+  // Shared accounts are not looked for at sign-in; the picker lists them, so it asks.
+  useEffect(() => {
+    void useFiles.getState().discoverShared();
+  }, []);
+
   useEffect(() => {
     void files.loadChildren(cur);
     // eslint-disable-next-line react-hooks/exhaustive-deps

web/src/views/files/FilesTree.tsx

@@ -32,6 +32,7 @@ async function refreshShares(force = false): Promise<void> {
     return;
   }
   await useFiles.getState().init();
+  await useFiles.getState().discoverShared();
 }