Skip to content

Commit 8d5bb5a

Browse files
os-zhuangclaude
andauthored
feat(metadata): saveMeta persists the operator spellings the spec normalized (objectui#2945) (#4107)
ViewFilterRuleSchema.operator is z.preprocess(normalizeFilterOperator, …), so a stored `notEquals`/`gt`/`isNull` is folded to canonical during save-time validation — and the result was then discarded, because saveMetaItem persists the authored body verbatim (deliberately: parsed.data strips the Studio-only aux fields that ride along with an overlay, ADR-0005 §Validation). So every save minted new legacy-alias rows. VIEW_FILTER_OPERATOR_ALIASES documents itself as "a migration bridge [that] may be dropped in a future major", but with new alias rows arriving continuously there is no point at which the last one is behind you — a migration rewriting existing rows would be obsolete at the next personalization PUT. This is prerequisite 2 of the consolidation blocked in objectui#2945. graftNormalizedOperators walks the authored body and parsed.data in lockstep BY STRUCTURE and copies exactly one thing: an `operator` whose parsed value differs. No path list — ViewFilterRule[] has five declared sites today and the walk covers all of them plus any added later; enumerating them would reproduce here the duplication #2945 exists to remove. Guarded on both sides being strings, so a `$`-token FilterCondition cannot be reshaped. Nothing added, removed, reordered or defaulted: the unary {field, operator} form does not acquire a `value` even though the schema's own output would. Returns by identity when nothing changed. Behaviour change, stated plainly: a GET after a PUT now returns the canonical spelling, not the one the author sent. That is the spelling the spec defines and every renderer accepts (objectui#2974, objectui#2989 pinned all three objectui translation tables to the full vocabulary). Existing rows are untouched — this stops the bleeding, it is not the migration. 11 new tests, one driving EVERY alias the spec still folds through the real ViewMetadataSchema; package suite 110 tests / 18 files green. Refs objectstack-ai/objectui#2945, objectstack-ai/objectui#2901 Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent fcb6cfa commit 8d5bb5a

4 files changed

Lines changed: 269 additions & 1 deletion

File tree

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
---
2+
"@objectstack/metadata-protocol": minor
3+
---
4+
5+
feat(metadata): `saveMeta` persists the operator spellings the spec normalized (objectui#2945)
6+
7+
`ViewFilterRuleSchema.operator` is `z.preprocess(normalizeFilterOperator, …)`, so
8+
a stored `notEquals` / `gt` / `isNull` is folded to its canonical form during
9+
save-time validation — and then the result was thrown away. `saveMetaItem`
10+
persists the authored body verbatim, deliberately: `parsed.data` strips the
11+
Studio-only auxiliary fields (`isPinned`, `isDefault`, `sortOrder`) that ride
12+
along with an overlay document (ADR-0005 §Validation).
13+
14+
The consequence is that **every save mints new legacy-alias rows.** The ~30
15+
entries in `VIEW_FILTER_OPERATOR_ALIASES` are documented as *"a migration bridge
16+
[that] may be dropped in a future major"*, but there is no point at which the
17+
last alias row is behind you, so the bridge can never be dismantled — a
18+
migration that rewrote every existing row would be obsolete the moment the next
19+
console personalization PUT landed. That is prerequisite 2 of the vocabulary
20+
consolidation blocked in objectstack-ai/objectui#2945.
21+
22+
`graftNormalizedOperators` grafts the normalization back on without giving up the
23+
verbatim body. It walks the authored value and `parsed.data` in lockstep **by
24+
structure** and copies across exactly one thing: an `operator` whose parsed value
25+
differs from the authored one.
26+
27+
- **No key list to maintain.** `ViewFilterRule[]` appears at five declared sites
28+
today (view `filter`, `ViewTab.filter`, page `filterBy`, and two
29+
`component.zod.ts` block props) and the structural walk covers all of them,
30+
plus any added later. Enumerating paths would have reproduced in this file the
31+
exact duplication #2945 exists to remove.
32+
- **Nothing else moves.** Only an `operator` string is rewritten, and only where
33+
both sides are strings — so a `$`-token `FilterCondition`, a different operator
34+
vocabulary entirely, cannot be reshaped by accident. No key is added, removed,
35+
reordered or defaulted; the unary `{field, operator}` form does not acquire a
36+
`value` even though the schema's own output would give it one.
37+
- **Nothing is allocated when nothing changed**, so a body already written in
38+
canonical form is returned by identity.
39+
40+
Behaviour change worth stating plainly: a `GET` after a `PUT` now returns the
41+
canonical spelling rather than the one the author sent. That is the spelling the
42+
spec defines, every renderer accepts it (objectstack-ai/objectui#2974,
43+
objectstack-ai/objectui#2989 pinned all three of objectui's translation tables to
44+
the full vocabulary), and it is the point of the change. Existing rows are not
45+
touched — this stops the bleeding, it is not the migration.
46+
47+
Verified: 11 new tests, including one that drives **every** alias the spec still
48+
folds through the real `ViewMetadataSchema` and asserts the persisted body comes
49+
out canonical; full `@objectstack/metadata-protocol` suite 110 tests / 18 files
50+
green.

packages/metadata-protocol/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
22

3-
export { ObjectStackProtocolImplementation, ConcurrentUpdateError, normalizeViewMetadata } from './protocol.js';
3+
export { ObjectStackProtocolImplementation, ConcurrentUpdateError, normalizeViewMetadata, graftNormalizedOperators } from './protocol.js';
44
export { createMetadataProtocolPlugin, assembleMetadataProtocol } from './plugin.js';
55
export type { MetadataProtocolPluginOptions } from './plugin.js';
66
export type { UninstallCleanup, UninstallCleanupOutcome } from './protocol.js';
Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* `saveMeta` persists the operator spellings the spec normalized (objectui#2945).
5+
*
6+
* `ViewFilterRuleSchema.operator` is `z.preprocess(normalizeFilterOperator, …)`,
7+
* so validation folds `notEquals`/`gt`/`isNull` to canonical — and the result
8+
* used to be discarded, because the authored body is persisted verbatim. Every
9+
* save therefore minted fresh `VIEW_FILTER_OPERATOR_ALIASES` rows, and an alias
10+
* table that keeps growing can never be retired.
11+
*
12+
* `graftNormalizedOperators` copies the normalization back on and nothing else.
13+
* The tests below pin both halves of that claim: operators DO change, and
14+
* everything else — auxiliary fields, unknown keys, `$`-token conditions, array
15+
* order, absent optionals — does NOT.
16+
*
17+
* Parsing goes through `ViewMetadataSchema`, which is what
18+
* `getMetadataTypeSchema('view')` returns and therefore what `saveMetaItem`
19+
* actually validates against.
20+
*/
21+
import { describe, it, expect } from 'vitest';
22+
import { ViewMetadataSchema, VIEW_FILTER_OPERATORS, VIEW_FILTER_OPERATOR_ALIASES } from '@objectstack/spec/ui';
23+
import { graftNormalizedOperators } from './protocol.js';
24+
25+
/** Graft through the real spec schema, the way `saveMetaItem` does. */
26+
function graftThroughSchema(authored: unknown): unknown {
27+
const parsed = (ViewMetadataSchema as unknown as {
28+
safeParse: (v: unknown) => { success: boolean; data?: unknown; error?: unknown };
29+
}).safeParse(authored);
30+
expect(parsed.success, JSON.stringify(parsed.error)).toBe(true);
31+
return graftNormalizedOperators(authored, parsed.data);
32+
}
33+
34+
/** Flattened runtime view overlay — the shape a console personalization PUT sends. */
35+
const view = (filter: unknown, extra: Record<string, unknown> = {}) => ({
36+
name: 'showcase_task.open',
37+
object: 'showcase_task',
38+
viewKind: 'list' as const,
39+
label: 'Open',
40+
type: 'grid' as const,
41+
columns: ['name'],
42+
filter,
43+
...extra,
44+
});
45+
46+
describe('graftNormalizedOperators — through the real view metadata schema', () => {
47+
it('canonicalizes a legacy operator spelling on the persisted body', () => {
48+
const authored = view([{ field: 'status', operator: 'notEquals', value: 'done' }]);
49+
const out = graftThroughSchema(authored) as { filter: Array<{ operator: string }> };
50+
expect(out.filter[0].operator).toBe('not_equals');
51+
});
52+
53+
it('canonicalizes every alias the spec still folds', () => {
54+
const canonical = new Set<string>(VIEW_FILTER_OPERATORS);
55+
const stillLegacy: string[] = [];
56+
for (const alias of Object.keys(VIEW_FILTER_OPERATOR_ALIASES)) {
57+
const out = graftThroughSchema(
58+
view([{ field: 'status', operator: alias, value: 'x' }]),
59+
) as { filter: Array<{ operator: string }> };
60+
if (!canonical.has(out.filter[0].operator)) stillLegacy.push(alias);
61+
}
62+
expect(stillLegacy).toEqual([]);
63+
});
64+
65+
it('leaves a canonical operator identical — same object, nothing allocated', () => {
66+
const authored = view([{ field: 'status', operator: 'not_equals', value: 'done' }]);
67+
expect(graftThroughSchema(authored)).toBe(authored);
68+
});
69+
70+
it('normalizes a tab filter, not just the view filter', () => {
71+
const authored = view(
72+
[{ field: 'status', operator: 'eq', value: 'open' }],
73+
{ tabs: [{ name: 'mine', label: 'Mine', filter: [{ field: 'owner', operator: 'isNotNull' }] }] },
74+
);
75+
const out = graftThroughSchema(authored) as {
76+
filter: Array<{ operator: string }>;
77+
tabs: Array<{ filter: Array<{ operator: string }> }>;
78+
};
79+
expect(out.filter[0].operator).toBe('equals');
80+
expect(out.tabs[0].filter[0].operator).toBe('is_not_null');
81+
});
82+
});
83+
84+
describe('graftNormalizedOperators — what it must never touch', () => {
85+
it('keeps Studio-only auxiliary fields a `parsed.data` swap would strip', () => {
86+
// The exact reason saveMeta persists verbatim (ADR-0005 §Validation).
87+
const authored = view(
88+
[{ field: 'status', operator: 'gt', value: 1 }],
89+
{ isPinned: true, isDefault: false, sortOrder: 3 },
90+
);
91+
const out = graftThroughSchema(authored) as Record<string, unknown>;
92+
expect(out.isPinned).toBe(true);
93+
expect(out.isDefault).toBe(false);
94+
expect(out.sortOrder).toBe(3);
95+
expect((out.filter as Array<{ operator: string }>)[0].operator).toBe('greater_than');
96+
});
97+
98+
it('adds no defaults and no keys the author did not write', () => {
99+
const authored = view([{ field: 'status', operator: 'ne', value: 'done' }]);
100+
const out = graftThroughSchema(authored) as Record<string, unknown>;
101+
expect(Object.keys(out).sort()).toEqual(Object.keys(authored).sort());
102+
// The unary form carries no `value`; grafting must not materialize one,
103+
// even though the schema's own output would.
104+
const unary = view([{ field: 'owner', operator: 'isNull' }]);
105+
const unaryOut = graftThroughSchema(unary) as { filter: Array<Record<string, unknown>> };
106+
expect(Object.keys(unaryOut.filter[0]).sort()).toEqual(['field', 'operator']);
107+
});
108+
109+
it('preserves filter order', () => {
110+
const authored = view([
111+
{ field: 'a', operator: 'eq', value: 1 },
112+
{ field: 'b', operator: 'gt', value: 2 },
113+
{ field: 'c', operator: 'notIn', value: ['x'] },
114+
]);
115+
const out = graftThroughSchema(authored) as { filter: Array<{ field: string; operator: string }> };
116+
expect(out.filter.map(f => f.field)).toEqual(['a', 'b', 'c']);
117+
expect(out.filter.map(f => f.operator)).toEqual(['equals', 'greater_than', 'not_in']);
118+
});
119+
});
120+
121+
describe('graftNormalizedOperators — structural safety, no schema involved', () => {
122+
it('rewrites only an `operator` string, never a nested object', () => {
123+
// A `$`-token FilterCondition is a DIFFERENT operator vocabulary. Even if a
124+
// parsed tree somehow offered a string here, the object must survive.
125+
const authored = { criteria: { operator: { $eq: 'active' } } };
126+
const parsed = { criteria: { operator: 'equals' } };
127+
expect(graftNormalizedOperators(authored, parsed)).toBe(authored);
128+
});
129+
130+
it('ignores a parsed tree whose shape does not match', () => {
131+
const authored = { filter: [{ field: 'a', operator: 'eq' }] };
132+
expect(graftNormalizedOperators(authored, { filter: 'not-an-array' })).toBe(authored);
133+
expect(graftNormalizedOperators(authored, undefined)).toBe(authored);
134+
expect(graftNormalizedOperators(authored, null)).toBe(authored);
135+
});
136+
137+
it('does not read past the authored array — a longer parsed array adds nothing', () => {
138+
const authored = { filter: [{ field: 'a', operator: 'eq' }] };
139+
const out = graftNormalizedOperators(authored, {
140+
filter: [{ field: 'a', operator: 'equals' }, { field: 'b', operator: 'equals' }],
141+
}) as { filter: unknown[] };
142+
expect(out.filter).toHaveLength(1);
143+
});
144+
145+
it('passes primitives and empty structures through unchanged', () => {
146+
expect(graftNormalizedOperators('x', 'y')).toBe('x');
147+
expect(graftNormalizedOperators(7, 8)).toBe(7);
148+
expect(graftNormalizedOperators(null, { a: 1 })).toBe(null);
149+
const empty = {};
150+
expect(graftNormalizedOperators(empty, { a: 1 })).toBe(empty);
151+
});
152+
});

packages/metadata-protocol/src/protocol.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,9 @@ const HAND_CRAFTED_SCHEMAS: Record<string, Record<string, unknown>> = {
288288
* - We do NOT replace the persisted document with `parsed.data`; the
289289
* original payload is stored verbatim so Studio-only auxiliary fields
290290
* (e.g. `isPinned`, `isDefault`, `sortOrder`) survive the round-trip.
291+
* The one exception is filter `operator` spellings, which are grafted back
292+
* from `parsed.data` so a save stops minting new legacy-alias rows — see
293+
* {@link graftNormalizedOperators}.
291294
* - Types without a registered schema (the wiring-layer types
292295
* `function`/`service`/`router`, and any plugin types that have not
293296
* yet called `registerMetadataTypeSchema()`) fall through unvalidated.
@@ -334,6 +337,64 @@ export function normalizeViewMetadata(type: string, item: unknown, saveName: str
334337
return { ...it, ...(it.name ? undefined : { name: saveName }), ...patch };
335338
}
336339

340+
/**
341+
* Persist the operator spellings the spec's own schema normalized, and nothing
342+
* else. objectui#2945.
343+
*
344+
* `ViewFilterRuleSchema.operator` is `z.preprocess(normalizeFilterOperator, …)`,
345+
* so a stored `notEquals` / `gt` / `isNull` is folded to its canonical form
346+
* during validation — and then the result was thrown away, because `saveMeta`
347+
* persists the authored body verbatim (deliberately: `parsed.data` strips the
348+
* Studio-only auxiliary fields that ride along with an overlay). The alias table
349+
* `VIEW_FILTER_OPERATOR_ALIASES` therefore keeps acquiring *new* rows with every
350+
* save, which is why it can never be retired: there is no point at which the
351+
* last alias row is behind you.
352+
*
353+
* This grafts the normalization back on without giving up the verbatim body.
354+
* It walks the authored value and the parsed value in lockstep **by structure**
355+
* and copies across exactly one thing: an `operator` whose parsed value differs
356+
* from the authored one. No key list to maintain — every filter site the schema
357+
* knows about is covered, including ones added later — and nothing is added,
358+
* removed, reordered or defaulted, so an auxiliary field cannot be lost the way
359+
* a wholesale `parsed.data` swap would lose it.
360+
*
361+
* Returns the input itself when nothing changed, so the common case allocates
362+
* nothing.
363+
*/
364+
export function graftNormalizedOperators(authored: unknown, parsed: unknown): unknown {
365+
if (Array.isArray(authored)) {
366+
if (!Array.isArray(parsed)) return authored;
367+
let changed = false;
368+
const out = authored.map((entry, i) => {
369+
const next = graftNormalizedOperators(entry, parsed[i]);
370+
if (next !== entry) changed = true;
371+
return next;
372+
});
373+
return changed ? out : authored;
374+
}
375+
376+
if (!authored || typeof authored !== 'object') return authored;
377+
if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) return authored;
378+
379+
const a = authored as Record<string, unknown>;
380+
const p = parsed as Record<string, unknown>;
381+
let patch: Record<string, unknown> | undefined;
382+
383+
for (const [key, value] of Object.entries(a)) {
384+
// The one value this function is allowed to rewrite. Guarded on both
385+
// sides being strings so a `$`-token condition object — a different
386+
// operator vocabulary entirely — cannot be reshaped by accident.
387+
if (key === 'operator' && typeof value === 'string' && typeof p[key] === 'string') {
388+
if (p[key] !== value) (patch ??= {})[key] = p[key];
389+
continue;
390+
}
391+
const next = graftNormalizedOperators(value, p[key]);
392+
if (next !== value) (patch ??= {})[key] = next;
393+
}
394+
395+
return patch ? { ...a, ...patch } : authored;
396+
}
397+
337398
/**
338399
* #2555 — compute the identity fields (`viewKind`, `object`, `label`) a view
339400
* overlay is missing but the registry entry it shadows carries. The overlay's
@@ -4452,6 +4513,11 @@ export class ObjectStackProtocolImplementation implements
44524513
(err as any).issues = issues;
44534514
throw err;
44544515
}
4516+
// Keep the body verbatim, but not its *legacy operator
4517+
// spellings*: the schema just folded them to canonical and the
4518+
// result would otherwise be discarded, so every save minted new
4519+
// alias rows. See {@link graftNormalizedOperators}.
4520+
request.item = graftNormalizedOperators(request.item, parsed.data);
44554521
}
44564522
}
44574523

0 commit comments

Comments
 (0)