diff --git a/changelog.d/callbacks-false-commit-callbacks.fixed.md b/changelog.d/callbacks-false-commit-callbacks.fixed.md new file mode 100644 index 0000000000..1ec7f37235 --- /dev/null +++ b/changelog.d/callbacks-false-commit-callbacks.fixed.md @@ -0,0 +1 @@ +- `save(callbacks=false)` and `delete(callbacks=false)` now skip `afterCommit` and `afterRollback` too, like every other callback. They used to queue them. With callbacks on, a save that changes nothing still queues `afterCommit` diff --git a/vendor/wheels/model/callbacks.cfm b/vendor/wheels/model/callbacks.cfm index 7aa4e1f93c..a4f607cca2 100644 --- a/vendor/wheels/model/callbacks.cfm +++ b/vendor/wheels/model/callbacks.cfm @@ -425,13 +425,15 @@ * fire; or, with no real transaction (none/false mode), fires afterCommit now * because the write is already committed (decision B1). Inside a foreign raw * transaction{} (detectable engines only) it skips both callbacks and warns once. + * With `callbacks = false` (a save or delete that skips its callbacks) only the saved-change + * restore is queued. */ - public void function $enqueueTransactionCallbacks(required string operation, struct savedBefore) { + public void function $enqueueTransactionCallbacks(required string operation, struct savedBefore, boolean callbacks = true) { // A delete changes no saved-change state, so a rollback puts back the current one. if (!StructKeyExists(arguments, "savedBefore")) { arguments.savedBefore = $savedChangesState(); } - if (!$hasTransactionCallbacks()) { + if (!arguments.callbacks || !$hasTransactionCallbacks()) { // No callbacks to fire, but a rollback must still put back what savedChanges() reported // before this save (F49), so the save is queued for that alone. $enqueueSavedChangesRestore(operation = arguments.operation, savedBefore = arguments.savedBefore); diff --git a/vendor/wheels/model/create.cfm b/vendor/wheels/model/create.cfm index d808d49069..2940723aae 100644 --- a/vendor/wheels/model/create.cfm +++ b/vendor/wheels/model/create.cfm @@ -181,7 +181,7 @@ local.rv = true; // v4.2.0: queue afterCommit/afterRollback (fires at the outermost // transaction resolve, or immediately in none/false mode). - $enqueueTransactionCallbacks(operation = "create", savedBefore = local.savedBefore); + $enqueueTransactionCallbacks(operation = "create", savedBefore = local.savedBefore, callbacks = arguments.callbacks); } else { $restoreSavedChanges(local.savedBefore); if (local.rollback) { @@ -214,7 +214,7 @@ } local.rv = true; // v4.2.0: queue afterCommit/afterRollback (see create branch). - $enqueueTransactionCallbacks(operation = "update", savedBefore = local.savedBefore); + $enqueueTransactionCallbacks(operation = "update", savedBefore = local.savedBefore, callbacks = arguments.callbacks); } else { $restoreSavedChanges(local.savedBefore); } diff --git a/vendor/wheels/model/delete.cfm b/vendor/wheels/model/delete.cfm index b7557a11f8..164a10e9d5 100644 --- a/vendor/wheels/model/delete.cfm +++ b/vendor/wheels/model/delete.cfm @@ -226,7 +226,7 @@ local.rv = true; // v4.2.0: queue afterCommit/afterRollback (fires at the outermost // transaction resolve, or immediately in none/false mode). - $enqueueTransactionCallbacks(operation = "delete"); + $enqueueTransactionCallbacks(operation = "delete", callbacks = arguments.callbacks); } } return local.rv; diff --git a/vendor/wheels/tests/specs/model/callbacksFalseAfterCommitSpec.cfc b/vendor/wheels/tests/specs/model/callbacksFalseAfterCommitSpec.cfc new file mode 100644 index 0000000000..9bcee2fbec --- /dev/null +++ b/vendor/wheels/tests/specs/model/callbacksFalseAfterCommitSpec.cfc @@ -0,0 +1,83 @@ +/** + * `callbacks = false` on save() / delete() also skips afterCommit and afterRollback, like every other + * callback. A save that changes nothing still queues afterCommit (unchanged, documented). The test + * runner uses transactionMode = "none", so specs pass `transaction` explicitly. + */ +component extends="wheels.WheelsTest" { + + function beforeAll() { + variables.g = application.wo; + } + + function run() { + + describe("callbacks = false and the commit callbacks", () => { + + beforeEach(() => { + request.$acLog = []; + variables.g.model("tag").$registerCallback(type = "afterCommit", methods = "recordAfterCommit"); + variables.g.model("tag").$registerCallback(type = "afterRollback", methods = "recordAfterRollback"); + }); + + afterEach(() => { + variables.g.model("tag").$clearCallbacks(type = "afterCommit"); + variables.g.model("tag").$clearCallbacks(type = "afterRollback"); + variables.g.model("tag").deleteAll(where = "name LIKE 'cbfalse-%'", instantiate = false, callbacks = false, transaction = "commit"); + }); + + it("skips afterCommit on a create with callbacks = false", () => { + variables.g.model("tag").new(name = "cbfalse-create").save(callbacks = false, transaction = "commit"); + expect(ArrayLen(request.$acLog)).toBe(0); + }); + + it("skips afterCommit on an update with callbacks = false", () => { + var t = variables.g.model("tag").new(name = "cbfalse-update"); + t.save(callbacks = false, transaction = "commit"); + t.name = "cbfalse-update-2"; + t.save(callbacks = false, transaction = "commit"); + expect(ArrayLen(request.$acLog)).toBe(0); + }); + + it("skips afterCommit on a delete with callbacks = false", () => { + var t = variables.g.model("tag").new(name = "cbfalse-delete"); + t.save(callbacks = false, transaction = "commit"); + t.delete(callbacks = false, transaction = "commit"); + expect(ArrayLen(request.$acLog)).toBe(0); + }); + + it("skips afterRollback on a rolled-back save with callbacks = false", () => { + variables.g.model("tag").new(name = "cbfalse-rollback").save(callbacks = false, transaction = "rollback"); + expect(ArrayLen(request.$acLog)).toBe(0); + }); + + it("still puts back savedChanges() when a callbacks = false save rolls back", () => { + var t = variables.g.model("tag").new(name = "cbfalse-saved"); + t.save(callbacks = false, transaction = "commit"); + var before = Duplicate(t.savedChanges()); + t.name = "cbfalse-saved-undone"; + t.save(callbacks = false, transaction = "rollback"); + expect(t.savedChanges()).toBe(before); + expect(ArrayLen(request.$acLog)).toBe(0); + }); + + it("still runs them with callbacks = true", () => { + var t = variables.g.model("tag").new(name = "cbfalse-on"); + t.save(transaction = "commit"); + t.delete(transaction = "commit"); + variables.g.model("tag").new(name = "cbfalse-on-rb").save(transaction = "rollback"); + expect(ArrayToList(request.$acLog)).toBe("commit:cbfalse-on,commit:cbfalse-on,rollback:cbfalse-on-rb"); + }); + + it("still queues afterCommit for a save that changes nothing", () => { + var t = variables.g.model("tag").new(name = "cbfalse-nochange"); + t.save(transaction = "commit"); + request.$acLog = []; + t.save(transaction = "commit"); + expect(ArrayLen(request.$acLog)).toBe(1); + }); + + }); + + } + +}