fix(job): name the queue row when a persisted jobClass cannot be resolved - #3358
Conversation
…lved
`enqueue()` persists `GetMetadata(this).name` into `wheels_jobs.jobClass`, and the drain
re-instantiates with `CreateObject("component", jobRow.jobClass)`. A string produced by engine
metadata is stored and later resolved as a component path, so the round trip is only safe if
that string keeps the casing of the file on disk — component paths are case-sensitive on Linux
and not on macOS or Windows. That is a bug shape which passes in development and fails on a
production redeploy, on rows the old instance wrote and the new one drains.
I could NOT reproduce a casing drift. Probed on Lucee 7: `GetMetadata().name` comes back
canonical (`...jobs.ProbeJob`) even when the component is instantiated through a lowercase
path, so Lucee derives it from the file rather than echoing what the caller typed. That
explains why this has never bitten — it is a property of the engine, not luck. It is also not
a guarantee across the other four engines, and the issue is explicit that it is unverified
there.
So rather than guess at a normalisation fix for a drift that may not exist, this pins the
invariant as a test and improves the failure when it does not hold.
JobClassRoundTripSpec asserts, case-sensitively, that the metadata name's last segment equals
the .cfc file name; that the name is identical however the component was instantiated (a
case-insensitive filesystem resolves both spellings, so an engine that echoed the caller's
path would persist whatever casing was typed); and that a job re-instantiates from its own
persisted name. Those run on every engine × database leg, so lucee6, adobe2023, adobe2025 and
boxlang each answer the open question directly instead of being assumed safe.
$instantiateJobClass() replaces the bare CreateObject on both processing paths
(Job.$processJob and JobWorker.$executeJob) and throws Wheels.JobClassNotFound naming the
class, the queue row id, and the three real causes — casing, rename, delete. The issue's point
is that the raw error is `component not found` for a class that plainly exists, which sends
investigators to mappings and deployment; the fix is to describe the actual shape of the
problem, a string read out of a queue row.
It also throws Wheels.InvalidJobClass when the path resolves to a component with no perform().
That narrows but does NOT close the database-string-to-CreateObject shape the issue flags —
the real guarantee is still that only $enqueueJob writes that column, and the docstring says
so rather than implying the check is a security boundary.
JobWorker.$scheduleRetry's CreateObject is deliberately left alone: it is a best-effort backoff
lookup already wrapped in a try/catch that falls back to defaults, and it must not start
throwing.
Red-first, with Job.cfc and JobWorker.cfc reverted: 4734 pass / 2 fail / 1 error. The three
invariant specs pass on Lucee 7 — they are meant to, on an engine that holds the invariant.
Verification, lucee7 + sqlite, full core suite:
develop ab901cf 4732 pass / 0 fail / 0 error
this branch 4737 pass / 0 fail / 0 error
Exactly +5, the new specs.
Closes #3351
Signed-off-by: Peter Amiri <peter@alurium.com>
![wheels-bot[bot]](https://avatars.githubusercontent.com/in/3657588?s=60&v=4){kind=small}
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR — This PR (#3351) addresses an unresolvable persisted jobClass by (a) pinning the metadata-name round-trip invariant across every engine with JobClassRoundTripSpec, and (b) replacing the bare CreateObject on both drain paths with $instantiateJobClass(), which throws a self-describing Wheels.JobClassNotFound / Wheels.InvalidJobClass instead of the engine's component not found`. The framework change is correct, cross-engine-safe, and red-first tested. My verdict is comment — the only issue is 8 unrelated generated TestBox mock stubs that appear to have been committed by accident.
Docs
- 8 stray MockBox stub artifacts unrelated to this fix. The PR adds 8 files under
public/testbox/system/stubs/(139184C0...,16CA3942...,30942F4D...,754BEF48...,B7681C49...,C383511C...,CDCB73D6..., plus one more). These are MockBox-generated mock-decorator caches for SSE/channels helpers (closeSSEStream,sendSSEComment,sendSSEEvent,initSSEStream,poll,checkError,$getChannelEngine) — none of them touch jobs. I confirmed they do not exist onorigin/develop(git show origin/develop:public/testbox/system/stubs/139184C0543CCDB338AEFB643110CB89.cfmreports "exists on disk, but not in origin/develop"), so this PR introduces all 8. The PR body itself says "Exactly +5 — the new specs, nothing else moved," which is inconsistent with committing these. Please drop them from the diff. There is already one such file tracked ondevelop(F952D54F...cfm); rather than accreting more, consider.gitignore-ingpublic/testbox/system/stubs/so generated mock caches stop landing in commits.
Correctness
No issues. $instantiateJobClass() (vendor/wheels/Job.cfc:287) wraps only the CreateObject in try/catch, re-throws with Throw(type="Wheels.JobClassNotFound", ...), then guards the result with StructKeyExists(local.rv, "perform") before returning — the same instance-method-existence idiom already used a few lines down (StructKeyExists(local.jobInstance, "baseDelay"), Job.cfc:361). Both throws land inside the existing $processJob / $executeJob try blocks, so control flow (retry/backoff/permanent-fail) is unchanged; only the logged message improves. JobWorker.$scheduleRetry's best-effort CreateObject is correctly left untouched.
Cross-engine
No issues. $instantiateJobClass is public with a $ prefix, satisfying the mixin-integration rule (Cross-Engine Invariant 7). No map(), application-scope function members, bracket-notation calls, reserved-scope parameters, or in-finally loops. JobWorker.$executeJob correctly reaches the shared helper through the lazy $jobBridge() singleton (JobWorker.cfc:641), which returns a new wheels.Job().
Tests
Solid and red-first. JobClassRoundTripSpec uses case-sensitive Compare() (not CompareNoCase()) to pin the canonical-name invariant, and covers both error paths (Wheels.JobClassNotFound naming class + row id, Wheels.InvalidJobClass for a perform()-less component). I verified the fixtures the specs depend on: ProbeJob.cfc exists with a perform() method, and wheels.tests._assets.models.Post has no perform() (no perform anywhere in Model.cfc/model/), so the InvalidJobClass assertion is sound.
Commits
fix(job): name the queue row when a persisted jobClass cannot be resolved — valid type/scope, subject well under 100 chars, describes the "why." Conforms to commitlint.config.js.
Wheels Test Results 31 files 9 800 suites 19m 44s ⏱️ For more details on these failures and errors, see this check. Results for commit 576bbbb. ♻️ This comment has been updated with latest results. |
The compat matrix run for this branch reported tests="0" on EVERY database for adobe2023 and
adobe2025 while lucee6, lucee7 and boxlang were all clean at +5. That is the compile-error
signature: one bad spec file zeroes the entire engine leg, because the core suite compiles
via directory="wheels.tests.specs".
From the adobe2023 artifact, not inferred:
coldfusion.compiler.CFMLParserBase$MissingNameException:
Invalid construct: Either argument or name is missing.
snippet: describe("Tests that the persisted jobClass round-trips", () => {
TEMPLATE: .../specs/jobs/JobClassRoundTripSpec.cfc LINE 17 TYPE SYNTAX
JobClassRoundTripSpec called `(new wheels.Job()).$instantiateJobClass(...)` — a parenthesized
`new` in RECEIVER position. Adobe rejects it and, as cross-engine invariant 16 warns, blames
the enclosing describe() line rather than the offending statement, so it reads like a broken
test-block signature.
Hoisting the instance to a variable is the fix. That form has 22 existing spec files as
precedent — `adapter.$getType(type = "boolean")` in CockroachDBUnitSpec is the same shape,
variable receiver with named arguments — and those files compile on the Adobe legs today.
CLAUDE.md invariant 16 is widened rather than left to be rediscovered. It documented only the
`application`-scope zero-argument form on Adobe 2025; this is a second shape in the same
MissingNameException family, it fails on Adobe 2023 as well, and named arguments do not save
it because the receiver is what the parser chokes on. Both are now written up as 16a/16b, with
the note that `(new X()).method()` is fine in application code that only runs on Lucee — it
appears in this file's own Background Jobs examples — and fatal in the core spec suite, which
compiles on all five engines.
Local Adobe verification was NOT possible: the adobe2023 container will not start on this
machine (`runc create failed ... error during container init`, reported as `engine-down` by
tools/test-matrix.sh), from both a worktree and the main checkout. So this rests on the
artifact root cause plus the in-repo precedent above, and the re-dispatched matrix is the
check.
lucee7 + sqlite, full core suite: 4737 pass / 0 fail / 0 error, unchanged from before the
hoist.
Refs #3351
Signed-off-by: Peter Amiri <peter@alurium.com>
github-actions
Bot
added
the
dependencies
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR — This PR (#3351) makes an unresolvable persisted jobClass fail loudly: $instantiateJobClass() replaces the bare CreateObject on both drain paths (Job.$processJob, JobWorker.$executeJob) and throws a self-describing Wheels.JobClassNotFound / Wheels.InvalidJobClass instead of the engine's bare component not found, and JobClassRoundTripSpec pins the metadata-name round-trip invariant across every engine. The framework change is correct, cross-engine-safe, and red-first tested. The head commit (9c369c9c4) also correctly resolves an Adobe compile break in the new spec. Verdict is comment — the only issues are stray generated/empty artifacts committed by accident. No blocking findings.
Correctness
No issues. $instantiateJobClass() (vendor/wheels/Job.cfc:283) wraps only the CreateObject in try/catch, re-throws Wheels.JobClassNotFound with extendedInfo naming the persisted string and the three real causes, then guards the result with StructKeyExists(local.rv, "perform") before returning — the same instance-method-existence idiom used a few lines down (StructKeyExists(local.jobInstance, "baseDelay"), Job.cfc:361). Both throws land inside the existing try blocks in $processJob / $executeJob, so retry/backoff/permanent-fail control flow is unchanged; only the message improves. JobWorker.$executeJob reaches the shared helper through the lazy $jobBridge() singleton (JobWorker.cfc:641-645, new wheels.Job()), and $scheduleRetry's best-effort CreateObject is correctly left untouched.
Cross-engine
No issues, and the head commit fixed one. The original spec called (new wheels.Job()).$instantiateJobClass(...) — a parenthesized new in receiver position — which Adobe 2023/2025 reject at compile time (MissingNameException), zeroing the whole engine leg. 9c369c9c4 hoists the receiver to a bridge variable (JobClassRoundTripSpec.cfc:50,62,81), which matches the 22-file precedent, and widens CLAUDE.md invariant 16 into 16a/16b to document the second shape. $instantiateJobClass is public with a $ prefix (mixin-integration rule, Invariant 7). No map(), application-scope function members, reserved-scope params, or in-finally loops.
Tests
Solid and red-first. JobClassRoundTripSpec uses case-sensitive Compare() (not CompareNoCase()) for the canonical-name invariant and covers both error paths. Fixtures verified: ProbeJob.cfc has a perform(); perform appears only in Job.cfc, the job assets, and McpServer.cfc — never in Model.cfc/model/, so wheels.tests._assets.models.Post genuinely has no perform() and the InvalidJobClass assertion (:85,90) is sound.
Docs
Two stray artifacts unrelated to the fix should be dropped from the diff:
-
Empty
box.jsonat repo root (NEW in9c369c9c4).git show origin/develop:box.json→ "exists on disk, but not in origin/develop", and the blob is the empty-file hash (e69de29bb, 0 bytes). An empty file is not valid JSON, so any CommandBox/tooling that reads it could error. This looks accidental — please remove it. -
8 MockBox stub caches under
public/testbox/system/stubs/(139184C0…,16CA3942…,30942F4D…,754BEF48…,B7681C49…,C383511C…,CDCB73D6…, plus one more). These are generated mock-decorator caches for SSE/channels helpers — none touch jobs — and none exist onorigin/develop. The PR body says "Exactly +5 — the new specs, nothing else moved," which is inconsistent with committing these. Please drop them, and consider.gitignore-ingpublic/testbox/system/stubs/so generated caches stop landing in commits (one,F952D54F…, is already tracked ondevelop).
Neither is a correctness or runtime concern; both are noise in the tree.
Commits
Both commits conform to commitlint.config.js: fix(job): … and fix(test): … — valid types/scopes, subjects well under 100 chars, describing the "why," DCO-signed. The changelog fragment (changelog.d/3351-jobclass-round-trip.fixed.md) is present and correctly typed.
The re-dispatched matrix confirmed the parser fix — adobe2023 compiles again, +5 tests on
every database — but surfaced a second, more interesting problem: one new ERROR per database
on Adobe.
Error | reports the same metadata name however the component was instantiated
| Could not find the ColdFusion component or interface
wheels.tests._assets.jobs.probejob.
Adobe's component resolver is CASE-SENSITIVE independently of the filesystem. A miscased path
does not resolve on macOS either, where the filesystem happily would. My spec instantiated
through a deliberately lowercase path to prove the engine does not echo the caller's casing
back into the persisted name — an assumption that only holds where the miscased path is
constructible at all.
So the spec was failing on Adobe for a reason *safer* than the one it was testing. Two engine
families close the same hole differently:
Lucee / BoxLang a miscased path RESOLVES, but the metadata name comes back canonical
Adobe a miscased path does not resolve at all — nothing can be persisted
because nothing can be constructed
The spec now asserts the property both satisfy: a caller's miscasing cannot reach
`wheels_jobs.jobClass`. Following cross-engine invariant 11, the flag lives on a struct set in
the try rather than a local set in the catch, so it survives on BoxLang.
This is the answer to the open question in the issue, which is the reason these specs run on
every leg instead of being assumed: the round trip is safe on all five engines, for two
different reasons, neither of which was verified before.
lucee7 + sqlite, full core suite: 4737 pass / 0 fail / 0 error.
Refs #3351
Signed-off-by: Peter Amiri <peter@alurium.com>
a7baa46 to
576bbbb
Compare
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR — This PR (issue 3351) makes an unresolvable persisted jobClass fail with a diagnostic instead of the engine bare component not found, by routing both drain paths through a new Job.$instantiateJobClass() helper (typed Wheels.JobClassNotFound / Wheels.InvalidJobClass errors plus a perform() guard), and pins the enqueue-to-drain round-trip invariant with JobClassRoundTripSpec on every engine leg. The framework change is correct, well-tested, cross-engine-careful, and honest about what it does and does not close. My only findings are two accidental artifacts checked into the diff, unrelated to the fix. Verdict: comment — no blocking issues; please strip the artifacts before merge.
What is right (kept short)
$instantiateJobClass(vendor/wheels/Job.cfc:281) ispublicand reached fromJobWorker.$executeJobvia the pre-existing$jobBridge()lazy-instance pattern (vendor/wheels/JobWorker.cfc:454,:641) — consistent with$restoreTenantContext/$ensureJobTableon the same bridge.$scheduleRetrybest-effortCreateObject(vendor/wheels/JobWorker.cfc:516) is correctly left inside its defaults-fallbacktry/catch— it must not start throwing, and the PR calls this out explicitly.- The new throw sits inside the same
trythat already wrapped the oldCreateObjecton both paths, so no previously-caught failure becomes uncaught. - Spec follows cross-engine invariant 11 (flag on a
structset in thetry, emptycatch,JobClassRoundTripSpec.cfc:47-54) and invariant 16a (hoistedbridge = new wheels.Job()receiver,:73), and CLAUDE.md invariant 16 is widened to 16a/16b to document the second MissingNameException shape.
Docs
box.json— a new, empty (0-byte) file is added at the repo root (introduced in9c369c9c4). It is not ondevelopand there is no.gitignoreentry for it. An emptybox.jsonis not a valid CommandBox descriptor and can trip JSON-parsing tooling; it is unrelated to the jobClass fix. Pleasegit rm box.jsonbefore merge.- Seven MockBox stub artifacts under
public/testbox/system/stubs/(139184C0…,16CA3942…,30942F4D…,754BEF48…,B7681C49…,C383511C…,CDCB73D6…) are hash-named test-run artifacts, not source.developalready tracks a few of these (e.g.F952D54F…) because the directory is not gitignored, so this is a pre-existing leak the PR widens rather than a new anti-pattern — but they are noise in a fix diff and should be dropped (git rmthe seven, and ideally addpublic/testbox/system/stubs/to.gitignorein a follow-up).
Tests
JobClassRoundTripSpecis a proper BDD spec extendingwheels.WheelsTest, covers the round-trip happy path plus both error types (Wheels.JobClassNotFound,Wheels.InvalidJobClass), and its fixtures check out:ProbeJobhasperform()andwheels.tests._assets.models.Postdeliberately does not, so the InvalidJobClass assertion exercises the real branch. Red-first result (4734 pass / 2 fail / 1 error with the fix reverted) is documented.
Commits
- All three headers are valid conventional commits within the length limit (
fix(job):,fix(test):,test(job):), messages explain the why, and each carries a DCO Signed-off-by. No commitlint concern.
Nice work — pinning the invariant on every leg rather than guessing at a normalization is the right call, and the failure-message rewrite targets the actual investigator pain. Just clear out box.json and the seven stub files.
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR — This PR (issue 3351) makes an unresolvable persisted jobClass fail with a diagnostic instead of the engine bare component not found, by routing both drain paths through a new Job.$instantiateJobClass() helper (typed Wheels.JobClassNotFound / Wheels.InvalidJobClass errors plus a perform() guard), and pins the enqueue-to-drain round-trip invariant with JobClassRoundTripSpec on every engine leg. The framework change is correct, well-tested, cross-engine-careful, and honest about what it does and does not close. My only findings are two accidental artifacts checked into the diff, unrelated to the fix. Verdict: comment — no blocking issues; please strip the artifacts before merge.
What is right (kept short)
$instantiateJobClass(vendor/wheels/Job.cfc:281) ispublicand reached fromJobWorker.$executeJobvia the pre-existing$jobBridge()lazy-instance pattern (vendor/wheels/JobWorker.cfc:454,:641) — consistent with$restoreTenantContext/$ensureJobTableon the same bridge.$scheduleRetrybest-effortCreateObject(vendor/wheels/JobWorker.cfc:516) is correctly left inside its defaults-fallbacktry/catch— it must not start throwing, and the PR calls this out explicitly.- The new throw sits inside the same
trythat already wrapped the oldCreateObjecton both paths, so no previously-caught failure becomes uncaught. - Spec follows cross-engine invariant 11 (flag on a
structset in thetry, emptycatch,JobClassRoundTripSpec.cfc:47-54) and invariant 16a (hoistedbridge = new wheels.Job()receiver,:73), and CLAUDE.md invariant 16 is widened to 16a/16b to document the second MissingNameException shape.
Docs
box.json— a new, empty (0-byte) file is added at the repo root (introduced in9c369c9c4). It is not ondevelopand there is no.gitignoreentry for it. An emptybox.jsonis not a valid CommandBox descriptor and can trip JSON-parsing tooling; it is unrelated to the jobClass fix. Pleasegit rm box.jsonbefore merge.- Seven MockBox stub artifacts under
public/testbox/system/stubs/(139184C0…,16CA3942…,30942F4D…,754BEF48…,B7681C49…,C383511C…,CDCB73D6…) are hash-named test-run artifacts, not source.developalready tracks a few of these (e.g.F952D54F…) because the directory is not gitignored, so this is a pre-existing leak the PR widens rather than a new anti-pattern — but they are noise in a fix diff and should be dropped (git rmthe seven, and ideally addpublic/testbox/system/stubs/to.gitignorein a follow-up).
Tests
JobClassRoundTripSpecis a proper BDD spec extendingwheels.WheelsTest, covers the round-trip happy path plus both error types (Wheels.JobClassNotFound,Wheels.InvalidJobClass), and its fixtures check out:ProbeJobhasperform()andwheels.tests._assets.models.Postdeliberately does not, so the InvalidJobClass assertion exercises the real branch. Red-first result (4734 pass / 2 fail / 1 error with the fix reverted) is documented.
Commits
- All three headers are valid conventional commits within the length limit (
fix(job):,fix(test):,test(job):), messages explain the why, and each carries a DCO Signed-off-by. No commitlint concern.
Nice work — pinning the invariant on every leg rather than guessing at a normalization is the right call, and the failure-message rewrite targets the actual investigator pain. Just clear out box.json and the seven stub files.
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR — This PR (#3351) makes an unresolvable persisted jobClass fail with a diagnostic instead of the engine's bare component not found, by routing both drain paths through a new $instantiateJobClass() that throws Wheels.JobClassNotFound / Wheels.InvalidJobClass, and pins the metadata round-trip invariant across every engine with JobClassRoundTripSpec. The change is correct, honestly scoped, cross-engine-aware, and well tested. Verdict: comment — clean diff, one non-blocking convention nit.
Correctness
The new throws are safe on both processing paths. In Job.$processJob, $instantiateJobClass() runs inside the existing try (vendor/wheels/Job.cfc:360) whose catch turns a failure into a retry/dead-letter — so an unresolvable class remains a job failure, never a batch abort. local.hasTenantContext = false is set before the try (Job.cfc:350), so the post-catch tenant cleanup can't read an undefined var. JobWorker.$executeJob mirrors this: the call is inside the try (vendor/wheels/JobWorker.cfc:454) and local.hasTenantContext = false is pre-set at JobWorker.cfc:449. The pre-existing regression test "reports an unresolvable job class as a job failure instead of throwing" (JobRobustnessSpec.cfc:40) still holds because the new message includes the job id.
The perform() guard is idiomatic:
if (!StructKeyExists(local.rv, "perform")) { // Job.cfc:303matching the same StructKeyExists member check already applied to local.jobInstance two lines up in the sibling path (Job.cfc:361, :364) — and wheels.tests._assets.models.Post has no perform(), so the Wheels.InvalidJobClass spec exercises a real negative.
Cross-engine
Handled well. JobClassRoundTripSpec follows invariant 11 by hanging the miscased-construction flag off a struct set in the try (resolved = {miscasedConstructed = false, ...}) rather than a local set in the catch, so it survives BoxLang. The parenthesized-new receiver was hoisted to a variable (bridge = new wheels.Job()), and JobWorker reaches $instantiateJobClass through $jobBridge() (a variable receiver), both dodging Adobe's MissingNameException. The newly-documented CLAUDE.md invariant 16a captures exactly that gotcha.
Tests
Strong. Three invariant specs (file-name/metadata-name equality via case-sensitive Compare(), miscasing cannot reach the column, self-round-trip) plus both error paths (Wheels.JobClassNotFound, Wheels.InvalidJobClass), all BDD under wheels.WheelsTest. They run on every engine × DB leg, which is the point — they answer the issue's "unverified across engines" question directly.
Conventions
Non-blocking nit: JobClassRoundTripSpec.cfc assigns spec locals unscoped (job = CreateObject(...), meta, bridge, revived, thrown, resolved, …), whereas every sibling in vendor/wheels/tests/specs/jobs/ uses local.-scoped vars (e.g. local.bootstrapJob = new wheels.Job(); in JobRobustnessSpec.cfc:19). It runs green, but the unscoped writes leak into the spec's variables scope; local.-prefixing would match the directory idiom. Purely stylistic — safe to leave.
Docs
Changelog fragment present and correctly named (changelog.d/3351-jobclass-round-trip.fixed.md, <slug>.fixed.md). CLAUDE.md invariant 16 split into 16a/16b documenting the parenthesized-new Adobe crash — a genuinely useful addition.
Commits
All three conform to commitlint (test(job):, fix(test):, fix(job): — valid types, lowercase subjects ≤ 100 chars) and the messages explain the why (the Adobe case-sensitivity discovery, the invariant-11 struct-flag rationale).
Nice work — the honesty about the CreateObject-from-DB-string gadget being narrowed, not closed is the right call, and pinning the invariant rather than guessing at a normalization is the correct instinct.
Second wheels-bot review follow-up from the same batch, on PR #3358. Every sibling in vendor/wheels/tests/specs/jobs/ declares spec variables with `local.` (e.g. `local.bootstrapJob = new wheels.Job();` in JobRobustnessSpec). JobClassRoundTripSpec assigned them unscoped, which runs green but leaks the writes into the spec's `variables` scope. Purely a convention fix — no behaviour change. Folded in here rather than opened as a third PR, since it is the same review round on the same merged batch. lucee7 + sqlite, full core suite: 4756 pass / 0 fail / 0 error, unchanged. Signed-off-by: Peter Amiri <peter@alurium.com>
…e SQL text (#3360) * fix(model): read a join's type from association metadata, not from the SQL text Follow-up to #3334, raised by wheels-bot on PR #3354 and correct. The grouping decision that PR introduced still asked "is this an INNER join?" by searching the generated SQL: FindNoCase("INNER", local.joins[local.parentPosition]) That misclassifies every table whose name contains the substring — `winners`, `spinners`, `beginners`. A parent LEFT OUTER JOIN on `c_o_r_e_winners` reads as INNER, so its nested child is emitted flat instead of grouped, which silently drops the parent rows #3334 exists to preserve. Not a regression: the pre-fix code scanned the same way, and no fixture table hits it. Worth fixing anyway, because it is the exact anti-pattern that PR argues against — its whole thesis is that structure should come from the association tree rather than from re-parsing the SQL string, and this was ten lines of new code doing the opposite. `$associationJoinsInner()` reads the association's declared `joinType`, which is the value `$expandedAssociations` turns into the leading `INNER JOIN` / `LEFT OUTER JOIN` text in the first place — so it is the authoritative source by construction. It falls back to the text scan only when an entry carries no `joinType`, keeping it total for a hand-built struct. Red-first: with the helper reverted to the bare text scan, the new spec fails `Expected [true] to be false`. A note on that red-check, because it nearly fooled me: run against a WARM server the reverted build still reported green — `tools/test-local.sh` reuses a running server and the reload did not recompile the changed CFC. The failure only appears after killing the server first. Any red-check on this suite needs a cold start to mean anything. lucee7 + sqlite, full core suite: develop 2c98644 4755 pass / 0 fail / 0 error this branch 4756 pass / 0 fail / 0 error Exactly +1, the new spec. Refs #3334 Signed-off-by: Peter Amiri <peter@alurium.com> * test(job): scope JobClassRoundTripSpec locals with local. Second wheels-bot review follow-up from the same batch, on PR #3358. Every sibling in vendor/wheels/tests/specs/jobs/ declares spec variables with `local.` (e.g. `local.bootstrapJob = new wheels.Job();` in JobRobustnessSpec). JobClassRoundTripSpec assigned them unscoped, which runs green but leaks the writes into the spec's `variables` scope. Purely a convention fix — no behaviour change. Folded in here rather than opened as a third PR, since it is the same review round on the same merged batch. lucee7 + sqlite, full core suite: 4756 pass / 0 fail / 0 error, unchanged. Signed-off-by: Peter Amiri <peter@alurium.com> * fix(test): keep the catch-block struct unscoped — local. breaks it on BoxLang The compat matrix for this branch came back +24 tests on all 28 legs, which is correct, and two NEW failures on boxlang for every database: Failed | throws Wheels.JobClassNotFound naming the row and the class | Expected [Wheels.JobClassNotFound] but received [] Failed | throws Wheels.InvalidJobClass when the path resolves to something that is not a job | Expected [Wheels.InvalidJobClass] but received [] Both are mine, and both were caused by the previous commit — the `local.`-scoping convention nit from the bot review. `thrown` is written from inside a catch block, and on BoxLang the catch body runs under a nested `local` that is discarded on exit. Prefixing the struct made `local.thrown.type = e.type` land on that discarded copy instead of mutating the outer struct, so the assertion read an empty type. Cross-engine invariant 11 already covers the scalar case. What it did not say is that the struct workaround it recommends only works when the struct is accessed WITHOUT the prefix — `local.state.flag = true` fails exactly like `local.X = ...`. The prefix is what breaks it, not the assignment shape. Widened the invariant with that, plus a worked example, because `local.`-scoping spec variables IS the house style everywhere else, which makes tidying a catch-using spec to match an easy and completely invisible way to break it. So the original unscoped form was correct and the "nit" was wrong. Reverted for `thrown` only — every other variable in the file stays `local.`-scoped, since those are written from try bodies and are unaffected — with a comment at both sites explaining why, so it does not get tidied back. Worth noting the failure mode: green on Lucee, green on Adobe, wrong only on BoxLang, and silent rather than an error. Nothing local would have caught it. lucee7 + sqlite, full core suite: 4756 pass / 0 fail / 0 error, unchanged. Signed-off-by: Peter Amiri <peter@alurium.com> --------- Signed-off-by: Peter Amiri <peter@alurium.com>
1 participant
Closes #3351.
What I could and could not reproduce
enqueue()persistsGetMetadata(this).nameintowheels_jobs.jobClass; the drain resolves that string as a component path. Component paths are case-sensitive on Linux and not on macOS or Windows — the classic shape that passes in development and fails on a production redeploy.I could not reproduce a casing drift. Probed on Lucee 7:
Instantiating through an all-lowercase path still yields the canonical metadata name. Lucee derives it from the file, not from what the caller typed. That explains why this has never bitten — it is a property of the engine, not luck.
It is also not a guarantee for the other four engines, and the issue is explicit that the invariant is unverified there.
So I pinned the invariant instead of guessing at a fix
JobClassRoundTripSpecasserts, case-sensitively (Compare(), notCompareNoCase()):.cfcfile name;These run on every engine × database leg, so lucee6, adobe2023, adobe2025 and boxlang each answer the open question directly rather than being assumed safe. If any engine reports a non-canonical name, this fails there and names it — which is a better outcome than a speculative normalisation that might paper over it.
And improved the failure when it does not hold
$instantiateJobClass()replaces the bareCreateObjecton both processing paths (Job.$processJobandJobWorker.$executeJob) and throwsWheels.JobClassNotFoundnaming the class, the queue row id, and the three real causes — casing, rename, delete.This is the issue's actual pain: the raw error is
component not foundfor a class that plainly exists, so investigators go looking at mappings and deployment. The fix is to describe the real shape of the problem — a string read back out of a queue row.On the
CreateObjectgadgetThe issue asks whether an arbitrary
jobClasscan be written.Wheels.InvalidJobClassnow fires when the path resolves to a component with noperform().To be straight about what that is worth: it narrows but does not close the database-string-to-
CreateObjectshape. The real guarantee is still that only$enqueueJobwrites that column, and the docstring says exactly that rather than implying the check is a security boundary. Its practical value is diagnostic — a renamed job class fails with a sentence instead of a stack trace.JobWorker.$scheduleRetry'sCreateObjectis deliberately left alone: it is a best-effort backoff lookup already wrapped in atry/catchthat falls back to defaults, and it must not start throwing.Red-first
With
Job.cfcandJobWorker.cfcreverted:4734 pass / 2 fail / 1 error.The two error-handling specs fail (
Expected [Wheels.JobClassNotFound] but received [expression]) and the round-trip spec errors on the missing helper. The three invariant specs pass — as they should on an engine that holds the invariant. That is the point of shipping them.Verification
lucee7 + sqlite, full core suite:
ab901cff7Exactly +5 — the new specs, nothing else moved.
Compat matrix dispatched. For this PR the matrix is not just a regression check — it is the experiment.
🤖 Generated with Claude Code