diff --git a/.changeset/21620-container-sibling-expansion-name.md b/.changeset/21620-container-sibling-expansion-name.md new file mode 100644 index 0000000000..fa582e0122 --- /dev/null +++ b/.changeset/21620-container-sibling-expansion-name.md @@ -0,0 +1,19 @@ +--- +'@objectstack/metadata-protocol': minor +--- + +The runtime save door refuses a view container saved under a name another stored container of the same object expands to + +Clause-②: no (narrowing) + + + +**BREAKING** accept-set narrowing at the runtime save door, shipped as `minor` under the repo's launch-window convention for breaking changes, the grade the same door's earlier name refusals shipped with. + +**What was accepted before.** `saveMetaItem`, which `PUT /api/v1/meta/view/:name` and the dispatcher's metadata save both call, accepted an aggregated view container (`list` / `form` / `listViews` / `formViews`) saved under a name that another stored container of the same object expands to. For example, with `{ name: 'crm_lead', object: 'crm_lead', list: { … }, listViews: { pipeline: { … } } }` stored, a second container `{ object: 'crm_lead', list: { … } }` saved as `crm_lead.pipeline`. The second container became that name's own stored row, and an expansion fills only a name with no row of its own, so the first container's `crm_lead.pipeline` view was no longer served: the object door (`GET /api/v1/meta/view?object=…`), which never lists a container, listed nothing under the name, and the by-name read answered the raw second container. Nothing said why. + +**What is refused now.** That save, with `VALIDATION_ERROR` / 400, before anything is stored or registered, in draft and in publish mode. The other containers are the stored rows the read doors select for the same caller (environment-wide rows plus the caller's organization's), each expanded exactly as the read doors expand it, so every member kind (a bare or named `list`, `listViews`, `form`, `formViews`), the expander's de-duplicated names, and the names a container on another package's object expands under its own name are all judged where the readers place them. A container with no `name` is judged under the save name the door stamps on it. + +**What still saves.** A container under its object's name, which expands as before, and its own re-save. A container under any other name of its own that no other stored container of its object expands to: this door keeps a container saved under a name other than its object, and this change leaves that alone. A view item (a body carrying `viewKind`) under an expanded name, the sanctioned override for that name. The read doors are unchanged. A row stored in this shape before this change keeps its bytes and is served as before; `migrate meta --stored` and package duplication, which re-save stored rows through this door, report such a row as failed with this refusal instead of re-saving it. + +**The fix.** Add the view as a member of the stored container that already expands the name (in the example, the container `crm_lead`, whose `listViews.pipeline` is that view), or save a view item (`name`, `object`, `viewKind`, `config`) under the expanded name (`crm_lead.pipeline`). diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 8a8053c40c..56bc12dce7 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -107,6 +107,10 @@ import { isMissingTableError } from '@objectstack/metadata/errors'; // door (`saveMetaItem`), the restore doors (`rollbackMetaItem`, `revertCommit`) // and the draft promotion (`promoteDraftForPublish`). import { savedItemNameRefusal } from '@objectstack/metadata/view-container-name'; +// [#21620] The one spelling of "which object a view container binds to" — the +// derivation the source registrars file a container under — so the save door's +// sibling-expansion refusal judges "the same object" as every other door does. +import { deriveViewContainerObject } from '@objectstack/metadata/view-container'; import type { BatchUpdateRequest, BatchUpdateResponse, @@ -17955,6 +17959,113 @@ export class ObjectStackProtocolImplementation implements return err; } + /** + * [#21620] The save door's refusal of a view container saved under a name + * that ANOTHER stored container of the same object expands to — with + * `{ name: 'crm_lead', object: 'crm_lead', listViews: { pipeline } }` + * stored, a second container `{ object: 'crm_lead', list }` saved as + * `crm_lead.pipeline`. + * + * The harm is #21558's, reached through a sibling: the second container + * becomes the stored row of `crm_lead.pipeline`, and both read doors give + * a name with a row of its own that row (#21510's one predicate, + * {@link namesWithOwnStoredRow}). So the first container's expansion no + * longer fills the name, the object door — which never enumerates a + * container — lists nothing under it, and the by-name read answers the raw + * second container: the sibling's view is gone from both doors and no door + * answers a view item for the name. #21558's check cannot see this: the + * second container's OWN expansion is `crm_lead.default`, never its save + * name. + * + * Triage's ruling on the card named a broader check — a container's name + * must be its object's name — and made it conditional on a census, with + * THIS narrower check as the fallback. The census hit: this door keeps a + * container saved under a name other than its object (#13407's live + * authoring path, which the platform checklist's live view-authoring item + * drives; #21412's P2 and P2b, ruled; #21334's arm expands one under its + * own name, ruled), and Studio's metadata editor re-saves such a container + * under its stored name. So the name is judged only against what the + * other stored containers of the same object expand to. + * + * The judgment is the readers' own, never a copy of it: + * - the rows are the ones {@link readActiveOverlayRows} selects for this + * caller, through the read gate the readers apply + * ({@link organizationIdForMetaRead}) and with no package filter, so + * every reader whose selection holds this container and a sibling is + * covered for this caller's scope; + * - each row is parsed by {@link storedOverlayEntries} and expanded by + * {@link expandStoredViewContainers}, with the row's own package + * binding, so every member kind, the expander's de-duplication and + * #21334's arm are judged where the readers place them; + * - the row stored under the save name itself is left out: it is the row + * this save replaces, not a sibling; + * - "the same object" is the expanded view's `object` against + * {@link deriveViewContainerObject} of the body, the one derivation + * every door files a container under. + * + * The body judged is the one the author sent, with the door's own `name` + * stamp applied first (a body with no `name` is judged under the save + * name), BEFORE {@link normalizeViewMetadata}'s identity patch — on an + * unscoped kernel the sibling's expansion is registered under the name, + * and a `form`-only container would take its `viewKind` there and reach + * the schema as a malformed view item instead of this refusal. + * + * A view item (`viewKind` set) is not a container and is untouched: under + * an expanded name it is that name's sanctioned override. Rows already + * stored in this shape keep their bytes and are served as before; only a + * new save of one is refused, and the re-savers that write through this + * door (`migrateStoredMetadata`, `duplicatePackage`) record that refusal + * as the row's failure instead of re-saving it. + * + * `VALIDATION_ERROR` / 400, the envelope of the two name checks it sits + * beside. The prescription names the stored container that expands the + * name, and gives two arms: add the view as a member of THAT container, or + * save a view item under the expanded name. ⛔ It never prescribes a save + * under a name another stored container holds — not even the object's own + * name, which in the card's pair IS the sibling: an author (or an AI) + * following such an arm literally would replace the sibling's row and drop + * the very view this refusal keeps serving. Runtime words carry no tracker + * number. + */ + private async containerSiblingExpansionNameRefusal( + type: string, + item: unknown, + saveName: string, + organizationId: string | undefined, + ): Promise<(Error & { code: 'VALIDATION_ERROR'; status: 400 }) | undefined> { + if ((PLURAL_TO_SINGULAR[type] ?? type) !== 'view') return undefined; + if (!item || typeof item !== 'object' || Array.isArray(item)) return undefined; + const body = item as Record; + const stamped = body.name ? body : { ...body, name: saveName }; + if (!isAggregatedViewContainer(stamped)) return undefined; + const object = deriveViewContainerObject(stamped); + if (!object) return undefined; + let records: any[] = []; + try { + records = await this.readActiveOverlayRows({ type }, organizationIdForMetaRead(type, organizationId)); + } catch (error) { + // [#5532] The readers' rule: only an unprovisioned store means "no + // rows". Any other failure is not answered as "no sibling". + this.rethrowUnlessMetadataStoreUnprovisioned(error, 'sys_metadata'); + } + const siblings = this.storedOverlayEntries({ type }, records) + .filter((entry) => entry.name !== saveName); + const hit = this.expandStoredViewContainers(type, siblings) + .find(({ item: expanded }) => expanded.name === saveName && expanded.object === object); + if (!hit) return undefined; + const err = new Error( + `Invalid view container: it is saved under '${saveName}', which is a name the stored container ` + + `'${hit.container.name}' expands (its ${String(hit.item.viewKind)} view on '${object}'). An expanded ` + + `view fills only a name that has no stored row of its own, and this container would be that row, so ` + + `that view would no longer be served and no read would answer a view under '${saveName}'. Add the ` + + `view as a member of the container '${hit.container.name}' (its list, listViews, form or formViews), ` + + `or save a view item (name, object, viewKind and config) under '${saveName}'.`, + ) as Error & { code: 'VALIDATION_ERROR'; status: 400 }; + err.code = 'VALIDATION_ERROR'; + err.status = 400; + return err; + } + // [#21207] `parentVersion` is a CALLER's version token — the keyed form a // receipt served — and is compared in that form (`storedParentForToken`). // `storedParentVersion` is the in-process twin for a caller that read the @@ -18446,6 +18557,16 @@ export class ObjectStackProtocolImplementation implements ); if (ownExpansionRefusal) throw ownExpansionRefusal; } + // [#21620] …and a view container saved under a name ANOTHER stored + // container of the same object expands to, with the same envelope, + // judged by the readers' own row selection and expansion. Also + // before the stamp. See {@link containerSiblingExpansionNameRefusal}. + { + const siblingExpansionRefusal = await this.containerSiblingExpansionNameRefusal( + singularType, request.item, request.name, request.organizationId, + ); + if (siblingExpansionRefusal) throw siblingExpansionRefusal; + } let baseline: unknown; if ((PLURAL_TO_SINGULAR[request.type] ?? request.type) === 'view' && typeof this.engine.registry?.getItem === 'function') { diff --git a/packages/metadata-protocol/src/view-container-runtime-expansion.test.ts b/packages/metadata-protocol/src/view-container-runtime-expansion.test.ts index 9df0f8928c..6b3924abe6 100644 --- a/packages/metadata-protocol/src/view-container-runtime-expansion.test.ts +++ b/packages/metadata-protocol/src/view-container-runtime-expansion.test.ts @@ -1292,6 +1292,230 @@ describe('#21334 a container on another package\'s object never takes that packa }); } }); + + /** + * #21620 — the save door refuses a view container saved under a name that + * ANOTHER stored container of the same object expands to. + * + * Measured on `origin/main` before this change, in-process on both + * kernels: with `{ name: 'crm_lead', object: 'crm_lead', list, listViews: + * { pipeline } }` stored, a second container `{ object: 'crm_lead', list }` + * saved as `crm_lead.pipeline` was accepted. It became that name's own + * row, so the first container's expansion no longer filled the name: the + * object door listed nothing under it and the by-name read answered the + * raw second container. #21558's check does not fire, because the second + * container's OWN expansion is `crm_lead.default`. + * + * Triage's ruling made a broader check (a container named only after its + * object) conditional on a census, with this narrower one as the + * fallback. The census hit — this door keeps a container saved under a + * name other than its object (the #13407 block above, #21412's P2 and + * P2b below, this block's own #21334 cases) — so only the name another + * stored container of the same object expands to is refused. + * + * The ruling's three pins, on both kernels and both scopes: the measured + * save is refused (every member kind the first container can expand the + * name from, the card's own pair, draft mode, a body with no `name`, a + * `form`-only body, and a sibling on another package's object); a + * container under its object's name saves and expands as before; a view + * item under an expanded name still saves. One more control is the + * census's: a container under a name of its own, not its object's and not + * a sibling's expansion, still saves. + */ + describe('#21620 the save door refuses a container saved under a name another stored container of the same object expands to', () => { + const LEAD = 'crm_lead'; + const leadData = { provider: 'object', object: LEAD }; + const leadList = (label: string) => ({ label, type: 'grid', data: leadData, columns: [{ field: 'name' }] }); + const leadForm = { type: 'simple', sections: [{ label: 'Main', fields: ['name'] }] }; + /** The card's second container: a bare `list`, bound to the same object (its own expansion is `crm_lead.default`). */ + const second = (name?: string) => ({ ...(name ? { name } : {}), object: LEAD, list: leadList('Other') }); + /** + * A second container whose own expansion (`crm_lead.other`) is never a + * name a first container below expands, so the refusal it meets is + * this check's and not #21558's. + */ + const secondKeyed = (name: string) => ({ name, object: LEAD, listViews: { other: leadList('Other') } }); + /** + * One first container per member kind, each on the runtime-authored + * object the card measured: the name it expands is the second + * container's save name. + */ + const FIRST_CASES: Record> = { + list: { list: leadList('First Default') }, + 'list#named': { list: { ...leadList('First Named'), name: 'hot' } }, + 'listViews.*': { listViews: { pipeline: leadList('First Pipeline') } }, + form: { form: leadForm }, + 'formViews.*': { formViews: { edit: leadForm } }, + }; + const FIRST = 'lead_first_views'; + + const saveIn = ( + protocol: Protocol, name: string, item: unknown, organizationId?: string, mode?: 'draft' | 'publish', + ) => protocol.saveMetaItem({ type: 'view', name, item, ...scoped(organizationId), ...(mode ? { mode } : {}) } as any); + const saved = async (write: Promise) => expect(((await write) as any)?.success).toBe(true); + const refusalOf = (write: Promise) => write.then(() => null, (e: any) => e); + const leadDoor = async (protocol: Protocol, organizationId?: string) => + switcherMatches(((await protocol.getMetaItems({ type: 'view', ...scoped(organizationId) } as any)) as any).items, LEAD); + /** The ADR-0112 envelope, and the two subjects the refusal names. */ + const expectRefused = (error: any, saveName: string, sibling: string) => { + expect(error).toBeInstanceOf(Error); + expect({ code: error?.code, status: error?.status }).toEqual({ code: 'VALIDATION_ERROR', status: 400 }); + expect(error.message, 'the refusal names the save name').toContain(`'${saveName}'`); + expect(error.message, 'the refusal names the container that expands it').toContain(`'${sibling}'`); + }; + /** Nothing of the refused save reached the store or the registry. */ + const expectNothingWritten = ( + rows: Map, registry: ReturnType, storedNames: string[], refused: string, + ) => { + expect([...rows.values()].filter((r) => r.type === 'view').map((r) => [r.name, r.state]), 'no row and no draft is stored') + .toEqual(storedNames.map((n) => [n, 'active'])); + expect( + registry.listItems('view').filter((it) => isAggregatedViewContainer(it) && it.name === refused), + 'no container is registered under the refused name', + ).toEqual([]); + }; + /** `name` answers one view item on BOTH doors — the same item — carrying `label`. */ + const expectServed = async (protocol: Protocol, name: string, organizationId: string | undefined, label: unknown) => { + const listed = named(await leadDoor(protocol, organizationId), name); + expect(listed, `exactly one item answers ${name} on the object door`).toHaveLength(1); + expect(listed[0]?.label).toBe(label); + const read = await byNameDoor(protocol, name, organizationId); + expect(isAggregatedViewContainer(read), `${name} by name is a view item, not a raw container`).toBe(false); + expect({ name: read?.name, viewKind: read?.viewKind, object: read?.object, config: read?.config }) + .toEqual({ name, viewKind: listed[0].viewKind, object: LEAD, config: listed[0].config }); + }; + + for (const [kernel, environmentId] of KERNELS) { + describe(`on ${kernel}`, () => { + for (const organizationId of [undefined, ORG]) { + const scope = organizationId ? 'organization-scoped' : 'environment-wide'; + + for (const [kind, member] of Object.entries(FIRST_CASES)) { + it(`${scope}, the first container's member ${kind}: a second container saved under the name it expands is refused VALIDATION_ERROR / 400; nothing is stored or registered, and the first container's view still answers on both doors`, async () => { + const { protocol, rows, registry } = showcaseHarness(environmentId); + const firstBody = { name: FIRST, object: LEAD, ...member }; + const [firstView] = expandViewContainer(LEAD, firstBody) as any[]; + expect(expandViewContainer(LEAD, firstBody), 'the first container expands one name').toHaveLength(1); + const name = String(firstView.name); + await saved(saveIn(protocol, FIRST, firstBody, organizationId)); + + expectRefused(await refusalOf(saveIn(protocol, name, secondKeyed(name), organizationId)), name, FIRST); + expectNothingWritten(rows, registry, [FIRST], name); + await expectServed(protocol, name, organizationId, firstView.label); + }); + } + + it(`${scope}: the card's save — '${LEAD}' stored with listViews.pipeline, then a second container saved as ${LEAD}.pipeline — is refused in publish and draft mode, with or without a body \`name\``, async () => { + const { protocol, rows, registry } = showcaseHarness(environmentId); + const first = { name: LEAD, object: LEAD, list: leadList('All Leads'), listViews: { pipeline: leadList('Lead Pipeline') } }; + await saved(saveIn(protocol, LEAD, first, organizationId)); + + const PIPELINE = `${LEAD}.pipeline`; + expectRefused(await refusalOf(saveIn(protocol, PIPELINE, second(PIPELINE), organizationId)), PIPELINE, LEAD); + // A body with no `name` is judged under the name the door stamps on it. + expectRefused(await refusalOf(saveIn(protocol, PIPELINE, second(), organizationId)), PIPELINE, LEAD); + expectRefused(await refusalOf(saveIn(protocol, PIPELINE, second(PIPELINE), organizationId, 'draft')), PIPELINE, LEAD); + expectNothingWritten(rows, registry, [LEAD], PIPELINE); + await expectServed(protocol, PIPELINE, organizationId, 'Lead Pipeline'); + await expectServed(protocol, `${LEAD}.default`, organizationId, 'All Leads'); + }); + + it(`${scope}: a \`form\`-only second container is refused by this check, before the identity stamp could turn it into a malformed view item`, async () => { + const { protocol, rows, registry } = showcaseHarness(environmentId); + await saved(saveIn(protocol, LEAD, { name: LEAD, object: LEAD, formViews: { edit: leadForm } }, organizationId)); + const EDIT = `${LEAD}.edit`; + expectRefused( + await refusalOf(saveIn(protocol, EDIT, { name: EDIT, object: LEAD, form: leadForm }, organizationId)), EDIT, LEAD, + ); + expectNothingWritten(rows, registry, [LEAD], EDIT); + }); + + it(`${scope}: a sibling on ANOTHER package's object is judged where the readers place its names — a container saved under its own-name expansion is refused`, async () => { + const { protocol, rows } = showcaseHarness(environmentId); + const first = { name: OWN, object: TASK, listViews: { in_progress: { ...listView, label: 'Own In Progress' } } }; + await saved(protocol.saveMetaItem({ type: 'view', name: OWN, item: first, packageId: REPAIR } as any)); + const UNDER = `${TASK}.${OWN}.in_progress`; + expect(named(await objectDoor(protocol, organizationId), UNDER), 'the sibling serves its own-name expansion').toHaveLength(1); + + const body = { name: UNDER, object: TASK, list: listView }; + expectRefused(await refusalOf(saveIn(protocol, UNDER, body, organizationId)), UNDER, OWN); + expect([...rows.values()].filter((r) => r.type === 'view').map((r) => r.name)).toEqual([OWN]); + expect(named(await objectDoor(protocol, organizationId), UNDER)).toHaveLength(1); + await expectEveryPackagedNameIntact(protocol, organizationId); + }); + + it(`${scope}: CONTROL — a container under its object's name saves and expands as before, beside a sibling, and re-saves`, async () => { + const { protocol } = showcaseHarness(environmentId); + await saved(saveIn(protocol, 'lead_hot_views', { name: 'lead_hot_views', object: LEAD, listViews: { hot: leadList('Hot Leads') } }, organizationId)); + const own = { name: LEAD, object: LEAD, list: leadList('All Leads'), listViews: { pipeline: leadList('Lead Pipeline') } }; + await saved(saveIn(protocol, LEAD, own, organizationId)); + // Its own row is the row a re-save replaces, never a sibling. + await saved(saveIn(protocol, LEAD, { ...own, list: leadList('All Leads, edited') }, organizationId)); + await expectServed(protocol, `${LEAD}.default`, organizationId, 'All Leads, edited'); + await expectServed(protocol, `${LEAD}.pipeline`, organizationId, 'Lead Pipeline'); + await expectServed(protocol, `${LEAD}.hot`, organizationId, 'Hot Leads'); + }); + + it(`${scope}: CONTROL — a view item saved under a sibling's expanded name saves, as that name's sanctioned override`, async () => { + const { protocol } = showcaseHarness(environmentId); + await saved(saveIn(protocol, LEAD, { name: LEAD, object: LEAD, listViews: { pipeline: leadList('Lead Pipeline') } }, organizationId)); + const PIPELINE = `${LEAD}.pipeline`; + const item = { name: PIPELINE, object: LEAD, viewKind: 'list', label: 'ByNameRow', config: { type: 'grid', data: leadData, columns: [{ field: 'name' }] } }; + await saved(saveIn(protocol, PIPELINE, item, organizationId)); + await expectServed(protocol, PIPELINE, organizationId, 'ByNameRow'); + }); + + it(`${scope}: CONTROL (the census) — a container saved under a name of its own, neither its object's nor a sibling's expansion, still saves and serves`, async () => { + const { protocol } = showcaseHarness(environmentId); + await saved(saveIn(protocol, LEAD, { name: LEAD, object: LEAD, list: leadList('All Leads'), listViews: { pipeline: leadList('Lead Pipeline') } }, organizationId)); + await saved(saveIn(protocol, 'lead_hot_views', { object: LEAD, listViews: { hot: leadList('Hot Leads') } }, organizationId)); + await expectServed(protocol, `${LEAD}.hot`, organizationId, 'Hot Leads'); + await expectServed(protocol, `${LEAD}.pipeline`, organizationId, 'Lead Pipeline'); + }); + } + + it('the prescription names the sibling container that expands the name and never prescribes a save under it — in the card\'s pair the object\'s own name IS that sibling', async () => { + const { protocol, rows, registry } = showcaseHarness(environmentId); + await saved(saveIn(protocol, LEAD, { name: LEAD, object: LEAD, listViews: { pipeline: leadList('Lead Pipeline') } })); + const PIPELINE = `${LEAD}.pipeline`; + const error = await refusalOf(saveIn(protocol, PIPELINE, second(PIPELINE))); + // The envelope, and the sibling named as the container that expands the name. + expectRefused(error, PIPELINE, LEAD); + // A save under the sibling's name would replace its row and drop `pipeline`, + // the view this refusal keeps serving: no arm may name that save. So every + // place the sibling's name appears names it as THE CONTAINER (or as the + // object a view binds to), never as a name to save under. + const leadIns = String(error.message).split(`'${LEAD}'`).slice(0, -1); + expect(leadIns.filter((s) => /container $/.test(s)).length, 'the sibling is named as the container').toBeGreaterThan(0); + expect( + leadIns.filter((s) => !/(container|on) $/.test(s)), + 'the sibling\'s (here the object\'s) name is never prescribed as a name to save under', + ).toEqual([]); + expect(error.message).not.toContain(`under '${LEAD}'`); + expectNothingWritten(rows, registry, [LEAD], PIPELINE); + await expectServed(protocol, PIPELINE, undefined, 'Lead Pipeline'); + }); + + it('an environment-wide sibling is in an organization caller\'s selection: that caller\'s save under its expanded name is refused', async () => { + const { protocol, rows, registry } = showcaseHarness(environmentId); + await saved(saveIn(protocol, LEAD, { name: LEAD, object: LEAD, listViews: { pipeline: leadList('Lead Pipeline') } })); + const PIPELINE = `${LEAD}.pipeline`; + expectRefused(await refusalOf(saveIn(protocol, PIPELINE, second(PIPELINE), ORG)), PIPELINE, LEAD); + expectNothingWritten(rows, registry, [LEAD], PIPELINE); + await expectServed(protocol, PIPELINE, ORG, 'Lead Pipeline'); + }); + + it('CONTROL — another organization\'s container is not this caller\'s sibling: the save is judged by the caller\'s own selection, as the readers judge', async () => { + const { protocol } = showcaseHarness(environmentId); + await saved(saveIn(protocol, LEAD, { name: LEAD, object: LEAD, listViews: { pipeline: leadList('Lead Pipeline') } }, 'org_globex')); + const PIPELINE = `${LEAD}.pipeline`; + await saved(saveIn(protocol, PIPELINE, second(PIPELINE), ORG)); + // The other organization still gets its own container's view, on both doors. + await expectServed(protocol, PIPELINE, 'org_globex', 'Lead Pipeline'); + }); + }); + } + }); }); /**