Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .changeset/29444-home-roster-gate-batch-read.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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`).
Expand All @@ -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';
Expand Down Expand Up @@ -59,7 +60,7 @@ const APPS = [
},
];

function makeAdapter(calls: string[]) {
function makeAdapter(calls: string[], failBatch = false) {
return {
clearCache: vi.fn(),
invalidateCache: vi.fn(),
Expand All @@ -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) => {
Expand All @@ -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(<MetadataProvider adapter={makeAdapter(calls)}>{children}</MetadataProvider>);
render(<MetadataProvider adapter={makeAdapter(calls, options.failBatch)}>{children}</MetadataProvider>);
return calls;
}

Expand Down Expand Up @@ -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(<GateProbe />);
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(<GateProbe />, { 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<void> } = {};
function RefreshHandle({ into }: { into: typeof handle }) {
const { refresh } = useMetadata();
React.useEffect(() => {
into.refresh = refresh;
}, [into, refresh]);
return null;
}
const calls = mount(
<>
<RefreshHandle into={handle} />
<GateProbe />
</>,
);
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 () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -212,14 +223,17 @@ 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));
await waitFor(() => expect(read().gates.checkCapability('object', 'notes_gone')).toBe(false));

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');
});
Expand Down
24 changes: 23 additions & 1 deletion ui/packages/app-shell/src/hooks/useNavMemberGates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<ReadNamesWithoutViews>;

/** 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 });

Expand Down Expand Up @@ -383,8 +392,19 @@ export function useNavMemberGates(forApp?: Record<string, unknown>): 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
Expand All @@ -393,7 +413,9 @@ export function useNavMemberGates(forApp?: Record<string, unknown>): 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(
Expand Down
Loading
Loading