Skip to content

docs(rest): the spec REFUSES the legacy lookup spellings, it does not fold them#13264

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
os-trump wants to merge 1 commit into from
+140 −21
Open

docs(rest): the spec REFUSES the legacy lookup spellings, it does not fold them #13264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
108 changes: 97 additions & 11 deletions packages/rest/src/public-form-lookup-picker.test.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -31,6 +31,9 @@ import { describe, expect, it, vi } from 'vitest';
// its suite green. Both predicates live in metadata-core.
import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFindOnePredicate, type EngineFindOneQueryInput } from '@objectstack/metadata-core';
import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol';
// [#13137] The instrument for the three-level control at the bottom of this
// file — the REAL `FieldSchema`, not a restatement of it.
import { FieldSchema } from '@objectstack/spec/data';
import { RestServer } from './rest-server.js';

// ─── the real save path (the seam that refused the picker before #7467) ─────
Expand DownExpand Up@@ -140,7 +143,9 @@ function mockRes() {
/**
* Mount the real routes over a protocol that serves the STORED view body.
* `objectDef` defaults to the canonical `leadObject`; the #7486 suite passes
* variants to cover the legacy pre-fold spellings of the same field.
* variants to cover the legacy spellings the route's fallback chain still
* reads — spellings `FieldSchema` REFUSES (#13137), so they can only ever have
* been STORED, never authored through the spec.
*/
function routesOver(storedView: any, foundRows: any[], objectDef: any = leadObject) {
const findData = vi.fn().mockResolvedValue({ data: foundRows });
Expand DownExpand Up@@ -321,12 +326,17 @@ describe('#7485 publicPicker.sort is retired — not declarable, and not read',

/**
* The defect this suite pins: the route's fallback chain read only the LEGACY
* field-def spellings (`referenceTo` / `target` / `options.objectName`), while
* `packages/spec/src/data/field.zod.ts` folds every one of those onto the
* canonical `reference` at parse. A parsed object schema therefore carried
* NONE of the keys the route read — the chain resolved `undefined` and a
* well-formed form got `500 LOOKUP_TARGET_MISSING`, making `publicPicker.object`
* de-facto REQUIRED while the schema and docs present it as optional.
* field-def spellings (`referenceTo` / `target` / `options.objectName`), not
* one of which `packages/spec/src/data/field.zod.ts` ACCEPTS. ⛔ [#13137] It
* does not fold them onto `reference` either — an earlier version of this
* paragraph claimed it did. `FieldSchema` is a `strictObject`, so
* `referenceTo` / `target` are REFUSED by name; the `aliases` table only adds
* *"Did you mean …"* to that refusal, because `strictObject` consults it solely
* from the `unrecognized_keys` path. Pinned three ways at the bottom of this
* file. A parsed object schema therefore carried NONE of the keys the route
* read — the chain resolved `undefined` and a well-formed form got
* `500 LOOKUP_TARGET_MISSING`, making `publicPicker.object` de-facto REQUIRED
* while the schema and docs present it as optional.
*
* Every case below omits `object` deliberately: that is the axis under test.
* The suite above covers the override branch and must stay that way — between
Expand DownExpand Up@@ -356,16 +366,27 @@ describe('#7486 the picker target resolves from the field definition when `objec
expect(findData.mock.calls[0][0].object).toBe('sys_user');
});

// Stored pre-fold rows never went through the alias table, so the legacy
// spellings are live data, not history. The fix EXTENDS the chain; one that
// replaced it would turn these three green cases into 500s.
// ⛔ [#13137] These are spellings `FieldSchema` REFUSES. There is no fold,
// so there is no "pre-fold row" for one of these to be — an earlier version
// of this comment said there was, and #12920 cites that sentence as its
// strongest surviving evidence that stored legacy rows exist. It carries
// ZERO observational content about stored data, and neither do these cases.
//
// What is true and what these pin: a def spelling the target this way never
// came through `FieldSchema`, and the serving read path replays ADR-0087
// conversions (`applyConversionsToStoredItem`) without any schema
// validation, so such a def would reach the route verbatim. These cases pin
// what the route DOES with one. ⚠️ Whether any exists is #12920's open
// production census — ⛔ assert nothing here in either direction.
// The fix EXTENDS the chain; one that replaced it would turn these three
// green cases into 500s.
const LEGACY_DEFS: Array<[string, Record<string, unknown>]> = [
['referenceTo', { type: 'lookup', referenceTo: 'sys_user' }],
['target', { type: 'lookup', target: 'sys_user' }],
['options.objectName', { type: 'lookup', options: { objectName: 'sys_user' } }],
];
for (const [spelling, ownerDef] of LEGACY_DEFS) {
it(`a stored PRE-FOLD row spelling the target \`${spelling}\` still resolves`, async () => {
it(`a stored row spelling the target the LEGACY way (\`${spelling}\`) still resolves`, async () => {
const stored = await savedWithoutObject();
const legacyObject = { ...leadObject, fields: { ...leadObject.fields, owner: ownerDef } };
const { findData, lookup } = routesOver(stored, [], legacyObject);
Expand DownExpand Up@@ -409,3 +430,68 @@ describe('#7486 the picker target resolves from the field definition when `objec
expect(findData).not.toHaveBeenCalled();
});
});

// ─── [#13137] the spec's alias table REFUSES, it does not fold ─────────────

/**
* The three-level control, pinned so the prose above cannot rot back.
*
* Two levels cannot carry this. "`FieldSchema` rejects `referenceTo`" reads
* equally well as *not measured*, and a blanket "everything is rejected" says
* nothing about the alias table at all. Three levels separate the claims:
* ACCEPT (the canonical key) · REFUSE **with** a rename hint (an alias-table
* entry) · REFUSE **without** one (a key on no list). Only the third makes the
* second mean something.
*
* The mechanism, if this ever reads surprising: `strictObject`'s `aliases`
* table is consulted ONLY from the `unrecognized_keys` path
* (`packages/spec/src/shared/strict-object.ts`), so an entry can only ever
* decorate a REFUSAL. ⛔ It is not a normaliser, on this surface or any other —
* three separate seats read the name `aliases` as "fold" on two different
* schemas in one day, which is what this file's prose used to say too.
*
* ⛔ SCOPE: this pins the SPEC's behaviour and nothing else. It asserts
* nothing about whether any stored row spells a target the legacy way — that
* is #12920's open production census, unanswerable from here, and ⛔ no
* assertion below may be cited as evidence in either direction.
*/
describe('#13137 `FieldSchema` REFUSES the legacy target spellings, it does not fold them', () => {
/** `type` is the only required key on `FieldSchema`; everything else is the axis under test. */
const parseField = (extra: Record<string, unknown>) => FieldSchema.safeParse({ type: 'lookup', ...extra });
const messagesOf = (result: { success: boolean; error?: { issues: Array<{ code: string; message: string }> } }) =>
(result.error?.issues ?? []).map((i) => i.message).join('\n');
const codesOf = (result: { success: boolean; error?: { issues: Array<{ code: string }> } }) =>
(result.error?.issues ?? []).map((i) => i.code);

it('LEVEL 1 — POSITIVE CONTROL: the canonical `reference` is ACCEPTED', () => {
// Without this the two refusals below are indistinguishable from a
// schema that refuses its own canonical key, i.e. a broken instrument.
const result = parseField({ reference: 'sys_user' });
expect(result.success).toBe(true);
expect(result.success && result.data.reference).toBe('sys_user');
});

it('LEVEL 2 — NEGATIVE CONTROL: a key on no list is refused `unrecognized_keys` with NO rename hint', () => {
const result = parseField({ zzz_not_a_key: 'sys_user' });
expect(result.success).toBe(false);
expect(codesOf(result)).toContain('unrecognized_keys');
expect(messagesOf(result)).toContain('`zzz_not_a_key`');
expect(messagesOf(result)).not.toContain('Did you mean');
});

// LEVEL 3 — all five spellings the alias table files under `reference`.
// ⛔ `success` is `false` for every one of them: that is the whole card.
for (const alias of ['relatedTo', 'referenceTo', 'target', 'targetObject', 'lookupObject']) {
it(`LEVEL 3 — \`${alias}\` is REFUSED with a rename hint onto \`reference\` — NOT folded onto it`, () => {
const result = parseField({ [alias]: 'sys_user' });
// Refused, not rewritten. If this ever flips to `true`, the alias
// table has become a normaliser and every "does not fold" sentence
// in this file and in `rest-server.ts` needs rewriting again.
expect(result.success).toBe(false);
expect(codesOf(result)).toContain('unrecognized_keys');
// The hint is what separates level 3 from level 2 — it names the
// canonical key the author should have written instead.
expect(messagesOf(result)).toContain(`Did you mean \`${alias}\` → \`reference\`?`);
});
}
});
53 changes: 43 additions & 10 deletions packages/rest/src/rest-server.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -8875,17 +8875,50 @@ export class RestServer {
const obj = items.find((o: any) => o?.name === match.object);
const def = obj?.fields?.[fieldName];
// [#7486] `reference` FIRST — it is the canonical
// key on `FieldSchema`, and `data/field.zod.ts`
// folds `relatedTo` / `referenceTo` / `target` /
// `targetObject` / `lookupObject` all onto it at
// parse. Reading only the legacy spellings meant a
// well-formed object schema carried NONE of them,
// the chain resolved `undefined`, and the route
// answered 500 — making `publicPicker.object`
// key on `FieldSchema`, and the ONLY spelling that
// schema accepts. Reading only the legacy spellings
// meant a well-formed object schema carried NONE of
// them, the chain resolved `undefined`, and the
// route answered 500 — making `publicPicker.object`
// de-facto required while the schema and docs
// present it as an optional override. The legacy
// spellings stay after it for stored pre-fold rows,
// which never went through the alias table.
// present it as an optional override.
//
// ⛔ [#13137] `data/field.zod.ts` does NOT fold the
// legacy spellings onto `reference`. An earlier
// version of this comment said it did, and that
// sentence is precisely what invited consumers to
// be lenient. Its `aliases` table is a RENAME HINT
// ON A REJECTED KEY, not a normaliser:
// `strictObject` consults `aliases` only from the
// `unrecognized_keys` path (the semantics are
// stated in `spec/src/shared/strict-object.ts`), so
// `relatedTo` / `referenceTo` / `target` /
// `targetObject` / `lookupObject` are REFUSED by
// `FieldSchema` — answered with *"Did you mean
// `referenceTo` → `reference`?"* and never
// rewritten. Pinned three ways (accept /
// alias-refusal-with-hint / unknown-key-refusal
// -without-hint) in
// `public-form-lookup-picker.test.ts`.
// ⇒ ⛔ this chain is NOT licence to be lenient
// anywhere else: nothing upstream folds for you,
// and a producer emitting a legacy spelling emits a
// document the spec refuses by name.
//
// The tail below reads exactly three spellings —
// `referenceTo`, `target`, `options.objectName` —
// which is NOT the spec's five-entry hint list:
// only the first two appear on it, and
// `options.objectName` appears on no list at all.
// They can reach here only on a STORED row that
// never went through `FieldSchema`, which is
// possible because the serving read path replays
// ADR-0087 conversions
// (`applyConversionsToStoredItem`) and performs no
// schema validation. ⚠️ Whether such a row is still
// reachable in production is #12920's OPEN census —
// ⛔ do not widen this chain here, and do not narrow
// it here either; #12920 decides its fate.
referenceTo = def?.reference
?? def?.referenceTo
?? def?.target
Expand Down
Loading