diff --git a/.changeset/29241-text-field-scalar-stored-string.md b/.changeset/29241-text-field-scalar-stored-string.md new file mode 100644 index 00000000000..c3939527f2f --- /dev/null +++ b/.changeset/29241-text-field-scalar-stored-string.md @@ -0,0 +1,16 @@ +--- +'@objectstack/objectql': patch +--- + +fix(objectql)!: a number or a boolean written to a text-family field is stored as the string the validator judged + +Clause-②: yes (narrowing) +Metadata-protocol: none + + + +**BREAKING** for one case only, a `unique` text-family field. FROM: `{ code: 42 }` and then `{ code: "42" }` (or `true` and then `"true"`) both answered `201` and stored two rows, on driver-memory and on SQLite. TO: both writes store `"42"`, so the second one is refused `409 UNIQUE_VIOLATION`. The fix for a caller that hits it: the two writes are the same value; send the string once, or update the existing row. In the other direction, SQLite used to refuse `true` after `1` (and `false` after `0`) because both were stored as `"1.0"` (`"0.0"`); both now land as two different strings (`"true"` and `"1"`). driver-memory admitted both before and still does. + +The record validator's string arm (`text`, `textarea`, `email`, `url`, `phone`, `password`, `markdown`, `html`, `richtext`, `code`, `signature`, `qrcode`) judges a number or a boolean by its string reading: `42` is bounded and format-checked as `"42"`, `true` as `"true"`. The write used to carry the raw scalar to the driver, so the stored value depended on the driver: driver-memory stored and read back the number `42` and the boolean `true`, and SQLite stored the TEXT `"42.0"` for `42` and `"1.0"` for `true` (a `maxLength: 2` field admitted `42` and then held four characters). The write door now writes the judged string into the payload, beside the numeric-string rewrite on number fields, on insert, insertMany, update by id, update by predicate and the `validate()` dry run. + +What you will see: a text-family column now stores and reads back `"42"` where driver-memory stored `42`, and `"42"` / `"true"` where SQLite stored `"42.0"` / `"1.0"`. A single write is answered as before: the same payloads are admitted and refused. What moves is the unique answer above, and hooks, formulas and filters now see the string. A filter that compares a text-family field to the number `42` (or to a boolean) no longer matches a row written as `42`, because that row is now stored as `"42"`: filter with the string. The fix on the filter side is tracked separately. Rows already stored are not rewritten, so an old SQLite `"42.0"` row, or a raw `42` in a driver-memory store with `persistence` enabled, does not collide with a new `"42"`. Unchanged: strings, `null`, missing keys, objects and arrays (refused), `json` and the structured types, and number and boolean fields. diff --git a/packages/objectql/src/protocol-save-notice-answer.test.ts b/packages/objectql/src/protocol-save-notice-answer.test.ts index 68ee70c1a86..b0e15dcad9f 100644 --- a/packages/objectql/src/protocol-save-notice-answer.test.ts +++ b/packages/objectql/src/protocol-save-notice-answer.test.ts @@ -36,7 +36,10 @@ const DEAL = { label: 'Deal', fields: { body: { name: 'body', label: 'Body', type: 'text' }, - say: { name: 'say', label: 'Say', type: 'text' }, + // `json`, not `text`: the refusal cases below hand the hook a raw number, + // and a text-family field stores (and hands its hooks) the string the + // validator judges a number by (#29241). + say: { name: 'say', label: 'Say', type: 'json' }, say_after: { name: 'say_after', label: 'Say after', type: 'text' }, }, }; diff --git a/packages/objectql/src/validation/record-validator.text-scalar-value.test.ts b/packages/objectql/src/validation/record-validator.text-scalar-value.test.ts new file mode 100644 index 00000000000..9b8d4fba184 --- /dev/null +++ b/packages/objectql/src/validation/record-validator.text-scalar-value.test.ts @@ -0,0 +1,182 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#29241] A number or a boolean written to a string-typed field + * (`BOUNDED_STRING_FIELD_TYPES`) is stored as the string the record + * validator's string arm judged it by. + * + * Measured on `origin/main` @ `a19759ac` before the change: the arm judged + * `String(value)` (`42` as `'42'`, `true` as `'true'`) while the write carried + * the raw scalar, so the driver's `create()` / `update()` received the number + * or the boolean as sent. The door now writes the judged string into the + * payload (`normalizeNumericStringValues`, beside the number arm's #20309 + * rewrite), on every engine door: insert, insertMany, update by id and update + * by predicate. (The `validate()` dry run calls the same function at the same + * point; its answer carries no values, so the arm's verdict there is unchanged.) + * + * A single write is admitted or refused exactly as before. The unique-key + * answer does move (Clause-② yes, narrowing): `42` after `"42"` on a `unique` + * text field now collides, pinned below at the driver door and against SQLite + * in `rest-data-text-scalar-value.test.ts`. Untouched, and pinned + * here: a string, `null`, a missing key, a plain object or an array (refused, + * #29133), a `json` field, a number field, a boolean field, and a declared + * field the validator never judges. + */ + +import { describe, it, expect } from 'vitest'; +import { BOUNDED_STRING_FIELD_TYPES } from '@objectstack/spec/data'; +import { normalizeNumericStringValues, validateRecord, ValidationError } from './record-validator.js'; +import { ObjectQL } from '../engine.js'; + +const STRING_TYPES = [...BOUNDED_STRING_FIELD_TYPES]; + +const SCALARS: ReadonlyArray = [ + [42, '42'], + [0, '0'], + [-1.5, '-1.5'], + [true, 'true'], + [false, 'false'], +]; + +describe('#29241 — the door writes the string the arm judged', () => { + it('reads the spec set, which still holds the twelve members this pin was written against', () => { + expect(STRING_TYPES).toHaveLength(12); + }); + + for (const type of STRING_TYPES) { + for (const [input, stored] of SCALARS) { + it(`${type}: ${JSON.stringify(input)} is written as ${JSON.stringify(stored)}`, () => { + const out = normalizeNumericStringValues({ fields: { f: { type } } }, { f: input }); + expect(out.f).toBe(stored); + }); + } + } + + it('the written string is the one the arm judges: a bound reads its length', () => { + const schema = { fields: { code: { type: 'text', label: 'Code', maxLength: 2 } } }; + const row = normalizeNumericStringValues(schema, { code: 123 }); + expect(row).toEqual({ code: '123' }); + let err: unknown = null; + try { + validateRecord(schema, row, 'insert'); + } catch (e) { + err = e; + } + expect(err).toBeInstanceOf(ValidationError); + expect((err as ValidationError).fields.map((f) => [f.field, f.code])).toEqual([['code', 'max_length']]); + }); + + it('a readonly field is written too: its value is judged on every write', () => { + const schema = { fields: { stamp: { type: 'text', readonly: true } } }; + expect(normalizeNumericStringValues(schema, { stamp: 7 })).toEqual({ stamp: '7' }); + }); + + it('a batch is rewritten per row, and the caller objects are never mutated', () => { + const schema = { fields: { title: { type: 'text' } } }; + const a = { title: 1 }; + const b = { title: 'two' }; + const list = [a, b]; + const out = normalizeNumericStringValues(schema, list); + expect(out).not.toBe(list); + expect(out[0]).toEqual({ title: '1' }); + expect(out[1]).toBe(b); + expect(a).toEqual({ title: 1 }); + }); +}); + +describe('#29241 — what the door leaves alone', () => { + const schema = { + fields: { + title: { type: 'text' }, + payload: { type: 'json' }, + amount: { type: 'number' }, + done: { type: 'boolean' }, + }, + }; + + it('a string, null, a missing key, an object and an array on a text field: the same row back', () => { + for (const row of [{ title: '42' }, { title: null }, { title: undefined }, {}, { title: { a: 1 } }, { title: ['a'] }]) { + expect(normalizeNumericStringValues(schema, row), JSON.stringify(row)).toBe(row); + } + }); + + it('a json, number or boolean field keeps its number or boolean', () => { + const row = { payload: 42, amount: 42, done: true }; + expect(normalizeNumericStringValues(schema, row)).toBe(row); + const flags = { payload: false, amount: 0, done: false }; + expect(normalizeNumericStringValues(schema, flags)).toBe(flags); + }); + + it('a declared field the validator never judges is not rewritten (a SKIP_FIELDS name, a system field)', () => { + const own = { fields: { id: { type: 'text' }, tenant_tag: { type: 'text', system: true } } }; + const row = { id: 42, tenant_tag: 7 }; + expect(normalizeNumericStringValues(own, row)).toBe(row); + }); + + it('an undeclared key is not rewritten', () => { + const row = { nope: 42 }; + expect(normalizeNumericStringValues(schema, row)).toBe(row); + }); +}); + +/** The driver-facing half: what each engine door hands the driver. */ +describe('#29241 — write path end to end: the driver receives the judged string', () => { + async function boot() { + const engine = new ObjectQL(); + const writes: unknown[] = []; + const rows = new Map>([['rec_1', { id: 'rec_1', title: 'orig' }]]); + engine.registerDriver({ + name: 'recording', version: '0.0.0', supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, + async execute() { return null; }, + async find() { return Array.from(rows.values()); }, + async findOne(_o: string, ast: any) { return rows.get(ast?.where?.id) ?? null; }, + async create(_o: string, data: Record) { writes.push(data); return { ...data, id: 'x' }; }, + async update(_o: string, id: string, data: Record) { writes.push(data); return { ...data, id }; }, + async updateMany(_o: unknown, _a: unknown, data: unknown) { writes.push(data); return 0; }, + async delete() { return true; }, async count() { return rows.size; }, + async bulkCreate(_o: string, data: Record[]) { writes.push(...data); return data.map((d, i) => ({ ...d, id: `b${i}` })); }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + } as any, true); + await engine.init(); + engine.registry.registerObject({ + name: 'scalar_task', label: 'Task', + fields: { + id: { name: 'id', label: 'ID', type: 'text', primaryKey: true }, + title: { name: 'title', label: 'Title', type: 'text' }, + mail: { name: 'mail', label: 'Mail', type: 'phone' }, + amount: { name: 'amount', label: 'Amount', type: 'number' }, + code: { name: 'code', label: 'Code', type: 'text', unique: true }, + }, + } as any); + return { engine, writes }; + } + + const titles = (writes: unknown[]) => writes.map((w) => (w as Record).title); + + it('insert, insertMany, update by id and update by predicate', async () => { + const { engine, writes } = await boot(); + await engine.insert('scalar_task', { title: 42, mail: 5551234, amount: 7 } as any); + await engine.insert('scalar_task', [{ title: true }, { title: 'kept' }] as any); + await engine.update('scalar_task', { title: false } as any, { where: { id: 'rec_1' } } as any); + await engine.update('scalar_task', { title: 0 } as any, { multi: true } as any); + expect(titles(writes)).toEqual(['42', 'true', 'kept', 'false', '0']); + for (const w of writes) { + const title = (w as Record).title; + expect(typeof title, JSON.stringify(w)).toBe('string'); + } + // A text-family format type is written the same way; a number field keeps its number. + expect(writes[0]).toEqual(expect.objectContaining({ mail: '5551234', amount: 7 })); + }); + + it('a unique text field: 42 and "42" reach the driver as one value, so its unique key sees one value', async () => { + // What any driver's unique check compares (driver-memory keys on the stored + // value): base handed it 42 and "42", two keys; now one. + const { engine, writes } = await boot(); + await engine.insert('scalar_task', { code: 42 } as any); + await engine.insert('scalar_task', { code: '42' } as any); + expect(writes.map((w) => (w as Record).code)).toEqual(['42', '42']); + }); +}); diff --git a/packages/objectql/src/validation/record-validator.ts b/packages/objectql/src/validation/record-validator.ts index bce552f0fba..b39c42d05db 100644 --- a/packages/objectql/src/validation/record-validator.ts +++ b/packages/objectql/src/validation/record-validator.ts @@ -945,6 +945,30 @@ function isPlainObjectValue(value: unknown): boolean { * driver copy. A value a `before*` hook writes after the door is the hook's * own and is not rewritten; the arm still judges it by the same grammar. * + * ## [#29241] The string arm's half: a number or a boolean on a text field + * + * The mirror case on the string-stored side. The record validator's string arm + * (`BOUNDED_STRING_FIELD_TYPES`, the spec's set: text, textarea, email, url, + * phone, password, markdown, html, richtext, code, signature, qrcode) judges a + * number or a boolean by its string reading, `String(value)`: `42` is bounded, + * domain-checked and format-checked as `'42'`, `true` as `'true'`. The write + * carried the raw scalar, so driver-memory stored and read back the number `42` + * while a TEXT column hands back a string: one write, two stored shapes, and + * an equality filter or a sort that differs by driver. This writes the string + * the arm judged, on the same fields the numeric half reads (the validator's + * walk, `readonly` included). A single write is admitted or refused exactly as + * before; what changes is the stored form, and with it every answer that reads + * the stored value. A `unique` text field's key now compares the string, so + * `42` after `"42"` is refused as a duplicate (and on SQLite, which stored + * `true` and `1` both as `'1.0'`, those two no longer collide), and a filter + * comparing the field to the number `42` no longer matches a row written as + * `42`. + * + * ⛔ Not touched: a string (judged and written as itself), `null` and a missing + * key, a plain object or an array (the arm refuses them, #29133), every other + * non-scalar, and every field outside the set — `json` and the structured + * types, the number and boolean fields, select and the reference types. + * * Same contract as {@link normalizeBlankTypedValues}: one record or an array of * them, pure — the same reference comes back when nothing changed, else a * shallow copy (per row, and a copied array). @@ -970,17 +994,33 @@ function normalizeNumericStringRow(fields: Record, row: unknow if (!isPlainRecord(row)) return row; let out: Record | undefined; for (const [name, value] of Object.entries(row)) { - if (typeof value !== 'string') continue; + const isString = typeof value === 'string'; + if (!isString && !isStringReadScalar(value)) continue; // Own-property: a field name may be `constructor` / `valueOf`. const def = Object.prototype.hasOwnProperty.call(fields, name) ? fields[name] : undefined; - if (!def || !isJudgedNumberType(def.type) || !isInReadonlyScope(name, def, 'include')) continue; - const n = parseNumericString(value); - if (n === undefined) continue; - (out ??= { ...row })[name] = n; + if (!def || !isInReadonlyScope(name, def, 'include')) continue; + if (isString) { + if (!isJudgedNumberType(def.type)) continue; + const n = parseNumericString(value); + if (n === undefined) continue; + (out ??= { ...row })[name] = n; + } else if (BOUNDED_STRING_FIELD_TYPES.has(def.type)) { + // [#29241] The string arm's reading of a number or a boolean. + (out ??= { ...row })[name] = String(value); + } } return out ?? row; } +/** + * [#29241] A number or a boolean: the scalars the record validator's string arm + * judges by their string reading (`String(value)`), so the door writes that + * string into a text-family field ({@link normalizeNumericStringValues}). + */ +function isStringReadScalar(value: unknown): value is number | boolean { + return typeof value === 'number' || typeof value === 'boolean'; +} + /** * Coerce `boolean`-typed fields from their SQL storage form (integer `0`/`1`, * or the strings `'0'`/`'1'`/`'true'`/`'false'`) into real JS booleans, on a @@ -1225,7 +1265,9 @@ function validateOne( // stored value for this class and neither has a string reading the author // meant, so the arm refuses it with the number arm's #20309 posture: never // silently alter a value. It is a SHAPE test, so it runs ahead of - // `shapeOnly`. A number or a boolean keeps its string reading; a + // `shapeOnly`. A number or a boolean keeps its string reading, and + // [#29241] `normalizeNumericStringValues` has already written that string + // into the payload at the door, so the driver stores what was judged; a // filter-operator object was already refused above, with its own sentence. if (isPlainObjectValue(value) || Array.isArray(value)) { return fail( diff --git a/packages/rest/src/rest-data-text-scalar-value.test.ts b/packages/rest/src/rest-data-text-scalar-value.test.ts new file mode 100644 index 00000000000..c892e1c0515 --- /dev/null +++ b/packages/rest/src/rest-data-text-scalar-value.test.ts @@ -0,0 +1,177 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#29241] A number or a boolean sent to a text-family field, through the REST + * write doors, on a real engine (`ObjectQL` + sqlite `SqlDriver`) and the real + * `RestServer` routes (the harness `rest-data-blank-typed-value.test.ts` + * boots). + * + * The record validator's string arm judges such a value by its string reading + * (`42` as `'42'`, `true` as `'true'`), and the write door now stores that + * string, so the physical TEXT column, the engine's `findOne` and `GET …/:id` + * all hand back the string. The PHYSICAL column is read with the driver's own + * query builder, past every engine read coercion. + * + * Controls: a string is stored as sent, and a number field keeps its number. + * + * The unique key compares what is stored, so the accept set of a `unique` + * text field moves (Clause-② yes, narrowing), measured on `origin/main` + * @ `7f277a86` (the file at base) and on this change, SQLite here and driver-memory beside it: + * + * sequence on a unique text field base (SQLite) base (memory) head (both) + * 42, then '42' 201, 201 201, 201 201, 409 + * true, then 'true' 201, 201 201, 201 201, 409 + * true, then 1 201, 409 201, 201 201, 201 + * false, then 0 201, 409 201, 201 201, 201 + * + * The first two rows are the narrowing: the validator reads both values as one + * string, and now the unique key does too. The last two are SQLite's widening: + * base stored `true` and `1` both as `'1.0'`. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { RestServer } from './rest-server'; + +const OBJ = { + name: 'scalar_rest', label: 'Scalar', systemFields: false, + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + title: { name: 'title', type: 'text' as const }, + note: { name: 'note', type: 'textarea' as const }, + amount: { name: 'amount', type: 'number' as const }, + code: { name: 'code', type: 'text' as const, unique: true }, + }, +}; + +const liveEngines: ObjectQL[] = []; +afterEach(async () => { + while (liveEngines.length) { + try { await liveEngines.pop()?.destroy(); } catch { /* noop */ } + } +}); + +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; +} + +async function boot() { + const engine = new ObjectQL(); + liveEngines.push(engine); + const driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }); + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject(OBJ as any); + await engine.syncSchemas(); + + const protocol = new ObjectStackProtocolImplementation(engine as any); + const rest = new RestServer(createMockServer() as any, protocol as any, { + api: { requireAuth: false }, batch: { enableBatchEndpoint: true }, + } as any); + (rest as any).resolveExecCtx = async () => ({ userId: 'test-user' }); + rest.registerRoutes(); + const call = async (method: string, path: string, params: Record, body: unknown) => { + const route = rest.getRoutes().find((r: any) => r.method === method && r.path === path); + expect(route, `${method} ${path}`).toBeDefined(); + const res = makeRes(); + await route!.handler({ params, body, query: {}, headers: {} } as any, res); + return { status: res._status ?? 200, body: res._json }; + }; + /** The physical row, read past every engine read coercion. */ + const stored = async (id: string) => + (await (driver as any).knex('scalar_rest').where({ id }).first()) as Record | undefined; + /** The physical row, the engine's read and `GET …/:id`, for one id. */ + const threeReads = async (id: string) => { + const g = await call('GET', '/api/v1/data/:object/:id', { object: 'scalar_rest', id }, undefined); + expect(g.status, JSON.stringify(g.body)).toBe(200); + return { + stored: await stored(id), + engine: (await engine.findOne('scalar_rest', { where: { id } })) as Record | null, + rest: (g.body.record ?? g.body) as Record, + }; + }; + return { engine, call, threeReads }; +} + +function expectStrings(reads: Record | null | undefined>, want: Record) { + for (const [where, row] of Object.entries(reads)) { + expect(row, where).toBeTruthy(); + for (const [k, v] of Object.entries(want)) expect(row![k], `${where} · ${k}`).toBe(v); + } +} + +describe('REST write doors on SQLite: a number or boolean on a text field is stored as its string (#29241)', () => { + let ctx: Awaited>; + beforeEach(async () => { ctx = await boot(); }); + + it('POST create: 42 and true are stored and read back as "42" and "true"', async () => { + const res = await ctx.call('POST', '/api/v1/data/:object', { object: 'scalar_rest' }, { id: 'c1', title: 42, note: true, amount: 7 }); + expect(res.status, JSON.stringify(res.body)).toBe(201); + const reads = await ctx.threeReads('c1'); + expectStrings(reads, { title: '42', note: 'true' }); + // Control: a number field keeps its number. + expect(reads.stored!.amount).toBe(7); + }); + + it('PATCH update: false and 0 are stored as "false" and "0"', async () => { + await ctx.engine.insert('scalar_rest', { id: 'u1', title: 'orig', note: 'orig' }); + const res = await ctx.call('PATCH', '/api/v1/data/:object/:id', { object: 'scalar_rest', id: 'u1' }, { title: 0, note: false }); + expect(res.status, JSON.stringify(res.body)).toBe(200); + expectStrings(await ctx.threeReads('u1'), { title: '0', note: 'false' }); + }); + + it('batch create: the judged string is stored', async () => { + const res = await ctx.call('POST', '/api/v1/data/:object/batch', { object: 'scalar_rest' }, + { operation: 'create', records: [{ data: { id: 'b1', title: 1.5, note: true } }] }); + expect(res.status).toBe(200); + expect(res.body.results.map((r: any) => r.success)).toEqual([true]); + expectStrings(await ctx.threeReads('b1'), { title: '1.5', note: 'true' }); + }); + + it('control: a string is stored as sent', async () => { + const res = await ctx.call('POST', '/api/v1/data/:object', { object: 'scalar_rest' }, { id: 's1', title: '42', note: 'true' }); + expect(res.status).toBe(201); + expectStrings(await ctx.threeReads('s1'), { title: '42', note: 'true' }); + }); +}); + +describe('a unique text field compares the stored string (#29241, Clause-② narrowing)', () => { + let ctx: Awaited>; + beforeEach(async () => { ctx = await boot(); }); + + const post = (id: string, code: unknown) => + ctx.call('POST', '/api/v1/data/:object', { object: 'scalar_rest' }, { id, code }); + + for (const [first, second] of [[42, '42'], [true, 'true']] as const) { + it(`${JSON.stringify(first)}, then ${JSON.stringify(second)}: the second is refused 409 UNIQUE_VIOLATION (base stored both)`, async () => { + expect((await post('a', first)).status).toBe(201); + const res = await post('b', second); + expect(res.status, JSON.stringify(res.body)).toBe(409); + expect(res.body?.error?.code ?? res.body?.code).toBe('UNIQUE_VIOLATION'); + expect((await ctx.threeReads('a')).stored!.code).toBe(String(first)); + }); + } + + for (const [first, second] of [[true, 1], [false, 0]] as const) { + it(`${JSON.stringify(first)}, then ${JSON.stringify(second)}: both land, as two strings (base refused the second on SQLite)`, async () => { + expect((await post('a', first)).status).toBe(201); + const res = await post('b', second); + expect(res.status, JSON.stringify(res.body)).toBe(201); + expect((await ctx.threeReads('a')).stored!.code).toBe(String(first)); + expect((await ctx.threeReads('b')).stored!.code).toBe(String(second)); + }); + } +});