diff --git a/.changeset/21571-unprojected-read-declared-fields.md b/.changeset/21571-unprojected-read-declared-fields.md new file mode 100644 index 00000000000..25d289817a2 --- /dev/null +++ b/.changeset/21571-unprojected-read-declared-fields.md @@ -0,0 +1,37 @@ +--- +'@objectstack/objectql': minor +--- + +fix(objectql)!: a read with no projection serves the object's declared fields and the platform's system columns, never a column no metadata declares (#21571) + +**BREAKING (narrowing)** — what a released read door serves shrinks. A column that +no metadata declares, typically a field retired in an upgrade whose column additive +schema sync leaves in the table until `os migrate apply --allow-destructive`, is no +longer returned by any read through the engine. + +| read | before | now | +| --- | --- | --- | +| `POST /api/v1/data/:object/query` or `GET /api/v1/data/:object` with no `fields` | every column of the table, retired ones and their values included | the declared fields, the registry's system columns, `id`, `created_at`, `updated_at` | +| `GET /api/v1/data/:object/:id`, export, search hits, the RPC dispatcher, `expand`ed records | the same whole row | the same declared set | +| `engine.find` / `engine.findOne` in process (hooks, flows, plugins), no `fields` | the whole row | the declared set | +| an explicit `fields` naming a declared field whose column does not exist yet (driver-sql retries `select('*')`) | the whole row, retired columns included | the declared set | +| `POST /api/v1/data/:object/:id/clone` of a record whose table carries a retired column | refused `INVALID_FIELD` (the copy carried the retired column into the insert) | cloned | + +**Unchanged:** naming a retired column in `fields` still answers `400 INVALID_FIELD` +on the data door. Declared fields keep their treatment: `internal: true` omission, +credential masking, formula evaluation and the hidden `__search` strip run as +before, and the registry-injected tenant, owner and audit columns are still served. +No driver changed: the engine shapes the rows any driver returns, so the answer is +the same on every driver and every door. Writes, and the rows a write returns, are +not changed by this release. + +**If you still read a retired column's values** (for example a one-time conversion +that copies the old columns into their replacement field): run that conversion +BEFORE upgrading to this release, while the old field is still declared, or, once +it lands, read the unmapped columns through the operator-only `os migrate` read +(objectstack#21573). There is no flag that re-opens undeclared columns on a runtime +door. An in-process reader that needs a column must declare it as a field. + +Clause-②: no (narrowing) + + diff --git a/content/docs/data-modeling/queries.mdx b/content/docs/data-modeling/queries.mdx index d6af5d686ec..5f9003db36f 100644 --- a/content/docs/data-modeling/queries.mdx +++ b/content/docs/data-modeling/queries.mdx @@ -285,6 +285,13 @@ pattern the metadata-revision / flow-run / notification list endpoints already u } ``` +Without `fields`, a read returns the object's **declared** fields plus the platform's +system columns (`id`, `created_at`, `updated_at`, and the tenant, owner and audit columns +the registry adds). A column no metadata declares is never returned — for example one a +retired field left in the table until `os migrate apply --allow-destructive` drops it — and +naming it in `fields` on the data API is refused with `400 INVALID_FIELD`. To read such a column's values +for a one-time conversion, run the conversion before the field is retired. + ### Nested / Related Fields {/* os:check */} diff --git a/packages/objectql/src/declared-read-columns.ts b/packages/objectql/src/declared-read-columns.ts new file mode 100644 index 00000000000..6e1d7f92c49 --- /dev/null +++ b/packages/objectql/src/declared-read-columns.ts @@ -0,0 +1,131 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#21571] The columns a read may serve: the object's DECLARED fields plus the + * platform-provisioned columns — one set, answered from the registry's field + * map, and the same set an explicit projection is judged against. + * + * ## The defect this closes + * + * A read that names no `fields` reached the driver with no projection, and + * every driver answers that with the whole row (`SELECT *` on SQL). A column + * that no metadata declares — a field retired in an upgrade, whose column + * additive schema sync leaves in the table until an operator runs a + * destructive apply — therefore rode back in every record body, on every door + * that reads through the engine (`POST /data/:object/query`, the list route, + * `GET /data/:object/:id`, export, search, the RPC dispatcher, a hook's or a + * flow's in-process read). Naming the same column in `fields` answered + * `400 INVALID_FIELD`: the platform knew it was not a field and served it + * anyway, outside any field-level rule, because there is no field to attach a + * rule to. + * + * ## Where it is decided, and why there + * + * Triage's ruling on #21571: the default projection of an unprojected read is + * decided ONCE, in the engine, so every driver and every door gets the same + * answer — not per driver, not per door. The declared field set is that + * projection; no allow-list of column names and no flag re-opens undeclared + * columns on a runtime door. + * + * The engine SHAPES THE ROWS the driver hands back, rather than pushing a + * projection down to the driver. Measured on driver-sql: its recovery ladder + * retries `select('*')` whenever a projected statement fails on an + * unresolvable column, so a pushed-down projection naming a declared field + * whose column does not exist yet (not migrated, or the read races a schema + * sync) would be answered by the whole row — orphaned columns included. An + * EXPLICIT projection reaches that rung today too. Shaping the returned rows + * holds whichever rung answered, and needs no driver edit. + * + * It runs on the rows as they arrive from the driver — before formula + * evaluation, `expand`, file-reference resolution and the `afterFind` hooks — + * so everything downstream of storage sees the declared record, the same scope + * `materializeDeclaredFields` gives every CEL surface. Keys added AFTER that + * (a formula's value, an expanded record, a hook's derived key) are the + * engine's or the hook's, not storage's, and are untouched. Declared fields + * keep their existing treatment: `internal: true` omission, credential + * masking and the `__search` companion strip all still run after the hooks. + * + * ## Where it deliberately has no opinion + * + * The rule the read and write doors already share: a door that cannot see the + * field map must not invent a verdict about it. A schema with no field map, an + * ARRAY field map (not checkable — `Object.keys` yields indices), or an EMPTY + * one (indistinguishable from an unpopulated map: a registered object always + * carries at least the injected system columns) leaves the rows exactly as the + * driver returned them. + * + * ## In-process readers of undeclared columns — measured before this landed + * + * The objectql, rest, runtime, plugin-auth, plugin-sharing, plugin-audit and + * service-automation suites ran with every undeclared key removed at this + * seam; no production reader needed one (the fallout was test fixtures). The + * operator reads that legitimately need a retired column's values go through + * the driver, never through this path: `os migrate plan`'s `unmapped_column` + * detection introspects the table, and `os migrate account-issuer` reads + * `sys_account` through the driver the engine routes it to. + */ + +/** + * The columns the platform provisions on every physical table without an + * author declaring them. `id` is the driver's primary key; the two audit + * timestamps are engine/driver-stamped. The registry injects the rest of the + * system columns (tenant, owner, the audit actors) INTO the field map, so they + * need no entry here. + * + * ONE list: the read verbs' explicit-projection filter, this module's row + * shaping and the write path's undeclared-key door all read it, so a key a read + * accepts is never refused by a write and never trimmed from a row. + */ +export const PLATFORM_PROVISIONED_COLUMNS = ['id', 'created_at', 'updated_at'] as const; + +/** + * The declared column set of an object — its field-map keys plus + * {@link PLATFORM_PROVISIONED_COLUMNS} — or `undefined` when there is no + * checkable field map (absent, an array, or empty). `undefined` means "no + * opinion", never "nothing is declared". + */ +export function declaredColumnSet( + schema: { fields?: unknown } | null | undefined, +): ReadonlySet | undefined { + const fields = schema?.fields; + if (!fields || typeof fields !== 'object' || Array.isArray(fields)) return undefined; + const names = Object.keys(fields as Record); + if (names.length === 0) return undefined; + const declared = new Set(names); + for (const provisioned of PLATFORM_PROVISIONED_COLUMNS) declared.add(provisioned); + return declared; +} + +/** + * The row with every key outside `declared` removed — a NEW object when there + * was anything to remove, the same reference otherwise. Never mutates the row + * it is given: a driver that hands back a live reference into its own store + * (a contract violation, but one a test double commits) must not lose data + * because a read happened. + */ +export function withDeclaredColumnsOnly(row: T, declared: ReadonlySet | undefined): T { + if (!declared || !row || typeof row !== 'object' || Array.isArray(row)) return row; + const source = row as unknown as Record; + let undeclared = false; + for (const key of Object.keys(source)) { + if (!declared.has(key)) { undeclared = true; break; } + } + if (!undeclared) return row; + const shaped: Record = {}; + for (const key of Object.keys(source)) { + if (declared.has(key)) shaped[key] = source[key]; + } + return shaped as unknown as T; +} + +/** {@link withDeclaredColumnsOnly} over a driver's result page. */ +export function rowsWithDeclaredColumnsOnly(rows: T[], declared: ReadonlySet | undefined): T[] { + if (!declared) return rows; + let changed = false; + const shaped = rows.map((row) => { + const next = withDeclaredColumnsOnly(row, declared); + if (next !== row) changed = true; + return next; + }); + return changed ? shaped : rows; +} diff --git a/packages/objectql/src/engine.test.ts b/packages/objectql/src/engine.test.ts index 6a45adc88e3..46f1a3397fb 100644 --- a/packages/objectql/src/engine.test.ts +++ b/packages/objectql/src/engine.test.ts @@ -2565,6 +2565,20 @@ describe('ObjectQL — file-as-reference migration flag (#3617)', () => { let engine: ObjectQL; let driver: IDataDriver; + // [#21571] The flag columns the real `sys_migration` declares + // (`platform-objects` sys-migration.object.ts). A read serves only declared + // columns, so a double declaring `id` alone would read every flag row as + // empty — the fixture declares what the production object declares. + const SYS_MIGRATION_DEF = { + name: 'sys_migration', + fields: { + id: { type: 'text' }, + last_run_at: { type: 'datetime' }, + verified_at: { type: 'datetime' }, + blocking: { type: 'number' }, + }, + }; + const verifiedRow = { id: 'adr-0104-file-references', last_run_at: '2026-07-27T00:00:00.000Z', @@ -2594,7 +2608,7 @@ describe('ObjectQL — file-as-reference migration flag (#3617)', () => { const withMediaObject = () => { vi.mocked(SchemaRegistry.getObject).mockImplementation((name: string) => { if (name === 'note') return { name: 'note', fields: { doc: { type: 'file' } } } as any; - if (name === 'sys_migration') return { name: 'sys_migration', fields: { id: { type: 'text' } } } as any; + if (name === 'sys_migration') return SYS_MIGRATION_DEF as any; return undefined; }); }; @@ -2636,7 +2650,7 @@ describe('ObjectQL — file-as-reference migration flag (#3617)', () => { it('costs no query for an object that declares no media field', async () => { vi.mocked(SchemaRegistry.getObject).mockImplementation((name: string) => { if (name === 'invoice') return { name: 'invoice', fields: { amount: { type: 'number' } } } as any; - if (name === 'sys_migration') return { name: 'sys_migration', fields: { id: { type: 'text' } } } as any; + if (name === 'sys_migration') return SYS_MIGRATION_DEF as any; return undefined; }); vi.mocked(driver.findOne).mockResolvedValue(verifiedRow as any); @@ -2736,7 +2750,7 @@ describe('ObjectQL — file-as-reference migration flag (#3617)', () => { vi.mocked(SchemaRegistry.getObject).mockImplementation((name: string) => objects[name]); vi.mocked((SchemaRegistry as any).getAllObjects).mockImplementation(() => Object.values(objects)); }; - const SYS_MIGRATION = { name: 'sys_migration', fields: { id: { type: 'text' } } }; + const SYS_MIGRATION = SYS_MIGRATION_DEF; const lines = (info: any) => info.mock.calls.map((c: any[]) => String(c[0])).join('\n'); it('names the command that closes an open value-shape gate', async () => { diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index f5793bff919..aadf4018201 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -290,6 +290,14 @@ import { readonlyWhenFkJudgementReadsParent } from './validation/rule-validator. // total over the MASTER's declared fields before it leaves this engine — the // same helper every other server seam materialises with (#1871/#4649/#4953). import { materializeDeclaredFields } from './declared-fields.js'; +// [#21571] The declared column set — the read verbs' default projection, their +// explicit-projection filter and the write path's undeclared-key door, one list. +import { + PLATFORM_PROVISIONED_COLUMNS, + declaredColumnSet, + rowsWithDeclaredColumnsOnly, + withDeclaredColumnsOnly, +} from './declared-read-columns.js'; import { applyInMemoryAggregation } from './in-memory-aggregation.js'; import { resolveEngineDeleteDispatch, @@ -1912,19 +1920,17 @@ function assertProjectionHasNoDottedPaths( * because the partial-success path (`insertMany`) reports per row and must * cull the bad rows instead of failing the batch around them. */ -const PLATFORM_PROVISIONED_COLUMNS = ['id', 'created_at', 'updated_at'] as const; - function undeclaredWriteFieldErrors( object: string, schema: { fields?: unknown } | undefined, rows: readonly unknown[], ): Array { const out: Array = new Array(rows.length); - const fields = schema?.fields; - if (!fields || typeof fields !== 'object' || Array.isArray(fields)) return out; - const declared = new Set(Object.keys(fields as Record)); - if (declared.size === 0) return out; - for (const provisioned of PLATFORM_PROVISIONED_COLUMNS) declared.add(provisioned); + // [#21571] The declared set and its "no opinion" cases (no map, an array + // map, an empty map) are `declaredColumnSet`'s, shared with the read verbs' + // default projection — one answer to "is this a column of the object". + const declared = declaredColumnSet(schema); + if (!declared) return out; for (let i = 0; i < rows.length; i++) { const row = rows[i]; if (!row || typeof row !== 'object' || Array.isArray(row)) continue; @@ -11894,13 +11900,12 @@ export class ObjectQL implements IObjectQLEngine { // projection is a different fact and no longer reaches this filter via // the engine — `assertProjectionHasNoDottedPaths` above refused it. if (_findSchema?.fields && Array.isArray(ast.fields) && ast.fields.length > 0) { - const known = new Set(Object.keys(_findSchema.fields)); // Always allow the primary key + audit columns even if not present in // schema.fields. Without this, callers requesting `select=id,name` // silently get the `id` projected away, breaking record navigation. - known.add('id'); - known.add('created_at'); - known.add('updated_at'); + // [#21571] The same three the default projection and the write door + // admit — `PLATFORM_PROVISIONED_COLUMNS`, one list. + const known = new Set([...Object.keys(_findSchema.fields), ...PLATFORM_PROVISIONED_COLUMNS]); // Whole names, no head-splitting: only plain entries reach here (the // dotted refusal above fired on anything carrying a '.'). const filtered = ast.fields.filter(f => known.has(f)); @@ -11939,6 +11944,19 @@ export class ObjectQL implements IObjectQLEngine { try { let result = await driver.find(object, hookContext.input.ast as QueryAST, hookContext.input.options as any); + // [#21571] The read's default projection is the DECLARED field set: + // a column no metadata declares (a field retired in an upgrade, + // whose column additive sync leaves behind) never leaves the engine. + // Shaped here, on the rows as the driver returned them, so it holds + // whichever driver answered and whichever rung of a driver's + // recovery ladder answered (driver-sql retries `select('*')` when a + // projected statement names a missing column) — and before formulas, + // `expand`, file references and the hooks, so all of them see the + // declared record. See `declared-read-columns.ts`. + if (Array.isArray(result)) { + result = rowsWithDeclaredColumnsOnly(result, declaredColumnSet(_findSchema)); + } + // Post-process: evaluate formula virtual fields against the raw rows. // [#20082] With the caller's permission map when a formula calls // `can` — one resolution for the whole result set, never per row. @@ -12174,12 +12192,9 @@ export class ObjectQL implements IObjectQLEngine { // the rationale, and for why this tolerance is plain-columns-only ([#7589] // refused any dotted entry above, so none reaches this filter). if (_findOneSchema?.fields && Array.isArray(ast.fields) && ast.fields.length > 0) { - const known = new Set(Object.keys(_findOneSchema.fields)); // Always allow the primary key + audit columns even if not present - // in schema.fields (matches `find()` behavior). - known.add('id'); - known.add('created_at'); - known.add('updated_at'); + // in schema.fields (matches `find()` behavior, one list). + const known = new Set([...Object.keys(_findOneSchema.fields), ...PLATFORM_PROVISIONED_COLUMNS]); const filtered = ast.fields.filter(f => known.has(f)); ast.fields = filtered.length > 0 ? filtered : undefined; } @@ -12215,6 +12230,10 @@ export class ObjectQL implements IObjectQLEngine { let result = await driver.findOne(objectName, hookContext.input.ast as QueryAST, hookContext.input.options as any); + // [#21571] Same default projection as `find`, same position: the + // declared field set, applied to the row as the driver returned it. + result = withDeclaredColumnsOnly(result, declaredColumnSet(_findOneSchema)); + // Post-process: evaluate formula virtual fields against the raw row // ([#20082] with the caller's permission map when a formula calls `can`). if (result != null) { diff --git a/packages/objectql/src/no-operator-object-door.ts b/packages/objectql/src/no-operator-object-door.ts index aa0361703bb..4ebc814b237 100644 --- a/packages/objectql/src/no-operator-object-door.ts +++ b/packages/objectql/src/no-operator-object-door.ts @@ -206,7 +206,7 @@ export function declaredNoOperatorObjectColumn(def: unknown): NoOperatorObjectCo * [#20745] The columns every record carries whether or not the declared map * lists them, with the type each stores: the same three names `find` / * `findOne` add to their known set and the write gate admits unconditionally - * (`PLATFORM_PROVISIONED_COLUMNS` in `engine.ts`), because the platform + * (`PLATFORM_PROVISIONED_COLUMNS` in `declared-read-columns.ts`), because the platform * provisions them rather than the author declaring them. */ const PLATFORM_PROVISIONED_COLUMN_TYPES: ReadonlyMap = new Map([ diff --git a/packages/objectql/src/unprojected-read-declared-fields-conformance.test.ts b/packages/objectql/src/unprojected-read-declared-fields-conformance.test.ts new file mode 100644 index 00000000000..9486a22e4c4 --- /dev/null +++ b/packages/objectql/src/unprojected-read-declared-fields-conformance.test.ts @@ -0,0 +1,255 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#21571] A row leaves the engine's read verbs carrying only the object's + * DECLARED fields plus the platform-provisioned columns — whatever the driver + * returned, on every door. + * + * ## Why a matrix over driver BEHAVIOURS + * + * The default projection is decided once, in the engine (`declared-read-columns.ts`), + * so no driver may diverge from it. The way a driver could reintroduce the + * defect is by returning more than it was asked for, and three shapes of that + * are real: + * + * - `whole row` — ignores the projection and always returns the stored row; + * - `projection` — honours an explicit projection, and answers no projection + * with the whole row (`SELECT *`), as driver-sql does; + * - `ladder` — honours a projection unless it names a column the table + * does not have, and then retries with the whole row + * (driver-sql's unresolvable-column recovery ladder). + * + * Every door runs the same assertion against each shape, so a door added later + * is one row and a driver shape added later is one entry. + * + * ## The table + * + * `rq_contact` was created by an older declaration: its stored rows carry + * `mailing_street` / `mailing_city`, which the current declaration retired. + * The stored row also carries the registry-injected system columns, a + * `password` field (masked on read, ADR-0100), an `internal: true` field + * (omitted on read) and the inputs of a `formula` field — the declared fields + * whose treatment must not move. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { SECRET_MASK } from '@objectstack/spec/data'; + +import { ObjectQL } from './engine.js'; + +type Row = Record; +type Behaviour = 'whole row' | 'projection' | 'ladder'; + +interface DriverAst { + where?: Record; + fields?: string[]; + limit?: number; +} + +function makeStoreDriver(behaviour: Behaviour) { + const tables = new Map>(); + const tableFor = (o: string): Map => { + let t = tables.get(o); + if (!t) { t = new Map(); tables.set(o, t); } + return t; + }; + const matches = (row: Row, where: Record | undefined): boolean => { + if (!where) return true; + for (const [k, v] of Object.entries(where)) { + if (k.startsWith('$')) continue; + if (v !== null && typeof v === 'object' && '$in' in (v as Row)) { + if (!((v as { $in: unknown[] }).$in).includes(row[k])) return false; + continue; + } + if ((row[k] ?? null) !== (v ?? null)) return false; + } + return true; + }; + const run = (object: string, ast: DriverAst | undefined): Row[] => { + let out = Array.from(tableFor(object).values()).filter((r) => matches(r, ast?.where)); + if (typeof ast?.limit === 'number') out = out.slice(0, ast.limit); + const fields = Array.isArray(ast?.fields) && ast.fields.length > 0 ? ast.fields : undefined; + // Physical columns = the keys every stored row of the table carries. + const columns = new Set(Array.from(tableFor(object).values()).flatMap((r) => Object.keys(r))); + const project = fields !== undefined + && behaviour !== 'whole row' + && !(behaviour === 'ladder' && fields.some((f) => !columns.has(f))); + // Shallow copies either way — the `IDataDriver` contract. + return project + ? out.map((r) => Object.fromEntries(fields!.map((f) => [f, r[f]]))) + : out.map((r) => ({ ...r })); + }; + let seq = 0; + const driver = { + name: `store-${behaviour.replace(' ', '-')}`, version: '0.0.0', supports: {}, + async connect(): Promise {}, + async disconnect(): Promise {}, + async checkHealth(): Promise { return true; }, + async execute(): Promise { return null; }, + async find(object: string, ast?: DriverAst): Promise { return run(object, ast); }, + async findOne(object: string, ast?: DriverAst): Promise { return run(object, ast)[0] ?? null; }, + async create(object: string, data: Row): Promise { + seq += 1; + const row: Row = { ...data, id: (data.id as string | undefined) ?? `new_${seq}` }; + tableFor(object).set(String(row.id), row); + return { ...row }; + }, + async update(object: string, id: string, data: Row): Promise { + const next: Row = { ...tableFor(object).get(id), ...data, id }; + tableFor(object).set(id, next); + return { ...next }; + }, + async delete(object: string, id: string): Promise { return tableFor(object).delete(id); }, + async count(object: string, ast?: DriverAst): Promise { return run(object, ast).length; }, + }; + return { + driver, + seed: (object: string, row: Row) => { tableFor(object).set(String(row.id), { ...row }); }, + stored: (object: string, id: string) => tableFor(object).get(id), + }; +} + +const CONTACT = 'rq_contact'; +const ACCOUNT = 'rq_account'; +const RETIRED = ['mailing_street', 'mailing_city']; + +const SYSTEM = { + created_at: '2026-01-01T00:00:00.000Z', + updated_at: '2026-01-01T00:00:00.000Z', + created_by: 'usr_1', + updated_by: 'usr_1', + organization_id: 'org_1', + owner_id: 'usr_1', +}; + +async function boot(behaviour: Behaviour) { + const engine = new ObjectQL(); + const store = makeStoreDriver(behaviour); + engine.registerDriver(store.driver as never, true); + await engine.init(); + engine.registry.registerObject({ + name: ACCOUNT, + label: 'Account', + fields: { name: { type: 'text' } }, + } as never, 'test'); + engine.registry.registerObject({ + name: CONTACT, + label: 'Contact', + fields: { + name: { type: 'text' }, + account: { type: 'lookup', reference: ACCOUNT }, + score: { type: 'number' }, + // Declared, no column: what an explicit projection names to reach the + // `ladder` shape's whole-row retry. + nickname: { type: 'text' }, + pin: { type: 'password' }, + token: { type: 'text', internal: true }, + double_score: { type: 'formula', expression: { dialect: 'cel', source: 'record.score * 2' } }, + }, + } as never, 'test'); + + store.seed(ACCOUNT, { id: 'acc_1', name: 'Acme', ...SYSTEM, legacy_region: 'retired' }); + store.seed(CONTACT, { + id: 'con_1', name: 'Ada', account: 'acc_1', score: 21, pin: 'plain', token: 'hash', + ...SYSTEM, + mailing_street: '1 Retired Way', mailing_city: 'Oldtown', + }); + return { engine, store, protocol: new ObjectStackProtocolImplementation(engine as never) }; +} + +/** Every record body a door can hand back, flattened, nested `expand` included. */ +function recordsOf(value: unknown): Row[] { + if (value == null || typeof value !== 'object') return []; + if (Array.isArray(value)) return value.flatMap((v) => recordsOf(v)); + const row = value as Row; + return [row, ...Object.values(row).filter((v) => v !== null && typeof v === 'object').flatMap(recordsOf)]; +} + +function expectNoRetiredColumn(value: unknown): void { + const rows = recordsOf(value); + expect(rows.length).toBeGreaterThan(0); + for (const row of rows) { + for (const retired of [...RETIRED, 'legacy_region']) expect(Object.keys(row)).not.toContain(retired); + } +} + +describe.each(['whole row', 'projection', 'ladder'])( + '[#21571] a read serves the declared fields only — driver shape: %s', + (behaviour) => { + let h: Awaited>; + beforeEach(async () => { h = await boot(behaviour); }); + + it('the fixture is real: the stored row DOES carry the retired columns', () => { + expect(h.store.stored(CONTACT, 'con_1')).toMatchObject({ mailing_street: '1 Retired Way', mailing_city: 'Oldtown' }); + }); + + it('door: find with no projection', async () => { + expectNoRetiredColumn(await h.engine.find(CONTACT, {})); + }); + + it('door: find with no query at all', async () => { + expectNoRetiredColumn(await h.engine.find(CONTACT)); + }); + + it('door: findOne', async () => { + expectNoRetiredColumn(await h.engine.findOne(CONTACT, { where: { id: 'con_1' } })); + }); + + it('door: an explicit projection naming a declared field with no column (the ladder rung)', async () => { + const rows = await h.engine.find(CONTACT, { fields: ['name', 'nickname'] }); + expect(rows[0]).toMatchObject({ name: 'Ada' }); + expectNoRetiredColumn(rows); + }); + + it('door: an explicit projection naming ONLY a retired column', async () => { + // The engine's unknown-plain tolerance drops the name and reads the + // default projection; it must not read every column. + const rows = await h.engine.find(CONTACT, { fields: ['mailing_street'] }); + expect(rows[0]).toMatchObject({ id: 'con_1', name: 'Ada' }); + expectNoRetiredColumn(rows); + }); + + it('door: expand — the related record is the related object\'s declared fields', async () => { + const rows = await h.engine.find(CONTACT, { expand: { account: { object: ACCOUNT } } } as never); + expect((rows[0]?.account as Row | undefined)?.name).toBe('Acme'); + expectNoRetiredColumn(rows); + }); + + it('door: findData — POST /data/:object/query and the list route', async () => { + const res: any = await h.protocol.findData({ object: CONTACT, query: {} }); + expectNoRetiredColumn(res.records); + }); + + it('door: getData — GET /data/:object/:id', async () => { + const res: any = await h.protocol.getData({ object: CONTACT, id: 'con_1' }); + expectNoRetiredColumn(res.record); + }); + + it('door: cloneData — the copy is made from the declared record', async () => { + // The clone copies every key of the source read into an insert, and + // the insert refuses a key the object does not declare: a source read + // carrying a retired column made every clone of such a record fail. + const res: any = await h.protocol.cloneData({ object: CONTACT, id: 'con_1', overrides: { name: 'Ada 2' } }); + expect(res.record).toMatchObject({ name: 'Ada 2' }); + expectNoRetiredColumn(res.record); + }); + + it('declared fields and system columns keep their treatment', async () => { + const row = (await h.engine.findOne(CONTACT, { where: { id: 'con_1' } }))!; + expect(row).toMatchObject({ + id: 'con_1', name: 'Ada', account: 'acc_1', score: 21, + double_score: 42, // formula: computed after the shaping, from declared inputs + pin: SECRET_MASK, // `password` on a generic object: masked, not dropped + ...SYSTEM, // the registry-injected system columns and the provisioned three + }); + expect(row).not.toHaveProperty('token'); // `internal: true`: omitted, as before + }); + + it('the driver\'s stored row is never mutated by the read', async () => { + await h.engine.find(CONTACT, {}); + await h.engine.findOne(CONTACT, { where: { id: 'con_1' } }); + expect(h.store.stored(CONTACT, 'con_1')).toMatchObject({ mailing_street: '1 Retired Way', pin: 'plain', token: 'hash' }); + }); + }, +); diff --git a/packages/rest/src/data-query-unprojected-declared-fields.test.ts b/packages/rest/src/data-query-unprojected-declared-fields.test.ts new file mode 100644 index 00000000000..ce1e5915b11 --- /dev/null +++ b/packages/rest/src/data-query-unprojected-declared-fields.test.ts @@ -0,0 +1,234 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#21571] A read with no projection serves the object's DECLARED fields plus + * the platform's own system columns — never a column no metadata declares. + * + * The door the card measured, reproduced on the composed REST harness this + * package already uses (`RestServer` → `ObjectStackProtocolImplementation` → + * `ObjectQL` → a real `SqlDriver` on better-sqlite3): + * + * 1. Boot ONE declares `rq_contact` with two mailing fields, syncs, and writes + * rows carrying values in them. + * 2. Boot TWO declares the same object with those two fields retired — the + * upgrade the card describes. Schema sync is additive, so the two columns + * and their values stay in the table, with no field behind them. + * 3. `POST /api/v1/data/rq_contact/query` with no `fields`. + * + * Before the engine owned the default projection, step 3 answered every row + * WITH the two retired columns and their values, while naming one of them in + * `fields` answered `400 INVALID_FIELD`: the platform knew they were not fields + * and served them anyway. The default projection is now decided once, in the + * engine, from the registry's field map plus the platform-provisioned columns — + * the same set an explicit projection is judged against. + * + * The last case is the driver-sql recovery ladder: an EXPLICIT projection that + * names a declared field whose column does not exist yet makes the statement + * fail, and the ladder answers it with `select('*')`. That rung served the + * retired columns too, and no driver edit is involved in closing it: the + * engine shapes the rows the driver hands back, whichever rung answered. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { RestServer } from './rest-server'; + +const OBJECT = 'rq_contact'; +const LADDER = 'rq_ladder'; +const RETIRED = ['mailing_street', 'mailing_city'] as const; + +const CONTACT_V1 = { + name: OBJECT, + label: 'Contact', + fields: { + name: { name: 'name', type: 'text' as const }, + email: { name: 'email', type: 'text' as const }, + mailing_street: { name: 'mailing_street', type: 'text' as const }, + mailing_city: { name: 'mailing_city', type: 'text' as const }, + }, +}; + +/** The upgrade: the two mailing fields are retired from the declaration. */ +const CONTACT_V2 = { + name: OBJECT, + label: 'Contact', + fields: { + name: { name: 'name', type: 'text' as const }, + email: { name: 'email', type: 'text' as const }, + }, +}; + +const LADDER_V1 = { + name: LADDER, + label: 'Ladder', + fields: { + name: { name: 'name', type: 'text' as const }, + legacy_note: { name: 'legacy_note', type: 'text' as const }, + }, +}; + +/** + * Boot two retires `legacy_note` and declares `pending_note`, registered AFTER + * the schema sync so its column is never created: a declared field with no + * column, the state that sends an explicit projection down the ladder. + */ +const LADDER_V2 = { + name: LADDER, + label: 'Ladder', + fields: { + name: { name: 'name', type: 'text' as const }, + pending_note: { name: 'pending_note', type: 'text' as const }, + }, +}; + +function createMockServer() { + const noop = () => {}; + return { get: noop, post: noop, put: noop, delete: noop, patch: noop, use: noop, listen: async () => {}, close: async () => {} }; +} + +function makeRes() { + const res: any = { + write: () => true, end: () => {}, + header: () => res, + status: (code: number) => { res._status = code; return res; }, + json: (body: any) => { res._json = body; return res; }, + }; + return res; +} + +const dir = mkdtempSync(join(tmpdir(), 'os-21571-')); +const filename = join(dir, 'retired-columns.sqlite'); +const engines: ObjectQL[] = []; +afterAll(async () => { + for (const e of engines) { + try { await e.destroy(); } catch { /* noop */ } + } + rmSync(dir, { recursive: true, force: true }); +}); + +const sqlDriver = () => + new SqlDriver({ client: 'better-sqlite3', connection: { filename }, useNullAsDefault: true }) as any; + +/** Boot one: the old declaration creates the tables and writes the values. */ +async function bootOne(): Promise { + const engine = new ObjectQL(); + engine.registerDriver(sqlDriver(), true); + await engine.init(); + engine.registry.registerObject(CONTACT_V1 as any); + engine.registry.registerObject(LADDER_V1 as any); + await engine.syncSchemas(); + await engine.insert(OBJECT, [ + { id: 'c1', name: 'Ada', email: 'ada@example.com', mailing_street: '1 Retired Way', mailing_city: 'Oldtown' }, + { id: 'c2', name: 'Bo', email: 'bo@example.com', mailing_street: '2 Retired Way', mailing_city: 'Oldtown' }, + ] as any); + await engine.insert(LADDER, [{ id: 'l1', name: 'one', legacy_note: 'retired value' }] as any); + await engine.destroy(); +} + +/** Boot two: the new declaration on the database boot one left behind. */ +async function bootTwo() { + const engine = new ObjectQL(); + engines.push(engine); + engine.registerDriver(sqlDriver(), true); + await engine.init(); + engine.registry.registerObject(CONTACT_V2 as any); + await engine.syncSchemas(); + engine.registry.registerObject(LADDER_V2 as any); + + const protocol = new ObjectStackProtocolImplementation(engine as any); + const rest = new RestServer(createMockServer() as any, protocol as any, { api: { requireAuth: false } } as any); + (rest as any).resolveExecCtx = async () => ({ userId: 'test-user' }); + rest.registerRoutes(); + const routes = rest.getRoutes(); + const queryRoute = routes.find((r: any) => r.method === 'POST' && r.path === '/api/v1/data/:object/query'); + const getRoute = routes.find((r: any) => r.method === 'GET' && r.path === '/api/v1/data/:object/:id'); + expect(queryRoute).toBeDefined(); + expect(getRoute).toBeDefined(); + const query = async (object: string, body: Record) => { + const res = makeRes(); + await queryRoute!.handler({ params: { object }, body } as any, res); + return res; + }; + const getById = async (object: string, id: string) => { + const res = makeRes(); + await getRoute!.handler({ params: { object, id }, query: {} } as any, res); + return res; + }; + return { engine, query, getById }; +} + +const rowsOf = (res: any): Array> => res._json?.records ?? res._json?.data ?? res._json; + +describe('[#21571] an unprojected read serves the declared fields, never a retired column', () => { + let engine: ObjectQL; + let query: Awaited>['query']; + let getById: Awaited>['getById']; + beforeAll(async () => { + await bootOne(); + ({ engine, query, getById } = await bootTwo()); + }); + + it('POST /data/:object/query with no `fields` omits the retired columns from every row', async () => { + const res = await query(OBJECT, {}); + expect(res._status ?? 200).toBe(200); + const rows = rowsOf(res); + expect(rows.map((r) => r.id).sort()).toEqual(['c1', 'c2']); + for (const row of rows) { + for (const retired of RETIRED) expect(row).not.toHaveProperty(retired); + } + }); + + it('GET /data/:object/:id omits them too (the findOne door)', async () => { + const res = await getById(OBJECT, 'c1'); + expect(res._status ?? 200).toBe(200); + const record = res._json?.record ?? res._json; + expect(record.id).toBe('c1'); + for (const retired of RETIRED) expect(record).not.toHaveProperty(retired); + }); + + it('declared fields and the system columns are served as before', async () => { + const [row] = rowsOf(await query(OBJECT, { where: { id: 'c1' } })); + expect(row).toMatchObject({ id: 'c1', name: 'Ada', email: 'ada@example.com' }); + // The registry-injected system columns, named one by one so a change that + // dropped any of them fails here, and the platform-provisioned three. + for (const system of [ + 'id', 'created_at', 'updated_at', 'created_by', 'updated_by', + 'organization_id', 'owner_id', 'owning_business_unit_id', + ]) { + expect(row).toHaveProperty(system); + } + expect(row.created_at).toEqual(expect.any(String)); + }); + + it('every key a row carries is a declared field or a platform-provisioned column', async () => { + const declared = new Set([ + ...Object.keys(engine.registry.getObject(OBJECT)!.fields as object), + 'id', 'created_at', 'updated_at', + ]); + for (const row of rowsOf(await query(OBJECT, {}))) { + expect(Object.keys(row).filter((key) => !declared.has(key))).toEqual([]); + } + }); + + it('naming a retired column in `fields` still answers INVALID_FIELD / 400', async () => { + const res = await query(OBJECT, { fields: ['name', 'mailing_street'] }); + expect(res._status).toBe(400); + expect(res._json?.error?.code ?? res._json?.code).toBe('INVALID_FIELD'); + }); + + it('the recovery ladder\'s select(*) rung serves no retired column either', async () => { + // `pending_note` is declared but has no column: the projected statement + // fails and driver-sql retries `select('*')`. + const res = await query(LADDER, { fields: ['name', 'pending_note'] }); + expect(res._status ?? 200).toBe(200); + const rows = rowsOf(res); + expect(rows.map((r) => r.id)).toEqual(['l1']); + expect(rows[0]).toMatchObject({ id: 'l1', name: 'one' }); + expect(rows[0]).not.toHaveProperty('legacy_note'); + }); +});