diff --git a/.changeset/29444-home-roster-gate-batch-read.md b/.changeset/29444-home-roster-gate-batch-read.md new file mode 100644 index 00000000000..3ac706e74b6 --- /dev/null +++ b/.changeset/29444-home-roster-gate-batch-read.md @@ -0,0 +1,10 @@ +--- +'@object-ui/app-shell': patch +--- + +perf(app-shell): Home's navigation roster gate asks the objects it requires in one `GET /meta/object?names=a,b,c` request, instead of one by-name read per object + +Clause-②: no +Metadata-protocol: none + +Outside an app (Home, whose activity feeds gate every app's navigation), the runtime-capability gate for `requiresObject` entries read each required object by name, one `GET /meta/object/NAME` per name (eight on app-crm). It now asks the names it has not asked yet in one `?names=` request (`meta.getItems('object', { names })`). A name the answer leaves out is treated as not registered, exactly as a by-name `404` was, so its entry stays hidden. A request that gets no answer leaves every name unanswered, so each entry shows, as when a by-name read failed. A refresh asks the same names again in one request. Inside an app nothing changes: the gate still reads the union by name, so the record form keeps sharing those reads with the data adapter's cache. Under the draft preview the gate also keeps the by-name reads. diff --git a/ui/packages/app-shell/src/console/home/__tests__/homeObjectReads-29121.test.tsx b/ui/packages/app-shell/src/console/home/__tests__/homeObjectReads-29121.test.tsx index e81c70c6baf..5cad87f82f6 100644 --- a/ui/packages/app-shell/src/console/home/__tests__/homeObjectReads-29121.test.tsx +++ b/ui/packages/app-shell/src/console/home/__tests__/homeObjectReads-29121.test.tsx @@ -17,7 +17,8 @@ * * These cases count the reads the REAL `MetadataProvider` sends to a fake * adapter, outside an app, for each hook reader: the names each asks for, by - * name, and neither list. The CONTROL: once the whole list is loaded, a reader + * name, and neither list; the roster gate asks its names in one `?names=` + * request (objectstack#29444). The CONTROL: once the whole list is loaded, a reader * reads it and asks nothing. `HomeLayout`'s palette and `HomePage`'s recent * rows are counted in their own files (`HomeLayout.paletteReads-29121`, * `HomePage.recentObjectReads-29121`). @@ -26,7 +27,7 @@ import '@testing-library/jest-dom/vitest'; import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest'; import React from 'react'; -import { cleanup, render, screen, waitFor } from '@testing-library/react'; +import { act, cleanup, render, screen, waitFor } from '@testing-library/react'; import { MetadataProvider, useMetadata } from '../../../providers/MetadataProvider'; import { readObjectsByName, useObjectPresence } from '../../../hooks/useObjectPresence'; import { useNavMemberGates } from '../../../hooks/useNavMemberGates'; @@ -59,7 +60,7 @@ const APPS = [ }, ]; -function makeAdapter(calls: string[]) { +function makeAdapter(calls: string[], failBatch = false) { return { clearCache: vi.fn(), invalidateCache: vi.fn(), @@ -70,10 +71,21 @@ function makeAdapter(calls: string[]) { }, getClient: () => ({ meta: { - getItems: (type: string, options?: { packageId?: string }) => { - calls.push(options?.packageId ? `${type}?package=${options.packageId}` : type); + getItems: (type: string, options?: { packageId?: string; names?: readonly string[] }) => { + calls.push( + options?.packageId + ? `${type}?package=${options.packageId}` + : options?.names + ? `${type}?names=${options.names.join(',')}` + : type, + ); + if (failBatch && options?.names) return Promise.reject(new Error('network down')); const all = type === 'object' ? OBJECTS : type === 'app' ? APPS : []; - const items = options?.packageId ? all.filter((o: any) => o._packageId === options.packageId) : all; + const items = options?.packageId + ? all.filter((o: any) => o._packageId === options.packageId) + : options?.names + ? all.filter((o: any) => options.names!.includes(o.name)) + : all; return Promise.resolve({ type, items: structuredClone(items) }); }, getItem: (type: string, name: string) => { @@ -90,9 +102,9 @@ function makeAdapter(calls: string[]) { const metaReads = (calls: string[]) => calls.filter((c) => c === 'object' || c === 'view' || c.startsWith('object?') || c.startsWith('object/')); -function mount(children: React.ReactNode): string[] { +function mount(children: React.ReactNode, options: { failBatch?: boolean } = {}): string[] { const calls: string[] = []; - render({children}); + render({children}); return calls; } @@ -140,12 +152,52 @@ describe('Home asks for the objects it names, never the whole registry (objectst expect(metaReads(calls)).toEqual(['object/sys_activity']); }); - it('outside an app, the roster’s capability gate asks the app list’s required objects by name, and answers as the whole list did', async () => { + // objectstack#29444 — in ONE `?names=` request, no longer one by-name read + // per name. `sys_cloud_only` is not in this deployment: the answer leaves it + // out, and the gate hides its entry, as it did on the by-name read's 404. + it('outside an app, the roster’s capability gate asks the app list’s required objects in one ?names= request, and answers as the whole list did', async () => { const calls = mount(); await waitFor(() => expect(screen.getByTestId('gate')).toHaveAttribute('data-cloud', 'false')); expect(screen.getByTestId('gate')).toHaveAttribute('data-queue', 'true'); await settle(); - expect([...metaReads(calls)].sort()).toEqual(['object/sys_cloud_only', 'object/sys_queue']); + expect(metaReads(calls)).toEqual(['object?names=sys_cloud_only,sys_queue']); + }); + + it('outside an app, a ?names= read that obtained no answer leaves every name unanswered: each entry shows, and nothing is asked again', async () => { + const calls = mount(, { failBatch: true }); + await waitFor(() => expect(metaReads(calls)).toEqual(['object?names=sys_cloud_only,sys_queue'])); + await settle(); + expect(screen.getByTestId('gate')).toHaveAttribute('data-cloud', 'true'); + expect(screen.getByTestId('gate')).toHaveAttribute('data-queue', 'true'); + expect(metaReads(calls)).toEqual(['object?names=sys_cloud_only,sys_queue']); + }); + + it('outside an app, a refresh asks the roster’s names again in one ?names= request', async () => { + const handle: { refresh?: (type?: string) => Promise } = {}; + function RefreshHandle({ into }: { into: typeof handle }) { + const { refresh } = useMetadata(); + React.useEffect(() => { + into.refresh = refresh; + }, [into, refresh]); + return null; + } + const calls = mount( + <> + + + , + ); + await waitFor(() => expect(screen.getByTestId('gate')).toHaveAttribute('data-cloud', 'false')); + await settle(); + await act(async () => { + await handle.refresh!('object'); + }); + await settle(); + expect(screen.getByTestId('gate')).toHaveAttribute('data-cloud', 'false'); + expect(metaReads(calls).filter((c) => c.startsWith('object?names=') || c.startsWith('object/'))).toEqual([ + 'object?names=sys_cloud_only,sys_queue', + 'object?names=sys_cloud_only,sys_queue', + ]); }); it('an activity row names its object from a read of that object alone', async () => { diff --git a/ui/packages/app-shell/src/hooks/__tests__/navGateUnionScope-12980.test.tsx b/ui/packages/app-shell/src/hooks/__tests__/navGateUnionScope-12980.test.tsx index 9da9d070cf0..93dfd34b639 100644 --- a/ui/packages/app-shell/src/hooks/__tests__/navGateUnionScope-12980.test.tsx +++ b/ui/packages/app-shell/src/hooks/__tests__/navGateUnionScope-12980.test.tsx @@ -77,13 +77,24 @@ function makeAdapter(calls: string[], apps: unknown[], behaviour: Behaviour = {} }, getClient: () => ({ meta: { - getItems: (type: string, options?: { packageId?: string }) => { - calls.push(options?.packageId ? `${type}?package=${options.packageId}` : type); - if (type === 'object' && !options?.packageId && behaviour.failWholeList) { + getItems: (type: string, options?: { packageId?: string; names?: readonly string[] }) => { + calls.push( + options?.packageId + ? `${type}?package=${options.packageId}` + : options?.names + ? `${type}?names=${options.names.join(',')}` + : type, + ); + if (type === 'object' && !options?.packageId && !options?.names && behaviour.failWholeList) { return Promise.reject(Object.assign(new Error('unavailable'), { httpStatus: 503 })); } - const all = (type === 'object' ? OBJECTS : type === 'app' ? apps : []) as Array<{ _packageId?: string }>; - const items = options?.packageId ? all.filter((o) => o._packageId === options.packageId) : all; + const all = (type === 'object' ? OBJECTS : type === 'app' ? apps : []) as Array<{ name?: string; _packageId?: string }>; + // objectstack#29444 — `?names=` keeps the items of those names; an unknown name is simply absent. + const items = options?.packageId + ? all.filter((o) => o._packageId === options.packageId) + : options?.names + ? all.filter((o) => options.names!.includes(o.name ?? '')) + : all; return Promise.resolve({ type, items: structuredClone(items) }); }, getItem: (type: string, name: string) => { @@ -212,7 +223,7 @@ describe('package-less app: the nav gate reads the union of required objects by expect(answers(failed(), NOTES_APP, APPS).sidebar).toEqual(['nav_note', 'nav_notes_gone']); }); - it('objectstack#29121 — outside an app (no app name, the home feeds), the gate reads the app list’s union by name, never the whole registry', async () => { + it('objectstack#29121 — outside an app (no app name, the home feeds), the gate reads the app list’s union in one ?names= request (objectstack#29444), never the whole registry', async () => { const calls: string[] = []; const read = mount({}, APPS, calls); await waitFor(() => expect(read().gates.checkCapability('object', 'legacy_gone')).toBe(false)); @@ -220,6 +231,9 @@ describe('package-less app: the nav gate reads the union of required objects by expect(read().gates.checkCapability('object', 'crm_account')).toBe(true); expect(objectCalls(calls)).not.toContain('object'); + expect(objectCalls(calls).filter((c) => c.startsWith('object?names=') || c.startsWith('object/'))).toEqual([ + 'object?names=area_gone,crm_account,legacy_gone,note,notes_gone', + ]); expect(calls).not.toContain('view'); expect(read().ctx.getTypeStatus!('object')).toBe('idle'); }); diff --git a/ui/packages/app-shell/src/hooks/useNavMemberGates.ts b/ui/packages/app-shell/src/hooks/useNavMemberGates.ts index 77709959ca9..26e495788c5 100644 --- a/ui/packages/app-shell/src/hooks/useNavMemberGates.ts +++ b/ui/packages/app-shell/src/hooks/useNavMemberGates.ts @@ -102,6 +102,15 @@ import type { ReadNamesWithoutViews } from './useObjectPresence.js'; const NO_MISSING: readonly string[] = []; +/** + * `readObjectScope` with the provider's package-local `batch: true` hand-off + * beside `views: false` (objectstack#29444, `MetadataProvider`): the names not + * asked yet are asked in one `?names=` request. + */ +type ReadNamesInOneRequest = ( + scope: { names: readonly string[]; views: false; batch: true }, +) => ReturnType; + /** No app requires an object: nothing is gated, and nothing is asked. */ const NO_REQUIRED_OBJECTS = Object.freeze({ objects: [] as readonly unknown[], missing: NO_MISSING }); @@ -383,8 +392,19 @@ export function useNavMemberGates(forApp?: Record): NavMemberGa // hold, so composing them read the whole `/meta/view` list on every route // of such an app. The route's own objects are composed with their views by // the route's reader (`AppContent`), not here. + // + // objectstack#29444 — outside an app (a frame whose app has no `name`: the + // home page, a frame that mounts no expression scope) the union is asked in + // ONE `GET /meta/object?names=a,b,c` (`batch: true`, the provider's + // batched names scope), not one by-name read per name. A name the answer + // leaves out is missing, as a by-name 404 was; a read that obtained no + // answer leaves every name unanswered, so each entry shows, as when a + // by-name read failed. Inside an app the union is still asked by name: the + // route's own objects are among its names, and their by-name read is the + // one the record form shares with the adapter (objectui#13044). const metadata = useMetadata(); const packageId = typeof app?._packageId === 'string' && app._packageId ? app._packageId : undefined; + const outsideApp = !(typeof app?.name === 'string' && app.name !== ''); const readObjectScope = metadata.getTypeStatus?.('object') === 'idle' ? metadata.readObjectScope : undefined; const unionNames = readObjectScope && !packageId ? requiredObjectUnion(metadata.apps, app) : NO_MISSING; const scope: { objects: readonly unknown[]; missing: readonly string[] } | null = !readObjectScope @@ -393,7 +413,9 @@ export function useNavMemberGates(forApp?: Record): NavMemberGa ? (readObjectScope as ReadNamesWithoutViews)({ packageId, views: false }) : unionNames.length === 0 ? NO_REQUIRED_OBJECTS - : (readObjectScope as ReadNamesWithoutViews)({ names: unionNames, views: false }); + : outsideApp + ? (readObjectScope as ReadNamesInOneRequest)({ names: unionNames, views: false, batch: true }) + : (readObjectScope as ReadNamesWithoutViews)({ names: unionNames, views: false }); const metadataObjects = scope ? scope.objects : metadata.objects; const missingObjects = scope ? scope.missing : NO_MISSING; const registeredObjectNames = useMemo( diff --git a/ui/packages/app-shell/src/providers/MetadataProvider.tsx b/ui/packages/app-shell/src/providers/MetadataProvider.tsx index 81443c50023..b68096f4178 100644 --- a/ui/packages/app-shell/src/providers/MetadataProvider.tsx +++ b/ui/packages/app-shell/src/providers/MetadataProvider.tsx @@ -167,6 +167,24 @@ const NO_NAMES: readonly string[] = Object.freeze([]) as readonly string[]; */ const NAMES_ONLY_SCOPE = ''; +/** + * objectstack#29444 — the key of the batched names scope: the objects a + * reader asks only the presence of, outside an app (the console's roster + * gate on Home), answered by ONE `GET /meta/object?names=a,b,c` per set of + * names not asked yet, instead of one by-name read each. A symbol, so no + * package id can collide with it. + * + * It is its own entry, never the names-only scope's: a list answer carries no + * by-name envelope (no `sortability`), shares no request with the adapter's + * `getObjectSchema` (objectui#13044), and cannot tell an absent object from a + * refused one (`OBJECT_PACKAGE_DISABLED`), so a route's reader never takes + * its answer for its own. + */ +const NAMES_BATCH_SCOPE: unique symbol = Symbol('names-batch'); + +/** A scope's key: a package id, {@link NAMES_ONLY_SCOPE}, or {@link NAMES_BATCH_SCOPE}. */ +type ObjectScopeKey = string | typeof NAMES_BATCH_SCOPE; + interface MetadataProviderProps { children: ReactNode; adapter: ObjectStackAdapter; @@ -946,8 +964,9 @@ export function MetadataProvider({ children, adapter, ttlMs = DEFAULT_TTL_MS }: const cacheRef = useRef>(new Map()); const itemPromisesRef = useRef>(new Map()); // objectui#12811 — the package-scoped object reads, keyed by package id, - // and the names-only scope under NAMES_ONLY_SCOPE (objectui#12980). - const scopesRef = useRef>(new Map()); + // and the names-only scope under NAMES_ONLY_SCOPE (objectui#12980), and + // the batched names scope under NAMES_BATCH_SCOPE (objectstack#29444). + const scopesRef = useRef>(new Map()); // Types whose session entry this boot wrote under NO_ORG_SCOPE because the // active organization had not resolved yet (objectui#5243). const unscopedSeedTypesRef = useRef>(new Set()); @@ -1412,7 +1431,7 @@ export function MetadataProvider({ children, adapter, ttlMs = DEFAULT_TTL_MS }: ); const fetchScopeName = useCallback( - (packageId: string, entry: ObjectScopeEntry, name: string): Promise => { + (packageId: ObjectScopeKey, entry: ObjectScopeEntry, name: string): Promise => { entry.names.set(name, 'pending'); return readItem('object', name).then(({ item, failed, refusal, answer }) => { if (scopesRef.current.get(packageId) !== entry) return; @@ -1452,6 +1471,66 @@ export function MetadataProvider({ children, adapter, ttlMs = DEFAULT_TTL_MS }: [bump, readItem], ); + // objectstack#29444 — the batched names scope's read: `names` in ONE + // `GET /meta/object?names=a,b,c` (`meta.getItems`, ruling A of + // objectstack#29236). The list door answers each name through the same + // per-caller gate, object mask and translation as the by-name read, and + // leaves out an object of a disabled package as that read refuses it. + // + // - A name the answer holds is answered with its definition. + // - A name the answer leaves out is answered MISSING, as a by-name 404 is: + // the door answers an unknown name by its absence, never a 404. + // - A read that obtained no answer fails EVERY name it asked, as a failed + // by-name read fails its one (objectui#13009, objectui#13052): no name is + // recorded missing, and none is asked again until `invalidate`, + // `refresh` or an organization switch. + // + // Only the asked names are read out of the answer, so an item the request + // did not name never lands in the scope. + const fetchScopeNames = useCallback( + (entry: ObjectScopeEntry, names: readonly string[]): Promise => { + for (const name of names) entry.names.set(name, 'pending'); + return adapterRef.current + .getClient() + .meta.getItems('object', { names }) + .then( + (res: unknown) => { + if (scopesRef.current.get(NAMES_BATCH_SCOPE) !== entry) return; + const asked = new Set(names); + const items = extractItems(res).filter((it: any) => asked.has(it?.name)); + ingestObjectList(items, res); + const served = new Set(items.map((it: any) => it.name as string)); + entry.items = [...entry.items.filter((it: any) => !asked.has(it?.name)), ...items]; + const missing = entry.missing.filter((name) => !asked.has(name)); + for (const name of names) { + entry.names.set(name, 'answered'); + if (!served.has(name)) missing.push(name); + } + entry.missing = missing; + const answers = { ...entry.answers }; + for (const name of names) delete answers[name]; + entry.answers = answers; + debug(`fetched object names scope names=${names.length} items=${items.length}`); + bump(); + }, + (err: unknown) => { + if (scopesRef.current.get(NAMES_BATCH_SCOPE) !== entry) return; + debug(`fetch failed object names scope names=${names.length}`, err); + const answer = failedReadAnswer(err); + const answers = { ...entry.answers }; + for (const name of names) { + entry.names.set(name, 'failed'); + if (answer) answers[name] = answer; + else delete answers[name]; + } + entry.answers = answers; + bump(); + }, + ); + }, + [bump], + ); + /** Refetch every scope in place: the items stay until the answers replace them. */ const refreshScopes = useCallback((): Promise[] => { // `getItem` answers from the `object` entry's by-name cache; a refresh @@ -1462,11 +1541,17 @@ export function MetadataProvider({ children, adapter, ttlMs = DEFAULT_TTL_MS }: const pending: Promise[] = []; for (const [packageId, entry] of scopesRef.current) { if (entry.status === 'idle') continue; + if (packageId === NAMES_BATCH_SCOPE) { + // objectstack#29444 — its names are asked again in one request, as they were asked. + const names = Array.from(entry.names.keys()); + if (names.length > 0) pending.push(fetchScopeNames(entry, names)); + continue; + } if (packageId !== NAMES_ONLY_SCOPE) pending.push(fetchScopeList(packageId, entry)); for (const name of Array.from(entry.names.keys())) pending.push(fetchScopeName(packageId, entry, name)); } return pending; - }, [dropAdapterObjectReads, fetchScopeList, fetchScopeName, getEntry]); + }, [dropAdapterObjectReads, fetchScopeList, fetchScopeName, fetchScopeNames, getEntry]); const refresh = useCallback( async (type?: string): Promise => { @@ -1781,10 +1866,19 @@ export function MetadataProvider({ children, adapter, ttlMs = DEFAULT_TTL_MS }: // registry is already loaded (Studio was opened, say), the scope is cut from // it and nothing is requested. With `names` and no `packageId` it is the // names-only scope (objectui#12980): no package list, each name by name. + // + // objectstack#29444 — `batch: true`, this package's own hand-off beside + // `views: false` (`useNavMemberGates`, outside an app), reads a names-only + // scope from the batched names scope: the names not asked yet are asked in + // one `?names=` request. Under the draft preview, whose client has no + // names filter, it is the names-only scope, each name by name. const readObjectScope = useCallback( - (scope: { packageId?: string; names?: readonly string[]; views?: false }) => { + (scope: { packageId?: string; names?: readonly string[]; views?: false; batch?: true }) => { const namesOnly = !scope.packageId && !!scope.names && scope.names.length > 0; - const packageId = scope.packageId || (namesOnly ? NAMES_ONLY_SCOPE : undefined); + const batched = namesOnly && scope.views === false && scope.batch === true && !previewClientRef.current; + const packageId: ObjectScopeKey | undefined = batched + ? NAMES_BATCH_SCOPE + : scope.packageId || (namesOnly ? NAMES_ONLY_SCOPE : undefined); if (packageId === undefined) { const status = getEntry('object').status; const served = readType('object'); @@ -1815,13 +1909,14 @@ export function MetadataProvider({ children, adapter, ttlMs = DEFAULT_TTL_MS }: // No package list to wait for: the names are asked at once. entry.status = 'ready'; entry.answered = true; - } else { + } else if (typeof packageId === 'string') { void fetchScopeList(packageId, entry); } } } let namesSettled = true; const failedNames: string[] = []; + const toBatch: string[] = []; if (scope.names && scope.names.length > 0) { if (!entry.answered) { namesSettled = false; @@ -1847,10 +1942,12 @@ export function MetadataProvider({ children, adapter, ttlMs = DEFAULT_TTL_MS }: continue; } namesSettled = false; - void fetchScopeName(packageId, entry, name); + if (batched) toBatch.push(name); + else void fetchScopeName(packageId, entry, name); } } } + if (toBatch.length > 0) void fetchScopeNames(entry, toBatch); // objectui#13052 — `settled` stays false while a name's read has // failed (a failure is not an answer, objectui#13009); `failed` names // those reads once nothing else is still in flight, so a reader can stop @@ -1897,7 +1994,7 @@ export function MetadataProvider({ children, adapter, ttlMs = DEFAULT_TTL_MS }: views: scopeViews.views, }; }, - [fetchScopeList, fetchScopeName, getEntry, readType, viewsForObjects], + [fetchScopeList, fetchScopeName, fetchScopeNames, getEntry, readType, viewsForObjects], ); const value = useMemo(() => {