Skip to content

Commit bbee302

Browse files
os-zhuangclaude
andauthored
test(spec,objectql): pin the IMetadataService register → get round-trip across every shipped implementation (#7223) (#7371)
`register(type, name, data)` and `get(type, name)` are the contract's first two CRUD members, and the round-trip between them was exercised in exactly one place — `packages/spec/src/contracts/metadata-service.test.ts`, against a hand-rolled `Map`-of-`Map`s double written inside the test itself. No shipped implementation was held to it, which is the hole #6725 fell through: a shipped, exported `IMetadataService` could not perform its own most basic round-trip while the full objectql suite and all 64 `lint.yml` gates stayed green. Adds `METADATA_ROUNDTRIP_CASES` (`@objectstack/spec/contracts`) — 15 cases, one table, a thin driver per implementation, the shape `data/filter-logic-conformance.ts` already uses for filter backends — plus the two drivers that run it: - the contract's own reference double, in `packages/spec` (the dependency root, which can see no implementation); - every implementation this repo ships, in `packages/objectql` (the only package that can see all three at once): `MetadataManager` with and without a writable loader, `createMemoryMetadata`, `MetadataFacade`. The pre-existing Map double in `metadata-service.test.ts` is untouched — it pins the contract's type surface and its own inline round-trip, independent of any implementation. No shipped behaviour changes. Three cases get different answers from `MetadataFacade` than from the other implementations and the reference double; each is pinned as measured under a `// DIVERGENCE` marker and filed as its own card rather than reconciled here. Verified the suite is not vacuous by re-introducing the #6725 split locally (dropping the contributor write from `MetadataFacade.registerObjectBothPlaces`): four rows go red, including the plain object round-trip. Refs #7223, #6725, PR #7211, #6745. Claude-Session: https://claude.ai/code/session_0193R6tMZqgrdFrCSnaogFc4 Co-authored-by: Claude <noreply@anthropic.com>
1 parent f9a5c59 commit bbee302

7 files changed

Lines changed: 789 additions & 0 deletions

File tree

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
test(spec,objectql): pin the `IMetadataService` `register``get` round-trip across every shipped implementation (#7223)
6+
7+
`register(type, name, data)` and `get(type, name)` are the contract's first two
8+
CRUD members, and until now the round-trip between them was exercised in exactly
9+
ONE place — `contracts/metadata-service.test.ts`, against a `Map`-of-`Map`s
10+
double written inside the test itself. No **shipped** implementation was held to
11+
it. That is the hole #6725 fell through: `MetadataFacade.register('object', …)`
12+
wrote into a map none of its own reads consulted, every read answered
13+
`undefined`, and the full `packages/objectql` suite plus all 64 `lint.yml` gates
14+
stayed green while a shipped, exported implementation of the platform's central
15+
metadata contract could not perform its own most basic round-trip.
16+
17+
**`METADATA_ROUNDTRIP_CASES`** (`@objectstack/spec/contracts`) is the shared
18+
table that closes it — 15 cases covering the plain round-trip on an object-typed
19+
and a non-object-typed write, the miss shape, re-registration, type scoping in
20+
both directions, name case sensitivity, and the `data`-keying and primitive-value
21+
edges. Same shape as `FILTER_LOGIC_CASES`: one table, a thin driver per
22+
implementation. Third-party authors implementing the contract can run it without
23+
depending on ObjectQL.
24+
25+
Two drivers ship with it: the contract's own reference double (in `spec`, which
26+
has no runtime and can see no implementation), and every implementation this repo
27+
ships — `MetadataManager` with and without a writable loader,
28+
`createMemoryMetadata`, and `MetadataFacade` — driven from `packages/objectql`,
29+
the only package that can see all three at once.
30+
31+
No shipped behaviour changes. Where implementations answer a case differently
32+
today, each answer is pinned as measured under a `// DIVERGENCE` marker rather
33+
than reconciled — see the notes in the objectql driver and the card they link.
Lines changed: 341 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,341 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* `METADATA_ROUNDTRIP_CASES` driver #2 — every `IMetadataService` this repo
5+
* SHIPS (#7223).
6+
*
7+
* The table lives in `@objectstack/spec`
8+
* (`contracts/metadata-service-roundtrip-conformance.ts`) together with the
9+
* reference answers and the argument for why it exists; this file is the thin
10+
* driver that replays it against real implementations. Read the table's header
11+
* first — in particular what `expected` does and does not claim.
12+
*
13+
* `packages/objectql` hosts it because it is the only package that can see all
14+
* three implementations at once: it depends on `@objectstack/metadata`
15+
* (`MetadataManager`) and `@objectstack/core` (`createMemoryMetadata`) and owns
16+
* `MetadataFacade`. That is the same argument
17+
* `metadata-service-getobject-equivalence.test.ts` (#6745) already makes for
18+
* living here, and it is why `packages/spec` cannot host this half.
19+
*
20+
* ## The four subjects, and why four for three implementations
21+
*
22+
* `MetadataManager` appears twice. Its `register` writes the in-memory registry
23+
* AND persists to every `datasource:` loader that declares write capability, so
24+
* a subject with no loaders never executes the second half. The writable-loader
25+
* subject is the one that would notice a `register` that threw, silently
26+
* skipped, or mutated the document on the way to `loader.save`.
27+
*
28+
* ## Two assertion strengths, declared per subject
29+
*
30+
* `documentFidelity` says whether a subject hands back the document it was
31+
* given. `MetadataManager` and `createMemoryMetadata` store and return the very
32+
* reference (`verbatim`), so they are held to exact equality. `MetadataFacade`
33+
* resolves objects through `SchemaRegistry`, which answers the RUNTIME-EFFECTIVE
34+
* object — system fields (`organization_id`, `created_at`, …) injected,
35+
* extensions merged — and copies non-object documents while filling in `name`.
36+
* `toEqual(input)` is therefore the wrong assertion for it, exactly as #7223
37+
* predicted; it is held to a recursive-subset match plus every key/visibility
38+
* assertion the others get. The weaker match is scoped to the ONE subject that
39+
* needs it rather than applied to the whole table.
40+
*
41+
* ## Divergences are PINNED, not resolved
42+
*
43+
* Three cases get different answers from `MetadataFacade` than from the other
44+
* implementations and the contract's reference double. Each is recorded below
45+
* as a `// DIVERGENCE` entry stating the measured behaviour — this file asserts
46+
* what each implementation does TODAY and changes no shipped behaviour. Which
47+
* answer is correct is a separate ruling, filed as its own card (see the
48+
* per-divergence notes). If you are here because one of these tests failed
49+
* after a behaviour change: that is the pin working. Update it in the PR that
50+
* makes the ruling, not silently.
51+
*
52+
* Refs #7223, #6725, PR #7211, #6745.
53+
*/
54+
55+
import { describe, it, expect } from 'vitest';
56+
import {
57+
METADATA_ROUNDTRIP_CASES,
58+
type MetadataRoundTripCase,
59+
type IMetadataService,
60+
} from '@objectstack/spec/contracts';
61+
import { SchemaRegistry } from './registry';
62+
import { MetadataFacade } from './metadata-facade';
63+
import { MetadataManager, type MetadataLoader } from '@objectstack/metadata';
64+
import { createMemoryMetadata } from '@objectstack/core';
65+
66+
/**
67+
* The members the table exercises, and nothing else. Typed against the contract
68+
* rather than the concrete classes on purpose: a signature change on any of the
69+
* four should reach this file through `tsc`.
70+
*/
71+
type RoundTrippingService = Pick<IMetadataService, 'register' | 'get' | 'exists' | 'listNames' | 'unregister'>;
72+
73+
/**
74+
* How a subject answers a `readable` row.
75+
*
76+
* - `verbatim` — `get` returns the document `register` was handed. Asserted
77+
* with exact equality.
78+
* - `runtime-effective` — `get` returns a derived document that CONTAINS the
79+
* authored one. Asserted as a recursive subset.
80+
*/
81+
type DocumentFidelity = 'verbatim' | 'runtime-effective';
82+
83+
/**
84+
* A per-subject answer that differs from the table's reference answer.
85+
* `readable-as-last-write` means "the document the case's final write carried".
86+
*/
87+
type DivergentAnswer =
88+
| { readonly kind: 'absent'; readonly note: string }
89+
| { readonly kind: 'readable-as-last-write'; readonly note: string };
90+
91+
interface PinnedImplementation {
92+
readonly label: string;
93+
readonly documentFidelity: DocumentFidelity;
94+
/** Keyed by {@link MetadataRoundTripCase.id}. Every key is checked to exist. */
95+
readonly divergences?: Readonly<Record<string, DivergentAnswer>>;
96+
create(): RoundTrippingService;
97+
}
98+
99+
/**
100+
* A minimal writable `datasource:` loader, so `MetadataManager.register`'s
101+
* persistence half actually runs. `save` AND `delete` are both required by
102+
* `assertWritableLoaderContract` — the sole gate into `MetadataManager`'s
103+
* loader map.
104+
*/
105+
class WritableFixtureLoader implements MetadataLoader {
106+
readonly contract: MetadataLoader['contract'] = {
107+
name: 'roundtrip-conformance-writable',
108+
protocol: 'datasource:',
109+
capabilities: { read: true, write: true, watch: false, list: true },
110+
};
111+
112+
private readonly storage = new Map<string, unknown>();
113+
114+
/**
115+
* NUL as the type/name separator, written as an escape rather than as a
116+
* literal byte -- a literal one makes git treat this file as binary and
117+
* trips `check:nul-bytes`. Neither a metadata type nor a name can contain
118+
* one, so no two distinct pairs can collide on a single key the way they
119+
* could with a `:` or `/` separator.
120+
*/
121+
private key(type: string, name: string): string {
122+
return `${type}\u0000${name}`;
123+
}
124+
125+
async save(type: string, name: string, data: unknown): Promise<void> {
126+
this.storage.set(this.key(type, name), data);
127+
}
128+
129+
async delete(type: string, name: string): Promise<void> {
130+
this.storage.delete(this.key(type, name));
131+
}
132+
133+
async load(type: string, name: string) {
134+
const data = this.storage.get(this.key(type, name));
135+
return data === undefined
136+
? { data: null }
137+
: { data, source: this.contract.name, format: 'json' as const, loadTime: 0 };
138+
}
139+
140+
async loadMany<T = unknown>(type: string): Promise<T[]> {
141+
return this.entriesOfType(type).map(([, value]) => value) as T[];
142+
}
143+
144+
async exists(type: string, name: string): Promise<boolean> {
145+
return this.storage.has(this.key(type, name));
146+
}
147+
148+
async stat() {
149+
return null;
150+
}
151+
152+
async list(type: string): Promise<string[]> {
153+
return this.entriesOfType(type).map(([key]) => key.slice(type.length + 1));
154+
}
155+
156+
private entriesOfType(type: string): Array<[string, unknown]> {
157+
return Array.from(this.storage.entries()).filter(([key]) => key.startsWith(`${type}\u0000`));
158+
}
159+
}
160+
161+
/**
162+
* ── DIVERGENCE 1 — the effective key is `data.name`, not the `name` argument ──
163+
*
164+
* Cases `key-is-the-name-argument-object` / `-nonobject`.
165+
*
166+
* `MetadataFacade.register` opens with
167+
* `{ ...data, name: data.name ?? name }` and then hands the DOCUMENT to
168+
* `SchemaRegistry.registerObject` / `registerItem`, which key on the document's
169+
* own `name`. The `name` argument is therefore only a fallback for a document
170+
* that carries none: when the two disagree, the item lands under `data.name`
171+
* and `get(type, <the name that was passed>)` answers `undefined`, `exists`
172+
* answers `false`, and `listNames` reports the other spelling. Measured on both
173+
* an object-typed and a view-typed write.
174+
*
175+
* `MetadataManager` and `createMemoryMetadata` both key on the argument, as does
176+
* the contract's reference double. The contract TSDoc names the parameter on
177+
* both members (`@param name - Item name/identifier (snake_case)`) and says
178+
* nothing about `data.name`, so nothing in-tree currently RULES which is right —
179+
* which is why this is pinned as measured and filed, not fixed here.
180+
*/
181+
const DIVERGENCE_1 = 'MetadataFacade keys on `data.name` when it disagrees with the `name` argument; the other implementations key on the argument. Pinned as measured (#7223).';
182+
183+
/**
184+
* ── DIVERGENCE 2 — the plural `objects` type is aliased to `object` ──
185+
*
186+
* Case `plural-objects-type-is-its-own-store`.
187+
*
188+
* `MetadataFacade`'s `isObjectType` treats `'object'` and `'objects'` as the
189+
* same type on the WRITE side (deliberately, per its header: #6725 left the
190+
* plural with the same read/write split the singular had). The consequence this
191+
* case measures is on the READ side: a `register('objects', n, …)` is visible
192+
* through `get('object', n)`, `exists('object', n)` and `listNames('object')`.
193+
*
194+
* `MetadataManager` and `createMemoryMetadata` key their type stores on the
195+
* string they are handed, so the two spellings are two stores and the item is
196+
* invisible under the singular.
197+
*/
198+
const DIVERGENCE_2 = 'MetadataFacade aliases the plural `objects` type to `object`; the other implementations keep one store per type string. Pinned as measured (#7223).';
199+
200+
/**
201+
* ── DIVERGENCE 3 — a non-object `data` value is dropped ──
202+
*
203+
* Case `primitive-data-roundtrips`.
204+
*
205+
* The contract declares `data: unknown`. `MetadataFacade.register` passes a
206+
* non-object value through unchanged (its `{ ...data }` branch is guarded on
207+
* `typeof data === 'object' && data !== null`) and then registers it under the
208+
* document's own `name` — which a string does not have. The write is ACCEPTED
209+
* (no throw), the registry logs `Registered setting: undefined`, and the value
210+
* is readable back through nothing: `get` answers `undefined`, `exists` answers
211+
* `false`, `listNames` is empty. Silent loss, the same family of failure as
212+
* #6725 — which is the reason this row is in the table at all.
213+
*
214+
* `MetadataManager` and `createMemoryMetadata` store the value against the key
215+
* and hand it straight back.
216+
*/
217+
const DIVERGENCE_3 = 'MetadataFacade silently drops a non-object `data` value — accepted by `register`, readable back through no member. The other implementations round-trip it. Pinned as measured (#7223).';
218+
219+
const IMPLEMENTATIONS: readonly PinnedImplementation[] = [
220+
{
221+
label: 'MetadataManager (registry only)',
222+
documentFidelity: 'verbatim',
223+
create: () => new MetadataManager({ formats: ['json'], loaders: [] }),
224+
},
225+
{
226+
// The half a loader-less manager never executes: `register` persists to
227+
// every writable `datasource:` loader before it announces.
228+
label: 'MetadataManager (writable datasource loader)',
229+
documentFidelity: 'verbatim',
230+
create: () => new MetadataManager({ formats: ['json'], loaders: [new WritableFixtureLoader()] }),
231+
},
232+
{
233+
label: 'createMemoryMetadata',
234+
documentFidelity: 'verbatim',
235+
create: () => createMemoryMetadata(),
236+
},
237+
{
238+
label: 'MetadataFacade',
239+
documentFidelity: 'runtime-effective',
240+
divergences: {
241+
'key-is-the-name-argument-object': { kind: 'absent', note: DIVERGENCE_1 },
242+
'key-is-the-name-argument-nonobject': { kind: 'absent', note: DIVERGENCE_1 },
243+
'plural-objects-type-is-its-own-store': { kind: 'readable-as-last-write', note: DIVERGENCE_2 },
244+
'primitive-data-roundtrips': { kind: 'absent', note: DIVERGENCE_3 },
245+
},
246+
create: () => new MetadataFacade(new SchemaRegistry({ multiTenant: false })),
247+
},
248+
];
249+
250+
/** The document a case's final write carried for the key being read. */
251+
function lastWrittenDocument(testCase: MetadataRoundTripCase): unknown {
252+
return testCase.writes[testCase.writes.length - 1]?.data;
253+
}
254+
255+
/**
256+
* The answer this subject is held to for this case: the table's reference
257+
* answer, unless the subject declares a divergence for it.
258+
*/
259+
function expectationFor(
260+
implementation: PinnedImplementation,
261+
testCase: MetadataRoundTripCase,
262+
): { kind: 'readable'; document: unknown } | { kind: 'absent' } {
263+
const divergence = implementation.divergences?.[testCase.id];
264+
if (!divergence) return testCase.expected;
265+
return divergence.kind === 'absent'
266+
? { kind: 'absent' }
267+
: { kind: 'readable', document: lastWrittenDocument(testCase) };
268+
}
269+
270+
describe.each(IMPLEMENTATIONS)(
271+
'IMetadataService round-trip conformance [$label]',
272+
(implementation) => {
273+
it.each(METADATA_ROUNDTRIP_CASES.map((testCase) => [testCase.id, testCase] as const))(
274+
'%s',
275+
async (_id, testCase) => {
276+
const service = implementation.create();
277+
278+
for (const write of testCase.writes) {
279+
await service.register(write.type, write.name, write.data);
280+
}
281+
for (const removal of testCase.removes ?? []) {
282+
await service.unregister(removal.type, removal.name);
283+
}
284+
285+
const got = await service.get(testCase.read.type, testCase.read.name);
286+
const exists = await service.exists(testCase.read.type, testCase.read.name);
287+
const names = await service.listNames(testCase.read.type);
288+
const expected = expectationFor(implementation, testCase);
289+
290+
if (expected.kind === 'readable') {
291+
// Anti-vacuity: `toMatchObject` against an absent document
292+
// would fail on its own, but stating this first makes a
293+
// regression read as "nothing came back" rather than as a
294+
// shape mismatch buried in a diff.
295+
expect(got).toBeDefined();
296+
297+
if (implementation.documentFidelity === 'verbatim' || typeof expected.document !== 'object' || expected.document === null) {
298+
expect(got).toEqual(expected.document);
299+
} else {
300+
// The runtime-effective document CONTAINS the authored one.
301+
expect(got).toMatchObject(expected.document as Record<string, unknown>);
302+
}
303+
304+
expect(exists).toBe(true);
305+
// Exactly once: an implementation that appended instead of
306+
// overwriting would satisfy every assertion above on the
307+
// re-register rows and fail only this one.
308+
expect(names.filter((name) => name === testCase.read.name)).toHaveLength(1);
309+
} else {
310+
expect(got).toBeUndefined();
311+
expect(exists).toBe(false);
312+
expect(names).not.toContain(testCase.read.name);
313+
}
314+
},
315+
);
316+
},
317+
);
318+
319+
describe('round-trip conformance table wiring', () => {
320+
it('declares no divergence for a case id the table does not contain', () => {
321+
// A renamed case would otherwise turn its divergence override into a
322+
// dead entry, and the subject would quietly be held to the reference
323+
// answer it is known to fail.
324+
const ids = new Set(METADATA_ROUNDTRIP_CASES.map((testCase) => testCase.id));
325+
for (const implementation of IMPLEMENTATIONS) {
326+
for (const id of Object.keys(implementation.divergences ?? {})) {
327+
expect(ids, `${implementation.label}${id}`).toContain(id);
328+
}
329+
}
330+
});
331+
332+
it('holds at least one implementation to every case', () => {
333+
// Guards the opposite failure from the one above: a case that every
334+
// subject declared a divergence for would be pinned by nobody against
335+
// the reference answer.
336+
for (const testCase of METADATA_ROUNDTRIP_CASES) {
337+
const conforming = IMPLEMENTATIONS.filter((i) => !i.divergences?.[testCase.id]);
338+
expect(conforming.length, testCase.id).toBeGreaterThan(0);
339+
}
340+
});
341+
});

packages/spec/api-surface/contracts.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,11 +179,16 @@
179179
"LockAcquireOptions (interface)",
180180
"LockHandle (interface)",
181181
"Logger (interface)",
182+
"METADATA_ROUNDTRIP_CASES (const)",
182183
"MarkReadResult (interface)",
183184
"MessageObservability (interface)",
184185
"MetadataExportOptions (interface)",
185186
"MetadataImportOptions (interface)",
186187
"MetadataImportResult (interface)",
188+
"MetadataRoundTripCase (interface)",
189+
"MetadataRoundTripExpectation (type)",
190+
"MetadataRoundTripRemoval (interface)",
191+
"MetadataRoundTripWrite (interface)",
187192
"MetadataTypeInfo (interface)",
188193
"MetadataWatchCallback (type)",
189194
"MetadataWatchHandle (interface)",

0 commit comments

Comments
 (0)