From 4c7b2ec370a910c0a324921a1dc15143b754f915 Mon Sep 17 00:00:00 2001 From: John Coffey Date: Wed, 16 Sep 2026 10:04:24 -0700 Subject: [PATCH] Ask shared accounts together, and about files only when Files opens At sign-in the files, contacts and calendar stores each asked every shared account a question, one account after another: a request apiece before the reader had opened any of those views. Files now only works out at sign-in whether it is available. Which shared accounts hold files is asked when the Files view or the file picker opens, which the Files view already did on every visit. Shared address books and calendars are asked for in one request, and the calendar store loads its calendars, identities and shared calendars side by side. --- .../__tests__/shared-accounts-batched.test.ts | 81 +++++++++++++++++++ web/src/store/calendar.ts | 32 ++++---- web/src/store/contacts.ts | 12 ++- web/src/store/files.ts | 42 +++++++--- web/src/views/compose/FilePicker.tsx | 5 ++ web/src/views/files/FilesTree.tsx | 1 + 6 files changed, 142 insertions(+), 31 deletions(-) create mode 100644 web/src/store/__tests__/shared-accounts-batched.test.ts diff --git a/web/src/store/__tests__/shared-accounts-batched.test.ts b/web/src/store/__tests__/shared-accounts-batched.test.ts new file mode 100644 index 0000000..2001a03 --- /dev/null +++ b/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: "me@example.com", 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]; + +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: "s2@example.com" }]); + }); + + 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); + }); +}); diff --git a/web/src/store/calendar.ts b/web/src/store/calendar.ts index 1cbb5ec..b2e1052 100644 --- a/web/src/store/calendar.ts +++ b/web/src/store/calendar.ts @@ -409,14 +409,13 @@ export const useCalendar = create((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>("ParticipantIdentity/get", { accountId, ids: null }).then( + (res) => set({ identities: res.list }), + () => set({ identities: [] }), + ); void get().loadSharedCalendars(); - try { - const res = await client.call>("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((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>("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>("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)) { diff --git a/web/src/store/contacts.ts b/web/src/store/contacts.ts index 1c3d75d..ecc6d62 100644 --- a/web/src/store/contacts.ts +++ b/web/src/store/contacts.ts @@ -294,7 +294,9 @@ export const useContacts = create((set, get) => ({ } const books: SharedBook[] = []; const cards: Record = {}; - 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>("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((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((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 }); }, diff --git a/web/src/store/files.ts b/web/src/store/files.ts index 9290806..1bd9256 100644 --- a/web/src/store/files.ts +++ b/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; + /** Ask each shared account whether it holds files; see the note on it. */ + discoverShared(): Promise; /** Browse an account: the reader's own, or one shared with them. */ openAccount(accountId: Id | null): void; loadChildren(parentId: Id | null): Promise; @@ -138,6 +141,17 @@ export const useFiles = create((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((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("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("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) { diff --git a/web/src/views/compose/FilePicker.tsx b/web/src/views/compose/FilePicker.tsx index 663ce7f..8d243a3 100644 --- a/web/src/views/compose/FilePicker.tsx +++ b/web/src/views/compose/FilePicker.tsx @@ -27,6 +27,11 @@ export function FilePicker({ onPick, onClose }: { onPick: (files: AttachableFile const [picked, setPicked] = useState>({}); 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 diff --git a/web/src/views/files/FilesTree.tsx b/web/src/views/files/FilesTree.tsx index 0c491ae..23d58bb 100644 --- a/web/src/views/files/FilesTree.tsx +++ b/web/src/views/files/FilesTree.tsx @@ -32,6 +32,7 @@ async function refreshShares(force = false): Promise { return; } await useFiles.getState().init(); + await useFiles.getState().discoverShared(); }