fix(model): namespace per-request query cache under request.wheels.$queryCache

changelog.d/3336-request-query-cache-namespace.fixed.md

@@ -0,0 +1 @@
+- The per-request finder cache is now namespaced under `request.wheels.$queryCache[ModelName]` instead of sitting directly in `request.wheels[ModelName]`. Because CFML struct keys are case-insensitive, the flat layout let a model name alias onto a framework-owned request key: an app with a model named `Tenant` — the documented name for the control-plane model in a database-per-tenant app — shared one key between its query cache and `request.wheels.tenant`. Two silent failures followed. `$clearRequestCache()`, which runs after every create/update/delete/bulk operation, wiped the resolved tenant to `{}`, so every tenant-scoped query later in that request fell back to the control-plane datasource and wrote to the wrong database with no error. And a `Tenant` finder running before `TenantResolver` (the obvious shape for a subdomain→tenant directory) populated `request.wheels.tenant` with query-cache entries, making an unresolved request look resolved to any `IsDefined("request.wheels.tenant")` guard. Caching behaviour is otherwise unchanged (#3336)

vendor/wheels/model/miscellaneous.cfc

@@ -1,9 +1,31 @@
 component {
+	/**
+	 * Internal function.
+	 * Creates this model's slot in the per-request query cache if it doesn't exist yet.
+	 *
+	 * The cache is namespaced under the reserved `$queryCache` key rather than sitting directly in
+	 * `request.wheels` under the bare model name. CFML struct keys are case-insensitive, so the flat
+	 * layout let a model name alias onto a framework-owned request key — a model named `Tenant`
+	 * shared one key with `request.wheels.tenant`, silently dropping tenant datasource routing (#3336).
+	 */
+	public void function $ensureRequestQueryCache() {
+		if (!StructKeyExists(request, "wheels")) {
+			request.wheels = {};
+		}
+		if (!StructKeyExists(request.wheels, "$queryCache")) {
+			request.wheels["$queryCache"] = {};
+		}
+		if (!StructKeyExists(request.wheels["$queryCache"], variables.wheels.class.modelName)) {
+			request.wheels["$queryCache"][variables.wheels.class.modelName] = {};
+		}
+	}
+
 	/**
 	 * Deletes all queries stored during the request for this model.
 	 */
 	public void function $clearRequestCache() {
-		request.wheels[variables.wheels.class.modelName] = {};
+		$ensureRequestQueryCache();
+		request.wheels["$queryCache"][variables.wheels.class.modelName] = {};
 	}
 
 	/**

vendor/wheels/model/read.cfc

@@ -310,10 +310,8 @@ component {
 			// Batch finders (findEach / findInBatches) opt out via $useRequestCache so their per-page results don't accumulate in the request scope for the remainder of the request.
 			local.useRequestCache = application.wheels.cacheQueriesDuringRequest && arguments.$useRequestCache;
 			if (local.useRequestCache) {
-				// Create a struct in the request scope to store cached queries.
-				if (!StructKeyExists(request.wheels, variables.wheels.class.modelName)) {
-					request.wheels[variables.wheels.class.modelName] = {};
-				}
+				// Create this model's slot in the request-scoped query cache namespace.
+				$ensureRequestQueryCache();
 
 				// Derive the request cache key from the SQL shell key computed above (it already encodes the model name and the full arguments struct) so we don't have to serialize all arguments a second time.
 				local.queryKey = $hashedKey(local.queryShellKey, local.originalWhere);
@@ -323,9 +321,9 @@ component {
 			if (
 				local.useRequestCache
 				&& !arguments.reload
-				&& StructKeyExists(request.wheels[variables.wheels.class.modelName], local.queryKey)
+				&& StructKeyExists(request.wheels["$queryCache"][variables.wheels.class.modelName], local.queryKey)
 			) {
-				local.findAll = request.wheels[variables.wheels.class.modelName][local.queryKey];
+				local.findAll = request.wheels["$queryCache"][variables.wheels.class.modelName][local.queryKey];
 			} else {
 				local.finderArgs = {};
 				local.finderArgs.sql = local.sql;
@@ -357,7 +355,7 @@ component {
 				local.findAll = variables.wheels.class.adapter.$querySetup(argumentCollection = local.finderArgs);
 				if (local.useRequestCache) {
 					// Store in request cache so we never run the exact same query twice in the same request.
-					request.wheels[variables.wheels.class.modelName][local.queryKey] = local.findAll;
+					request.wheels["$queryCache"][variables.wheels.class.modelName][local.queryKey] = local.findAll;
 				}
 			}
 

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

@@ -0,0 +1,14 @@
+component extends="Model" {
+
+	/*
+	 * Exists purely to exercise the request-query-cache key collision guarded by
+	 * requestQueryCacheTenantCollisionSpec (#3336): `Tenant` is the natural model name for the
+	 * control-plane model in a database-per-tenant app, and CFML struct keys are case-insensitive,
+	 * so it is the one model name that can alias onto framework-owned `request.wheels.tenant`.
+	 * Backed by the existing authors fixture table so no populate.cfm changes are needed.
+	 */
+	function config() {
+		table("c_o_r_e_authors");
+	}
+
+}

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

@@ -17,16 +17,16 @@ component extends="wheels.WheelsTest" {
 			it("stores a single entry per unique findAll call when enabled", () => {
 				application.wheels.cacheQueriesDuringRequest = true;
 				model("author").findAll(where = "lastName = 'Djurner'");
-				expect(StructCount(request.wheels["author"])).toBe(1);
+				expect(StructCount(request.wheels["$queryCache"]["author"])).toBe(1);
 				model("author").findAll(where = "lastName = 'Djurner'");
-				expect(StructCount(request.wheels["author"])).toBe(1);
+				expect(StructCount(request.wheels["$queryCache"]["author"])).toBe(1);
 			})
 
 			it("keeps distinct entries for same-shape queries that differ only by where values", () => {
 				application.wheels.cacheQueriesDuringRequest = true;
 				var djurner = model("author").findAll(where = "lastName = 'Djurner'");
 				var petruzzi = model("author").findAll(where = "lastName = 'Petruzzi'");
-				expect(StructCount(request.wheels["author"])).toBe(2);
+				expect(StructCount(request.wheels["$queryCache"]["author"])).toBe(2);
 				expect(djurner.recordCount).toBe(1);
 				expect(petruzzi.recordCount).toBe(1);
 				expect(djurner.lastName).toBe("Djurner");
@@ -36,7 +36,7 @@ component extends="wheels.WheelsTest" {
 			it("does not store query results when cacheQueriesDuringRequest is disabled", () => {
 				application.wheels.cacheQueriesDuringRequest = false;
 				model("author").findAll(where = "lastName = 'Djurner'");
-				expect(StructCount(request.wheels["author"])).toBe(0);
+				expect(StructCount(request.wheels["$queryCache"]["author"])).toBe(0);
 			})
 
 			it("findEach does not accumulate per-batch queries in the request cache", () => {
@@ -52,7 +52,7 @@ component extends="wheels.WheelsTest" {
 					}
 				);
 				// Only the single up-front COUNT query may be cached, the per-batch id/data queries must not accumulate.
-				expect(StructCount(request.wheels["author"])).toBeLTE(1);
+				expect(StructCount(request.wheels["$queryCache"]["author"])).toBeLTE(1);
 				expect(result.count).toBe(expectedTotal);
 			})
 
@@ -79,7 +79,7 @@ component extends="wheels.WheelsTest" {
 					}
 				);
 				// Pre-fix the empty case ran a second COUNT (findAll only honors `count` when > 0), which showed up as a second cached entry.
-				expect(StructCount(request.wheels["author"])).toBe(1);
+				expect(StructCount(request.wheels["$queryCache"]["author"])).toBe(1);
 				expect(result.count).toBe(0);
 			})
 
@@ -93,7 +93,7 @@ component extends="wheels.WheelsTest" {
 					}
 				);
 				// Pre-fix the empty case ran a second COUNT (findAll only honors `count` when > 0), which showed up as a second cached entry.
-				expect(StructCount(request.wheels["author"])).toBe(1);
+				expect(StructCount(request.wheels["$queryCache"]["author"])).toBe(1);
 				expect(result.batchCount).toBe(0);
 			})
 
@@ -111,7 +111,7 @@ component extends="wheels.WheelsTest" {
 					}
 				);
 				// Only the single up-front COUNT query may be cached, the per-batch id/data queries must not accumulate.
-				expect(StructCount(request.wheels["author"])).toBeLTE(1);
+				expect(StructCount(request.wheels["$queryCache"]["author"])).toBeLTE(1);
 				expect(result.totalRecords).toBe(expectedTotal);
 				expect(result.batchCount).toBeGTE(2);
 			})

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

@@ -0,0 +1,118 @@
+/**
+ * Regression coverage for #3336.
+ *
+ * The per-request finder cache used to key itself on the bare model name directly in
+ * `request.wheels`. CFML struct keys are case-insensitive, so an app with a model named `Tenant`
+ * — the documented name for the control-plane model in a database-per-tenant app — had its query
+ * cache and its resolved-tenant context aliased onto the single key `request.wheels.tenant`.
+ *
+ * Two failure modes followed:
+ *   1. Write path: `$clearRequestCache()` (called after every create/update/delete/bulk op) set
+ *      `request.wheels.tenant = {}`, wiping the resolved tenant for the rest of the request. Every
+ *      later tenant-scoped query silently fell back to the control-plane datasource.
+ *   2. Read path: a `Tenant` finder running before resolution created `request.wheels.tenant` as a
+ *      query-cache struct, so `IsDefined("request.wheels.tenant")` reported an unresolved request
+ *      as resolved.
+ *
+ * The cache now lives under the reserved `request.wheels.$queryCache` sub-struct.
+ */
+component extends="wheels.WheelsTest" {
+
+	function run() {
+
+		g = application.wo;
+
+		describe("request query cache / tenant key collision (##3336)", () => {
+
+			beforeEach(() => {
+				originalCacheSetting = application.wheels.cacheQueriesDuringRequest;
+				StructDelete(request.wheels, "tenant");
+				StructDelete(request.wheels, "$queryCache");
+			})
+
+			afterEach(() => {
+				application.wheels.cacheQueriesDuringRequest = originalCacheSetting;
+				StructDelete(request.wheels, "tenant");
+				StructDelete(request.wheels, "$queryCache");
+			})
+
+			it("keeps the query cache out of the bare model-name key", () => {
+				application.wheels.cacheQueriesDuringRequest = true;
+				model("Tenant").findAll(where = "lastName = 'Djurner'");
+
+				expect(StructKeyExists(request.wheels, "$queryCache")).toBeTrue();
+				expect(StructKeyExists(request.wheels["$queryCache"], "Tenant")).toBeTrue();
+				// The bare key is what aliased onto request.wheels.tenant.
+				expect(StructKeyExists(request.wheels, "Tenant")).toBeFalse();
+			})
+
+			it("still caches repeat finder calls once namespaced", () => {
+				application.wheels.cacheQueriesDuringRequest = true;
+				model("Tenant").findAll(where = "lastName = 'Djurner'");
+				model("Tenant").findAll(where = "lastName = 'Djurner'");
+
+				expect(StructCount(request.wheels["$queryCache"]["Tenant"])).toBe(1);
+			})
+
+			// Failure mode 2 — read path.
+			it("does not fabricate a resolved tenant when a Tenant finder runs before resolution", () => {
+				application.wheels.cacheQueriesDuringRequest = true;
+				model("Tenant").findAll(where = "lastName = 'Djurner'");
+
+				expect(IsDefined("request.wheels.tenant")).toBeFalse();
+				expect(StructIsEmpty(g.tenant())).toBeTrue();
+			})
+
+			// Failure mode 1 — write path, via the helper every write path calls.
+			it("does not erase resolved tenant context when a Tenant model clears its request cache", () => {
+				request.wheels.tenant = {id = "acme", dataSource = "tenant_acme", config = {}, "$locked" = true};
+
+				model("Tenant").$clearRequestCache();
+
+				expect(IsDefined("request.wheels.tenant")).toBeTrue();
+				expect(request.wheels.tenant.id).toBe("acme");
+				expect(request.wheels.tenant.dataSource).toBe("tenant_acme");
+				expect(g.$tenantDataSource()).toBe("tenant_acme");
+			})
+
+			// Failure mode 1 — write path, through a real bulk write rather than the helper directly.
+			// The where clause matches nothing, so this mutates no fixture data but still reaches
+			// $updateAll() -> $clearRequestCache(). The tenant datasource must be the real one: the
+			// write executes for real, and pointing it at a non-existent datasource throws inside
+			// updateAll's transaction and leaves transaction state dirty for later specs.
+			it("keeps the resolved tenant intact after a Tenant write in the same request", () => {
+				application.wheels.cacheQueriesDuringRequest = true;
+				request.wheels.tenant = {
+					id = "acme",
+					dataSource = application.wheels.dataSourceName,
+					config = {},
+					"$locked" = true
+				};
+
+				model("Tenant").updateAll(
+					where = "lastName = 'NoSuchTenantXYZ'",
+					instantiate = false,
+					firstName = "ignored"
+				);
+
+				expect(IsDefined("request.wheels.tenant")).toBeTrue();
+				expect(request.wheels.tenant.id).toBe("acme");
+				expect(g.$tenantDataSource()).toBe(application.wheels.dataSourceName);
+			})
+
+			it("clears only its own model's cache, leaving sibling models untouched", () => {
+				application.wheels.cacheQueriesDuringRequest = true;
+				model("author").findAll(where = "lastName = 'Djurner'");
+				model("Tenant").findAll(where = "lastName = 'Djurner'");
+				expect(StructCount(request.wheels["$queryCache"]["author"])).toBe(1);
+
+				model("Tenant").$clearRequestCache();
+
+				expect(StructCount(request.wheels["$queryCache"]["Tenant"])).toBe(0);
+				expect(StructCount(request.wheels["$queryCache"]["author"])).toBe(1);
+			})
+
+		})
+
+	}
+}

web/content/blog/posts/caching-in-wheels-4.md

@@ -138,11 +138,11 @@ This is honored only when `cacheQueries=true`. The arg name is literally `cache`
 
 ### Request-level: the dedupe you already have
 
-The second query cache is on by default and you've been using it without knowing. `cacheQueriesDuringRequest` (default `true`, even in dev) stores each unique `findAll` result in `request.wheels[ModelName]`, so an identical query within a single request runs exactly one SQL statement:
+The second query cache is on by default and you've been using it without knowing. `cacheQueriesDuringRequest` (default `true`, even in dev) stores each unique `findAll` result in `request.wheels.$queryCache[ModelName]`, so an identical query within a single request runs exactly one SQL statement:
 
 ```cfm
 // These two run only ONE SQL query within the same request —
-// the second is served from request.wheels["Author"].
+// the second is served from request.wheels.$queryCache["Author"].
 model("Author").findAll(where="lastName='Djurner'");
 model("Author").findAll(where="lastName='Djurner'");
 
@@ -319,7 +319,7 @@ Everything here is real, cited behavior — the parts that will cost you an afte
 
 - **`findAll(cache=N)` is NOT in Wheels' own cache struct.** It hands a native `cachedWithin` timespan to the DB adapter — engine-level `cfquery` caching keyed on the generated SQL. It's *not* cleared by `clearCachableActions()` and *not* affected by editing `application.wheels.cache`. It clears on app reload (cacheKey rotation) or on expiry.
 
-- **Two query caches, easily confused.** Request-level (`cacheQueriesDuringRequest`, in `request.wheels[ModelName]`) dedupes within one request and is always on — bypass it with `reload=true`. Cross-request (`findAll(cache=N)` + `cacheQueries`) is the engine cache. `reload=true` defeats the *request-level* one; it has nothing to do with `cache=N`.
+- **Two query caches, easily confused.** Request-level (`cacheQueriesDuringRequest`, in `request.wheels.$queryCache[ModelName]` — it sat directly in `request.wheels[ModelName]` before [#3336](https://github.com/wheels-dev/wheels/issues/3336)) dedupes within one request and is always on — bypass it with `reload=true`. Cross-request (`findAll(cache=N)` + `cacheQueries`) is the engine cache. `reload=true` defeats the *request-level* one; it has nothing to do with `cache=N`.
 
 - **No targeted invalidation API.** No public `clearCache(key)` / `expire(key)` for action/partial/page entries — only the `$`-prefixed internals (and `$clearCache` clears a whole category, not a single key). `clearCachableActions()` unregisters cachability; it does **not** evict already-cached content. Real invalidation = short `time` values + a full `?reload=true`, which re-inits the cache and flushes `cfcache`.