diff --git a/.changeset/25073-autonumber-stored-expression.md b/.changeset/25073-autonumber-stored-expression.md new file mode 100644 index 00000000000..940c9cd222f --- /dev/null +++ b/.changeset/25073-autonumber-stored-expression.md @@ -0,0 +1,20 @@ +--- +'@objectstack/spec': patch +'@objectstack/objectql': patch +'@objectstack/driver-sql': patch +'@objectstack/driver-turso': patch +'@objectstack/plugin-pinyin-search': patch +--- + +fix(objectql, driver-sql, spec): a stored field whose `expression` reads an `autonumber` stores the value computed with the number + +Clause-②: yes (widening) +Metadata-protocol: none + +A stored (non-`formula`) field whose `expression` reads a record number, such as `record.case_number + " - " + record.subject`, was computed before the number existed. On insert it stored `null` (or the `has()` fallback) until a later update by id computed it again. Now the insert computes it again right after the number is issued, and writes the row once, carrying both. + +- **`IDataDriver.issueAutonumbers` (new, optional).** A driver that owns its record-number counter can issue a row's numbers ahead of the write, in place, and answer after a unique violation whether the collision was on a number it issued (re-seeding the counter and emptying those slots). `SqlDriver` implements it over its existing counter, so `driver-sqlite-wasm` and `driver-turso` (both faces) inherit it. A driver without it is unchanged. +- **The engine.** For an object whose stored expression reads a record number (directly or through another stored field), the engine asks the driver for the number after validation, where it already issued numbers itself, so a rejected row still burns none. It computes the readers again and hands `create()` / `bulkCreate()` the row with the number filled: one INSERT, no transaction. A collision on a single row is re-issued through the member by the engine's existing retry loop, which computes the readers again. A batch collision re-seeds the counter and is not re-issued (the batch policy the engine already had). Objects with no such field keep the driver's own issue-at-write path. Where the engine issues the number itself (driver-memory, driver-mongodb), the readers are computed again after it does. +- **A driver that owns its counter without the member** stores the readers computed without the number, as before, and the engine now says so once per object at `error` level, with the remedy. +- **`ObjectQL.registerInsertRecomputeListener(id, fn)` (new).** Called per row after that second computation, on the row the driver is handed. `plugin-pinyin-search` registers on it, so a `__search` companion whose name field reads the record number is stamped from the value that carries the number, and `$search` finds the record by pinyin and by its number. +- **One change in what a refused insert reports.** On an object whose stored expression reads the number, a SQL driver's collision that survives every retry now leaves as `ERR_AUTONUMBER_COLLISION` (the engine's three attempts) instead of `DUPLICATE_RECORD` (the driver's own retries). Every other object is unchanged. diff --git a/packages/drivers/driver-sql/src/sql-driver-autonumber-issue-ahead.test.ts b/packages/drivers/driver-sql/src/sql-driver-autonumber-issue-ahead.test.ts new file mode 100644 index 00000000000..a61296df0ba --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-autonumber-issue-ahead.test.ts @@ -0,0 +1,215 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#25073] `SqlDriver.issueAutonumbers` — the record number on request, ahead + * of the write. + * + * `create()` / `bulkCreate()` fill an empty `autonumber` slot as the last step + * before the INSERT, so a stored field whose `expression` reads the number was + * computed by the engine before the number existed (null for a concatenation, + * the `has()` fallback otherwise). The member issues the same number earlier, + * on the caller's row, and `create()` then finds the slot filled and writes + * the row as it stands: one statement, carrying the number the engine computed + * from. + * + * What is pinned here is the member's own half, on a real SQLite database: + * + * 1. ISSUE fills what `create()` would have filled (same counter, same + * organization scope), leaves a typed value alone, and `create()` of the + * filled row draws no second number. + * 2. AFTER A COLLISION it is `create()`'s #5495 re-seed split at the point + * only the caller can go on: it judges the error as `create()` does, + * re-seeds the stale counter once and EMPTIES the slot, so the caller's + * next issue lands on MAX + 1. It never answers for a duplicate on a + * value the caller typed, for a non-unique error, or inside a caller's + * transaction. + * 3. A batch: every row that drew from the stale counter is emptied (#6943's + * per-counter reading), the counter re-seeded once. + * + * The engine's half (issue after validation, recompute, retry) is pinned in + * `packages/runtime/src/autonumber-stored-expression.integration.test.ts`. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { isUniqueViolationError } from '@objectstack/types'; +import { SqlDriver } from './index.js'; + +const SEQUENCES_TABLE = '_objectstack_sequences'; + +const CRM_CASE = { + name: 'crm_case', + fields: { + organization_id: { type: 'string' }, + case_number: { type: 'autonumber', format: 'CASE-{00000}', unique: true }, + title: { type: 'string' }, + }, +} as any; + +describe('[#25073] SqlDriver.issueAutonumbers', () => { + let driver: SqlDriver; + const knex = () => (driver as any).knex; + + const lastValue = async (tenant: string): Promise => { + const rows = await knex()(SEQUENCES_TABLE).select('*'); + const row = rows.find((r: any) => r.object === 'crm_case' && r.field === 'case_number' && r.tenant_id === tenant); + return row ? Number(row.last_value) : undefined; + }; + + /** Land rows by a path that bypasses the counter, as a seed replay does. */ + const bypassInsert = async (org: string, from: number, to: number) => { + const rows = []; + for (let n = from; n <= to; n++) { + rows.push({ id: `${org}-s${n}`, organization_id: org, case_number: `CASE-${String(n).padStart(5, '0')}`, title: `seed ${n}` }); + } + await knex()('crm_case').insert(rows); + }; + + /** The error a write raises, or a failure when it lands. */ + const refusal = async (write: Promise): Promise => { + try { + await write; + } catch (error) { + return error; + } + throw new Error('expected the write to be refused'); + }; + + beforeEach(async () => { + driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }); + await driver.initObjects([CRM_CASE]); + }); + + afterEach(async () => { + await driver.disconnect(); + }); + + describe('issue', () => { + it('fills the empty slot in place, from the counter create() draws from, and create() draws no second number', async () => { + await driver.create('crm_case', { organization_id: 'orgA', title: 'by create' }); + expect(await lastValue('orgA')).toBe(1); + + const row: Record = { organization_id: 'orgA', title: 'ahead' }; + expect(await driver.issueAutonumbers('crm_case', [row])).toEqual([['case_number']]); + expect(row.case_number).toBe('CASE-00002'); + expect(await lastValue('orgA')).toBe(2); + + const stored = await driver.create('crm_case', row); + expect(stored.case_number).toBe('CASE-00002'); + expect(await lastValue('orgA')).toBe(2); + // And the counter simply goes on from there. + expect((await driver.create('crm_case', { organization_id: 'orgA', title: 'next' })).case_number).toBe('CASE-00003'); + }); + + it('issues per organization from the call options, as create() stamps the organization first', async () => { + await driver.create('crm_case', { organization_id: 'orgA', title: 'a1' }); + const row: Record = { title: 'b, organization from options' }; + expect(await driver.issueAutonumbers('crm_case', [row], { tenantId: 'orgB' } as any)).toEqual([['case_number']]); + expect(row.case_number).toBe('CASE-00001'); + expect(await lastValue('orgB')).toBe(1); + expect(await lastValue('orgA')).toBe(1); + // Only the number lands on the caller's row; `create()` stamps the organization. + expect(row).not.toHaveProperty('organization_id'); + expect((await driver.create('crm_case', row, { tenantId: 'orgB' } as any)).organization_id).toBe('orgB'); + }); + + it('leaves a typed value alone and does not report it', async () => { + const row: Record = { organization_id: 'orgA', case_number: 'CASE-07777', title: 'import' }; + expect(await driver.issueAutonumbers('crm_case', [row])).toEqual([[]]); + expect(row.case_number).toBe('CASE-07777'); + expect(await lastValue('orgA')).toBeUndefined(); + }); + + it('answers index-aligned with the rows of a batch', async () => { + const rows: Record[] = [ + { organization_id: 'orgA', title: 'one' }, + { organization_id: 'orgA', case_number: 'CASE-00500', title: 'typed' }, + { organization_id: 'orgA', title: 'two' }, + ]; + expect(await driver.issueAutonumbers('crm_case', rows)).toEqual([['case_number'], [], ['case_number']]); + expect(rows.map((r) => r.case_number)).toEqual(['CASE-00001', 'CASE-00500', 'CASE-00002']); + const stored = await driver.bulkCreate('crm_case', rows); + expect(stored.map((r) => r.case_number)).toEqual(['CASE-00001', 'CASE-00500', 'CASE-00002']); + }); + }); + + describe('after a collision', () => { + it('re-seeds the counter it proves stale and empties the slot, so the next issue lands on MAX + 1', async () => { + await bypassInsert('orgA', 9, 9); + await driver.create('crm_case', { organization_id: 'orgA', title: 'first' }); + expect(await lastValue('orgA')).toBe(10); + await bypassInsert('orgA', 11, 39); + + const row: Record = { organization_id: 'orgA', title: 'after the seeds' }; + await driver.issueAutonumbers('crm_case', [row]); + expect(row.case_number).toBe('CASE-00011'); + const error = await refusal(driver.create('crm_case', row)); + expect(isUniqueViolationError(error)).toBe(true); + + expect(await driver.issueAutonumbers('crm_case', [row], undefined, error)).toEqual([['case_number']]); + expect(row).not.toHaveProperty('case_number'); + expect(await lastValue('orgA')).toBe(39); + + expect(await driver.issueAutonumbers('crm_case', [row])).toEqual([['case_number']]); + expect(row.case_number).toBe('CASE-00040'); + expect((await driver.create('crm_case', row)).case_number).toBe('CASE-00040'); + }); + + it('answers nothing, and changes nothing, for a duplicate on a value the caller typed', async () => { + await driver.create('crm_case', { organization_id: 'orgA', case_number: 'CASE-00001', title: 'typed' }); + const row: Record = { organization_id: 'orgA', case_number: 'CASE-00001', title: 'typed again' }; + expect(await driver.issueAutonumbers('crm_case', [row])).toEqual([[]]); + const error = await refusal(driver.create('crm_case', row)); + expect(isUniqueViolationError(error)).toBe(true); + + expect(await driver.issueAutonumbers('crm_case', [row], undefined, error)).toEqual([[]]); + expect(row.case_number).toBe('CASE-00001'); + }); + + it('answers nothing for an error that is not a unique violation', async () => { + const row: Record = { organization_id: 'orgA', title: 'x' }; + await driver.issueAutonumbers('crm_case', [row]); + const issued = row.case_number; + expect(await driver.issueAutonumbers('crm_case', [row], undefined, new Error('SQLITE_BUSY: database is locked'))).toEqual([[]]); + expect(row.case_number).toBe(issued); + }); + + it('answers nothing inside a caller transaction, where create() retries nothing either', async () => { + await driver.create('crm_case', { organization_id: 'orgA', title: 'seeds the counter at 1' }); + await bypassInsert('orgA', 2, 5); + const row: Record = { organization_id: 'orgA', title: 'in a transaction' }; + await driver.issueAutonumbers('crm_case', [row]); + const error = await refusal(driver.create('crm_case', row)); + expect(isUniqueViolationError(error)).toBe(true); + const trx = await knex().transaction(); + try { + expect(await driver.issueAutonumbers('crm_case', [row], { transaction: trx } as any, error)).toEqual([[]]); + } finally { + await trx.rollback(); + } + expect(row.case_number).toBe('CASE-00002'); + expect(await lastValue('orgA')).toBe(2); + }); + + it('empties every row of a batch that drew from the stale counter, and re-seeds it once', async () => { + await bypassInsert('orgA', 1, 1); + await driver.create('crm_case', { organization_id: 'orgA', title: 'seeds the counter at 2' }); + await bypassInsert('orgA', 3, 3); + + const rows: Record[] = [ + { organization_id: 'orgA', title: 'collides on 3' }, + { organization_id: 'orgA', title: 'would be 4, did not collide' }, + { organization_id: 'orgB', title: 'another counter' }, + ]; + await driver.issueAutonumbers('crm_case', rows); + expect(rows.map((r) => r.case_number)).toEqual(['CASE-00003', 'CASE-00004', 'CASE-00001']); + const error = await refusal(driver.bulkCreate('crm_case', rows)); + + expect(await driver.issueAutonumbers('crm_case', rows, undefined, error)).toEqual([['case_number'], ['case_number'], []]); + expect(rows[0]).not.toHaveProperty('case_number'); + expect(rows[1]).not.toHaveProperty('case_number'); + expect(rows[2].case_number).toBe('CASE-00001'); + expect(await lastValue('orgA')).toBe(4); + expect(await lastValue('orgB')).toBe(1); + }); + }); +}); diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index 24448296bf2..d95df966784 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -8489,6 +8489,98 @@ export class SqlDriver implements IDataDriver { return reservations; } + /** + * The reservations {@link issueAutonumbers} made, keyed by the caller's row + * object, so its collision call can tell a number it issued from one the + * caller typed — the distinction `create()` draws from its own local + * `reservations`. Weak, so a row the caller drops takes its entry with it. + */ + private readonly issuedAutoNumbers = new WeakMap(); + + /** + * [#25073] `IDataDriver.issueAutonumbers`: {@link fillAutoNumberFields} on + * request, ahead of the write, so the engine can compute a stored field + * whose `expression` reads the number before it hands the row to + * {@link create} / {@link bulkCreate}. Those find the slot filled and + * reserve nothing, so the row goes in as ONE statement carrying the number + * the engine computed from. + * + * After a collision it is {@link create}'s and {@link bulkCreate}'s own + * re-seed (#5495, #6943), split at the point where only the caller can go + * on: which numbers collided is decided as there + * ({@link collidingAutoNumberReservations}, so a duplicate on a value the + * caller typed is never this counter's), each stale counter is re-seeded + * ONCE, and every slot drawn from a stale counter is emptied — on a batch + * that includes rows whose own value did not collide, for the reason + * {@link bulkCreate} gives. The caller issues again; this method never + * re-issues on its own, because the caller has to recompute what reads + * the number. Inside a caller's transaction nothing is retried, as + * {@link create} retries nothing there (Postgres has aborted the handle). + * + * Inherited by `driver-sqlite-wasm` and `driver-turso`: the remote face + * issues through its own `getNextSequenceValue` and probes through its own + * `autoNumberValueExists` / `resyncSequenceToDataMax`, the overrides its + * `create()` already calls. + */ + async issueAutonumbers( + object: string, + rows: Record[], + options?: DriverOptions, + afterCollision?: unknown, + ): Promise { + if (afterCollision === undefined) { + const filled: string[][] = []; + for (const row of rows) { + if (!row || typeof row !== 'object') { + filled.push([]); + continue; + } + // Filled as `create()` fills it: the caller's organization stamped + // first (numbers are issued per organization), on a view, so only + // the numbers land on the caller's row. + const view: Record = { ...row }; + this.injectTenantOnInsert(object, view, options); + const reserved = await this.fillAutoNumberFields(object, view, options); + for (const r of reserved) row[r.field] = view[r.field]; + // A value kept from an earlier issue on this row is still this + // driver's, so it stays described beside the fresh ones (#6943). + const fresh = new Set(reserved.map((r) => r.field)); + const kept = (this.issuedAutoNumbers.get(row) ?? []).filter((r) => !fresh.has(r.field)); + this.issuedAutoNumbers.set(row, [...kept, ...reserved]); + filled.push(reserved.map((r) => r.field)); + } + return filled; + } + + const none = rows.map((): string[] => []); + if (options?.transaction !== undefined) return none; + const issued = rows.map((row) => (row && typeof row === 'object' ? this.issuedAutoNumbers.get(row) ?? [] : [])); + const colliding = await this.collidingAutoNumberReservations(afterCollision, issued.flat(), options); + if (colliding.length === 0) return none; + + const stale = new Set(); + for (const reservation of colliding) { + const key = this.autoNumberCounterKey(reservation); + if (stale.has(key)) continue; + stale.add(key); + await this.resyncSequenceToDataMax(reservation); + } + return rows.map((row, i) => { + const emptied: string[] = []; + const kept: AutoNumberReservation[] = []; + for (const reservation of issued[i]) { + if (stale.has(this.autoNumberCounterKey(reservation))) { + delete row[reservation.field]; + emptied.push(reservation.field); + } else { + kept.push(reservation); + } + } + if (emptied.length > 0) this.issuedAutoNumbers.set(row, kept); + return emptied; + }); + } + /** * Does a row already hold `reservation.value` in the partition this * counter covers? (#5495) diff --git a/packages/drivers/driver-turso/src/turso-driver.ts b/packages/drivers/driver-turso/src/turso-driver.ts index ad933a4d535..ac6da0f3a12 100644 --- a/packages/drivers/driver-turso/src/turso-driver.ts +++ b/packages/drivers/driver-turso/src/turso-driver.ts @@ -1051,6 +1051,11 @@ export const REMOTE_FACE_ANSWERS = { // Deprecated aliases: they call `this.commit` / `this.rollback`, which refuse. commitTransaction: 'inherited', rollbackTransaction: 'inherited', + // [#25073] Record numbers ahead of the write: `fillAutoNumberFields` and the + // collision re-seed, which already reach the remote database through this + // face's own `getNextSequenceValue`, `autoNumberValueExists` and + // `resyncSequenceToDataMax` (see "Autonumber on the remote face" above). + issueAutonumbers: 'inherited', // Knex-only reads. findWithWindowFunctions: 'refused', analyzeQuery: 'refused', diff --git a/packages/drivers/driver-turso/src/turso-remote-autonumber-issue-ahead.test.ts b/packages/drivers/driver-turso/src/turso-remote-autonumber-issue-ahead.test.ts new file mode 100644 index 00000000000..d18da4efdaf --- /dev/null +++ b/packages/drivers/driver-turso/src/turso-remote-autonumber-issue-ahead.test.ts @@ -0,0 +1,122 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#25073] `issueAutonumbers` on the Turso REMOTE face: INHERITED from + * `SqlDriver`, and it reaches the remote database. + * + * The member is `fillAutoNumberFields` on request plus `create()`'s #5495 + * re-seed, split where the engine must go on. Every piece of state it touches + * already goes through this face's own overrides: `getNextSequenceValue` + * (the remote counter statement, #21113), `autoNumberValueExists` and + * `resyncSequenceToDataMax` (the remote re-seed). So the face needs no arm of + * its own, and `REMOTE_FACE_ANSWERS` names it `inherited` — which + * `turso-remote-inherited-members.test.ts` holds to the code (no redeclaration + * on `TursoDriver`). + * + * Measured here on a real SQLite wearing the `@libsql/client` interface: + * the number is issued from the same persistent counter the remote `create()` + * draws from, `create()` writes the issued row through unchanged, and a + * collision on an issued number re-seeds the remote counter and empties the + * slot, so the next issue lands on MAX + 1. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { TursoDriver } from './index.js'; +import { makeLibsqlSqliteStub, type LibsqlSqliteStub } from './libsql-sqlite-stub.testkit.js'; + +const NUMBERED_OBJECT = { + name: 'crm_case', + fields: { + organization_id: { type: 'string' }, + case_number: { type: 'autonumber', format: 'CASE-{00000}', unique: true }, + title: { type: 'string' }, + }, +}; + +const open: TursoDriver[] = []; +afterEach(async () => { + while (open.length) await open.pop()!.disconnect().catch(() => {}); +}); + +async function makeRemote() { + const stub = makeLibsqlSqliteStub(); + const driver = new TursoDriver({ url: 'libsql://probe.turso.io', client: stub as never }); + open.push(driver); + await driver.connect(); + expect(driver.transportMode).toBe('remote'); + await driver.initObjects([NUMBERED_OBJECT] as any); + return { driver, stub }; +} + +const rowCount = (stub: LibsqlSqliteStub, where = '1=1', args: unknown[] = []) => + (stub.raw.prepare(`select count(*) as c from "crm_case" where ${where}`).all(...args) as Array<{ c: number }>)[0].c; + +const lastValue = (stub: LibsqlSqliteStub): number => + Number((stub.raw.prepare('select last_value from "_objectstack_sequences"').all() as Array<{ last_value: number }>)[0].last_value); + +const landByBypass = (stub: LibsqlSqliteStub, from: number, to: number) => { + const insert = stub.raw.prepare( + 'insert into "crm_case" ("id", "organization_id", "case_number", "title") values (?, ?, ?, ?)', + ); + for (let n = from; n <= to; n++) insert.run(`bypass-${n}`, 'orgA', `CASE-${String(n).padStart(5, '0')}`, `row ${n}`); +}; + +const refusal = (write: Promise) => + write.then( + (row) => { throw new Error(`expected a refusal, got ${JSON.stringify(row)}`); }, + (e) => e as Error, + ); + +describe('[#25073] REMOTE: issueAutonumbers, inherited', () => { + it('is inherited, not redeclared, on TursoDriver', () => { + expect(Object.prototype.hasOwnProperty.call(TursoDriver.prototype, 'issueAutonumbers')).toBe(false); + expect(typeof TursoDriver.prototype.issueAutonumbers).toBe('function'); + }); + + it('issues from the counter the remote create() draws from, and create() writes the row as issued', async () => { + const { driver, stub } = await makeRemote(); + await driver.create('crm_case', { organization_id: 'orgA', title: 'by create' }); + expect(lastValue(stub)).toBe(1); + + const row: Record = { organization_id: 'orgA', title: 'ahead' }; + expect(await driver.issueAutonumbers('crm_case', [row])).toEqual([['case_number']]); + expect(row.case_number).toBe('CASE-00002'); + expect(lastValue(stub)).toBe(2); + + const written = await driver.create('crm_case', row); + expect(written.case_number).toBe('CASE-00002'); + expect(lastValue(stub)).toBe(2); + expect(rowCount(stub, '"case_number" = ?', ['CASE-00002'])).toBe(1); + }); + + it('after a collision on an issued number, re-seeds the remote counter and empties the slot', async () => { + const { driver, stub } = await makeRemote(); + await driver.create('crm_case', { organization_id: 'orgA', title: 'first' }); + landByBypass(stub, 2, 30); + + const row: Record = { organization_id: 'orgA', title: 'after the replay' }; + await driver.issueAutonumbers('crm_case', [row]); + expect(row.case_number).toBe('CASE-00002'); + const error = await refusal(driver.create('crm_case', row)); + expect(error.message).toMatch(/UNIQUE constraint failed/i); + + expect(await driver.issueAutonumbers('crm_case', [row], undefined, error)).toEqual([['case_number']]); + expect(row).not.toHaveProperty('case_number'); + expect(lastValue(stub)).toBe(30); + + await driver.issueAutonumbers('crm_case', [row]); + expect(row.case_number).toBe('CASE-00031'); + expect((await driver.create('crm_case', row)).case_number).toBe('CASE-00031'); + expect(rowCount(stub)).toBe(31); + }); + + it('answers nothing for a duplicate on a value the caller typed', async () => { + const { driver } = await makeRemote(); + await driver.create('crm_case', { organization_id: 'orgA', title: 'typed', case_number: 'CASE-00500' }); + const row: Record = { organization_id: 'orgA', title: 'dup', case_number: 'CASE-00500' }; + expect(await driver.issueAutonumbers('crm_case', [row])).toEqual([[]]); + const error = await refusal(driver.create('crm_case', row)); + expect(await driver.issueAutonumbers('crm_case', [row], undefined, error)).toEqual([[]]); + expect(row.case_number).toBe('CASE-00500'); + }); +}); diff --git a/packages/objectql/src/engine-autonumber-stored-expression.test.ts b/packages/objectql/src/engine-autonumber-stored-expression.test.ts new file mode 100644 index 00000000000..0519f744d50 --- /dev/null +++ b/packages/objectql/src/engine-autonumber-stored-expression.test.ts @@ -0,0 +1,415 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #25073 — a stored field whose `expression` reads an `autonumber` stores the + * value computed WITH the number, on every insert path. + * + * Before this card the stored-expression passes ran before the number + * existed: before `beforeInsert` and again before validation, while the + * number was issued after validation (the engine's fallback counter) or inside + * the driver's `create()` (a driver that owns its counter). So + * `record.case_number + " - " + record.subject` stored `null`, and + * `has(record.product_code) ? … : record.name` stored its fallback, until a + * later update by id computed it again. Measured at the base on driver-memory + * and on SQLite before the fix (#25073's probe): `display_title: null`, + * `title: "Probe product"`. + * + * Now the insert computes those fields again right after it issues the + * number, on the row it hands the driver, so the write is still ONE statement + * carrying both. Two issuers, both pinned here over hand-rolled drivers: + * + * - **engine** — a driver that leaves the number to the engine + * (`supports = {}`): `applyAutonumbers` issues, the readers are computed + * again, and the #6806 collision loop computes them again after each + * re-issue. + * - **driver** — a driver that owns its counter (`supports.autonumber`) and + * implements `IDataDriver.issueAutonumbers`: for an object a stored + * expression reads a number on, the engine asks the member for the number + * at the same point, and the collision loop re-issues through it. An + * object no stored expression reads a number on keeps the driver's own + * issue-at-write (`issueAutonumbers` is never called for it). + * + * The SQL drivers' real member is pinned in driver-sql's + * `sql-driver-autonumber-issue-ahead.test.ts`, and the whole path over a real + * SQLite database in runtime's `autonumber-stored-expression.integration.test.ts`. + * + * NOTE this file imports no driver package (hand-rolled drivers below), so it + * adds no `driver-memory` consumer for `check:driver-memory-census`. + */ + +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { ObjectQL } from './engine.js'; + +type Row = Record; + +const CASE = { + name: 'acase', + fields: { + subject: { type: 'text' }, + case_number: { type: 'autonumber', format: 'CASE-{00000}', unique: true }, + display_title: { type: 'text', expression: { dialect: 'cel', source: 'record.case_number + " - " + record.subject' } }, + // Reads the number only through `display_title`. + headline: { type: 'text', expression: { dialect: 'cel', source: 'record.display_title + "!"' } }, + // Control: reads no record number. + plain_title: { type: 'text', expression: { dialect: 'cel', source: '"S:" + record.subject' } }, + must: { type: 'text', required: true }, + }, +}; + +const PRODUCT = { + name: 'aproduct', + fields: { + name: { type: 'text' }, + product_code: { type: 'autonumber', format: 'PRD-{0000}' }, + title: { type: 'text', expression: { dialect: 'cel', source: 'has(record.product_code) ? record.product_code + " " + record.name : record.name' } }, + }, +}; + +/** Control object: an autonumber that no stored expression reads. */ +const NOTE = { + name: 'anote', + fields: { + name: { type: 'text' }, + code: { type: 'autonumber', format: 'N-{000}' }, + summary: { type: 'text', expression: { dialect: 'cel', source: '"N:" + record.name' } }, + }, +}; + +/** `prefix`, zero-padded width — the formats above, as a driver that owns its counter renders them. */ +const AUTONUMBERS: Record> = { + acase: { case_number: ['CASE-', 5] }, + aproduct: { product_code: ['PRD-', 4] }, + anote: { code: ['N-', 3] }, +}; + +function cond(value: unknown, c: unknown): boolean { + if (c === null || typeof c !== 'object' || Array.isArray(c) || c instanceof Date) return value === c; + return Object.entries(c as Record).every(([op, t]) => { + switch (op) { + case '$eq': return value === t; + case '$ne': return value !== t; + case '$in': return Array.isArray(t) && t.includes(value); + case '$startsWith': return typeof value === 'string' && value.startsWith(String(t)); + case '$gt': return value != null && String(value) > String(t); + default: throw new Error(`fake driver: unsupported operator ${op}`); + } + }); +} + +function matches(row: Row, where: any): boolean { + if (!where || typeof where !== 'object') return true; + return Object.entries(where).every(([k, v]: [string, any]) => { + if (k === '$and') return (v as any[]).every((w) => matches(row, w)); + if (k === '$or') return (v as any[]).some((w) => matches(row, w)); + if (k.startsWith('$')) throw new Error(`fake driver: unsupported logical operator ${k}`); + return cond(row[k], v); + }); +} + +/** Postgres names the conflicting column in DETAIL, which `uniqueViolationColumn` reads. */ +const duplicate = (column: string, value: unknown) => + Object.assign(new Error(`duplicate key value violates unique constraint "${column}_key"`), { + code: '23505', + detail: `Key (${column})=(${String(value)}) already exists.`, + }); + +type Issuer = 'engine' | 'driver'; + +interface DriverOpts { + /** A driver that owns its counter but does not implement the member. */ + withoutMember?: boolean; + /** Refuse every write of a number as a duplicate. */ + alwaysCollide?: boolean; +} + +/** + * A store with unique record numbers. As the `engine` issuer it leaves the + * number to the engine; as the `driver` issuer it owns a counter per field + * (seeded from the store on first use, re-seeded forward after a collision on + * a number it issued) and fills an empty slot inside `create()` / + * `bulkCreate()`, as driver-sql does. + */ +function makeDriver(issuer: Issuer, opts: DriverOpts = {}) { + const stores = new Map(); + const storeFor = (o: string) => { + let s = stores.get(o); + if (!s) { s = []; stores.set(o, s); } + return s; + }; + const counters = new Map(); + const issuedTo = new WeakMap(); + const tail = (value: unknown, prefix: string) => + typeof value === 'string' && value.startsWith(prefix) ? Number(value.slice(prefix.length)) || 0 : 0; + const storedMax = (object: string, field: string) => + Math.max(0, ...storeFor(object).map((r) => tail(r[field], AUTONUMBERS[object][field][0]))); + /** Fill the empty slots on `row` from the counters; the names filled. */ + const fill = (object: string, row: Row): string[] => { + const filled: string[] = []; + for (const [field, [prefix, width]] of Object.entries(AUTONUMBERS[object] ?? {})) { + if (row[field] !== undefined && row[field] !== null && row[field] !== '') continue; + const key = `${object}.${field}`; + const next = (counters.get(key) ?? storedMax(object, field)) + 1; + counters.set(key, next); + row[field] = `${prefix}${String(next).padStart(width, '0')}`; + filled.push(field); + } + return filled; + }; + const refuseDuplicates = (object: string, rows: Row[]) => { + const seen = new Map>(); + for (const row of rows) { + for (const field of Object.keys(AUTONUMBERS[object] ?? {})) { + const value = row[field]; + if (value == null) continue; + const inBatch = seen.get(field) ?? new Set(); + seen.set(field, inBatch); + if (opts.alwaysCollide || inBatch.has(value) || storeFor(object).some((r) => r[field] === value)) { + throw duplicate(field, value); + } + inBatch.add(value); + } + } + }; + let n = 0; + const write = (object: string, data: Row) => { + n += 1; + const row = { ...data, id: data.id ?? `r_${n}` }; + storeFor(object).push(row); + return { ...row }; + }; + const driver: any = { + name: issuer === 'engine' ? 'fallback-fake' : 'counter-fake', + version: '0.0.0', + supports: issuer === 'engine' ? {} : { autonumber: true }, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async find(object: string, ast: any) { + let rows = storeFor(object).filter((r) => matches(r, ast?.where)); + const orderBy = ast?.orderBy; + if (Array.isArray(orderBy) && orderBy.length > 0) { + const { field, order } = orderBy[0]; + rows = [...rows].sort((a, b) => { + const av = String(a[field] ?? ''); + const bv = String(b[field] ?? ''); + const cmp = av < bv ? -1 : av > bv ? 1 : 0; + return order === 'desc' ? -cmp : cmp; + }); + } + if (typeof ast?.limit === 'number') rows = rows.slice(0, ast.limit); + return rows.map((r) => ({ ...r })); + }, + async findOne(object: string, ast: any) { + const row = storeFor(object).find((r) => matches(r, ast?.where)); + return row ? { ...row } : null; + }, + create: vi.fn(async (object: string, data: Row) => { + const row = { ...data }; + if (issuer === 'driver') fill(object, row); + refuseDuplicates(object, [row]); + return write(object, row); + }), + bulkCreate: vi.fn(async (object: string, data: Row[]) => { + const rows = data.map((d) => ({ ...d })); + if (issuer === 'driver') rows.forEach((row) => fill(object, row)); + refuseDuplicates(object, rows); + return rows.map((row) => write(object, row)); + }), + async update(object: string, id: string, data: Row) { + const s = storeFor(object); + const i = s.findIndex((r) => r.id === id); + s[i] = { ...s[i], ...data, id }; + return { ...s[i] }; + }, + async delete() { return true; }, + async count(object: string, ast: any) { return storeFor(object).filter((r) => matches(r, ast?.where)).length; }, + }; + if (issuer === 'driver' && !opts.withoutMember) { + driver.issueAutonumbers = vi.fn(async (object: string, rows: Row[], options?: any, afterCollision?: unknown) => { + if (afterCollision === undefined) { + return rows.map((row) => { + const filled = fill(object, row); + issuedTo.set(row, [...(issuedTo.get(row) ?? []).filter((f) => !filled.includes(f)), ...filled]); + return filled; + }); + } + if (options?.transaction !== undefined) return rows.map(() => []); + // Stale = a number this member issued that the store already holds. + const stale = new Set(); + for (const row of rows) { + for (const field of issuedTo.get(row) ?? []) { + if (opts.alwaysCollide || storeFor(object).some((r) => r[field] === row[field])) stale.add(field); + } + } + for (const field of stale) { + const key = `${object}.${field}`; + counters.set(key, Math.max(counters.get(key) ?? 0, storedMax(object, field))); + } + return rows.map((row) => { + const emptied = (issuedTo.get(row) ?? []).filter((field) => stale.has(field)); + for (const field of emptied) delete row[field]; + issuedTo.set(row, (issuedTo.get(row) ?? []).filter((field) => !stale.has(field))); + return emptied; + }); + }); + } + return { driver, storeFor }; +} + +const SYSTEM = { context: { isSystem: true } } as any; + +async function boot(issuer: Issuer, opts: DriverOpts = {}) { + const engine = new ObjectQL(); + const { driver, storeFor } = makeDriver(issuer, opts); + engine.registerDriver(driver, true); + await engine.init(); + for (const schema of [CASE, PRODUCT, NOTE]) engine.registry.registerObject(schema as any); + return { engine, driver, storeFor }; +} + +const stored = (storeFor: (o: string) => Row[], object: string) => storeFor(object).map((r) => ({ ...r })); + +describe.each(['engine', 'driver'])('a stored expression reading the record number (#25073), %s-issued', (issuer) => { + let engine: ObjectQL; + let driver: any; + let storeFor: (o: string) => Row[]; + + beforeEach(async () => { + ({ engine, driver, storeFor } = await boot(issuer)); + }); + + it('stores the value computed WITH the number on a single insert, and answers it', async () => { + const one: any = await engine.insert('acase', { subject: 'one', must: 'x' }, SYSTEM); + expect(one.case_number).toBe('CASE-00001'); + expect(one.display_title).toBe('CASE-00001 - one'); + // Through another stored field that reads it. + expect(one.headline).toBe('CASE-00001 - one!'); + const [row] = stored(storeFor, 'acase'); + expect(row).toMatchObject({ case_number: 'CASE-00001', display_title: 'CASE-00001 - one', headline: 'CASE-00001 - one!' }); + // One write. + expect(driver.create).toHaveBeenCalledTimes(1); + }); + + it('stores each row\'s own number in a batch, in one bulkCreate', async () => { + await engine.insert('acase', [{ subject: 'b1', must: 'x' }, { subject: 'b2', must: 'x' }], SYSTEM); + expect(stored(storeFor, 'acase').map((r) => r.display_title)).toEqual(['CASE-00001 - b1', 'CASE-00002 - b2']); + expect(driver.bulkCreate).toHaveBeenCalledTimes(1); + }); + + it('numbers only the live rows in partial mode, and computes each from its own number', async () => { + const outcomes: any[] = await engine.insertMany( + 'acase', + [{ subject: 'p1', must: 'x' }, { subject: 'dead' }, { subject: 'p3', must: 'x' }], + SYSTEM, + ); + expect(outcomes.map((o) => o.ok)).toEqual([true, false, true]); + expect(outcomes.filter((o) => o.ok).map((o) => o.record.display_title)).toEqual(['CASE-00001 - p1', 'CASE-00002 - p3']); + expect(stored(storeFor, 'acase').map((r) => r.display_title)).toEqual(['CASE-00001 - p1', 'CASE-00002 - p3']); + }); + + it('takes the has() branch that sees the number', async () => { + const product: any = await engine.insert('aproduct', { name: 'Widget' }, SYSTEM); + expect(product.title).toBe('PRD-0001 Widget'); + expect(stored(storeFor, 'aproduct')[0].title).toBe('PRD-0001 Widget'); + }); + + it('computes again from the re-issued number after a collision', async () => { + await engine.insert('acase', { subject: 'seed', must: 'x' }, SYSTEM); + // A writer this engine cannot observe takes the next number. + storeFor('acase').push({ id: 'oob', subject: 'oob', must: 'x', case_number: 'CASE-00002' }); + const collided: any = await engine.insert('acase', { subject: 'collide', must: 'x' }, SYSTEM); + expect(collided.case_number).toBe('CASE-00003'); + expect(collided.display_title).toBe('CASE-00003 - collide'); + expect(stored(storeFor, 'acase').find((r) => r.subject === 'collide')).toMatchObject({ + case_number: 'CASE-00003', display_title: 'CASE-00003 - collide', headline: 'CASE-00003 - collide!', + }); + }); + + it('tells a registered listener, per row, after each computation, on the row the driver is handed', async () => { + const seen: Array<{ object: string; number: unknown; title: unknown; fields: string[] }> = []; + engine.registerInsertRecomputeListener('probe', ({ object, row, fields }) => { + seen.push({ object, number: row.case_number, title: row.display_title, fields }); + row.headline = `${String(row.display_title)} (listened)`; + }); + await engine.insert('acase', { subject: 'seed', must: 'x' }, SYSTEM); + storeFor('acase').push({ id: 'oob', subject: 'oob', must: 'x', case_number: 'CASE-00002' }); + await engine.insert('acase', { subject: 'collide', must: 'x' }, SYSTEM); + await engine.insert('anote', { name: 'n' }, SYSTEM); + + expect(seen).toEqual([ + { object: 'acase', number: 'CASE-00001', title: 'CASE-00001 - seed', fields: ['display_title', 'headline'] }, + { object: 'acase', number: 'CASE-00002', title: 'CASE-00002 - collide', fields: ['display_title', 'headline'] }, + { object: 'acase', number: 'CASE-00003', title: 'CASE-00003 - collide', fields: ['display_title', 'headline'] }, + ]); + // What it writes on the row is written. + expect(stored(storeFor, 'acase').find((r) => r.subject === 'collide')?.headline).toBe('CASE-00003 - collide (listened)'); + }); + + describe('controls', () => { + it('an expression that reads no record number is computed as before', async () => { + const one: any = await engine.insert('acase', { subject: 'one', must: 'x' }, SYSTEM); + expect(one.plain_title).toBe('S:one'); + const note: any = await engine.insert('anote', { name: 'n' }, SYSTEM); + expect(note).toMatchObject({ code: 'N-001', summary: 'N:n' }); + }); + + it('an update by id computes the reader from the stored number', async () => { + const one: any = await engine.insert('acase', { subject: 'one', must: 'x' }, SYSTEM); + const updated: any = await engine.update('acase', { id: one.id, subject: 'renamed' }, SYSTEM); + expect(updated.display_title).toBe('CASE-00001 - renamed'); + expect(stored(storeFor, 'acase')[0].display_title).toBe('CASE-00001 - renamed'); + }); + }); +}); + +describe('a driver that owns the counter (#25073)', () => { + it('is asked for the number only on an object a stored expression reads it on', async () => { + const { engine, driver, storeFor } = await boot('driver'); + await engine.insert('anote', { name: 'n' }, SYSTEM); + expect(driver.issueAutonumbers).not.toHaveBeenCalled(); + expect(stored(storeFor, 'anote')[0].code).toBe('N-001'); + + await engine.insert('acase', { subject: 'one', must: 'x' }, SYSTEM); + expect(driver.issueAutonumbers).toHaveBeenCalledTimes(1); + expect(driver.issueAutonumbers.mock.calls[0][3]).toBeUndefined(); + }); + + it('re-seeds through the member on a batch collision and does not re-issue the batch', async () => { + const { engine, driver, storeFor } = await boot('driver'); + await engine.insert('acase', { subject: 'seed', must: 'x' }, SYSTEM); + storeFor('acase').push({ id: 'oob', subject: 'oob', must: 'x', case_number: 'CASE-00003' }); + + const error: any = await engine + .insert('acase', [{ subject: 'b1', must: 'x' }, { subject: 'b2', must: 'x' }], SYSTEM) + .then(() => null, (e) => e); + expect(error?.code).toBe('DUPLICATE_RECORD'); + expect(driver.bulkCreate).toHaveBeenCalledTimes(1); + const collisionCall = driver.issueAutonumbers.mock.calls.find((call: any[]) => call[3] !== undefined); + expect(collisionCall?.[1]).toHaveLength(2); + expect(stored(storeFor, 'acase').map((r) => r.case_number)).toEqual(['CASE-00001', 'CASE-00003']); + + // The counter the collision proved stale was re-seeded: the next number is past the store's. + const next: any = await engine.insert('acase', { subject: 'next', must: 'x' }, SYSTEM); + expect(next).toMatchObject({ case_number: 'CASE-00004', display_title: 'CASE-00004 - next' }); + }); + + it('gives up with ERR_AUTONUMBER_COLLISION after the bounded attempts', async () => { + const { engine, driver } = await boot('driver', { alwaysCollide: true }); + const error: any = await engine.insert('acase', { subject: 'x', must: 'x' }, SYSTEM).then(() => null, (e) => e); + expect(error?.code).toBe('ERR_AUTONUMBER_COLLISION'); + expect(driver.create).toHaveBeenCalledTimes(3); + }); + + it('without the member, keeps the number inside create() and says so once per object', async () => { + const { engine, storeFor } = await boot('driver', { withoutMember: true }); + const error = vi.spyOn((engine as any).logger, 'error'); + await engine.insert('acase', { subject: 'one', must: 'x' }, SYSTEM); + await engine.insert('acase', { subject: 'two', must: 'x' }, SYSTEM); + const said = error.mock.calls.filter((call) => String(call[0]).includes('read a record number')); + expect(said).toHaveLength(1); + expect(said[0][2]).toMatchObject({ object: 'acase', fields: ['display_title', 'headline'], driver: 'counter-fake' }); + // The documented degradation: the number lands, the reader is computed without it. + expect(stored(storeFor, 'acase').map((r) => [r.case_number, r.display_title])).toEqual([ + ['CASE-00001', null], + ['CASE-00002', null], + ]); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 240d4b5be0b..381658ee201 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -276,7 +276,7 @@ import { applyProtection, pluralToSingular, ExternalWriteForbiddenError } from ' import { SchemaRegistry, computeFQN, type ArtifactInstallScope } from './registry.js'; import { expandSearchToFilter, normalizeSearch, SEARCHABLE_ENUM_TYPES, SEARCHABLE_TEXTUAL_TYPES } from './search-filter.js'; import { isSearchCompanionRequested, stripSearchCompanion } from './search-companion.js'; -import { ExpressionEngine, toEvalPermissions, type EvalError, type EvalPermissions } from '@objectstack/formula'; +import { ExpressionEngine, analyzeRelationshipTraversals, toEvalPermissions, type EvalError, type EvalPermissions } from '@objectstack/formula'; import type { Expression } from '@objectstack/spec'; import { isAggregatedViewContainer, @@ -3053,6 +3053,64 @@ function planStoredExpressions(schema: any): FormulaPlanEntry[] { return plan; } +/** [#25073] {@link storedExpressionsReading}'s per-source member reads, bounded like the rule validator's cache. */ +const storedExpressionReadsCache = new Map | null>(); +const STORED_EXPRESSION_READS_CACHE_CAP = 512; + +/** The `record.` names one plan entry reads, or `null` when the platform's member reader cannot say. */ +function storedExpressionReads(entry: FormulaPlanEntry): ReadonlySet | null { + const expr = entry.expression as { dialect?: string; source?: unknown } | undefined; + if (!expr || (expr.dialect !== undefined && expr.dialect !== 'cel') || typeof expr.source !== 'string') return null; + const hit = storedExpressionReadsCache.get(expr.source); + if (hit !== undefined) return hit; + const analysis = analyzeRelationshipTraversals(expr.source); + const reads = analysis ? new Set([...analysis.bareFields, ...analysis.traversals.keys()]) : null; + if (storedExpressionReadsCache.size < STORED_EXPRESSION_READS_CACHE_CAP) storedExpressionReadsCache.set(expr.source, reads); + return reads; +} + +/** + * [#25073] The entries of a stored-expression plan that read one of `fields` + * (the object's `autonumber` fields), directly or through another entry that + * does, in plan order: the stored fields an insert must compute again once + * the record number is issued, because every earlier pass ran before it + * existed. + * + * Reads are the platform's one member reader's (`analyzeRelationshipTraversals`, + * `record.x`, `record.?x`, `record['x']`, inside `has()` too), so this cannot + * disagree with the lint about which spellings name a field. An entry whose + * reads it cannot name (another dialect) counts as a reader: computing it + * twice costs a pass, missing it stores a stale value. + */ +function storedExpressionsReading(plan: FormulaPlanEntry[], fields: ReadonlySet): FormulaPlanEntry[] { + if (plan.length === 0 || fields.size === 0) return []; + const moved = new Set(fields); + const readers = new Set(); + for (let grew = true; grew;) { + grew = false; + for (const entry of plan) { + if (readers.has(entry)) continue; + const reads = storedExpressionReads(entry); + if (reads !== null && ![...reads].some((name) => moved.has(name))) continue; + readers.add(entry); + moved.add(entry.name); + grew = true; + } + } + return plan.filter((entry) => readers.has(entry)); +} + +/** The `autonumber` fields an object declares. */ +function autonumberFieldNames(schema: any): Set { + const names = new Set(); + const fields = schema?.fields; + if (!fields || typeof fields !== 'object' || Array.isArray(fields)) return names; + for (const [name, def] of Object.entries(fields as Record)) { + if (def?.type === 'autonumber') names.add(name); + } + return names; +} + /** What {@link computeStoredExpressions} reads its permission map, fault sink and hops from. */ interface StoredExpressionServing { permissionsFor: (plan: FormulaPlanEntry[], records: readonly unknown[]) => Promise; @@ -3088,6 +3146,37 @@ async function computeStoredExpressions( }); } +/** + * [#25073] The stored fields an insert computes again once it holds a record + * number ({@link storedExpressionsReading}), with what computing them reads. + */ +interface AutonumberReaders { + readonly plan: FormulaPlanEntry[]; + readonly execCtx: ExecutionContext | undefined; + readonly serving: StoredExpressionServing; + /** The {@link ObjectQL.registerInsertRecomputeListener} listeners, told per row; absent when none is registered. */ + readonly recomputed?: (row: Record, fields: string[]) => Promise; +} + +/** + * [#25073] Compute the readers again over rows that now carry their issued + * number, and write the values onto the rows the driver is handed — the step + * that makes the stored value the declared expression over the stored row. + * The replaced-value report is NOT made again: it judged the caller's value + * once, before validation and before any number existed. + */ +async function recomputeAutonumberReaders( + readers: AutonumberReaders | undefined, + rows: Array>, +): Promise { + if (!readers || readers.plan.length === 0 || rows.length === 0) return; + const computed = await computeStoredExpressions(readers.plan, rows, readers.execCtx, readers.serving); + rows.forEach((row, k) => Object.assign(row, computed[k])); + if (!readers.recomputed) return; + const fields = readers.plan.map((entry) => entry.name); + for (const row of rows) await readers.recomputed(row, fields); +} + /** * [#24619] The one value a PREDICATE update can store for each stored * computed field, judged over every matched row. `updateMany` writes ONE @@ -4971,6 +5060,9 @@ export class ObjectQL implements IObjectQLEngine { */ private readonly transactionUnsupportedReported = new Set(); + /** [#25073] Objects already reported by {@link warnAutonumberReadersUnserved}: once per engine instance. */ + private readonly autonumberReadersUnservedReported = new Set(); + /** * Objects already reported by {@link warnCascadeNotAtomic} — the `'split'` * verdict of {@link planCascadeAtomicity} — so a cascade that cannot be one @@ -6318,6 +6410,39 @@ export class ObjectQL implements IObjectQLEngine { this.logger.debug('Registered effective object-permission resolver for current_user.can()'); } + /** [#25073] {@link registerInsertRecomputeListener}'s listeners, by registrant id. */ + private readonly insertRecomputeListeners = new Map< + string, + (event: { object: string; row: Record; fields: string[] }) => void | Promise + >(); + + /** + * [#25073] Be told, per row, when an insert computes stored fields again + * after issuing the row's record number, so a value DERIVED from those + * fields can be derived again before the row is written. + * + * A stored field whose `expression` reads an `autonumber` is computed once + * before the `beforeInsert` hooks, when the number does not exist yet, and + * again right after the number is issued (after validation). A + * `beforeInsert` hook that derived something from the field saw the first + * value: `plugin-pinyin-search` stamps the `__search` companion from the + * name field there, so a name field reading the number was searchable + * without it. The listener runs at the second computation, on the very + * row the driver is handed, with the names of the fields computed again + * (`fields`), and on every re-issue after a collision too. What it writes + * on `row` is written; a throw fails the insert, as a hook's does. + * + * Keyed by `id`, so a registrant that binds again replaces itself rather + * than running twice. + */ + registerInsertRecomputeListener( + id: string, + fn: (event: { object: string; row: Record; fields: string[] }) => void | Promise, + ): void { + this.insertRecomputeListeners.set(id, fn); + this.logger.debug('Registered insert recompute listener', { id }); + } + /** * The states {@link composeReadableIdsAtBoundary} has already reported for * this engine. It records only that the report was made and caches no @@ -8532,9 +8657,24 @@ export class ObjectQL implements IObjectQLEngine { * |:---|:---|:---|:---|:---| * | driver-memory | `supports = {}` | **yes** | field-level `unique`, since commit 56c093c4d (ADR-0120 D1/D3 scoping) | `UNIQUE_VIOLATION` / 409 → re-seed + re-issue, here | * | driver-mongodb | absent (`{ batchSchemaSync: true }`) | **yes** | single-field unique index when the field declares `unique` | `E11000 duplicate key` → re-seed + re-issue, here | - * | driver-sql | `autonumber: true` | no | — | — | - * | driver-sqlite-wasm | inherited (`extends SqlDriver`, no `supports` override) | no | — | — | - * | driver-turso | inherited (`...super.supports`) | no | — | — | + * | driver-sql | `autonumber: true` | no | — | — (but see #25073 below) | + * | driver-sqlite-wasm | inherited (`extends SqlDriver`, no `supports` override) | no | — | — (inherited) | + * | driver-turso | inherited (`...super.supports`) | no | — | — (inherited) | + * + * # Numbers the DRIVER issued ahead of the write (#25073) + * + * For an object whose stored `expression` reads its record number, a driver + * that owns its counter issues the number through + * `IDataDriver.issueAutonumbers` before this method runs, and the row + * arrives here carrying it (`driverIssued`). The driver's own retry inside + * `create()` cannot answer a collision on it any more — it reserved + * nothing — and must not: a number re-issued there would leave the stored + * expression computed from the old one. So the retry is this loop's, the + * same loop the fallback drivers use. The member decides whether the error + * is a collision on its numbers (never a duplicate the caller typed, never + * inside a caller's transaction) and re-seeds the stale counter; this loop + * then issues again, recomputes the readers and writes again, under the + * same {@link AUTONUMBER_COLLISION_ATTEMPTS} bound. * * So the retry protects the TWO fallback backends — and it protected only one * of them until commit 56c093c4d, because `driver-memory` enforced no uniqueness at all @@ -8596,7 +8736,12 @@ export class ObjectQL implements IObjectQLEngine { issued: IssuedAutonumber[], execCtx: ExecutionContext | undefined, driverOwnsAutonumber: boolean, + driverIssued: string[] = [], + readers?: AutonumberReaders, ): Promise { + // [#25073] Who issued this row's numbers decides who judges a collision on + // them: the driver, through the member that issued them, or this engine. + const byDriver = driverIssued.length > 0; let attempt = 1; for (;;) { try { @@ -8607,14 +8752,19 @@ export class ObjectQL implements IObjectQLEngine { // driver's error. `envelopeUniqueViolation` returns everything that is // not a unique violation untouched (a deadlock, a NOT NULL, an // unreachable store), so only the recognised conflict changes shape. - if (!this.isIssuedAutonumberCollision(error, issued)) { + // [#25073] On a driver-issued number the member answers instead, and + // in answering re-seeds the counter it proved stale. + const collided: string[] = byDriver + ? ((await driver.issueAutonumbers(object, [row], driverOptions, error)) as string[][] | undefined)?.[0] ?? [] + : this.isIssuedAutonumberCollision(error, issued) ? issued.map((one) => one.field) : []; + if (collided.length === 0) { throw envelopeUniqueViolation(error, object, this.getSchema(object)); } // Whatever happens next, the stale counter must not survive this call: // leaving it in place is what turned one collision into a storm. for (const one of issued) this.autonumberCounters.delete(one.counterKey); if (attempt >= AUTONUMBER_COLLISION_ATTEMPTS) { - const fields = issued.map((one) => one.field).join(', '); + const fields = collided.join(', '); throw Object.assign( new Error( `Autonumber collision on '${object}' field(s) [${fields}]: the record number was ` + @@ -8626,19 +8776,28 @@ export class ObjectQL implements IObjectQLEngine { } attempt += 1; this.logger.warn('Autonumber collided — re-seeding the counter and re-issuing', { - object, fields: issued.map((one) => one.field), attempt, + object, fields: collided, attempt, }); - // Clear the slots so `applyAutonumbers` treats them as empty again — - // leaving the burned value in place would read as an exempt writer's - // and be adopted rather than re-issued. - for (const one of issued) delete row[one.field]; - issued = await this.applyAutonumbers(object, row, execCtx, driverOwnsAutonumber); + let renumbered: boolean; + if (byDriver) { + // [#25073] The member emptied the slots it proved stale; issue them again. + renumbered = (((await driver.issueAutonumbers(object, [row], driverOptions)) as string[][] | undefined)?.[0] ?? []).length > 0; + } else { + // Clear the slots so `applyAutonumbers` treats them as empty again — + // leaving the burned value in place would read as an exempt writer's + // and be adopted rather than re-issued. + for (const one of issued) delete row[one.field]; + issued = await this.applyAutonumbers(object, row, execCtx, driverOwnsAutonumber); + renumbered = issued.length > 0; + } + // [#25073] A new number is a new input to every stored field that reads it. + await recomputeAutonumberReaders(readers, [row]); // Nothing left to re-issue (the field vanished from the schema // mid-flight) — the next failure is the caller's to see, enveloped on // the same terms as the exit above (#14095): this call is OUTSIDE the // loop's own `try`, so its rejection is a second driver-error exit and // not a path the branch above covers. - if (issued.length === 0) { + if (!renumbered) { try { return await driver.create(object, row, driverOptions); } catch (retryError) { @@ -8663,6 +8822,31 @@ export class ObjectQL implements IObjectQLEngine { return issued.some((one) => one.field === column); } + /** + * [#25073] A driver that owns the record-number counter + * (`supports.autonumber`) but cannot issue a number ahead of the write + * (no `IDataDriver.issueAutonumbers`) still fills it inside `create()`, so a + * stored field whose `expression` reads it is stored as computed WITHOUT + * it, and every insert looks successful. That is a stored value that is + * not what its declaration says (AGENTS.md, degradation log levels: `error`), + * said once per object per engine instance, with the remedy. + */ + private warnAutonumberReadersUnserved(object: string, plan: readonly FormulaPlanEntry[], driver: unknown): void { + if (this.autonumberReadersUnservedReported.has(object)) return; + this.autonumberReadersUnservedReported.add(object); + const fields = plan.map((entry) => entry.name); + const driverName = (driver as { name?: unknown } | null)?.name; + this.logger.error( + `Stored field(s) [${fields.join(', ')}] on '${object}' read a record number that driver ` + + `'${String(driverName ?? 'unknown')}' issues only inside create(), so every insert stores them computed ` + + 'WITHOUT the number while the write reports success; an update by id recomputes them later. ' + + 'Use a driver that implements IDataDriver.issueAutonumbers (driver-sql and the drivers built on it do), ' + + 'or keep the record number out of the stored expression. Reported once per object.', + undefined, + { object, fields, driver: driverName }, + ); + } + /** * Seed the autonumber counter from the current max in store, scoped to * `prefix`. With a non-empty prefix (date/field formats) only rows in the @@ -17045,23 +17229,62 @@ export class ObjectQL implements IObjectQLEngine { // have already consumed a sequence value for every good row on the // failed attempt, leaving gaps. Required-validation exempts autonumber // fields either way (they are engine-assigned), and a driver that owns - // autonumber assigns nothing here — so no validation rule can depend on - // the value, making this reorder safe. In partial mode dead rows are - // skipped, so they never consume a sequence value either. + // autonumber issues here only through `issueAutonumbers` (#25073), at + // this same point — so no validation rule can depend on the value, + // making this reorder safe. In partial mode dead rows are skipped, so + // they never consume a sequence value either. // [#6806] What each row's autonumbers were ISSUED from, so a // unique-violation on the write below can be attributed to a counter // and answered by re-seeding it. Empty for every row when the driver // owns autonumber, and for any row the engine numbered nothing on. const issuedPerRow: IssuedAutonumber[][] = new Array(rows.length).fill(null).map(() => []); + // [#25073] A stored field whose `expression` reads the record number + // was computed above, before the number existed. Where the ENGINE + // issues (the fallback drivers) it is computed again right after + // `applyAutonumbers`. Where the DRIVER owns the counter, the number + // used to appear only inside `create()`, too late for anything to read + // it, so for an object with such a field the driver is asked for it + // HERE, at the same point after validation, through + // `IDataDriver.issueAutonumbers`; the row then reaches `create()` / + // `bulkCreate()` carrying it, and the write stays one statement. An + // object no stored expression reads a number on keeps the driver's + // own issue-at-write path, unchanged (and its in-driver batch re-seed, + // #6943, which the engine cannot take over: it never re-issues a batch). + const autonumberReaders: AutonumberReaders | undefined = (() => { + const plan = storedExpressionsReading(storedExpressionPlan, autonumberFieldNames(schemaForValidation)); + if (plan.length === 0) return undefined; + const listeners = [...this.insertRecomputeListeners.values()]; + const recomputed = listeners.length === 0 ? undefined : async (row: Record, fields: string[]) => { + for (const listener of listeners) await listener({ object, row, fields }); + }; + return { plan, execCtx: opCtx.context, serving: storedExpressionServing, recomputed }; + })(); + const issueThroughDriver = driverOwnsAutonumber && autonumberReaders !== undefined + && typeof (driver as any)?.issueAutonumbers === 'function'; + if (driverOwnsAutonumber && autonumberReaders !== undefined && !issueThroughDriver) { + this.warnAutonumberReadersUnserved(object, autonumberReaders.plan, driver); + } + // [#25073] The fields the DRIVER issued on each row, through the member. + const driverIssuedPerRow: string[][] = rows.map(() => []); for (let i = 0; i < rows.length; i++) { if (rowErrors[i] !== undefined) continue; try { - issuedPerRow[i] = await this.applyAutonumbers(object, rows[i], opCtx.context, driverOwnsAutonumber); + if (issueThroughDriver) { + driverIssuedPerRow[i] = (await (driver as any).issueAutonumbers(object, [rows[i]], driverOptions))?.[0] ?? []; + } else { + issuedPerRow[i] = await this.applyAutonumbers(object, rows[i], opCtx.context, driverOwnsAutonumber); + } } catch (e) { if (!partialMode) throw e; rowErrors[i] = e; } } + // [#25073] The readers, computed again over every live row that now + // carries a number it did not carry when they were last computed. + await recomputeAutonumberReaders( + autonumberReaders, + rows.filter((_, i) => rowErrors[i] === undefined && (issuedPerRow[i].length > 0 || driverIssuedPerRow[i].length > 0)), + ); // Live rows = the ones that survived per-row preparation. In // non-partial mode rowErrors is all-empty, so this is exactly `rows`. const liveIndexes: number[] = []; @@ -17114,13 +17337,24 @@ export class ObjectQL implements IObjectQLEngine { object, fields: [...new Set(batchIssued.map((one) => one.field))], }); } + // [#25073] The same answer for numbers the driver issued: the + // member re-seeds the counters it proves stale, and the batch is + // not re-issued, for the reason above. + if (liveIndexes.some((i) => driverIssuedPerRow[i].length > 0)) { + const emptied: string[][] = (await (driver as any).issueAutonumbers(object, liveRows, driverOptions, error)) ?? []; + if (emptied.some((fields) => fields.length > 0)) { + this.logger.warn('Autonumber collided in a batch insert — counter re-seeded, batch not re-issued', { + object, fields: [...new Set(emptied.flat())], + }); + } + } throw envelopeUniqueViolation(error, object, this.getSchema(object)); } } } else { result = await this.createWithAutonumberResync( driver, object, liveRows[0], driverOptions, issuedPerRow[liveIndexes[0]], - opCtx.context, driverOwnsAutonumber, + opCtx.context, driverOwnsAutonumber, driverIssuedPerRow[liveIndexes[0]], autonumberReaders, ); } diff --git a/packages/plugins/plugin-pinyin-search/src/companion-autonumber-recompute.test.ts b/packages/plugins/plugin-pinyin-search/src/companion-autonumber-recompute.test.ts new file mode 100644 index 00000000000..5cec75374b0 --- /dev/null +++ b/packages/plugins/plugin-pinyin-search/src/companion-autonumber-recompute.test.ts @@ -0,0 +1,224 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #25073 — the `__search` companion of a name field that reads the record + * number carries the number. + * + * The companion is stamped in `beforeInsert` from the name field. A name field + * whose stored `expression` reads an `autonumber` is computed there BEFORE the + * number exists, and computed again once the engine issues it (after + * validation). Measured on this branch before the listener below, over + * SQLite (`SqlDriver`) and driver-memory alike, with `nameField` the stored + * title: `"CASE-00001 - 张伟"` stored beside `__search: null`, so `$search` + * found the record neither by pinyin nor by its number; the `has()` shape + * stored `"PRD-0001 茶杯"` beside `"chabei cb"`. + * + * The plugin now also listens on the engine's insert recompute + * (`registerInsertRecomputeListener`) and stamps the companion again from the + * recomputed value, on the row the driver is handed — one write. This drives + * the REAL engine over a hand-rolled driver (both record-number issuers), so + * no driver package is imported. + */ + +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { bindSearchCompanionHooks } from './companion-projection.js'; + +type Row = Record; + +const CASE = { + name: 'scase', + nameField: 'display_title', + fields: { + subject: { type: 'text' }, + case_number: { type: 'autonumber', format: 'CASE-{00000}' }, + display_title: { type: 'text', expression: { dialect: 'cel', source: 'record.case_number + " - " + record.subject' } }, + }, +}; + +const PRODUCT = { + name: 'sproduct', + nameField: 'title', + fields: { + name: { type: 'text' }, + product_code: { type: 'autonumber', format: 'PRD-{0000}' }, + title: { type: 'text', expression: { dialect: 'cel', source: 'has(record.product_code) ? record.product_code + " " + record.name : record.name' } }, + }, +}; + +/** Control: the name field is plain text, so the recomputed title is not a companion source. */ +const TICKET = { + name: 'sticket', + nameField: 'subject', + fields: { + subject: { type: 'text' }, + ticket_number: { type: 'autonumber', format: 'T-{000}' }, + label: { type: 'text', expression: { dialect: 'cel', source: 'record.ticket_number + " " + record.subject' } }, + }, +}; + +/** Control: no eligible name field, so no companion column is provisioned at all. */ +const MEMO = { + name: 'smemo', + nameField: 'secret', + fields: { + secret: { type: 'text', hidden: true }, + memo_number: { type: 'autonumber', format: 'M-{000}' }, + label: { type: 'text', expression: { dialect: 'cel', source: 'record.memo_number + " " + record.secret' } }, + }, +}; + +const AUTONUMBERS: Record = { + scase: ['case_number', 'CASE-', 5], + sproduct: ['product_code', 'PRD-', 4], + sticket: ['ticket_number', 'T-', 3], + smemo: ['memo_number', 'M-', 3], +}; + +function cond(value: unknown, c: unknown): boolean { + if (c === null || typeof c !== 'object' || Array.isArray(c)) return value === c; + return Object.entries(c as Record).every(([op, t]) => { + switch (op) { + case '$eq': return value === t; + case '$startsWith': return typeof value === 'string' && value.startsWith(String(t)); + case '$gt': return value != null && String(value) > String(t); + case '$contains': return typeof value === 'string' && value.includes(String(t)); + case '$icontains': return typeof value === 'string' && value.toLowerCase().includes(String(t).toLowerCase()); + default: throw new Error(`fake driver: unsupported operator ${op}`); + } + }); +} + +function matches(row: Row, where: any): boolean { + if (!where || typeof where !== 'object') return true; + return Object.entries(where).every(([k, v]: [string, any]) => { + if (k === '$and') return (v as any[]).every((w) => matches(row, w)); + if (k === '$or') return (v as any[]).some((w) => matches(row, w)); + if (k.startsWith('$')) throw new Error(`fake driver: unsupported logical operator ${k}`); + return cond(row[k], v); + }); +} + +/** `engine`: leaves the number to the engine. `driver`: owns a counter and issues it through the member. */ +function makeDriver(issuer: 'engine' | 'driver') { + const stores = new Map(); + const storeFor = (o: string) => { + let s = stores.get(o); + if (!s) { s = []; stores.set(o, s); } + return s; + }; + const counters = new Map(); + const fill = (object: string, row: Row): string[] => { + const [field, prefix, width] = AUTONUMBERS[object]; + if (row[field] != null && row[field] !== '') return []; + const next = (counters.get(object) ?? 0) + 1; + counters.set(object, next); + row[field] = `${prefix}${String(next).padStart(width, '0')}`; + return [field]; + }; + let n = 0; + const write = (object: string, data: Row) => { + const row = { ...data, id: data.id ?? `r_${++n}` }; + if (issuer === 'driver') fill(object, row); + storeFor(object).push(row); + return { ...row }; + }; + const driver: any = { + name: `${issuer}-fake`, + version: '0.0.0', + supports: issuer === 'engine' ? {} : { autonumber: true }, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async find(object: string, ast: any) { + let rows = storeFor(object).filter((r) => matches(r, ast?.where)); + if (typeof ast?.limit === 'number') rows = rows.slice(0, ast.limit); + return rows.map((r) => ({ ...r })); + }, + async findOne(object: string, ast: any) { + const row = storeFor(object).find((r) => matches(r, ast?.where)); + return row ? { ...row } : null; + }, + async create(object: string, data: Row) { return write(object, data); }, + async bulkCreate(object: string, rows: Row[]) { return rows.map((r) => write(object, r)); }, + async update(object: string, id: string, data: Row) { + const s = storeFor(object); + const i = s.findIndex((r) => r.id === id); + s[i] = { ...s[i], ...data, id }; + return { ...s[i] }; + }, + async delete() { return true; }, + async count(object: string, ast: any) { return storeFor(object).filter((r) => matches(r, ast?.where)).length; }, + }; + if (issuer === 'driver') { + driver.issueAutonumbers = async (object: string, rows: Row[], _options?: unknown, afterCollision?: unknown) => + afterCollision === undefined ? rows.map((row) => fill(object, row)) : rows.map(() => []); + } + return { driver, storeFor }; +} + +const SYSTEM = { context: { isSystem: true } } as any; +const quiet = { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }; + +let pinyinSwitch: string | undefined; +beforeAll(() => { + pinyinSwitch = process.env.OS_SEARCH_PINYIN_ENABLED; + process.env.OS_SEARCH_PINYIN_ENABLED = 'true'; +}); +afterAll(() => { + if (pinyinSwitch === undefined) delete process.env.OS_SEARCH_PINYIN_ENABLED; + else process.env.OS_SEARCH_PINYIN_ENABLED = pinyinSwitch; +}); + +async function boot(issuer: 'engine' | 'driver') { + const engine = new ObjectQL(); + const { driver, storeFor } = makeDriver(issuer); + engine.registerDriver(driver, true); + await engine.init(); + for (const schema of [CASE, PRODUCT, TICKET, MEMO]) engine.registry.registerObject(schema as any); + bindSearchCompanionHooks(engine as any, quiet); + return { engine, storeFor }; +} + +const searchCount = async (engine: ObjectQL, object: string, term: string) => + ((await engine.find(object, { search: term }, SYSTEM)) as unknown[]).length; + +describe.each(['engine', 'driver'] as const)('the __search companion of a name field reading the record number (#25073), %s-issued', (issuer) => { + it('carries the number on the concatenation shape, and $search finds the record by pinyin and by number', async () => { + const { engine, storeFor } = await boot(issuer); + await engine.insert('scase', { subject: '张伟' }, SYSTEM); + expect(storeFor('scase')[0]).toMatchObject({ + display_title: 'CASE-00001 - 张伟', + __search: 'case00001zhangwei case00001zw', + }); + expect(await searchCount(engine, 'scase', 'zhangwei')).toBe(1); + expect(await searchCount(engine, 'scase', 'case00001')).toBe(1); + }); + + it('carries the number on the has() shape', async () => { + const { engine, storeFor } = await boot(issuer); + await engine.insert('sproduct', { name: '茶杯' }, SYSTEM); + expect(storeFor('sproduct')[0]).toMatchObject({ title: 'PRD-0001 茶杯', __search: 'prd0001chabei prd0001cb' }); + expect(await searchCount(engine, 'sproduct', 'prd0001')).toBe(1); + }); + + it('carries each row\'s own number in a batch', async () => { + const { engine, storeFor } = await boot(issuer); + await engine.insert('scase', [{ subject: '张伟' }, { subject: '李娜' }], SYSTEM); + expect(storeFor('scase').map((r) => r.__search)).toEqual(['case00001zhangwei case00001zw', 'case00002lina case00002ln']); + }); + + describe('controls', () => { + it('leaves a companion whose source the recompute did not touch as beforeInsert stamped it', async () => { + const { engine, storeFor } = await boot(issuer); + await engine.insert('sticket', { subject: '张伟' }, SYSTEM); + expect(storeFor('sticket')[0]).toMatchObject({ label: 'T-001 张伟', __search: 'zhangwei zw' }); + }); + + it('writes no companion on an object that has no companion column', async () => { + const { engine, storeFor } = await boot(issuer); + expect(engine.registry.getObject('smemo')?.fields).not.toHaveProperty('__search'); + await engine.insert('smemo', { secret: '张伟' }, SYSTEM); + expect(storeFor('smemo')[0]).toMatchObject({ label: 'M-001 张伟' }); + expect(storeFor('smemo')[0]).not.toHaveProperty('__search'); + }); + }); +}); diff --git a/packages/plugins/plugin-pinyin-search/src/companion-projection.ts b/packages/plugins/plugin-pinyin-search/src/companion-projection.ts index 710969a8962..2f9679f06ae 100644 --- a/packages/plugins/plugin-pinyin-search/src/companion-projection.ts +++ b/packages/plugins/plugin-pinyin-search/src/companion-projection.ts @@ -35,6 +35,10 @@ interface MinimalEngine { options?: { object?: string | string[]; priority?: number; packageId?: string }, ): void; unregisterHooksByPackage(packageId: string): number; + registerInsertRecomputeListener?( + id: string, + fn: (event: { object: string; row: Record; fields: string[] }) => void | Promise, + ): void; find(object: string, query?: any, options?: any): Promise; update(object: string, data: any, options?: any): Promise; registry?: { @@ -54,8 +58,16 @@ interface MinimalLogger { * field is part of the write. Recomputes from the NEW value; a non-CJK new * value clears the companion (null) so stale pinyin never recalls a renamed * record. Never throws — a normalization failure must not fail the write. + * + * `recomputed` narrows "part of the write" to the fields the engine computed + * again after issuing the record number (#25073). */ -async function stampCompanion(engine: MinimalEngine, ctx: any, logger?: MinimalLogger): Promise { +async function stampCompanion( + engine: MinimalEngine, + ctx: any, + logger?: MinimalLogger, + recomputed?: readonly string[], +): Promise { const object = ctx?.object; if (!object) return; const schema = engine.registry?.getObject?.(object); @@ -66,7 +78,9 @@ async function stampCompanion(engine: MinimalEngine, ctx: any, logger?: MinimalL const sources = resolveSearchCompanionSources(schema); if (sources.length === 0) return; - const touched = sources.filter((s) => Object.prototype.hasOwnProperty.call(data, s)); + const touched = sources.filter((s) => + recomputed ? recomputed.includes(s) : Object.prototype.hasOwnProperty.call(data, s), + ); if (touched.length === 0) return; // source unchanged → no recompute (no write amplification) try { @@ -95,6 +109,14 @@ export function bindSearchCompanionHooks(engine: MinimalEngine, logger?: Minimal const handler = (ctx: any) => stampCompanion(engine, ctx, logger); engine.registerHook('beforeInsert', handler, opts); engine.registerHook('beforeUpdate', handler, opts); + // [#25073] A name field whose stored `expression` reads the record number is + // computed again once the number is issued, after `beforeInsert` ran, so + // the companion is stamped again from that value before the row is written. + if (typeof engine.registerInsertRecomputeListener === 'function') { + engine.registerInsertRecomputeListener(PINYIN_SEARCH_HOOK_PACKAGE, ({ object, row, fields }) => + stampCompanion(engine, { object, input: { data: row } }, logger, fields), + ); + } logger?.info?.('[pinyin-search] companion hooks bound (beforeInsert/beforeUpdate, all objects)'); } diff --git a/packages/runtime/src/autonumber-stored-expression.integration.test.ts b/packages/runtime/src/autonumber-stored-expression.integration.test.ts new file mode 100644 index 00000000000..c38878e2e09 --- /dev/null +++ b/packages/runtime/src/autonumber-stored-expression.integration.test.ts @@ -0,0 +1,194 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #25073 — over a real SQLite database: a stored field whose `expression` + * reads an `autonumber` is stored computed WITH the number, in the one INSERT. + * + * `SqlDriver` owns its record-number counter (`supports.autonumber`), and + * used to issue the number inside `create()` / `bulkCreate()` — after the + * engine had computed every stored field. Measured at the base on this very + * setup (#25073's probe): + * + * | insert | stored `display_title` | stored `title` (`has()`) | + * |:--|:--|:--| + * | before | `null` | `"Probe product"` (the fallback) | + * | after | `"CASE-00001 - one"` | `"PRD-0001 Probe product"` | + * + * The engine now asks the driver for the number through + * `IDataDriver.issueAutonumbers` after validation, computes the readers + * again, and hands the row to the write, so the stored row is the declared + * expression over itself the moment it lands, and `afterInsert` sees it. An + * object no stored expression reads a number on keeps the driver's own + * issue-at-write path. A collision on an issued number is re-seeded by the + * member and re-issued by the engine's one retry loop, which computes the + * readers again. + * + * The engine's half over hand-rolled drivers (both issuers) is + * `packages/objectql/src/engine-autonumber-stored-expression.test.ts`; the + * member's own half is driver-sql's + * `sql-driver-autonumber-issue-ahead.test.ts`. + */ + +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; + +const CASE = { + name: 'ncase', + fields: { + subject: { type: 'text' }, + case_number: { type: 'autonumber', format: 'CASE-{00000}', unique: true }, + display_title: { type: 'text', expression: 'record.case_number + " - " + record.subject' }, + headline: { type: 'text', expression: 'record.display_title + "!"' }, + plain_title: { type: 'text', expression: '"S:" + record.subject' }, + must: { type: 'text', required: true }, + }, +} as any; + +const PRODUCT = { + name: 'nproduct', + fields: { + name: { type: 'text' }, + product_code: { type: 'autonumber', format: 'PRD-{0000}' }, + title: { type: 'text', expression: 'has(record.product_code) ? record.product_code + " " + record.name : record.name' }, + }, +} as any; + +/** Control: an autonumber that no stored expression reads. */ +const NOTE = { + name: 'nnote', + fields: { + name: { type: 'text' }, + code: { type: 'autonumber', format: 'N-{000}' }, + summary: { type: 'text', expression: '"N:" + record.name' }, + }, +} as any; + +const SYSTEM = { context: { isSystem: true } } as any; + +const open: SqlDriver[] = []; +afterEach(async () => { + while (open.length) await open.pop()!.disconnect().catch(() => {}); +}); + +async function boot() { + const driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }); + open.push(driver); + const engine = new ObjectQL(); + engine.registerDriver(driver as any, true); + await engine.init(); + for (const schema of [CASE, PRODUCT, NOTE]) engine.registry.registerObject(schema, 'probe'); + await driver.initObjects([CASE, PRODUCT, NOTE]); + const knex = (driver as any).knex; + const rows = async (table: string) => (await knex(table).select('*').orderBy('case_number')) as any[]; + const lastValue = async (object: string) => + Number(((await knex('_objectstack_sequences').where({ object }).select('last_value')) as any[])[0]?.last_value); + return { + engine, + driver, + knex, + rows, + lastValue, + issue: vi.spyOn(driver, 'issueAutonumbers'), + create: vi.spyOn(driver, 'create'), + bulkCreate: vi.spyOn(driver, 'bulkCreate'), + }; +} + +describe('[#25073] SQLite: a stored expression reading the record number', () => { + it('single insert: the stored row carries the number in its readers, written once, one number drawn', async () => { + const { engine, rows, lastValue, issue, create } = await boot(); + const one: any = await engine.insert('ncase', { subject: 'one', must: 'x' }, SYSTEM); + expect(one).toMatchObject({ case_number: 'CASE-00001', display_title: 'CASE-00001 - one', headline: 'CASE-00001 - one!' }); + expect(await rows('ncase')).toMatchObject([ + { case_number: 'CASE-00001', display_title: 'CASE-00001 - one', headline: 'CASE-00001 - one!', plain_title: 'S:one' }, + ]); + expect(create).toHaveBeenCalledTimes(1); + expect(issue).toHaveBeenCalledTimes(1); + expect(await lastValue('ncase')).toBe(1); + }); + + it('batch insert: each row its own number, one bulkCreate', async () => { + const { engine, rows, bulkCreate } = await boot(); + await engine.insert('ncase', [{ subject: 'b1', must: 'x' }, { subject: 'b2', must: 'x' }], SYSTEM); + expect((await rows('ncase')).map((r) => r.display_title)).toEqual(['CASE-00001 - b1', 'CASE-00002 - b2']); + expect(bulkCreate).toHaveBeenCalledTimes(1); + }); + + it('partial mode: a rejected row draws no number', async () => { + const { engine, rows, lastValue } = await boot(); + const outcomes: any[] = await engine.insertMany( + 'ncase', + [{ subject: 'p1', must: 'x' }, { subject: 'dead' }, { subject: 'p3', must: 'x' }], + SYSTEM, + ); + expect(outcomes.map((o) => o.ok)).toEqual([true, false, true]); + expect((await rows('ncase')).map((r) => r.display_title)).toEqual(['CASE-00001 - p1', 'CASE-00002 - p3']); + expect(await lastValue('ncase')).toBe(2); + }); + + it('the has() shape takes the branch that sees the number', async () => { + const { engine, knex } = await boot(); + const product: any = await engine.insert('nproduct', { name: 'Probe product' }, SYSTEM); + expect(product.title).toBe('PRD-0001 Probe product'); + expect((await knex('nproduct').select('title'))[0].title).toBe('PRD-0001 Probe product'); + }); + + it('a forced collision re-issues past the store and computes the readers from the new number', async () => { + const { engine, knex, rows } = await boot(); + await engine.insert('ncase', { subject: 'seed', must: 'x' }, SYSTEM); + await knex('ncase').insert({ id: 'oob', subject: 'oob', must: 'x', case_number: 'CASE-00002' }); + const collided: any = await engine.insert('ncase', { subject: 'collide', must: 'x' }, SYSTEM); + expect(collided).toMatchObject({ case_number: 'CASE-00003', display_title: 'CASE-00003 - collide' }); + expect((await rows('ncase')).find((r) => r.subject === 'collide')).toMatchObject({ + case_number: 'CASE-00003', display_title: 'CASE-00003 - collide', headline: 'CASE-00003 - collide!', + }); + }); + + it('a batch collision lands nothing, re-seeds the counter, and the next insert is past the store', async () => { + const { engine, knex, rows } = await boot(); + await engine.insert('ncase', { subject: 'seed', must: 'x' }, SYSTEM); + await knex('ncase').insert({ id: 'oob', subject: 'oob', must: 'x', case_number: 'CASE-00005' }); + // Issued 2 and 3: no collision. Push the store past them so the batch collides. + await knex('ncase').insert({ id: 'oob2', subject: 'oob2', must: 'x', case_number: 'CASE-00003' }); + const error: any = await engine + .insert('ncase', [{ subject: 'b1', must: 'x' }, { subject: 'b2', must: 'x' }], SYSTEM) + .then(() => null, (e) => e); + expect(error?.code).toBe('DUPLICATE_RECORD'); + expect((await rows('ncase')).map((r) => r.case_number)).toEqual(['CASE-00001', 'CASE-00003', 'CASE-00005']); + const next: any = await engine.insert('ncase', { subject: 'next', must: 'x' }, SYSTEM); + expect(next).toMatchObject({ case_number: 'CASE-00006', display_title: 'CASE-00006 - next' }); + }); + + it('afterInsert sees the stored value', async () => { + const { engine } = await boot(); + const seen: unknown[] = []; + engine.registerHook('afterInsert', async (ctx: any) => { seen.push(ctx.result?.display_title); }, { object: 'ncase' }); + await engine.insert('ncase', { subject: 'hooked', must: 'x' }, SYSTEM); + expect(seen).toEqual(['CASE-00001 - hooked']); + }); + + it('inside a caller transaction: the number and its readers commit together', async () => { + const { engine, rows } = await boot(); + await engine.transaction(async (ctx: any) => engine.insert('ncase', { subject: 'unit', must: 'x' }, { context: ctx } as any)); + expect(await rows('ncase')).toMatchObject([{ case_number: 'CASE-00001', display_title: 'CASE-00001 - unit' }]); + }); + + describe('controls', () => { + it('an object no stored expression reads a number on keeps the driver\'s issue-at-write', async () => { + const { engine, knex, issue } = await boot(); + const note: any = await engine.insert('nnote', { name: 'n' }, SYSTEM); + expect(note).toMatchObject({ code: 'N-001', summary: 'N:n' }); + expect(issue).not.toHaveBeenCalled(); + expect((await knex('nnote').select('code', 'summary'))[0]).toEqual({ code: 'N-001', summary: 'N:n' }); + }); + + it('an update by id computes the reader from the stored number', async () => { + const { engine, rows } = await boot(); + const one: any = await engine.insert('ncase', { subject: 'one', must: 'x' }, SYSTEM); + const updated: any = await engine.update('ncase', { id: one.id, subject: 'renamed' }, SYSTEM); + expect(updated.display_title).toBe('CASE-00001 - renamed'); + expect((await rows('ncase'))[0].display_title).toBe('CASE-00001 - renamed'); + }); + }); +}); diff --git a/packages/spec/src/contracts/data-driver.ts b/packages/spec/src/contracts/data-driver.ts index 1e18b8b93fa..29e25ff5d94 100644 --- a/packages/spec/src/contracts/data-driver.ts +++ b/packages/spec/src/contracts/data-driver.ts @@ -268,6 +268,56 @@ export interface IDataDriver { /** Delete multiple records matching a query (optional) */ deleteMany?(object: string, query: DriverQuery, options?: DriverOptions): Promise; + // =========================================================================== + // Record Numbers (`autonumber`, #25073) + // =========================================================================== + + /** + * Issue the record numbers of rows the caller is about to insert, ahead of + * the write — the public form of a driver that owns its `autonumber` + * counter (`supports.autonumber: true`). + * + * Such a driver fills an empty `autonumber` slot inside `create()` / + * `bulkCreate()`, the last moment before the row is stored, so a value + * computed FROM the number (a stored field whose `expression` reads it) was + * computed before the number existed. Through this member the engine asks + * for the numbers after validation, computes what reads them, and hands + * `create()` / `bulkCreate()` a row that already carries its number: one + * write, no transaction, and the driver stays the one owner of the counter + * (ADR-0034 §3). + * + * **Issue** (no `afterCollision`): fill every `autonumber` field a row + * leaves empty (`undefined`, `null` or `''`) from the driver's own counter, + * in place, exactly as `create()` would have filled it; a field the row + * already carries is the caller's and is left alone. Answers, index-aligned + * with `rows`, the names of the fields filled on each row. A row whose + * number cannot be issued (an interpolated `{field}` token is empty) is + * refused by a throw, as `create()` refuses it. + * + * **After a collision** (`afterCollision` is the error a `create()` / + * `bulkCreate()` of exactly these row objects raised, after an issue call + * filled them): decide whether that error is a unique violation on a number + * this member issued to them. If it is, re-seed each counter the collision + * proves stale, EMPTY the slots on these rows that drew from it, and answer + * those field names per row; the caller then issues again and recomputes + * what reads the numbers. Otherwise answer an empty list for every row and + * change nothing: the error is the caller's to see (a duplicate on a value + * the caller typed; a collision inside a caller's transaction, which cannot + * be retried on the same handle). `rows` must be the very objects the issue + * call filled, because the driver may recognise its own numbers by them. + * + * Optional: a driver that leaves the number to the engine (no + * `supports.autonumber`) omits it, and so may a driver that owns its counter + * but cannot issue ahead of the write; the engine then leaves the number to + * `create()` and says once that a stored expression reading it cannot see it. + */ + issueAutonumbers?( + object: string, + rows: Record[], + options?: DriverOptions, + afterCollision?: unknown, + ): Promise; + // =========================================================================== // Temporal Storage Convention (ADR-0053 D-A1/D-A2, #3912) // ===========================================================================