diff --git a/changelog.d/4370-soft-delete-dependents.changed.md b/changelog.d/4370-soft-delete-dependents.changed.md new file mode 100644 index 0000000000..3e20fce63d --- /dev/null +++ b/changelog.d/4370-soft-delete-dependents.changed.md @@ -0,0 +1 @@ +- Soft-deleting a record no longer destroys or unlinks `dependent=` children that a restore would need. Children whose model has a soft-delete column are soft-deleted with it, as before. Other children are left alone: `dependent="delete"` / `"deleteAll"` no longer hard-deletes them and `dependent="remove"` / `"removeAll"` no longer sets their foreign keys to `NULL`. On a soft delete, `dependent=` now applies to them only on a real delete (#4370) diff --git a/changelog.d/4371-soft-delete-false-purges.changed.md b/changelog.d/4371-soft-delete-false-purges.changed.md new file mode 100644 index 0000000000..2148613469 --- /dev/null +++ b/changelog.d/4371-soft-delete-false-purges.changed.md @@ -0,0 +1 @@ +- `delete()`, `deleteAll()`, `deleteOne()` and `deleteByKey()` with `softDelete=false` are always real deletes. They now also remove rows that were already soft-deleted, and the `dependent=` children of a deleted record, without `includeSoftDeletes=true`. An explicit `includeSoftDeletes=false` no longer keeps them. With `instantiate=true` those rows now run their delete callbacks too (#4371) diff --git a/changelog.d/4372-reload-missing-row.changed.md b/changelog.d/4372-reload-missing-row.changed.md new file mode 100644 index 0000000000..dd7fc2f8d7 --- /dev/null +++ b/changelog.d/4372-reload-missing-row.changed.md @@ -0,0 +1 @@ +- `reload()` throws `Wheels.RecordNotFound` when the object's row no longer exists, instead of silently blanking every property. Uncaught in a controller action, that renders the 404 page. An object with no key (unsaved, or saved on a database that returns no generated key, such as Oracle) is left as it is, where it used to have every property blanked (#4372) diff --git a/changelog.d/4372-soft-deleted-instance.fixed.md b/changelog.d/4372-soft-deleted-instance.fixed.md new file mode 100644 index 0000000000..2014e6503b --- /dev/null +++ b/changelog.d/4372-soft-deleted-instance.fixed.md @@ -0,0 +1,4 @@ +- An object whose row is soft-deleted behaves consistently (#4372): + - After a soft `delete()`, the object's `deletedAt` holds the timestamp written. It goes back to its earlier value if the delete is vetoed or rolled back, so a retry works. + - A second soft `delete()`, or one from a stale copy of a row soft-deleted elsewhere, returns `false` and changes nothing. It no longer rewrites `deletedAt`, and it no longer soft-deletes the dependents of a row it didn't delete. + - `reload()` reloads a soft-deleted row instead of blanking every property. diff --git a/cli/lucli/templates/app/.ai/models.md b/cli/lucli/templates/app/.ai/models.md index 58378e7e59..c3ff4a5b75 100644 --- a/cli/lucli/templates/app/.ai/models.md +++ b/cli/lucli/templates/app/.ai/models.md @@ -61,6 +61,7 @@ Finders: `model("User").findAll()`, `findOne(where="...")`, `findByKey(params.ke Create: `model("User").new(params.user).save()`, or `model("User").create(params.user)`. Include associations: `findAll(include="role,orders")`. Pagination: `findAll(page=params.page, perPage=25)`. What the last save wrote (4.2+): `savedChanges()`, `hasSavedChange("status")`, `savedChangeFrom("status")`, `savedChangedProperties()`, the saved counterparts of `allChanges()` / `hasChanged()` / `changedFrom()` / `changedProperties()`. Use them in `afterCommit`: each call sees the save that queued it, so a record saved twice in one transaction is told apart, and no hand-kept flag is needed (`if (hasSavedChange("published") && this.published) ...`). +Soft delete (table has `deletedAt`, which `t.timestamps()` adds): `delete()` sets `deletedAt`, and finders skip such rows unless `includeSoftDeletes=true`. `softDelete=false` is a real DELETE, including rows that are already soft-deleted (4.2+). On a soft delete, `dependent=` children with their own `deletedAt` are soft-deleted and all others are left untouched. `reload()` finds a soft-deleted row and throws `Wheels.RecordNotFound` when the row is gone. A second soft `delete()` returns `false`. Opt a `select=false` calculated property into one call (additive): `findAll(includeCalculated="fullName")`. Unknown names throw `Wheels.CalculatedPropertyNotFound` in dev/testing. Literal LIKE search: `findAll(where="title LIKE '%#escapeForLike(params.q)#%' ESCAPE '\'")`. `escapeForLike()` escapes `\` `%` `_` (and `[` on SQL Server — `\[` is illegal on Oracle; on SQL Server `[` is escaped only after a model has initialised the adapter, so call a `model()` finder before using it in a fresh/just-reloaded app) so a user's term isn't read as wildcards; the quoted literal is bound. Always declare `ESCAPE '\'`: MySQL/PG/CockroachDB/H2 default the escape char to `\` but SQLite/Oracle/SQL Server have none, and the 3-arg builder `where("title","LIKE",...)` emits no `ESCAPE`, so without it `\` matches literally. Escapes LIKE metacharacters only, not SQL quotes. diff --git a/docs/consumer-ai/.ai/models.md b/docs/consumer-ai/.ai/models.md index 58378e7e59..c3ff4a5b75 100644 --- a/docs/consumer-ai/.ai/models.md +++ b/docs/consumer-ai/.ai/models.md @@ -61,6 +61,7 @@ Finders: `model("User").findAll()`, `findOne(where="...")`, `findByKey(params.ke Create: `model("User").new(params.user).save()`, or `model("User").create(params.user)`. Include associations: `findAll(include="role,orders")`. Pagination: `findAll(page=params.page, perPage=25)`. What the last save wrote (4.2+): `savedChanges()`, `hasSavedChange("status")`, `savedChangeFrom("status")`, `savedChangedProperties()`, the saved counterparts of `allChanges()` / `hasChanged()` / `changedFrom()` / `changedProperties()`. Use them in `afterCommit`: each call sees the save that queued it, so a record saved twice in one transaction is told apart, and no hand-kept flag is needed (`if (hasSavedChange("published") && this.published) ...`). +Soft delete (table has `deletedAt`, which `t.timestamps()` adds): `delete()` sets `deletedAt`, and finders skip such rows unless `includeSoftDeletes=true`. `softDelete=false` is a real DELETE, including rows that are already soft-deleted (4.2+). On a soft delete, `dependent=` children with their own `deletedAt` are soft-deleted and all others are left untouched. `reload()` finds a soft-deleted row and throws `Wheels.RecordNotFound` when the row is gone. A second soft `delete()` returns `false`. Opt a `select=false` calculated property into one call (additive): `findAll(includeCalculated="fullName")`. Unknown names throw `Wheels.CalculatedPropertyNotFound` in dev/testing. Literal LIKE search: `findAll(where="title LIKE '%#escapeForLike(params.q)#%' ESCAPE '\'")`. `escapeForLike()` escapes `\` `%` `_` (and `[` on SQL Server — `\[` is illegal on Oracle; on SQL Server `[` is escaped only after a model has initialised the adapter, so call a `model()` finder before using it in a fresh/just-reloaded app) so a user's term isn't read as wildcards; the quoted literal is bound. Always declare `ESCAPE '\'`: MySQL/PG/CockroachDB/H2 default the escape char to `\` but SQLite/Oracle/SQL Server have none, and the 3-arg builder `where("title","LIKE",...)` emits no `ESCAPE`, so without it `\` matches literally. Escapes LIKE metacharacters only, not SQL quotes. diff --git a/vendor/wheels/model/associations.cfm b/vendor/wheels/model/associations.cfm index 54b9702efd..fc9405f12d 100644 --- a/vendor/wheels/model/associations.cfm +++ b/vendor/wheels/model/associations.cfm @@ -242,12 +242,18 @@ /* * Called when a model object is deleted (e.g. post.delete()). - * Deletes all associated records (or sets their foreign key values to NULL). + * Deletes all associated records (or sets their foreign key values to NULL). When the object itself + * was soft-deleted (`parentSoftDeleted`), nothing a restore would need is destroyed or unlinked: + * dependents that can be soft-deleted are, and every other dependent is left alone. */ - public void function $deleteDependents(boolean softDelete = true, boolean includeSoftDeletes = false) { + public void function $deleteDependents(boolean softDelete = true, boolean includeSoftDeletes = false, boolean parentSoftDeleted = false) { for (local.key in variables.wheels.class.associations) { local.association = variables.wheels.class.associations[local.key]; - if (ListFindNoCase("hasMany,hasOne", local.association.type) && local.association.dependent != false) { + if ( + ListFindNoCase("hasMany,hasOne", local.association.type) + && local.association.dependent != false + && !$dependentKeptBySoftDelete(association = local.association, parentSoftDeleted = arguments.parentSoftDeleted) + ) { local.all = ""; if (local.association.type == "hasMany") { local.all = "All"; @@ -285,6 +291,21 @@ } } + /** + * Internal function. True when a soft-deleted parent leaves this dependent association alone: + * `remove` / `removeAll` would unlink the children, and `delete` / `deleteAll` would destroy children + * whose model has no soft-delete column. + */ + public boolean function $dependentKeptBySoftDelete(required struct association, required boolean parentSoftDeleted) { + if (!arguments.parentSoftDeleted) { + return false; + } + if (ListFindNoCase("remove,removeAll", arguments.association.dependent)) { + return true; + } + return !model(arguments.association.modelName).$softDeletion(); + } + /** * Internal function. * Registers a foreign key property with integer type for BoxLang compatibility. diff --git a/vendor/wheels/model/callbacks.cfm b/vendor/wheels/model/callbacks.cfm index a4f607cca2..11dd5f6a9c 100644 --- a/vendor/wheels/model/callbacks.cfm +++ b/vendor/wheels/model/callbacks.cfm @@ -428,7 +428,7 @@ * 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, boolean callbacks = true) { + public void function $enqueueTransactionCallbacks(required string operation, struct savedBefore, boolean callbacks = true, struct softDeleteBefore) { // A delete changes no saved-change state, so a rollback puts back the current one. if (!StructKeyExists(arguments, "savedBefore")) { arguments.savedBefore = $savedChangesState(); @@ -436,7 +436,7 @@ 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); + $enqueueSavedChangesRestore(argumentCollection = arguments); return; } local.conn = this.$hashedConnectionArgs(); @@ -456,16 +456,17 @@ && StructKeyExists(request.wheels.$txnCallbacks, local.conn) && request.wheels.$txnCallbacks[local.conn].real ) { - ArrayAppend( - request.wheels.$txnCallbacks[local.conn].queue, - { - object = this, - operation = arguments.operation, - callbacks = true, - savedChanges = $savedChangesState(), - savedBefore = arguments.savedBefore - } - ); + local.entry = { + object = this, + operation = arguments.operation, + callbacks = true, + savedChanges = $savedChangesState(), + savedBefore = arguments.savedBefore + }; + if (StructKeyExists(arguments, "softDeleteBefore")) { + local.entry.softDeleteBefore = arguments.softDeleteBefore; + } + ArrayAppend(request.wheels.$txnCallbacks[local.conn].queue, local.entry); } else { this.$runTransactionCallbacks(type = "afterCommit", operation = arguments.operation); } @@ -476,7 +477,7 @@ * Wheels transaction only, so a rollback puts back the saved-change state from before it (F49). * Checked cheaply first: outside a Wheels transaction there is nothing to queue. */ - public void function $enqueueSavedChangesRestore(required string operation, required struct savedBefore) { + public void function $enqueueSavedChangesRestore(required string operation, required struct savedBefore, struct softDeleteBefore) { if ( !StructKeyExists(request, "wheels") || !StructKeyExists(request.wheels, "$txnCallbacks") @@ -486,16 +487,17 @@ } local.conn = this.$hashedConnectionArgs(); if (StructKeyExists(request.wheels.$txnCallbacks, local.conn) && request.wheels.$txnCallbacks[local.conn].real) { - ArrayAppend( - request.wheels.$txnCallbacks[local.conn].queue, - { - object = this, - operation = arguments.operation, - callbacks = false, - savedChanges = $savedChangesState(), - savedBefore = arguments.savedBefore - } - ); + local.entry = { + object = this, + operation = arguments.operation, + callbacks = false, + savedChanges = $savedChangesState(), + savedBefore = arguments.savedBefore + }; + if (StructKeyExists(arguments, "softDeleteBefore")) { + local.entry.softDeleteBefore = arguments.softDeleteBefore; + } + ArrayAppend(request.wheels.$txnCallbacks[local.conn].queue, local.entry); } } @@ -842,6 +844,10 @@ if (StructKeyExists(local.entry, "savedBefore")) { local.entry.object.$restoreSavedChanges(local.entry.savedBefore); } + // A rolled-back soft delete also puts back the object's deletedAt. + if (StructKeyExists(local.entry, "softDeleteBefore")) { + local.entry.object.$restoreSoftDeleteState(local.entry.softDeleteBefore); + } } } diff --git a/vendor/wheels/model/delete.cfm b/vendor/wheels/model/delete.cfm index 164a10e9d5..6c4b04c307 100644 --- a/vendor/wheels/model/delete.cfm +++ b/vendor/wheels/model/delete.cfm @@ -32,6 +32,10 @@ boolean softDelete = true ) { $args(name = "deleteAll", args = arguments); + // A permanent delete is a real DELETE: it reaches rows that are already soft-deleted too. + if (!arguments.softDelete) { + arguments.includeSoftDeletes = true; + } arguments.include = $listClean(arguments.include); arguments.where = $cleanInList(arguments.where); if (arguments.instantiate) { @@ -106,6 +110,10 @@ boolean softDelete = true ) { $args(name = "deleteByKey", args = arguments); + // A permanent delete is a real DELETE: it reaches rows that are already soft-deleted too. + if (!arguments.softDelete) { + arguments.includeSoftDeletes = true; + } $keyLengthCheck(arguments.key); local.where = $keyWhereWithScope(key = arguments.key, where = StructKeyExists(arguments, "where") ? arguments.where : ""); return deleteOne( @@ -144,6 +152,10 @@ boolean softDelete = true ) { $args(name = "deleteOne", args = arguments); + // A permanent delete is a real DELETE: it reaches rows that are already soft-deleted too. + if (!arguments.softDelete) { + arguments.includeSoftDeletes = true; + } local.object = findOne( includeSoftDeletes = arguments.includeSoftDeletes, order = arguments.order, @@ -184,9 +196,28 @@ boolean softDelete = true ) { $args(name = "delete", args = arguments); + if (variables.wheels.class.softDeletion && arguments.softDelete) { + // Already soft-deleted (as loaded, not as edited in memory): there is nothing to do, and + // stamping deletedAt again would lose when it was first deleted. + if (Len($persistedSoftDeleteValue())) { + return false; + } + } else if (!arguments.softDelete) { + // A permanent delete is a real DELETE, so dependents that are already soft-deleted go too. + // A default delete of a model without a soft-delete column still soft-deletes its + // soft-delete dependents, and leaves the ones already soft-deleted as they are. + arguments.includeSoftDeletes = true; + } arguments.sql = []; arguments.sql = $addDeleteClause(sql = arguments.sql, softDelete = arguments.softDelete); arguments.sql = $addKeyWhereClause(sql = arguments.sql, includeSoftDeletes = arguments.includeSoftDeletes); + if (variables.wheels.class.softDeletion && arguments.softDelete) { + // Only a row that isn't soft-deleted yet, so a stale copy of a row soft-deleted elsewhere + // changes nothing. Keep the timestamp being written for the object to hold afterwards. + ArrayAppend(arguments.sql, " AND " & $quoteColumn(variables.wheels.class.softDeleteColumn) & " IS NULL"); + arguments.$softDeletedAt = arguments.sql[2].value; + arguments.$softDeleteBefore = $softDeleteState(); + } return invokeWithTransaction(method = "$delete", argumentCollection = arguments); } @@ -208,27 +239,128 @@ */ public boolean function $delete() { local.rv = false; - if ($callback("beforeDelete", arguments.callbacks)) { - // Delete dependent record(s). - // Done before the main record is deleted to make sure eventual foreign key constraints does not prevent deletion. - $deleteDependents( - softDelete = arguments.softDelete, - includeSoftDeletes = arguments.includeSoftDeletes - ); + // A soft delete that doesn't complete (afterDelete returns false, or a dependent throws) puts + // back the object's deletedAt, so a retry isn't refused as "already deleted". Catch-free, so + // it also runs when the request aborts on BoxLang. + try { + local.rv = $deleteRecordAndDependents(argumentCollection = arguments); + } finally { + if (!local.rv && StructKeyExists(arguments, "$softDeleteBefore")) { + $restoreSoftDeleteState(arguments.$softDeleteBefore); + } + } + return local.rv; + } - local.deleted = variables.wheels.class.adapter.$querySetup( - sql = arguments.sql, - parameterize = arguments.parameterize - ); - $markMigrationDidWork(); - $clearRequestCache(); - if (local.deleted.result.recordCount == 1 && $callback("afterDelete", arguments.callbacks)) { + /** + * Internal function. The body of `$delete()`: callbacks, dependents and the statement itself. + */ + public boolean function $deleteRecordAndDependents() { + local.rv = false; + if ($callback("beforeDelete", arguments.callbacks)) { + if (StructKeyExists(arguments, "$softDeletedAt")) { + // A soft delete keeps the row, so no foreign key needs the dependents handled first: + // mark this row, then soft-delete its dependents only if this call is what did it. + local.recordCount = $runDeleteStatement(sql = arguments.sql, parameterize = arguments.parameterize); + if (local.recordCount == 1) { + $markSoftDeleted(arguments.$softDeletedAt); + $deleteDependents(softDelete = true, includeSoftDeletes = arguments.includeSoftDeletes, parentSoftDeleted = true); + } + } else { + // Delete dependent record(s) before the record itself so foreign key constraints don't + // prevent the deletion. + $deleteDependents(softDelete = arguments.softDelete, includeSoftDeletes = arguments.includeSoftDeletes); + local.recordCount = $runDeleteStatement(sql = arguments.sql, parameterize = arguments.parameterize); + } + if (local.recordCount == 1 && $callback("afterDelete", arguments.callbacks)) { local.rv = true; // v4.2.0: queue afterCommit/afterRollback (fires at the outermost // transaction resolve, or immediately in none/false mode). - $enqueueTransactionCallbacks(operation = "delete", callbacks = arguments.callbacks); + if (StructKeyExists(arguments, "$softDeleteBefore")) { + // A rollback of the surrounding transaction puts deletedAt back too. + $enqueueTransactionCallbacks(operation = "delete", callbacks = arguments.callbacks, softDeleteBefore = arguments.$softDeleteBefore); + } else { + $enqueueTransactionCallbacks(operation = "delete", callbacks = arguments.callbacks); + } } } return local.rv; } + + /** + * Internal function. The soft-delete column's value as last loaded or saved, ignoring an unsaved + * in-memory edit; "" when the object doesn't hold the column. + */ + public string function $persistedSoftDeleteValue() { + local.column = variables.wheels.class.softDeleteColumn; + if (hasChanged(local.column)) { + return changedFrom(local.column); + } + if (StructKeyExists(this, local.column) && IsSimpleValue(this[local.column])) { + return this[local.column]; + } + return ""; + } + + /** + * Internal function. The object's soft-delete column, in memory and as last persisted, for + * `$restoreSoftDeleteState()`. + */ + public struct function $softDeleteState() { + local.column = variables.wheels.class.softDeleteColumn; + local.rv = { + hasValue = StructKeyExists(this, local.column), + hasPersisted = StructKeyExists(variables, "$persistedProperties") && StructKeyExists(variables.$persistedProperties, local.column) + }; + if (local.rv.hasValue) { + local.rv.value = this[local.column]; + } + if (local.rv.hasPersisted) { + local.rv.persisted = variables.$persistedProperties[local.column]; + } + return local.rv; + } + + /** + * Internal function. Puts the soft-delete column back as `$softDeleteState()` captured it. + */ + public void function $restoreSoftDeleteState(required struct state) { + local.column = variables.wheels.class.softDeleteColumn; + if (arguments.state.hasValue) { + this[local.column] = arguments.state.value; + } else { + StructDelete(this, local.column); + } + if (StructKeyExists(variables, "$persistedProperties")) { + if (arguments.state.hasPersisted) { + variables.$persistedProperties[local.column] = arguments.state.persisted; + } else { + StructDelete(variables.$persistedProperties, local.column); + } + } + } + + /** + * Internal function. Records a soft delete on the object: the column holds the timestamp written and + * counts as saved. Only this column's baseline changes; every other property keeps its own. + */ + public void function $markSoftDeleted(required any value) { + local.column = variables.wheels.class.softDeleteColumn; + this[local.column] = arguments.value; + if (!StructKeyExists(variables, "$persistedProperties")) { + variables.$persistedProperties = {}; + } + variables.$persistedProperties[local.column] = arguments.value; + } + + /** + * Internal function. Runs an object's DELETE (or soft-delete UPDATE) statement and returns how many + * rows it changed. + */ + public numeric function $runDeleteStatement(required array sql, any parameterize) { + local.deleted = variables.wheels.class.adapter.$querySetup(sql = arguments.sql, parameterize = arguments.parameterize); + $markMigrationDidWork(); + $clearRequestCache(); + return local.deleted.result.recordCount; + } diff --git a/vendor/wheels/model/read.cfm b/vendor/wheels/model/read.cfm index 038b4ad62b..6d9aa47a27 100644 --- a/vendor/wheels/model/read.cfm +++ b/vendor/wheels/model/read.cfm @@ -615,7 +615,21 @@ public void function reload() { // A reloaded object reports no saved changes, as after clearChangeInformation() (F49). StructDelete(variables, "$savedChanges"); - local.query = findByKey(key = key(), reload = true, returnAs = "query"); + // An object without a key has no row to read back: an unsaved object, or a save on a + // database that returns no generated key (Oracle). Leave it as it is. + if (!Len(key())) { + return; + } + // The row as it is now, soft-deleted or not; a row that is gone can't be reloaded. A finder + // can answer false instead of an empty query (BoxLang), which also means no row. + local.query = findByKey(key = key(), reload = true, returnAs = "query", includeSoftDeletes = true); + if (!IsQuery(local.query) || !local.query.recordCount) { + Throw( + type = "Wheels.RecordNotFound", + message = "Can't reload this #variables.wheels.class.modelName# object: no row exists for its key (#key()#).", + extendedInfo = "The row was deleted after the object was loaded." + ); + } local.properties = propertyNames(); local.columnInfo = variables.wheels.class.properties; local.iEnd = ListLen(local.properties); diff --git a/vendor/wheels/tests/_assets/models/AuthorDependentAllPosts.cfc b/vendor/wheels/tests/_assets/models/AuthorDependentAllPosts.cfc new file mode 100644 index 0000000000..00c5bc3917 --- /dev/null +++ b/vendor/wheels/tests/_assets/models/AuthorDependentAllPosts.cfc @@ -0,0 +1,8 @@ +component extends="Model" { + + function config() { + table("c_o_r_e_authors"); + hasMany(name = "posts", modelName = "post", foreignKey = "authorId", dependent = "deleteAll"); + } + +} diff --git a/vendor/wheels/tests/_assets/models/AuthorDependentPosts.cfc b/vendor/wheels/tests/_assets/models/AuthorDependentPosts.cfc new file mode 100644 index 0000000000..eebfb9e42f --- /dev/null +++ b/vendor/wheels/tests/_assets/models/AuthorDependentPosts.cfc @@ -0,0 +1,8 @@ +component extends="Model" { + + function config() { + table("c_o_r_e_authors"); + hasMany(name = "posts", modelName = "post", foreignKey = "authorId", dependent = "delete"); + } + +} diff --git a/vendor/wheels/tests/_assets/models/SoftDeletePostChildPosts.cfc b/vendor/wheels/tests/_assets/models/SoftDeletePostChildPosts.cfc new file mode 100644 index 0000000000..f942b5d7f9 --- /dev/null +++ b/vendor/wheels/tests/_assets/models/SoftDeletePostChildPosts.cfc @@ -0,0 +1,12 @@ +/** + * A post whose "child posts" are the posts with authorId = this post's id. The fixtures only have one + * soft-delete table, so this pairs it with itself to give a soft-delete parent soft-delete children. + */ +component extends="Model" { + + function config() { + table("c_o_r_e_posts"); + hasMany(name = "childPosts", modelName = "post", foreignKey = "authorId", dependent = "delete"); + } + +} diff --git a/vendor/wheels/tests/_assets/models/SoftDeletePostDependentComments.cfc b/vendor/wheels/tests/_assets/models/SoftDeletePostDependentComments.cfc new file mode 100644 index 0000000000..307f45ef24 --- /dev/null +++ b/vendor/wheels/tests/_assets/models/SoftDeletePostDependentComments.cfc @@ -0,0 +1,8 @@ +component extends="Model" { + + function config() { + table("c_o_r_e_posts"); + hasMany(name = "comments", modelName = "comment", foreignKey = "postId", dependent = "delete"); + } + +} diff --git a/vendor/wheels/tests/_assets/models/SoftDeletePostRemovedComments.cfc b/vendor/wheels/tests/_assets/models/SoftDeletePostRemovedComments.cfc new file mode 100644 index 0000000000..0198ed8199 --- /dev/null +++ b/vendor/wheels/tests/_assets/models/SoftDeletePostRemovedComments.cfc @@ -0,0 +1,8 @@ +component extends="Model" { + + function config() { + table("c_o_r_e_posts"); + hasMany(name = "comments", modelName = "comment", foreignKey = "postId", dependent = "removeAll"); + } + +} diff --git a/vendor/wheels/tests/_assets/models/SoftDeletePostVetoedDelete.cfc b/vendor/wheels/tests/_assets/models/SoftDeletePostVetoedDelete.cfc new file mode 100644 index 0000000000..57f4ddd2f3 --- /dev/null +++ b/vendor/wheels/tests/_assets/models/SoftDeletePostVetoedDelete.cfc @@ -0,0 +1,12 @@ +component extends="Model" { + + function config() { + table("c_o_r_e_posts"); + afterDelete("vetoDelete"); + } + + function vetoDelete() { + return false; + } + +} diff --git a/vendor/wheels/tests/specs/model/SoftDeleteSemanticsSpec.cfc b/vendor/wheels/tests/specs/model/SoftDeleteSemanticsSpec.cfc new file mode 100644 index 0000000000..c1a1d84217 --- /dev/null +++ b/vendor/wheels/tests/specs/model/SoftDeleteSemanticsSpec.cfc @@ -0,0 +1,308 @@ +/** + * Soft delete is reversible and a permanent delete is a real DELETE. + * + * - Soft-deleting a parent never destroys or unlinks its dependents: children that can be + * soft-deleted are, everything else is left alone (4370). + * - `softDelete=false` reaches rows that are already soft-deleted, including dependents (4371). + * - An instance tracks its own soft delete: reload() finds a soft-deleted row, a second soft + * delete is refused, and the object holds the deletedAt it wrote (4372). + */ +component extends="wheels.WheelsTest" { + + function run() { + + g = application.wo; + + describe("Soft-deleting a parent", () => { + + it("leaves dependent=delete children that can't be soft-deleted in place", () => { + var state = {}; + transaction { + var postId = g.model("comment").findOne(order = "id").postId; + state.before = g.model("comment").count(where = "postId = #postId#"); + var post = g.model("SoftDeletePostDependentComments").findByKey(postId); + state.deleted = post.delete(); + state.after = g.model("comment").count(where = "postId = #postId#"); + state.postStillThere = g.model("post").count(where = "id = #postId#", includeSoftDeletes = true); + transaction action="rollback"; + } + expect(state.before).toBeGT(0); + expect(state.deleted).toBeTrue(); + expect(state.after).toBe(state.before); + expect(state.postStillThere).toBe(1); + }); + + it("leaves dependent=removeAll children linked to it", () => { + var state = {}; + transaction { + var postId = g.model("comment").findOne(order = "id").postId; + state.before = g.model("comment").count(where = "postId = #postId#"); + var post = g.model("SoftDeletePostRemovedComments").findByKey(postId); + state.deleted = post.delete(); + state.after = g.model("comment").count(where = "postId = #postId#"); + transaction action="rollback"; + } + expect(state.deleted).toBeTrue(); + expect(state.after).toBe(state.before); + }); + + it("soft-deletes dependents that can be soft-deleted", () => { + var state = {}; + transaction { + var parentId = $parentPostWithChildren(); + state.children = g.model("post").count(where = "authorId = #parentId# AND id <> #parentId#"); + var parent = g.model("SoftDeletePostChildPosts").findByKey(parentId); + state.deleted = parent.delete(); + state.liveChildren = g.model("post").count(where = "authorId = #parentId# AND id <> #parentId#"); + state.allChildren = g.model("post").count(where = "authorId = #parentId# AND id <> #parentId#", includeSoftDeletes = true); + transaction action="rollback"; + } + expect(state.children).toBeGT(0); + expect(state.deleted).toBeTrue(); + expect(state.liveChildren).toBe(0); + expect(state.allChildren).toBe(state.children); + }); + + it("leaves dependents alone when a stale copy's soft delete changes nothing, inside a caller's transaction", () => { + var state = {}; + transaction { + var parentId = $parentPostWithChildren(); + state.children = g.model("post").count(where = "authorId = #parentId# AND id <> #parentId#"); + var stale = g.model("SoftDeletePostChildPosts").findByKey(parentId); + // Soft-delete the parent row alone, behind the stale copy's back. + g.model("post").updateAll(where = "id = #parentId#", deletedAt = Now(), includeSoftDeletes = true); + state.deleted = stale.delete(transaction = "none"); + state.liveChildren = g.model("post").count(where = "authorId = #parentId# AND id <> #parentId#"); + transaction action="rollback"; + } + expect(state.deleted).toBeFalse(); + expect(state.children).toBe(2); + expect(state.liveChildren).toBe(state.children); + }); + + it("doesn't re-stamp soft-deleted children when a parent without deletedAt is deleted", () => { + var state = {}; + transaction { + var authorId = g.model("post").findOne(where = "authorId IS NOT NULL", order = "id").authorId; + var earlier = g.model("post").findOne(where = "authorId = #authorId#", order = "id"); + var stamp = CreateDateTime(2020, 1, 2, 3, 4, 5); + g.model("post").updateAll(where = "id = #earlier.id#", deletedAt = stamp, includeSoftDeletes = true); + var author = g.model("AuthorDependentAllPosts").findByKey(authorId); + state.deleted = author.delete(); + state.earlierDeletedAt = g.model("post").findByKey(key = earlier.id, includeSoftDeletes = true).deletedAt; + transaction action="rollback"; + } + expect(state.deleted).toBeTrue(); + expect(Year(state.earlierDeletedAt)).toBe(2020, "deletedAt of the earlier soft delete: [" & state.earlierDeletedAt & "]"); + }); + + it("still runs dependent= on a permanent delete", () => { + var state = {}; + transaction { + var postId = g.model("comment").findOne(order = "id").postId; + var post = g.model("SoftDeletePostDependentComments").findByKey(postId); + state.deleted = post.delete(softDelete = false); + state.after = g.model("comment").count(where = "postId = #postId#"); + state.postLeft = g.model("post").count(where = "id = #postId#", includeSoftDeletes = true); + transaction action="rollback"; + } + expect(state.deleted).toBeTrue(); + expect(state.after).toBe(0); + expect(state.postLeft).toBe(0); + }); + + }); + + describe("A permanent delete", () => { + + it("deleteAll(softDelete=false) removes rows that are already soft-deleted", () => { + var state = {}; + transaction { + var post = g.model("post").findOne(order = "id"); + post.delete(); + state.removed = g.model("post").deleteAll(where = "id = #post.id#", softDelete = false); + state.left = g.model("post").count(where = "id = #post.id#", includeSoftDeletes = true); + transaction action="rollback"; + } + expect(state.removed).toBe(1); + expect(state.left).toBe(0); + }); + + it("deleteByKey(softDelete=false) removes a row that is already soft-deleted", () => { + var state = {}; + transaction { + var post = g.model("post").findOne(order = "id"); + post.delete(); + state.removed = g.model("post").deleteByKey(key = post.id, softDelete = false); + state.left = g.model("post").count(where = "id = #post.id#", includeSoftDeletes = true); + transaction action="rollback"; + } + expect(state.removed).toBeTrue(); + expect(state.left).toBe(0); + }); + + it("reaches dependent children that are already soft-deleted", () => { + var state = {}; + transaction { + var authorId = g.model("post").findOne(where = "authorId IS NOT NULL", order = "id").authorId; + state.posts = g.model("post").count(where = "authorId = #authorId#"); + g.model("post").findOne(where = "authorId = #authorId#", order = "id").delete(); + var author = g.model("AuthorDependentPosts").findByKey(authorId); + state.deleted = author.delete(softDelete = false); + state.left = g.model("post").count(where = "authorId = #authorId#", includeSoftDeletes = true); + transaction action="rollback"; + } + expect(state.posts).toBeGT(0); + expect(state.deleted).toBeTrue(); + expect(state.left).toBe(0); + }); + + }); + + describe("An instance on a soft-deleted row", () => { + + it("holds the deletedAt its soft delete wrote", () => { + var state = {}; + transaction { + var post = g.model("post").findOne(order = "id"); + post.delete(); + state.inMemory = post.deletedAt; + state.changed = post.hasChanged("deletedAt"); + state.stored = g.model("post").findByKey(key = post.id, includeSoftDeletes = true).deletedAt; + transaction action="rollback"; + } + expect(IsDate(state.inMemory)).toBeTrue("deletedAt on the object: [" & state.inMemory & "]"); + // A DATETIME column without fractional seconds can round the stored value to the next second. + expect(Abs(DateDiff("s", state.inMemory, state.stored))).toBeLTE(1); + expect(state.changed).toBeFalse(); + }); + + it("refuses a second soft delete and keeps the first deletedAt", () => { + var state = {}; + transaction { + var post = g.model("post").findOne(order = "id"); + post.delete(); + state.first = g.model("post").findByKey(key = post.id, includeSoftDeletes = true).deletedAt; + state.again = post.delete(); + state.second = g.model("post").findByKey(key = post.id, includeSoftDeletes = true).deletedAt; + transaction action="rollback"; + } + expect(state.again).toBeFalse(); + expect(DateCompare(state.first, state.second, "s")).toBe(0); + }); + + it("deletes an object whose deletedAt was only set in memory", () => { + var state = {}; + transaction { + var post = g.model("post").findOne(order = "id"); + post.deletedAt = Now(); + state.deleted = post.delete(); + state.live = g.model("post").count(where = "id = #post.id#"); + transaction action="rollback"; + } + expect(state.deleted).toBeTrue(); + expect(state.live).toBe(0); + }); + + it("lets a soft delete that was rolled back be retried", () => { + var post = g.model("post").findOne(order = "id"); + var state = {}; + state.first = post.delete(transaction = "rollback"); + state.afterRollback = post.deletedAt; + state.changed = post.hasChanged("deletedAt"); + state.retry = post.delete(transaction = "rollback"); + state.live = g.model("post").count(where = "id = #post.id#"); + expect(state.first).toBeTrue(); + expect(state.afterRollback).toBe(""); + expect(state.changed).toBeFalse(); + expect(state.retry).toBeTrue(); + expect(state.live).toBe(1, "both deletes were rolled back"); + }); + + it("puts deletedAt back when afterDelete vetoes the soft delete", () => { + var post = g.model("SoftDeletePostVetoedDelete").findOne(order = "id"); + var state = {}; + state.deleted = post.delete(transaction = "commit"); + state.inMemory = post.deletedAt; + state.live = g.model("post").count(where = "id = #post.id#"); + expect(state.deleted).toBeFalse(); + expect(state.inMemory).toBe(""); + expect(state.live).toBe(1, "the vetoed delete was rolled back"); + }); + + it("refuses a soft delete from a stale copy of a row soft-deleted elsewhere", () => { + var state = {}; + transaction { + var first = g.model("post").findOne(order = "id"); + var stale = g.model("post").findByKey(first.id); + first.delete(); + state.again = stale.delete(); + transaction action="rollback"; + } + expect(state.again).toBeFalse(); + }); + + it("reloads the soft-deleted row instead of blanking the object", () => { + var state = {}; + transaction { + var post = g.model("post").findOne(order = "id"); + state.title = post.title; + post.delete(); + post.reload(); + state.reloadedTitle = post.title; + state.reloadedDeletedAt = post.deletedAt; + transaction action="rollback"; + } + expect(state.reloadedTitle).toBe(state.title); + expect(IsDate(state.reloadedDeletedAt)).toBeTrue(); + }); + + it("leaves an object without a key alone on reload()", () => { + // An unsaved object, or a save on a database that returns no generated key (Oracle). + var post = g.model("post").new(title = "no key yet", body = "b"); + var state = {type = ""}; + try { + post.reload(); + } catch (any e) { + state.type = e.type; + } + expect(state.type).toBe(""); + expect(post.title).toBe("no key yet"); + }); + + it("throws from reload() when the row no longer exists", () => { + var state = {type = ""}; + transaction { + var post = g.model("post").findOne(order = "id"); + post.delete(softDelete = false); + try { + post.reload(); + } catch (any e) { + state.type = e.type; + } + transaction action="rollback"; + } + expect(state.type).toBe("Wheels.RecordNotFound"); + }); + + }); + + } + + + /** + * A new post with two "child" posts whose authorId is its id, for SoftDeletePostChildPosts (whose + * childPosts association keys on authorId). Created rather than found, so the pairing doesn't rely + * on post and author ids overlapping, which they don't on CockroachDB. Call inside a transaction + * that rolls back. Returns the parent's id. + */ + private any function $parentPostWithChildren() { + var tag = Replace(CreateUUID(), "-", "", "all"); + var parent = g.model("post").create(authorId = 1, title = "sd-parent-#tag#", body = "parent", views = 0, transaction = "none"); + for (var i = 1; i <= 2; i++) { + g.model("post").create(authorId = parent.id, title = "sd-child-#i#-#tag#", body = "child", views = 0, transaction = "none"); + } + return parent.id; + } + +} diff --git a/web/sites/guides/src/content/docs/v4-2-0/basics/associations.mdx b/web/sites/guides/src/content/docs/v4-2-0/basics/associations.mdx index bea54ee0ee..4317b31580 100644 --- a/web/sites/guides/src/content/docs/v4-2-0/basics/associations.mdx +++ b/web/sites/guides/src/content/docs/v4-2-0/basics/associations.mdx @@ -90,6 +90,7 @@ The association methods read like English because they use the association name, - **`dependent="deleteAll"`** — issue a single `DELETE FROM children WHERE parentId = ?` statement. Fast, but skips the child's callbacks. - **`dependent="remove"`** — instantiate each child and set its foreign key to `NULL`. Child's `beforeUpdate` / `afterUpdate` fire. Use when children can survive without a parent. - **`dependent="removeAll"`** — single `UPDATE children SET parentId = NULL WHERE parentId = ?`. Fast, skips callbacks. +- **On a soft delete** — when the parent has a `deletedAt` column and is soft-deleted, children whose model also has one are soft-deleted, and every other child is left alone (no delete, no `NULL` foreign key), so restoring the parent finds them intact. `dependent=` applies to them only when the parent is really deleted (`delete(softDelete=false)`, or a parent without a soft-delete column). - **Default (not set)** — no cascade. Deleting the parent leaves orphaned child rows whose foreign key points at a row that no longer exists. You almost always want a `dependent` value, even on small associations. Choose `delete` (or `deleteAll` if the children don't have callbacks) for strict ownership — comments, line items, anything that can't outlive its parent. Choose `remove` or `removeAll` for looser links — employees who survive their department, posts that survive their author, and so on. diff --git a/web/sites/guides/src/content/docs/v4-2-0/basics/models-and-the-orm.mdx b/web/sites/guides/src/content/docs/v4-2-0/basics/models-and-the-orm.mdx index 352ea25907..cbff71c688 100644 --- a/web/sites/guides/src/content/docs/v4-2-0/basics/models-and-the-orm.mdx +++ b/web/sites/guides/src/content/docs/v4-2-0/basics/models-and-the-orm.mdx @@ -198,6 +198,21 @@ component extends="Controller" { For a key-only delete without loading the record, use `model("Post").deleteByKey(params.key)`. For bulk deletes, `deleteAll(where="...")` runs a single statement and returns the row count; like `updateAll`, it skips callbacks by default. +### Soft delete + +A table with a `deletedAt` column (`t.timestamps()` adds one) is soft-deleted: `delete()` sets `deletedAt` instead of removing the row, and finders skip such rows unless you pass `includeSoftDeletes=true`. A soft delete is meant to be reversible, so: + +- After `post.delete()`, `post.deletedAt` holds the timestamp written. A second `delete()` on the same object returns `false`. +- `dependent=` children whose model also has a `deletedAt` column are soft-deleted with the parent. Other children are left alone, neither deleted nor unlinked: `dependent=` applies to them only on a real delete. +- `post.reload()` reloads a soft-deleted row. It throws `Wheels.RecordNotFound` if the row no longer exists at all. +- Restoring is an update: `model("Post").updateByKey(key = id, deletedAt = "", includeSoftDeletes = true)`. It doesn't restore the children that were soft-deleted with the parent. + +Pass `softDelete=false` for a real `DELETE`. It removes rows that are already soft-deleted too, and runs `dependent=` on every child: + +```cfm +model("Post").deleteAll(where = "deletedAt < '#cutoff#'", softDelete = false); // purge old soft-deleted posts +``` + ## Lifecycle callbacks Callbacks register a method to run at a specific point in the record's lifecycle. For the write-cycle callbacks the method takes no arguments (the record is `this`), returns nothing, and can mutate any property on `this` before the write happens. Return `false` from a `before*` callback to cancel the operation. (`afterFind` is the exception — see below.) diff --git a/web/sites/guides/src/content/docs/v4-2-0/upgrading/4x-1-to-4x-2.mdx b/web/sites/guides/src/content/docs/v4-2-0/upgrading/4x-1-to-4x-2.mdx index 4ef1387a94..4969b7a58e 100644 --- a/web/sites/guides/src/content/docs/v4-2-0/upgrading/4x-1-to-4x-2.mdx +++ b/web/sites/guides/src/content/docs/v4-2-0/upgrading/4x-1-to-4x-2.mdx @@ -412,6 +412,22 @@ If your app builds storage keys from user input or file names, check that none o `insertAll()` and `upsertAll()` now honour their `transaction` argument, which defaults to your `transactionMode` setting (`commit` unless you changed it). Records are still written in batches of up to 1000 rows (fewer on SQL Server, to stay within its parameter limit), but all the batches now run in one transaction: a large import is all-or-nothing, and a failing batch rolls back the ones before it. In 4.1 each batch committed on its own. If you relied on that, for example to keep a partial import, pass `transaction="none"`. `transaction="rollback"` writes nothing, like `save(transaction="rollback")`. One transaction holds its locks until the end, so an import of many thousands of rows holds them for the whole call. +### Soft-deleting a parent leaves non-soft-delete dependents alone + +On a model with a soft-delete column (`deletedAt`), a soft `delete()` no longer runs `dependent=` actions that a restore couldn't undo ([#4370](https://github.com/wheels-dev/wheels/issues/4370)). Children whose model also has a soft-delete column are soft-deleted, as before. Children without one are left alone: `dependent="delete"` / `"deleteAll"` no longer hard-deletes them, and `dependent="remove"` / `"removeAll"` no longer sets their foreign keys to `NULL`. On a soft delete, `dependent=` therefore applies only to a real delete. If an app counted on a soft delete removing such children, delete them in a `beforeDelete` / `afterDelete` callback, or delete the parent with `softDelete=false`. + +### `softDelete=false` also deletes rows that are already soft-deleted + +`delete(softDelete=false)`, `deleteAll(softDelete=false)`, `deleteOne(softDelete=false)` and `deleteByKey(softDelete=false)` are now always real deletes, including rows that were soft-deleted earlier and the `dependent=` children of a deleted record ([#4371](https://github.com/wheels-dev/wheels/issues/4371)). In 4.1 they skipped soft-deleted rows unless `includeSoftDeletes=true` was passed too. An explicit `includeSoftDeletes=false` alongside `softDelete=false` no longer keeps soft-deleted rows. With `instantiate=true`, `deleteAll(softDelete=false)` now loads those rows and runs their `beforeDelete`, `afterDelete` and `afterCommit` callbacks too. + +### `reload()` and `delete()` on a soft-deleted object + +Three changes for an object whose row is soft-deleted ([#4372](https://github.com/wheels-dev/wheels/issues/4372)): + +- `reload()` reloads the soft-deleted row. In 4.1 it blanked every property. When the row is gone altogether, `reload()` now throws `Wheels.RecordNotFound` instead of blanking the object; uncaught in an action, that error renders the 404 page. +- A soft `delete()` on an object that is already soft-deleted, or on a stale copy of a row soft-deleted elsewhere, returns `false` and changes nothing. In 4.1 it wrote a new `deletedAt` and returned `true`. +- After a soft `delete()`, the object's `deletedAt` holds the timestamp written. If the delete is rolled back, it goes back to what it was. + ## CLI and deployment changes ### Dev servers listen on `127.0.0.1`