From 21a21d1b14590b7f4ed513504a7958dca03c2378 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ju=CC=88rg=20Lehni?= Date: Sun, 18 Oct 2026 17:04:37 +0200 Subject: [PATCH] Throw when a patchAndFetch() or updateAndFetch() query is executed a second time --- doc/api/query-builder/other-methods.md | 2 + doc/release-notes/changelog.md | 6 +- .../operations/UpdateAndFetchOperation.js | 17 +++++ tests/integration/misc/executeWritesOnce.js | 72 +++++++++++++++++++ 4 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 tests/integration/misc/executeWritesOnce.js diff --git a/doc/api/query-builder/other-methods.md b/doc/api/query-builder/other-methods.md index 19a8c7876..15f922ebb 100644 --- a/doc/api/query-builder/other-methods.md +++ b/doc/api/query-builder/other-methods.md @@ -803,6 +803,8 @@ const promise = queryBuilder.execute(); Executes the query and returns a Promise. +A query can be executed again, e.g. by awaiting it twice, except for the `*AndFetch()` writes: they merge the fetched row into their patch, which a second run would write back, so it throws. Create a new query for each run instead. A run that fails before the fetch, e.g. on a deadlock, can be retried. Concurrent runs of the same query aren't detected. + ##### Return value | Type | Description | diff --git a/doc/release-notes/changelog.md b/doc/release-notes/changelog.md index f95cc1294..30b788064 100644 --- a/doc/release-notes/changelog.md +++ b/doc/release-notes/changelog.md @@ -1,6 +1,10 @@ # Changelog -## 4.1.1 +## 4.2.0 + +### Changes + +- `patchAndFetch()`, `patchAndFetchById()`, `updateAndFetch()` and `updateAndFetchById()` throw when the same query is executed a second time, e.g. awaited twice. Since at least 1.0.0, the second run wrote back the row that the first one fetched, undoing other writes in between [#519](https://github.com/ditojs/objection/issues/519) ### Fixes diff --git a/lib/queryBuilder/operations/UpdateAndFetchOperation.js b/lib/queryBuilder/operations/UpdateAndFetchOperation.js index ff7a2098f..1c89c01df 100644 --- a/lib/queryBuilder/operations/UpdateAndFetchOperation.js +++ b/lib/queryBuilder/operations/UpdateAndFetchOperation.js @@ -13,6 +13,8 @@ export class UpdateAndFetchOperation extends DelegateOperation { this.id = null; this.skipIdWhere = false; this.selectOperations = null; + // Shared with the clones that each execution runs: + this.state = { isMerged: false }; } get model() { @@ -24,6 +26,19 @@ export class UpdateAndFetchOperation extends DelegateOperation { return this.delegate.onAdd(builder, args.slice(1)); } + onBefore1(builder, result) { + // A second run would write back the row that the first one merged into the + // patch: + if (this.state.isMerged) { + throw new Error( + `Unable to execute ${this.name}() twice, as the fetched row was merged into ` + + 'its patch. Create a new query for each run.', + ); + } + + return super.onBefore1(builder, result); + } + onBefore3(builder, result) { // Selects have no effect on the update query, so remember them here to // apply them to the fetch query. Collect them before the query is built, @@ -76,6 +91,7 @@ export class UpdateAndFetchOperation extends DelegateOperation { } this.model.$set(fetched); + this.state.isMerged = true; // A custom `select` may not include the id. The row was fetched by // id, so make sure the result still has it. if (!this.model.$hasId()) { @@ -102,6 +118,7 @@ export class UpdateAndFetchOperation extends DelegateOperation { clone.id = this.id; clone.skipIdWhere = this.skipIdWhere; + clone.state = this.state; return clone; } diff --git a/tests/integration/misc/executeWritesOnce.js b/tests/integration/misc/executeWritesOnce.js new file mode 100644 index 000000000..6459464db --- /dev/null +++ b/tests/integration/misc/executeWritesOnce.js @@ -0,0 +1,72 @@ +import { describe, it, expect, beforeEach } from 'vitest'; + +export default (session) => { + describe('executing an *AndFetch() query twice', () => { + const Model1 = session.models.Model1; + + beforeEach(() => { + return session.populate([{ id: 1, model1Prop1: 'hello 1' }]); + }); + + it('should throw for patchAndFetchById()', async () => { + const query = Model1.query().patchAndFetchById(1, { model1Prop2: 2 }); + await query; + await Model1.query().patch({ model1Prop1: 'changed' }).findById(1); + // A second run would write back the row that the first one fetched: + await expect(query).rejects.toThrow( + 'Unable to execute patchAndFetch() twice, as the fetched row was merged into ' + + 'its patch. Create a new query for each run.', + ); + expect(await Model1.query().findById(1)).toMatchObject({ model1Prop1: 'changed' }); + }); + + it('should throw for $query().patchAndFetch()', async () => { + const model = await Model1.query().findById(1); + const query = model.$query().patchAndFetch({ model1Prop2: 2 }); + await query; + await expect(query).rejects.toThrow('Create a new query for each run.'); + }); + + it('should run again after a run that failed before the fetch', async () => { + let shouldFail = true; + const query = Model1.query() + .context({ + runBefore() { + if (shouldFail) { + throw new Error('deadlock'); + } + }, + }) + .patchAndFetchById(1, { model1Prop2: 2 }); + + await expect(query).rejects.toThrow('deadlock'); + shouldFail = false; + expect(await query).toMatchObject({ id: 1, model1Prop1: 'hello 1', model1Prop2: 2 }); + }); + + it('should run patch(), delete() and finds again', async () => { + const patch = Model1.query().patch({ model1Prop2: 2 }).findById(1); + expect(await patch).toBe(1); + expect(await patch).toBe(1); + + const find = Model1.query().findById(1); + expect(await find).toMatchObject({ id: 1 }); + expect(await find).toMatchObject({ id: 1 }); + + const deleteQuery = Model1.query().delete().where('model1Prop1', 'nobody'); + expect(await deleteQuery).toBe(0); + expect(await deleteQuery).toBe(0); + }); + + it.skipIf(session.isMsSql())('should run an insert with onConflict() again', async () => { + const query = Model1.query() + .insert({ id: 2, model1Prop1: 'inserted' }) + .onConflict('id') + .merge(); + + await query; + await query; + expect(await Model1.query().findById(2)).toMatchObject({ model1Prop1: 'inserted' }); + }); + }); +};