From 41abe4a9127218886bdd556e405d0a33ce44edad Mon Sep 17 00:00:00 2001 From: Jack Zhuang <50353452+hotlong@users.noreply.github.com> Date: Tue, 29 Sep 2026 05:07:40 +0800 Subject: [PATCH 1/4] fix(rest): /import reads a comma in a number cell only as a thousands group `parseNumberCell` stripped every comma before parsing, so a decimal-comma cell ('3,14', '1,5', '1.000,5') or a stray comma ('1,2,3') was stored as a different number (314, 15, 1.0005, 123) with ok 1, errors 0. A comma is now admitted only in a well-formed thousands grouping (1 to 3 leading digits, then groups of exactly three, only before any '.'); any other comma makes the cell the row's invalid_number error, the code the plain write doors already answer. No locale is guessed. Pins /import on InMemoryDriver and SqlDriver (better-sqlite3) with the four cells refused per row and 1,000 / 12,345.67 / (1,234) as admitted controls; rest gains @objectstack/driver-memory as a devDependency, aliased to source. Claude-Session: https://claude.ai/code/session_local_1d2a197c-c20e-4e90-9be8-413d4d432289 Co-authored-by: Claude --- packages/rest/package.json | 1 + packages/rest/src/import-coerce.test.ts | 30 +++ packages/rest/src/import-coerce.ts | 26 ++- .../src/import-number-thousands-group.test.ts | 179 ++++++++++++++++++ packages/rest/vitest.config.ts | 8 + pnpm-lock.yaml | 3 + 6 files changed, 246 insertions(+), 1 deletion(-) create mode 100644 packages/rest/src/import-number-thousands-group.test.ts diff --git a/packages/rest/package.json b/packages/rest/package.json index 5885f560b75..d8829d51734 100644 --- a/packages/rest/package.json +++ b/packages/rest/package.json @@ -39,6 +39,7 @@ "zod": "^4.6.1" }, "devDependencies": { + "@objectstack/driver-memory": "workspace:*", "@objectstack/driver-sql": "workspace:*", "@objectstack/metadata": "workspace:*", "@objectstack/metadata-protocol": "workspace:*", diff --git a/packages/rest/src/import-coerce.test.ts b/packages/rest/src/import-coerce.test.ts index da120e07104..f5317ae7bef 100644 --- a/packages/rest/src/import-coerce.test.ts +++ b/packages/rest/src/import-coerce.test.ts @@ -49,6 +49,36 @@ describe('parseNumberCell', () => { expect(parseNumberCell('12x3')).toBeUndefined(); expect(parseNumberCell('')).toBeUndefined(); }); + + // [#20497] A comma is read only where it groups thousands: 1 to 3 leading + // digits, then groups of exactly three, and only before any `.`. + it.each([ + ['1,000', 1000], + ['12,345.67', 12345.67], + ['1,234,567.89', 1234567.89], + ['-1,234', -1234], + ['(1,234)', -1234], + ['$1,000', 1000], + ['1,234%', 1234], + ['1,000e3', 1_000_000], + ])('admits the well-formed thousands grouping %j as %d', (cell, n) => { + expect(parseNumberCell(cell)).toBe(n); + }); + + it.each([ + // The card's four cells: stored as 314, 15, 1.0005 and 123 before. + '3,14', '1,5', '1.000,5', '1,2,3', + // A decimal comma, however it is dressed. + '0,5', '(3,14)', '$1,5', '1,5%', + // A group that is not exactly three digits, or a comma out of place. + '1,23', '1,0000', '1234,567', ',123', '-,123', '1,000,', '.5,000', + // A comma after the `.`. + '12,345.6,7', + // A grouping other than thousands — no locale is guessed. + '12,34,567', '1,00,000', + ])('refuses %j rather than reading it as some other number', (cell) => { + expect(parseNumberCell(cell)).toBeUndefined(); + }); }); describe('parseDateCell', () => { diff --git a/packages/rest/src/import-coerce.ts b/packages/rest/src/import-coerce.ts index e8f963e25cb..afaf602da19 100644 --- a/packages/rest/src/import-coerce.ts +++ b/packages/rest/src/import-coerce.ts @@ -218,12 +218,33 @@ export function parseBooleanCell(raw: unknown): boolean | undefined { // ── numbers ──────────────────────────────────────────────────────── +/** + * The one comma placement a numeric cell may carry: a thousands grouping of the + * integer part — an optional sign, 1 to 3 leading digits, then one or more + * groups of exactly three digits, ending the integer part (end of cell, `.` or + * the exponent marker follows) — and no comma anywhere after it. `1,000`, + * `12,345.67`, `-1,234,567` match; `3,14`, `1,5`, `1.000,5`, `1,2,3`, + * `1234,567`, `1,0000` and `12,345.6,7` do not. + */ +const THOUSANDS_GROUPED_INTEGER = /^[+-]?\d{1,3}(?:,\d{3})+(?![\d,])[^,]*$/; + /** * Parse a numeric cell, tolerating the punctuation spreadsheets add: thousands * separators (`1,234`), a leading currency symbol (`$` `¥` `€` `£` `￥`), a * trailing percent sign (`25%` → `25`), and accounting-style parenthesised * negatives (`(1,234)` → `-1234`). Returns `undefined` when the residue is not * a finite number. + * + * A comma is a thousands separator and nothing else, and only where it groups + * thousands (#20497): 1 to 3 leading digits, then groups of exactly three, + * and only before any `.` (`1,000`, `12,345.67`) — see + * {@link THOUSANDS_GROUPED_INTEGER}. Any other comma makes the cell + * unparseable (`undefined`, so the row's `invalid_number` error), never a + * different number: a decimal-comma spelling (`3,14`, `1,5`, `1.000,5`) or a + * stray comma (`1,2,3`) used to have every comma stripped and was stored as + * `314`, `15`, `1.0005`, `123`. No locale is guessed — `1,500` is always one + * thousand five hundred, never 1.5 — so a file written with a decimal comma is + * refused cell by cell rather than read as some other value. */ export function parseNumberCell(raw: unknown): number | undefined { if (typeof raw === 'number') return Number.isFinite(raw) ? raw : undefined; @@ -233,7 +254,10 @@ export function parseNumberCell(raw: unknown): number | undefined { if (/^\(.*\)$/.test(s)) { negative = true; s = s.slice(1, -1).trim(); } s = s.replace(/^[$¥€£￥]\s*/, ''); // leading currency symbol s = s.replace(/%$/, '').trim(); // trailing percent - s = s.replace(/,/g, ''); // thousands separators + if (s.includes(',')) { + if (!THOUSANDS_GROUPED_INTEGER.test(s)) return undefined; + s = s.replace(/,/g, ''); // a well-formed thousands grouping + } if (s === '' || !/^[+-]?\d*\.?\d+(e[+-]?\d+)?$/i.test(s)) return undefined; const n = Number(s); if (!Number.isFinite(n)) return undefined; diff --git a/packages/rest/src/import-number-thousands-group.test.ts b/packages/rest/src/import-number-thousands-group.test.ts new file mode 100644 index 00000000000..c89b674099f --- /dev/null +++ b/packages/rest/src/import-number-thousands-group.test.ts @@ -0,0 +1,179 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#20497] `POST /api/v1/data/:object/import` reads a comma in a number cell + * as a thousands separator ONLY where it groups thousands — 1 to 3 leading + * digits, then groups of exactly three, and only before any `.` — and refuses + * every other comma as the row's `invalid_number` error, on `InMemoryDriver` + * and on `SqlDriver` (better-sqlite3 `:memory:`), through the real route. + * + * Measured on the base (`9449512a31`) through this route, JSON rows, + * `writeMode: 'insert'`, both drivers alike: + * + * | cell | base: stored · answer | head | + * |:--|:--|:--| + * | `'3,14'` | `314` · ok 1, errors 0 | row refused, `invalid_number` | + * | `'1,5'` | `15` · ok 1, errors 0 | row refused, `invalid_number` | + * | `'1.000,5'` | `1.0005` · ok 1, errors 0 | row refused, `invalid_number` | + * | `'1,2,3'` | `123` · ok 1, errors 0 | row refused, `invalid_number` | + * | `'1,000'` / `'12,345.67'` / `'(1,234)'` | `1000` / `12345.67` / `-1234` | unchanged | + * + * The plain create door already answered `400 VALIDATION_FAILED` with field + * code `invalid_number` for each of the four; the import row now answers the + * same code. The reader's own case table (every documented form, the spec + * grammar's rows, the comma probes) is `import-coerce.test.ts`'s + * `parseNumberCell` block; this file pins the door, per driver. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { InMemoryDriver } from '@objectstack/driver-memory'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { RestServer } from './rest-server'; + +const OBJECT = 'import_thousands_20497'; + +const LEDGER = { + name: OBJECT, label: 'Ledger 20497', systemFields: false, + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + amount: { name: 'amount', type: 'number' as const, label: 'Amount' }, + }, +}; + +/** The card's four cells: each used to be stored as a different number. */ +const REFUSED = ['3,14', '1,5', '1.000,5', '1,2,3'] as const; + +/** The admitted controls: a well-formed thousands grouping keeps its reading. */ +const ADMITTED: ReadonlyArray = [ + ['1,000', 1000], + ['12,345.67', 12345.67], + ['(1,234)', -1234], +]; + +const DRIVERS = [ + ['InMemoryDriver', () => new InMemoryDriver({ persistence: false })], + ['SqlDriver (better-sqlite3)', () => new SqlDriver({ + client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true, + })], +] as const; + +function createMockServer() { + const noop = () => {}; + return { get: noop, post: noop, put: noop, delete: noop, patch: noop, use: noop, listen: async () => {}, close: async () => {} }; +} + +function makeRes() { + const res: any = { + write: () => true, end: () => {}, + header: () => res, + status: (code: number) => { res._status = code; return res; }, + json: (body: any) => { res._json = body; return res; }, + }; + return res; +} + +const liveEngines: ObjectQL[] = []; +afterEach(async () => { + while (liveEngines.length) { + try { await liveEngines.pop()?.destroy(); } catch { /* noop */ } + } +}); + +async function boot(makeDriver: () => unknown) { + const engine = new ObjectQL(); + liveEngines.push(engine); + engine.registerDriver(makeDriver() as any, true); + await engine.init(); + engine.registry.registerObject(LEDGER 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 } } as any); + (rest as any).resolveExecCtx = async () => ({ userId: 'test-user' }); + rest.registerRoutes(); + const find = (method: string, path: string) => + rest.getRoutes().find((r: any) => r.method === method && r.path === path) as any; + const importRoute = find('POST', '/api/v1/data/:object/import'); + const createRoute = find('POST', '/api/v1/data/:object'); + expect(importRoute).toBeDefined(); + expect(createRoute).toBeDefined(); + const send = async (route: any, body: unknown) => { + const res = makeRes(); + await route.handler({ params: { object: OBJECT }, body } as any, res); + return res; + }; + return { + engine, + importRows: (body: Record) => send(importRoute, body), + create: (body: Record) => send(createRoute, body), + }; +} + +describe.each(DRIVERS)('[#20497] /import — a comma is read only as a thousands group (%s)', (_name, makeDriver) => { + let ctx: Awaited>; + beforeEach(async () => { ctx = await boot(makeDriver); }); + + it('refuses each decimal-comma / stray-comma cell per row and stores the admitted controls', async () => { + const cells = [...REFUSED, ...ADMITTED.map(([cell]) => cell)]; + const res = await ctx.importRows({ + format: 'json', writeMode: 'insert', + rows: cells.map((amount, i) => ({ id: `r${i}`, amount })), + }); + + expect(res._status ?? 200).toBe(200); + expect(res._json).toMatchObject({ + total: cells.length, ok: ADMITTED.length, errors: REFUSED.length, created: ADMITTED.length, + }); + for (const [i] of REFUSED.entries()) { + expect(res._json.results[i]).toMatchObject({ + row: i + 1, ok: false, action: 'failed', field: 'amount', code: 'invalid_number', + }); + // Refused, not stored as some other number. + expect(await ctx.engine.findOne(OBJECT, { where: { id: `r${i}` } })).toBeNull(); + } + for (const [j, [, stored]] of ADMITTED.entries()) { + const i = REFUSED.length + j; + expect(res._json.results[i]).toMatchObject({ row: i + 1, ok: true, action: 'created' }); + expect((await ctx.engine.findOne(OBJECT, { where: { id: `r${i}` } }))?.amount).toBe(stored); + } + }); + + it('gives the same verdicts to quoted CSV cells', async () => { + const csv = [ + 'ID,Amount', + ...REFUSED.map((cell, i) => `r${i},"${cell}"`), + ...ADMITTED.map(([cell], j) => `c${j},"${cell}"`), + ].join('\n'); + const res = await ctx.importRows({ format: 'csv', csv, writeMode: 'insert', mapping: { ID: 'id', Amount: 'amount' } }); + + expect(res._json).toMatchObject({ total: REFUSED.length + ADMITTED.length, ok: ADMITTED.length, errors: REFUSED.length }); + const failedFields = res._json.results.filter((r: any) => !r.ok).map((r: any) => [r.field, r.code]); + expect(failedFields).toEqual(REFUSED.map(() => ['amount', 'invalid_number'])); + for (const [j, [, stored]] of ADMITTED.entries()) { + expect((await ctx.engine.findOne(OBJECT, { where: { id: `c${j}` } }))?.amount).toBe(stored); + } + }); + + it('dry run predicts the same refusals and persists nothing', async () => { + const res = await ctx.importRows({ + format: 'json', writeMode: 'insert', dryRun: true, + rows: REFUSED.map((amount, i) => ({ id: `d${i}`, amount })), + }); + + expect(res._json).toMatchObject({ dryRun: true, total: REFUSED.length, ok: 0, errors: REFUSED.length }); + for (const r of res._json.results) expect(r).toMatchObject({ ok: false, field: 'amount', code: 'invalid_number' }); + for (const [i] of REFUSED.entries()) { + expect(await ctx.engine.findOne(OBJECT, { where: { id: `d${i}` } })).toBeNull(); + } + }); + + it('answers the code the plain create door answers for the same cells', async () => { + for (const [i, amount] of REFUSED.entries()) { + const res = await ctx.create({ id: `p${i}`, amount }); + expect(res._status).toBe(400); + expect(res._json).toMatchObject({ code: 'VALIDATION_FAILED' }); + expect(res._json.fields[0]).toMatchObject({ field: 'amount', code: 'invalid_number' }); + } + }); +}); diff --git a/packages/rest/vitest.config.ts b/packages/rest/vitest.config.ts index 6561f301ccb..82b0e285c0a 100644 --- a/packages/rest/vitest.config.ts +++ b/packages/rest/vitest.config.ts @@ -208,6 +208,14 @@ export default defineConfig({ find: /^@objectstack\/service-datasource$/, replacement: path.resolve(__dirname, '../services/service-datasource/src/index.ts'), }, + // #20497 — `import-number-thousands-group.test.ts` drives `/import` on + // `InMemoryDriver` beside `SqlDriver`. Aliased to source for the same + // reason as the two above: a new unaliased artifact import is refused by + // `scripts/check-test-source-alias.mjs`, whose ledger is shrink-only. + { + find: /^@objectstack\/driver-memory$/, + replacement: path.resolve(__dirname, '../drivers/driver-memory/src/index.ts'), + }, ], }, }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9e73b448d03..f80c8bcdbd0 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2209,6 +2209,9 @@ importers: specifier: ^4.6.1 version: 4.6.1 devDependencies: + '@objectstack/driver-memory': + specifier: workspace:* + version: link:../drivers/driver-memory '@objectstack/driver-sql': specifier: workspace:* version: link:../drivers/driver-sql From 17d80ca18c724ceb604d0d8b8df457f1497fcf64 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <50353452+hotlong@users.noreply.github.com> Date: Tue, 29 Sep 2026 05:09:18 +0800 Subject: [PATCH 2/4] test(rest): pin each refused cell and each admitted control as its own case Claude-Session: https://claude.ai/code/session_local_1d2a197c-c20e-4e90-9be8-413d4d432289 Co-authored-by: Claude --- packages/rest/src/import-coerce.test.ts | 2 +- .../src/import-number-thousands-group.test.ts | 36 ++++++++++--------- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/packages/rest/src/import-coerce.test.ts b/packages/rest/src/import-coerce.test.ts index f5317ae7bef..361f6b4b3f6 100644 --- a/packages/rest/src/import-coerce.test.ts +++ b/packages/rest/src/import-coerce.test.ts @@ -61,7 +61,7 @@ describe('parseNumberCell', () => { ['$1,000', 1000], ['1,234%', 1234], ['1,000e3', 1_000_000], - ])('admits the well-formed thousands grouping %j as %d', (cell, n) => { + ])('admits the well-formed thousands grouping %j as %s', (cell, n) => { expect(parseNumberCell(cell)).toBe(n); }); diff --git a/packages/rest/src/import-number-thousands-group.test.ts b/packages/rest/src/import-number-thousands-group.test.ts index c89b674099f..034467a6848 100644 --- a/packages/rest/src/import-number-thousands-group.test.ts +++ b/packages/rest/src/import-number-thousands-group.test.ts @@ -114,29 +114,31 @@ describe.each(DRIVERS)('[#20497] /import — a comma is read only as a thousands let ctx: Awaited>; beforeEach(async () => { ctx = await boot(makeDriver); }); - it('refuses each decimal-comma / stray-comma cell per row and stores the admitted controls', async () => { - const cells = [...REFUSED, ...ADMITTED.map(([cell]) => cell)]; + it.each(REFUSED)('refuses %j as that row\'s invalid_number error and writes its sibling row', async (cell) => { const res = await ctx.importRows({ format: 'json', writeMode: 'insert', - rows: cells.map((amount, i) => ({ id: `r${i}`, amount })), + rows: [{ id: 'bad', amount: cell }, { id: 'good', amount: 7 }], }); expect(res._status ?? 200).toBe(200); - expect(res._json).toMatchObject({ - total: cells.length, ok: ADMITTED.length, errors: REFUSED.length, created: ADMITTED.length, + expect(res._json).toMatchObject({ total: 2, ok: 1, errors: 1, created: 1 }); + expect(res._json.results[0]).toMatchObject({ + row: 1, ok: false, action: 'failed', field: 'amount', code: 'invalid_number', }); - for (const [i] of REFUSED.entries()) { - expect(res._json.results[i]).toMatchObject({ - row: i + 1, ok: false, action: 'failed', field: 'amount', code: 'invalid_number', - }); - // Refused, not stored as some other number. - expect(await ctx.engine.findOne(OBJECT, { where: { id: `r${i}` } })).toBeNull(); - } - for (const [j, [, stored]] of ADMITTED.entries()) { - const i = REFUSED.length + j; - expect(res._json.results[i]).toMatchObject({ row: i + 1, ok: true, action: 'created' }); - expect((await ctx.engine.findOne(OBJECT, { where: { id: `r${i}` } }))?.amount).toBe(stored); - } + // Refused, not stored as some other number. + expect(await ctx.engine.findOne(OBJECT, { where: { id: 'bad' } })).toBeNull(); + expect((await ctx.engine.findOne(OBJECT, { where: { id: 'good' } }))?.amount).toBe(7); + }); + + it.each(ADMITTED)('admits the thousands grouping %j and stores %s', async (cell, stored) => { + const res = await ctx.importRows({ + format: 'json', writeMode: 'insert', + rows: [{ id: 'r', amount: cell }], + }); + + expect(res._json).toMatchObject({ total: 1, ok: 1, errors: 0, created: 1 }); + expect(res._json.results[0]).toMatchObject({ row: 1, ok: true, action: 'created' }); + expect((await ctx.engine.findOne(OBJECT, { where: { id: 'r' } }))?.amount).toBe(stored); }); it('gives the same verdicts to quoted CSV cells', async () => { From b5dc4ee2e203927a579b6927d5d1f3f9ac84d873 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <50353452+hotlong@users.noreply.github.com> Date: Tue, 29 Sep 2026 05:11:15 +0800 Subject: [PATCH 3/4] =?UTF-8?q?chore(changeset):=20rest=20minor,=20Clause-?= =?UTF-8?q?=E2=91=A1=20no=20(narrowing),=20with=20the=20FROM=20->=20TO=20f?= =?UTF-8?q?or=20each=20refused=20cell=20shape?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_local_1d2a197c-c20e-4e90-9be8-413d4d432289 Co-authored-by: Claude --- .../20497-import-number-thousands-group.md | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 .changeset/20497-import-number-thousands-group.md diff --git a/.changeset/20497-import-number-thousands-group.md b/.changeset/20497-import-number-thousands-group.md new file mode 100644 index 00000000000..c48ead9c197 --- /dev/null +++ b/.changeset/20497-import-number-thousands-group.md @@ -0,0 +1,47 @@ +--- +'@objectstack/rest': minor +--- + +fix(rest): `POST /api/v1/data/:object/import` reads a comma in a number cell only as a thousands group, and refuses every other comma instead of storing a different number (#20497) + +Clause-②: no (narrowing) + +**BREAKING for callers of the import door.** A cell for a numeric field +(`number`, `currency`, `percent` and the other numeric value types) that +carries a comma is now admitted only when the comma groups thousands: 1 to 3 +leading digits, then groups of exactly three digits, and only before any `.` +(`1,000`, `12,345.67`, `-1,234,567.89`). Any other +comma makes the cell that row's `invalid_number` error, the same code the +plain write doors (create, update, batch) already answer for the cell. The +reader used to strip every comma before parsing, so each of these was stored +as a DIFFERENT number while the import reported `ok 1, errors 0`. For each +shape, change the cell FROM the refused spelling TO the one that says what you +mean: + +- **A decimal comma.** FROM `3,14`, `1,5`, `0,5`, `1.000,5` (stored as `314`, + `15`, `5`, `1.0005`) TO a `.` decimal point with no grouping, or with comma + grouping: `3.14`, `1.5`, `0.5`, `1000.5` or `1,000.5`. A file exported with a + decimal-comma locale has to be converted before import. No locale is + guessed: `1,500` is always one thousand five hundred. +- **A comma that does not group thousands.** FROM `1,2,3`, `1,23`, `1,0000`, + `1234,567`, `,123`, `1,000,` or a comma after the `.` (`12,345.6,7`), each + stored with its commas removed, TO the number with no separators, or with + well-formed thousands grouping. +- **Grouping by twos (`12,34,567`, `1,00,000`).** FROM that grouping TO + `1234567` / `100000`, or `1,234,567` / `100,000`. These used to import as + the number they denote. They are refused now because the reader cannot tell + a two-digit group from a decimal comma, and it no longer guesses. + +**What is not affected.** Every cell without a comma reads exactly as before. +That is measured over the platform numeric grammar's 41 case rows: the only +row whose import reading changed is `1.000,5`. A well-formed thousands grouping +reads as before, with or without a leading currency symbol, a trailing `%`, a +sign or accounting parentheses (`$1,000`, `1,234%`, `(1,234)`). A JSON number, +and an xlsx cell Excel stores as a number, never pass through this reading. +The dry run answers the same verdicts as the real write. A refused cell fails +only its own row, and the rest of the batch imports as before. + +**If you are refused.** The row's result carries `code: 'invalid_number'` and +quotes the cell, so the file can be corrected and re-imported. + + From c76a3c95f27e053b6c9fe4f9066d7b06d5860325 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <50353452+hotlong@users.noreply.github.com> Date: Tue, 29 Sep 2026 05:16:31 +0800 Subject: [PATCH 4/4] test(rest): pin the import door on SqlDriver only; no new driver-memory test consumer The driver-memory test consumers are a ruled, ledgered set (scripts/driver-memory-census.ledger.json): a new one needs a maintainer ruling, so the InMemoryDriver arm, its devDependency and its vitest alias come back out. The cell is judged before any driver is reached; the memory reading stays recorded as a measurement in the pin's header. Claude-Session: https://claude.ai/code/session_local_1d2a197c-c20e-4e90-9be8-413d4d432289 Co-authored-by: Claude --- packages/rest/package.json | 1 - .../src/import-number-thousands-group.test.ts | 48 +++++++++++-------- packages/rest/vitest.config.ts | 8 ---- pnpm-lock.yaml | 3 -- 4 files changed, 27 insertions(+), 33 deletions(-) diff --git a/packages/rest/package.json b/packages/rest/package.json index d8829d51734..5885f560b75 100644 --- a/packages/rest/package.json +++ b/packages/rest/package.json @@ -39,7 +39,6 @@ "zod": "^4.6.1" }, "devDependencies": { - "@objectstack/driver-memory": "workspace:*", "@objectstack/driver-sql": "workspace:*", "@objectstack/metadata": "workspace:*", "@objectstack/metadata-protocol": "workspace:*", diff --git a/packages/rest/src/import-number-thousands-group.test.ts b/packages/rest/src/import-number-thousands-group.test.ts index 034467a6848..3820e2a21b1 100644 --- a/packages/rest/src/import-number-thousands-group.test.ts +++ b/packages/rest/src/import-number-thousands-group.test.ts @@ -4,31 +4,38 @@ * [#20497] `POST /api/v1/data/:object/import` reads a comma in a number cell * as a thousands separator ONLY where it groups thousands — 1 to 3 leading * digits, then groups of exactly three, and only before any `.` — and refuses - * every other comma as the row's `invalid_number` error, on `InMemoryDriver` - * and on `SqlDriver` (better-sqlite3 `:memory:`), through the real route. + * every other comma as that row's `invalid_number` error, through the real + * route over a real `SqlDriver` (better-sqlite3 `:memory:`). * - * Measured on the base (`9449512a31`) through this route, JSON rows, - * `writeMode: 'insert'`, both drivers alike: + * Measured through this route, JSON rows, `writeMode: 'insert'`, on + * `InMemoryDriver` and on `SqlDriver` (better-sqlite3) alike, at the base + * (`9449512a31`) and at the head of the PR that landed this file: * * | cell | base: stored · answer | head | * |:--|:--|:--| - * | `'3,14'` | `314` · ok 1, errors 0 | row refused, `invalid_number` | - * | `'1,5'` | `15` · ok 1, errors 0 | row refused, `invalid_number` | - * | `'1.000,5'` | `1.0005` · ok 1, errors 0 | row refused, `invalid_number` | - * | `'1,2,3'` | `123` · ok 1, errors 0 | row refused, `invalid_number` | + * | `'3,14'` | `314` · ok 1, errors 0 | row refused, `invalid_number`, nothing stored | + * | `'1,5'` | `15` · ok 1, errors 0 | row refused, `invalid_number`, nothing stored | + * | `'1.000,5'` | `1.0005` · ok 1, errors 0 | row refused, `invalid_number`, nothing stored | + * | `'1,2,3'` | `123` · ok 1, errors 0 | row refused, `invalid_number`, nothing stored | * | `'1,000'` / `'12,345.67'` / `'(1,234)'` | `1000` / `12345.67` / `-1234` | unchanged | * + * The `InMemoryDriver` row is this file's by construction, not by a second + * arm: the cell is judged by the import's own reader (`parseNumberCell`) + * before any driver is reached, so one verdict holds on every driver. A test + * import of `@objectstack/driver-memory` is also not this file's to add — its + * test consumers are a ruled, ledgered set + * (`scripts/driver-memory-census.ledger.json`, `pnpm check:driver-memory-census`). + * * The plain create door already answered `400 VALIDATION_FAILED` with field * code `invalid_number` for each of the four; the import row now answers the - * same code. The reader's own case table (every documented form, the spec - * grammar's rows, the comma probes) is `import-coerce.test.ts`'s - * `parseNumberCell` block; this file pins the door, per driver. + * same code. The reader's own case table (every documented form, the comma + * probes) is `import-coerce.test.ts`'s `parseNumberCell` block; this file pins + * the door. */ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { ObjectQL } from '@objectstack/objectql'; import { SqlDriver } from '@objectstack/driver-sql'; -import { InMemoryDriver } from '@objectstack/driver-memory'; import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; import { RestServer } from './rest-server'; @@ -52,12 +59,11 @@ const ADMITTED: ReadonlyArray = [ ['(1,234)', -1234], ]; -const DRIVERS = [ - ['InMemoryDriver', () => new InMemoryDriver({ persistence: false })], - ['SqlDriver (better-sqlite3)', () => new SqlDriver({ +function makeSqliteDriver() { + return new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true, - })], -] as const; + }); +} function createMockServer() { const noop = () => {}; @@ -81,10 +87,10 @@ afterEach(async () => { } }); -async function boot(makeDriver: () => unknown) { +async function boot() { const engine = new ObjectQL(); liveEngines.push(engine); - engine.registerDriver(makeDriver() as any, true); + engine.registerDriver(makeSqliteDriver(), true); await engine.init(); engine.registry.registerObject(LEDGER as any); await engine.syncSchemas(); @@ -110,9 +116,9 @@ async function boot(makeDriver: () => unknown) { }; } -describe.each(DRIVERS)('[#20497] /import — a comma is read only as a thousands group (%s)', (_name, makeDriver) => { +describe('[#20497] /import — a comma is read only as a thousands group', () => { let ctx: Awaited>; - beforeEach(async () => { ctx = await boot(makeDriver); }); + beforeEach(async () => { ctx = await boot(); }); it.each(REFUSED)('refuses %j as that row\'s invalid_number error and writes its sibling row', async (cell) => { const res = await ctx.importRows({ diff --git a/packages/rest/vitest.config.ts b/packages/rest/vitest.config.ts index 82b0e285c0a..6561f301ccb 100644 --- a/packages/rest/vitest.config.ts +++ b/packages/rest/vitest.config.ts @@ -208,14 +208,6 @@ export default defineConfig({ find: /^@objectstack\/service-datasource$/, replacement: path.resolve(__dirname, '../services/service-datasource/src/index.ts'), }, - // #20497 — `import-number-thousands-group.test.ts` drives `/import` on - // `InMemoryDriver` beside `SqlDriver`. Aliased to source for the same - // reason as the two above: a new unaliased artifact import is refused by - // `scripts/check-test-source-alias.mjs`, whose ledger is shrink-only. - { - find: /^@objectstack\/driver-memory$/, - replacement: path.resolve(__dirname, '../drivers/driver-memory/src/index.ts'), - }, ], }, }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f80c8bcdbd0..9e73b448d03 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2209,9 +2209,6 @@ importers: specifier: ^4.6.1 version: 4.6.1 devDependencies: - '@objectstack/driver-memory': - specifier: workspace:* - version: link:../drivers/driver-memory '@objectstack/driver-sql': specifier: workspace:* version: link:../drivers/driver-sql