Skip to content

feat(model): report what the last save wrote with savedChanges() - #4349

Merged
bpamiri merged 2 commits into
developfrom
peter/saved-changes
Oct 5, 2026
Merged

bpamiri merged 2 commits into
developfrom
peter/saved-changes

Conversation

@bpamiri

@bpamiri bpamiri commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

hasChanged(), changedFrom(), changedProperties() and allChanges() describe unsaved changes, so a successful save resets them. An afterCommit (or the code after save()) 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:

Before the save After the save
allChanges() savedChanges(): {property: {changedFrom, changedTo}}
hasChanged(property) hasSavedChange(property); any property when omitted
changedFrom(property) savedChangeFrom(property)
changedProperties() savedChangedProperties()

Semantics

  • When it's set. Right after the row write, before afterCreate / afterUpdate / afterSave, so those callbacks, afterCommit and the caller all see the same thing.
    • A value an afterSave sets without saving it isn't reported as saved.
    • A save that fails after the write (an after-callback returns false) puts back the previous state.
    • A save that changed nothing reports {}.
  • On a create: every property the insert wrote, from "", including the generated key. Properties the object never set aren't listed.
  • Each afterCommit sees the save that queued it.
    • Each transaction queue entry keeps a snapshot of its save, and the object shows that snapshot while the entry's callbacks run. The object's properties hold the committed, final values throughout.
    • So one object saved twice in a transaction gives two different snapshots. A record that goes live and back down in one transaction is not "announced" by if (hasSavedChange("published") && this.published), and it is announced the next time it goes live.
    • This differs from Rails, where after_commit runs once per record with the last save's changes; Wheels fires afterCommit per save.
  • An afterRollback sees the save that was rolled back (useful for compensating actions). Afterwards, savedChanges() reports the state from before that save.
    • This also applies to a model with no transaction callbacks: such a save is queued for the restore alone, but only inside a real Wheels transaction.
  • Replaced by the next successful save, including a save inside one of the object's own callbacks. That inner save's changes include the outer save's, because the outer save hasn't reset its baseline yet; this is documented in the guide and pinned by a spec.
  • Cleared by reload() and clearChangeInformation(); clearChangeInformation(property) removes only that property. save(reload=true) keeps it.

Implementation notes

  • Public mixin functions in model/properties.cfm, $-free (invariant 7). Internal helpers ($captureSavedChanges, $savedChangesState, $restoreSavedChanges, $reloadKeepingSavedChanges) are public and $-prefixed.
  • Transaction queue (model/callbacks.cfm):
    • entries carry savedChanges / savedBefore snapshots;
    • $runQueueEntryCallbacks() applies the snapshot around one entry's callbacks;
    • $restoreSavedChangesBeforeQueue() restores state after a rollback.
  • Restore paths (invariants 11 and 12): finally blocks only call helpers, with no loops inside finally, and nothing writes local. inside a catch.
  • Non-model queue entries (a deferred job enqueue, fix(job): defer an enqueue inside a tenant-datasource transaction to its commit #4282) carry no snapshot and fire unchanged.
  • Delete passes no "before" state, so a rolled-back delete keeps the current one.
  • Complexity: the gate passes; $save stays under 30.

Per-save cost (measured, from review)

Each save now runs allChanges() once more, with two Duplicate() copies. Inside a real Wheels transaction it also queues one entry per save, including for a model with no afterCommit / afterRollback callbacks, so a rollback can put the state back.

I measured with a callback-free tag model on Lucee 7 × SQLite, in milliseconds per 500 update() calls. Develop and this branch were run interleaved: 9 runs each inside one transaction (the bulk-import case), and 9 runs each outside one.

develop this branch
Inside one transaction, median (range) ~260 (207–303) ~247 (203–309)
Outside a transaction 1512–7791 1056–2238

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

  • The v4-2-0 models guide: "What a save changed: savedChanges()", after the afterCommit section.
  • .ai/models.md (the consumer copy and the wheels new template copy).
  • changelog.d/saved-changes.added.md.

Tests

model/SavedChangesSpec covers:

  • a never-saved object;
  • create and update;
  • a no-op save;
  • a failed save keeping the previous state;
  • reload(), clearChangeInformation() (whole and per property) and save(reload=true);
  • afterSave seeing the save in progress;
  • two saves of one object in a transaction, each afterCommit seeing its own save;
  • the announce-once shape (live and back down in one transaction: 0; live again: 1; saved again while live: still 1);
  • afterRollback seeing 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:

Engine Pass
Lucee 7 6823
BoxLang 6823
Adobe 2025 6818
Adobe 2023 6818
RustCFML 6798

All four CFML engines discovered 542 bundles. These also passed with 0 fail / 0 error:

  • Lucee 7 × MySQL and PostgreSQL, and BoxLang × MySQL (model specs);
  • after rebasing onto develop: Lucee 7 model, internal and cli specs, and the guides build.

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

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>
@bpamiri bpamiri added this to the v4.2.0 milestone Oct 5, 2026
@bpamiri

bpamiri commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

Review @ c8874a7: source matches the approved design, one LOW. Final verdict after matrix 37331117757 and PR CI

Checked against the F49 proposal, point by point:

When it's captured. $save() takes local.savedBefore = $savedChangesState() before the row write, then calls $create() / $update(), then $captureSavedChanges(). All of that happens before $saveAssociations() and afterCreate / afterUpdate / afterSave.

  • At that point the baseline isn't reset yet ($updatePersistedProperties() runs after the callbacks), so allChanges() is exactly what the write sent. It's copied by value.
  • Neither $create() nor $update() reloads internally. save(reload = true) goes through $reloadKeepingSavedChanges(), so the reload doesn't wipe the capture.
  • Create semantics. On a create, $captureSavedChanges() drops properties the object doesn't hold, and pairs that are blank on both sides. So it reports what the INSERT wrote, including the generated key and timestamps, from "", as specified.
  • A failed save (an after-callback or association returns false) calls $restoreSavedChanges(local.savedBefore) on both branches, so a rolled-back write is never reported.

Replaced and cleared.

  • A nested save inside a callback captures after the outer one, so it wins. Its changes are measured against the not-yet-reset baseline, so they include the outer save's. That matches what the proposal documents.
  • reload() deletes variables.$savedChanges first.
  • clearChangeInformation() clears everything, and clearChangeInformation(property) clears that key only.

Two saves in one transaction. Each $enqueueTransactionCallbacks() entry now carries savedChanges = $savedChangesState() (that save's snapshot) and savedBefore. $runQueueEntryCallbacks() swaps the snapshot in for that entry's callbacks and restores the object's current state in a catch-free finally, also when a callback throws. So each afterCommit sees its own save.

afterRollback.

  • $runQueueCallbacks() runs the entries, then, in a catch-free finally, calls the $restoreSavedChangesBeforeQueue() helper (no loop in the finally, invariant 12). The helper walks the queue last to first, so the earliest savedBefore wins.
  • Savepoint rollbacks go through $runQueueCallbacks() too, so a rolled-back savepoint unit restores as well.
  • For models with no transaction callbacks, a callbacks = false entry is queued, only inside a real Wheels transaction, and it's skipped when callbacks fire.

#4282's deferred enqueues share the queue. A JobDeferredEnqueue entry has no savedChanges key, so $runQueueEntryCallbacks() fires it unchanged, and $restoreSavedChangesBeforeQueue() skips it (no savedBefore). Mixing the two in one queue is safe.

Docs.

  • The two .ai/models.md copies are byte-identical.
  • The v4-2-0 guide section matches the behaviour above, and its afterCommit(methods=…, on="create,update") example uses the real signature.
  • The added fragment is accurate.
  • No app or example model defines a method with these names.

Finding

  • LOW (cost on a hot path; please measure). Every successful save now does extra work, whether or not anything reads it:

    • allChanges(): hasChanged() plus changedProperties() plus changedFrom() per changed property;
    • two to three Duplicate() copies;
    • and inside a real transaction, one queue entry with two more snapshots even for models with no transaction callbacks.

    For a bulk import of N records in one transaction, the queue now holds N object references and 2N snapshots until the commit. Before this PR it held none for callback-free models. A quick before/after like perf(controller): apply a per-class precomputed method struct instead of the per-instance mixin loop #4149's (per-save time, and memory for a 10k-save transaction) would show whether it matters. If it does, the callbacks = false entries could store only savedBefore, or be skipped once a per-request size cap is reached. Not blocking.

  • INFO: the nested-save case (a save inside afterSave reports outer and inner changes together) is documented in the proposal, but not in the guide's bullet ("replaces it with that save's changes"). Half a sentence there would avoid surprise.

On the matrix I'll read SavedChangesSpec plus the transaction, callback and jobs specs on every leg, and the PR CI tally.

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>
@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Wheels Test Results

222 964 tests   217 843 ✅  1h 25m 35s ⏱️
 17 344 suites    5 121 💤
     32 files          0 ❌

Results for commit 4d48bda.

@bpamiri

bpamiri commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

Final: APPROVE @ 4d48bda (matrix 37332738128, all 32 legs read; PR CI read)

Delta since c8874a7. Both of my notes are resolved.

  • The measured cost is in the PR body: interleaved develop / branch runs, 9 each, with medians and ranges. Inside one transaction (the bulk-import case) it's 247 vs 260 ms per 500 updates, within noise. The restore entry for callback-free models is kept, so the "rollback restores the previous state" rule holds, and a fallback is named.
  • The nested-save combination is now in the guide (the title + slug example) and pinned by a new spec: savedChangedProperties() = description,name, and savedChangeFrom("name") = the value before the outer save. The registered afterSave is cleared in afterEach.

Matrix

  • 32/32 legs have 0 fail, 0 error, and every job succeeded, including rustcfml and the Compatibility Matrix Gate.
  • SavedChangesSpec passes 12/12 with 0 skipped on every leg (lucee6, lucee7, adobe2023, adobe2025, boxlang × every database). That covers:
    • create and update;
    • a no-op save;
    • a failed save keeping the previous state;
    • reload and clearChangeInformation;
    • save(reload = true);
    • afterSave;
    • the nested save;
    • two saves in one transaction, each afterCommit seeing its own;
    • the announce-once shape;
    • afterRollback with and without transaction callbacks.
  • Neighbouring specs are unchanged:
  • Skips. The skipped specs on boxlang-mysql, adobe2023-sqlserver, lucee7-postgres and adobe2025-oracle are identical to a current develop-based run (fix(model): read the sort direction in an order string whatever its case #4346's final matrix 37328749853).

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.

@bpamiri
bpamiri merged commit ca1ea83 into develop Oct 5, 2026
72 checks passed
@bpamiri
bpamiri deleted the peter/saved-changes branch October 5, 2026 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant