fix(client): refuse a write whose predicate the builder cannot read

packages/bun-query-builder/test/client.write-predicate-required.test.tsadded+182-0

Changes to packages/bun-query-builder/test/client.write-predicate-required.test.ts

		@@ -0,0 +1,182 @@
	1	+/**
	2	+ * A write must never run without the predicate the caller supplied.
	3	+ * stacksjs/bun-query-builder#1101 (and the empty-object half of #1095).
	4	+ *
	5	+ * `updateTable(...).where(x)` and `deleteFrom(...).where(x)` accepted argument
	6	+ * shapes they could not turn into a predicate and then returned `this`
	7	+ * unchanged. The statement kept no WHERE at all, so it ran against every row
	8	+ * and reported the full affected count as success:
	9	+ *
	10	+ * deleteFrom(t).where(undefined) -> deleted all 4 rows
	11	+ * updateTable(t).set(v).where(raw('id = 1')) -> rewrote all 4 rows
	12	+ *
	13	+ * The `raw()` case is the serious one, because it is not a mistake the types
	14	+ * catch: the declared signature is `WhereExpression | K | SqlFragment` and
	15	+ * `raw` is a public export, so that call reads correctly and passes review.
	16	+ *
	17	+ * Two different remedies, deliberately:
	18	+ * - a fragment is SUPPORTED, because the signature promises it,
	19	+ * - anything the builder genuinely cannot read is REFUSED, because a write
	20	+ * builder is the wrong place to treat "I could not understand your filter"
	21	+ * as "match every row".
	22	+ *
	23	+ * `where({})` throws with a message naming the problem instead of the SQL
	24	+ * syntax error it used to produce — that is #1095, whose reported symptom was
	25	+ * always the safe one.
	26	+ *
	27	+ * Every assertion below checks SURVIVING ROWS, not just that a call threw. The
	28	+ * bug was damage, so the table is the thing worth asserting on.
	29	+ */
	30	+
	31	+import { mkdtempSync, rmSync } from 'node:fs'
	32	+import { tmpdir } from 'node:os'
	33	+import { join } from 'node:path'
	34	+import { Database } from 'bun:sqlite'
	35	+import { afterEach, beforeEach, describe, expect, it } from 'bun:test'
	36	+import { buildDatabaseSchema, buildSchemaMeta, createQueryBuilder, raw } from '../src'
	37	+import { config, setConfig } from '../src/config'
	38	+import { resetConnection } from '../src/db'
	39	+
	40	+const MODELS = {
	41	+ t: { columns: { id: { type: 'integer', isPrimaryKey: true }, name: { type: 'string' } } },
	42	+} as any
	43	+
	44	+let dir: string
	45	+let dbPath: string
	46	+let snapshot: { dialect: string, database: Record<string, unknown> }
	47	+
	48	+function qb(): any {
	49	+ return createQueryBuilder({
	50	+ schema: buildDatabaseSchema(MODELS),
	51	+ meta: buildSchemaMeta(MODELS),
	52	+ autoMigration: { enabled: false } as any,
	53	+ })
	54	+}
	55	+
	56	+function rows(): Array<{ id: number, name: string }> {
	57	+ const probe = new Database(dbPath)
	58	+ const out = probe.query('SELECT id, name FROM t ORDER BY id').all() as any[]
	59	+ probe.close()
	60	+ return out
	61	+}
	62	+
	63	+beforeEach(() => {
	64	+ snapshot = { dialect: config.dialect, database: { ...config.database } }
	65	+ dir = mkdtempSync(join(tmpdir(), 'qb-1101-'))
	66	+ dbPath = join(dir, 'writes.sqlite')
	67	+ const seed = new Database(dbPath, { create: true })
	68	+ seed.run('CREATE TABLE t (id INTEGER PRIMARY KEY, name TEXT)')
	69	+ seed.run(`INSERT INTO t (id, name) VALUES (1,'a'),(2,'b'),(3,'c'),(4,'d')`)
	70	+ seed.close()
	71	+ setConfig({ dialect: 'sqlite', database: { database: dbPath } } as any)
	72	+ resetConnection()
	73	+})
	74	+
	75	+afterEach(() => {
	76	+ config.dialect = snapshot.dialect as any
	77	+ for (const k of Object.keys(config.database)) delete (config.database as any)[k]
	78	+ Object.assign(config.database, snapshot.database)
	79	+ resetConnection()
	80	+ rmSync(dir, { recursive: true, force: true })
	81	+})
	82	+
	83	+describe('DELETE refuses to run without a predicate (#1101)', () => {
	84	+ for (const [label, value] of [['undefined', undefined], ['null', null]] as const) {
	85	+ it(`where(${label}) throws and leaves every row in place`, () => {
	86	+ // Synchronously, at the point of the mistake — no statement is ever built.
	87	+ expect(() => qb().deleteFrom('t').where(value as any))
	88	+ .toThrow(/expected a condition/)
	89	+
	90	+ // The assertion that matters: all four rows are still here.
	91	+ expect(rows()).toHaveLength(4)
	92	+ })
	93	+ }
	94	+
	95	+ it('where({}) names the problem instead of emitting a broken statement', () => {
	96	+ expect(() => qb().deleteFrom('t').where({}))
	97	+ .toThrow(/an empty object is not a filter/)
	98	+ expect(rows()).toHaveLength(4)
	99	+ })
	100	+})
	101	+
	102	+describe('UPDATE refuses to run without a predicate (#1101)', () => {
	103	+ for (const [label, value] of [['undefined', undefined], ['null', null]] as const) {
	104	+ it(`where(${label}) throws and rewrites nothing`, () => {
	105	+ expect(() => qb().updateTable('t').set({ name: 'X' }).where(value as any))
	106	+ .toThrow(/expected a condition/)
	107	+
	108	+ expect(rows().filter(r => r.name === 'X')).toHaveLength(0)
	109	+ })
	110	+ }
	111	+
	112	+ it('where({}) names the problem instead of emitting a broken statement', () => {
	113	+ expect(() => qb().updateTable('t').set({ name: 'X' }).where({}))
	114	+ .toThrow(/an empty object is not a filter/)
	115	+ expect(rows().filter(r => r.name === 'X')).toHaveLength(0)
	116	+ })
	117	+})
	118	+
	119	+describe('a SQL fragment is honoured, not dropped (#1101)', () => {
	120	+ // The signature accepts SqlFragment, so these have to work — refusing them
	121	+ // would swap a silent catastrophe for a broken advertised API.
	122	+
	123	+ it('DELETE where(raw(...)) removes only the matching row', async () => {
	124	+ await qb().deleteFrom('t').where(raw('id = 1')).execute()
	125	+
	126	+ // Previously this threw on Postgres (the fragment was bound as a value)
	127	+ // and, on the update side, wiped the table.
	128	+ expect(rows().map(r => r.id)).toEqual([2, 3, 4])
	129	+ })
	130	+
	131	+ it('UPDATE where(raw(...)) rewrites only the matching row', async () => {
	132	+ await qb().updateTable('t').set({ name: 'X' }).where(raw('id = 1')).execute()
	133	+
	134	+ const after = rows()
	135	+ expect(after.filter(r => r.name === 'X').map(r => r.id)).toEqual([1])
	136	+ expect(after).toHaveLength(4)
	137	+ })
	138	+
	139	+ it('DELETE honours a parameterised fragment', async () => {
	140	+ await qb().deleteFrom('t').where({ sql: 'id = ?', parameters: [2] } as any).execute()
	141	+ expect(rows().map(r => r.id)).toEqual([1, 3, 4])
	142	+ })
	143	+
	144	+ it('UPDATE honours a parameterised fragment', async () => {
	145	+ await qb().updateTable('t').set({ name: 'X' }).where({ sql: 'id = ?', parameters: [2] } as any).execute()
	146	+ expect(rows().filter(r => r.name === 'X').map(r => r.id)).toEqual([2])
	147	+ })
	148	+
	149	+ it('does not mistake a bound fragment for a column map', async () => {
	150	+ // `{ sql, parameters }` is an object, so an ordering slip would render it
	151	+ // as `"sql" = ? AND "parameters" = ?` and match nothing — silently.
	152	+ await qb().deleteFrom('t').where({ sql: 'id = ?', parameters: [3] } as any).execute()
	153	+ expect(rows().map(r => r.id)).toEqual([1, 2, 4])
	154	+ })
	155	+})
	156	+
	157	+describe('ordinary writes are unaffected (#1101)', () => {
	158	+ it('DELETE with an object filter still removes exactly its rows', async () => {
	159	+ await qb().deleteFrom('t').where({ id: 1 }).execute()
	160	+ expect(rows().map(r => r.id)).toEqual([2, 3, 4])
	161	+ })
	162	+
	163	+ it('UPDATE with an object filter still rewrites exactly its rows', async () => {
	164	+ await qb().updateTable('t').set({ name: 'X' }).where({ id: 2 }).execute()
	165	+ expect(rows().filter(r => r.name === 'X').map(r => r.id)).toEqual([2])
	166	+ })
	167	+
	168	+ it('the 3-arg form still works on both builders', async () => {
	169	+ await qb().updateTable('t').set({ name: 'X' }).where('id', '=', 3).execute()
	170	+ expect(rows().filter(r => r.name === 'X').map(r => r.id)).toEqual([3])
	171	+
	172	+ await qb().deleteFrom('t').where('id', '=', 4).execute()
	173	+ expect(rows().map(r => r.id)).toEqual([1, 2, 3])
	174	+ })
	175	+
	176	+ it('an intentional unfiltered write is still possible without where()', async () => {
	177	+ // Refusing an unreadable filter must not remove the ability to write the
	178	+ // whole table on purpose — the point is that it has to be explicit.
	179	+ await qb().deleteFrom('t').execute()
	180	+ expect(rows()).toHaveLength(0)
	181	+ })
	182	+})