fix(model): clear the whole per-request query cache on any ORM write and on rollback

changelog.d/4429-request-cache-cross-model.fixed.md

@@ -0,0 +1 @@
+- The per-request finder cache (`cacheQueriesDuringRequest`) is now cleared across every model on any ORM write and on a transaction rollback, so a cached `include=` query no longer returns a joined model's pre-write columns — or a rolled-back transaction's phantom rows — for the rest of the request (#4429)

vendor/wheels/global/cache.cfm

@@ -542,6 +542,12 @@
 	 *   forgetCachedQueries("Post")           clears just the Post model's slot, by name (works anywhere)
 	 *   forgetCachedQueries(all = true)        clears every model's cached results this request
 	 *
+	 * This is only needed after RAW writes (`queryExecute(...)`) — ORM writes (create / update / delete
+	 * and the `*All` / `*ByKey` forms) already clear the whole request cache themselves (#4429). After a
+	 * raw write, prefer `all = true` when the tables you wrote are joined by other models' `include=`
+	 * queries: a cross-model result is cached in the *base* model's slot, so the single-model named form
+	 * misses it. The named form is for a raw single-table write whose table no other cached query joins.
+	 *
 	 * Called as a method on a model instance it scopes to that model automatically, because only a model
 	 * carries `variables.wheels.class.modelName` (Controller / view / job / base Global do not). Called
 	 * outside a model with neither a `modelName` nor `all`, it throws rather than silently wiping every
@@ -589,8 +595,7 @@
 			if (arguments.all) {
 				StructDelete(request.wheels, "$queryCache");
 			} else {
-				// Empty just this model's slot, keeping the key — the same shape $clearRequestCache
-				// leaves behind, so a re-query repopulates it in place.
+				// Empty just this model's slot, keeping the key, so a re-query repopulates it in place.
 				request.wheels["$queryCache"][local.slot] = {};
 			}
 		}

vendor/wheels/model/miscellaneous.cfm

@@ -21,11 +21,22 @@
 	}
 
 	/**
-	 * Deletes all queries stored during the request for this model.
+	 * Clears the ENTIRE per-request query cache — every model's slot, not just this model's (#4429).
+	 *
+	 * A finder's result is cached in the base model's slot keyed by its SQL, so a query that joins
+	 * another model through `include=` lives in the base model's slot. Clearing only the writing
+	 * model's slot left such a cross-model query returning this model's pre-write columns for the rest
+	 * of the request. Every ORM write path funnels through here (create / update / delete / *All /
+	 * *ByKey / the #4397 soft-delete UPDATE via $runDeleteStatement), so clearing all of it here means
+	 * no write path can forget the cross-model case. The cache is request-scoped, so re-running the
+	 * affected reads after a write is both correct and cheap. Raw (non-ORM) writes are the caller's job
+	 * via `forgetCachedQueries(all = true)`.
 	 */
 	public void function $clearRequestCache() {
-		$ensureRequestQueryCache();
-		request.wheels["$queryCache"][variables.wheels.class.modelName] = {};
+		if (!StructKeyExists(request, "wheels")) {
+			request.wheels = {};
+		}
+		request.wheels["$queryCache"] = {};
 	}
 
 	/**

vendor/wheels/model/transactions.cfm

@@ -167,6 +167,12 @@
 			) {
 				$resolveTransactionCallbacks(connection = arguments.connectionArgs, type = "afterRollback");
 			}
+			if (arguments.closeTransaction) {
+				// The transaction rolled back (the body threw). Reads made inside it may have cached
+				// uncommitted rows, so drop the whole request query cache — otherwise a later read in
+				// this request returns phantom data that never committed (#4429).
+				$clearRequestCache();
+			}
 			rethrow;
 		}
 		// Transaction block closed without an exception: fire afterCommit on commit, or
@@ -182,6 +188,12 @@
 				connection = arguments.connectionArgs,
 				type = local.ctx.txnState.rolledBack ? "afterRollback" : "afterCommit"
 			);
+			if (local.ctx.txnState.rolledBack) {
+				// A non-exception rollback (the method returned false, or transaction = "rollback"):
+				// same phantom-read risk as the throw path above, so drop the request query cache.
+				// A commit does not need this — its reads are of committed rows (#4429).
+				$clearRequestCache();
+			}
 		}
 		return local.ctx.rv;
 	}
@@ -414,11 +426,16 @@
 			// A rollback that itself fails is logged, never allowed to replace the
 			// method's exception, which is what the caller needs to see.
 			$rollbackToSavepointQuietly(name = local.savepoint, connection = local.connectionArgs, mark = local.mark);
+			// The savepoint unit's reads may have cached rows it just rolled back; clear the request
+			// cache so the outer transaction, which carries on, does not serve them as phantoms (#4429).
+			$clearRequestCache();
 			rethrow;
 		}
 		// Same failure test as the commit branch: a numeric count (0-row bulk op) is not a failure.
 		if (!IsBoolean(local.rv) || (!IsNumeric(local.rv) && !local.rv)) {
 			$rollbackToSavepoint(name = local.savepoint, connection = local.connectionArgs, mark = local.mark);
+			// Same phantom-read risk as the exception path above (#4429).
+			$clearRequestCache();
 		}
 		if (!IsBoolean(local.rv)) {
 			Throw(

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

@@ -19,4 +19,34 @@ component extends="Model" {
 		return true;
 	}
 
+	// #4429 rollback-cache probe: inside a transaction, write (uncommitted) then read — caching the
+	// uncommitted row — and return false so the transaction rolls back. The spec then checks that the
+	// phantom row is not still served from the request cache after the rollback.
+	public boolean function $cacheRollbackProbe4429() {
+		this.update(firstName = "Phantom4429", transaction = "none");
+		model("Author").findByKey(key = this.id);
+		return false;
+	}
+
+	// #4429 throw-path rollback probe: write + read (caching the uncommitted row) then THROW, so the
+	// transaction rolls back through the exception branch rather than the returned-false branch.
+	public boolean function $throwRollbackProbe4429() {
+		this.update(firstName = "PhantomThrow4429", transaction = "none");
+		model("Author").findByKey(key = this.id);
+		Throw(type = "Test.Rollback4429", message = "rollback the throw-path probe");
+	}
+
+	// #4429 savepoint-rollback probe: the inner unit (run as a savepoint) writes + reads the uncommitted
+	// row then returns false so only the savepoint rolls back; the outer transaction carries on.
+	public boolean function $savepointInner4429() {
+		this.update(firstName = "PhantomSavepoint4429", transaction = "none");
+		model("Author").findByKey(key = this.id);
+		return false;
+	}
+
+	public boolean function $savepointOuter4429() {
+		this.invokeWithTransaction(method = "$savepointInner4429", transaction = "savepoint");
+		return true;
+	}
+
 }

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

@@ -36,7 +36,11 @@ 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["$queryCache"]["author"])).toBe(0);
+				// beforeEach's $clearRequestCache now empties the whole $queryCache (#4429), so with the
+				// cache disabled the author slot may be absent rather than present-but-empty; either way
+				// nothing was cached for author.
+				var cache = request.wheels["$queryCache"];
+				expect(StructKeyExists(cache, "author") ? StructCount(cache["author"]) : 0).toBe(0);
 			})
 
 			it("findEach does not accumulate per-batch queries in the request cache", () => {

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

@@ -100,16 +100,18 @@ component extends="wheels.WheelsTest" {
 				expect(g.$tenantDataSource()).toBe(application.wheels.dataSourceName);
 			})
 
-			it("clears only its own model's cache, leaving sibling models untouched", () => {
+			it("clears every model's cache, not just its own, so cross-model include= results can't go stale", () => {
 				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);
+				// #4429: $clearRequestCache now empties the whole request cache, siblings included — a query
+				// that joins another model via include= is cached under the base model, so a write to one
+				// model has to clear the others or they keep serving its pre-write rows.
+				expect(StructIsEmpty(request.wheels["$queryCache"])).toBeTrue();
 			})
 
 		})

vendor/wheels/tests/specs/requestcache/RequestCacheInvalidationSpec.cfc

@@ -0,0 +1,117 @@
+/**
+ * #4429: the per-request finder cache (cacheQueriesDuringRequest) must be invalidated across models
+ * on any ORM write, and on a transaction rollback — not just in the writing model's own slot.
+ *
+ * The core runner forces cacheQueriesDuringRequest OFF (tests/runner.cfm), so these specs turn it ON
+ * themselves. The setting is read at call time in model/read.cfm, so a per-group flip works. beforeEach
+ * flips it on; afterEach restores it AND restores the one post + author these specs mutate, and wipes
+ * request.wheels["$queryCache"] — all of that runs even when a spec fails, so a failing spec can't leak
+ * a mutated row or a cached row into the next spec (the whole suite runs in one request). Catch writes
+ * would use `var state` (cross-engine invariant 11); none are needed here.
+ */
+component extends="wheels.WheelsTest" {
+
+	function beforeAll() {
+		var wo = application.wo;
+		var post = wo.model("Post").findOne(where = "authorid IS NOT NULL", order = "id", returnAs = "query");
+		variables.postId = post.id;
+		variables.postTitle = post.title;
+		variables.authorId = post.authorid;
+		variables.authorFirstName = wo.model("Author").findByKey(variables.authorId).firstName;
+	}
+
+	function run() {
+
+		describe("request query cache invalidation (4429)", () => {
+
+			beforeEach(() => {
+				variables.g = application.wo;
+				variables.cacheWas = application.wheels.cacheQueriesDuringRequest;
+				application.wheels.cacheQueriesDuringRequest = true;
+				$wipeRequestQueryCache();
+			});
+
+			afterEach(() => {
+				// Restore the rows the specs mutate, unconditionally, so a failed spec leaves no pollution.
+				g.model("Post").findByKey(variables.postId).update(title = variables.postTitle);
+				g.model("Author").findByKey(variables.authorId).update(firstName = variables.authorFirstName);
+				application.wheels.cacheQueriesDuringRequest = variables.cacheWas;
+				$wipeRequestQueryCache();
+			});
+
+			it("serves a repeated read from cache when no ORM write intervenes (cache really is on)", () => {
+				var first = g.model("Post").findAll(where = "id = #variables.postId#", returnAs = "query").title;
+				// A RAW write bypasses ORM invalidation, so with the cache ON the next read is still the
+				// cached value — proof the cache is active (and why raw writes need forgetCachedQueries).
+				$rawPostTitle(variables.postId, "RawTitle4429");
+				var second = g.model("Post").findAll(where = "id = #variables.postId#", returnAs = "query").title;
+				expect(second).toBe(first);
+				expect(second).notToBe("RawTitle4429");
+			});
+
+			it("an ORM write to a model clears that model's own cached finder", () => {
+				g.model("Post").findAll(where = "id = #variables.postId#", returnAs = "query");
+				g.model("Post").findByKey(variables.postId).update(title = "Changed4429a");
+				expect(g.model("Post").findAll(where = "id = #variables.postId#", returnAs = "query").title).toBe("Changed4429a");
+			});
+
+			it("an ORM write to an INCLUDED model clears a base model's include= cached query", () => {
+				var sel = "c_o_r_e_posts.id, c_o_r_e_authors.firstName AS authorName4429";
+				// Cache the Post finder that joins Author (lives in the Post slot, keyed by this SQL).
+				g.model("Post").findAll(where = "c_o_r_e_posts.id = #variables.postId#", include = "author", select = sel, returnAs = "query");
+				// An ORM write to the INCLUDED model. Pre-fix this cleared only the Author slot, so the
+				// Post slot kept serving the pre-write author column for the rest of the request.
+				g.model("Author").findByKey(variables.authorId).update(firstName = "Changed4429b");
+				var after = g.model("Post").findAll(where = "c_o_r_e_posts.id = #variables.postId#", include = "author", select = sel, returnAs = "query").authorName4429;
+				expect(after).toBe("Changed4429b");
+			});
+
+			it("a transaction rollback clears reads cached inside it (no phantom rows)", () => {
+				// The probe writes then reads inside the transaction (caching the uncommitted row), then
+				// returns false to roll back. Pre-fix the phantom row stayed cached after the rollback.
+				g.model("Author").findByKey(variables.authorId).invokeWithTransaction(method = "$cacheRollbackProbe4429", transaction = "rollback");
+				expect(g.model("Author").findByKey(variables.authorId).firstName).toBe(variables.authorFirstName);
+			});
+
+			it("a rollback through the exception (throw) path clears cached reads too", () => {
+				// The probe throws instead of returning false, so the transaction rolls back through the
+				// begin-catch branch of $runInTransaction, which must clear the cache just like the other path.
+				var state = {threw = false};
+				try {
+					g.model("Author").findByKey(variables.authorId).invokeWithTransaction(method = "$throwRollbackProbe4429", transaction = "commit");
+				} catch (any e) {
+					state.threw = true;
+				}
+				expect(state.threw).toBeTrue();
+				expect(g.model("Author").findByKey(variables.authorId).firstName).toBe(variables.authorFirstName);
+			});
+
+			it("a savepoint rollback clears reads cached inside the savepoint (no phantom), outer carries on", () => {
+				// The outer transaction commits; inside it a savepoint unit writes + reads (caching the
+				// uncommitted row) then rolls back. Pre-fix the savepoint's phantom row stayed cached while
+				// the outer transaction continued and committed.
+				g.model("Author").findByKey(variables.authorId).invokeWithTransaction(method = "$savepointOuter4429", transaction = "commit");
+				expect(g.model("Author").findByKey(variables.authorId).firstName).toBe(variables.authorFirstName);
+			});
+
+		});
+
+	}
+
+	private void function $wipeRequestQueryCache() {
+		if (StructKeyExists(request, "wheels")) {
+			request.wheels["$queryCache"] = {};
+		}
+	}
+
+	// id is untyped and bound as cf_sql_bigint: CockroachDB ids are 64-bit, which overflow
+	// cf_sql_integer (and lose precision through a `numeric` arg) on Adobe (#4437 review).
+	private void function $rawPostTitle(required any id, required string title) {
+		queryExecute(
+			"UPDATE c_o_r_e_posts SET title = :title WHERE id = :id",
+			{title = {value = arguments.title}, id = {value = arguments.id, cfsqltype = "cf_sql_bigint"}},
+			{datasource = application.wheels.dataSourceName}
+		);
+	}
+
+}

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

@@ -193,6 +193,19 @@ Query caching is ideal when:
 - Per-request result variation is acceptable (a 5-minute-old revenue number is fine on a dashboard).
 - The view output depends on other per-request data (current user, feature flags) so whole-action caching won't work.
 
+## The per-request query cache
+
+Separate from `findAll(cache=N)` above, Wheels keeps a second, much shorter-lived query cache that is **on by default**: `cacheQueriesDuringRequest`. Within a **single request**, the first time a finder runs a given SQL statement its result is remembered, and an identical finder later in the same request returns that result instead of hitting the database again. It is what keeps a view that calls the same finder in a loop — or across several partials — from running the query many times. The cache lives in `request` scope, so it never outlives the request and is never shared between requests, visitors, or nodes.
+
+You rarely interact with it directly, because Wheels keeps it correct for you:
+
+- **Any ORM write clears the whole per-request cache.** `create()`, `update()`, `delete()`, `save()`, and the bulk forms (`updateAll` / `deleteAll` / `updateByKey` / `deleteByKey` / `insertAll` / `upsertAll`) drop every cached finder result for the rest of the request — not just the written model's. This matters for `include=`: a query that joins another model is cached under the base model, so a write to the included model has to clear it too, or the base finder would keep returning the joined model's pre-write columns.
+- **A transaction rollback clears it too.** A finder that ran inside a transaction can cache rows the transaction then rolled back; clearing on rollback keeps those phantom rows from being served later in the request.
+- **`reload=true` bypasses it for one call:** `model("Post").findByKey(key=id, reload=true)` always goes to the database.
+- **After a raw write you clear it yourself.** A `queryExecute("UPDATE …")` is invisible to the ORM, so cached finders can go stale. Call `forgetCachedQueries(all=true)` after a raw write when the tables you changed are joined by other models' `include=` queries; the single-model form `forgetCachedQueries("Post")` is enough for a raw single-table write that no other cached query joins. The same applies to a **raw `transaction {}` block that rolls back** — the automatic clear only fires for ORM transactions (`model.transaction()` / a write's `transaction=` argument), so after rolling back a raw block call `forgetCachedQueries(all=true)` to drop any rows it cached.
+
+Turn the whole thing off with `set(cacheQueriesDuringRequest=false)` in `config/settings.cfm` if you want every finder to hit the database.
+
 ## Cache your own data
 
 When the slow part is a value you compute yourself (a total, a call to another service, a struct built from several queries), cache it with `appCacheFetch()`. It returns the cached value, or calls your function, caches what it returns and returns that: