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
19 changes: 19 additions & 0 deletions .changeset/21412-metadata-protocol-save-door-container-name.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
'@objectstack/metadata-protocol': minor
---

The runtime save door refuses a view container whose own `name` disagrees with the name it is saved under

Clause-②: no (narrowing)

<!-- adr-0087: not-required (no-migration-prescription) A validity narrowing at one door over an existing key: `ViewSchema.name` is not removed, renamed or re-shaped, so there is no tombstone and nothing mechanical for `objectstack migrate meta` to rewrite. Which of the two names a divergent container meant (the body's, or the one it was saved under) is authoring intent no conversion entry can decide. New saves are refused with the remedy; a row stored before this change keeps its bytes. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id covers this rule and this diff adds none (not registered / already-registered); and the change narrows what a runtime write door accepts, not a runtime interface or a type surface alone (not runtime-interface-only / type-surface-only). -->

**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 ObjectQL boot loop's refusal of the same divergence 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`) whose body carried a `name` different from the name it was saved under. It stored the row under the save name and registered the container under the body's `name`, so one document answered under two names. The source registrars (the ObjectQL boot loop and the artifact/HMR loader) and `os validate` already refused a container whose `name` disagrees with the key they file it under.

**What is refused now.** That body, with `VALIDATION_ERROR` / 400, before anything is stored or registered, through the same judge the source registrars call (`@objectstack/metadata/view-container-name`). The key judged here is the save name: a container saved under a name other than the object it binds to still saves, and so does the body the door stores for it when it is read and sent back.

**The fix.** Drop the body's `name` (the door stamps the save name), or set it to the name the container is saved under.

Not judged here: a standalone view record (`viewKind`) and every other metadata type.
10 changes: 10 additions & 0 deletions .changeset/21412-metadata-view-container-name-judge.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
'@objectstack/metadata': minor
---

One judge for a view container's own `name` at every door that files a container: the new `@objectstack/metadata/view-container-name` entry

Clause-②: yes

- New subpath `@objectstack/metadata/view-container-name`. It exports `viewContainerNameRefusal(container, sourceLabel, ownerId)`, the source registrars' entry, whose key is the object the container binds to (its own `object`, else `list.data.object` / `form.data.object`). It also exports `savedViewContainerNameRefusal(container, saveName)`, the runtime save door's entry, whose key is the name the row is saved under, and the `ViewContainerNameRefusal` type. Both return a `VALIDATION_ERROR` / 400 refusal for an aggregated view container whose own `name` is set and differs from that key, and `undefined` otherwise. A container with no `name`, and a standalone view record (`viewKind`), are not judged.
- The artifact/HMR loader's container branch now refuses such a container through the judge, before it files anything. What it refuses and the envelope are unchanged (`VALIDATION_ERROR` / 400). The message is now the judge's, the words the ObjectQL boot loop and `os validate` print, where it was the generic `IMetadataService.register` contract's.
9 changes: 9 additions & 0 deletions .changeset/21412-objectql-view-container-name-reexport.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'@objectstack/objectql': patch
---

`viewContainerNameRefusal` is now re-exported from `@objectstack/metadata/view-container-name`

Clause-②: no

The divergent view-container `name` judge moved to `@objectstack/metadata`, the one layer the boot loop, the artifact/HMR loader and the runtime save door all depend on, so all three call one judge. `@objectstack/objectql` keeps the `viewContainerNameRefusal` export, its signature and the `ViewContainerNameRefusal` type. The boot loop's refusal and the words it and `os validate` print are unchanged, byte for byte.
9 changes: 9 additions & 0 deletions .changeset/21412-spec-view-container-name-comment.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'@objectstack/spec': patch
---

The comment above `ViewSchema`'s `guidance:` states who writes a view container's `name`, and the rule every door applies to it

Clause-②: no

`src/ui/view.zod.ts` ships as source, and the comment also ships in the `ui` JavaScript output. It used to say that `saveMetaItem` sends a container's `name`, that artifact-shipped containers do, and that the validation sweep injects it. It now says the metadata door's own stamp (`normalizeViewMetadata`) is the only platform writer of the key. Artifact-shipped containers carry none, and the sweep passes its name as the request name. It also states the rule: when an authored `name` is set, it must equal the key the door files the container under, or the door refuses it. ⛔ No schema, parse, export or accept-set change.
25 changes: 24 additions & 1 deletion packages/metadata-protocol/src/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ import {
// `sys-metadata-repository.ts` in this package and with `DatabaseLoader` in
// `@objectstack/metadata` (#5108). See `rethrowUnlessMetadataStoreUnprovisioned`.
import { isMissingTableError } from '@objectstack/metadata/errors';
// [#21412] The divergent view-container `name` refusal — the one judge the
// boot loop, `os validate` and the artifact/HMR loader call too; this door
// passes the name it files the row under. See `saveMetaItem`.
import { savedViewContainerNameRefusal } from '@objectstack/metadata/view-container-name';
import type {
BatchUpdateRequest,
BatchUpdateResponse,
Expand Down Expand Up @@ -1218,7 +1222,10 @@ export { stripReadDecorations };
* into a record risks producing an invalid record (e.g. a non-`<object>.<key>`
* name). Structural validity is enforced separately by the view metadata schema
* during the spec-validation step. No-op for non-view types and bodies that
* already carry a `name`.
* already carry a `name`. [#21412] A CONTAINER's authored `name` reaches this
* function only when it equals `saveName`: `saveMetaItem` refuses a
* disagreeing one first, through `savedViewContainerNameRefusal`, so keeping
* the authored `name` can no longer file a container under a second key.
*
* When `baseline` is provided (the registry entry this overlay will shadow),
* missing identity fields — `viewKind`, `object`, `label` — are inherited onto
Expand Down Expand Up @@ -17713,6 +17720,22 @@ export class ObjectStackProtocolImplementation implements
// (#2555 — a console personalization PUT sends only the raw config).
// See {@link normalizeViewMetadata}.
{
// [#21412] FIRST, before the stamp below can keep an authored
// `name`: a view CONTAINER whose own `name` disagrees with the name
// this door files the row under is refused, `VALIDATION_ERROR` /
// 400, through the one judge the source registrars call. Accepted,
// it was stored under the row name and registered under the
// body's (`hydrateOverlayIntoRegistry` keys by `body.name`), so one
// document answered under two names. The key here is the save
// name, not the derived binding: this door keeps a container saved
// under a name other than its object (#13407, #21334), and the
// body it stamps for one must pass when sent back. A body with no
// `name` passes and is stamped below. Containers only — the
// every-type half is #21470.
if (singularType === 'view') {
const nameRefusal = savedViewContainerNameRefusal(request.item, request.name);
if (nameRefusal) throw nameRefusal;
}
let baseline: unknown;
if ((PLURAL_TO_SINGULAR[request.type] ?? request.type) === 'view'
&& typeof this.engine.registry?.getItem === 'function') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@
*/
import { describe, expect, it } from 'vitest';
import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFindOnePredicate, isCodeArtifactBody } from '@objectstack/metadata-core';
import { expandViewContainer, ViewSchema } from '@objectstack/spec/ui';
import { expandViewContainer, isAggregatedViewContainer, ViewSchema } from '@objectstack/spec/ui';
import { MetadataPlugin } from '@objectstack/metadata';
import { savedViewContainerNameRefusal } from '@objectstack/metadata/view-container-name';
import { ObjectStackProtocolImplementation } from './index.js';

interface Row {
Expand Down Expand Up @@ -773,3 +775,122 @@ describe('#21334 a container on another package\'s object never takes that packa
await expectEveryPackagedNameIntact(protocol);
});
});

/**
* #21412 — the runtime save door refuses a view container whose own `name`
* disagrees with the name it is saved under, through the one judge the source
* registrars call (`@objectstack/metadata/view-container-name`).
*
* Before: the card's probe was ACCEPTED — stored as row `crm_lead` with body
* `name` `lead_views`, and registered as `lead_views` (the container, keyed by
* `body.name` in `hydrateOverlayIntoRegistry`) plus `crm_lead.default`: one
* document answering under a name its row does not have. The two source
* registrars refuse the same document.
*
* The key judged here is the SAVE name, not the binding: this door keeps a
* container saved under a name other than its object (the #13407 case above,
* #21334's arm), so the body it stamps for such a container must pass when it
* is sent back. Shapes as the card's measurement named them (row = save name):
* P1 row crm_lead / `name` lead_views; P2 row lead_views / `name` lead_views,
* bound to crm_lead; P2b P2 with no `name`; P3 row lead_views / `name`
* crm_lead; P4 row crm_lead / `name` lead_views, no other binding.
*/
describe('#21412 the save door refuses a container whose own name disagrees with the name it is saved under', () => {
const named = (name: string | undefined, body: Record<string, unknown>) =>
(name === undefined ? { ...body } : { name, ...body });
/** Bound to crm_lead through its own `object`, no `data` on any arm. */
const objectBound = { object: 'crm_lead', list: { label: 'All Leads', type: 'grid', columns: [{ field: 'name' }] } };
/** No binding but whatever `name` it carries. */
const unbound = { list: { label: 'All', type: 'grid', columns: [{ field: 'name' }] } };

async function save(name: string, item: unknown) {
const harness = makeStubEngine();
const protocol = new ObjectStackProtocolImplementation(harness.engine);
let error: any = null;
try {
await protocol.saveMetaItem({ type: 'view', name, item });
} catch (e) {
error = e;
}
const viewRows = Array.from(harness.rows.values()).filter((r) => r.type === 'view');
// The keys the registry holds a CONTAINER under — its expansions carry
// `viewKind` and are the container's derived items, not a second key
// for the document (seat answer Q4).
const containerKeys = Array.from(harness.registered.get('view')?.entries() ?? [])
.filter(([, v]) => isAggregatedViewContainer(v))
.map(([k]) => k);
return { ...harness, protocol, error, viewRows, containerKeys };
}

function expectRefused(outcome: Awaited<ReturnType<typeof save>>) {
// The minimum a rejection pin asserts: the ADR-0112 envelope.
expect(outcome.error).toBeInstanceOf(Error);
expect(outcome.error.code).toBe('VALIDATION_ERROR');
expect(outcome.error.status).toBe(400);
// ...and the refused document reached nothing.
expect(outcome.viewRows).toEqual([]);
expect(outcome.registered.get('view')?.size ?? 0).toBe(0);
}

it('P1, the card\'s probe: refused VALIDATION_ERROR / 400, nothing stored, nothing registered', async () => {
expectRefused(await save('crm_lead', named('lead_views', leadContainer)));
});

it('P1 is refused THROUGH the judge: the door throws exactly what it returns for that document', async () => {
const body = named('lead_views', leadContainer);
const { error } = await save('crm_lead', body);
expect(error.message).toBe(savedViewContainerNameRefusal(body, 'crm_lead')!.message);
});

it('P1 answers the envelope a source registrar answers for the same document', async () => {
const body = named('lead_views', objectBound);
const { error: saveDoor } = await save('crm_lead', body);
const plugin = new MetadataPlugin({ watch: false, config: { bootstrap: 'lazy' } }) as any;
const ctx = {
logger: { info: () => {}, warn: () => {}, error: () => {}, debug: () => {} },
registerService: () => {}, getService: () => undefined, trigger: async () => {},
} as any;
const registrar = await plugin._parseAndRegisterArtifact(ctx, JSON.parse(JSON.stringify({
manifest: { id: 'com.acme.crm', name: 'CRM', version: '1.0.0', type: 'app' },
views: [body],
})), 'fixture-21412').then(() => null, (e: any) => e);
expect(registrar).toBeInstanceOf(Error);
expect([saveDoor.code, saveDoor.status]).toEqual([registrar.code, registrar.status]);
expect([saveDoor.code, saveDoor.status]).toEqual(['VALIDATION_ERROR', 400]);
});

it('P3: a `name` equal to the binding but not to the row is refused', async () => {
expectRefused(await save('lead_views', named('crm_lead', leadContainer)));
});

it('P4: a `name` that is the only binding, but not the row, is refused', async () => {
expectRefused(await save('crm_lead', named('lead_views', unbound)));
});

it('P2: a `name` equal to the row passes though the container binds elsewhere — one key, the row\'s', async () => {
const outcome = await save('lead_views', named('lead_views', objectBound));
expect(outcome.error).toBeNull();
expect(outcome.viewRows.map((r) => [r.name, JSON.parse(r.metadata).name])).toEqual([['lead_views', 'lead_views']]);
expect(outcome.containerKeys).toEqual(['lead_views']);
const list: any = await outcome.protocol.getMetaItems({ type: 'view' });
expect(switcherMatches(list.items, 'crm_lead').map((v: any) => v.name)).toEqual(['crm_lead.default']);
});

it('P2b: an absent `name` passes and is stamped with the row name — and the stamped body passes when sent back', async () => {
const outcome = await save('lead_views', named(undefined, objectBound));
expect(outcome.error).toBeNull();
expect(outcome.viewRows.map((r) => JSON.parse(r.metadata).name)).toEqual(['lead_views']);
expect(outcome.containerKeys).toEqual(['lead_views']);

const read: any = await outcome.protocol.getMetaItem({ type: 'view', name: 'lead_views' });
expect(read.item.name).toBe('lead_views');
const { _diagnostics: _drop, ...sentBack } = read.item;
await expect(outcome.protocol.saveMetaItem({ type: 'view', name: 'lead_views', item: sentBack })).resolves.toBeTruthy();
});

it('CONTROL: a `name` equal to the row and the binding passes, under one key', async () => {
const outcome = await save('crm_lead', named('crm_lead', leadContainer));
expect(outcome.error).toBeNull();
expect(outcome.containerKeys).toEqual(['crm_lead']);
});
});
10 changes: 10 additions & 0 deletions packages/metadata/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,16 @@
"types": "./dist/view-container.d.cts",
"default": "./dist/view-container.cjs"
}
},
"./view-container-name": {
"import": {
"types": "./dist/view-container-name.d.ts",
"default": "./dist/view-container-name.js"
},
"require": {
"types": "./dist/view-container-name.d.cts",
"default": "./dist/view-container-name.cjs"
}
}
},
"files": [
Expand Down
14 changes: 14 additions & 0 deletions packages/metadata/src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,10 @@ import { isAggregatedViewContainer, expandViewContainer } from '@objectstack/spe
// chain of their own; it carries the same order #13407 settled at the runtime
// door (`expandRuntimeViewContainer` in `packages/metadata-protocol`).
import { deriveViewContainerObject } from './view-container-expansion.js';
// [#21412] The divergent container `name` refusal — the one judge the boot
// loop, `os validate` and the runtime save door call too. See the container
// branch of `_registerArtifactBodyCollections`.
import { viewContainerNameRefusal } from './view-container-name.js';
import type { IHttpServer } from '@objectstack/spec/contracts';


Expand Down Expand Up @@ -1177,6 +1181,16 @@ export class MetadataPlugin implements Plugin {
// container, so the flattened copy is the duplicate, not a
// second definition.
if (slots.skip?.('view', viewObject)) continue;
// [#21412] A container whose own `name` disagrees with the
// key derived above is refused through the one judge every
// door that files a container calls, in its words — and
// BEFORE `memLoader.save`, so a refusal files nothing.
// `manager.register` below would refuse the same document
// too (#7378 row 1, `assertMetadataRegisterContract`), but
// in the generic register contract's words and only after
// the loader write; row 1 is unchanged for every type.
const nameRefusal = viewContainerNameRefusal(item, 'artifact', packageId);
if (nameRefusal) throw nameRefusal;
applyProtection(item as any, {
packageId: packageId,
packageVersion: packageVersion,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,9 @@ describe('TypeScriptSerializer annotation, per metadata type', () => {
it('no exports entry of the package re-exports the internal channel', async () => {
// Control: the name is spelled right, so the absences below can fail.
expect(Object.keys(await import('./typescript-serializer.js'))).toContain('serializeTypeScriptForMetadataType');
expect(EXPORT_ENTRY_SOURCES.length).toBe(5);
// Six since `./view-container-name` (#21412): the count is the control
// that the loop below visits every entry, so a new entry moves it here.
expect(EXPORT_ENTRY_SOURCES.length).toBe(6);
for (const source of EXPORT_ENTRY_SOURCES) {
const entry = (await import(source)) as Record<string, unknown>;
expect(Object.keys(entry).length, source).toBeGreaterThan(0);
Expand Down
Loading
Loading