diff --git a/.changeset/savemeta-persists-normalized-operators.md b/.changeset/savemeta-persists-normalized-operators.md new file mode 100644 index 0000000000..6ede06d2d6 --- /dev/null +++ b/.changeset/savemeta-persists-normalized-operators.md @@ -0,0 +1,50 @@ +--- +"@objectstack/metadata-protocol": minor +--- + +feat(metadata): `saveMeta` persists the operator spellings the spec normalized (objectui#2945) + +`ViewFilterRuleSchema.operator` is `z.preprocess(normalizeFilterOperator, …)`, so +a stored `notEquals` / `gt` / `isNull` is folded to its canonical form during +save-time validation — and then the result was thrown away. `saveMetaItem` +persists the authored body verbatim, deliberately: `parsed.data` strips the +Studio-only auxiliary fields (`isPinned`, `isDefault`, `sortOrder`) that ride +along with an overlay document (ADR-0005 §Validation). + +The consequence is that **every save mints new legacy-alias rows.** The ~30 +entries in `VIEW_FILTER_OPERATOR_ALIASES` are documented as *"a migration bridge +[that] may be dropped in a future major"*, but there is no point at which the +last alias row is behind you, so the bridge can never be dismantled — a +migration that rewrote every existing row would be obsolete the moment the next +console personalization PUT landed. That is prerequisite 2 of the vocabulary +consolidation blocked in objectstack-ai/objectui#2945. + +`graftNormalizedOperators` grafts the normalization back on without giving up the +verbatim body. It walks the authored value and `parsed.data` in lockstep **by +structure** and copies across exactly one thing: an `operator` whose parsed value +differs from the authored one. + +- **No key list to maintain.** `ViewFilterRule[]` appears at five declared sites + today (view `filter`, `ViewTab.filter`, page `filterBy`, and two + `component.zod.ts` block props) and the structural walk covers all of them, + plus any added later. Enumerating paths would have reproduced in this file the + exact duplication #2945 exists to remove. +- **Nothing else moves.** Only an `operator` string is rewritten, and only where + both sides are strings — so a `$`-token `FilterCondition`, a different operator + vocabulary entirely, cannot be reshaped by accident. No key is added, removed, + reordered or defaulted; the unary `{field, operator}` form does not acquire a + `value` even though the schema's own output would give it one. +- **Nothing is allocated when nothing changed**, so a body already written in + canonical form is returned by identity. + +Behaviour change worth stating plainly: a `GET` after a `PUT` now returns the +canonical spelling rather than the one the author sent. That is the spelling the +spec defines, every renderer accepts it (objectstack-ai/objectui#2974, +objectstack-ai/objectui#2989 pinned all three of objectui's translation tables to +the full vocabulary), and it is the point of the change. Existing rows are not +touched — this stops the bleeding, it is not the migration. + +Verified: 11 new tests, including one that drives **every** alias the spec still +folds through the real `ViewMetadataSchema` and asserts the persisted body comes +out canonical; full `@objectstack/metadata-protocol` suite 110 tests / 18 files +green. diff --git a/packages/metadata-protocol/src/index.ts b/packages/metadata-protocol/src/index.ts index 4e13d3fdaa..4d4c1b5e4c 100644 --- a/packages/metadata-protocol/src/index.ts +++ b/packages/metadata-protocol/src/index.ts @@ -1,6 +1,6 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. -export { ObjectStackProtocolImplementation, ConcurrentUpdateError, normalizeViewMetadata } from './protocol.js'; +export { ObjectStackProtocolImplementation, ConcurrentUpdateError, normalizeViewMetadata, graftNormalizedOperators } from './protocol.js'; export { createMetadataProtocolPlugin, assembleMetadataProtocol } from './plugin.js'; export type { MetadataProtocolPluginOptions } from './plugin.js'; export type { UninstallCleanup, UninstallCleanupOutcome } from './protocol.js'; diff --git a/packages/metadata-protocol/src/protocol.graft-normalized-operators.test.ts b/packages/metadata-protocol/src/protocol.graft-normalized-operators.test.ts new file mode 100644 index 0000000000..bdcd7f8ca1 --- /dev/null +++ b/packages/metadata-protocol/src/protocol.graft-normalized-operators.test.ts @@ -0,0 +1,152 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `saveMeta` persists the operator spellings the spec normalized (objectui#2945). + * + * `ViewFilterRuleSchema.operator` is `z.preprocess(normalizeFilterOperator, …)`, + * so validation folds `notEquals`/`gt`/`isNull` to canonical — and the result + * used to be discarded, because the authored body is persisted verbatim. Every + * save therefore minted fresh `VIEW_FILTER_OPERATOR_ALIASES` rows, and an alias + * table that keeps growing can never be retired. + * + * `graftNormalizedOperators` copies the normalization back on and nothing else. + * The tests below pin both halves of that claim: operators DO change, and + * everything else — auxiliary fields, unknown keys, `$`-token conditions, array + * order, absent optionals — does NOT. + * + * Parsing goes through `ViewMetadataSchema`, which is what + * `getMetadataTypeSchema('view')` returns and therefore what `saveMetaItem` + * actually validates against. + */ +import { describe, it, expect } from 'vitest'; +import { ViewMetadataSchema, VIEW_FILTER_OPERATORS, VIEW_FILTER_OPERATOR_ALIASES } from '@objectstack/spec/ui'; +import { graftNormalizedOperators } from './protocol.js'; + +/** Graft through the real spec schema, the way `saveMetaItem` does. */ +function graftThroughSchema(authored: unknown): unknown { + const parsed = (ViewMetadataSchema as unknown as { + safeParse: (v: unknown) => { success: boolean; data?: unknown; error?: unknown }; + }).safeParse(authored); + expect(parsed.success, JSON.stringify(parsed.error)).toBe(true); + return graftNormalizedOperators(authored, parsed.data); +} + +/** Flattened runtime view overlay — the shape a console personalization PUT sends. */ +const view = (filter: unknown, extra: Record = {}) => ({ + name: 'showcase_task.open', + object: 'showcase_task', + viewKind: 'list' as const, + label: 'Open', + type: 'grid' as const, + columns: ['name'], + filter, + ...extra, +}); + +describe('graftNormalizedOperators — through the real view metadata schema', () => { + it('canonicalizes a legacy operator spelling on the persisted body', () => { + const authored = view([{ field: 'status', operator: 'notEquals', value: 'done' }]); + const out = graftThroughSchema(authored) as { filter: Array<{ operator: string }> }; + expect(out.filter[0].operator).toBe('not_equals'); + }); + + it('canonicalizes every alias the spec still folds', () => { + const canonical = new Set(VIEW_FILTER_OPERATORS); + const stillLegacy: string[] = []; + for (const alias of Object.keys(VIEW_FILTER_OPERATOR_ALIASES)) { + const out = graftThroughSchema( + view([{ field: 'status', operator: alias, value: 'x' }]), + ) as { filter: Array<{ operator: string }> }; + if (!canonical.has(out.filter[0].operator)) stillLegacy.push(alias); + } + expect(stillLegacy).toEqual([]); + }); + + it('leaves a canonical operator identical — same object, nothing allocated', () => { + const authored = view([{ field: 'status', operator: 'not_equals', value: 'done' }]); + expect(graftThroughSchema(authored)).toBe(authored); + }); + + it('normalizes a tab filter, not just the view filter', () => { + const authored = view( + [{ field: 'status', operator: 'eq', value: 'open' }], + { tabs: [{ name: 'mine', label: 'Mine', filter: [{ field: 'owner', operator: 'isNotNull' }] }] }, + ); + const out = graftThroughSchema(authored) as { + filter: Array<{ operator: string }>; + tabs: Array<{ filter: Array<{ operator: string }> }>; + }; + expect(out.filter[0].operator).toBe('equals'); + expect(out.tabs[0].filter[0].operator).toBe('is_not_null'); + }); +}); + +describe('graftNormalizedOperators — what it must never touch', () => { + it('keeps Studio-only auxiliary fields a `parsed.data` swap would strip', () => { + // The exact reason saveMeta persists verbatim (ADR-0005 §Validation). + const authored = view( + [{ field: 'status', operator: 'gt', value: 1 }], + { isPinned: true, isDefault: false, sortOrder: 3 }, + ); + const out = graftThroughSchema(authored) as Record; + expect(out.isPinned).toBe(true); + expect(out.isDefault).toBe(false); + expect(out.sortOrder).toBe(3); + expect((out.filter as Array<{ operator: string }>)[0].operator).toBe('greater_than'); + }); + + it('adds no defaults and no keys the author did not write', () => { + const authored = view([{ field: 'status', operator: 'ne', value: 'done' }]); + const out = graftThroughSchema(authored) as Record; + expect(Object.keys(out).sort()).toEqual(Object.keys(authored).sort()); + // The unary form carries no `value`; grafting must not materialize one, + // even though the schema's own output would. + const unary = view([{ field: 'owner', operator: 'isNull' }]); + const unaryOut = graftThroughSchema(unary) as { filter: Array> }; + expect(Object.keys(unaryOut.filter[0]).sort()).toEqual(['field', 'operator']); + }); + + it('preserves filter order', () => { + const authored = view([ + { field: 'a', operator: 'eq', value: 1 }, + { field: 'b', operator: 'gt', value: 2 }, + { field: 'c', operator: 'notIn', value: ['x'] }, + ]); + const out = graftThroughSchema(authored) as { filter: Array<{ field: string; operator: string }> }; + expect(out.filter.map(f => f.field)).toEqual(['a', 'b', 'c']); + expect(out.filter.map(f => f.operator)).toEqual(['equals', 'greater_than', 'not_in']); + }); +}); + +describe('graftNormalizedOperators — structural safety, no schema involved', () => { + it('rewrites only an `operator` string, never a nested object', () => { + // A `$`-token FilterCondition is a DIFFERENT operator vocabulary. Even if a + // parsed tree somehow offered a string here, the object must survive. + const authored = { criteria: { operator: { $eq: 'active' } } }; + const parsed = { criteria: { operator: 'equals' } }; + expect(graftNormalizedOperators(authored, parsed)).toBe(authored); + }); + + it('ignores a parsed tree whose shape does not match', () => { + const authored = { filter: [{ field: 'a', operator: 'eq' }] }; + expect(graftNormalizedOperators(authored, { filter: 'not-an-array' })).toBe(authored); + expect(graftNormalizedOperators(authored, undefined)).toBe(authored); + expect(graftNormalizedOperators(authored, null)).toBe(authored); + }); + + it('does not read past the authored array — a longer parsed array adds nothing', () => { + const authored = { filter: [{ field: 'a', operator: 'eq' }] }; + const out = graftNormalizedOperators(authored, { + filter: [{ field: 'a', operator: 'equals' }, { field: 'b', operator: 'equals' }], + }) as { filter: unknown[] }; + expect(out.filter).toHaveLength(1); + }); + + it('passes primitives and empty structures through unchanged', () => { + expect(graftNormalizedOperators('x', 'y')).toBe('x'); + expect(graftNormalizedOperators(7, 8)).toBe(7); + expect(graftNormalizedOperators(null, { a: 1 })).toBe(null); + const empty = {}; + expect(graftNormalizedOperators(empty, { a: 1 })).toBe(empty); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 6e7cdb7c50..501e6b40cb 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -288,6 +288,9 @@ const HAND_CRAFTED_SCHEMAS: Record> = { * - We do NOT replace the persisted document with `parsed.data`; the * original payload is stored verbatim so Studio-only auxiliary fields * (e.g. `isPinned`, `isDefault`, `sortOrder`) survive the round-trip. + * The one exception is filter `operator` spellings, which are grafted back + * from `parsed.data` so a save stops minting new legacy-alias rows — see + * {@link graftNormalizedOperators}. * - Types without a registered schema (the wiring-layer types * `function`/`service`/`router`, and any plugin types that have not * yet called `registerMetadataTypeSchema()`) fall through unvalidated. @@ -334,6 +337,64 @@ export function normalizeViewMetadata(type: string, item: unknown, saveName: str return { ...it, ...(it.name ? undefined : { name: saveName }), ...patch }; } +/** + * Persist the operator spellings the spec's own schema normalized, and nothing + * else. objectui#2945. + * + * `ViewFilterRuleSchema.operator` is `z.preprocess(normalizeFilterOperator, …)`, + * so a stored `notEquals` / `gt` / `isNull` is folded to its canonical form + * during validation — and then the result was thrown away, because `saveMeta` + * persists the authored body verbatim (deliberately: `parsed.data` strips the + * Studio-only auxiliary fields that ride along with an overlay). The alias table + * `VIEW_FILTER_OPERATOR_ALIASES` therefore keeps acquiring *new* rows with every + * save, which is why it can never be retired: there is no point at which the + * last alias row is behind you. + * + * This grafts the normalization back on without giving up the verbatim body. + * It walks the authored value and the parsed value in lockstep **by structure** + * and copies across exactly one thing: an `operator` whose parsed value differs + * from the authored one. No key list to maintain — every filter site the schema + * knows about is covered, including ones added later — and nothing is added, + * removed, reordered or defaulted, so an auxiliary field cannot be lost the way + * a wholesale `parsed.data` swap would lose it. + * + * Returns the input itself when nothing changed, so the common case allocates + * nothing. + */ +export function graftNormalizedOperators(authored: unknown, parsed: unknown): unknown { + if (Array.isArray(authored)) { + if (!Array.isArray(parsed)) return authored; + let changed = false; + const out = authored.map((entry, i) => { + const next = graftNormalizedOperators(entry, parsed[i]); + if (next !== entry) changed = true; + return next; + }); + return changed ? out : authored; + } + + if (!authored || typeof authored !== 'object') return authored; + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) return authored; + + const a = authored as Record; + const p = parsed as Record; + let patch: Record | undefined; + + for (const [key, value] of Object.entries(a)) { + // The one value this function is allowed to rewrite. Guarded on both + // sides being strings so a `$`-token condition object — a different + // operator vocabulary entirely — cannot be reshaped by accident. + if (key === 'operator' && typeof value === 'string' && typeof p[key] === 'string') { + if (p[key] !== value) (patch ??= {})[key] = p[key]; + continue; + } + const next = graftNormalizedOperators(value, p[key]); + if (next !== value) (patch ??= {})[key] = next; + } + + return patch ? { ...a, ...patch } : authored; +} + /** * #2555 — compute the identity fields (`viewKind`, `object`, `label`) a view * overlay is missing but the registry entry it shadows carries. The overlay's @@ -4452,6 +4513,11 @@ export class ObjectStackProtocolImplementation implements (err as any).issues = issues; throw err; } + // Keep the body verbatim, but not its *legacy operator + // spellings*: the schema just folded them to canonical and the + // result would otherwise be discarded, so every save minted new + // alias rows. See {@link graftNormalizedOperators}. + request.item = graftNormalizedOperators(request.item, parsed.data); } }