fix(model): preserve database column case for auto-derived properties - #2852
Conversation
When a model declares no property() mappings, Wheels derives its properties from database column metadata. A change in the 3.0 line (Model.cfc, aimed at normalizing Oracle's fixed-case identifiers) began calling lCase() on every derived property name unconditionally, so an `isHidden` column surfaced as the property `ishidden` on SQL Server, MySQL, SQLite, etc. — silently breaking case-sensitive consumers of serialized model output (returnAs="structs", renderWith(), serializeJSON()) for apps upgrading from CFWheels 2.x. Property names now preserve the database's reported column casing, gated by a new $lowerCaseColumnNames() adapter capability (Base default false). OracleModel and H2Model override it to true because their databases fold unquoted identifiers to a non-meaningful uppercase default, so those engines keep the lowercased behavior they have today. Models that explicitly declare property(name=..., column=...) were always unaffected. Adds propertyCasePreservationSpec with an undeclared mixed-case `isHidden` fixture column; the assertion is adapter-aware to match each engine's identifier folding. Verified locally on Lucee 7 across SQLite, H2, MySQL, and PostgreSQL (0 failures); Oracle and Adobe/BoxLang via CI. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com>
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR — This PR correctly fixes a 3.0-era regression where Model.cfc unconditionally force-lowercased all auto-derived property names, breaking case-sensitive consumers (JSON serialization, returnAs=\"structs\") for apps migrating from CFWheels 2.x. The adapter-capability pattern mirrors the existing $supportsAdvisoryLocks() approach cleanly. The test is well-constructed and the CHANGELOG is thorough. Verdict: comment (minor docs gaps only — no correctness, cross-engine, or security issues).
Correctness
The fix is sound. Key points verified:
vendor/wheels/Model.cfcline 148 comparesvariables.wheels.class.mapping[local.key].value == local.property— CFML's==is case-insensitive for strings, so"isHidden" == "ishidden"istrue. Existingproperty(name=..., column=...)declarations continue to resolve correctly regardless of the reported column casing.StructKeyExists(local.processedColumns, local.columnName)(line 141) is also case-insensitive on CFML structs, so the duplicate-column guard still works.aliasedPropertyListinvendor/wheels/model/sql.cfcis consumed viaListFindNoCase(lines 527, 545), so mixed-case property names don't break JOIN aliasing.
No off-by-one, null-deref, or race concerns.
Conventions
Clean. No mixed positional/named argument issues, no misuse of query-vs-array in views, no route or migration concerns.
Cross-engine
The $lowerCaseColumnNames() pattern mirrors $supportsAdvisoryLocks() at vendor/wheels/databaseAdapters/Base.cfc:582 — a well-established capability hook in this codebase.
One note worth calling out explicitly: the fix relies on CFML engines (Lucee, Adobe CF, BoxLang) all reporting the same column casing via cfdbinfo for a given underlying database. If Adobe CF's cfdbinfo normalizes column names to lowercase internally, the adapter flag wouldn't help and the test would fail on Adobe+MySQL expecting isHidden. The PR says Adobe 2023/2025 run via the compat-matrix — confirm those matrix results are green before merge. The local Lucee 7 verification is strong but Adobe CF's cfdbinfo behavior for mixed-case column identifiers is the one gap not locally verified.
Tests
vendor/wheels/tests/specs/model/propertyCasePreservationSpec.cfc — solid construction:
- Uses
ListFind(case-sensitive) for the assertion, so the bug is genuinely invisible to aListFindNoCasecheck. Good explicit choice. get("adapterName")adapter-detection follows the same pattern asvendor/wheels/tests/specs/model/propertiesSpec.cfc:551and others.- Fixture table + model in
tests/_assets/models/CasePreservation.cfcfollow the established convention. - Table cleanup added to
populate.cfm's drop list. ✓
One minor concern: the adapter-detection list is
// vendor/wheels/tests/specs/model/propertyCasePreservationSpec.cfc:33
var preservesCase = ListFindNoCase("SQLiteModel,MySQLModel,MicrosoftSQLServerModel", get("adapterName")) GT 0;This hard-codes the three preserve-case adapters rather than deriving from $lowerCaseColumnNames(). If a future adapter is added that also preserves case, the list needs a manual update. Not a bug today, but consider whether calling g.model("CasePreservation").$getAdapter().$lowerCaseColumnNames() (or the equivalent) would make the test self-consistent with the production logic. Keeping both in sync is a maintenance hazard.
Docs
Two gaps worth filling before or shortly after merge:
-
.ai/wheels/cross-engine-compatibility.mdnot updated. The new$lowerCaseColumnNames()capability and the per-engine identifier-folding behavior (preserve / lowercase-fold / uppercase-fold-then-lowercase) is exactly the kind of cross-engine nuance that belongs in that reference. Future maintainers adding a new database adapter need to know to check this flag. -
Secondary migration note missing. The CHANGELOG entry accurately describes the 2.x→3.x regression. There is no note for apps that adapted to the regression — i.e., apps that, after upgrading to Wheels 3.x/4.x, built on lowercased property names in JSON outputs. For those apps, applying this patch silently changes
{"ishidden": 1}back to{"isHidden": 1}. One sentence covering this case ("if your app adapted to the lowercased output and serializes model data to JSON, review your consumers after applying this patch") would be a useful heads-up.
Commits
Both commits conform to commitlint:
fix(model): preserve database column case for auto-derived properties— valid type, ≤100 chars. ✓docs(changelog): note property-case preservation fix (#2852)— valid type, ≤100 chars. ✓
Both carry Signed-off-by: Peter Amiri <peter@alurium.com> — DCO satisfied. ✓
Security
No SQL injection, XSS, CSRF, or secret-leakage concerns. The change operates entirely on metadata returned from cfdbinfo, not user input.
Wheels Bot — Reviewer B (round 1)A's review holds up. The correctness claims are verified against the live code, the findings are concrete, and the SycophancyNone detected. A did not say LGTM or approve without justification. Each section cites specific lines or patterns. False positivesNone detected. Verified each claim against the actual repo state:
Missed issuesNone significant. One minor observation A did not raise: the multi-paragraph docblock added to Verdict alignmentA's ConvergenceAligned with A. The two docs gaps A identified are concrete and addressable now:
A's note about hard-coding the adapter list in the spec is a maintenance concern worth tracking but not a correctness issue today — no action needed before merge. Joint recommendation: address the two docs gaps above, then the PR is ready to merge pending green compat-matrix results (especially Adobe+MySQL) as A noted. |
- Document the $lowerCaseColumnNames() adapter capability and per-engine identifier-folding behavior (preserve / lowercase-fold / uppercase-fold) in .ai/wheels/cross-engine-compatibility.md, including a table and a "when adding a new adapter" note so future maintainers know to check the flag. - Extend the CHANGELOG entry (#2852) with a reverse-migration heads-up for apps that adapted to the 3.x/4.x force-lowercased property names in their JSON consumers — those outputs revert to declared casing on SQL Server / MySQL / SQLite after this patch. Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Wheels Bot — Address Review (round 1)Applied consensus findings from Reviewer A and Reviewer B's convergence (round 1):
Skipped (intentionally — not in consensus):
No code paths were touched, so no test layer was affected — The new commit will trigger a fresh Reviewer A run on the updated SHA. Convergence loop continues until reviewers align on `approve` or the outer-loop cap (5 rounds) is reached. |
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR — Round 1 docs commit (12eccdb4c) correctly addresses both documentation gaps flagged in the previous review: .ai/wheels/cross-engine-compatibility.md now has the $lowerCaseColumnNames() table and new-adapter guidance, and the CHANGELOG entry carries the reverse-migration heads-up for case-adapted consumers. No correctness, cross-engine, or security issues in the new commit. Two open convention nits from the original diff remain; one open test-maintainability concern also remains. Verdict: comment.
Resolved since round 1
Both docs gaps from the previous review are closed:
.ai/wheels/cross-engine-compatibility.md— new section with the per-engine identifier-folding table, code examples for all three adapters, new-adapter authoring guidance, and an explicit note that explicitproperty(name=..., column=...)declarations bypass the flag entirely. ✓CHANGELOG.md— reverse-migration heads-up for apps that adapted to the 3.x/4.x force-lowercased output is present and specific enough to be actionable. ✓
The new $lowerCaseColumnNames() docstring in Base.cfc (vendor/wheels/databaseAdapters/Base.cfc:586–598) follows the established /** multi-line pattern already used by $supportsAdvisoryLocks() at lines 569–581. Consistent. ✓
Conventions
Multi-line comment block in Model.cfc — still present
vendor/wheels/Model.cfc lines 128–135 contain an 8-line // block comment inside executable code:
//
// Preserve the database's reported column casing for the derived
// property name (so SQL Server / MySQL / SQLite keep `isHidden`),
// except on adapters whose database folds unquoted identifiers to
// a non-meaningful UPPERCASE default (Oracle, H2), where we
// lowercase to keep property names sane. Prior to 4.0 this was an
// unconditional lCase() that silently lowercased every
// auto-derived property on every engine.
local.columnName = local.columns["column_name"][local.i];CLAUDE.md: "Never write multi-paragraph docstrings or multi-line comment blocks — one short line max." The WHY here is genuinely non-obvious and worth a comment, but the format violates the limit. Condense to one line:
// preserve reported case; unconditional lCase() regressed non-Oracle engines in 3.0 (see $lowerCaseColumnNames)
local.columnName = local.columns["column_name"][local.i];Multi-line comment blocks in propertyCasePreservationSpec.cfc — still present
vendor/wheels/tests/specs/model/propertyCasePreservationSpec.cfc:
- Lines 7–26: 20-line
/* ... */block insidedescribe(). - Lines 30–32: 3-line
//block before theListFindNoCaseline.
Both violate the same rule. The it() description string ("preserves the database column case for undeclared properties") and the existing describe() string already communicate intent. A single-line comment at the key decision point — e.g. // case-sensitive; the bug is invisible to ListFindNoCase — is sufficient.
Tests
Adapter-name list still hard-coded — open from round 1
vendor/wheels/tests/specs/model/propertyCasePreservationSpec.cfc line 33:
var preservesCase = ListFindNoCase("SQLiteModel,MySQLModel,MicrosoftSQLServerModel", get("adapterName")) GT 0;This was flagged in round 1 and is still unaddressed. If a new adapter is added that preserves case (or if an existing one is renamed), the test diverges from production logic silently — it would expect ishidden but get isHidden, producing a false failure. The production gate is $lowerCaseColumnNames(), and the test should derive from the same source:
var preservesCase = !g.model("CasePreservation").$getAdapter().$lowerCaseColumnNames();
var expected = preservesCase ? "isHidden" : "ishidden";This keeps the spec self-consistent with the adapter capability contract and survives new-adapter additions without a manual update.
Commits
New commit 12eccdb4c:
- Header
docs: address Reviewer A/B consensus findings (round 1)— 56 chars, valid type, ≤ 100 chars. ✓ Signed-off-by: claude[bot]— DCO satisfied. ✓
Merge gate reminder
As noted in round 1: the Adobe CF 2023/2025 matrix runs are the critical validation path. The PR body says they run via the compat-matrix; confirm those results are green before merging. The Lucee 7 + SQLite/H2/MySQL/PostgreSQL coverage is strong, but Adobe CF's cfdbinfo column-name reporting is the one behavior not locally verifiable.
|
Wheels Bot — Reviewer B (round 1) The round-2 review from A correctly closes the docs gaps from round 1. Both remaining convention findings rest on a misattributed rule, and the one test-maintainability concern was explicitly excluded from round-1 consensus. Stripping those leaves no actionable findings. Converging on approve. Sycophancy None detected. The False positives Comment-style rule misattributed to CLAUDE.md (affects two findings) A cites
Both the Missed issues Suggested adapter-based fix references a non-existent API A proposes replacing the hard-coded Verdict alignment The Convergence Aligned. With the false-positive comment-style findings stripped and the already-excluded adapter-list concern not re-opened, there are no remaining actionable findings. The PR is correctness-clean, the cross-engine handling is sound, the docs are complete. The compat-matrix (Adobe CF 2023/2025) is the only remaining gate — a CI concern, not a code concern. Joint recommendation: approve and let the compat-matrix results speak. |
Address Reviewer A's comment-length nits: collapse the 8-line block in Model.cfc and the multi-line blocks in propertyCasePreservationSpec to single lines (CLAUDE.md one-line-comment guidance). Base.$lowerCaseColumnNames() keeps its docstring — it matches the existing $supportsAdvisoryLocks() convention, which both reviewers endorsed. The spec's adapter-name list is kept (deriving it from $lowerCaseColumnNames() needs a public model->adapter accessor that doesn't exist — getClass() isn't a model method and there is no $getAdapter()); reviewers flagged this as a non-blocking maintenance note. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com>
Addressed Reviewer A/B (round 2) —
|
There was a problem hiding this comment.
Wheels Bot --- Reviewer A (Round 3)
TL;DR -- Round 2 style commit (27fddae) correctly addresses the two comment-length violations flagged in the previous review: the 8-line block in Model.cfc is condensed to a single meaningful line, and the 20-line block plus multi-line inline comments in propertyCasePreservationSpec.cfc are condensed to single-line annotations. No correctness, cross-engine, or security concerns in this commit. The adapter-name list maintenance note from round 2 remains acknowledged but technically unresolvable without a public model->adapter accessor (see commit message justification). Verdict: comment -- PR is clear for merge once the compat-matrix results for Adobe CF 2023/2025 are confirmed green.
Resolved since round 2
vendor/wheels/Model.cfc line 128 is now a single-line comment explaining the WHY (a cross-engine regression with a prior-art reference), which earns its place under CLAUDE.md single-line-comment rule.
vendor/wheels/tests/specs/model/propertyCasePreservationSpec.cfc -- the 20-line block and the multi-line inline annotations are now four single-line comments, each explaining a non-obvious decision (fixture source, adapter folding, case-sensitivity rationale).
vendor/wheels/databaseAdapters/Base.cfc -- the $lowerCaseColumnNames() docstring is unchanged, consistent with both reviewers endorsement of the $supportsAdvisoryLocks() pattern.
Conventions -- Clean in this commit. No new multi-line blocks introduced.
Commits -- New commit 27fddae: header style(model): condense review-flagged inline comments is valid type, <= 100 chars, not ALL-CAPS. Signed-off-by present -- DCO satisfied. Commit message explains the adapter-list decision and the docstring exception.
Remaining open item (non-blocking, from round 2)
propertyCasePreservationSpec.cfc line 13 still hard-codes the adapter-name list (SQLiteModel,MySQLModel,MicrosoftSQLServerModel) rather than deriving from $lowerCaseColumnNames(). The commit message gives the right technical explanation: there is no public model->adapter accessor (getClass() is not a model method; $getAdapter() does not exist), so deriving from production logic would require adding a new accessor -- a scope change not warranted by this bug fix. This was marked non-blocking in round 2 and remains so. If a new adapter is added that overrides $lowerCaseColumnNames(), this list will need a manual update; that risk is understood.
Merge gate reminder -- As noted in rounds 1 and 2: confirm the Adobe CF 2023/2025 compat-matrix runs are green before merging. Lucee 7 + SQLite/H2/MySQL/PostgreSQL coverage is strong; Adobe CF cfdbinfo column-name reporting for mixed-case identifiers is the one behavior not locally verifiable.
Wheels Bot — Reviewer B (round 1)A's review filed on this SHA ( The commit itself ( SycophancyNot applicable — A made no findings. False positivesNot applicable — A made no claims. Missed issuesNone. The style-only commit introduces no new correctness, cross-engine, or test concerns. Diff-confirmed: only Verdict alignmentA filed a ConvergenceAligned on approve. The PR is correctness-clean, cross-engine handling is sound, docs are complete ( |
Wheels Bot — Reviewer B (round 1)Review 4423209318 is a test/auth placeholder (body: "test body for auth check", zero line comments) — there is no substantive Reviewer A review on this SHA to critique. The previous cycle closed with SycophancyN/A — the submitted review is a test placeholder with no content. False positivesN/A — no claims were made. Missed issuesNone in the new commit. The
Verdict alignmentN/A — no verdict was issued by the placeholder review. ConvergenceAligned. The prior convergence on |
Wheels Bot — Reviewer B (round 1)A's round-3 review correctly characterizes the style commit as clean. No sycophancy, no false positives, no missed issues. Minor verdict note below, but converging on approve. SycophancyNone detected. False positivesNone detected. Verified A's claims against the diff:
Missed issuesNone detected. Verdict alignmentA says "PR is clear for merge once the compat-matrix results for Adobe CF 2023/2025 are confirmed green" but uses ConvergenceAligned. The style commit is correctness-clean and contains exactly the changes both reviewers requested. No new code paths, no new cross-engine concerns, no new docs gaps. The compat-matrix (Adobe CF 2023/2025) is the one remaining gate and is a CI concern, not a code-review concern. Joint recommendation: approve. The PR is ready to merge once compat-matrix results are confirmed green. |
Problem
Migrating from CFWheels 2.5 → Wheels 3/4, auto-derived model property names come back lowercased: an
isHiddencolumn surfaces as the propertyishidden, breaking case-sensitive consumers of serialized model output (returnAs="structs",renderWith(),serializeJSON()). Reported on Slack; no prior issue. The same code worked in 2.5 on the same Lucee 7 + SQL Server, so it's a framework change — not a Lucee/driver change.Root cause
When a model declares no
property()mappings,Model.cfcauto-derives its properties from the database column metadata. Commitdbfbdda71("Oracle compatibility", 2025-07-15) replaced the raw column name withlCase(local.columns["column_name"][local.i]), force-lowercasing every auto-derived property name on every engine. Oracle reports unquoted identifiers in uppercase, so normalizing there made sense; applying it to all adapters regressed case preservation for SQL Server / MySQL / SQLite. Shipped inv3.0.0(build +33) through thev4.0.xline.Fix
Preserve the database's reported column casing by default; lowercase only on adapters whose database folds unquoted identifiers to a non-meaningful uppercase default.
$lowerCaseColumnNames()capability ondatabaseAdapters/Base.cfc(defaultfalse), mirroring the existing$supportsAdvisoryLocks()pattern.OracleModelandH2Modeloverride it totrue(both upper-fold unquoted identifiers).Model.cfcconsults it instead of unconditionally callinglCase().Behavior by engine:
isHidden) — restores 2.5 behaviorModels that explicitly declare
property(name="isHidden", column="isHidden")were always unaffected and remain so.Tests
Adds
propertyCasePreservationSpecand ac_o_r_e_casepreservationfixture table with an undeclared mixed-caseisHiddencolumn. The assertion is adapter-aware (preserve-case engines expectisHidden; fold engines expectishidden) and uses a case-sensitiveListFindso the regression can't hide behind case-insensitive lookups.Verified locally on Lucee 7 across SQLite, H2, MySQL, and PostgreSQL — 0 failures, 0 errors on each (RED confirmed first: the spec fails pre-fix with
actual [0] is not greater than [0]). Oracle + Adobe 2023/2025 + BoxLang + Lucee 6 run via the compat-matrix on this PR.Pre-release stopgap for affected users
property(name="isHidden", column="isHidden")on the affected models restores casing today without waiting for the release.🤖 Generated with Claude Code