Skip to content

Commit 53cad07

Browse files
committed
fix(metadata-protocol): label diffMetaItem's default to side with the active row's own version
With no toVersion, diffMetaItem compared the active sys_metadata row's body but labelled it with the newest sys_metadata_history row, which is a draft save whenever a draft is pending. One read of the active row now supplies both the body and its version; with no active row both labels are null. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N8TPEsoJxPsdSdNKGnNGEN
1 parent dbddf02 commit 53cad07

4 files changed

Lines changed: 451 additions & 12 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@objectstack/metadata-protocol': patch
3+
---
4+
5+
fix(metadata-protocol): `diffMetaItem`'s default range labels its to side with the active row's own version, so `GET /meta/:type/:name/diff` with no `from` / `to` names the versions it compares while a draft is pending (#20397)
6+
7+
With no `toVersion`, the to side is the current active `sys_metadata` row. Its body was compared, but `toVersion` came from the newest `sys_metadata_history` row, which is a draft save whenever a draft is pending: every draft save appends a history row. The labels and the bodies then named different rows. Measured on the real REST stack, an app with one active save and two draft saves answered `fromVersion 2 → toVersion 3` over its version-1 body, and a view with one active save and one draft save answered "no changes" labelled `1 → 2` while version 2 differs.
8+
9+
- **Now:** `toVersion` is the active row's own `version`, read in the same read as its body. The default `fromVersion` is still the history version immediately before that label. An item whose active row is version 2 with a draft pending answers `1 → 2`, the same answer as `?from=1&to=2`.
10+
- **No active row** (a draft-only item, or a deleted one): the to side is absent, and both labels are `null` with empty buckets, as the response schema declares for an absent side. Before, a draft-only item was labelled with its newest draft save, and its from side could be an earlier draft save's body. A deleted item was labelled `N-1 → N` up to its tombstone. That deletion is still read by naming its versions (`?from=N-1&to=N`).
11+
- Unchanged: the response shape, explicit `from` / `to` ranges, and the default range of an item with no draft pending.

‎packages/metadata-protocol/src/protocol.diff-dead-history-read.test.ts‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,3 +282,157 @@ describe('#8798 — a history-table outage now answers the same way for every ty
282282
expect(err.status).toBe(503);
283283
});
284284
});
285+
286+
/**
287+
* [#20397] The DEFAULT range (no `toVersion`) labels its to side with the
288+
* version whose body it compares.
289+
*
290+
* The to-side body is the active `sys_metadata` row; the label used to be the
291+
* newest `sys_metadata_history` row's version, which is a draft save whenever a
292+
* draft is pending (every draft save appends a history row). The label is now
293+
* the active row's own `version` column, the one `SysMetadataRepository.put`
294+
* stamps with the version of the history row it appends in the same
295+
* transaction. Pinned here over seeded rows, beside this file's double, which
296+
* already honours the `where` on both tables; the same readings through the
297+
* real REST stack are `packages/rest/src/meta-diff-default-range-labels.test.ts`.
298+
*/
299+
describe('#20397 — the default range labels the to side with the active row\'s own version', () => {
300+
type Seed = { version: number; op: string; body: Record<string, unknown> | null };
301+
/** History rows in version order, plus the active and draft rows they left behind. */
302+
function seedLineage(
303+
tables: Record<string, Array<Record<string, unknown>>>,
304+
type: string,
305+
name: string,
306+
history: Seed[],
307+
rows: { active?: Seed; draft?: Seed },
308+
) {
309+
const base = { organization_id: null, type, name };
310+
history.forEach((h, i) => {
311+
tables.sys_metadata_history!.push({
312+
...base,
313+
id: `h_${h.version}`,
314+
version: h.version,
315+
event_seq: i + 1,
316+
operation_type: h.op,
317+
metadata: h.body == null ? null : JSON.stringify(h.body),
318+
checksum: h.body == null ? null : hashSpec(h.body),
319+
recorded_at: new Date(i + 1).toISOString(),
320+
});
321+
});
322+
for (const state of ['active', 'draft'] as const) {
323+
const row = rows[state];
324+
if (!row) continue;
325+
tables.sys_metadata!.push({
326+
...base,
327+
id: `m_${state}`,
328+
state,
329+
version: row.version,
330+
metadata: JSON.stringify(row.body),
331+
checksum: hashSpec(row.body!),
332+
});
333+
}
334+
}
335+
336+
for (const type of [ORDINARY_TYPE, 'app']) {
337+
it(`${type}: with a draft pending, toVersion is the active row's version and the answer is the explicit range's`, async () => {
338+
const { engine, tables } = makeStubEngine();
339+
const one = { name: 'item', label: 'One' };
340+
const two = { name: 'item', label: 'Two' };
341+
const pending = { name: 'item', label: 'Pending draft' };
342+
seedLineage(tables, type, 'item', [
343+
{ version: 1, op: 'create', body: one },
344+
{ version: 2, op: 'update', body: two },
345+
{ version: 3, op: 'create', body: pending },
346+
], { active: { version: 2, op: 'update', body: two }, draft: { version: 3, op: 'create', body: pending } });
347+
const protocol = new ObjectStackProtocolImplementation(engine);
348+
349+
const res: any = await protocol.diffMetaItem({ type, name: 'item' });
350+
const explicit: any = await protocol.diffMetaItem({ type, name: 'item', fromVersion: 1, toVersion: 2 });
351+
352+
expect(res).toEqual({
353+
type,
354+
name: 'item',
355+
fromVersion: 1,
356+
toVersion: 2,
357+
added: [],
358+
removed: [],
359+
changed: [{ path: 'label', from: 'One', to: 'Two' }],
360+
});
361+
expect(res).toEqual(explicit);
362+
});
363+
}
364+
365+
it('the card\'s app reading: one active save and two draft saves label the to side 1 (was 3), with no from side before it', async () => {
366+
const { engine, tables } = makeStubEngine();
367+
const v1 = { name: 'atlas', label: 'Atlas v1' };
368+
const d2 = { name: 'atlas', label: 'Atlas v2 draft' };
369+
const d3 = { name: 'atlas', label: 'Atlas v3 draft' };
370+
seedLineage(tables, 'app', 'atlas', [
371+
{ version: 1, op: 'create', body: v1 },
372+
{ version: 2, op: 'create', body: d2 },
373+
{ version: 3, op: 'update', body: d3 },
374+
], { active: { version: 1, op: 'create', body: v1 }, draft: { version: 3, op: 'update', body: d3 } });
375+
const protocol = new ObjectStackProtocolImplementation(engine);
376+
377+
const res: any = await protocol.diffMetaItem({ type: 'app', name: 'atlas' });
378+
379+
expect(res).toEqual({
380+
type: 'app',
381+
name: 'atlas',
382+
fromVersion: null,
383+
toVersion: 1,
384+
added: [{ path: 'name', value: 'atlas' }, { path: 'label', value: 'Atlas v1' }],
385+
removed: [],
386+
changed: [],
387+
});
388+
});
389+
390+
it('no active row (draft saves only): the to side is absent, so its label is null and no draft body is either side', async () => {
391+
const { engine, tables } = makeStubEngine();
392+
const d1 = { name: 'item', label: 'Only draft one' };
393+
const d2 = { name: 'item', label: 'Only draft two' };
394+
seedLineage(tables, ORDINARY_TYPE, 'item', [
395+
{ version: 1, op: 'create', body: d1 },
396+
{ version: 2, op: 'update', body: d2 },
397+
], { draft: { version: 2, op: 'update', body: d2 } });
398+
const protocol = new ObjectStackProtocolImplementation(engine);
399+
400+
const res: any = await protocol.diffMetaItem({ type: ORDINARY_TYPE, name: 'item' });
401+
402+
expect(res).toEqual({
403+
type: ORDINARY_TYPE, name: 'item', fromVersion: null, toVersion: null, added: [], removed: [], changed: [],
404+
});
405+
});
406+
407+
it('no active row (deleted): the default range answers null on both sides; the deletion stays readable by naming its versions', async () => {
408+
// The same rule as the draft-only case: the to side is absent, so is
409+
// its label. Before #20397 this one case happened to agree (the newest
410+
// history row was the tombstone, whose body is also absent); the
411+
// deletion itself is still one explicit range away.
412+
const { engine, tables } = makeStubEngine();
413+
const one = { name: 'item', label: 'One' };
414+
const two = { name: 'item', label: 'Two' };
415+
seedLineage(tables, ORDINARY_TYPE, 'item', [
416+
{ version: 1, op: 'create', body: one },
417+
{ version: 2, op: 'update', body: two },
418+
{ version: 3, op: 'delete', body: null },
419+
], {});
420+
const protocol = new ObjectStackProtocolImplementation(engine);
421+
422+
const res: any = await protocol.diffMetaItem({ type: ORDINARY_TYPE, name: 'item' });
423+
const deletion: any = await protocol.diffMetaItem({ type: ORDINARY_TYPE, name: 'item', fromVersion: 2, toVersion: 3 });
424+
425+
expect(res).toEqual({
426+
type: ORDINARY_TYPE, name: 'item', fromVersion: null, toVersion: null, added: [], removed: [], changed: [],
427+
});
428+
expect(deletion).toEqual({
429+
type: ORDINARY_TYPE,
430+
name: 'item',
431+
fromVersion: 2,
432+
toVersion: 3,
433+
added: [],
434+
removed: [{ path: 'name', value: 'item' }, { path: 'label', value: 'Two' }],
435+
changed: [],
436+
});
437+
});
438+
});

‎packages/metadata-protocol/src/protocol.ts‎

Lines changed: 38 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21286,9 +21286,10 @@ export class ObjectStackProtocolImplementation implements
2128621286
/**
2128721287
* Compute a shallow structural diff between two historical
2128821288
* versions of a metadata item. Either side may be omitted: when
21289-
* `toVersion` is undefined the current active body is used; when
21290-
* `fromVersion` is undefined the immediately previous history row
21291-
* is used. Returns `{ added, removed, changed }` keyed by JSON
21289+
* `toVersion` is undefined the current active body is used, labelled
21290+
* with that active row's own `version` (`null` when there is no active
21291+
* row); when `fromVersion` is undefined the immediately previous history
21292+
* row is used. Returns `{ added, removed, changed }` keyed by JSON
2129221293
* pointer-style paths for primitive leaves; nested objects/arrays
2129321294
* are reported as a single change record.
2129421295
*
@@ -21391,12 +21392,6 @@ export class ObjectStackProtocolImplementation implements
2139121392
// `catch` below is this function's only stated intent for that failure,
2139221393
// so removing the call makes every type take it. Pinned in
2139321394
// `protocol.diff-dead-history-read.test.ts`.
21394-
const repo = this.getOverlayRepo(orgId);
21395-
const fullRef = {
21396-
type: singularType,
21397-
name: request.name,
21398-
org: orgId ?? 'env',
21399-
} as { type: string; name: string; org: string };
2140021395
const histRows: Array<{ version: number; body: Record<string, unknown> | null }> = [];
2140121396
try {
2140221397
const engineAny = this.engine as any;
@@ -21467,9 +21462,40 @@ export class ObjectStackProtocolImplementation implements
2146721462
toVersion = request.toVersion;
2146821463
toBody = byVersion.get(request.toVersion) ?? null;
2146921464
} else {
21470-
const current = await repo.get(fullRef as any, { state: 'active' });
21471-
toBody = current ? (current.body as Record<string, unknown>) : null;
21472-
toVersion = histRows.length ? histRows[histRows.length - 1]!.version : null;
21465+
// [#20397] The default `to` side is the CURRENT ACTIVE ROW, and ONE
21466+
// read of that row supplies both of its facts: the body compared and
21467+
// the `version` it is labelled with. `SysMetadataRepository.put`
21468+
// stamps that column in the same transaction that appends the history
21469+
// row carrying the same body, so it names the version this body is.
21470+
//
21471+
// The label used to come from the NEWEST `sys_metadata_history` row
21472+
// instead, which is a draft save whenever a draft is pending (every
21473+
// draft save appends a row too). Body and label then named different
21474+
// rows: on the real REST stack an app answered `2 → 3` over its
21475+
// version-1 body, and a view answered "no changes" labelled `1 → 2`
21476+
// while version 2 differs.
21477+
//
21478+
// Read here, not through `SysMetadataRepository.get`: its
21479+
// `MetadataItem` projection carries the row's content hash but not
21480+
// its lineage `version`. Same predicate as that read (active state, no
21481+
// package scope), and VERBATIM like the history bodies it is compared
21482+
// against: no ADR-0087 conversion on either side.
21483+
//
21484+
// No active row (a draft-only item, a deleted one) ⇒ that side is
21485+
// absent and so is its label: `null`, as `DiffMetaItemResponseSchema`
21486+
// declares, never the number of a row whose body is not the one
21487+
// compared. ⛔ Do not recover a number by matching bodies or hashes
21488+
// against history: a publish and a revert both write rows whose
21489+
// bodies repeat earlier ones.
21490+
const current = (await this.engine.findOne('sys_metadata', {
21491+
where: { organization_id: orgId, type: singularType, name: request.name, state: 'active' },
21492+
})) as { metadata?: unknown; version?: unknown } | null;
21493+
toBody = current?.metadata == null
21494+
? null
21495+
: (typeof current.metadata === 'string'
21496+
? JSON.parse(current.metadata)
21497+
: current.metadata as Record<string, unknown>);
21498+
toVersion = current && typeof current.version === 'number' ? current.version : null;
2147321499
}
2147421500
if (request.fromVersion !== undefined) {
2147521501
fromVersion = request.fromVersion;

0 commit comments

Comments
 (0)