fix(orm): save() inserts or updates by whether the row exists

packages/bun-query-builder/src/orm.tsmodified+44-16

Changes to packages/bun-query-builder/src/orm.ts

		⋯1,061 linesShow the 1,061 lines above this hunk@@ -1062,10 +1062,21 @@ class ModelInstance<
1062	1062	private _forced: Set<string> = new Set()
1063	1063	private _definition: TDef
1064	1064	private _hasSaved = false
	1065	+ /**
	1066	+ * Whether this instance is a row in the table: true when it was read from
	1067	+ * the database or has been inserted, false for one built by `create()`,
	1068	+ * `make()` or `replicate()` that has not been saved yet. save() branches on
	1069	+ * this. It used to branch on whether the primary key was truthy, which sent
	1070	+ * a caller-supplied key (`create({ id: 5 })`, or any string key) down the
	1071	+ * UPDATE path, so nothing was inserted, and sent a loaded row whose key was
	1072	+ * `0` down the INSERT path, so it was duplicated.
	1073	+ */
	1074	+ private _exists: boolean
1065	1075	private _relations: Record<string, ModelInstance<any, any>[] | ModelInstance<any, any> | null> = {}
1066	1076
1067		- constructor(definition: TDef, attributes: Partial<ModelAttributes<TDef>> = {}) {
	1077	+ constructor(definition: TDef, attributes: Partial<ModelAttributes<TDef>>, exists: boolean) {
1068	1078	this._definition = definition
	1079	+ this._exists = exists
1069	1080	// The spread already gives us a private copy, so the normalizer may reuse
1070	1081	// it instead of building a second object per hydrated row.
1071	1082	this._attributes = normalizeAttributeKeysInPlace({ ...attributes })
		⋯234 linesShow the 234 lines above this hunk@@ -1306,13 +1317,17 @@ class ModelInstance<
1306	1317	}
1307	1318	}
1308	1319
1309		- if (this._attributes[pk]) {
	1320	+ if (this._exists) {
1310	1321	// Update
1311	1322	await hooks?.beforeUpdate?.(this as unknown as ModelHookInstance, this.getChanges())
1312	1323
1313	1324	const changes = this.getChanges()
1314	1325	const changeKeys = Object.keys(changes)
1315	1326	if (changeKeys.length > 0) {
	1327	+ // A row read without its key (`select('title')`) cannot be addressed.
	1328	+ // This used to take the INSERT branch and write a copy of the row.
	1329	+ if (this._attributes[pk] == null)
	1330	+ throw new Error(`Cannot save a model without a primary key: '${pk}' was not selected`)
1316	1331	const sets = changeKeys.map(k => `${k} = ?`).join(', ')
1317	1332	const values = [...Object.values(changes), this._attributes[pk]]
1318	1333
		⋯56 linesShow the 56 lines above this hunk@@ -1375,6 +1390,15 @@ class ModelInstance<
1375	1390	}
1376	1391	}
1377	1392
	1393	+ // A key the caller supplied is written. Neither loop above picks it up
	1394	+ // unless the model declares it as an attribute, and a string key has no
	1395	+ // database default to fall back on.
	1396	+ const suppliedKey = this._attributes[pk]
	1397	+ const keyAttr = attrs[pk]
	1398	+ const keyGuarded = keyAttr?.guarded && !this._forced.has(pk)
	1399	+ if (suppliedKey != null && !(pk in data) && !keyGuarded)
	1400	+ data[pk] = suppliedKey
	1401	+
1378	1402	if (timestampsEnabled(this._definition)) {
1379	1403	const now = formatNow()
1380	1404	// An explicitly supplied `created_at` wins.
		⋯35 linesShow the 35 lines above this hunk@@ -1416,8 +1440,12 @@ class ModelInstance<
1416	1440	for (const [key, value] of Object.entries(data)) {
1417	1441	this._attributes[key] = value
1418	1442	}
1419		- if (result.lastInsertId != null)
	1443	+ // Only a generated key is read back. For a supplied one the driver's
	1444	+ // insert id is the rowid (SQLite) or 0 (MySQL on a non-AUTO_INCREMENT
	1445	+ // key), not the key.
	1446	+ if (!(pk in data) && result.lastInsertId != null)
1420	1447	this._attributes[pk] = result.lastInsertId
	1448	+ this._exists = true
1421	1449
1422	1450	await hooks?.afterCreate?.(this as unknown as ModelHookInstance)
1423	1451	}
		⋯22 linesShow the 22 lines above this hunk@@ -1446,7 +1474,7 @@ class ModelInstance<
1446	1474	if (id == null) return null
1447	1475	const row = await exec.get(`SELECT * FROM ${this._definition.table} WHERE ${pk} = ?`, [id])
1448	1476	if (!row) return null
1449		- return new ModelInstance(this._definition, row as Partial<ModelAttributes<TDef>>)
	1477	+ return new ModelInstance(this._definition, row as Partial<ModelAttributes<TDef>>, true)
1450	1478	}
1451	1479
1452	1480	async delete(): Promise<boolean> {
		⋯116 linesShow the 116 lines above this hunk@@ -1569,7 +1597,7 @@ class ModelInstance<
1569	1597	delete attrs.uuid
1570	1598	delete attrs.created_at
1571	1599	delete attrs.updated_at
1572		- return new ModelInstance<TDef, TSelected>(this._definition, attrs as any)
	1600	+ return new ModelInstance<TDef, TSelected>(this._definition, attrs as any, false)
1573	1601	}
1574	1602
1575	1603	toArray(): Record<string, unknown> {
		⋯704 linesShow the 704 lines above this hunk@@ -2280,7 +2308,7 @@ export class BelongsToManyRelationBuilder<TRel extends ModelDefinition> {
2280	2308	else if (k !== fkParent && k !== fkRelated)
2281	2309	pivotExtras[k] = v
2282	2310	}
2283		- const inst = new ModelInstance(this._relatedDef, relatedRow as any)
	2311	+ const inst = new ModelInstance(this._relatedDef, relatedRow as any, true)
2284	2312	;(inst as any).pivot = pivotExtras
2285	2313	return inst
2286	2314	})
		⋯749 linesShow the 749 lines above this hunk@@ -3036,7 +3064,7 @@ class ModelQueryBuilder<
3036	3064	}
3037	3065	for (const instance of instances) {
3038	3066	const related = grouped.get(instance.get(pk as any)) || []
3039		- instance.setRelation(relationName, related.map(r => new ModelInstance(relDef as any, r as any)))
	3067	+ instance.setRelation(relationName, related.map(r => new ModelInstance(relDef as any, r as any, true)))
3040	3068	}
3041	3069	}
3042	3070	else {
		⋯8 linesShow the 8 lines above this hunk@@ -3051,7 +3079,7 @@ class ModelQueryBuilder<
3051	3079	}
3052	3080	for (const instance of instances) {
3053	3081	const row = byFk.get(instance.get(pk as any))
3054		- instance.setRelation(relationName, row ? new ModelInstance(relDef as any, row as any) : null)
	3082	+ instance.setRelation(relationName, row ? new ModelInstance(relDef as any, row as any, true) : null)
3055	3083	}
3056	3084	}
3057	3085	}
		⋯25 linesShow the 25 lines above this hunk@@ -3083,7 +3111,7 @@ class ModelQueryBuilder<
3083	3111	for (const instance of instances) {
3084	3112	const fkVal = (instance as any)._attributes[rel.foreignKey]
3085	3113	const row = byPk.get(fkVal)
3086		- instance.setRelation(relationName, row ? new ModelInstance(relDef as any, row as any) : null)
	3114	+ instance.setRelation(relationName, row ? new ModelInstance(relDef as any, row as any, true) : null)
3087	3115	}
3088	3116	}
3089	3117
		⋯54 linesShow the 54 lines above this hunk@@ -3144,7 +3172,7 @@ class ModelQueryBuilder<
3144	3172	.map((p) => {
3145	3173	const relRow = relatedByPk.get(p[rel.pivotFkRelated!])
3146	3174	if (!relRow) return null
3147		- const inst = new ModelInstance(relDef as any, relRow as any)
	3175	+ const inst = new ModelInstance(relDef as any, relRow as any, true)
3148	3176	// Attach pivot extras under instance.pivot
3149	3177	const extras: Record<string, unknown> = {}
3150	3178	for (const [k, v] of Object.entries(p)) {
		⋯46 linesShow the 46 lines above this hunk@@ -3197,7 +3225,7 @@ class ModelQueryBuilder<
3197	3225	const parentVal = throughToParent.get(row[rel.targetForeignKey!])
3198	3226	if (parentVal == null) continue
3199	3227	if (!byParent.has(parentVal)) byParent.set(parentVal, [])
3200		- byParent.get(parentVal)!.push(new ModelInstance(relDef as any, row as any))
	3228	+ byParent.get(parentVal)!.push(new ModelInstance(relDef as any, row as any, true))
3201	3229	}
3202	3230
3203	3231	for (const instance of instances) {
		⋯7 linesShow the 7 lines above this hunk@@ -3211,7 +3239,7 @@ class ModelQueryBuilder<
3211	3239	const exec = getExecutor()
3212	3240	const { sql, params } = this.buildQuery()
3213	3241	const rows = await exec.all(sql, params)
3214		- const instances = rows.map(row => new ModelInstance<TDef, TSelected>(this._definition, row as any))
	3242	+ const instances = rows.map(row => new ModelInstance<TDef, TSelected>(this._definition, row as any, true))
3215	3243
3216	3244	// Eager load relations
3217	3245	if (this._withRelations.length > 0) {
		⋯492 linesShow the 492 lines above this hunk@@ -3710,7 +3738,7 @@ function createModelInternal<const TDef extends ModelDefinition>(definition: TDe
3710	3738	// Exclude soft-deleted rows by default (use withTrashed()/query() to override).
3711	3739	const sd = softDeletesEnabled(definition) ? ` AND ${SOFT_DELETE_COLUMN} IS NULL` : ''
3712	3740	const row = await exec.get(`SELECT * FROM ${definition.table} WHERE ${pk} = ?${sd}`, [id])
3713		- return row ? new ModelInstance<TDef>(definition, row as any) as ModelRecord<TDef> : undefined
	3741	+ return row ? new ModelInstance<TDef>(definition, row as any, true) as ModelRecord<TDef> : undefined
3714	3742	},
3715	3743
3716	3744	async findOrFail(id: number | string): Promise<ModelRecord<TDef>> {
		⋯7 linesShow the 7 lines above this hunk@@ -3724,7 +3752,7 @@ function createModelInternal<const TDef extends ModelDefinition>(definition: TDe
3724	3752	const pk = definition.primaryKey || 'id'
3725	3753	const sd = softDeletesEnabled(definition) ? ` AND ${SOFT_DELETE_COLUMN} IS NULL` : ''
3726	3754	const rows = await exec.all(`SELECT * FROM ${definition.table} WHERE ${pk} IN (${ids.map(() => '?').join(', ')})${sd}`, ids)
3727		- return rows.map(row => new ModelInstance<TDef>(definition, row as any) as ModelRecord<TDef>)
	3755	+ return rows.map(row => new ModelInstance<TDef>(definition, row as any, true) as ModelRecord<TDef>)
3728	3756	},
3729	3757
3730	3758	all: () => new ModelQueryBuilder<TDef>(definition).get(),
		⋯14 linesShow the 14 lines above this hunk@@ -3745,7 +3773,7 @@ function createModelInternal<const TDef extends ModelDefinition>(definition: TDe
3745	3773	},
3746	3774
3747	3775	async create(data: FillableAttributes<TDef>): Promise<ModelRecord<TDef>> {
3748		- const instance = new ModelInstance<TDef>(definition, data as any)
	3776	+ const instance = new ModelInstance<TDef>(definition, data as any, false)
3749	3777	await instance.save()
3750	3778	return instance as ModelRecord<TDef>
3751	3779	},
		⋯91 linesShow the 91 lines above this hunk@@ -3843,7 +3871,7 @@ function createModelInternal<const TDef extends ModelDefinition>(definition: TDe
3843	3871	getTable: () => definition.table,
3844	3872
3845	3873	make(data: Partial<Attrs> = {}): ModelInstance<TDef> {
3846		- return new ModelInstance<TDef>(definition, data as any)
	3874	+ return new ModelInstance<TDef>(definition, data as any, false)
3847	3875	},
3848	3876
3849	3877	latest: (column: Cols = 'created_at' as Cols) => new ModelQueryBuilder<TDef>(definition).orderByDesc(column),