Repository navigation
feat(model): report what the last save wrote with savedChanges() - #4349
Conversation
hasChanged(), changedFrom(), changedProperties() and allChanges() describe unsaved changes, so a successful save resets them, and an afterCommit had no way to tell what its commit changed. Apps kept their own flags across a transaction and got their lifecycle wrong. savedChanges(), hasSavedChange(), savedChangeFrom() and savedChangedProperties() are their saved counterparts: - set right after the row write, before the after* callbacks, so afterSave, afterCommit and the caller see the same thing; a failed save puts back the previous state, a save that changed nothing reports nothing; - each transaction queue entry keeps its save's snapshot, so each afterCommit sees the save that queued it, and an afterRollback the save that was undone, after which the state from before it is restored (also for a model with no transaction callbacks); - on a create, every property the insert wrote, from ""; - reload() and clearChangeInformation() clear it; save(reload=true) keeps it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com>
Review @ c8874a7: source matches the approved design, one LOW. Final verdict after matrix 37331117757 and PR CIChecked against the F49 proposal, point by point: When it's captured.
Replaced and cleared.
Two saves in one transaction. Each
#4282's deferred enqueues share the queue. A Docs.
Finding
On the matrix I'll read |
A save inside one of the object's own callbacks replaces savedChanges() with its own changes, which include the outer save's because the outer save hasn't reset its baseline yet. The guide says so with an example, and a spec pins it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com>
Wheels Test Results222 964 tests 217 843 ✅ 1h 25m 35s ⏱️ Results for commit 4d48bda. |
Final: APPROVE @ 4d48bda (matrix 37332738128, all 32 legs read; PR CI read)Delta since c8874a7. Both of my notes are resolved.
Matrix
PR CI: 27 pass, 4 skipping, 0 fail, 0 pending. No open findings. The INFO limits of the measurement (wall-clock only, SQLite only) stand as notes, with the PR's fallback if a cost ever shows. |
Summary
hasChanged(),changedFrom(),changedProperties()andallChanges()describe unsaved changes, so a successful save resets them. AnafterCommit(or the code aftersave()) therefore had no way to tell what the commit changed. Apps kept their own flags across a transaction and got their lifecycle wrong: not reset after a commit, or acting without re-checking the final state.This adds the saved counterparts:
allChanges()savedChanges():{property: {changedFrom, changedTo}}hasChanged(property)hasSavedChange(property); any property when omittedchangedFrom(property)savedChangeFrom(property)changedProperties()savedChangedProperties()Semantics
afterCreate/afterUpdate/afterSave, so those callbacks,afterCommitand the caller all see the same thing.afterSavesets without saving it isn't reported as saved.{}."", including the generated key. Properties the object never set aren't listed.afterCommitsees the save that queued it.if (hasSavedChange("published") && this.published), and it is announced the next time it goes live.after_commitruns once per record with the last save's changes; Wheels firesafterCommitper save.afterRollbacksees the save that was rolled back (useful for compensating actions). Afterwards,savedChanges()reports the state from before that save.reload()andclearChangeInformation();clearChangeInformation(property)removes only that property.save(reload=true)keeps it.Implementation notes
model/properties.cfm,$-free (invariant 7). Internal helpers ($captureSavedChanges,$savedChangesState,$restoreSavedChanges,$reloadKeepingSavedChanges) are public and$-prefixed.model/callbacks.cfm):savedChanges/savedBeforesnapshots;$runQueueEntryCallbacks()applies the snapshot around one entry's callbacks;$restoreSavedChangesBeforeQueue()restores state after a rollback.finallyblocks only call helpers, with no loops insidefinally, and nothing writeslocal.inside acatch.$savestays under 30.Per-save cost (measured, from review)
Each save now runs
allChanges()once more, with twoDuplicate()copies. Inside a real Wheels transaction it also queues one entry per save, including for a model with noafterCommit/afterRollbackcallbacks, so a rollback can put the state back.I measured with a callback-free
tagmodel on Lucee 7 × SQLite, in milliseconds per 500update()calls. Develop and this branch were run interleaved: 9 runs each inside one transaction (the bulk-import case), and 9 runs each outside one.There's no measurable overhead inside a transaction: the difference is well inside run-to-run noise. Outside a transaction, the timings are dominated by host load (shared Docker host), so they show only that the change doesn't add anything visible.
So the commit-queue entry for callback-free models is kept, which preserves the "a rollback restores the previous state" guarantee. If a heavier model ever shows a cost, the cheap fallback is to skip that entry for callback-free models.
Docs
savedChanges()", after theafterCommitsection..ai/models.md(the consumer copy and thewheels newtemplate copy).changelog.d/saved-changes.added.md.Tests
model/SavedChangesSpeccovers:reload(),clearChangeInformation()(whole and per property) andsave(reload=true);afterSaveseeing the save in progress;afterCommitseeing its own save;afterRollbackseeing the undone save and the state restored afterwards, with and without transaction callbacks.RED on develop (only the spec and the Tag helpers added): all 11 checks error, because the functions don't exist.
GREEN locally, full core suite on SQLite, all with 0 fail / 0 error:
All four CFML engines discovered 542 bundles. These also passed with 0 fail / 0 error:
A full run caught one bug before this was pushed: the queue also carries deferred job enqueues. Only model-save entries touch the snapshot now.
🤖 Generated with Claude Code