diff --git a/.changeset/25078-csv-text-cell.md b/.changeset/25078-csv-text-cell.md new file mode 100644 index 00000000000..1cd854421c1 --- /dev/null +++ b/.changeset/25078-csv-text-cell.md @@ -0,0 +1,21 @@ +--- +'@objectstack/rest': patch +--- + +fix(rest)!: the CSV export writes a text cell its consumer would interpret in a form that consumer reads as text, and the CSV import reverses exactly that + +Clause-②: no (narrowing) — the CSV export serves different bytes for some text cells, and the CSV import parse reads a cell of the same shape differently; nothing is widened. +Metadata-protocol: none (a REST export's bytes and a REST import's parse; nothing a metadata author writes changes) + + + +**BREAKING** (the CSV export's bytes change for some text cells, and the CSV import reads a user-authored cell of the same shape differently), shipped as `patch` because the metadata protocol does not move. + +The CSV export (`GET /api/v1/data/:object/export?format=csv`) wrote every text cell exactly as stored, so a cell whose leading character the program the file is opened in interprets was read there as something other than text. The rule below is defined once, in `csvTextCellNeedsMark` (`packages/rest/src/export-format.ts`), and both directions read it. + +- **The export.** A CSV text cell whose first character is one of a small fixed set that its consumer interprets is written with one apostrophe (U+0027) in front of its content, before the usual RFC-4180 quoting. A text cell is a string in a column whose field is not declared numeric, boolean, date or datetime, a header label, or a reference field's display title; with no field metadata (the raw path) every string cell is one. A cell made only of digits, spaces and the characters a signed number or a phone number is written with is never marked, so phone numbers and signed amounts are byte-identical. +- **The import.** `parseCsvToRows` removes exactly one leading apostrophe from a cell, header row included, when the rest of the cell is one the export would mark. It serves the sync import (`POST /api/v1/data/:object/import`), the async import job (`POST /api/v1/data/:object/import/jobs`) and plugin-auth's admin user import (`POST /api/v1/auth/admin/import-users`), with an inline mapping or a named one. So a CSV exported and imported back returns the stored value, with the one exception in the next point. +- **A cell that begins with an apostrophe** followed by a cell the export would mark now imports without that apostrophe; the file cannot tell it from an exported one. That holds for a user-authored cell, and for a stored text value of that shape, which the export writes unchanged, so it comes back without its leading apostrophe. A cell beginning with an apostrophe followed by anything else imports unchanged. +- **Measured.** A value beginning with TAB or CR is marked; the import removes the mark, and the default `trimWhitespace: true` then trims the leading TAB or CR exactly as it did before. With `trimWhitespace: false` it returns exactly. +- **Unchanged.** Every other cell; numeric, boolean, date and datetime cells; the xlsx and json export formats; json and xlsx import payloads. +- **What to do.** Nothing for a file that goes back through these import doors. A program that reads the CSV export some other way and needs the stored text applies the same reversal: remove one leading apostrophe from a cell whose rest the export would mark, by the rule in this repository's `packages/rest/src/export-format.ts`. diff --git a/packages/plugins/plugin-auth/src/admin-import-users.test.ts b/packages/plugins/plugin-auth/src/admin-import-users.test.ts index ebc770a6ddd..c02a17176f4 100644 --- a/packages/plugins/plugin-auth/src/admin-import-users.test.ts +++ b/packages/plugins/plugin-auth/src/admin-import-users.test.ts @@ -5,6 +5,7 @@ import { readFileSync } from 'node:fs'; import { dirname, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import { assertEngineUpdateDispatch } from '@objectstack/objectql'; +import { RestServer } from '@objectstack/rest'; import { runAdminImportUsers, IMPORT_USERS_MAX_ROWS, type IdentityImportDeps } from './admin-import-users.js'; import { settleImportResetDelivery } from './account-invitation-marker.js'; import type { AdminActor } from './admin-user-endpoints.js'; @@ -719,6 +720,75 @@ describe('runAdminImportUsers — CSV payloads', () => { }); }); +/** + * [#25078] Export → admin user import round trip, through the CSV text-cell rule. + * + * `@objectstack/rest`'s CSV export writes a text cell its consumer would + * interpret in a form that consumer reads as text, and its CSV import parse + * (`parseCsvToRows`, reached through `prepareImportRequest`) reverses exactly + * that. This door parses a `csv` body through that same `prepareImportRequest`, + * so a user list exported as CSV and imported back here must hand better-auth's + * `createUser` every stored name exactly. + * + * The file comes from the REAL export route of `@objectstack/rest`'s + * `RestServer`, over a protocol double serving a two-field `sys_user` schema and + * the rows below. The rule is pinned at its readers in `@objectstack/rest` + * (`csv-text-cell.test.ts`), and its other CSV doors in that package's + * `csv-text-cell-round-trip.test.ts`. + */ +describe('runAdminImportUsers — CSV export → import round trip (#25078)', () => { + /** Plain words after a trigger character: marked by the export. */ + const MARKED = ['=Total', '+Name', '-Note', '@acme']; + /** Ordinary business values, and a user-authored name that begins with the mark itself. */ + const UNMARKED = ['+86 138 0013 8000', '+1 555 010 0000', '-120.50', 'Acme Ltd', '120.50', "'Quoted"]; + const NAMES = [...MARKED, ...UNMARKED]; + + async function exportUsersCsv(rows: Array>): Promise { + const noop = () => {}; + const server = { get: noop, post: noop, put: noop, delete: noop, patch: noop, use: noop, listen: async () => {}, close: async () => {} }; + const schema = { name: 'sys_user', fields: { email: { name: 'email', type: 'email' }, name: { name: 'name', type: 'text' } } }; + const protocol = { + getMetaItem: vi.fn(async () => ({ type: 'object', name: 'sys_user', item: schema })), + findData: vi.fn(async () => ({ object: 'sys_user', records: rows, total: rows.length })), + }; + const rest = new RestServer(server as any, protocol as any, { api: { requireAuth: false } } as any); + (rest as any).resolveExecCtx = async () => ({ userId: ACTOR.id }); + rest.registerRoutes(); + const route: any = rest.getRoutes().find( + (r: any) => r.method === 'GET' && r.path === '/api/v1/data/:object/export', + ); + expect(route).toBeDefined(); + const chunks: string[] = []; + const res: any = { + write: (s: unknown) => { chunks.push(String(s)); return true; }, + end: () => {}, + header: () => res, + status: () => res, + json: () => res, + }; + await route.handler({ params: { object: 'sys_user' }, query: { format: 'csv' } }, res); + return chunks.join(''); + } + + it('hands createUser every exported name exactly', async () => { + const rows = NAMES.map((name, i) => ({ email: `user${i + 1}@example.com`, name })); + const csv = await exportUsersCsv(rows); + + // The file carried the marks, so the round trip below is not a pass-through. + expect(csv.split('\r\n')[0]).toBe('email,name'); + for (const [i, v] of MARKED.entries()) expect(csv).toContain(`user${i + 1}@example.com,'${v}\r\n`); + expect(csv).toContain(',+86 138 0013 8000\r\n'); + expect(csv).toContain(",'Quoted\r\n"); + + const m = makeDeps(); + const res = await runAdminImportUsers(m.deps, makeRequest({ passwordPolicy: 'temporary', csv }), ACTOR); + expect(res.status).toBe(200); + expect((res.body.data as any).summary.created).toBe(NAMES.length); + const byEmail = new Map(m.createUser.mock.calls.map((c: any[]) => [c[0].body.email, c[0].body.name])); + expect(rows.map((r) => byEmail.get(r.email))).toEqual(NAMES); + }); +}); + /** * [#17422] The IMPLEMENTOR half of #16952's contract. * diff --git a/packages/rest/src/csv-text-cell-round-trip.test.ts b/packages/rest/src/csv-text-cell-round-trip.test.ts new file mode 100644 index 00000000000..c6c14eef2ba --- /dev/null +++ b/packages/rest/src/csv-text-cell-round-trip.test.ts @@ -0,0 +1,228 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#25078] Export → import round trip through each REST door that parses CSV. + * + * The CSV export writes a text cell its consumer would interpret in a form that + * consumer reads as text; the CSV import parse reverses exactly that. The rule + * is pinned at its readers in `csv-text-cell.test.ts`. This file drives the + * REAL routes over a REAL `ObjectQL` engine with a better-sqlite3 `:memory:` + * driver: rows are stored in `note`, exported as CSV through + * `GET /data/note/export`, and that file is imported into `note_copy`'s text + * fields through + * + * - the sync door, `POST /data/note_copy/import`, with an inline mapping; + * - the sync door with a NAMED mapping, whose rows keep their source headers + * until core's `applyMappingToRows` — the reversal must not depend on a + * target field's type; + * - the async door, `POST /data/note_copy/import/jobs`. + * + * Every stored value comes back exactly. Each case also asserts that the file + * it imported carried the marks, so the round trip cannot pass by the export + * never having marked anything. plugin-auth's admin user import is pinned in + * its own package (`admin-import-users.test.ts`). + * + * Control characters are written as escapes in this file, never as raw bytes. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { SysMetadataAuditObject, SysMetadataCommitObject, SysMetadataHistoryObject, SysMetadataObject } from '@objectstack/metadata-core'; +import { SysImportJob } from '@objectstack/platform-objects/audit'; +import { RestServer } from './rest-server'; +import { installDefaultOrganization } from './default-organization.testkit.js'; + +const liveEngines: ObjectQL[] = []; +afterEach(async () => { + while (liveEngines.length) { + try { await liveEngines.pop()?.destroy(); } catch { /* noop */ } + } +}); + +/** + * Text fields only, so every value lands in a text field. `memo`'s label starts + * with a trigger character, so the header row carries a marked label too. + */ +function noteObject(name: string) { + return { + name, + label: name, + systemFields: false, + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true, label: 'ID' }, + body: { name: 'body', type: 'text' as const, label: 'Body' }, + memo: { name: 'memo', type: 'text' as const, label: '-Memo' }, + }, + }; +} + +/** Plain words after a trigger character: marked by the export. */ +const MARKED = ['=Total', '+Name', '-Note', '@acme']; +/** Ordinary business values, and a user-authored cell that begins with the mark itself. */ +const UNMARKED = ['+86 138 0013 8000', '+1 555 010 0000', '-120.50', 'Acme Ltd', '120.50', "'Quoted"]; +const VALUES = [...MARKED, ...UNMARKED]; + +const INLINE_MAPPING = { ID: 'id', Body: 'body', '-Memo': 'memo' }; + +function createMockServer() { + const noop = () => {}; + return { get: noop, post: noop, put: noop, delete: noop, patch: noop, use: noop, listen: async () => {}, close: async () => {} }; +} + +function makeRes() { + const chunks: string[] = []; + const res: any = { + write: (s: string) => { chunks.push(typeof s === 'string' ? s : String(s)); return true; }, + end: () => {}, + header: () => res, + status: (code: number) => { res._status = code; return res; }, + json: (body: any) => { res._json = body; return res; }, + }; + return { res, text: () => chunks.join('') }; +} + +async function boot(values: string[]) { + const engine = new ObjectQL(); + liveEngines.push(engine); + engine.registerDriver(new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }), true); + await engine.init(); + engine.registry.registerObject(noteObject('note') as any); + engine.registry.registerObject(noteObject('note_copy') as any); + engine.registry.registerObject(SysImportJob as any); + await engine.syncSchemas(); + // A real `single` install's Default Organization: the system writes below + // derive it (ADR-0131 D9), as a fresh NOT NULL tenant column requires (D1). + await installDefaultOrganization(engine); + for (let i = 0; i < values.length; i++) { + await engine.insert('note', { id: `r${i + 1}`, body: values[i], memo: values[i] }); + } + for (const o of [SysMetadataObject, SysMetadataHistoryObject, SysMetadataAuditObject, SysMetadataCommitObject]) { + if (!engine.registry.getObject(o.name)) engine.registry.registerObject(o as any); + } + engine.registry.registerItem('mapping', { + name: 'note_copy_feed', + label: 'Note copy feed', + sourceFormat: 'csv', + targetObject: 'note_copy', + fieldMapping: [ + { source: 'ID', target: 'id', transform: 'none' }, + { source: 'Body', target: 'body', transform: 'none' }, + { source: '-Memo', target: 'memo', transform: 'none' }, + ], + } as any, 'name'); + const protocol = new ObjectStackProtocolImplementation(engine as any); + const rest = new RestServer(createMockServer() as any, protocol as any, { api: { requireAuth: false } } as any); + (rest as any).resolveExecCtx = async () => ({ userId: 'test-user' }); + rest.registerRoutes(); + const routes = rest.getRoutes(); + const find = (method: string, path: string) => routes.find((r: any) => r.method === method && r.path === path)!; + return { + engine, + exportRoute: find('GET', '/api/v1/data/:object/export'), + importRoute: find('POST', '/api/v1/data/:object/import'), + jobCreate: find('POST', '/api/v1/data/:object/import/jobs'), + jobProgress: find('GET', '/api/v1/data/import/jobs/:jobId'), + }; +} + +type Booted = Awaited>; + +async function exportCsv(ctx: Booted): Promise { + const { res, text } = makeRes(); + await ctx.exportRoute.handler({ params: { object: 'note' }, query: { format: 'csv' } } as any, res); + return text(); +} + +async function importSync(ctx: Booted, body: Record): Promise { + const { res } = makeRes(); + await ctx.importRoute.handler({ params: { object: 'note_copy' }, body } as any, res); + return res._json; +} + +async function importAsync(ctx: Booted, body: Record): Promise { + const { res } = makeRes(); + await ctx.jobCreate.handler({ params: { object: 'note_copy' }, body } as any, res); + expect(res._status).toBe(201); + const jobId = res._json.jobId; + for (let i = 0; i < 200; i++) { + const p = makeRes(); + await ctx.jobProgress.handler({ params: { jobId }, query: {} } as any, p.res); + const status = p.res._json?.status; + if (status === 'succeeded' || status === 'failed' || status === 'cancelled') return p.res._json; + await new Promise((r) => setTimeout(r, 5)); + } + throw new Error(`import job ${jobId} did not finish`); +} + +async function storedCopies(ctx: Booted, count: number): Promise> { + const out: Array<{ body: unknown; memo: unknown }> = []; + for (let i = 1; i <= count; i++) { + const row: any = await ctx.engine.findOne('note_copy', { where: { id: `r${i}` } }); + out.push({ body: row?.body, memo: row?.memo }); + } + return out; +} + +/** The file carried the marks: the round trip below is not a pass-through. */ +function expectMarkedFile(csv: string): void { + expect(csv.split('\r\n')[0]).toBe("ID,Body,'-Memo"); + for (const v of MARKED) expect(csv).toContain(`,'${v},'${v}\r\n`); + for (const v of ['+86 138 0013 8000', '-120.50', 'Acme Ltd']) expect(csv).toContain(`,${v},${v}\r\n`); +} + +describe('CSV export → import round trip, every value returns exactly (#25078)', () => { + let ctx: Booted; + beforeEach(async () => { + ctx = await boot(VALUES); + }); + + it('sync door, inline mapping', async () => { + const csv = await exportCsv(ctx); + expectMarkedFile(csv); + const res = await importSync(ctx, { format: 'csv', csv, mapping: INLINE_MAPPING }); + expect(res).toMatchObject({ total: VALUES.length, ok: VALUES.length, errors: 0, created: VALUES.length }); + expect(await storedCopies(ctx, VALUES.length)).toEqual(VALUES.map((v) => ({ body: v, memo: v }))); + }); + + it('sync door, named mapping (reversal runs before the mapping renames the columns)', async () => { + const csv = await exportCsv(ctx); + expectMarkedFile(csv); + const res = await importSync(ctx, { format: 'csv', csv, mappingName: 'note_copy_feed' }); + expect(res).toMatchObject({ total: VALUES.length, ok: VALUES.length, errors: 0, created: VALUES.length }); + expect(await storedCopies(ctx, VALUES.length)).toEqual(VALUES.map((v) => ({ body: v, memo: v }))); + }); + + it('async import-job door', async () => { + const csv = await exportCsv(ctx); + expectMarkedFile(csv); + const done = await importAsync(ctx, { format: 'csv', csv, mapping: INLINE_MAPPING }); + expect(done).toMatchObject({ status: 'succeeded', total: VALUES.length, created: VALUES.length, errors: 0 }); + expect(await storedCopies(ctx, VALUES.length)).toEqual(VALUES.map((v) => ({ body: v, memo: v }))); + }); +}); + +describe('CSV round trip of a value that begins with TAB or CR (#25078)', () => { + const LEADING_WHITESPACE = ['\tTotal', '\rTotal']; + let ctx: Booted; + beforeEach(async () => { + ctx = await boot(LEADING_WHITESPACE); + }); + + it('the file carries the mark, and `trimWhitespace: false` returns the stored value exactly', async () => { + const csv = await exportCsv(ctx); + expect(csv).toContain("r1,'\tTotal,'\tTotal\r\n"); + expect(csv).toContain(`r2,"'\rTotal","'\rTotal"\r\n`); + const res = await importSync(ctx, { format: 'csv', csv, mapping: INLINE_MAPPING, trimWhitespace: false }); + expect(res).toMatchObject({ ok: 2, errors: 0 }); + expect(await storedCopies(ctx, 2)).toEqual(LEADING_WHITESPACE.map((v) => ({ body: v, memo: v }))); + }); + + it("under the default `trimWhitespace`, core's text trim still removes the leading TAB / CR after the reversal", async () => { + const csv = await exportCsv(ctx); + const res = await importSync(ctx, { format: 'csv', csv, mapping: INLINE_MAPPING }); + expect(res).toMatchObject({ ok: 2, errors: 0 }); + expect(await storedCopies(ctx, 2)).toEqual([{ body: 'Total', memo: 'Total' }, { body: 'Total', memo: 'Total' }]); + }); +}); diff --git a/packages/rest/src/csv-text-cell.test.ts b/packages/rest/src/csv-text-cell.test.ts new file mode 100644 index 00000000000..2c2fa3d4a7c --- /dev/null +++ b/packages/rest/src/csv-text-cell.test.ts @@ -0,0 +1,271 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#25078] The CSV text-cell rule, pinned at its three readers. + * + * The CSV export writes a text cell its consumer would interpret in a form that + * consumer reads as text, and the CSV import reverses exactly that. The rule is + * defined once (`csvTextCellNeedsMark` in `./export-format.ts`); this file pins + * + * 1. the predicate, as one table, boundary characters included; + * 2. the writer (`rowsToCsv`), per value, on the metadata path and on the raw + * path (empty meta map): which cells it marks, which it leaves + * byte-identical, and that a column's DECLARED type decides, never the + * value's JS type; + * 3. the CSV parse (`parseCsvToRows`), which removes exactly one mark from a + * cell whose remainder the rule would have marked, header row included. + * + * The round trip through each import door that parses CSV is pinned in + * `csv-text-cell-round-trip.test.ts` (sync, async job, named mapping) and in + * plugin-auth's `admin-import-users.test.ts` (admin user import). + * + * Control characters are written as escapes in this file, never as raw bytes. + */ + +import { describe, it, expect } from 'vitest'; +import { + CSV_TEXT_CELL_MARK, + csvTextCellNeedsMark, + markCsvTextCell, + unmarkCsvTextCell, + isCsvTextColumn, + type ExportFieldMeta, +} from './export-format'; +import { rowsToCsv } from './rest-server'; +import { parseCsvToRows } from './import-prepare'; + +/** Values the rule marks: a plain word after each trigger character. */ +const MARKED = ['=Total', '+Name', '-Note', '@acme', '\tTotal', '\rTotal']; + +/** Ordinary business values the rule leaves byte-identical. */ +const UNMARKED_BUSINESS = ['+86 138 0013 8000', '+1 555 010 0000', '-120.50', 'Acme Ltd', '120.50']; + +describe('csvTextCellNeedsMark — the one predicate (#25078)', () => { + // [text, marked?, why] + const TABLE: Array<[string, boolean, string]> = [ + // ── the six trigger characters, each before a plain word ── + ['=Total', true, 'leads with = and holds a letter'], + ['+Name', true, 'leads with + and holds a letter'], + ['-Note', true, 'leads with - and holds a letter'], + ['@acme', true, 'leads with @ and holds a letter'], + ['\tTotal', true, 'leads with TAB'], + ['\rTotal', true, 'leads with CR'], + // ── a trigger character alone: = @ TAB CR are themselves outside the set ── + ['=', true, '= is outside the numeric-text set'], + ['@', true, '@ is outside the numeric-text set'], + ['\t', true, 'TAB is outside the numeric-text set'], + ['\r', true, 'CR is outside the numeric-text set'], + ['+', false, '+ alone is all numeric-text characters'], + ['-', false, '- alone is all numeric-text characters'], + // ── second half: every character of the numeric-text set, after + and - ── + ['+0', false, 'digit 0'], ['+9', false, 'digit 9'], ['+ ', false, 'space'], + ['++', false, 'plus'], ['+-', false, 'minus'], ['+(', false, 'open paren'], + ['+)', false, 'close paren'], ['+.', false, 'dot'], ['+,', false, 'comma'], + ['-0', false, 'digit 0'], ['-9', false, 'digit 9'], ['- ', false, 'space'], + ['-+', false, 'plus'], ['--', false, 'minus'], ['-(', false, 'open paren'], + ['-)', false, 'close paren'], ['-.', false, 'dot'], ['-,', false, 'comma'], + // ── second half: one character outside the set anywhere marks the cell ── + ['+A', true, 'a capital letter'], ['-z', true, 'a small letter'], + ['+86 138 0013 8000 ext', true, 'a phone number followed by a word'], + ['-120.50 net', true, 'an amount followed by a word'], + ['+中文', true, 'a non-ASCII letter'], + // ── ordinary business values: unmarked ── + ['+86 138 0013 8000', false, 'a phone number'], + ['+1 555 010 0000', false, 'a phone number'], + ['+1 (555) 010-0000', false, 'a phone number with parentheses'], + ['-120.50', false, 'a signed amount'], + ['-1,200.50', false, 'a signed amount with a group separator'], + ['Acme Ltd', false, 'a name'], + ['120.50', false, 'an unsigned amount'], + ['', false, 'the empty cell'], + // ── first character: the code-point neighbours of each trigger ── + ['Total', false, 'U+003E, after ='], + ['*Name', false, 'U+002A, before +'], [',Name', false, 'U+002C, after + and before -'], + ['.Note', false, 'U+002E, after -'], + ['?acme', false, 'U+003F, before @'], ['Aacme', false, 'U+0041, after @'], + ['\bTotal', false, 'U+0008, before TAB'], ['\nTotal', false, 'U+000A, after TAB'], + ['\fTotal', false, 'U+000C, before CR'], ['\u000eTotal', false, 'U+000E, after CR'], + [' =Total', false, 'a leading space is not a trigger'], + ["'Quoted", false, 'the mark itself is not a trigger'], + ]; + + it.each(TABLE)('%j → %s (%s)', (text, marked) => { + expect(csvTextCellNeedsMark(text)).toBe(marked); + }); + + it('marks with exactly one U+0027', () => { + expect(CSV_TEXT_CELL_MARK).toBe(String.fromCharCode(0x27)); + for (const [text, marked] of TABLE) { + expect(markCsvTextCell(text)).toBe(marked ? `'${text}` : text); + } + }); + + it('the parse half reverses the writer half for every value in the table', () => { + for (const [text] of TABLE) { + expect(unmarkCsvTextCell(markCsvTextCell(text))).toBe(text); + } + }); +}); + +describe('isCsvTextColumn — decided by the declared type (#25078)', () => { + it.each([ + 'number', 'currency', 'percent', 'rating', 'slider', 'progress', 'summary', + 'boolean', 'toggle', 'date', 'datetime', + ])('a %s column is not a text column', (type) => { + expect(isCsvTextColumn({ name: 'f', type })).toBe(false); + }); + + it.each([ + 'text', 'textarea', 'email', 'url', 'phone', 'select', 'multiselect', 'lookup', + 'master_detail', 'user', 'formula', 'autonumber', 'time', 'json', + ])('a %s column is a text column', (type) => { + expect(isCsvTextColumn({ name: 'f', type })).toBe(true); + }); + + it('a column with no metadata, or no declared type, is a text column', () => { + expect(isCsvTextColumn(undefined)).toBe(true); + expect(isCsvTextColumn({ name: 'f' })).toBe(true); + }); +}); + +/** The CSV writer's cells for one data row, header row dropped, split on the line ending. */ +function dataLines(csv: string): string[] { + return csv.split('\r\n').slice(1, -1); +} + +describe('rowsToCsv — the writer table, metadata path (#25078)', () => { + const META = new Map([ + ['id', { name: 'id', type: 'text', label: 'ID' }], + ['body', { name: 'body', type: 'text', label: 'Body' }], + ]); + + it.each(MARKED)('a text cell %j is written with one leading mark', (value) => { + const csv = rowsToCsv(['id', 'body'], [{ id: '1', body: value }], true, META); + const expected = value.startsWith('\r') ? `1,"'${value}"` : `1,'${value}`; + expect(dataLines(csv)).toEqual([expected]); + }); + + it.each(UNMARKED_BUSINESS)('a text cell %j is byte-identical', (value) => { + const csv = rowsToCsv(['id', 'body'], [{ id: '1', body: value }], true, META); + expect(dataLines(csv)).toEqual([`1,${value}`]); + }); + + it('marks BEFORE RFC-4180 quoting: the quote wraps the mark', () => { + const csv = rowsToCsv(['id', 'body'], [{ id: '1', body: '-Note, later' }], true, META); + expect(dataLines(csv)).toEqual([`1,"'-Note, later"`]); + }); + + it('a number / currency / percent field holding -120.5 stays numeric and unprefixed', () => { + const meta = new Map([ + ['n', { name: 'n', type: 'number', label: 'N' }], + ['c', { name: 'c', type: 'currency', label: 'C' }], + ['p', { name: 'p', type: 'percent', label: 'P' }], + ]); + const csv = rowsToCsv(['n', 'c', 'p'], [{ n: -120.5, c: -120.5, p: -120.5 }], true, meta); + expect(csv).toBe('N,C,P\r\n-120.5,-120.5,-120.5\r\n'); + }); + + it('the DECLARED type decides, never the JS type: a string in a numeric, boolean, date or datetime column is never marked', () => { + const meta = new Map([ + ['n', { name: 'n', type: 'number', label: 'N' }], + ['c', { name: 'c', type: 'currency', label: 'C' }], + ['p', { name: 'p', type: 'percent', label: 'P' }], + ['b', { name: 'b', type: 'boolean', label: 'B' }], + ['d', { name: 'd', type: 'date', label: 'D' }], + ['t', { name: 't', type: 'datetime', label: 'T' }], + ]); + const row = { n: '-Note', c: '-Note', p: '-Note', b: '-Note', d: '-Note', t: '-Note' }; + const csv = rowsToCsv(['n', 'c', 'p', 'b', 'd', 't'], [row], true, meta); + expect(dataLines(csv)).toEqual(['-Note,-Note,-Note,-Note,-Note,-Note']); + }); + + it('a JS number in a text column stays a number', () => { + const csv = rowsToCsv(['id', 'body'], [{ id: '1', body: -120.5 }], true, META); + expect(dataLines(csv)).toEqual(['1,-120.5']); + }); + + it('a header label starting with a trigger character is marked', () => { + const meta = new Map([ + ['id', { name: 'id', type: 'text', label: 'ID' }], + ['note', { name: 'note', type: 'number', label: '-Note' }], + ['tag', { name: 'tag', type: 'text', label: '@acme' }], + ]); + const csv = rowsToCsv(['id', 'note', 'tag'], [], true, meta); + expect(csv).toBe("ID,'-Note,'@acme\r\n"); + }); + + it('a reference display title starting with a trigger character is marked', () => { + const meta = new Map([ + ['owner', { name: 'owner', type: 'lookup', label: 'Owner', reference: 'account', displayField: 'name' }], + ]); + const rows = [ + { owner: { id: 'a1', name: '@acme' } }, + { owner: { id: 'a2', name: 'Acme Ltd' } }, + { owner: [{ id: 'a1', name: '@acme' }, { id: 'a3', name: '+86 138 0013 8000' }] }, + ]; + const csv = rowsToCsv(['owner'], rows, true, meta); + expect(dataLines(csv)).toEqual(["'@acme", 'Acme Ltd', `"'@acme, +86 138 0013 8000"`]); + }); +}); + +describe('rowsToCsv — the writer table, raw path (empty meta map) (#25078)', () => { + const RAW = new Map(); + + it.each(MARKED)('a string cell %j is written with one leading mark', (value) => { + const csv = rowsToCsv(['id', 'body'], [{ id: '1', body: value }], true, RAW); + const expected = value.startsWith('\r') ? `1,"'${value}"` : `1,'${value}`; + expect(dataLines(csv)).toEqual([expected]); + }); + + it.each(UNMARKED_BUSINESS)('a string cell %j is byte-identical', (value) => { + const csv = rowsToCsv(['id', 'body'], [{ id: '1', body: value }], true, RAW); + expect(dataLines(csv)).toEqual([`1,${value}`]); + }); + + it('a JS number stays a number', () => { + const csv = rowsToCsv(['id', 'amount'], [{ id: '1', amount: -120.5 }], true, RAW); + expect(dataLines(csv)).toEqual(['1,-120.5']); + }); + + it('the header row is the field names, marked by the same rule', () => { + expect(rowsToCsv(['id', '-note'], [], true, RAW)).toBe("id,'-note\r\n"); + }); +}); + +describe('parseCsvToRows — the import reversal (#25078)', () => { + it.each(MARKED)('a marked cell %j parses back to the value', (value) => { + const csv = rowsToCsv(['id', 'body'], [{ id: '1', body: value }], true, new Map()); + expect(parseCsvToRows(csv)).toEqual([{ id: '1', body: value }]); + }); + + it.each(UNMARKED_BUSINESS)('an unmarked cell %j parses unchanged', (value) => { + expect(parseCsvToRows(`id,body\r\n1,"${value}"\r\n`)).toEqual([{ id: '1', body: value }]); + }); + + it('removes exactly one mark, and only when the remainder satisfies the rule', () => { + const csv = [ + 'id,body', + "1,'Quoted", + "2,'+86 138 0013 8000", + "3,''=Total", + "4,'-Note", + "5,'", + ].join('\r\n'); + expect(parseCsvToRows(csv).map((r) => r.body)).toEqual([ + "'Quoted", + "'+86 138 0013 8000", + "''=Total", + '-Note', + "'", + ]); + }); + + it('a user-authored cell that is a mark before a rule-satisfying remainder imports without that mark', () => { + expect(parseCsvToRows("id,body\r\n1,'@acme\r\n")).toEqual([{ id: '1', body: '@acme' }]); + }); + + it('reverses a marked header BEFORE the mapping lookup, so a mapping names the label as exported', () => { + const csv = "ID,'-Note\r\n1,'=Total\r\n"; + expect(parseCsvToRows(csv, { ID: 'id', '-Note': 'body' })).toEqual([{ id: '1', body: '=Total' }]); + }); +}); diff --git a/packages/rest/src/data-action-routes.ts b/packages/rest/src/data-action-routes.ts index 53f7b7e9e90..67e7b22dc84 100644 --- a/packages/rest/src/data-action-routes.ts +++ b/packages/rest/src/data-action-routes.ts @@ -888,7 +888,14 @@ export function registerDataActionEndpoints(this: DataActionRouteHost, basePath: // user fields resolve to a name (via injected $expand), select fields to // their option label, booleans to the caller's yes / no words, dates to // YYYY-MM-DD. When the schema is unavailable the raw stored values stream - // through unchanged. + // through unchanged, but for the csv rule below. + // + // [#25078] csv only, on both paths: a text cell its consumer would + // interpret is written in a form that consumer reads as text, by the CSV + // text-cell rule (`markCsvTextCell`, `./export-format.ts`), and the CSV + // import parse reverses exactly that (`parseCsvToRows`), so the file + // re-imports to the stored value (one exception: `unmarkCsvTextCell`). + // xlsx and json are unaffected. // // [#24945] Boolean / toggle cells are written in the language of the // header row: `templateText(locale).booleanWords` (yes / no, or 是 / 否 for diff --git a/packages/rest/src/export-format.ts b/packages/rest/src/export-format.ts index 42ece978e36..0a787e5eab9 100644 --- a/packages/rest/src/export-format.ts +++ b/packages/rest/src/export-format.ts @@ -28,10 +28,22 @@ * Fourth contract, on the year (#20602): every `date` and `datetime` cell takes * its day from core's `temporalStorageForm` `date` rule — see * {@link calendarDay} — so the year keeps four digits and the export re-imports. + * + * Fifth contract, CSV only (#25078): the value formatters above stay + * pass-throughs on the raw path, but the CSV writer applies the CSV text-cell + * rule ({@link markCsvTextCell}) to every text cell on BOTH paths, and the CSV + * import parse reverses it ({@link unmarkCsvTextCell}). xlsx and json never + * read it. */ import { temporalStorageForm, type ExportFieldMeta } from '@objectstack/core'; -import { resolveDisplayField } from '@objectstack/spec/data'; +import { + resolveDisplayField, + NUMERIC_VALUE_TYPES, + BOOLEAN_VALUE_TYPES, + CALENDAR_DATE_TYPES, + INSTANT_TYPES, +} from '@objectstack/spec/data'; import { templateText, type TemplateText } from './import-template.js'; // `ExportFieldMeta` and `buildFieldMetaMap` live in `@objectstack/core` @@ -191,6 +203,95 @@ export function headerLabel(field: string, metaMap: Map return metaMap.get(field)?.label || field; } +// ───────────────────────────────────────────────────────────────────────────── +// [#25078] The CSV text-cell rule — defined ONCE, read by both directions. +// +// The CSV export writes a TEXT cell that the program its consumer opens the +// file in would interpret, rather than read as text, in a form that program +// reads as text: one {@link CSV_TEXT_CELL_MARK} in front of the cell's content +// ({@link markCsvTextCell}, called by the CSV writer `rowsToCsv` in +// `./rest-server.ts`). The CSV import parse (`parseCsvToRows` in +// `./import-prepare.ts`) reverses exactly that ({@link unmarkCsvTextCell}), so +// an export → import round trip returns the stored value, with the one +// exception {@link unmarkCsvTextCell} states. +// +// Scope, by construction: CSV only — the xlsx and json formats and the json +// import door never read this rule. Which cells are TEXT cells is the writer's +// question and is answered by the field's DECLARED type +// ({@link isCsvTextColumn}), never by the value's JS type. The import side +// cannot ask the same question before a named mapping has renamed its columns +// (core's `applyMappingToRows` runs after the parse), so it asks only what the +// cell's text says: the trigger is a property of the text alone, and the set of +// cells it can reverse is exactly the set the export can have marked. +// ───────────────────────────────────────────────────────────────────────────── + +/** The one character the CSV export puts in front of a text cell it marks (U+0027). */ +export const CSV_TEXT_CELL_MARK = "'"; + +/** + * The first characters that make a CSV text cell one its consumer would + * interpret. TAB and CR are spelled as escapes, never as raw bytes. + */ +const CSV_TEXT_CELL_LEADS: ReadonlySet = new Set(['=', '+', '-', '@', '\t', '\r']); + +/** + * A cell made ONLY of these characters is left as written whatever its first + * character: digits, space, `+`, `-`, `(`, `)`, `.` and `,` — the characters a + * signed amount or a phone number is written with, which stay byte-identical. + */ +const CSV_NUMERIC_TEXT_ONLY = /^[0-9 +\-().,]*$/; + +/** + * The trigger, both halves required: the text's first character is one of + * {@link CSV_TEXT_CELL_LEADS}, AND the text holds at least one character + * outside {@link CSV_NUMERIC_TEXT_ONLY}'s set (the first character counts). + */ +export function csvTextCellNeedsMark(text: string): boolean { + return text.length > 0 && CSV_TEXT_CELL_LEADS.has(text[0]) && !CSV_NUMERIC_TEXT_ONLY.test(text); +} + +/** Export direction: the text cell as the CSV writer writes it, before RFC-4180 quoting. */ +export function markCsvTextCell(text: string): string { + return csvTextCellNeedsMark(text) ? CSV_TEXT_CELL_MARK + text : text; +} + +/** + * Import direction: removes exactly ONE leading {@link CSV_TEXT_CELL_MARK} + * when the remainder satisfies {@link csvTextCellNeedsMark}, and returns every + * other cell unchanged. Applied to every cell of a parsed CSV, header row + * included, so a user-authored cell of that same shape loses its leading mark + * too: the file cannot tell the two apart, and the rule is one rule. The same + * holds for a STORED text value of that shape — the writer leaves it as is + * (its first character is the mark, not a trigger), so it is the one value an + * export → import round trip does not return unchanged. + */ +export function unmarkCsvTextCell(cell: string): string { + return cell.startsWith(CSV_TEXT_CELL_MARK) && csvTextCellNeedsMark(cell.slice(CSV_TEXT_CELL_MARK.length)) + ? cell.slice(CSV_TEXT_CELL_MARK.length) + : cell; +} + +/** + * Declared types whose CSV cell is NOT a text cell: numeric, boolean, date and + * datetime — the spec's own value classes, composed rather than re-listed. + */ +const CSV_NON_TEXT_TYPES: ReadonlySet = new Set([ + ...NUMERIC_VALUE_TYPES, + ...BOOLEAN_VALUE_TYPES, + ...CALENDAR_DATE_TYPES, + ...INSTANT_TYPES, +]); + +/** + * Whether a column's string cells are TEXT cells under the CSV text-cell rule: + * every column whose field is not declared numeric, boolean, date or datetime, + * reference display titles included. A column with no metadata (the raw path, + * an empty meta map, or a field the map does not hold) is a text column. + */ +export function isCsvTextColumn(meta: ExportFieldMeta | undefined): boolean { + return !meta?.type || !CSV_NON_TEXT_TYPES.has(meta.type); +} + function pad2(n: number): string { return n < 10 ? `0${n}` : String(n); } diff --git a/packages/rest/src/import-prepare.ts b/packages/rest/src/import-prepare.ts index 7f349a61f36..9d9ccc7c1aa 100644 --- a/packages/rest/src/import-prepare.ts +++ b/packages/rest/src/import-prepare.ts @@ -10,6 +10,7 @@ import { buildFieldMetaMap, + unmarkCsvTextCell, type ExportFieldMeta, } from './export-format.js'; import { @@ -27,6 +28,18 @@ import { asXlsxLoadInput, loadExcelJs } from './xlsx-module.js'; * argument (e.g. `{ "First Name": "first_name" }`); unmapped headers * pass through unchanged. Empty cells become empty strings. * + * [#25078] Every parsed cell, header row included, is read through + * `unmarkCsvTextCell` (`./export-format.ts`), the import half of the CSV + * text-cell rule the CSV export writes with: it removes exactly one leading + * mark from a cell whose remainder the rule would have marked, and leaves every + * other cell as parsed. It runs HERE, before any mapping, because it reads the + * cell's text alone — a named mapping's rows keep their source headers until + * core's `applyMappingToRows`, so no target field type is known yet. Header + * cells are reversed before they are trimmed and looked up in `mapping`, so a + * mapping names the header as the export labelled it. Serves every CSV door + * that parses through `prepareImportRequest`: the sync import, the async import + * job and plugin-auth's admin user import. + * * Kept dependency-free so REST stays runtime-portable (Hono / Express * adapters both consume this without pulling a CSV lib transitively). */ @@ -64,7 +77,7 @@ export function parseCsvToRows(csv: string, mapping: Record = {} while (cells.length > 0 && cells[cells.length - 1].every(c => c === '')) cells.pop(); if (cells.length < 2) return []; - const header = cells[0].map(h => h.trim()); + const header = cells[0].map(h => unmarkCsvTextCell(h).trim()); const fields = header.map(h => mapping[h] ?? h); const out: Array> = []; for (let r = 1; r < cells.length; r++) { @@ -74,7 +87,7 @@ export function parseCsvToRows(csv: string, mapping: Record = {} const key = fields[c]; if (!key) continue; const raw = row[c] ?? ''; - obj[key] = raw; + obj[key] = unmarkCsvTextCell(raw); } out.push(obj); } diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index e729af2dec7..4c759f44a64 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -285,6 +285,8 @@ import { headerLabel, formatRowCells, exportContentDisposition, + isCsvTextColumn, + markCsvTextCell, type ExportBooleanWords, type ExportFieldMeta, } from './export-format.js'; @@ -1117,11 +1119,18 @@ export function importJobToSummary(row: any): Record { * quotes doubled. `null` / `undefined` become an empty cell. Objects and * arrays are serialised as compact JSON so nested data round-trips * without flattening surprises. + * + * [#25078] `textCell` says whether the column is a TEXT column under the CSV + * text-cell rule (`isCsvTextColumn`, `./export-format.ts`, decided by the + * field's declared type). A string value in a text column is written through + * `markCsvTextCell` BEFORE the quoting below; every other value — a number, + * a boolean, a `Date`, serialised JSON, or a string in a numeric, boolean, + * date or datetime column — is written exactly as before. */ -function formatCsvCell(value: any): string { +function formatCsvCell(value: any, textCell: boolean): string { if (value === null || value === undefined) return ''; let s: string; - if (typeof value === 'string') s = value; + if (typeof value === 'string') s = textCell ? markCsvTextCell(value) : value; else if (typeof value === 'number' || typeof value === 'boolean' || typeof value === 'bigint') s = String(value); // [#14078] Total `Date` arm. An Invalid `Date` renders as `String(value)`, // which is exactly the visible text `Invalid Date` — the cell an operator @@ -1144,8 +1153,13 @@ function formatCsvCell(value: any): string { * list of field names and a (possibly empty) field-metadata map. With metadata, * the header row uses field labels and cell values are formatted to readable * display values (lookup names, select labels, yes / no words, formatted - * dates). With an empty map the output is byte-identical to the raw, - * un-formatted behaviour. + * dates). With an empty map the cells carry the raw, un-formatted values. + * + * [#25078] On both paths every header label and every string cell of a text + * column passes through the CSV text-cell rule (`markCsvTextCell`, + * `./export-format.ts`), which `parseCsvToRows` reverses on import. On the raw + * path every column is a text column; with metadata, a column declared + * numeric, boolean, date or datetime is not (`isCsvTextColumn`). * * `timezone` (#8373) is the caller's business timezone, used to render * `datetime` cells in the same clock the UI shows; absent, they render in UTC. @@ -1161,9 +1175,14 @@ export function rowsToCsv( booleanWords?: ExportBooleanWords, ): string { const lines: string[] = []; - if (includeHeader) lines.push(fields.map(f => formatCsvCell(headerLabel(f, metaMap))).join(',')); + const textColumns = fields.map((f) => isCsvTextColumn(metaMap.get(f))); + if (includeHeader) lines.push(fields.map(f => formatCsvCell(headerLabel(f, metaMap), true)).join(',')); for (const row of rows) { - lines.push(formatRowCells(row, fields, metaMap, timezone, booleanWords).map(formatCsvCell).join(',')); + lines.push( + formatRowCells(row, fields, metaMap, timezone, booleanWords) + .map((cell, i) => formatCsvCell(cell, textColumns[i])) + .join(','), + ); } return lines.join('\r\n') + (lines.length > 0 ? '\r\n' : ''); }