fix(model): take MySQL and PostgreSQL advisory locks with the shared connection options

changelog.d/4223-advisory-lock-tenant-datasource.changed.md

@@ -0,0 +1 @@
+- On MySQL and PostgreSQL, `withAdvisoryLock()` on a tenant (non-shared) model takes its lock on the active tenant's datasource instead of the application's default one, like the model's queries, SQL Server and `withAdvisoryLock(transaction = true)`. Two tenants on different databases using the same lock name no longer exclude each other, and no longer wait for each other inside one application (on MySQL, whose lock names are server-wide, tenant databases on the same MySQL server still share them); for one lock across all tenants, call `withAdvisoryLock()` through a shared model. The lock queries also build their connection options like other model queries, leaving out an empty username and password (#4223)

vendor/wheels/databaseAdapters/MySQL/MySQLModel.cfc

@@ -145,7 +145,7 @@ component extends="wheels.databaseAdapters.Base" output=false {
 		local.result = queryExecute(
 			"SELECT GET_LOCK(?, ?) AS lockResult, CONNECTION_ID() AS sessionId",
 			[arguments.name, arguments.timeout],
-			{datasource: variables.dataSource, username: variables.username, password: variables.password}
+			$advisoryLockConnection()
 		);
 		if (!IsQuery(local.result) || local.result.lockResult != 1) {
 			Throw(
@@ -164,7 +164,7 @@ component extends="wheels.databaseAdapters.Base" output=false {
 		queryExecute(
 			"SELECT RELEASE_LOCK(?)",
 			[arguments.name],
-			{datasource: variables.dataSource, username: variables.username, password: variables.password}
+			$advisoryLockConnection()
 		);
 	}
 
@@ -175,7 +175,7 @@ component extends="wheels.databaseAdapters.Base" output=false {
 		local.result = queryExecute(
 			"SELECT RELEASE_LOCK(?) AS released",
 			[arguments.name],
-			{datasource: variables.dataSource, username: variables.username, password: variables.password}
+			$advisoryLockConnection()
 		);
 		return IsQuery(local.result) && IsNumeric(local.result.released) && local.result.released == 1;
 	}
@@ -188,7 +188,7 @@ component extends="wheels.databaseAdapters.Base" output=false {
 		local.result = queryExecute(
 			"SELECT IS_USED_LOCK(?) AS holder",
 			[arguments.name],
-			{datasource: variables.dataSource, username: variables.username, password: variables.password}
+			$advisoryLockConnection()
 		);
 		if (!IsQuery(local.result) || !IsNumeric(local.result.holder)) {
 			return false;

vendor/wheels/databaseAdapters/PostgreSQL/PostgreSQLModel.cfc

@@ -195,7 +195,7 @@ component extends="wheels.databaseAdapters.Base" output=false {
 			local.result = queryExecute(
 				"SELECT pg_try_advisory_lock(hashtext(?)) AS lockresult, pg_backend_pid() AS sessionid",
 				[arguments.name],
-				{datasource: variables.dataSource, username: variables.username, password: variables.password}
+				$advisoryLockConnection()
 			);
 			if (IsQuery(local.result) && IsBoolean(local.result.lockresult) && local.result.lockresult) {
 				return local.result.sessionid;
@@ -218,7 +218,7 @@ component extends="wheels.databaseAdapters.Base" output=false {
 		queryExecute(
 			"SELECT pg_advisory_unlock(hashtext(?))",
 			[arguments.name],
-			{datasource: variables.dataSource, username: variables.username, password: variables.password}
+			$advisoryLockConnection()
 		);
 	}
 
@@ -229,7 +229,7 @@ component extends="wheels.databaseAdapters.Base" output=false {
 		local.result = queryExecute(
 			"SELECT pg_advisory_unlock(hashtext(?)) AS released",
 			[arguments.name],
-			{datasource: variables.dataSource, username: variables.username, password: variables.password}
+			$advisoryLockConnection()
 		);
 		return IsQuery(local.result) && IsBoolean(local.result.released) && local.result.released;
 	}
@@ -254,7 +254,7 @@ component extends="wheels.databaseAdapters.Base" output=false {
 		local.result = queryExecute(
 			local.sql,
 			local.params,
-			{datasource: variables.dataSource, username: variables.username, password: variables.password}
+			$advisoryLockConnection()
 		);
 		return IsQuery(local.result) && Val(local.result.holders) > 0;
 	}

vendor/wheels/model/locking.cfm

@@ -80,10 +80,12 @@
 
 	/**
 	 * Internal function. The application-server lock name that serialises withAdvisoryLock() callers
-	 * for one database lock (#4197).
+	 * for one database lock (#4197). It is keyed on the datasource the lock is taken on, the active
+	 * tenant's for a tenant model (#4223): another tenant's datasource is another connection pool,
+	 * which can't borrow the session holding this lock, so its callers needn't wait here.
 	 */
 	public string function $advisoryLockLocalName(required string name) {
-		return "wheels_advisory_" & Hash(variables.wheels.class.dataSource & "|" & arguments.name);
+		return "wheels_advisory_" & Hash(variables.wheels.class.adapter.$effectiveDataSource() & "|" & arguments.name);
 	}
 
 	/**

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

@@ -0,0 +1,12 @@
+component extends="Model" {
+
+	/*
+	 * Backs advisoryLockTenantSpec: the authors table as a shared (non-tenant) model, which keeps
+	 * the application's default datasource when a tenant datasource is active.
+	 */
+	function config() {
+		table("c_o_r_e_authors");
+		sharedModel();
+	}
+
+}

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

@@ -0,0 +1,184 @@
+/**
+ * Which datasource withAdvisoryLock() takes its lock on, by default (no transaction), when a tenant
+ * datasource is active (#4223). A tenant (non-shared) model locks on the tenant's datasource, like
+ * its queries; a shared model keeps the application's default datasource, so a lock taken through a
+ * shared model is one lock for every tenant.
+ */
+component extends="wheels.WheelsTest" {
+
+	function beforeAll() {
+		variables.g = application.wo;
+		variables.adapterName = variables.g.get("adapterName");
+		variables.applies = ListFindNoCase("MySQLModel,PostgreSQLModel,MicrosoftSQLServerModel", variables.adapterName) > 0;
+		variables.missingDataSource = "wheels_lock_tenant_missing";
+	}
+
+	function lockName() {
+		return "wheels_tenant_lock_" & Replace(CreateUUID(), "-", "", "all");
+	}
+
+	function activateMissingTenant(string dataSource = variables.missingDataSource) {
+		request.wheels.tenant = {id = arguments.dataSource, dataSource = arguments.dataSource, config = {}, "$locked" = true};
+	}
+
+	// Starts a thread holding the in-application lock named `localName` (what withAdvisoryLock()
+	// waits on first) for `holdMs`, and waits until it holds it. Started from a method, not a
+	// closure, and sharing state through the application scope, like advisoryLockExclusionSpec.
+	function startLocalHolder(required string localName, required numeric holdMs) {
+		var threadName = "advisoryLockTenantHolder" & Replace(CreateUUID(), "-", "", "all");
+		application.advisoryLockTenantSpec = {started = false};
+		thread name="#threadName#" action="run" localName="#arguments.localName#" holdMs="#arguments.holdMs#" {
+			lock name="#attributes.localName#" type="exclusive" timeout="5" {
+				application.advisoryLockTenantSpec.started = true;
+				sleep(attributes.holdMs);
+			}
+		}
+		var waited = 0;
+		while (!application.advisoryLockTenantSpec.started && waited < 5000) {
+			sleep(50);
+			waited += 50;
+		}
+		return threadName;
+	}
+
+	// The error type and text withAdvisoryLock() gives under the given tenant datasource, with a
+	// 1 second timeout, and whether the callback ran.
+	function lockUnderTenant(required string dataSource, required string name) {
+		var state = {ran = false, type = "", text = ""};
+		var saved = IsDefined("request.wheels.tenant") ? Duplicate(request.wheels.tenant) : "";
+		activateMissingTenant(arguments.dataSource);
+		try {
+			variables.g.model("author").withAdvisoryLock(
+				name = arguments.name,
+				timeout = 1,
+				callback = function() {
+					state.ran = true;
+				}
+			);
+		} catch (any e) {
+			state.type = e.type;
+			state.text = e.message & " " & e.detail & " " & (StructKeyExists(e, "extendedInfo") ? e.extendedInfo : "");
+		} finally {
+			restoreTenant(saved);
+		}
+		return state;
+	}
+
+	function restoreTenant(required any saved) {
+		if (IsStruct(arguments.saved)) {
+			request.wheels.tenant = arguments.saved;
+		} else {
+			StructDelete(request.wheels, "tenant");
+		}
+	}
+
+	function run() {
+
+		describe("withAdvisoryLock() under a tenant datasource", () => {
+
+			it("takes a tenant model's lock on the tenant's datasource", () => {
+				if (!variables.applies) {
+					skip("Advisory locks: MySQL, PostgreSQL and SQL Server.");
+				}
+				var state = {ran = false, error = ""};
+				var saved = IsDefined("request.wheels.tenant") ? Duplicate(request.wheels.tenant) : "";
+				activateMissingTenant();
+				try {
+					variables.g.model("author").withAdvisoryLock(
+						name = lockName(),
+						callback = function() {
+							state.ran = true;
+						}
+					);
+				} catch (any e) {
+					state.error = e.message & " " & e.detail & " " & (StructKeyExists(e, "extendedInfo") ? e.extendedInfo : "");
+				} finally {
+					restoreTenant(saved);
+				}
+				// the tenant's datasource doesn't exist, so the lock can only fail there
+				expect(state.ran).toBeFalse("the callback ran: the lock was not taken on the tenant datasource");
+				expect(state.error).toInclude(variables.missingDataSource);
+			});
+
+			it("takes a shared model's lock on the default datasource, whatever the tenant", () => {
+				if (!variables.applies) {
+					skip("Advisory locks: MySQL, PostgreSQL and SQL Server.");
+				}
+				var name = lockName();
+				var shared = variables.g.model("sharedAuthor");
+				var adapter = shared.$classData().adapter;
+				var state = {ran = false, held = false, lockDataSource = ""};
+				var saved = IsDefined("request.wheels.tenant") ? Duplicate(request.wheels.tenant) : "";
+				activateMissingTenant();
+				try {
+					shared.withAdvisoryLock(
+						name = name,
+						callback = function() {
+							state.ran = true;
+							state.lockDataSource = adapter.$advisoryLockConnection().datasource;
+							state.held = adapter.$isAdvisoryLockHeld(name);
+						}
+					);
+				} finally {
+					restoreTenant(saved);
+				}
+				expect(state.ran).toBeTrue();
+				expect(state.lockDataSource).toBe(variables.g.get("dataSourceName"));
+				expect(state.held).toBeTrue("the lock is held on the default datasource while the callback runs");
+				expect(adapter.$isAdvisoryLockHeld(name)).toBeFalse("released afterwards");
+			});
+
+			// Two tenants using the same lock name on one server: while a caller holds tenant A's
+			// in-application wait, tenant B gets past it at once (and reaches its own datasource,
+			// which here doesn't exist), and a second tenant-A caller still waits and times out.
+			it("doesn't make two tenants with the same lock name wait for each other on one server", () => {
+				if (!variables.applies) {
+					skip("Advisory locks: MySQL, PostgreSQL and SQL Server.");
+				}
+				var name = lockName();
+				var tenantA = variables.missingDataSource & "_a";
+				var tenantB = variables.missingDataSource & "_b";
+				var saved = IsDefined("request.wheels.tenant") ? Duplicate(request.wheels.tenant) : "";
+				activateMissingTenant(tenantA);
+				try {
+					var localNameA = variables.g.model("author").$advisoryLockLocalName(name);
+				} finally {
+					restoreTenant(saved);
+				}
+				var holder = startLocalHolder(localNameA, 4000);
+				var other = lockUnderTenant(tenantB, name);
+				var same = lockUnderTenant(tenantA, name);
+				thread action="join" name="#holder#" timeout="15000";
+				expect(application.advisoryLockTenantSpec.started).toBeTrue("the holder thread took tenant A's lock");
+				expect(other.type).notToBe("Wheels.AdvisoryLockTimeout", "tenant B waited behind tenant A");
+				expect(other.text).toInclude(tenantB);
+				expect(same.type).toBe("Wheels.AdvisoryLockTimeout");
+				expect(same.text).toInclude("Another caller in this application");
+				expect(other.ran || same.ran).toBeFalse();
+			});
+
+			// The in-application wait (#4197) follows the lock's datasource: callers of another
+			// tenant's database don't queue behind this one on the same server.
+			it("keys the in-application wait on the datasource the lock is taken on", () => {
+				var name = lockName();
+				var author = variables.g.model("author");
+				var shared = variables.g.model("sharedAuthor");
+				var names = {defaultAuthor = author.$advisoryLockLocalName(name), defaultShared = shared.$advisoryLockLocalName(name)};
+				var saved = IsDefined("request.wheels.tenant") ? Duplicate(request.wheels.tenant) : "";
+				activateMissingTenant();
+				try {
+					names.tenantAuthor = author.$advisoryLockLocalName(name);
+					names.tenantShared = shared.$advisoryLockLocalName(name);
+				} finally {
+					restoreTenant(saved);
+				}
+				expect(names.defaultShared).toBe(names.defaultAuthor, "without a tenant, both use the default datasource");
+				expect(names.tenantAuthor).notToBe(names.defaultAuthor, "a tenant model waits per tenant datasource");
+				expect(names.tenantShared).toBe(names.defaultAuthor, "a shared model keeps one wait for the whole application");
+			});
+
+		});
+
+	}
+
+}

web/sites/guides/src/content/docs/v4-2-0/digging-deeper/multi-tenancy.mdx

@@ -204,6 +204,15 @@ component extends="Model" {
 }
 ```
 
+Advisory locks follow the same rule. `withAdvisoryLock()` on a tenant model takes the lock in that tenant's database, so it only excludes callers working on the same tenant database (on MySQL, lock names are server-wide, so on the same MySQL server). For one lock across every tenant, such as a nightly job that must not run twice, call it through a shared model, which takes the lock on the default datasource:
+
+```cfm {test:compile} title="anywhere you call a model: controller, job or model"
+// one lock for the whole application, whichever tenant is active
+model("Plan").withAdvisoryLock(name = "nightly-billing", callback = function() {
+    // ...
+});
+```
+
 Rule of thumb: if the table exists in every tenant database, don't share it. If it exists only in the central database, share it. `User` is the classic borderline case — share it if users span tenants (one account, many tenants), scope it per tenant if accounts belong to exactly one tenant.
 
 ## Accessing the tenant inside your app

web/sites/guides/src/content/docs/v4-2-0/upgrading/4x-1-to-4x-2.mdx

@@ -360,6 +360,20 @@ On MySQL, PostgreSQL and SQL Server, `withAdvisoryLock()` now checks that the lo
 
 On SQL Server, `withAdvisoryLock()` called outside a transaction failed with "The statement or function must be executed in the context of a user transaction" ([#4220](https://github.com/wheels-dev/wheels/issues/4220)). The lock now belongs to the database session (`sp_getapplock @LockOwner = 'Session'`), as on MySQL and PostgreSQL, so it works anywhere, with the same release check. To identify which session holds a lock, Wheels reads `sys.dm_tran_locks`, which needs the `VIEW SERVER STATE` (SQL Server 2022: `VIEW SERVER PERFORMANCE STATE`) permission. Without it, a release waits until no session holds the lock, so it can report a failure in the rare case where another server takes the lock the moment ours is freed.
 
+### A tenant model's `withAdvisoryLock()` locks on the tenant database
+
+On MySQL and PostgreSQL, `withAdvisoryLock()` on a tenant (non-shared) model now takes the lock on the active tenant's datasource, the database its queries use ([#4223](https://github.com/wheels-dev/wheels/issues/4223)). 4.1 took it on the application's default datasource. SQL Server and `withAdvisoryLock(transaction = true)` already worked this way.
+
+The consequence: two tenants on different databases that use the same lock name no longer exclude each other, on one application server or across several. Each one gets its own lock. On MySQL, lock names are shared by every database on the same MySQL server, so tenant databases hosted on one MySQL server still exclude each other for the same name. If a lock must cover every tenant, call `withAdvisoryLock()` through a shared model (one with `sharedModel()` in its `config()`), which keeps the default datasource whichever tenant is active:
+
+```cfm
+model("Plan").withAdvisoryLock(name = "nightly-billing", callback = function() {
+    // runs once across all tenants
+});
+```
+
+Without a tenant datasource, nothing changes.
+
 ### `LocalDisk` storage keys are checked per segment
 
 `LocalDisk` now validates an object key one path segment at a time ([#3912](https://github.com/wheels-dev/wheels/issues/3912)):