diff --git a/CHANGELOG.md b/CHANGELOG.md index 1bc397c..7f3ed83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,25 @@ Versions follow [Semantic Versioning](https://semver.org/) — minor bump per su week containing the year's first Thursday. Early-January dates correctly report the previous year's week 52/53, and late-December dates week 1 of the next year. Accepts ISO `YYYY-MM-DD` or `MM/DD/YY`; invalid input returns 0. (#44) +- `BROWSE` now validates each cell edit against its column's declared type before + committing. An invalid edit keeps the cell in edit mode, outlines it in red and shows + why (`HH:MM`, `multiple of 15`, `at most 2 decimal place(s)`, `not a real date`, …); + the error clears as soon as the value becomes valid. `DATE`, `TIME`/`TIME(n)`, + `NUM(p,s)`, `INT` and `LOGICAL` are checked; `CHAR`/`MEMO` stay unconstrained. The rules + live in `src/shared/cellValidation.ts` and run on both the client (instant feedback) and + the server (`grid-edit` is now validated authoritatively — previously it wrote straight + to SQLite with no check at all). (#45) + +### Fixed +- `CREATE TABLE t (price NUM(8,2))` silently created a **phantom column named `2`** of + type `)`: the parser read the precision, then treated the scale as the next column + definition. The shipped `PRODUCTS`, `DEALS` and `SALES` demo tables all carried this + stray column. `NUM(p,s)` now parses correctly. Tables created before this fix keep the + stray column until recreated. (#45) +- Column type metadata is now scoped per database. Two databases holding same-named + tables previously shared (and overwrote) one another's declared column types, so a + `TIME(15)` column in one database could be validated against another database's + `CHAR(20)` declaration of the same name. (#45) --- diff --git a/CLAUDE.md b/CLAUDE.md index 1570138..d96fd90 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -51,7 +51,7 @@ server/ ServerDatabaseBridge.ts IDatabaseBridge impl wrapping better-sqlite3 ProgramStore.ts .prg program storage in data/system.sqlite3 IndexStore.ts Index metadata + active index in data/system.sqlite3 - ColumnMetaStore.ts Declared column types (TIME, TIME(n)) in data/system.sqlite3 — SQLite affinity can't distinguish TIME from CHAR/DATE + ColumnMetaStore.ts Declared column types per (db, table, column) in data/system.sqlite3 — SQLite affinity can't distinguish TIME/DATE/CHAR, LOGICAL/INT, or recover NUM(p,s) ReportStore.ts Report definition storage in data/system.sqlite3 (reports table) ReportRunner.ts ASCII and HTML report rendering, group breaks, subtotals, grand totals DemoSeeder.ts Seeds demos/*.prg into the program store and demos/reports/*.json into the report store at startup (demos win) @@ -68,7 +68,7 @@ src/ Terminal.ts REPL UI — command history, multi-line block accumulation ui/ - Grid.ts BROWSE spreadsheet — inline cell editing, keyboard nav + Grid.ts BROWSE spreadsheet — inline cell editing with per-column type validation, keyboard nav FormLayout.ts @ SAY GET form engine — character-cell coordinates ProgramEditor.ts .prg source editor UI ReportPreview.ts iframe-based HTML report preview panel (Esc to close, Ctrl+P to print) @@ -81,7 +81,8 @@ src/ WsClient.ts Browser WebSocket client — sends commands, receives messages shared/ - types.ts Shared TS types (IDatabaseBridge, IIndexStore, WS message shapes) + types.ts Shared TS types (IDatabaseBridge, IIndexStore, IColumnMetaStore, WS message shapes) + cellValidation.ts Declared-type cell validation, shared by Grid.ts (inline UX) and Session (authoritative) main.ts Boot: connect WS → wire terminal/grid/form/editor @@ -111,6 +112,9 @@ tests/ ProgramStore.test.ts AlterTable.test.ts ALTER TABLE + MODIFY STRUCTURE integration tests TimeType.test.ts TIME / TIME(n) columns — creation, structure, write validation + ColumnMeta.test.ts NUM(p,s) parsing, declared types in LIST STRUCTURE, grid-open columnTypes, server-side grid-edit validation + ColumnMetaStore.test.ts Per-(db,table,column) type metadata + legacy-schema migration + CellValidation.test.ts Shared per-type cell validation rules Print.test.ts `?` / `??` print command Aggregate.test.ts `SUM` / `AVERAGE` Builtins.test.ts / BuiltinsParse.test.ts built-in functions (direct + through the parser) @@ -181,7 +185,24 @@ WebBase-III supports **unlimited work areas** (no DOS 10-area limit). Cross-area | `DATE` | | `TEXT` | | `TIME` / `TIME(n)` | | `TEXT` | -`TIME` stores `HH:MM` (24-hour). The optional `TIME(n)` qualifier (only via `CREATE TABLE` — not yet carried through `ALTER TABLE`) requires minutes to be a multiple of `n`, e.g. `TIME(15)` only accepts `:00`/`:15`/`:30`/`:45`. `APPEND RECORD` leaves new fields `NULL` (unvalidated); `REPLACE ... WITH` rejects a malformed or off-granularity `TIME` value with `** Error: ...` and does not write it. `LIST STRUCTURE` prints the declared type (`TIME`, `TIME(15)`) rather than the raw SQLite storage class. +`TIME` stores `HH:MM` (24-hour). The optional `TIME(n)` qualifier (only via `CREATE TABLE` — not carried through `ALTER TABLE`) requires minutes to be a multiple of `n`, e.g. `TIME(15)` only accepts `:00`/`:15`/`:30`/`:45`. `APPEND RECORD` leaves new fields `NULL` (unvalidated); `REPLACE ... WITH` rejects a malformed or off-granularity `TIME` value with `** Error: ...` and does not write it. `LIST STRUCTURE` prints the declared type (`TIME`, `NUM(8,2)`, `TIME(15)`) rather than the raw SQLite storage class. + +Declared types are recorded per `(database, table, column)` in `server/ColumnMetaStore.ts`, because SQLite only keeps a storage affinity: `TIME`/`DATE`/`CHAR` are all `TEXT`, `LOGICAL`/`INT` are both `INTEGER`, and a `NUM(p,s)` qualifier is lost entirely. + +#### Cell validation (`BROWSE`) + +`src/shared/cellValidation.ts` holds the rules and runs on **both** sides: `Grid.ts` checks before commit (an invalid edit keeps the cell in edit mode, outlined red, with the reason shown; the error clears as the value becomes valid), and `Session`'s `grid-edit` handler re-checks authoritatively before writing — a WS message can reach the server without passing through the grid. + +| Type | Accepted | +|---|---| +| `DATE` | `YYYY-MM-DD`, a real calendar date (rejects `2023-02-29`) | +| `TIME` / `TIME(n)` | `HH:MM`; minutes a multiple of `n` when set | +| `NUM(p,s)` | numeric; ≤ `s` decimals, ≤ `p - s` integer digits | +| `INT` | a whole number | +| `LOGICAL` | `.T.`/`.F.`/`.TRUE.`/`.FALSE.`/`T`/`F`/`TRUE`/`FALSE`/`1`/`0` | +| `CHAR` / `MEMO` | anything (length is not enforced) | + +An empty value is always allowed (clears the cell). Columns with no recorded declared type are unconstrained. `REPLACE` enforces only `TIME` — widening it would change the semantics of existing programs. ### Indexing & search | Command | What it does | @@ -293,7 +314,7 @@ line. - ~~`TIME` column type~~ — `TIME`/`TIME(n)` columns storing `HH:MM`, with a minute-granularity qualifier validated on write; declared types tracked in `server/ColumnMetaStore.ts` (#43) ✅ - ~~`WEEK()` built-in~~ — ISO-8601 week number (#44) ✅ -- BROWSE per-cell validation — grid rejects invalid edits per column type (#45) +- ~~BROWSE per-cell validation~~ — grid rejects invalid edits per column type, validated on both client and server via `src/shared/cellValidation.ts` (#45) ✅ - `demos/overtime.prg` — overtime tracker showcasing all three of the above (#46) ## Boolean literals @@ -303,11 +324,11 @@ Both styles accepted: `TRUE`/`FALSE` and `.T.`/`.TRUE.`/`.F.`/`.FALSE.` (dBASE I ## Testing ```bash -npm test # Vitest unit + integration (281 tests) +npm test # Vitest unit + integration (316 tests) npx playwright test # E2E browser tests — requires dev server on :5173/:3000 ``` -Playwright suites (75 tests): `tests/integration.spec.ts` (20 tests — full REPL scenario), `tests/assistant.spec.ts` (21 tests — sidebar, wizards, report designer, MODIFY STRUCTURE round-trip, `TIME(15)` column + REPLACE validation, program run, CSV/SORT/SUM-AVERAGE/REINDEX/PACK actions, demo launchers), `tests/inventory.spec.ts` (8 tests — INVENTORY.prg menu + valuation/low-stock report/sort/CSV/JOIN), `tests/crm.spec.ts` (6 tests — CRM demo menu, pipeline summary, sort, report, CSV, JOIN), `tests/parity-commands.spec.ts` (5 tests — `?`/`??`, built-in functions, `WEEK()`, `SUM`/`AVERAGE`, `SORT ON … TO`), `tests/multiarea.spec.ts` (4 tests — multi-work-area, relations, alias.field), `tests/demos.spec.ts` (4 tests — demo program + report seeding), `tests/copycsv.spec.ts` (2 tests — COPY TO download + APPEND FROM upload), `tests/splash.spec.ts` (2 tests — version banner + demo discoverability), `tests/join.spec.ts` (1 test — JOIN materialization), `tests/propagation.spec.ts` (1 test — live multiuser refresh), `tests/program-side-effects.spec.ts` (1 test — CSV/report side-effects fire from inside a program block). +Playwright suites (79 tests): `tests/assistant.spec.ts` (22 tests — sidebar, wizards, report designer, MODIFY STRUCTURE round-trip, `TIME(15)` column + REPLACE validation, Browse-action grid validation, program run, CSV/SORT/SUM-AVERAGE/REINDEX/PACK actions, demo launchers), `tests/integration.spec.ts` (20 tests — full REPL scenario), `tests/inventory.spec.ts` (8 tests — INVENTORY.prg menu + valuation/low-stock report/sort/CSV/JOIN), `tests/crm.spec.ts` (6 tests — CRM demo menu, pipeline summary, sort, report, CSV, JOIN), `tests/parity-commands.spec.ts` (5 tests — `?`/`??`, built-in functions, `WEEK()`, `SUM`/`AVERAGE`, `SORT ON … TO`), `tests/multiarea.spec.ts` (4 tests — multi-work-area, relations, alias.field), `tests/demos.spec.ts` (4 tests — demo program + report seeding), `tests/grid-validation.spec.ts` (3 tests — BROWSE per-cell validation: TIME(15), NUM(p,s)/DATE, Esc abandons), `tests/copycsv.spec.ts` (2 tests — COPY TO download + APPEND FROM upload), `tests/splash.spec.ts` (2 tests — version banner + demo discoverability), `tests/join.spec.ts` (1 test — JOIN materialization), `tests/propagation.spec.ts` (1 test — live multiuser refresh), `tests/program-side-effects.spec.ts` (1 test — CSV/report side-effects fire from inside a program block). ## Definition of done diff --git a/README.md b/README.md index abaa01b..d2c732c 100644 --- a/README.md +++ b/README.md @@ -229,7 +229,7 @@ WebBase-III supports **unlimited work areas** — each independently holding a t > Column ops that can invalidate an index (DROP, RENAME, ALTER type) drop all of the table's indexes and warn you to rebuild with `INDEX ON`. -**Column types**: `CHAR(n)` (aliases `CHARACTER`/`VARCHAR`/`STRING`/`MEMO`), `NUM` (`NUMERIC`/`FLOAT`/`DOUBLE`/`DECIMAL`), `INT`/`INTEGER`, `LOGICAL`/`BOOLEAN`, `DATE`, and `TIME`/`TIME(n)`. `TIME` stores `HH:MM` (24-hour); the optional `TIME(n)` qualifier (e.g. `TIME(15)`) requires minutes to be a multiple of `n`. `REPLACE ... WITH` rejects a malformed or off-granularity `TIME` value instead of silently coercing it. +**Column types**: `CHAR(n)` (aliases `CHARACTER`/`VARCHAR`/`STRING`/`MEMO`), `NUM`/`NUM(p,s)` (`NUMERIC`/`FLOAT`/`DOUBLE`/`DECIMAL`), `INT`/`INTEGER`, `LOGICAL`/`BOOLEAN`, `DATE`, and `TIME`/`TIME(n)`. `TIME` stores `HH:MM` (24-hour); the optional `TIME(n)` qualifier (e.g. `TIME(15)`) requires minutes to be a multiple of `n`. `REPLACE ... WITH` rejects a malformed or off-granularity `TIME` value instead of silently coercing it, and `LIST STRUCTURE` prints the declared type (`NUM(8,2)`, `TIME(15)`) rather than SQLite's storage class. > **CSV format (`COPY TO` / `APPEND FROM`):** Unlike dBASE III's headerless, > positional `DELIMITED`/`SDF` formats, WebBase-III uses modern **header-based CSV** @@ -380,6 +380,8 @@ Logical operators are accepted in both styles too: `NOT` / `.NOT.`, `AND` / `.AN | F5 | Refresh from DB | | Esc | Exit grid, return to terminal | +> **Cell validation.** An edit is checked against the column's declared type before it commits. An invalid value keeps the cell in edit mode, outlined in red, with the reason shown (`HH:MM`, `multiple of 15`, `at most 2 decimal place(s)`, `not a real date`); the error clears as soon as you fix it, and `Esc` abandons the edit. `DATE`, `TIME`/`TIME(n)`, `NUM(p,s)`, `INT` and `LOGICAL` are validated; `CHAR`/`MEMO` are not. The server re-checks every edit independently. + --- ## Architecture diff --git a/server/ColumnMetaStore.ts b/server/ColumnMetaStore.ts index deef153..ce43b6a 100644 --- a/server/ColumnMetaStore.ts +++ b/server/ColumnMetaStore.ts @@ -6,6 +6,16 @@ import type { IColumnMetaStore, ColumnTypeInfo } from '../src/shared/types.js'; const DATA_DIR = path.join(process.cwd(), 'data'); const DB_PATH = path.join(DATA_DIR, 'system.sqlite3'); +/** + * Declared column types, keyed by (database, table, column). + * + * SQLite only records a storage affinity (TEXT/REAL/INTEGER), which cannot tell + * TIME from DATE from CHAR, LOGICAL from INT, or recover a NUM(p,s) qualifier. + * The grid and REPLACE need the declared type to validate writes. + * + * Scoping by database matters: two databases may each hold a table of the same + * name with different column types. + */ export class ColumnMetaStore implements IColumnMetaStore { private db: Database.Database; @@ -15,51 +25,73 @@ export class ColumnMetaStore implements IColumnMetaStore { this.db.pragma('journal_mode = WAL'); this.db.exec(` CREATE TABLE IF NOT EXISTS column_types ( + db_name TEXT NOT NULL, table_name TEXT NOT NULL, col_name TEXT NOT NULL, base_type TEXT NOT NULL, qualifier INTEGER, - PRIMARY KEY (table_name, col_name) + scale INTEGER, + PRIMARY KEY (db_name, table_name, col_name) ); `); + // v1.2.0 dev migration: the first cut of this table (#43) had neither db_name + // nor scale. The rows only cache what CREATE TABLE re-records, so rebuilding + // is cheaper and safer than back-filling an unscoped key. + const cols = this.db.prepare('PRAGMA table_info(column_types)').all() as { name: string }[]; + if (!cols.some(c => c.name === 'db_name') || !cols.some(c => c.name === 'scale')) { + this.db.exec(` + DROP TABLE column_types; + CREATE TABLE column_types ( + db_name TEXT NOT NULL, + table_name TEXT NOT NULL, + col_name TEXT NOT NULL, + base_type TEXT NOT NULL, + qualifier INTEGER, + scale INTEGER, + PRIMARY KEY (db_name, table_name, col_name) + ); + `); + } } - setColumnType(tableName: string, colName: string, baseType: string, qualifier: number | null): void { + setColumnType(dbName: string, tableName: string, colName: string, baseType: string, qualifier: number | null, scale: number | null): void { this.db.prepare(` - INSERT INTO column_types (table_name, col_name, base_type, qualifier) - VALUES (?, ?, ?, ?) - ON CONFLICT(table_name, col_name) DO UPDATE SET base_type = excluded.base_type, qualifier = excluded.qualifier - `).run(tableName, colName, baseType, qualifier); + INSERT INTO column_types (db_name, table_name, col_name, base_type, qualifier, scale) + VALUES (?, ?, ?, ?, ?, ?) + ON CONFLICT(db_name, table_name, col_name) DO UPDATE SET + base_type = excluded.base_type, qualifier = excluded.qualifier, scale = excluded.scale + `).run(dbName, tableName, colName, baseType, qualifier, scale); } - getColumnType(tableName: string, colName: string): ColumnTypeInfo | null { + getColumnType(dbName: string, tableName: string, colName: string): ColumnTypeInfo | null { const row = this.db.prepare( - 'SELECT base_type AS baseType, qualifier FROM column_types WHERE table_name = ? AND col_name = ?' - ).get(tableName, colName) as ColumnTypeInfo | undefined; + 'SELECT base_type AS baseType, qualifier, scale FROM column_types WHERE db_name = ? AND table_name = ? AND col_name = ?' + ).get(dbName, tableName, colName) as ColumnTypeInfo | undefined; return row ?? null; } - listColumnTypes(tableName: string): Record { + listColumnTypes(dbName: string, tableName: string): Record { const rows = this.db.prepare( - 'SELECT col_name AS colName, base_type AS baseType, qualifier FROM column_types WHERE table_name = ?' - ).all(tableName) as Array; + 'SELECT col_name AS colName, base_type AS baseType, qualifier, scale FROM column_types WHERE db_name = ? AND table_name = ?' + ).all(dbName, tableName) as Array; const out: Record = {}; - for (const r of rows) out[r.colName] = { baseType: r.baseType, qualifier: r.qualifier }; + for (const r of rows) out[r.colName] = { baseType: r.baseType, qualifier: r.qualifier, scale: r.scale }; return out; } - renameColumn(tableName: string, oldName: string, newName: string): void { + renameColumn(dbName: string, tableName: string, oldName: string, newName: string): void { this.db.prepare( - 'UPDATE column_types SET col_name = ? WHERE table_name = ? AND col_name = ?' - ).run(newName, tableName, oldName); + 'UPDATE column_types SET col_name = ? WHERE db_name = ? AND table_name = ? AND col_name = ?' + ).run(newName, dbName, tableName, oldName); } - dropColumn(tableName: string, colName: string): void { - this.db.prepare('DELETE FROM column_types WHERE table_name = ? AND col_name = ?').run(tableName, colName); + dropColumn(dbName: string, tableName: string, colName: string): void { + this.db.prepare('DELETE FROM column_types WHERE db_name = ? AND table_name = ? AND col_name = ?') + .run(dbName, tableName, colName); } - dropTable(tableName: string): void { - this.db.prepare('DELETE FROM column_types WHERE table_name = ?').run(tableName); + dropTable(dbName: string, tableName: string): void { + this.db.prepare('DELETE FROM column_types WHERE db_name = ? AND table_name = ?').run(dbName, tableName); } } diff --git a/server/Session.ts b/server/Session.ts index 1ffd776..b1724b6 100644 --- a/server/Session.ts +++ b/server/Session.ts @@ -7,6 +7,7 @@ import { programStore } from './ProgramStore.js'; import { reportStore } from './ReportStore.js'; import { indexStore } from './IndexStore.js'; import { columnMetaStore } from './ColumnMetaStore.js'; +import { validateCellValue } from '../src/shared/cellValidation.js'; import type { ClientMessage, ServerMessage, ColInfo } from '../src/shared/types.js'; export class Session { @@ -67,6 +68,15 @@ export class Session { const { rowid, col, value } = msg; const table = this.executor.area.table; if (table) { + // Authoritative check — the grid validates client-side for fast + // feedback, but a message can reach here without passing through it. + const db = this.executor.area.db ?? ''; + const err = validateCellValue(col, value, columnMetaStore.getColumnType(db, table, col)); + if (err) { + this.send({ type: 'output', lines: [{ text: `** ${err}`, cls: 'error' }] }); + await this.sendGridData(); + break; + } await this.bridge.exec( `UPDATE ${q(table)} SET ${q(col)} = ? WHERE rowid = ?`, [value, rowid] @@ -346,8 +356,9 @@ export class Session { return; } const columns = await this.bridge.getStructure(area.table); + const columnTypes = columnMetaStore.listColumnTypes(area.db ?? '', area.table); const rows = await this.executor.getOrderedRowsWithIds(2000); - this.send({ type: 'grid-open', table: area.table, filter: area.filter, columns, rows }); + this.send({ type: 'grid-open', table: area.table, filter: area.filter, columns, columnTypes, rows }); } private sendStatus(): void { diff --git a/src/interpreter/Executor.ts b/src/interpreter/Executor.ts index 3452fc1..acf1a4b 100644 --- a/src/interpreter/Executor.ts +++ b/src/interpreter/Executor.ts @@ -1,10 +1,11 @@ -import { IDatabaseBridge, IIndexStore, IColumnMetaStore, OutputLine, FormField, WorkArea, ClientSideEffect } from '../shared/types'; +import { IDatabaseBridge, IIndexStore, IColumnMetaStore, ColumnTypeInfo, OutputLine, FormField, WorkArea, ClientSideEffect } from '../shared/types'; import { ASTNode, Expr, ColDef, Parser } from './Parser'; import { Lexer } from './Lexer'; import { callStateless } from './Builtins'; import { IndexCommands, IndexCommandsHost } from './IndexCommands'; import { ReportCommands } from './ReportCommands'; import { toCSV, parseCSV, MAX_EXPORT_ROWS, MAX_IMPORT_BYTES, MAX_IMPORT_SKIPS } from '../shared/csv'; +import { validateCellValue } from '../shared/cellValidation'; export type { OutputLine, FormField } from '../shared/types'; @@ -42,21 +43,12 @@ function mapType(t: string): DbType { } } -const TIME_RE = /^([01]\d|2[0-3]):([0-5]\d)$/; - -// Throws if `value` is not a well-formed HH:MM string satisfying `qualifier` -// (a minute-granularity requirement, e.g. 15 for TIME(15)). -function validateTimeValue(field: string, value: unknown, qualifier: number | null): void { - if (value === null || value === undefined) return; - const s = String(value); - const m = TIME_RE.exec(s); - if (!m) { - throw new Error(`${field}: invalid TIME value "${s}" — expected HH:MM (00-23:00-59)`); - } - const minutes = Number(m[2]); - if (qualifier && minutes % qualifier !== 0) { - throw new Error(`${field}: TIME value "${s}" violates TIME(${qualifier}) granularity — minutes must be a multiple of ${qualifier}`); - } +/** Render a declared column type the way it was written: NUM(8,2), TIME(15), DATE. */ +function declaredTypeText(info: ColumnTypeInfo | undefined): string | null { + if (!info) return null; + if (info.qualifier === null) return info.baseType; + if (info.scale !== null && info.scale !== undefined) return `${info.baseType}(${info.qualifier},${info.scale})`; + return `${info.baseType}(${info.qualifier})`; } function makeArea(alias: string): WorkArea { @@ -100,6 +92,11 @@ export class Executor implements IndexCommandsHost { return this.areas.get(this.activeAlias)!; } + /** Database key for column metadata — same-named tables in different DBs must not collide. */ + private get metaDb(): string { + return this.area.db ?? ''; + } + /** Backwards-compat shim — Session.ts and legacy tests still read executor.state */ get state(): State { const a = this.area; @@ -385,15 +382,14 @@ export class Executor implements IndexCommandsHost { private async doListStruct(): Promise { this.requireTable(); const cols = await this.db.getStructure(this.area.table!); - const meta = this.columnMetaStore?.listColumnTypes(this.area.table!) ?? {}; + const meta = this.columnMetaStore?.listColumnTypes(this.metaDb, this.area.table!) ?? {}; const out: OutputLine[] = [ { text: `Structure of table: ${this.area.table}`, cls: 'hdr' }, { text: `${'#'.padEnd(4)} ${'Field'.padEnd(20)} ${'Type'.padEnd(10)} ${'Null'.padEnd(5)} ${'PK'}`, cls: 'hdr' }, { text: `${'─'.repeat(55)}`, cls: 'sep' }, ]; cols.forEach(c => { - const info = meta[c.name]; - const typeText = info ? (info.qualifier ? `${info.baseType}(${info.qualifier})` : info.baseType) : c.type; + const typeText = declaredTypeText(meta[c.name]) ?? c.type; out.push({ text: `${String(c.cid + 1).padEnd(4)} ${c.name.padEnd(20)} ${typeText.padEnd(10)} ${c.notnull ? 'NO' : 'YES'.padEnd(5)} ${c.pk ? 'PK' : ''}` }); }); return { output: out }; @@ -432,10 +428,15 @@ export class Executor implements IndexCommandsHost { this.requireTable(); await this.refreshRecCount(); const pairs = fields.map(f => ({ field: f.field, value: this.evalExpr(f.value) })); + // TIME is the only declared type REPLACE enforces (#43). The grid validates + // every declared type (#45) because it has no other guard; widening REPLACE + // would change the semantics of existing programs. for (const p of pairs) { - const info = this.columnMetaStore?.getColumnType(this.area.table!, p.field); + if (p.value === null || p.value === undefined) continue; + const info = this.columnMetaStore?.getColumnType(this.metaDb, this.area.table!, p.field); if (info?.baseType === 'TIME') { - validateTimeValue(p.field, p.value, info.qualifier); + const err = validateCellValue(p.field, String(p.value), info); + if (err) throw new Error(err); } } const setClauses = pairs.map(p => `${q(p.field)} = ?`).join(', '); @@ -774,10 +775,14 @@ export class Executor implements IndexCommandsHost { } const sql = `CREATE TABLE IF NOT EXISTS ${q(name)} (${colsSql})`; await this.db.exec(sql); + // Record the *declared* type of every column. SQLite only stores an affinity + // (TEXT/REAL/INTEGER), which can't tell TIME from DATE from CHAR, LOGICAL from + // INT, or recover a NUM(p,s) precision — the grid needs the declared type to + // validate edits. for (const c of cols) { - if (c.colType.toUpperCase() === 'TIME') { - this.columnMetaStore?.setColumnType(name, c.name, 'TIME', c.size ?? null); - } + this.columnMetaStore?.setColumnType( + this.metaDb, name, c.name, c.colType.toUpperCase(), c.size ?? null, c.scale ?? null, + ); } this.area.table = name; this.area.filter = null; @@ -885,7 +890,7 @@ export class Executor implements IndexCommandsHost { private async doDropTable(name: string): Promise { await this.db.exec(`DROP TABLE IF EXISTS ${q(name)}`); this.indexStore?.dropTable(name); - this.columnMetaStore?.dropTable(name); + this.columnMetaStore?.dropTable(this.metaDb, name); if (this.area.table === name) { this.area.table = null; this.area.activeIndex = null; @@ -911,6 +916,8 @@ export class Executor implements IndexCommandsHost { if (node.op === 'ADD') { if (has(node.col)) return { output: [{ text: `ALTER TABLE: column already exists: ${node.col}`, cls: 'error' }] }; await this.db.exec(`ALTER TABLE ${q(name)} ADD COLUMN ${q(node.col)} ${mapType(node.colType)}`); + // ALTER TABLE drops any (n)/(p,s) qualifier — the parser skips it. + this.columnMetaStore?.setColumnType(this.metaDb, name, node.col, node.colType.toUpperCase(), null, null); await this.refreshIfActive(name); return { output: [{ text: `Added column ${node.col} to ${name}.`, cls: 'ok' }] }; } @@ -919,7 +926,7 @@ export class Executor implements IndexCommandsHost { if (cols.length <= 1) return { output: [{ text: 'ALTER TABLE: cannot drop the only column', cls: 'error' }] }; const dropped = await this.dropAllIndexes(name); await this.db.exec(`ALTER TABLE ${q(name)} DROP COLUMN ${q(node.col)}`); - this.columnMetaStore?.dropColumn(name, node.col); + this.columnMetaStore?.dropColumn(this.metaDb, name, node.col); await this.refreshIfActive(name); return { output: [ { text: `Dropped column ${node.col} from ${name}.`, cls: 'ok' }, @@ -932,7 +939,7 @@ export class Executor implements IndexCommandsHost { if (has(node.newName)) return { output: [{ text: `ALTER TABLE: column already exists: ${node.newName}`, cls: 'error' }] }; const dropped = await this.dropAllIndexes(name); await this.db.exec(`ALTER TABLE ${q(name)} RENAME COLUMN ${q(node.col)} TO ${q(node.newName)}`); - this.columnMetaStore?.renameColumn(name, node.col, node.newName); + this.columnMetaStore?.renameColumn(this.metaDb, name, node.col, node.newName); await this.refreshIfActive(name); return { output: [ { text: `Renamed ${node.col} to ${node.newName} in ${name}.`, cls: 'ok' }, @@ -961,11 +968,7 @@ export class Executor implements IndexCommandsHost { await this.db.exec(`CREATE TABLE ${q(name)} (${colDefs})`); await this.db.exec(`INSERT INTO ${q(name)} SELECT ${colList} FROM ${q(tmp)}`); await this.db.exec(`DROP TABLE ${q(tmp)}`); - if (node.colType.toUpperCase() === 'TIME') { - this.columnMetaStore?.setColumnType(name, node.col, 'TIME', null); - } else { - this.columnMetaStore?.dropColumn(name, node.col); - } + this.columnMetaStore?.setColumnType(this.metaDb, name, node.col, node.colType.toUpperCase(), null, null); await this.refreshIfActive(name); return { output: [ { text: `Changed type of ${node.col} to ${node.colType} in ${name}.`, cls: 'ok' }, diff --git a/src/interpreter/Parser.ts b/src/interpreter/Parser.ts index b38f40d..db4a66f 100644 --- a/src/interpreter/Parser.ts +++ b/src/interpreter/Parser.ts @@ -76,7 +76,7 @@ export type ASTNode = | { type: 'FIND'; value: string } | { type: 'UNKNOWN'; raw: string }; -export interface ColDef { name: string; colType: string; size?: number; } +export interface ColDef { name: string; colType: string; size?: number; scale?: number; } export type Expr = | { k: 'lit'; v: string | number | boolean } @@ -476,12 +476,19 @@ export class Parser { const cname = this.ident(); const ctype = this.ident(); let size: number | undefined; + let scale: number | undefined; if (this.peek().type === 'LPAREN') { this.adv(); size = this.tryNum() ?? undefined; + // NUM(p,s) — the second argument is the scale. Without consuming it the + // comma ends the column and the scale is parsed as the next column name. + if (this.peek().type === 'COMMA') { + this.adv(); + scale = this.tryNum() ?? undefined; + } if (this.peek().type === 'RPAREN') this.adv(); } - cols.push({ name: cname, colType: ctype, size }); + cols.push({ name: cname, colType: ctype, size, scale }); if (this.peek().type === 'COMMA') this.adv(); } if (this.peek().type === 'RPAREN') this.adv(); @@ -506,12 +513,13 @@ export class Parser { throw new Error('Expected ADD, DROP, RENAME, or ALTER after ALTER TABLE '); } - // Consume an optional "(n)" length suffix on a type (e.g. CHAR(20)); the - // length is ignored — SQLite types are not length-bound (matches CREATE TABLE). + // Consume an optional "(n)" or "(p,s)" suffix on a type (e.g. CHAR(20), NUM(8,2)); + // the values are ignored — ALTER TABLE does not carry the qualifier through. private skipTypeSize(): void { if (this.peek().type === 'LPAREN') { this.adv(); this.tryNum(); + if (this.peek().type === 'COMMA') { this.adv(); this.tryNum(); } if (this.peek().type === 'RPAREN') this.adv(); } } diff --git a/src/shared/cellValidation.ts b/src/shared/cellValidation.ts new file mode 100644 index 0000000..6ad5579 --- /dev/null +++ b/src/shared/cellValidation.ts @@ -0,0 +1,89 @@ +/** + * Declared-column-type validation, shared by the browser grid (fast inline + * feedback while typing) and the server (the authoritative check on write). + * + * Both sides must agree, so the rules live here rather than being duplicated. + * Returns an error message, or null when the value is acceptable. + */ + +export interface ColumnMeta { + baseType: string; + qualifier: number | null; // CHAR(n) length, TIME(n) minute granularity, NUM(p,s) precision + scale: number | null; // NUM(p,s) scale +} + +const TIME_RE = /^([01]\d|2[0-3]):([0-5]\d)$/; +const ISO_DATE_RE = /^(\d{4})-(\d{2})-(\d{2})$/; +const LOGICAL_VALUES = new Set(['.T.', '.F.', '.TRUE.', '.FALSE.', 'T', 'F', 'TRUE', 'FALSE', '1', '0']); + +/** True when y-m-d names a real calendar day (rejects Feb 30, month 13, …). */ +export function isRealDate(y: number, m: number, d: number): boolean { + const dt = new Date(Date.UTC(y, m - 1, d)); + return dt.getUTCFullYear() === y && dt.getUTCMonth() === m - 1 && dt.getUTCDate() === d; +} + +export function validateCellValue( + colName: string, + value: string, + meta: ColumnMeta | null | undefined, +): string | null { + if (!meta) return null; // untracked column — no constraint + const v = value.trim(); + if (v === '') return null; // clearing a cell is always allowed + + switch (meta.baseType.toUpperCase()) { + case 'TIME': { + const m = TIME_RE.exec(v); + if (!m) return `${colName}: expected a time as HH:MM (00-23:00-59)`; + if (meta.qualifier && Number(m[2]) % meta.qualifier !== 0) { + return `${colName}: minutes must be a multiple of ${meta.qualifier}`; + } + return null; + } + + case 'DATE': { + const m = ISO_DATE_RE.exec(v); + if (!m) return `${colName}: expected a date as YYYY-MM-DD`; + if (!isRealDate(Number(m[1]), Number(m[2]), Number(m[3]))) { + return `${colName}: "${v}" is not a real date`; + } + return null; + } + + case 'LOGICAL': + case 'BOOLEAN': + return LOGICAL_VALUES.has(v.toUpperCase()) + ? null + : `${colName}: expected a logical value (.T. / .F.)`; + + case 'INT': + case 'INTEGER': + return /^[+-]?\d+$/.test(v) ? null : `${colName}: expected a whole number`; + + case 'NUM': + case 'NUMERIC': + case 'FLOAT': + case 'DOUBLE': + case 'DECIMAL': { + if (!/^[+-]?(\d+(\.\d*)?|\.\d+)$/.test(v)) return `${colName}: expected a number`; + const [intPart, decPart = ''] = v.replace(/^[+-]/, '').split('.'); + const scale = meta.scale ?? 0; + if (meta.scale !== null && decPart.length > meta.scale) { + return `${colName}: at most ${meta.scale} decimal place(s)`; + } + if (meta.qualifier !== null) { + // NUM(p,s): p is total digits, so p - s bounds the integer part. + // NUM(p) with no scale: p bounds the integer part directly. + const maxIntDigits = meta.qualifier - scale; + const digits = intPart.replace(/^0+(?=\d)/, ''); + if (digits.length > maxIntDigits) { + return `${colName}: at most ${maxIntDigits} digit(s) before the decimal point`; + } + } + return null; + } + + default: + return null; // CHAR / MEMO / anything else — unconstrained + } +} diff --git a/src/shared/types.ts b/src/shared/types.ts index fb5e8e0..5118e4f 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -50,21 +50,20 @@ export interface IIndexStore { dropTable(tableName: string): void; } -// Metadata for column types SQLite's own affinity can't distinguish (e.g. TIME -// vs CHAR — both store as TEXT). qualifier carries a type-specific parameter, -// e.g. the minute-granularity in TIME(15). -export interface ColumnTypeInfo { - baseType: string; - qualifier: number | null; -} +// Metadata for the column types SQLite's own affinity can't distinguish (TIME vs +// DATE vs CHAR are all TEXT; LOGICAL vs INT are both INTEGER; NUM(p,s) loses its +// precision/scale). qualifier carries CHAR(n) length / TIME(n) granularity / +// NUM(p,s) precision; scale carries the NUM(p,s) scale. +export type ColumnTypeInfo = import('./cellValidation').ColumnMeta; +// Scoped by database: two databases can hold same-named tables with different types. export interface IColumnMetaStore { - setColumnType(tableName: string, colName: string, baseType: string, qualifier: number | null): void; - getColumnType(tableName: string, colName: string): ColumnTypeInfo | null; - listColumnTypes(tableName: string): Record; - renameColumn(tableName: string, oldName: string, newName: string): void; - dropColumn(tableName: string, colName: string): void; - dropTable(tableName: string): void; + setColumnType(dbName: string, tableName: string, colName: string, baseType: string, qualifier: number | null, scale: number | null): void; + getColumnType(dbName: string, tableName: string, colName: string): ColumnTypeInfo | null; + listColumnTypes(dbName: string, tableName: string): Record; + renameColumn(dbName: string, tableName: string, oldName: string, newName: string): void; + dropColumn(dbName: string, tableName: string, colName: string): void; + dropTable(dbName: string, tableName: string): void; } export interface ReportColumn { @@ -145,7 +144,7 @@ export type ServerMessage = | { type: 'output'; lines: OutputLine[] } | { type: 'status'; db: string | null; table: string | null; record: number; total: number } | { type: 'input-request'; prompt: string } - | { type: 'grid-open'; table: string; filter: string | null; columns: ColInfo[]; rows: Record[] } + | { type: 'grid-open'; table: string; filter: string | null; columns: ColInfo[]; columnTypes: Record; rows: Record[] } | { type: 'modstruct-open'; table: string; columns: ColInfo[] } | { type: 'form-open'; fields: FormField[] } | { type: 'program-open'; name: string; content: string } diff --git a/src/styles/main.css b/src/styles/main.css index da91c23..be6181f 100644 --- a/src/styles/main.css +++ b/src/styles/main.css @@ -240,7 +240,7 @@ html, body { outline-offset: -2px; } -#grid-table td.editing { padding: 0; } +#grid-table td.editing { padding: 0; position: relative; } #grid-table td input.cell-ed { width: 100%; height: 100%; min-height: 25px; padding: 3px 10px; @@ -248,6 +248,15 @@ html, body { color: #ffffff; font-family: var(--font); font-size: 13px; outline: none; } +/* A rejected edit (#45): the cell stays in edit mode and explains why. */ +#grid-table td.cell-invalid input.cell-ed { border-color: #cc0000; background: #2a0000; } +#grid-table td.cell-invalid .cell-error { + position: absolute; left: 0; top: 100%; z-index: 20; + max-width: 320px; padding: 3px 8px; + background: #cc0000; color: #ffffff; + font-family: var(--font); font-size: 12px; white-space: normal; +} + /* ── FORM VIEW ── */ #form-view { flex: 1; display: flex; flex-direction: column; diff --git a/src/terminal/Terminal.ts b/src/terminal/Terminal.ts index c9e7112..04df9c5 100644 --- a/src/terminal/Terminal.ts +++ b/src/terminal/Terminal.ts @@ -67,7 +67,7 @@ export class Terminal { ws.on('grid-open', (msg) => { const m = msg as any; - this.openGrid(m.table, m.filter, m.columns, m.rows); + this.openGrid(m.table, m.filter, m.columns, m.columnTypes, m.rows); }); ws.on('data-changed', (msg) => { @@ -227,7 +227,7 @@ export class Terminal { // ── Views ────────────────────────────────────────────────────────────── - private openGrid(table: string, filter: string | null, columns: any[], rows: any[]) { + private openGrid(table: string, filter: string | null, columns: any[], columnTypes: any, rows: any[]) { this.termView.classList.add('hidden'); this.gridView.classList.remove('hidden'); @@ -235,6 +235,7 @@ export class Terminal { table, filter, columns, + columnTypes: columnTypes ?? {}, rows, ws: this.ws, onExit: () => this.closeGrid(), diff --git a/src/ui/Grid.ts b/src/ui/Grid.ts index dfdf6d7..5c9f7f1 100644 --- a/src/ui/Grid.ts +++ b/src/ui/Grid.ts @@ -1,10 +1,12 @@ import type { WsClient } from '../ws/WsClient'; -import type { ColInfo } from '../shared/types'; +import type { ColInfo, ColumnTypeInfo } from '../shared/types'; +import { validateCellValue } from '../shared/cellValidation'; export interface GridOptions { table: string; filter: string | null; columns: ColInfo[]; + columnTypes: Record; rows: Record[]; ws: WsClient; onExit: () => void; @@ -22,6 +24,7 @@ export class Grid { private rows: Row[] = []; private cols: string[] = []; + private columnTypes: Record = {}; private selRow = 0; private selCol = 1; private editingCell: { r: number; c: number } | null = null; @@ -42,6 +45,7 @@ export class Grid { this.onStatus = opts.onStatusChange; this.rows = opts.rows as Row[]; + this.columnTypes = opts.columnTypes ?? {}; this.cols = this.rows.length > 0 ? Object.keys(this.rows[0]).filter(c => c !== '_rowid') : opts.columns.map(c => c.name); @@ -61,6 +65,7 @@ export class Grid { this.ws.on('grid-open', (msg) => { const m = msg as any; this.rows = m.rows as Row[]; + this.columnTypes = m.columnTypes ?? this.columnTypes; this.cols = this.rows.length > 0 ? Object.keys(this.rows[0]).filter((c: string) => c !== '_rowid') : m.columns.map((c: ColInfo) => c.name); @@ -174,10 +179,15 @@ export class Grid { inp.focus(); inp.select(); this.editingCell = { r: ri, c: ci }; + // Clear a stale error as soon as the value becomes valid again. + inp.addEventListener('input', () => { + if (!validateCellValue(colName, inp.value, this.columnTypes[colName])) this.clearCellError(td); + }); + inp.addEventListener('keydown', (e) => { if (e.key === 'Enter' || e.key === 'Tab') { e.preventDefault(); e.stopPropagation(); - this.commitEdit(inp.value); + if (!this.commitEdit(inp.value)) return; // invalid — stay in edit mode if (e.key === 'Tab') this.selectCell(ri, ci + 2); } else if (e.key === 'Escape') { e.preventDefault(); e.stopPropagation(); @@ -186,15 +196,46 @@ export class Grid { }); } - private commitEdit(newValue: string) { - if (!this.editingCell) return; + /** @returns false when the value was rejected and the cell stays in edit mode. */ + private commitEdit(newValue: string): boolean { + if (!this.editingCell) return false; const { r, c } = this.editingCell; + const colName = this.cols[c]; + + const error = validateCellValue(colName, newValue, this.columnTypes[colName]); + if (error) { + const td = this.tbody.querySelector(`td[data-ri="${r}"][data-ci="${c}"]`); + if (td) this.showCellError(td, error); + this.onStatus(error); + return false; + } + const row = this.rows[r]; - this.ws.send({ type: 'grid-edit', rowid: row._rowid as number, col: this.cols[c], value: newValue }); - row[this.cols[c]] = newValue; + this.ws.send({ type: 'grid-edit', rowid: row._rowid as number, col: colName, value: newValue }); + row[colName] = newValue; this.editingCell = null; this.renderBody(); this.refreshSelection(); + return true; + } + + private showCellError(td: HTMLTableCellElement, message: string) { + td.classList.add('cell-invalid'); + td.title = message; + let tip = td.querySelector('.cell-error'); + if (!tip) { + tip = document.createElement('div'); + tip.className = 'cell-error'; + td.appendChild(tip); + } + tip.textContent = message; + td.querySelector('.cell-ed')?.focus(); + } + + private clearCellError(td: HTMLTableCellElement) { + td.classList.remove('cell-invalid'); + td.removeAttribute('title'); + td.querySelector('.cell-error')?.remove(); } private cancelEdit() { diff --git a/tests/CellValidation.test.ts b/tests/CellValidation.test.ts new file mode 100644 index 0000000..da4fbf3 --- /dev/null +++ b/tests/CellValidation.test.ts @@ -0,0 +1,107 @@ +import { describe, it, expect } from 'vitest'; +import { validateCellValue } from '../src/shared/cellValidation'; +import type { ColumnMeta } from '../src/shared/cellValidation'; + +const meta = (baseType: string, qualifier: number | null = null, scale: number | null = null): ColumnMeta => + ({ baseType, qualifier, scale }); + +describe('validateCellValue', () => { + it('accepts any value for an unknown/untracked column', () => { + expect(validateCellValue('X', 'anything', undefined)).toBeNull(); + expect(validateCellValue('X', 'anything', null)).toBeNull(); + }); + + it('accepts an empty value for every type (clearing a cell to NULL)', () => { + for (const t of ['TIME', 'DATE', 'NUM', 'INT', 'LOGICAL']) { + expect(validateCellValue('X', '', meta(t))).toBeNull(); + expect(validateCellValue('X', ' ', meta(t))).toBeNull(); + } + }); + + it('does not constrain CHAR/MEMO', () => { + expect(validateCellValue('X', 'anything at all', meta('CHAR', 5))).toBeNull(); + expect(validateCellValue('X', 'anything at all', meta('MEMO'))).toBeNull(); + }); + + describe('TIME', () => { + it('accepts well-formed HH:MM', () => { + expect(validateCellValue('T', '00:00', meta('TIME'))).toBeNull(); + expect(validateCellValue('T', '23:59', meta('TIME'))).toBeNull(); + expect(validateCellValue('T', '09:07', meta('TIME'))).toBeNull(); + }); + it('rejects malformed or out-of-range values', () => { + expect(validateCellValue('T', '9:30', meta('TIME'))).toMatch(/HH:MM/); + expect(validateCellValue('T', '24:00', meta('TIME'))).toMatch(/HH:MM/); + expect(validateCellValue('T', '08:60', meta('TIME'))).toMatch(/HH:MM/); + expect(validateCellValue('T', 'noon', meta('TIME'))).toMatch(/HH:MM/); + }); + it('enforces the minute granularity qualifier', () => { + expect(validateCellValue('T', '08:15', meta('TIME', 15))).toBeNull(); + expect(validateCellValue('T', '08:45', meta('TIME', 15))).toBeNull(); + expect(validateCellValue('T', '08:07', meta('TIME', 15))).toMatch(/multiple of 15/); + expect(validateCellValue('T', '08:30', meta('TIME', 30))).toBeNull(); + expect(validateCellValue('T', '08:15', meta('TIME', 30))).toMatch(/multiple of 30/); + }); + }); + + describe('DATE', () => { + it('accepts a valid ISO calendar date', () => { + expect(validateCellValue('D', '2024-02-29', meta('DATE'))).toBeNull(); + expect(validateCellValue('D', '2026-12-31', meta('DATE'))).toBeNull(); + }); + it('rejects a wrong format', () => { + expect(validateCellValue('D', '12/25/26', meta('DATE'))).toMatch(/YYYY-MM-DD/); + expect(validateCellValue('D', '2024-2-9', meta('DATE'))).toMatch(/YYYY-MM-DD/); + }); + it('rejects an impossible calendar date', () => { + expect(validateCellValue('D', '2023-02-29', meta('DATE'))).toMatch(/not a real date/); + expect(validateCellValue('D', '2024-02-30', meta('DATE'))).toMatch(/not a real date/); + expect(validateCellValue('D', '2024-13-01', meta('DATE'))).toMatch(/not a real date/); + }); + }); + + describe('LOGICAL', () => { + it('accepts the dBASE and plain boolean literal set', () => { + for (const v of ['.T.', '.F.', '.TRUE.', '.FALSE.', 'T', 'F', 'true', 'FALSE', '1', '0']) { + expect(validateCellValue('L', v, meta('LOGICAL'))).toBeNull(); + } + }); + it('rejects anything else', () => { + expect(validateCellValue('L', 'yes', meta('LOGICAL'))).toMatch(/\.T\.|\.F\./); + expect(validateCellValue('L', '2', meta('LOGICAL'))).toMatch(/\.T\.|\.F\./); + }); + }); + + describe('INT', () => { + it('accepts integers, including negatives', () => { + expect(validateCellValue('I', '42', meta('INT'))).toBeNull(); + expect(validateCellValue('I', '-7', meta('INT'))).toBeNull(); + }); + it('rejects decimals and non-numbers', () => { + expect(validateCellValue('I', '4.2', meta('INT'))).toMatch(/whole number/); + expect(validateCellValue('I', 'abc', meta('INT'))).toMatch(/whole number/); + }); + }); + + describe('NUM', () => { + it('accepts any number when unqualified', () => { + expect(validateCellValue('N', '3.14159', meta('NUM'))).toBeNull(); + expect(validateCellValue('N', '-12', meta('NUM'))).toBeNull(); + }); + it('rejects non-numeric input', () => { + expect(validateCellValue('N', 'abc', meta('NUM'))).toMatch(/number/); + }); + it('enforces scale (digits after the decimal point)', () => { + expect(validateCellValue('N', '10.25', meta('NUM', 8, 2))).toBeNull(); + expect(validateCellValue('N', '10.257', meta('NUM', 8, 2))).toMatch(/2 decimal/); + }); + it('enforces precision (total digits)', () => { + expect(validateCellValue('N', '123456.78', meta('NUM', 8, 2))).toBeNull(); // 8 digits + expect(validateCellValue('N', '1234567.89', meta('NUM', 8, 2))).toMatch(/6 digit/); // 7 int digits > 8-2 + }); + it('treats NUM(n) with no scale as an integer-width limit', () => { + expect(validateCellValue('N', '123456', meta('NUM', 6))).toBeNull(); + expect(validateCellValue('N', '1234567', meta('NUM', 6))).toMatch(/6 digit/); + }); + }); +}); diff --git a/tests/ColumnMeta.test.ts b/tests/ColumnMeta.test.ts new file mode 100644 index 0000000..173e58f --- /dev/null +++ b/tests/ColumnMeta.test.ts @@ -0,0 +1,174 @@ +import { describe, it, expect, afterEach } from 'vitest'; +import { Lexer } from '../src/interpreter/Lexer'; +import { Parser } from '../src/interpreter/Parser'; +import { Session } from '../server/Session'; +import type { ServerMessage } from '../src/shared/types'; +import fs from 'fs'; +import path from 'path'; + +let dbCounter = 0; +function makeSession() { + const sent: ServerMessage[] = []; + return { session: new Session((m: ServerMessage) => { sent.push(m); }), sent }; +} +function uniqueDb() { return `test_colmeta_${Date.now()}_${++dbCounter}`; } + +afterEach(() => { + const dataDir = path.join(process.cwd(), 'data'); + if (fs.existsSync(dataDir)) { + fs.readdirSync(dataDir) + .filter(f => f.toLowerCase().startsWith('test_colmeta_')) + .forEach(f => fs.unlinkSync(path.join(dataDir, f))); + } +}); + +async function run(session: Session, sent: ServerMessage[], text: string): Promise { + sent.length = 0; + await session.handleMessage({ type: 'command', text }); + const out = sent.find(m => m.type === 'output') as any; + return (out?.lines ?? []).map((l: any) => l.text); +} + +function parse(src: string) { + return new Parser(new Lexer(src).tokenize()).parse(); +} + +describe('Parser: NUM(p,s) precision/scale', () => { + it('captures both precision and scale, without inventing a phantom column', () => { + const ast = parse('CREATE TABLE t (price NUM(8,2))')[0] as any; + expect(ast.cols).toEqual([{ name: 'PRICE', colType: 'NUM', size: 8, scale: 2 }]); + }); + + it('keeps parsing the columns that follow a NUM(p,s)', () => { + const ast = parse('CREATE TABLE t (price NUM(8,2), active LOGICAL)')[0] as any; + expect(ast.cols.map((c: any) => c.name)).toEqual(['PRICE', 'ACTIVE']); + expect(ast.cols[1]).toEqual({ name: 'ACTIVE', colType: 'LOGICAL' }); + }); + + it('still parses a single-arg size', () => { + const ast = parse('CREATE TABLE t (name CHAR(40), qty NUM(6))')[0] as any; + expect(ast.cols).toEqual([ + { name: 'NAME', colType: 'CHAR', size: 40 }, + { name: 'QTY', colType: 'NUM', size: 6 }, + ]); + }); +}); + +describe('CREATE TABLE with NUM(p,s) creates only the declared columns', () => { + it('does not create a phantom column named after the scale', async () => { + const { session, sent } = makeSession(); + await run(session, sent, `USE DATABASE ${uniqueDb()}`); + await run(session, sent, 'CREATE TABLE products (name CHAR(10), price NUM(8,2), active LOGICAL)'); + await run(session, sent, 'USE products'); + const lines = await run(session, sent, 'LIST STRUCTURE'); + const struct = lines.join('\n'); + expect(struct).toContain('NAME'); + expect(struct).toContain('PRICE'); + expect(struct).toContain('ACTIVE'); + expect(struct).not.toMatch(/^\d+\s+2\s/m); // no column literally named "2" + }); +}); + +describe('LIST STRUCTURE prints declared types', () => { + it('shows CHAR(n), NUM(p,s), DATE, TIME(n), LOGICAL as declared', async () => { + const { session, sent } = makeSession(); + await run(session, sent, `USE DATABASE ${uniqueDb()}`); + await run(session, sent, 'CREATE TABLE t (a CHAR(10), b NUM(8,2), c DATE, d TIME(15), e LOGICAL, f INT)'); + await run(session, sent, 'USE t'); + const struct = (await run(session, sent, 'LIST STRUCTURE')).join('\n'); + expect(struct).toMatch(/A\s+CHAR\(10\)/); + expect(struct).toMatch(/B\s+NUM\(8,2\)/); + expect(struct).toMatch(/C\s+DATE/); + expect(struct).toMatch(/D\s+TIME\(15\)/); + expect(struct).toMatch(/E\s+LOGICAL/); + expect(struct).toMatch(/F\s+INT/); + }); +}); + +describe('grid-open carries declared column types', () => { + it('sends a columnTypes map alongside the raw SQLite columns', async () => { + const { session, sent } = makeSession(); + await run(session, sent, `USE DATABASE ${uniqueDb()}`); + await run(session, sent, 'CREATE TABLE t (name CHAR(10), price NUM(8,2), shift TIME(15))'); + await run(session, sent, 'USE t'); + sent.length = 0; + await session.handleMessage({ type: 'command', text: 'BROWSE' }); + const grid = sent.find(m => m.type === 'grid-open') as any; + expect(grid).toBeDefined(); + expect(grid.columnTypes.PRICE).toEqual({ baseType: 'NUM', qualifier: 8, scale: 2 }); + expect(grid.columnTypes.SHIFT).toEqual({ baseType: 'TIME', qualifier: 15, scale: null }); + expect(grid.columnTypes.NAME).toEqual({ baseType: 'CHAR', qualifier: 10, scale: null }); + }); +}); + +describe('grid-edit is validated server-side', () => { + async function browseTable(session: Session, sent: ServerMessage[]) { + await run(session, sent, `USE DATABASE ${uniqueDb()}`); + await run(session, sent, 'CREATE TABLE t (shift TIME(15), price NUM(8,2))'); + await run(session, sent, 'USE t'); + await run(session, sent, 'APPEND RECORD'); + sent.length = 0; + await session.handleMessage({ type: 'command', text: 'BROWSE' }); + const grid = sent.find(m => m.type === 'grid-open') as any; + return grid.rows[0]._rowid as number; + } + + it('rejects an invalid TIME(15) cell edit and does not write it', async () => { + const { session, sent } = makeSession(); + const rowid = await browseTable(session, sent); + + sent.length = 0; + await session.handleMessage({ type: 'grid-edit', rowid, col: 'SHIFT', value: '08:07' }); + const out = sent.find(m => m.type === 'output') as any; + expect(out).toBeDefined(); + expect(out.lines.map((l: any) => l.text).join('\n')).toMatch(/multiple of 15/); + + const lines = await run(session, sent, 'LIST'); + expect(lines.join('\n')).not.toContain('08:07'); + }); + + it('accepts a valid cell edit and writes it', async () => { + const { session, sent } = makeSession(); + const rowid = await browseTable(session, sent); + + await session.handleMessage({ type: 'grid-edit', rowid, col: 'SHIFT', value: '08:15' }); + const lines = await run(session, sent, 'LIST'); + expect(lines.join('\n')).toContain('08:15'); + }); + + it('rejects an out-of-scale NUM(8,2) cell edit', async () => { + const { session, sent } = makeSession(); + const rowid = await browseTable(session, sent); + + sent.length = 0; + await session.handleMessage({ type: 'grid-edit', rowid, col: 'PRICE', value: '1.234' }); + const out = sent.find(m => m.type === 'output') as any; + expect(out.lines.map((l: any) => l.text).join('\n')).toMatch(/2 decimal/); + }); +}); + +describe('column metadata is scoped per database', () => { + it('does not leak a declared type between same-named tables in different databases', async () => { + const { session, sent } = makeSession(); + const dbA = uniqueDb(); + const dbB = uniqueDb(); + + await run(session, sent, `USE DATABASE ${dbA}`); + await run(session, sent, 'CREATE TABLE shared (val TIME(15))'); + + await run(session, sent, `USE DATABASE ${dbB}`); + await run(session, sent, 'CREATE TABLE shared (val CHAR(20))'); + await run(session, sent, 'USE shared'); + await run(session, sent, 'APPEND RECORD'); + // CHAR is unconstrained — this must be accepted, not judged against TIME(15). + const lines = await run(session, sent, 'REPLACE val WITH "hello"'); + expect(lines.join('\n')).toContain('Replaced'); + + // And dbA's TIME(15) must still be enforced. + await run(session, sent, `USE DATABASE ${dbA}`); + await run(session, sent, 'USE shared'); + await run(session, sent, 'APPEND RECORD'); + const bad = await run(session, sent, 'REPLACE val WITH "08:07"'); + expect(bad.join('\n')).toMatch(/\*\* Error/); + }); +}); diff --git a/tests/ColumnMetaStore.test.ts b/tests/ColumnMetaStore.test.ts new file mode 100644 index 0000000..357c012 --- /dev/null +++ b/tests/ColumnMetaStore.test.ts @@ -0,0 +1,82 @@ +import { describe, it, expect, afterEach } from 'vitest'; +import Database from 'better-sqlite3'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import { ColumnMetaStore } from '../server/ColumnMetaStore'; + +const tmpFiles: string[] = []; +function tmpDbPath(): string { + const p = path.join(fs.mkdtempSync(path.join(os.tmpdir(), 'wb3-colmeta-')), 'system.sqlite3'); + tmpFiles.push(p); + return p; +} + +afterEach(() => { + while (tmpFiles.length) { + const p = tmpFiles.pop()!; + fs.rmSync(path.dirname(p), { recursive: true, force: true }); + } +}); + +describe('ColumnMetaStore', () => { + it('round-trips a declared type with qualifier and scale', () => { + const store = new ColumnMetaStore(tmpDbPath()); + store.setColumnType('DB', 'T', 'PRICE', 'NUM', 8, 2); + expect(store.getColumnType('DB', 'T', 'PRICE')).toEqual({ baseType: 'NUM', qualifier: 8, scale: 2 }); + }); + + it('scopes metadata per database', () => { + const store = new ColumnMetaStore(tmpDbPath()); + store.setColumnType('A', 'SHARED', 'VAL', 'TIME', 15, null); + store.setColumnType('B', 'SHARED', 'VAL', 'CHAR', 20, null); + expect(store.getColumnType('A', 'SHARED', 'VAL')).toEqual({ baseType: 'TIME', qualifier: 15, scale: null }); + expect(store.getColumnType('B', 'SHARED', 'VAL')).toEqual({ baseType: 'CHAR', qualifier: 20, scale: null }); + }); + + it('returns null for an untracked column', () => { + const store = new ColumnMetaStore(tmpDbPath()); + expect(store.getColumnType('DB', 'T', 'NOPE')).toBeNull(); + }); + + it('drops, renames, and lists per (db, table)', () => { + const store = new ColumnMetaStore(tmpDbPath()); + store.setColumnType('DB', 'T', 'A', 'TIME', 15, null); + store.setColumnType('DB', 'T', 'B', 'DATE', null, null); + store.setColumnType('DB', 'OTHER', 'A', 'INT', null, null); + + expect(Object.keys(store.listColumnTypes('DB', 'T')).sort()).toEqual(['A', 'B']); + + store.renameColumn('DB', 'T', 'A', 'RENAMED'); + expect(store.getColumnType('DB', 'T', 'RENAMED')?.baseType).toBe('TIME'); + expect(store.getColumnType('DB', 'T', 'A')).toBeNull(); + + store.dropColumn('DB', 'T', 'B'); + expect(store.getColumnType('DB', 'T', 'B')).toBeNull(); + + store.dropTable('DB', 'T'); + expect(store.listColumnTypes('DB', 'T')).toEqual({}); + expect(store.getColumnType('DB', 'OTHER', 'A')?.baseType).toBe('INT'); // untouched + }); + + // The #43 cut of this table had neither db_name nor scale. Opening an old file + // must migrate rather than throw "no such column". + it('migrates a pre-#45 column_types table', () => { + const p = tmpDbPath(); + const legacy = new Database(p); + legacy.exec(` + CREATE TABLE column_types ( + table_name TEXT NOT NULL, col_name TEXT NOT NULL, + base_type TEXT NOT NULL, qualifier INTEGER, + PRIMARY KEY (table_name, col_name) + ); + `); + legacy.prepare('INSERT INTO column_types VALUES (?,?,?,?)').run('SHIFTS', 'STARTTIME', 'TIME', 15); + legacy.close(); + + const store = new ColumnMetaStore(p); + store.setColumnType('MYDB', 'SHIFTS', 'STARTTIME', 'TIME', 15, null); + expect(store.getColumnType('MYDB', 'SHIFTS', 'STARTTIME')).toEqual({ baseType: 'TIME', qualifier: 15, scale: null }); + expect(store.getColumnType('OTHERDB', 'SHIFTS', 'STARTTIME')).toBeNull(); + }); +}); diff --git a/tests/assistant.spec.ts b/tests/assistant.spec.ts index 8166f76..9e347e5 100644 --- a/tests/assistant.spec.ts +++ b/tests/assistant.spec.ts @@ -57,6 +57,39 @@ test.describe('Assistant sidebar', () => { await page.keyboard.press('Escape'); await expect(page.locator('#terminal-view')).toBeVisible({ timeout: 5000 }); }); + + // #45 — the grid opened from the Assistant validates edits per declared type. + test('grid opened via the Assistant Browse action validates cell edits', async ({ page }) => { + await boot(page); + for (const c of [ + 'USE DATABASE ASSISTDEMO', + 'DROP TABLE asst_shifts', + 'CREATE TABLE asst_shifts (STARTTIME TIME(15))', + 'USE asst_shifts', + 'APPEND RECORD', + ]) { + await page.locator('#terminal-input').fill(c); + await page.locator('#terminal-input').press('Enter'); + await page.waitForTimeout(400); + } + + await clickAction(page, 'Browse'); + await expect(page.locator('#grid-view')).toBeVisible({ timeout: 5000 }); + + const td = page.locator('#grid-tbody td[data-ri="0"][data-ci="0"]'); + await td.dblclick(); + await td.locator('input.cell-ed').fill('08:07'); + await page.keyboard.press('Enter'); + await expect(td).toHaveClass(/cell-invalid/); + await expect(td.locator('.cell-error')).toContainText('multiple of 15'); + + await td.locator('input.cell-ed').fill('08:30'); + await page.keyboard.press('Enter'); + await expect(td).toContainText('08:30'); + + await page.keyboard.press('Escape'); + await expect(page.locator('#terminal-view')).toBeVisible({ timeout: 5000 }); + }); }); test.describe('Assistant wizards — table', () => { diff --git a/tests/grid-validation.spec.ts b/tests/grid-validation.spec.ts new file mode 100644 index 0000000..1e077b9 --- /dev/null +++ b/tests/grid-validation.spec.ts @@ -0,0 +1,115 @@ +/** #45 — BROWSE per-cell validation, exercised in a real browser. */ +import { test, expect, Page } from '@playwright/test'; + +async function cmd(page: Page, command: string, waitMs = 600): Promise { + const input = page.locator('#terminal-input'); + await input.fill(command); + await input.press('Enter'); + await page.waitForTimeout(waitMs); +} + +async function boot(page: Page, db: string): Promise { + await page.goto('/'); + await expect(page.locator('#terminal-output')).toContainText('Connected.', { timeout: 8000 }); + await cmd(page, `USE DATABASE ${db}`); +} + +/** Open the cell editor for a given row/column index. */ +async function editCell(page: Page, ri: number, ci: number) { + const td = page.locator(`#grid-tbody td[data-ri="${ri}"][data-ci="${ci}"]`); + await td.dblclick(); + await expect(td.locator('input.cell-ed')).toBeVisible(); + return td; +} + +test.describe('BROWSE cell validation', () => { + test('rejects an invalid TIME(15) edit inline and commits a valid one', async ({ page }) => { + await boot(page, `e2e_gridval_time_${Date.now()}`); + await cmd(page, 'CREATE TABLE shifts (person CHAR(20), starttime TIME(15))'); + await cmd(page, 'USE shifts'); + await cmd(page, 'APPEND RECORD'); + await cmd(page, 'BROWSE', 1000); + await expect(page.locator('#grid-view')).toBeVisible({ timeout: 5000 }); + + // Malformed time — rejected, cell stays in edit mode with a visible reason. + const td = await editCell(page, 0, 1); + await td.locator('input.cell-ed').fill('9:30'); + await page.keyboard.press('Enter'); + await expect(td).toHaveClass(/cell-invalid/); + await expect(td.locator('.cell-error')).toContainText('HH:MM'); + await expect(td.locator('input.cell-ed')).toBeVisible(); // still editing + + // Off-granularity time — rejected for a different reason. + await td.locator('input.cell-ed').fill('08:07'); + await page.keyboard.press('Enter'); + await expect(td).toHaveClass(/cell-invalid/); + await expect(td.locator('.cell-error')).toContainText('multiple of 15'); + await expect(td.locator('input.cell-ed')).toBeVisible(); + + // Valid quarter-hour — the error clears as you type and the edit commits. + await td.locator('input.cell-ed').fill('08:15'); + await expect(td).not.toHaveClass(/cell-invalid/); + await page.keyboard.press('Enter'); + await expect(td.locator('input.cell-ed')).toHaveCount(0); // edit closed + await expect(td).toContainText('08:15'); + + // And it really landed in the database. + await page.keyboard.press('Escape'); + await expect(page.locator('#terminal-view')).toBeVisible({ timeout: 5000 }); + await cmd(page, 'LIST'); + await expect(page.locator('#terminal-output')).toContainText('08:15'); + }); + + test('rejects a bad NUM(8,2) and DATE edit, and an unconstrained CHAR accepts anything', async ({ page }) => { + await boot(page, `e2e_gridval_types_${Date.now()}`); + await cmd(page, 'CREATE TABLE t (name CHAR(20), price NUM(8,2), due DATE)'); + await cmd(page, 'USE t'); + await cmd(page, 'APPEND RECORD'); + await cmd(page, 'BROWSE', 1000); + await expect(page.locator('#grid-view')).toBeVisible({ timeout: 5000 }); + + // CHAR is unconstrained — commits as typed. + const name = await editCell(page, 0, 0); + await name.locator('input.cell-ed').fill('anything at all'); + await page.keyboard.press('Enter'); + await expect(name).toContainText('anything at all'); + + // NUM(8,2) — too many decimals. + const price = await editCell(page, 0, 1); + await price.locator('input.cell-ed').fill('1.234'); + await page.keyboard.press('Enter'); + await expect(price).toHaveClass(/cell-invalid/); + await expect(price.locator('.cell-error')).toContainText('2 decimal'); + await price.locator('input.cell-ed').fill('1.23'); + await page.keyboard.press('Enter'); + await expect(price).toContainText('1.23'); + + // DATE — not a real calendar date. + const due = await editCell(page, 0, 2); + await due.locator('input.cell-ed').fill('2023-02-29'); + await page.keyboard.press('Enter'); + await expect(due).toHaveClass(/cell-invalid/); + await expect(due.locator('.cell-error')).toContainText('not a real date'); + await due.locator('input.cell-ed').fill('2024-02-29'); + await page.keyboard.press('Enter'); + await expect(due).toContainText('2024-02-29'); + }); + + test('Escape abandons an invalid edit and restores the original value', async ({ page }) => { + await boot(page, `e2e_gridval_esc_${Date.now()}`); + await cmd(page, 'CREATE TABLE s (starttime TIME(15))'); + await cmd(page, 'USE s'); + await cmd(page, 'APPEND RECORD'); + await cmd(page, 'REPLACE starttime WITH "09:00"'); + await cmd(page, 'BROWSE', 1000); + + const td = await editCell(page, 0, 0); + await td.locator('input.cell-ed').fill('99:99'); + await page.keyboard.press('Enter'); + await expect(td).toHaveClass(/cell-invalid/); + + await page.keyboard.press('Escape'); // abandon the edit + await expect(td.locator('input.cell-ed')).toHaveCount(0); + await expect(td).toContainText('09:00'); // original value intact + }); +});