Skip to content

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

Merged
bpamiri merged 4 commits into
developfrom
fix/3336-namespace-request-query-cache
Aug 3, 2026
Merged

fix(model): namespace per-request query cache under request.wheels.$queryCache#3338
bpamiri merged 4 commits into
developfrom
fix/3336-namespace-request-query-cache

Conversation

@bpamiri

@bpamiri bpamiri commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Fixes #3336.

The collision

The per-request finder cache keyed itself on the bare model name directly in request.wheels. CFML struct keys are case-insensitive, so a model named Tenant — the natural name for the control-plane model in a database-per-tenant app, and the one used in TenantResolver's own docblock — shared a single key between its query cache and the framework-owned request.wheels.tenant.

The fix moves the cache under a reserved request.wheels.$queryCache sub-struct. Caching behaviour is unchanged: same per-model slots, same query keys, same lifecycle.

// before — aliases onto request.wheels.tenant for a model named Tenant
request.wheels[variables.wheels.class.modelName][local.queryKey] = local.findAll;

// after
request.wheels["$queryCache"][variables.wheels.class.modelName][local.queryKey] = local.findAll;

model/miscellaneous.cfc gains $ensureRequestQueryCache(), which owns namespace creation for the four call sites (three in read.cfc, one in $clearRequestCache()).

Scope

The sweep found three independent families of dynamic keys in request.wheels. Only the first is fixed here:

Site Key Auto-collides with tenant?
model/read.cfc ×3, model/miscellaneous.cfc ×1 bare model name Yes — this issue
Global.cfc pagination() / setPagination() user-supplied handle (default "query") Only if an app names a handle tenant — filed as #3339
model/callbacks.cfc:331 query hash No

The pagination handles are a real-but-latent collision surface; they need an app author to actively pick the name, so they're tracked separately in #3339 rather than bundled in.

Tenant-context hardening (second commit)

Folded in at maintainer request. The first commit stops the framework from ever writing a query cache onto request.wheels.tenant; these two stop anything else that lands there from being read as a resolved tenant.

  • tenant() requires a non-empty dataSource before treating the key as an active tenant — the same test $tenantDataSource() already applies before routing a query. It previously returned whatever occupied the key, so a malformed value satisfied every IsDefined("request.wheels.tenant") or truthiness guard while carrying no usable routing information. The two now agree by construction and can't disagree about whether a request is tenanted.
  • TenantResolver deletes any pre-existing value on the key when its resolver returns no match. Previously an unresolved request only looked unresolved after the finally block, so a stale or foreign value could read as resolved for the whole downstream request.

All four framework producers — switchTenant() (throws without a dataSource), TenantResolver, Job.$restoreTenantContext() and TenantMigrator (both guard) — already guarantee a non-empty dataSource, so correctly-resolved tenants are unaffected. Only foreign values are filtered.

Net effect: a malformed tenant context degrades from wrong behaviour to a no-op.

Tests

vendor/wheels/tests/specs/model/requestQueryCacheTenantCollisionSpec.cfc — 6 specs covering both failure modes plus the namespacing invariant. Backed by a new Tenant fixture model mapped onto the existing c_o_r_e_authors table, so populate.cfm is untouched.

Written red first. The failures before the fix were exactly the two reported behaviours:

  • key [ID] doesn't exist reading request.wheels.tenant.id after a Tenant write → failure mode 1, tenant context wiped to {}
  • IsDefined("request.wheels.tenant") returning true after a Tenant finder → failure mode 2, tenant context fabricated

requestQueryCacheSpec.cfc's 8 existing assertions are repointed at the namespaced key — they're white-box assertions on the exact structure being moved.

Verification

Lucee 7 + SQLite via bash tools/test-local.sh (full suite, not scoped):

Result
develop @ fc59492 4707 passed, 0 failed
after commit 1 (namespacing) 4713 passed, 0 failed
after commit 2 (hardening) 4717 passed, 0 failed

Delta is exactly the 10 new specs (6 + 4).

Both sets were written red first. Reverting just the two hardening files while keeping their specs yields exactly 4 failures, confirming those specs exercise the change rather than restating existing behaviour.

⚠️ The cross-engine matrix did not run. tools/test-matrix.sh lucee7 mysql fails on this machine before any test executes — Docker Desktop can't bind-mount tools/docker/lucee7/box.json onto /wheels-test-suite/box.json under virtiofs:

error mounting ".../tools/docker/lucee7/box.json" to rootfs at "/wheels-test-suite/box.json":
mountpoint "/run/host_virtiofs/.../box.json" is outside of rootfs

It fails identically on clean develop, so it's environmental, not this change. Because I couldn't verify Adobe/BoxLang locally, the implementation was deliberately rewritten to avoid anything engine-sensitive: an earlier draft returned the cache struct by reference and used StructClear(), which would have depended on struct-reference write semantics holding on Adobe (cf. cross-engine invariant #6, where Adobe copies arrays by value in struct literals). What's here instead is a literal key-path swap of the original code plus an ensure-helper — no new semantics on any path.

What PR CI does and does not cover

To be precise about the engine coverage this PR actually carries:

Engine Coverage on this PR
Lucee 7 Full core suiteLucee 7 + SQLite (LuCLI), plus my local run
Adobe 2023 Boot + HTTP probes onlysmoke-env.yml cold-boots the demo app under production/testing and asserts clean 404s / no stack traces / reload behaviour. Per its own comments the probes never query the database, so no model spec runs. It does prove the new $-prefixed public mixin compiles and integrates via $integrateComponents() on Adobe.
Adobe 2025, Lucee 6, BoxLang Not exercised by PR CI

compat-matrix.yml — the workflow that runs the full suite across all five engines — is weekly cron + workflow_dispatch only, and continue-on-error: true. It does not trigger on pull requests. I've manually dispatched it against this branch: https://github.com/wheels-dev/wheels/actions/runs/30772173443

Constructs used are all matrix-safe by inspection: StructKeyExists, bracket-notation struct keys, no closures, no attributeCollection, no reserved scope names, no finally loops. The new helper is public + $-prefixed per cross-engine invariant #7 so it mixes in correctly, and lives in wheels.model.*, which is outside the protectedControllerMethods surface — so it can't shadow a controller action name.

Docs

web/content/blog/posts/caching-in-wheels-4.md documents the old request.wheels[ModelName] path in two places; both are updated. Drop that hunk if you'd rather not touch a published post.

…ueryCache

The per-request finder cache keyed itself on the bare model name directly in
`request.wheels`. CFML struct keys are case-insensitive, so 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 the
framework-owned `request.wheels.tenant`.

Two silent failures followed:

1. Write path. `$clearRequestCache()` runs after every create/update/delete/bulk
   operation and set `request.wheels.tenant = {}`, erasing the resolved tenant
   for the rest of the request. `databaseAdapters/Base.cfc` only swaps in the
   tenant datasource when `request.wheels.tenant.dataSource` exists, so every
   tenant-scoped query after that point fell back to the control-plane
   datasource and wrote to the wrong database with no exception.

2. Read path. A `Tenant` finder running before `TenantResolver` — which is what
   happens when the resolver itself looks tenants up, the obvious shape for a
   subdomain to tenant directory — populated `request.wheels.tenant` with
   query-cache entries. The struct is non-empty but has no `id`, `dataSource`,
   or `config`, so `IsDefined("request.wheels.tenant")` guards read an
   unresolved request as resolved.

The cache now lives under the reserved `request.wheels.$queryCache` sub-struct,
confining the blast radius to one key that no model name can reach. Caching
behaviour is otherwise unchanged: same per-model slots, same query keys, same
lifecycle.

Scoped deliberately to the finder cache. `Global.cfc`'s pagination handles are
also dynamic keys in `request.wheels`, but an app author has to actively name a
handle `tenant` to collide; the model-name key aliased automatically.

Regression coverage adds a `Tenant` fixture model mapped onto the existing
authors table (no populate.cfm changes) and covers both failure modes plus the
namespacing invariant.

Verified on Lucee 7 + SQLite: 4707 passed on develop, 4713 passed on this branch
(+6 new specs, no regressions). The Docker matrix could not run locally —
`tools/test-matrix.sh` fails to bind-mount box.json under Docker Desktop
virtiofs on this machine, and fails identically on clean develop.

Refs #3336

Signed-off-by: Peter Amiri <peter@alurium.com>
wheels-bot[bot]
wheels-bot Bot previously approved these changes Aug 2, 2026

@wheels-bot wheels-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wheels Bot — Reviewer

TL;DR — This PR namespaces the per-request finder cache under request.wheels.$queryCache[ModelName] instead of the bare request.wheels[ModelName], closing a case-insensitive struct-key collision where a model named Tenant aliased onto the framework-owned request.wheels.tenant. The change is a mechanical key-path swap plus an ensure-helper, the migration is complete, and it ships red-first regression tests, a changelog fragment, and doc updates. Verdict: approve.

Correctness

The fix is complete and self-consistent:

  • All four model-name cache sites migrated — three in vendor/wheels/model/read.cfc (read-hit check, read, write-back) and one in vendor/wheels/model/miscellaneous.cfc ($clearRequestCache()). A grep for request.wheels[variables.wheels.class.modelName] now returns zero hits.
  • The read-path access request.wheels["$queryCache"][variables.wheels.class.modelName] (read.cfc:323, 326) is safe: it is only reached when local.useRequestCache is true, and $ensureRequestQueryCache() is called unconditionally at the top of that same if (local.useRequestCache) block (read.cfc:314), so the sub-struct always exists first.
  • $ensureRequestQueryCache() is more defensive than the code it replaces — it now guards StructKeyExists(request, "wheels"), whereas the old read.cfc assumed request.wheels already existed.
  • The collision target is genuinely vacated: tenant() (Global.cfc:732) and $tenantDataSource() (Global.cfc:745) read request.wheels.tenant, which the cache no longer touches. Both referenced framework functions exist, so the specs exercise real code.

Cross-engine

No concerns. Constructs are matrix-safe by inspection: bracket-notation $queryCache keys (no member-function collision), no closures, no attributeCollection, no reserved-scope parameter names, no finally loops. The new $ensureRequestQueryCache() helper is public + $-prefixed (Cross-Engine Invariant 7) so it mixes into the model object correctly, and lives in wheels.model.*, outside the protectedControllerMethods surface. The mixed quoted/unquoted struct-literal keys in the tenant fixture struct ("$locked" = true) are valid on every engine. Noting for the record that the author disclosed the Docker matrix did not run locally (environmental virtiofs mount failure — it fails identically on clean develop); CI's compat matrix remains the real gate, but nothing in this diff reads as engine-sensitive.

Tests

Strong coverage. requestQueryCacheTenantCollisionSpec.cfc reproduces both reported failure modes (write-path $clearRequestCache() wiping tenant context; read-path finder fabricating a resolved tenant), plus a sibling-isolation spec and the namespacing invariant. The write-path spec correctly uses the real datasource with a no-match where to avoid leaving transaction state dirty for later specs — a nice touch. requestQueryCacheSpec.cfc's eight existing white-box assertions are repointed at the namespaced key, which is the right call since they assert on the exact structure being moved. The new Tenant fixture reuses c_o_r_e_authors, so populate.cfm is untouched.

Docs

Changelog fragment present and correctly named (changelog.d/3336-request-query-cache-namespace.fixed.md, fixed type). No direct CHANGELOG.md [Unreleased] edit. Blog post web/content/blog/posts/caching-in-wheels-4.md updated in both places that documented the old path.

Commits

fix(model): namespace per-request query cache under request.wheels.$queryCache — valid type, scoped, subject well under 100 chars, describes the why.

Non-blocking note

The two sibling collision families (pagination handles in Global.cfc:3847/3915; query-hash keys in callbacks.cfc:331) are explicitly and reasonably scoped out — the pagination one requires an app author to actively name a handle tenant, so deferring it is defensible. Likewise the two defensive-hardening items (TenantResolver stale-key delete, tenant() shape validation) change tenancy semantics rather than fix the collision; a follow-up issue for the pagination-handle surface would be worth filing so it is not lost.

@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Wheels Test Results

     32 files  +  1    9 716 suites  +28   20m 20s ⏱️ -48s
130 641 tests +280  130 166 ✅ +280  397 💤 ±0  42 ❌ ±0  36 🔥 ±0 
132 573 runs  +280  132 098 ✅ +280  397 💤 ±0  42 ❌ ±0  36 🔥 ±0 

For more details on these failures and errors, see this check.

Results for commit bc0b7b3. ± Comparison against base commit fc59492.

♻️ This comment has been updated with latest results.

@bpamiri

bpamiri commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

Cross-engine verification (manual workflow_dispatch)

compat-matrix.yml doesn't trigger on PRs, so I dispatched it against both this branch and clean develop and diffed the JUnit artifacts.

This PR's specs

The 6 new specs ran 168 times across 28 engine×DB legs (Lucee 6, Lucee 7, Adobe 2023, Adobe 2025, BoxLang × sqlite / h2 / mysql / postgres / sqlserver / cockroachdb) and passed every execution.

Testcase totals are exactly +6 on every leg vs develop, confirming they compiled and executed everywhere rather than being silently skipped.

No regressions

The set of failing spec names is identical between develop and this branch on all 23 legs that produced results:

Failures on branch but not develop none
Failures on develop but not branch none

Per-leg failure counts are unchanged (adobe2023 3–5, adobe2025 4–6, boxlang 3–4, lucee6 1–2, lucee7 0–1; adobe2023-oracle 11). Those are pre-existing — the leg debt tracked by #3302. The most frequent ones are HTML-encodes the path in the verb-mismatch message (17×), float() applies its default='' / allowNull=true outlier defaults (11×), is including partial with query grouped by a column (11×), and updateAll with multiple includes updates matching rows (5×, which is #3294).

Unrelated observation for #3302

Both runs report conclusion: success while all six engine jobs report failurecontinue-on-error: true means six red engines roll up green. Additionally the oracle legs on adobe2025 / boxlang / lucee6 emitted 0 testcases (engine never came up), which SOFT_FAIL_DBS="oracle" then absorbs. Two stacked layers of invisibility, exactly what #3302 describes.

Follow-on hardening for #3336, folded in at maintainer request. The
namespacing commit stops the framework from ever writing a query cache onto
`request.wheels.tenant`. These two changes stop anything else that lands there
from being read as a resolved tenant.

`tenant()` now requires a non-empty `dataSource` before treating the key as an
active tenant, which is the same test `$tenantDataSource()` already applies
before it routes a query. Previously it returned whatever occupied the key, so
a malformed value satisfied every `IsDefined("request.wheels.tenant")` or
truthiness guard in application code while carrying no usable routing
information — wrong behaviour rather than a no-op.

`TenantResolver` now deletes any pre-existing value on the key when its
resolver returns no match. Before, an unresolved request only looked
unresolved after the `finally` block ran, so a stale or foreign value could be
read as resolved for the whole downstream request.

The two agree by construction: both use the non-empty `dataSource` test, so
`tenant()` and `$tenantDataSource()` can no longer disagree about whether a
request is tenanted.

All four framework producers of the key — `switchTenant()` (throws without a
dataSource), `TenantResolver`, `Job.$restoreTenantContext()` and
`TenantMigrator` (both guard) — already guarantee a non-empty `dataSource`, so
correctly-resolved tenants see no change. Only foreign values are filtered.

Specs written red first: 4 new specs fail with these two changes reverted and
the specs kept, confirming they exercise the hardening rather than restating
existing behaviour.

Lucee 7 + SQLite full core suite: 4713 passed before, 4717 passed after, zero
failures.

Refs #3336

Signed-off-by: Peter Amiri <peter@alurium.com>

@wheels-bot wheels-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wheels Bot — Reviewer

TL;DR — This PR fixes #3336, where the per-request finder cache keyed itself on the bare model name directly in request.wheels, letting a model named Tenant alias onto the framework-owned request.wheels.tenant (CFML struct keys are case-insensitive). The fix namespaces the cache under a reserved request.wheels.$queryCache sub-struct and adds tenant-shape hardening in tenant() / TenantResolver. The change is well-scoped, red-first tested, and matrix-safe by inspection. I found no correctness, cross-engine, or security issues — only one docs nit (a stale PR description). Verdict: comment.

Correctness

The namespacing is complete and internally consistent. All four bare-model-name sites are migrated and every read of request.wheels["$queryCache"][modelName] is dominated by an $ensureRequestQueryCache() call:

  • vendor/wheels/model/read.cfc:314 calls the ensure helper inside the local.useRequestCache guard, and the subsequent reads at :324/:326 and the store at :358 are all short-circuited behind local.useRequestCache — no unguarded key access.
  • vendor/wheels/model/miscellaneous.cfc:27 (\$clearRequestCache()) calls the ensure helper first, so it no longer assumes request.wheels exists.

A repo-wide grep for request.wheels[ confirms only the two documented out-of-scope families remain (pagination handles in Global.cfc:3859/3927, the query-hash map in callbacks.cfc:331) — no dangling reader of the old bare-model-name path.

The tenant() hardening in Global.cfc:766 now gates on a non-empty dataSource, matching \$tenantDataSource() (Global.cfc:758) exactly, so the two agree on what "resolved" means. Every framework producer (switchTenant() throws on empty dataSource at Global.cfc:778; TenantResolver.handle() requires Len(dataSource) at :57) already guarantees a non-empty dataSource, so no legitimately-resolved tenant regresses.

TenantResolver.cfc:76-81 correctly drops a stale/foreign request.wheels.tenant on a no-match, guarded by StructKeyExists(request, "wheels").

Cross-engine

Clean. The new code uses only IsDefined, IsStruct, StructKeyExists, StructDelete, Len, and bracket-notation struct keys — no closures-as-constructor-args, no attributeCollection, no reserved-scope params, no finally loops, no Left(str,0), no obj.map(). The new \$ensureRequestQueryCache() is public + \$-prefixed per cross-engine invariant #7, so it integrates via \$integrateComponents(), and it lives in wheels.model.* (outside the protectedControllerMethods surface), so it can't shadow a controller action.

Tests

Strong coverage. requestQueryCacheTenantCollisionSpec.cfc exercises both reported failure modes (write-path erasure via \$clearRequestCache() and a real updateAll(); read-path fabrication via a Tenant finder) plus the namespacing invariant and per-model isolation. requestQueryCacheSpec.cfc's 8 white-box assertions are correctly repointed at the namespaced key. MultiTenantSpec.cfc and TenantResolverSpec.cfc gain matching hardening specs that follow the existing pipeline/closure-scope idioms in those files. The new Tenant.cfc fixture reuses c_o_r_e_authors, so populate.cfm is untouched.

Docs

  • Nit — stale PR description. The PR body's "Also not included" section states the two defensive-hardening items (tenant() shape validation, TenantResolver stale-tenant deletion) are not in this PR, but commit e689e193b ("harden tenant context against malformed values") ships exactly those, and changelog.d/3336-tenant-shape-hardening.fixed.md documents them. Worth reconciling the description before merge so reviewers/changelog-readers aren't misled — no code change needed.

Changelog fragments (changelog.d/*.fixed.md) use the correct fragment mechanism rather than a direct CHANGELOG.md edit, and the published blog post's two request.wheels[ModelName] references are updated.

Commits

Both commits conform to commitlint.config.jsfix(model): …, subject non-empty, not ALL-CAPS, ≤100 chars.

Nice work — the red-first tests and the deliberately semantics-free key-path swap (avoiding the by-reference StructClear() draft that would have leaned on Adobe struct-reference semantics) are exactly right.

… StructKeyExists

Caught by the compat matrix on Adobe 2025 — every TenantResolver "no match" spec
errored with `Element WHEELS is undefined in REQUEST` across all five of its
database legs. Lucee 6, Lucee 7, Adobe 2023 and BoxLang were unaffected, so the
local suite and PR CI both reported clean.

`handle()` takes a parameter named `request`. On Adobe 2025 the bare `request`
token does not resolve consistently between the two positions used by the guard
added in the previous commit: passed as an argument to
`StructKeyExists(request, "wheels")` it saw the key and returned true, but the
`request.wheels` member-access expression in the guarded `StructDelete` then
resolved somewhere without it and threw.

Switch the guard to `IsDefined("request.wheels.tenant")`, which string-resolves
the whole dotted path in a single evaluation and therefore cannot disagree with
itself. This is the same guard the `finally` block in this function already
uses, so it is proven on Adobe 2025 rather than merely plausible.

Behaviour is unchanged: deleting a key that isn't there was already a no-op, so
narrowing the guard from "request.wheels exists" to "request.wheels.tenant
exists" drops no work.

Lucee 7 + SQLite full core suite: 4717 passed, 0 failed.

Refs #3336

Signed-off-by: Peter Amiri <peter@alurium.com>

@wheels-bot wheels-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wheels Bot — Reviewer

TL;DR — This PR fixes #3336: the per-request finder cache keyed on the bare model name in request.wheels, so a model named Tenant aliased onto the framework-owned request.wheels.tenant (CFML struct keys are case-insensitive). The fix namespaces the cache under a reserved request.wheels.$queryCache sub-struct, adds shape-hardening in tenant() / TenantResolver, and the new head commit bc0b7b3e3 narrows a no-match guard that the Adobe 2025 compat leg had surfaced as a real error. I found no correctness, cross-engine, or security issues — only one optional docs nit. Verdict: comment.

Correctness

The namespacing is complete and self-consistent. Every read of request.wheels["$queryCache"][modelName] is dominated by an $ensureRequestQueryCache() call:

  • vendor/wheels/model/read.cfc:314 calls the ensure helper at the top of the local.useRequestCache block; the reads at :324/:326 and the store at :358 are all gated behind the same local.useRequestCache, so no unguarded key access.
  • vendor/wheels/model/miscellaneous.cfc:27 ($clearRequestCache()) calls the ensure helper first, so it no longer assumes request.wheels pre-exists.
  • A repo-wide grep for request.wheels[variables.wheels.class.modelName] now returns zero hits — all four bare-model-name sites are migrated, and the two out-of-scope families (Global.cfc pagination handles, the callbacks.cfc:331 query-hash map) are correctly left for #3339.

The new-head fix is right. Previously the no-match branch was else if (StructKeyExists(request, "wheels")) and then StructDelete(request.wheels, "tenant"). Because handle() declares a parameter named request (a reserved scope — CLAUDE.md anti-pattern #11), Adobe 2025 resolved the bare request inconsistently between the StructKeyExists argument and the request.wheels member-access, throwing Element WHEELS is undefined in REQUEST. Switching to IsDefined("request.wheels.tenant") (TenantResolver.cfc:76) string-resolves the whole dotted path in one evaluation — and it is the same guard the finally block at :95 already uses, so it is proven on Adobe 2025 rather than merely plausible. The narrowing is behaviour-preserving: IsDefined("request.wheels.tenant") being true implies request.wheels exists, so the guarded StructDelete(request.wheels, "tenant") at :88 is safe, and deleting an absent key was already a no-op.

The tenant() hardening at Global.cfc:739-748 now gates on IsStruct + StructKeyExists(...,"dataSource") + Len(dataSource), matching $tenantDataSource() (Global.cfc:757-764) exactly, so the two agree on what "resolved" means. Every framework producer already guarantees a non-empty dataSource (switchTenant() throws at :778; TenantResolver.handle() requires Len(dataSource) at :57), so no legitimately-resolved tenant regresses.

Cross-engine

No concerns. The changed constructs are all matrix-safe: bracket-notation ["$queryCache"] keys (no .map()-style member collision), IsDefined / StructKeyExists / StructDelete, no closures on the hot path, no attributeCollection, no finally loops. $ensureRequestQueryCache() is public + $-prefixed per Cross-Engine Invariant 7, so it integrates via $integrateComponents(), and lives in wheels.model.* (outside protectedControllerMethods). The reserved-scope hazard that the flat guard hit is exactly what the new commit resolves.

Tests

Well-covered and red-first. TenantResolverSpec.cfc:296 ("drops a pre-existing request.wheels.tenant when the resolver finds no match") genuinely exercises the new else if branch, not the finally: the handler runs inside try before finally, and it observes IsDefined("request.wheels.tenant") as false — which can only be true if the pre-try delete ran. requestQueryCacheTenantCollisionSpec.cfc covers both failure modes plus per-model isolation, backed by a Tenant fixture mapped onto the existing authors table (no populate.cfm change). MultiTenantSpec.cfc adds the shape-hardening cases and has an afterEach that clears request.wheels.tenant, so no state leaks.

Docs

Optional nit only: the top-of-function comment at TenantResolver.cfc:48-52 asserts "bare request inside a function always refers to the built-in request scope, even when a parameter is named request," while the new comment at :81-87 documents the Adobe 2025 case where bare request did not resolve consistently. The two read as being in tension. Consider scoping the older comment ("on most engines") or cross-referencing the #3336 exception, so a future reader does not trust the absolute wording. Non-blocking.

Changelog fragments (changelog.d/3336-*.fixed.md) and the blog-post update are handled correctly (no direct CHANGELOG.md edit). Commit messages conform to commitlint. Cross-engine matrix limitation is disclosed and was in fact manually dispatched — which is how the Adobe 2025 issue was caught and fixed.

…nt 15

Documents the cross-engine bug the compat matrix caught in bc0b7b3, so the next
person writing middleware doesn't rediscover it the same way.

In a function that declares a parameter named `request`, Adobe CF 2025 does not
resolve the bare `request` token consistently across expression positions — it
can mean the built-in scope as a function argument and `arguments.request` in a
member-access expression within the same function. A guard written one way
therefore cannot protect an access written the other way.

This is not a niche shape: `wheels.middleware.MiddlewareInterface` mandates
`handle(required struct request, required any next)`, so every middleware
component declares the parameter and is exposed. Anti-pattern 11's advice —
never name a parameter after a reserved scope — is unavailable there, so the
two safe forms (IsDefined on the full dotted path, or assign-before-use) are
the only options and must not be mixed with a bare-token guard.

Adds CLAUDE.md cross-engine invariant 15 and a matching deep-reference entry in
.ai/wheels/cross-engine-compatibility.md, both noting that Lucee 6/7, BoxLang
and Adobe 2023 resolve consistently — so a green local run and green Adobe 2023
smokes do not cover this, and compat-matrix.yml does not run on PRs.

Also rewrites the misleading note at the top of TenantResolver.handle(), which
asserted that bare `request` "always refers to the built-in request scope, even
when a parameter is named `request`". That is true on Lucee and Adobe 2023 and
false on Adobe 2025, and it is the assumption that produced the bug.

Audited the rest of the framework while here: no other function that declares a
`request` parameter pairs a `StructKeyExists(request, ...)` guard with a
`request.x` access. The two existing sites (TenantResolver.handle,
RequestId.handle) both use the safe assign-before-use form.

Docs and comments only — no behaviour change. Lucee 7 + SQLite full core suite:
4717 passed, 0 failed.

Refs #3336

Signed-off-by: Peter Amiri <peter@alurium.com>
@bpamiri

bpamiri commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

Adobe 2025 regression, caught and fixed

The StructKeyExists(request, "wheels") guard added in e689e19 errored on all five Adobe 2025 database legs — 8 TenantResolver specs each — with Element WHEELS is undefined in REQUEST. Every other engine was green, so local Lucee 7 and PR CI both reported clean.

Cause: handle() declares a parameter named request (the MiddlewareInterface signature mandates it), and on Adobe 2025 the bare request token doesn't resolve consistently across expression positions — StructKeyExists(request, "wheels") returned true while the request.wheels member-access in the guarded StructDelete resolved somewhere without the key.

Fixed in bc0b7b3 by switching to IsDefined("request.wheels.tenant"), the same guard the finally block in this function already uses — so it's proven on Adobe 2025, not merely plausible. Behaviour is unchanged: deleting an absent key was already a no-op.

Matrix re-run on bc0b7b3 vs the develop baseline

The PR's 10 specs 280/280 passing across 28 engine×DB legs
Testcase delta vs develop exactly +10 on every leg
Failures on branch but not develop none

All five Adobe 2025 legs now match develop exactly (6/6, 5/5, 4/4, 4/4, 4/4). Remaining per-leg failures are unchanged pre-existing #3302 debt.

Documented as invariant 15 (8d9e20a)

Added to CLAUDE.md and .ai/wheels/cross-engine-compatibility.md, since every middleware component declares a request parameter and is exposed to this. Anti-pattern 11 ("never name a parameter after a reserved scope") can't apply there — the interface mandates the name — so the two safe forms are the only options:

// safe (a) — one evaluation of the whole dotted path
if (IsDefined("request.wheels.tenant")) { StructDelete(request.wheels, "tenant"); }

// safe (b) — assign before use
if (!StructKeyExists(request, "wheels")) { request.wheels = {}; }
request.wheels.tenant = local.tenant;

Also rewrote the note at the top of TenantResolver.handle(), which asserted bare request "always refers to the built-in request scope, even when a parameter is named request" — true on Lucee and Adobe 2023, false on Adobe 2025, and the assumption that produced this bug.

Audited the rest of the framework: no other function declaring a request parameter pairs a StructKeyExists(request, …) guard with a request.x access. The two existing sites (TenantResolver.handle, RequestId.handle) both use safe form (b).

@wheels-bot wheels-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wheels Bot — Reviewer

TL;DR — This PR fixes #3336: the per-request finder cache keyed on the bare model name directly in request.wheels, so a case-insensitive struct-key collision let a model named Tenant alias onto framework-owned request.wheels.tenant — silently wiping (write path) or fabricating (read path) tenant context. The fix namespaces the cache under request.wheels.$queryCache[ModelName], and two folded-in commits harden tenant() and TenantResolver so a malformed value on the key degrades to a no-op instead of wrong behaviour. Every source change is correct, cross-engine-safe, and backed by a dedicated red-first spec. Verdict: approve.

Correctness

Verified each of the three behavioural changes against the current source:

  • model/read.cfc:314 calls $ensureRequestQueryCache() inside the if (local.useRequestCache) block, so the request.wheels["$queryCache"][modelName] path is guaranteed to exist before the reads at read.cfc:324/:326 and the write at :358 — all three gated on the same local.useRequestCache. No unguarded deref.
  • model/miscellaneous.cfc:170$ensureRequestQueryCache() builds the namespace defensively (request.wheels then $queryCache then model slot), and $clearRequestCache() resets only its own model's slot, matching the "clears only its own model's cache" spec.
  • Global.cfc:739 tenant() now requires IsDefined + IsStruct + a non-empty dataSource, exactly the $tenantDataSource() predicate — the "agrees with $tenantDataSource()" spec pins the two together.
  • TenantResolver.handle() line 86 else-if drops a pre-existing/foreign value on a no-match before next(), so the request looks unresolved for its whole duration, not just after the finally — covered by the new "drops a pre-existing request.wheels.tenant" spec.

Cross-engine

Clean, and notably self-aware: the no-match delete at TenantResolver.cfc:86 uses IsDefined("request.wheels.tenant") rather than the StructKeyExists(request, "wheels") + request.wheels member-access shape this very PR documents as broken on Adobe 2025 (CLAUDE.md invariant 15 / .ai/wheels/cross-engine-compatibility.md). $ensureRequestQueryCache() is public + $-prefixed per invariant 7 and lives in wheels.model.*, so it mixes in via $integrateComponents() and stays outside the protectedControllerMethods surface. No closures-as-constructor-args, no attributeCollection, no Left(str,0), no finally loops.

Tests

requestQueryCacheTenantCollisionSpec.cfc (6 specs) covers both failure modes plus the namespacing invariant and sibling-isolation; MultiTenantSpec.cfc and TenantResolverSpec.cfc gain the hardening cases; requestQueryCacheSpec.cfc's white-box assertions are repointed at the new key. All BDD, extending wheels.WheelsTest. The Tenant fixture reuses c_o_r_e_authors so populate.cfm is untouched. The one caveat the author discloses honestly: compat-matrix.yml does not run on PRs, so the Adobe 2025 legs that would exercise invariant 15 are not gated here — the manual dispatch link is provided.

Docs

Changelog fragments present and correctly typed (changelog.d/3336-*.fixed.md), CLAUDE.md invariant 15 and the deep-reference entry added, blog post caching-in-wheels-4.md updated. No direct CHANGELOG.md [Unreleased] edit.

Commits

All four conform to commitlint.config.js (fix(model), fix(middleware), docs), subjects under 100 chars, bodies explain the why (the invariant-15 commit body is exemplary).

Nice work — thorough, well-scoped, and the deferred-collision surfaces (pagination handles to #3339, callback query hash no-collide) are correctly triaged out rather than bundled in.

@bpamiri
bpamiri merged commit af69eb5 into develop Aug 3, 2026
17 checks passed
@bpamiri
bpamiri deleted the fix/3336-namespace-request-query-cache branch August 3, 2026 04:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Per-request finder cache key collides with request.wheels.tenant for a model named Tenant (silently drops tenant datasource routing)

1 participant