From b834c019726a90403b48ada7599c671b46669b03 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 18:36:05 +0000 Subject: [PATCH] fix(metadata-protocol): expand a runtime-authored view container so its views are served (#7736) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Publishing views through the runtime metadata API using the documented `defineView` container shape succeeded at every step and produced nothing a user could see: query the object and the list came back empty, while reading the row by name returned the full body badged `_diagnostics.valid: true`. "Object has-many View" (ADR-0017 §2, §3.2) makes container ingestion dual-read — register the container under the bare `` key, AND register every named view as an independent ViewItem under `.`. Only the expanded items carry the `viewKind` + `object` pair the object-bound read paths filter on. Both source registrars do this (the ObjectQL boot loop and the metadata artifact/HMR loader); the runtime door did not, so `getMetaItems` dropped the container from enumeration — correctly, on its stated assumption that "the registrar expands it" — in favour of an expansion that never ran. Fixed at `hydrateOverlayIntoRegistry`, the one choke point all three runtime hydration callers already share (boot, read-side, write-through). There are two independent object-bound readers — the REST route reads through `getMetaItems`, `getViewsByObject()` reads `MetadataManager.list` — so expanding at either read exit would fix the literal repro and leave its sibling empty. One expansion at the shared seam serves every reader, survives a restart, and keeps read-your-writes. The canonical-shape filter is left alone: its invariant (a container's expanded items are also present) is what was false, and this restores it rather than loosening the filter. Nothing extra is persisted — the container remains one byte-identical row and the ViewItems are derived on hydration, so an edited container leaves no stale expanded rows. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CmkoMgyZoYRznuVifcXGmB --- .../view-container-runtime-expansion.md | 53 ++++ packages/metadata-protocol/src/protocol.ts | 80 +++++- .../view-container-runtime-expansion.test.ts | 251 ++++++++++++++++++ 3 files changed, 383 insertions(+), 1 deletion(-) create mode 100644 .changeset/view-container-runtime-expansion.md create mode 100644 packages/metadata-protocol/src/view-container-runtime-expansion.test.ts diff --git a/.changeset/view-container-runtime-expansion.md b/.changeset/view-container-runtime-expansion.md new file mode 100644 index 0000000000..adb3431153 --- /dev/null +++ b/.changeset/view-container-runtime-expansion.md @@ -0,0 +1,53 @@ +--- +"@objectstack/metadata-protocol": patch +--- + +fix(metadata-protocol): expand a runtime-authored `defineView` container so the views it declares are actually served (#7736) + +Publishing views through the runtime metadata API using the documented +`defineView` container shape succeeded at every step and produced nothing a user +could see. `PUT` the container to the draft endpoint, `POST` to publish, then +query the object — an empty list. Reading the row directly by name returned the +full body, badged `_diagnostics.valid: true`, and a server restart changed +nothing. + +**Why.** "Object has-many View" (ADR-0017 §2, §3.2) makes container ingestion +**dual-read**: register the container under the bare `` key for +back-compatible single-item reads, *and* register every named view as an +independent `ViewItem` under `.`. Only the expanded items carry +the `viewKind` + `object` pair that every object-bound read path filters on, so +the expanded layer — never the container — is what `GET /meta/view?object=`, +`getViewsByObject()` and the view switcher actually read. + +Both **source** registrars do this: the ObjectQL boot loop (`engine.ts`) and the +metadata artifact/HMR loader (`plugin.ts`). The **runtime** door did not. A +container written through it was stored verbatim, carrying neither `object` nor +`viewKind`, and `getMetaItems` then dropped it from enumeration — correctly, on +its stated assumption that "the registrar expands it into independent +ViewItems". For a runtime-written row no registrar ever had, so the container was +filtered out and the expansion it was filtered out *in favour of* did not exist. +Measured on the card's repro: the stored container expands cleanly to two items +that would match the switcher, and both object-bound exits answered zero. + +**Where the fix goes.** At `hydrateOverlayIntoRegistry` — the one choke point all +three runtime hydration callers already share (boot `loadMetaFromDb`, read-side +`getMetaItems`, write-through `applyRegistryWriteThrough`). That matters, +because there are **two independent object-bound readers**: the REST route reads +through `getMetaItems`, while `getViewsByObject()` reads `MetadataManager.list`. +Expanding at either read exit fixes the card's literal repro and leaves its +sibling answering empty. One expansion at the shared seam serves every reader, +survives a restart, and keeps read-your-writes — the "single, universally-applied +location" #7163 asked for after the same defect was closed one seam further in. + +The canonical-shape filter is deliberately **left alone**. Its invariant — a +container's expanded items are also present — is precisely what was false here, +and this restores it rather than loosening the filter, which would surface the +legacy wrapper shape to every list consumer (Studio list, REST, AI retriever) +and still show the switcher nothing, since a container carries no `viewKind`. + +Nothing extra is persisted: the container is still stored as exactly one +byte-identical row and the ViewItems are derived on hydration, so an edited +container cannot leave stale expanded rows behind. An already-independent +`ViewItem`, a non-view type, and an object with no container authored are all +unaffected — pinned, along with the headline behaviour, in +`view-container-runtime-expansion.test.ts`. diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 544a80b7e0..641f33db50 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -62,7 +62,7 @@ import { } from '@objectstack/spec/data'; import { PLURAL_TO_SINGULAR, SINGULAR_TO_PLURAL } from '@objectstack/spec/shared'; import { applyConversionsToStoredItem, type ConversionNotice } from '@objectstack/spec'; -import { type FormView, isAggregatedViewContainer } from '@objectstack/spec/ui'; +import { type FormView, isAggregatedViewContainer, expandViewContainer } from '@objectstack/spec/ui'; import { METADATA_FORM_REGISTRY, CORE_SERVICE_PROVIDER, serviceUnavailableMessage, inProcessServiceMessage } from '@objectstack/spec/system'; import { DEFAULT_METADATA_TYPE_REGISTRY, getMetadataTypeSchema, getMetadataTypeActions, getMetadataCreateSeed, PROTOCOL_VERSION } from '@objectstack/spec/kernel'; import { @@ -9556,9 +9556,87 @@ export class ObjectStackProtocolImplementation implements if (!registry || typeof registry.registerItem !== 'function') return false; const artifact = this.lookupArtifactItem(type, (data as any).name, options.packageId ?? undefined); registry.registerItem(type, mergeArtifactProtection(data, artifact), 'name' as any); + this.hydrateExpandedViewItems(type, data, options, registry); return true; } + /** + * [#7736] Expand an aggregated `defineView` container that arrived through + * the RUNTIME door into the same independent ViewItems the two SOURCE + * registrars produce — at the one hydration choke point all three runtime + * callers already share. + * + * "Object has-many View" (ADR-0017 §2, §3.2) makes container ingestion + * DUAL-READ: register the container under the bare `` key for + * back-compatible single-item reads, AND register every expanded ViewItem + * under `.`, because only the expanded items carry the + * `viewKind` + `object` pair that every object-bound read path filters on + * (`GET /meta/view?object=` in `rest-server.ts`, `getViewsByObject()` in + * `metadata-manager.ts`). Both source registrars do exactly this — the + * ObjectQL boot loop (`engine.ts`, `key === 'views'`) and the metadata + * artifact/HMR loader (`plugin.ts`, `isAggregatedViewContainer`) — and the + * runtime door did not, so a container authored against a writable runtime + * package was stored, badged `_diagnostics.valid: true`, and then served by + * nothing: `getMetaItems` DROPS containers from enumeration (the + * canonical-shape filter below) on the stated assumption that "the + * registrar expands it", and for a runtime-written row no registrar ever + * had. Measured before the fix: the stored container expands cleanly to two + * items that WOULD match the switcher, and both object-bound exits answered + * zero. + * + * Here rather than at either read exit deliberately. There are two + * independent object-bound readers — the REST route reads through + * `getMetaItems`, while `getViewsByObject()` reads `MetadataManager.list` + * — so expanding at one of them fixes the card's literal repro and leaves + * its sibling exit answering empty. This function is the ONE place all + * three runtime hydration callers (boot `loadMetaFromDb`, read-side + * `getMetaItems`, write-through `applyRegistryWriteThrough`) already + * funnel through, so one expansion serves every reader, survives a restart, + * and keeps read-your-writes — the "single, universally-applied location" + * #7163 asked for after the same defect was fixed one seam further in. + * + * The canonical-shape filter in `getMetaItems` is deliberately left alone: + * its invariant ("a container's expanded items are also present") is what + * was false here, and this restores it rather than loosening the filter — + * which would surface the legacy wrapper shape to every list consumer and + * still show the switcher nothing, since a container carries no `viewKind`. + * + * Object-name derivation mirrors `plugin.ts` (`list.data.object` → + * `form.data.object`), falling back to the row's own name — for a container + * the metadata door's save name IS the object. No derivable object means no + * expansion, exactly as the artifact loader already decides. + */ + private hydrateExpandedViewItems( + type: string, + data: unknown, + options: { packageId?: string | null; organizationId: string | null }, + registry: any, + ): void { + if ((PLURAL_TO_SINGULAR[type] ?? type) !== 'view') return; + if (!isAggregatedViewContainer(data)) return; + const container = data as Record; + const viewObject = + container?.list?.data?.object + ?? container?.form?.data?.object + ?? (typeof container.name === 'string' ? container.name : undefined); + if (!viewObject) return; + for (const vi of expandViewContainer(viewObject, container)) { + // Carry the container's package provenance onto each expanded item + // so the package-disable filter and ADR-0048 artifact scoping judge + // them by the same owner the container has. + const item: Record = { ...(vi as any) }; + if (container._packageId !== undefined && item._packageId === undefined) { + item._packageId = container._packageId; + } + const viArtifact = this.lookupArtifactItem( + type, + vi.name, + (item._packageId as string | undefined) ?? options.packageId ?? undefined, + ); + registry.registerItem(type, mergeArtifactProtection(item, viArtifact), 'name' as any); + } + } + /** * [#4521] Write-through the SchemaRegistry after a mutation goes LIVE, so * a just-saved item is dispatchable — not merely listable. diff --git a/packages/metadata-protocol/src/view-container-runtime-expansion.test.ts b/packages/metadata-protocol/src/view-container-runtime-expansion.test.ts new file mode 100644 index 0000000000..7a9e70e963 --- /dev/null +++ b/packages/metadata-protocol/src/view-container-runtime-expansion.test.ts @@ -0,0 +1,251 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #7736 — a `defineView` container authored through the RUNTIME door is served, + * not merely stored. + * + * "Object has-many View" (ADR-0017 §2, §3.2) makes container ingestion + * dual-read: the container is registered under the bare `` key, and + * every named view is ALSO registered as an independent ViewItem under + * `.`. Only the expanded items carry the `viewKind` + `object` + * pair that the object-bound read paths filter on, so the expanded layer — not + * the container — is what `GET /meta/view?object=`, `getViewsByObject()` and + * the view switcher actually read. + * + * Both SOURCE registrars do this (the ObjectQL boot loop and the metadata + * artifact/HMR loader). The RUNTIME door did not. Measured before the fix, on + * the card's own repro: the write is accepted, the body is stored verbatim + * carrying neither `object` nor `viewKind`, `getMetaItem` by name serves it + * badged `_diagnostics.valid: true` — and the enumerating read answers ZERO, + * because `getMetaItems` drops containers from enumeration on the stated + * assumption that "the registrar expands it into independent ViewItems", which + * for a runtime-written row never happened. + * + * The pin is at the protocol, not at either read exit, because there are two + * independent object-bound readers (the REST route reads through + * `getMetaItems`; `getViewsByObject()` reads `MetadataManager.list`) and a fix + * at one leaves the other empty. `hydrateOverlayIntoRegistry` is the single + * choke point all three runtime hydration callers share. + */ +import { describe, expect, it } from 'vitest'; +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objectstack/metadata-core'; +import { ObjectStackProtocolImplementation } from './index.js'; + +interface Row { + id: string; type: string; name: string; organization_id: string | null; + package_id: string | null; state: string; metadata: string; checksum?: string; version?: number; +} + +function matches(r: Row, where: Record): boolean { + for (const [k, v] of Object.entries(where)) { + if (v === undefined) continue; + if ((r as any)[k] !== v) return false; + } + return true; +} + +function keyOf(w: Record) { + return `${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}|${w.package_id ?? '__nopkg__'}`; +} + +/** + * A registry stub that actually STORES what `registerItem` hands it and returns + * it from `listItems` — the two halves this card turns on. A no-op registry + * would pass every assertion below vacuously. + */ +function makeStubEngine() { + const rows = new Map(); + const registered = new Map>(); + let nextId = 0; + const findRow = (w: Record) => { + for (const [k, r] of rows) if (matches(r, w)) return { key: k, row: r }; + return null; + }; + const engine: any = { + async findOne(_t: string, opts: { where: Record }) { return findRow(opts.where)?.row ?? null; }, + async find(_t: string, opts: { where: Record }) { + return Array.from(rows.values()).filter((r) => matches(r, opts.where)); + }, + async insert(table: string, data: Record) { + if (table !== 'sys_metadata') return { id: 'side_table' }; + const row = { id: `r_${++nextId}`, ...(data as any) } as Row; + rows.set(keyOf(data), row); + return { id: row.id }; + }, + async update(table: string, data: Record, opts: { where: Record }) { + assertEngineUpdateDispatch(data, opts); + if (table !== 'sys_metadata') return { id: null }; + const found = findRow(opts.where); + if (!found) return { id: null }; + const merged = { ...found.row, ...(data as any) }; + rows.delete(found.key); rows.set(keyOf(merged), merged); + return { id: found.row.id }; + }, + async delete(_t: string, opts?: Record) { assertEngineDeleteDispatch(opts); return { deleted: 0 }; }, + async transaction(cb: (ctx: any, info: { owned: boolean }) => Promise): Promise { return cb(undefined, { owned: true }); }, + async syncObjectSchema() { }, + registry: { + listItems: (type: string) => Array.from(registered.get(type)?.values() ?? []), + isPackageDisabled: () => false, + getItem: (type: string, name: string) => registered.get(type)?.get(name), + registerItem: (type: string, item: any) => { + if (!registered.has(type)) registered.set(type, new Map()); + registered.get(type)!.set(item?.name, item); + }, + registerObject: () => { }, + getPackage: () => undefined, + }, + }; + return { engine, rows, registered }; +} + +/** The card's repro body: a `defineView` container, as `defineView` emits it. */ +const leadContainer = { + list: { + label: 'All Leads', + type: 'grid', + data: { provider: 'object', object: 'crm_lead' }, + columns: [{ field: 'name' }, { field: 'company' }], + }, + listViews: { + pipeline: { + label: 'Lead Pipeline', + type: 'grid', + data: { provider: 'object', object: 'crm_lead' }, + columns: [{ field: 'name' }], + }, + }, +}; + +/** The object-bound predicate BOTH read exits filter on, verbatim. */ +const switcherMatches = (items: any[], object: string) => + items.filter((v: any) => v && typeof v === 'object' && v.viewKind && v.object === object); + +describe('#7736 a runtime-authored view container is served', () => { + it('serves the expanded ViewItems the object-bound read paths filter on', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + + await protocol.saveMetaItem({ type: 'view', name: 'crm_lead', item: leadContainer }); + + const list: any = await protocol.getMetaItems({ type: 'view' }); + const served = switcherMatches(list.items, 'crm_lead'); + + expect( + served.map((v: any) => v.name).sort(), + 'A runtime-authored container must expand into the independent ViewItems ' + + 'the switcher reads, exactly as the two source registrars do.', + ).toEqual(['crm_lead.default', 'crm_lead.pipeline']); + for (const v of served) expect(v.viewKind).toBe('list'); + }); + + it('…and what it serves is the STORED container, not a default', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + + await protocol.saveMetaItem({ type: 'view', name: 'crm_lead', item: leadContainer }); + const list: any = await protocol.getMetaItems({ type: 'view' }); + const byName = Object.fromEntries(list.items.map((v: any) => [v.name, v])); + + // The authored payload survives expansion — label and columns are the + // ones this test wrote, so the served view cannot be a stand-in. + expect(byName['crm_lead.pipeline'].label).toBe('Lead Pipeline'); + expect(byName['crm_lead.pipeline'].config.columns).toEqual([{ field: 'name' }]); + expect(byName['crm_lead.default'].label).toBe('All Leads'); + expect(byName['crm_lead.default'].config.columns).toEqual([{ field: 'name' }, { field: 'company' }]); + }); + + it('still never surfaces the aggregated container itself (ADR-0017 canonical shape)', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + + await protocol.saveMetaItem({ type: 'view', name: 'crm_lead', item: leadContainer }); + const list: any = await protocol.getMetaItems({ type: 'view' }); + + // The bare `` key is a back-compat single-item read, never an + // enumeration entry — loosening that filter was the tempting fix and is + // not the one taken. + expect(list.items.some((v: any) => v?.name === 'crm_lead')).toBe(false); + const byName: any = await protocol.getMetaItem({ type: 'view', name: 'crm_lead' }); + expect(byName.item).toBeTruthy(); + expect(byName.item.listViews).toBeDefined(); + }); + + /** + * ANTI-VACUITY. The suite above would pass just as well against a change + * that served "whatever exists" for every object. These two cases prove the + * fixtures actually distinguish an authored container from no container. + */ + describe('anti-vacuity — absence still reads as absence', () => { + it('an object with NO view container serves nothing for that object', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + + // A container authored for crm_lead must not make crm_account — + // an object nobody authored a view for — start answering non-empty. + await protocol.saveMetaItem({ type: 'view', name: 'crm_lead', item: leadContainer }); + + const list: any = await protocol.getMetaItems({ type: 'view' }); + expect(switcherMatches(list.items, 'crm_account')).toEqual([]); + expect(switcherMatches(list.items, 'crm_lead')).toHaveLength(2); + }); + + it('with no view written at all, the view read is empty', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + + const list: any = await protocol.getMetaItems({ type: 'view' }); + expect(list.items).toEqual([]); + expect(switcherMatches(list.items, 'crm_lead')).toEqual([]); + }); + }); + + /** + * The path must not have become "expand anything". A view that is already an + * independent ViewItem, and a non-view type, go through the same hydration + * choke point and must come back byte-identical to their pre-fix behaviour. + */ + describe('the untouched arms', () => { + it('an already-independent ViewItem is served exactly once, unchanged', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + + const record = { + name: 'crm_lead.mine', object: 'crm_lead', viewKind: 'list', label: 'My Leads', + config: { type: 'grid', data: { provider: 'object', object: 'crm_lead' }, columns: [{ field: 'name' }] }, + }; + await protocol.saveMetaItem({ type: 'view', name: 'crm_lead.mine', item: record }); + + const list: any = await protocol.getMetaItems({ type: 'view' }); + const served = switcherMatches(list.items, 'crm_lead'); + expect(served).toHaveLength(1); + expect(served[0].name).toBe('crm_lead.mine'); + expect(served[0].config).toEqual(record.config); + }); + + it('a non-view type stores and serves a byte-identical body', async () => { + const { engine, rows } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + + const authored = { name: 'crm_invoice', label: 'Invoice', fields: { amount: { type: 'currency', label: 'Amount' } } }; + await protocol.saveMetaItem({ type: 'object', name: 'crm_invoice', item: authored }); + + const stored = Array.from(rows.values()).find((r) => r.name === 'crm_invoice')!; + expect(JSON.parse(stored.metadata)).toEqual(authored); + }); + + it('a view container stores a byte-identical body — expansion is derived, never persisted', async () => { + const { engine, rows } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + + await protocol.saveMetaItem({ type: 'view', name: 'crm_lead', item: leadContainer }); + + // Exactly ONE row: the container as authored (plus the `name` the + // write door has always stamped). No expanded rows are persisted, so + // there is nothing to go stale when the container is next edited. + const viewRows = Array.from(rows.values()).filter((r) => r.type === 'view'); + expect(viewRows).toHaveLength(1); + expect(JSON.parse(viewRows[0].metadata)).toEqual({ ...leadContainer, name: 'crm_lead' }); + }); + }); +});