fix(model): dependent deletes follow callbacks = false; check dependent when declared

changelog.d/dependent-callbacks.fixed.md

@@ -0,0 +1 @@
+- `delete(callbacks=false)` now also skips the callbacks of the dependent records it deletes or removes through `hasMany()` / `hasOne()` `dependent=`, including their `afterFind` / `afterInitialization` and transaction callbacks; a `hasOne` with `dependent="deleteAll"` or `"removeAll"` (which loads the record) is covered too. `deleteAll(instantiate=true)`, `deleteOne()` and `updateOne()` with `callbacks=false` no longer run `afterFind` / `afterInitialization` on the records they load, like `updateAll(instantiate=true)`. An invalid `dependent` value on `hasMany()` or `hasOne()` now throws `Wheels.InvalidArgument` when the association is declared, instead of at the first delete

vendor/wheels/model/associations.cfm

@@ -71,6 +71,9 @@
 		string as = ""
 	) {
 		$args(name = "hasMany", args = arguments);
+		if (StructKeyExists(arguments, "dependent")) {
+			$validateDependent(arguments.dependent);
+		}
 		local.singularizedName = capitalize(singularize(arguments.name));
 		local.capitalizedName = capitalize(arguments.name);
 		arguments.type = "hasMany";
@@ -125,6 +128,9 @@
 		string as = ""
 	) {
 		$args(name = "hasOne", args = arguments);
+		if (StructKeyExists(arguments, "dependent")) {
+			$validateDependent(arguments.dependent);
+		}
 		local.capitalizedName = capitalize(arguments.name);
 		arguments.type = "hasOne";
 
@@ -240,13 +246,37 @@
 		variables.wheels.class.associations[local.associationName] = arguments;
 	}
 
+	/**
+	 * Internal function. Throws Wheels.InvalidArgument for a `dependent` value hasMany() and hasOne()
+	 * don't support, so the mistake surfaces when the association is declared, not at the first delete.
+	 */
+	public void function $validateDependent(required any dependent) {
+		if (IsBoolean(arguments.dependent) && !arguments.dependent) {
+			return;
+		}
+		if (IsSimpleValue(arguments.dependent) && ListFindNoCase("delete,deleteAll,remove,removeAll", arguments.dependent)) {
+			return;
+		}
+		Throw(
+			type = "Wheels.InvalidArgument",
+			message = "'#IsSimpleValue(arguments.dependent) ? arguments.dependent : "(complex value)"#' is not a valid dependency.",
+			extendedInfo = "Use `delete`, `deleteAll`, `remove`, `removeAll` or `false`."
+		);
+	}
+
 	/*
 	 * Called when a model object is deleted (e.g. post.delete()).
 	 * 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.
+	 * dependents that can be soft-deleted are, and every other dependent is left alone. With
+	 * `callbacks = false` the instantiated dependents skip their callbacks too.
 	 */
-	public void function $deleteDependents(boolean softDelete = true, boolean includeSoftDeletes = false, boolean parentSoftDeleted = false) {
+	public void function $deleteDependents(
+		boolean softDelete = true,
+		boolean includeSoftDeletes = false,
+		boolean callbacks = true,
+		boolean parentSoftDeleted = false
+	) {
 		for (local.key in variables.wheels.class.associations) {
 			local.association = variables.wheels.class.associations[local.key];
 			if (
@@ -262,23 +292,29 @@
 					case "delete":
 						local.invokeArgs = {};
 						local.invokeArgs.instantiate = true;
+						local.invokeArgs.callbacks = arguments.callbacks;
 						local.invokeArgs.softDelete = arguments.softDelete;
 						local.invokeArgs.includeSoftDeletes = arguments.includeSoftDeletes;
 						$invoke(componentReference = this, method = "delete#local.all##local.key#", invokeArgs = local.invokeArgs);
 						break;
 					case "remove":
 						local.invokeArgs = {};
 						local.invokeArgs.instantiate = true;
+						local.invokeArgs.callbacks = arguments.callbacks;
 						$invoke(componentReference = this, method = "remove#local.all##local.key#", invokeArgs = local.invokeArgs);
 						break;
 					case "deleteAll":
+						// a hasOne's deleteAll still instantiates (deleteOne), so it needs callbacks too
 						local.invokeArgs = {};
+						local.invokeArgs.callbacks = arguments.callbacks;
 						local.invokeArgs.softDelete = arguments.softDelete;
 						local.invokeArgs.includeSoftDeletes = arguments.includeSoftDeletes;
 						$invoke(componentReference = this, method = "delete#local.all##local.key#", invokeArgs = local.invokeArgs);
 						break;
 					case "removeAll":
-						$invoke(componentReference = this, method = "remove#local.all##local.key#");
+						local.invokeArgs = {};
+						local.invokeArgs.callbacks = arguments.callbacks;
+						$invoke(componentReference = this, method = "remove#local.all##local.key#", invokeArgs = local.invokeArgs);
 						break;
 					default:
 						Throw(

vendor/wheels/model/delete.cfm

@@ -41,6 +41,7 @@
 		if (arguments.instantiate) {
 			local.rv = 0;
 			local.objects = findAll(
+				callbacks = arguments.callbacks,
 				include = arguments.include,
 				includeSoftDeletes = arguments.includeSoftDeletes,
 				parameterize = arguments.parameterize,
@@ -157,6 +158,7 @@
 			arguments.includeSoftDeletes = true;
 		}
 		local.object = findOne(
+			callbacks = arguments.callbacks,
 			includeSoftDeletes = arguments.includeSoftDeletes,
 			order = arguments.order,
 			reload = arguments.reload,
@@ -264,12 +266,21 @@
 				local.recordCount = $runDeleteStatement(sql = arguments.sql, parameterize = arguments.parameterize);
 				if (local.recordCount == 1) {
 					$markSoftDeleted(arguments.$softDeletedAt);
-					$deleteDependents(softDelete = true, includeSoftDeletes = arguments.includeSoftDeletes, parentSoftDeleted = true);
+					$deleteDependents(
+						softDelete = true,
+						includeSoftDeletes = arguments.includeSoftDeletes,
+						callbacks = arguments.callbacks,
+						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);
+				$deleteDependents(
+					softDelete = arguments.softDelete,
+					includeSoftDeletes = arguments.includeSoftDeletes,
+					callbacks = arguments.callbacks
+				);
 				local.recordCount = $runDeleteStatement(sql = arguments.sql, parameterize = arguments.parameterize);
 			}
 			if (local.recordCount == 1 && $callback("afterDelete", arguments.callbacks)) {

vendor/wheels/model/update.cfm

@@ -204,6 +204,7 @@
 	) {
 		$args(name = "updateOne", args = arguments);
 		local.object = findOne(
+			callbacks = arguments.callbacks,
 			includeSoftDeletes = arguments.includeSoftDeletes,
 			order = arguments.order,
 			reload = arguments.reload,

vendor/wheels/tests/_assets/models/DepAuthorDelete.cfc

@@ -0,0 +1,8 @@
+component extends="Model" {
+
+	function config() {
+		table("c_o_r_e_authors");
+		hasMany(name = "depPosts", modelName = "depPost", foreignKey = "authorId", dependent = "delete");
+	}
+
+}

vendor/wheels/tests/_assets/models/DepAuthorInvalid.cfc

@@ -0,0 +1,8 @@
+component extends="Model" {
+
+	function config() {
+		table("c_o_r_e_authors");
+		hasMany(name = "depPosts", modelName = "depPost", foreignKey = "authorId", dependent = "destroy");
+	}
+
+}

vendor/wheels/tests/_assets/models/DepAuthorOneDelete.cfc

@@ -0,0 +1,8 @@
+component extends="Model" {
+
+	function config() {
+		table("c_o_r_e_authors");
+		hasOne(name = "depPost", modelName = "depPost", foreignKey = "authorId", dependent = "delete");
+	}
+
+}

vendor/wheels/tests/_assets/models/DepAuthorOneDeleteAll.cfc

@@ -0,0 +1,8 @@
+component extends="Model" {
+
+	function config() {
+		table("c_o_r_e_authors");
+		hasOne(name = "depPost", modelName = "depPost", foreignKey = "authorId", dependent = "deleteAll");
+	}
+
+}

vendor/wheels/tests/_assets/models/DepAuthorOneRemove.cfc

@@ -0,0 +1,8 @@
+component extends="Model" {
+
+	function config() {
+		table("c_o_r_e_authors");
+		hasOne(name = "depPost", modelName = "depPost", foreignKey = "authorId", dependent = "remove");
+	}
+
+}

vendor/wheels/tests/_assets/models/DepAuthorOneRemoveAll.cfc

@@ -0,0 +1,8 @@
+component extends="Model" {
+
+	function config() {
+		table("c_o_r_e_authors");
+		hasOne(name = "depPost", modelName = "depPost", foreignKey = "authorId", dependent = "removeAll");
+	}
+
+}

vendor/wheels/tests/_assets/models/DepAuthorRemove.cfc

@@ -0,0 +1,8 @@
+component extends="Model" {
+
+	function config() {
+		table("c_o_r_e_authors");
+		hasMany(name = "depPosts", modelName = "depPost", foreignKey = "authorId", dependent = "remove");
+	}
+
+}

vendor/wheels/tests/_assets/models/DepPost.cfc

@@ -0,0 +1,41 @@
+/**
+ * A post whose delete and save callbacks log to request.$depLog, for dependentCallbacksSpec.
+ */
+component extends="Model" {
+
+	function config() {
+		table("c_o_r_e_posts");
+		beforeDelete("logDelete");
+		beforeSave("logSave");
+		afterFind("logFind");
+		afterRollback("logRollback");
+	}
+
+	function logFind() {
+		$logDependentCallback("find");
+		return true;
+	}
+
+	function logRollback() {
+		$logDependentCallback("rollback");
+		return true;
+	}
+
+	function logDelete() {
+		$logDependentCallback("delete");
+		return true;
+	}
+
+	function logSave() {
+		$logDependentCallback("save");
+		return true;
+	}
+
+	function $logDependentCallback(required string event) {
+		if (!StructKeyExists(request, "$depLog")) {
+			request.$depLog = [];
+		}
+		ArrayAppend(request.$depLog, arguments.event);
+	}
+
+}

vendor/wheels/tests/specs/model/dependentCallbacksSpec.cfc

@@ -0,0 +1,106 @@
+/**
+ * delete(callbacks = false) covers the dependent deletes and removes it triggers (the children's
+ * find, write and transaction callbacks, for hasMany and hasOne), and an invalid
+ * `dependent` value fails when the association is declared rather than at the first delete. Each
+ * delete runs with transaction = "rollback", so the seeded rows are kept.
+ */
+component extends="wheels.WheelsTest" {
+
+	function beforeAll() {
+		variables.g = application.wo;
+		variables.authorId = variables.g.model("post").findOne(order = "id").authorId;
+	}
+
+	function failureOf(required any callback) {
+		var state = {type = "", message = ""};
+		var target = arguments.callback;
+		try {
+			target();
+		} catch (any e) {
+			state.type = e.type;
+			state.message = e.message;
+		}
+		return state;
+	}
+
+	function run() {
+
+		describe("Dependent records and callbacks = false", () => {
+
+			beforeEach(() => {
+				request.$depLog = [];
+			});
+
+			it("deletes dependents without their callbacks when callbacks = false", () => {
+				var author = variables.g.model("depAuthorDelete").findByKey(variables.authorId);
+				request.$depLog = [];
+				author.delete(callbacks = false, transaction = "rollback");
+				expect(ArrayLen(request.$depLog)).toBe(0);
+			});
+
+			it("removes dependents without their callbacks when callbacks = false", () => {
+				var author = variables.g.model("depAuthorRemove").findByKey(variables.authorId);
+				request.$depLog = [];
+				author.delete(callbacks = false, transaction = "rollback");
+				expect(ArrayLen(request.$depLog)).toBe(0);
+			});
+
+			it("skips a hasOne dependent's callbacks with callbacks = false, for every dependent value", () => {
+				var parents = ["depAuthorOneDelete", "depAuthorOneDeleteAll", "depAuthorOneRemove", "depAuthorOneRemoveAll"];
+				var logged = {};
+				for (var parent in parents) {
+					var author = variables.g.model(parent).findByKey(variables.authorId);
+					request.$depLog = [];
+					author.delete(callbacks = false, transaction = "rollback");
+					logged[parent] = ArrayToList(request.$depLog);
+				}
+				for (var parent in parents) {
+					expect(logged[parent]).toBe("", parent);
+				}
+			});
+
+			it("still runs the dependents' callbacks with callbacks = true", () => {
+				var deleting = variables.g.model("depAuthorDelete").findByKey(variables.authorId);
+				request.$depLog = [];
+				deleting.delete(transaction = "rollback");
+				var deleted = ArrayToList(request.$depLog);
+				expect(deleted).toInclude("find");
+				expect(deleted).toInclude("delete");
+				expect(deleted).toInclude("rollback");
+				var removing = variables.g.model("depAuthorRemove").findByKey(variables.authorId);
+				request.$depLog = [];
+				removing.delete(transaction = "rollback");
+				expect(ArrayToList(request.$depLog)).toInclude("save");
+				var one = variables.g.model("depAuthorOneDeleteAll").findByKey(variables.authorId);
+				request.$depLog = [];
+				one.delete(transaction = "rollback");
+				var oneLogged = ArrayToList(request.$depLog);
+				expect(oneLogged).toInclude("find");
+				expect(oneLogged).toInclude("delete");
+				expect(oneLogged).toInclude("rollback");
+			});
+
+		});
+
+		describe("An invalid dependent value", () => {
+
+			it("fails when the association is declared", () => {
+				var failure = failureOf(() => variables.g.model("depAuthorInvalid"));
+				expect(failure.type).toBe("Wheels.InvalidArgument");
+				expect(failure.message).toInclude("destroy");
+			});
+
+			it("accepts every documented value", () => {
+				var post = variables.g.model("post");
+				expect(failureOf(() => post.$validateDependent("delete")).type).toBe("");
+				expect(failureOf(() => post.$validateDependent("deleteAll")).type).toBe("");
+				expect(failureOf(() => post.$validateDependent("remove")).type).toBe("");
+				expect(failureOf(() => post.$validateDependent("removeAll")).type).toBe("");
+				expect(failureOf(() => post.$validateDependent(false)).type).toBe("");
+			});
+
+		});
+
+	}
+
+}