Skip to content

Commit 73e69e1

Browse files
os-zhuangclaude
andcommitted
fix: array field types persist + Field.time accepts time-of-day
Two field-type runtime gaps found driving the showcase field-zoo (which had no seed data, so neither was ever exercised at runtime). 1. Array/object fields broke every write (driver-sql). multiselect/checkboxes/ tags/repeater/vector were missing from the SQL driver's JSON-field classification, so their arrays reached the better-sqlite3 binder un-serialized → "SQLite3 can only bind numbers, strings, bigints, buffers, and null" 500 on insert/update (incl. task.labels on a normal object). The DDL column-type switch and isJsonField had drifted into two lists; unified into one JSON_COLUMN_TYPES source covering the array/object types, plus a formatInput safety net that serializes any stray array/object so an unclassified field degrades to a stored string instead of crashing. 2. Field.time rejected every valid value (objectql). The validator reused the date/datetime Date.parse branch, which is NaN for any bare time string, so `time` could never accept 14:30 / 09:05:30. Now validates HH:MM / HH:MM:SS (optional fractional seconds + Z/offset), still accepts a full ISO datetime; date/datetime unchanged. (Date.parse('14:60') returns a bogus number, so the fallback requires a real date component — malformed times are rejected.) Verified live on app-showcase: full field-zoo specimen now persists + round-trips. Tests: driver-sql array round-trip (155 green) + time validator (objectql 652 green). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 575448d commit 73e69e1

5 files changed

Lines changed: 196 additions & 7 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
"@objectstack/driver-sql": minor
3+
"@objectstack/objectql": patch
4+
---
5+
6+
fix: array-valued field types persist, and `Field.time` accepts time-of-day — two field-type runtime gaps found driving the showcase field-zoo (which had no seed data, so neither was ever exercised).
7+
8+
**Array/object fields broke every write (driver-sql).** `multiselect` / `checkboxes` / `tags` / `repeater` / `vector` were absent from the SQL driver's JSON-field classification, so their array values reached the better-sqlite3 binder un-serialized and threw *"SQLite3 can only bind numbers, strings, bigints, buffers, and null"* — a 500 on insert/update for common field types (even `task.labels` on a normal object). The DDL column-type switch and `isJsonField` had drifted into two separate lists; they now share one `JSON_COLUMN_TYPES` source that includes the array/object types, so these columns are created as JSON and round-trip as arrays/objects. A `formatInput` safety net additionally serializes any stray array/object value so an unclassified field degrades to a stored string instead of crashing.
9+
10+
**`Field.time` rejected every valid value (objectql).** The validator reused the date/datetime branch (`Date.parse`), which is `NaN` for any bare time string — so a `time` field could never accept `14:30` or `09:05:30`. `time` now validates a time-of-day (`HH:MM` / `HH:MM:SS`, optional fractional seconds and `Z`/offset) and still accepts a full ISO datetime; `date`/`datetime` are unchanged.
11+
12+
Verified live on app-showcase: the full field-zoo specimen (all input-able field types) now persists and round-trips. Regression tests added for both.

packages/objectql/src/validation/record-validator.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,35 @@ describe('validateRecord — required + autonumber exemption', () => {
3333
).not.toThrow();
3434
});
3535
});
36+
37+
/**
38+
* `Field.time` is a wall-clock time-of-day, not an instant. The old validator
39+
* reused the date/datetime branch (`Date.parse`), which is `NaN` for every
40+
* bare time string — so a `time` field rejected ALL valid values (found
41+
* driving the showcase field-zoo). It must accept `HH:MM` / `HH:MM:SS`.
42+
*/
43+
describe('validateRecord — time field accepts time-of-day', () => {
44+
const schema = { fields: { at: { type: 'time' } } };
45+
46+
for (const v of ['14:30', '09:05:30', '23:59', '00:00:00', '14:30:00Z', '14:30:00.500', '08:15:00+02:00']) {
47+
it(`accepts ${v}`, () => {
48+
expect(() => validateRecord(schema, { at: v }, 'insert')).not.toThrow();
49+
});
50+
}
51+
52+
it('accepts a full ISO datetime for a time field (lenient)', () => {
53+
expect(() => validateRecord(schema, { at: '2026-06-17T14:30:00Z' }, 'insert')).not.toThrow();
54+
});
55+
56+
for (const v of ['25:00', '14:60', 'not-a-time', '14']) {
57+
it(`rejects ${v}`, () => {
58+
expect(() => validateRecord(schema, { at: v }, 'insert')).toThrow(/invalid_time/i);
59+
});
60+
}
61+
62+
it('does NOT regress date/datetime (still ISO-parsed)', () => {
63+
const ds = { fields: { d: { type: 'date' }, dt: { type: 'datetime' } } };
64+
expect(() => validateRecord(ds, { d: '2026-06-17', dt: '2026-06-17T10:00:00Z' }, 'insert')).not.toThrow();
65+
expect(() => validateRecord(ds, { d: 'not-a-date' }, 'insert')).toThrow(/invalid_date/i);
66+
});
67+
});

packages/objectql/src/validation/record-validator.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ export interface FieldValidationError {
6565
| 'invalid_number'
6666
| 'invalid_boolean'
6767
| 'invalid_date'
68+
| 'invalid_time'
6869
| 'invalid_option'
6970
// Object-level validation rules (ADR-0020, see rule-validator.ts)
7071
| 'invalid_transition'
@@ -173,12 +174,31 @@ function validateOne(name: string, def: FieldDef, value: unknown): FieldValidati
173174
}
174175

175176
// ── date/datetime ───────────────────────────────────────────────
176-
if (t === 'date' || t === 'datetime' || t === 'time') {
177+
if (t === 'date' || t === 'datetime') {
177178
if (value instanceof Date) return null;
178179
if (typeof value === 'string' && !Number.isNaN(Date.parse(value))) return null;
179180
return { field: name, code: 'invalid_date', message: `${name} must be a valid ${t} (ISO-8601)` };
180181
}
181182

183+
// ── time (time-of-day) ──────────────────────────────────────────
184+
// A `Field.time` is a wall-clock time, NOT an instant — `Date.parse('14:30')`
185+
// is NaN, so reusing the date branch rejected every valid time. Accept
186+
// `HH:MM`, `HH:MM:SS`, optional fractional seconds and an optional Z/offset;
187+
// also accept a Date or a full ISO datetime (callers that send a timestamp
188+
// for a time field).
189+
if (t === 'time') {
190+
if (value instanceof Date) return null;
191+
if (typeof value === 'string') {
192+
const timeOfDay = /^([01]\d|2[0-3]):[0-5]\d(:[0-5]\d(\.\d+)?)?(Z|[+-]([01]\d|2[0-3]):?[0-5]\d)?$/;
193+
// Accept a valid time-of-day, OR a full datetime that carries a real date
194+
// component. NOT a bare `Date.parse` check — `Date.parse('14:60')` returns
195+
// a (bogus) number in Node, which would let malformed times through.
196+
const hasDate = /\d{4}-\d{2}-\d{2}/.test(value);
197+
if (timeOfDay.test(value.trim()) || (hasDate && !Number.isNaN(Date.parse(value)))) return null;
198+
}
199+
return { field: name, code: 'invalid_time', message: `${name} must be a valid time (HH:MM or HH:MM:SS)` };
200+
}
201+
182202
// ── select / multiselect / radio ────────────────────────────────
183203
if (t === 'select' || t === 'radio') {
184204
const allowed = optionValues(def.options);
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* Array/object-valued field types must be stored as JSON columns and
5+
* round-tripped as arrays/objects. Regression: `multiselect`/`checkboxes`/
6+
* `tags`/`repeater`/`vector` were absent from the driver's JSON-field
7+
* classification, so their array values reached the better-sqlite3 binder
8+
* un-serialized and crashed with "SQLite3 can only bind numbers, strings,
9+
* bigints, buffers, and null" — breaking common field types on every write
10+
* (found driving the showcase field-zoo, which had no seed data to surface it).
11+
*
12+
* The classification (`isJsonField`) and the DDL column-type switch now share
13+
* one `JSON_COLUMN_TYPES` source, plus a `formatInput` safety net stringifies
14+
* any stray array/object so an unclassified field degrades to a stored string
15+
* instead of a 500.
16+
*/
17+
18+
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
19+
import { SqlDriver } from '../src/index.js';
20+
21+
describe('SqlDriver array/object field persistence', () => {
22+
let driver: SqlDriver;
23+
24+
beforeEach(async () => {
25+
driver = new SqlDriver({
26+
client: 'better-sqlite3',
27+
connection: { filename: ':memory:' },
28+
useNullAsDefault: true,
29+
});
30+
await driver.initObjects([
31+
{
32+
name: 'zoo',
33+
fields: {
34+
name: { type: 'string' },
35+
tags: { type: 'tags' },
36+
ms: { type: 'multiselect' },
37+
cbs: { type: 'checkboxes' },
38+
rep: { type: 'repeater' },
39+
vec: { type: 'vector' },
40+
comp: { type: 'composite' },
41+
loc: { type: 'location' },
42+
},
43+
},
44+
]);
45+
});
46+
47+
afterEach(async () => {
48+
await driver.disconnect();
49+
});
50+
51+
it('persists and round-trips array-valued fields as arrays', async () => {
52+
await driver.create(
53+
'zoo',
54+
{
55+
id: 'z1', name: 'Specimen',
56+
tags: ['x', 'y'],
57+
ms: ['red', 'green'],
58+
cbs: ['email', 'push'],
59+
rep: [{ a: 1 }, { a: 2 }],
60+
vec: [0.1, 0.2, 0.3],
61+
comp: { k: 'v' },
62+
loc: { lat: 1.5, lng: 2.5 },
63+
},
64+
{ bypassTenantAudit: true },
65+
);
66+
const row = await driver.findOne('zoo', 'z1', { bypassTenantAudit: true });
67+
expect(row.tags).toEqual(['x', 'y']);
68+
expect(row.ms).toEqual(['red', 'green']);
69+
expect(row.cbs).toEqual(['email', 'push']);
70+
expect(row.rep).toEqual([{ a: 1 }, { a: 2 }]);
71+
expect(row.vec).toEqual([0.1, 0.2, 0.3]);
72+
expect(row.comp).toEqual({ k: 'v' });
73+
expect(row.loc).toEqual({ lat: 1.5, lng: 2.5 });
74+
});
75+
76+
it('updates an array field to a new array', async () => {
77+
await driver.create('zoo', { id: 'z2', name: 'B', tags: ['a'] }, { bypassTenantAudit: true });
78+
await driver.update('zoo', 'z2', { tags: ['a', 'b', 'c'] }, { bypassTenantAudit: true });
79+
const row = await driver.findOne('zoo', 'z2', { bypassTenantAudit: true });
80+
expect(row.tags).toEqual(['a', 'b', 'c']);
81+
});
82+
83+
it('does not crash on an empty array', async () => {
84+
await driver.create('zoo', { id: 'z3', name: 'C', ms: [] }, { bypassTenantAudit: true });
85+
const row = await driver.findOne('zoo', 'z3', { bypassTenantAudit: true });
86+
expect(row.ms).toEqual([]);
87+
});
88+
});

packages/plugins/driver-sql/src/sql-driver.ts

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,23 @@ const SEQUENCES_TABLE = '_objectstack_sequences';
3434
*/
3535
const GLOBAL_TENANT = '__global__';
3636

37+
/**
38+
* Field types whose value is an array or object and must be stored as a JSON
39+
* column (and JSON-(de)serialized at the driver boundary). SINGLE SOURCE for
40+
* both the DDL column-type switch and `isJsonField` so the two can't drift —
41+
* the drift between them is exactly what let array-valued fields (multiselect/
42+
* checkboxes/tags/repeater/vector) reach the SQLite binder un-serialized and
43+
* crash with "SQLite3 can only bind numbers, strings, bigints, buffers, and
44+
* null" (#field-zoo). `image`/`file`/`avatar` hold structured upload metadata;
45+
* `composite`/`address`/`location` are objects; the rest are arrays.
46+
*/
47+
const JSON_COLUMN_TYPES = new Set<string>([
48+
'json', 'object', 'array',
49+
'image', 'file', 'avatar',
50+
'location', 'address', 'composite',
51+
'multiselect', 'checkboxes', 'tags', 'repeater', 'vector',
52+
]);
53+
3754
// ── Introspection Types ──────────────────────────────────────────────────────
3855

3956
export interface IntrospectedColumn {
@@ -1845,6 +1862,13 @@ export class SqlDriver implements IDataDriver {
18451862
case 'file':
18461863
case 'avatar':
18471864
case 'location':
1865+
case 'address':
1866+
case 'composite':
1867+
case 'multiselect':
1868+
case 'checkboxes':
1869+
case 'tags':
1870+
case 'repeater':
1871+
case 'vector':
18481872
col = table.json(name);
18491873
break;
18501874
case 'lookup':
@@ -1976,7 +2000,7 @@ export class SqlDriver implements IDataDriver {
19762000
}
19772001

19782002
protected isJsonField(type: string, field: any): boolean {
1979-
return ['json', 'object', 'array', 'image', 'file', 'avatar', 'location'].includes(type) || field.multiple;
2003+
return JSON_COLUMN_TYPES.has(type) || !!field.multiple;
19802004
}
19812005

19822006
// ── SQLite serialisation ────────────────────────────────────────────────────
@@ -2022,12 +2046,25 @@ export class SqlDriver implements IDataDriver {
20222046
if (!this.isSqlite) return copy;
20232047

20242048
const fields = this.jsonFields[object];
2025-
if (!fields || fields.length === 0) return copy;
2049+
if (fields && fields.length > 0) {
2050+
if (!copied) { copy = { ...copy }; copied = true; }
2051+
for (const field of fields) {
2052+
if (copy[field] !== undefined && typeof copy[field] === 'object' && copy[field] !== null) {
2053+
copy[field] = JSON.stringify(copy[field]);
2054+
}
2055+
}
2056+
}
20262057

2027-
if (!copied) { copy = { ...copy }; copied = true; }
2028-
for (const field of fields) {
2029-
if (copy[field] !== undefined && typeof copy[field] === 'object' && copy[field] !== null) {
2030-
copy[field] = JSON.stringify(copy[field]);
2058+
// Safety net: better-sqlite3 can only bind numbers/strings/bigints/buffers/
2059+
// null. Any value still an array or plain object here (a field type not
2060+
// classified as JSON, a `Field.multiple` we didn't catch, or an ad-hoc
2061+
// payload) would otherwise throw a raw TypeError mid-insert. Serialize it
2062+
// to JSON so the write degrades to a stored string instead of a 500.
2063+
for (const key of Object.keys(copy)) {
2064+
const v = copy[key];
2065+
if (v !== null && typeof v === 'object' && !(v instanceof Date) && !Buffer.isBuffer(v)) {
2066+
if (!copied) { copy = { ...copy }; copied = true; }
2067+
copy[key] = JSON.stringify(v);
20312068
}
20322069
}
20332070
return copy;

0 commit comments

Comments
 (0)