Skip to content

Commit 27fddae

Browse files
bpamiriclaude
andcommitted
style(model): condense review-flagged inline comments (#2852)
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>
1 parent 12eccdb commit 27fddae

2 files changed

Lines changed: 5 additions & 33 deletions

File tree

vendor/wheels/Model.cfc

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -125,14 +125,7 @@ component output="false" displayName="Model" extends="wheels.Global"{
125125
local.iEnd = local.columns.recordCount;
126126
for (local.i = 1; local.i <= local.iEnd; local.i++) {
127127
// set up properties and column mapping
128-
//
129-
// Preserve the database's reported column casing for the derived
130-
// property name (so SQL Server / MySQL / SQLite keep `isHidden`),
131-
// except on adapters whose database folds unquoted identifiers to
132-
// a non-meaningful UPPERCASE default (Oracle, H2), where we
133-
// lowercase to keep property names sane. Prior to 4.0 this was an
134-
// unconditional lCase() that silently lowercased every
135-
// auto-derived property on every engine.
128+
// preserve the DB's reported column case; an unconditional lCase() here regressed non-Oracle engines in 3.0 (see $lowerCaseColumnNames)
136129
local.columnName = local.columns["column_name"][local.i];
137130
if (variables.wheels.class.adapter.$lowerCaseColumnNames()) {
138131
local.columnName = lCase(local.columnName);

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

Lines changed: 4 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,38 +3,17 @@ component extends="wheels.WheelsTest" {
33
function run() {
44
g = application.wo
55

6+
// Regression for the 3.0-era force-lowercasing of auto-derived property names (#2852).
67
describe("Auto-derived property name casing", () => {
7-
/*
8-
Regression coverage for the 2.x -> 3.x property-casing change.
9-
10-
When a model declares no property() mappings, Wheels derives its
11-
properties from the database column metadata. Up through the 2.x
12-
line this preserved the column's original casing, so a column
13-
named `isHidden` produced the property `isHidden`. A change in the
14-
3.0 line (aimed at Oracle, whose driver reports columns in a fixed
15-
case) began force-lowercasing ALL auto-derived property names, so
16-
`isHidden` became `ishidden` on every engine — breaking
17-
case-sensitive consumers of serialized model output.
18-
19-
Wheels now preserves the database's reported column casing, and
20-
only lowercases on adapters whose database folds unquoted
21-
identifiers to a non-meaningful uppercase default (Oracle, H2),
22-
where lowercasing keeps property names sane.
23-
24-
`c_o_r_e_casepreservation` has an undeclared, mixed-case `isHidden`
25-
column (see tests/populate.cfm).
26-
*/
278
it("preserves the database column case for undeclared properties", () => {
9+
// c_o_r_e_casepreservation has an undeclared, mixed-case `isHidden` column (see populate.cfm)
2810
var names = g.model("CasePreservation").propertyNames();
2911

30-
// Databases that preserve the declared identifier case report
31-
// `isHidden`; lower-folding (Postgres/CockroachDB) and
32-
// upper-folding-then-lowercased (Oracle/H2) report `ishidden`.
12+
// preserve-case engines report `isHidden`; lower/upper-folding engines report `ishidden`
3313
var preservesCase = ListFindNoCase("SQLiteModel,MySQLModel,MicrosoftSQLServerModel", get("adapterName")) GT 0;
3414
var expected = preservesCase ? "isHidden" : "ishidden";
3515

36-
// Case-sensitive membership check — the bug is invisible to a
37-
// case-insensitive lookup, so ListFind (not ListFindNoCase).
16+
// case-sensitive: the regression is invisible to ListFindNoCase
3817
expect(ListFind(names, expected)).toBeGT(0);
3918
});
4019
});

0 commit comments

Comments
 (0)