Skip to content

Commit ab901cf

Browse files
authored
fix(model): resolve association foreign key defaults against either reference convention (#3353)
`useUnderscoreReferenceColumns` (framework default `false`, `wheels new` template default `true`) makes the migrator emit `user_id`, but the model layer derived `userid` by unconditional concatenation. A stock new app that declared `belongsTo("user")` without an explicit `foreignKey` therefore threw `key [userid] doesn't exist` the first time any `include=` traversed the association — a migrator and a model layer that could never agree. Reproduced before changing anything, same flag state throughout: migrator_emits = author_id model_derives_flagON = authorid model_derives_flagOFF = authorid <- identical, so the model never read the flag The default is now resolved against the columns that actually exist on whichever side owns the foreign key — `belongsTo` looks at the declaring model, `hasMany`/`hasOne` at the associated one (verified empirically, not assumed). Both conventions work, including apps that enabled the flag mid-life and hold a mix of both shapes. Schema-driven rather than flag-driven, deliberately. `references()` re-reads `$get()` on every call, while this result is memoized for the application lifetime (`expandedMetadataFilled`), so a flag-driven default would let a runtime flip change migrations without changing models — replacing one inconsistency with another. It is also strictly error-reducing: the underscore form is only consulted when the legacy form is absent, which is a case that threw before, so it cannot break a working app. That matters because the CLI ships a mixed-convention advisory (`Module.cfc:5267`) precisely because apps with the flag on and legacy columns exist — an unconditional flag-driven change would have broken exactly those. Also adds `Wheels.AssociationForeignKeyNotFound`: when a DERIVED default matches no column on the owning model, throw at association-resolution time naming the association, both candidate shapes, and `foreignKey=` as the fix. Runs inside the existing memo so the success path costs one check per application lifetime, gated on `showErrorInformation`, and never second-guesses an explicit `foreignKey=`. Scope: polymorphic associations are NOT covered. `belongsTo(polymorphic=true)` and `hasMany`/`hasOne` with `as=` pin their foreign key at registration time (`associations.cfc:30`, `:81`, `:134`), before the schema is available, so the join-time resolution never sees a blank to fill. Documented rather than silently left — the root CLAUDE.md previously claimed the `_type` half worked too. Corrects three documents that asserted an alignment no code implemented: root CLAUDE.md, `vendor/wheels/migrator/CLAUDE.md`, and — worst — the comment `wheels new` writes into every generated app's `config/settings.cfm`. 7 regression specs plus a legacy-shape guard, red-first: with `sql.cfc` reverted, 7 of the 8 fail and the two central ones fail with the reported symptom, `key [refParentid] doesn't exist`. Fixtures `RefParent` / `RefChild` and their tables use the underscore shape; the framework's own fixtures are all legacy-shaped, which the guard spec pins. Verification, lucee7, full core suite in one container: sqlite 4721 pass / 7 fail / 4 error / 4750 specs mysql 4729 pass / 7 fail / 4 error / 4746 specs All 8 new specs pass on both. The 11 remaining failures are an identical pre-existing cluster on both databases (`app.controllers.Controller` missing its mixed-in helpers), local to this container and absent from CI's legs. Closes #3337 Signed-off-by: Peter Amiri <peter@alurium.com>
1 parent ca835c3 commit ab901cf

8 files changed

Lines changed: 349 additions & 7 deletions

File tree

CLAUDE.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,11 @@ t.primaryKey(name="userId", autoIncrement=true);
266266
267267
For new migrator helpers or anywhere you accept a column-name argument: declare `string columnNames` (NOT `required`), and call `$combineArguments(args=arguments, combine="columnNames,columnName", required=true)` at the top of the body. The pattern is documented in [vendor/wheels/migrator/CLAUDE.md](vendor/wheels/migrator/CLAUDE.md). Boolean nullable flag is `allowNull` everywhere — never `null`.
268268
269-
`t.references()` also respects `useUnderscoreReferenceColumns` (boolean, framework default `false`, `wheels new` template default `true`) — when true it produces `<name>_id` / `<name>_type` columns matching Wheels model `belongsTo` defaults.
269+
`t.references()` also respects `useUnderscoreReferenceColumns` (boolean, framework default `false`, `wheels new` template default `true`) — when true it produces `<name>_id` / `<name>_type` columns instead of `<name>id` / `<name>type`.
270+
271+
Association foreign-key defaults resolve **either** convention: the default derivation checks which column actually exists on whichever side owns the foreign key, rather than reading the setting ([#3337](https://github.com/wheels-dev/wheels/issues/3337) — before that fix the model layer derived `<modelName><key>` unconditionally and a stock `wheels new` app threw `key [<name>id] doesn't exist` on any `include=`). It is schema-driven on purpose: the migrator reads the flag per call, but the model-side default is memoized for the application lifetime, so honouring the flag there would let a runtime flip change migrations without changing models. Apps holding a mix of both shapes work for the same reason.
272+
273+
**Polymorphic associations are not covered.** `belongsTo(polymorphic=true)` and `hasMany`/`hasOne` with `as=` fix their foreign key to `<name>id` at *registration* time (`vendor/wheels/model/associations.cfc:30`, `:81`, `:134`), before the schema is available, so the join-time resolution never sees a blank to fill. Against an underscore-shaped schema those still need an explicit `foreignKey="<name>_id"`.
270274
271275
## Wheels Conventions
272276
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
- Association foreign-key defaults now resolve either reference-column convention instead of only the legacy `<modelName><key>` one. `useUnderscoreReferenceColumns` (framework default `false`, `wheels new` template default `true`) makes the migrator emit `user_id`, but the model layer derived `userid` unconditionally — so a stock new app that declared `belongsTo("user")` without an explicit `foreignKey` threw `key [userid] doesn't exist` the first time any `include=` traversed the association. The default is now resolved against the columns that actually exist on whichever side owns the foreign key (`belongsTo` looks at the declaring model, `hasMany`/`hasOne` at the associated one), so both conventions work — including apps that enabled the flag mid-life and hold a mix of both shapes. This is deliberately schema-driven rather than reading the setting: `references()` re-reads the flag on every call while the model-side default is memoized for the application lifetime, so a flag-driven default would let a runtime flip change migrations without changing models. It is also strictly error-reducing — the underscore form is only consulted when the legacy form is absent, which is a case that used to throw. Polymorphic associations are not covered; they pin their foreign key at registration time, before the schema is available, and still need an explicit `foreignKey=` under the underscore convention (#3337)
2+
- An association whose *derived* default foreign key matches no column on the model that owns it now throws `Wheels.AssociationForeignKeyNotFound` at association-resolution time, naming the association, both candidate column shapes, and `foreignKey=` as the fix. Previously this surfaced as `key [userid] doesn't exist` from deep inside the join builder, which named neither the association nor the argument that resolves it. Development and testing only, and only for defaults Wheels derived itself — an explicit `foreignKey=` is left alone (#3337)

vendor/wheels/migrator/CLAUDE.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,11 @@ New code should pass `columnNames`. Both keep working.
4848
| `false` (framework default) | `userid` | `userid`, `usertype` |
4949
| `true` (new-app template default) | `user_id` | `user_id`, `user_type` |
5050

51-
The framework default is `false` so existing apps with applied migrations keep matching their database schemas. The `wheels new` template at `cli/lucli/templates/app/config/settings.cfm` opts new apps into `true` so they match Wheels model `belongsTo` defaults out of the box.
51+
The framework default is `false` so existing apps with applied migrations keep matching their database schemas. The `wheels new` template at `cli/lucli/templates/app/config/settings.cfm` opts new apps into `true`.
52+
53+
**The model side does not read this flag, and must not.** Association foreign-key defaults resolve against the columns that actually exist — `vendor/wheels/model/sql.cfc::$deriveAssociationForeignKey()` tries the legacy `<modelName><key>` shape first and falls back to `<modelName>_<key>` — so both conventions work, including a schema holding a mix of the two. Making it flag-driven instead would break: this function's result is memoized for the application lifetime (`expandedMetadataFilled`), whereas `references()` re-reads `$get()` on every call, so a runtime flip would change migrations without changing models. Before [#3337](https://github.com/wheels-dev/wheels/issues/3337) the model layer derived `<modelName><key>` unconditionally, which meant a stock `wheels new` app had a migrator and a model layer that could never agree.
54+
55+
The exception is **polymorphic** associations, which pin their foreign key to `<name>id` at registration time — see the note in the root `CLAUDE.md`. Those still need an explicit `foreignKey=` under the underscore convention.
5256

5357
The flag is read via `$get("useUnderscoreReferenceColumns")` inside `references()` at runtime — apps can flip the setting in `config/settings.cfm` without reloading the framework. Migrations already applied to a real database are unaffected; only the column name the *next* migration produces changes.
5458

vendor/wheels/model/sql.cfc

Lines changed: 140 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1388,13 +1388,39 @@ component {
13881388
lock name="wheelsJoinMemo#application.applicationName#" type="exclusive" timeout="10" {
13891389
if (!StructKeyExists(local.classAssociations[local.name], "expandedMetadataFilled")) {
13901390
if (!Len(local.classAssociations[local.name].foreignKey)) {
1391-
// cfformat-ignore-start
1391+
// The foreign key column lives on a different side depending on the association
1392+
// type: for `belongsTo` it is a column on THIS model's table, for `hasMany` /
1393+
// `hasOne` it is a column on the ASSOCIATED model's table. Resolve the default
1394+
// against whichever side actually owns it so both the legacy `<modelName><key>`
1395+
// form and the `<modelName>_<key>` form that `useUnderscoreReferenceColumns`
1396+
// makes the migrator emit are honoured (#3337).
13921397
if (local.classAssociations[local.name].type == "belongsTo") {
1393-
local.classAssociations[local.name].foreignKey = local.associatedClass.$classData().modelName & Replace(local.associatedClass.$classData().keys, ",", ",#local.associatedClass.$classData().modelName#", "all");
1398+
local.fkNameSource = local.associatedClass;
1399+
local.fkColumnOwner = local.class;
13941400
} else {
1395-
local.classAssociations[local.name].foreignKey = local.class.$classData().modelName & Replace(local.class.$classData().keys, ",", ",#local.class.$classData().modelName#", "all");
1401+
local.fkNameSource = local.class;
1402+
local.fkColumnOwner = local.associatedClass;
1403+
}
1404+
local.classAssociations[local.name].foreignKey = $deriveAssociationForeignKey(
1405+
columnOwner = local.fkColumnOwner,
1406+
modelName = local.fkNameSource.$classData().modelName,
1407+
keys = local.fkNameSource.$classData().keys
1408+
);
1409+
// A derived default matching no column on the owning side can only fail later,
1410+
// deep inside the join builder, as `key [xxx] doesn't exist` — a message naming
1411+
// neither the association nor the `foreignKey=` argument that fixes it. Report it
1412+
// here instead, while both candidate shapes are still in hand (#3337). Runs inside
1413+
// the memo so the success path costs one check per application lifetime, and only
1414+
// for defaults derived here — an explicit `foreignKey=` is the developer's call.
1415+
if (application.wheels.showErrorInformation) {
1416+
$assertDerivedForeignKeyResolves(
1417+
associationName = local.name,
1418+
foreignKey = local.classAssociations[local.name].foreignKey,
1419+
columnOwner = local.fkColumnOwner,
1420+
modelName = local.fkNameSource.$classData().modelName,
1421+
keys = local.fkNameSource.$classData().keys
1422+
);
13961423
}
1397-
// cfformat-ignore-end
13981424
}
13991425
if (!Len(local.classAssociations[local.name].joinKey)) {
14001426
if (local.classAssociations[local.name].type == "belongsTo") {
@@ -1551,6 +1577,116 @@ component {
15511577
return local.rv;
15521578
}
15531579

1580+
/**
1581+
* Internal function.
1582+
* Builds the conventional foreign key list for an association default: the model name
1583+
* prefixed onto each of the target primary keys, joined by `separator`.
1584+
*
1585+
* `keys` may be a comma list for composite primary keys, so every element gets the
1586+
* prefix — `user` + `a,b` yields `usera,userb`, or `user_a,user_b` with an underscore.
1587+
*/
1588+
public string function $buildForeignKeyList(
1589+
required string modelName,
1590+
required string keys,
1591+
string separator = ""
1592+
) {
1593+
local.rv = "";
1594+
local.keysArray = ListToArray(arguments.keys);
1595+
local.iEnd = ArrayLen(local.keysArray);
1596+
for (local.i = 1; local.i <= local.iEnd; local.i++) {
1597+
local.rv = ListAppend(local.rv, arguments.modelName & arguments.separator & Trim(local.keysArray[local.i]));
1598+
}
1599+
return local.rv;
1600+
}
1601+
1602+
/**
1603+
* Internal function.
1604+
* True when every element of a foreign key list is a property on the supplied class.
1605+
* Checks property names rather than column names because that is the lookup the join
1606+
* builder performs (`properties[foreignKey].column`).
1607+
*/
1608+
public boolean function $foreignKeyListResolves(required any columnOwner, required string foreignKey) {
1609+
local.properties = arguments.columnOwner.$classData().properties;
1610+
local.keysArray = ListToArray(arguments.foreignKey);
1611+
local.iEnd = ArrayLen(local.keysArray);
1612+
for (local.i = 1; local.i <= local.iEnd; local.i++) {
1613+
if (!StructKeyExists(local.properties, Trim(local.keysArray[local.i]))) {
1614+
return false;
1615+
}
1616+
}
1617+
return local.iEnd > 0;
1618+
}
1619+
1620+
/**
1621+
* Internal function.
1622+
* Derives the default foreign key for an association, preferring whichever conventional
1623+
* shape actually exists on the model that owns the column.
1624+
*
1625+
* Wheels has two conventions in play. The legacy `<modelName><key>` form is what this
1626+
* function has always produced, and `useUnderscoreReferenceColumns` (the `wheels new`
1627+
* default) makes the migrator emit `<modelName>_<key>` instead — leaving stock new apps
1628+
* with a schema the association default could never match (#3337).
1629+
*
1630+
* Resolving against the real columns rather than reading the setting fixes both
1631+
* conventions at once, including apps that flipped the flag mid-life and therefore hold
1632+
* a mix of both shapes. It is also strictly error-reducing: the underscore form is only
1633+
* consulted when the legacy form is absent, which is a case that throws today. The
1634+
* setting is deliberately NOT consulted — it is read per call by the migrator, whereas
1635+
* this result is memoized for the application lifetime, so honouring it here would make
1636+
* a runtime flip take effect for migrations but not for models.
1637+
*
1638+
* Falls back to the legacy shape when neither resolves, leaving the existing error path
1639+
* (and `$assertDerivedForeignKeyResolves`) to report it.
1640+
*/
1641+
public string function $deriveAssociationForeignKey(
1642+
required any columnOwner,
1643+
required string modelName,
1644+
required string keys
1645+
) {
1646+
local.legacy = $buildForeignKeyList(modelName = arguments.modelName, keys = arguments.keys);
1647+
if ($foreignKeyListResolves(columnOwner = arguments.columnOwner, foreignKey = local.legacy)) {
1648+
return local.legacy;
1649+
}
1650+
local.underscored = $buildForeignKeyList(modelName = arguments.modelName, keys = arguments.keys, separator = "_");
1651+
if ($foreignKeyListResolves(columnOwner = arguments.columnOwner, foreignKey = local.underscored)) {
1652+
return local.underscored;
1653+
}
1654+
return local.legacy;
1655+
}
1656+
1657+
/**
1658+
* Internal function.
1659+
* Throws a descriptive error when an association's DERIVED default foreign key matches no
1660+
* property on the model that owns the column. Without this the failure surfaces much later
1661+
* as `key [xxx] doesn't exist` from inside the join builder, which names neither the
1662+
* association nor the argument that fixes it (#3337).
1663+
*
1664+
* Skipped when the owner has no properties at all — an un-migrated or missing table would
1665+
* otherwise produce this error instead of the clearer one the query itself raises.
1666+
*/
1667+
public void function $assertDerivedForeignKeyResolves(
1668+
required string associationName,
1669+
required string foreignKey,
1670+
required any columnOwner,
1671+
required string modelName,
1672+
required string keys
1673+
) {
1674+
if (!StructCount(arguments.columnOwner.$classData().properties)) {
1675+
return;
1676+
}
1677+
if ($foreignKeyListResolves(columnOwner = arguments.columnOwner, foreignKey = arguments.foreignKey)) {
1678+
return;
1679+
}
1680+
local.ownerName = arguments.columnOwner.$classData().modelName;
1681+
local.legacy = $buildForeignKeyList(modelName = arguments.modelName, keys = arguments.keys);
1682+
local.underscored = $buildForeignKeyList(modelName = arguments.modelName, keys = arguments.keys, separator = "_");
1683+
Throw(
1684+
type = "Wheels.AssociationForeignKeyNotFound",
1685+
message = "The `#arguments.associationName#` association derives a default foreign key of `#arguments.foreignKey#`, which is not a property on the `#local.ownerName#` model.",
1686+
extendedInfo = "Wheels looks for the conventional `#local.legacy#` and, for schemas built with `useUnderscoreReferenceColumns` enabled, `#local.underscored#`. Neither exists on `#local.ownerName#`. Either pass `foreignKey=""<column>""` explicitly when setting up the `#arguments.associationName#` association, or rename the column on `#local.ownerName#` to one of those two forms."
1687+
);
1688+
}
1689+
15541690
/**
15551691
* Internal function.
15561692
*/
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
/**
2+
* Fixture for #3337: the child side of an association whose foreign key column uses the
3+
* `<name>_id` convention that `useUnderscoreReferenceColumns` makes the migrator emit.
4+
*
5+
* The association passes no `foreignKey`, so the default derivation has to resolve
6+
* `refparent_id` on this model. Before #3337 it derived `refparentid` unconditionally and
7+
* any `include=` threw `key [refparentid] doesn't exist`.
8+
*/
9+
component extends="Model" {
10+
11+
function config() {
12+
table("c_o_r_e_refchildren");
13+
belongsTo("refParent");
14+
}
15+
16+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/**
2+
* Fixture for #3337: the parent side of an association whose foreign key column uses the
3+
* `<name>_id` convention that `useUnderscoreReferenceColumns` makes the migrator emit.
4+
*
5+
* Exercises the hasMany branch of the association foreign-key default, where the column
6+
* lives on the ASSOCIATED model (`c_o_r_e_refchildren.refparent_id`).
7+
*/
8+
component extends="Model" {
9+
10+
function config() {
11+
table("c_o_r_e_refparents");
12+
hasMany("refChildren");
13+
}
14+
15+
}

vendor/wheels/tests/populate.cfm

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@
9696
</cfloop>
9797

9898
<!--- list of tables to delete --->
99-
<cfset local.tables = "c_o_r_e_memberteams,c_o_r_e_members,c_o_r_e_teams,c_o_r_e_polycomments,c_o_r_e_polyarticles,c_o_r_e_polyphotos,c_o_r_e_authors,c_o_r_e_cities,c_o_r_e_classifications,c_o_r_e_comments,c_o_r_e_galleries,c_o_r_e_photos,c_o_r_e_posts,c_o_r_e_profiles,c_o_r_e_shops,c_o_r_e_trucks,c_o_r_e_tags,c_o_r_e_users,c_o_r_e_collisiontests,c_o_r_e_combikeys,c_o_r_e_tblusers,c_o_r_e_sqltypes,c_o_r_e_CATEGORIES,c_o_r_e_bulkitems,c_o_r_e_casepreservation,c_o_r_e_uuidrecords">
99+
<cfset local.tables = "c_o_r_e_memberteams,c_o_r_e_members,c_o_r_e_teams,c_o_r_e_polycomments,c_o_r_e_polyarticles,c_o_r_e_polyphotos,c_o_r_e_authors,c_o_r_e_cities,c_o_r_e_classifications,c_o_r_e_comments,c_o_r_e_galleries,c_o_r_e_photos,c_o_r_e_posts,c_o_r_e_profiles,c_o_r_e_shops,c_o_r_e_trucks,c_o_r_e_tags,c_o_r_e_users,c_o_r_e_collisiontests,c_o_r_e_combikeys,c_o_r_e_tblusers,c_o_r_e_sqltypes,c_o_r_e_CATEGORIES,c_o_r_e_bulkitems,c_o_r_e_casepreservation,c_o_r_e_uuidrecords,c_o_r_e_refchildren,c_o_r_e_refparents">
100100
<!---
101101
On Oracle, append CASCADE CONSTRAINTS so the drop removes incoming FK
102102
references along with the table. PURGE skips the recycle bin so the
@@ -171,6 +171,30 @@ CREATE TABLE c_o_r_e_collisiontests
171171
) #local.storageEngine#
172172
</cfquery>
173173

174+
<!---
175+
#3337 fixtures: a parent/child pair whose foreign key column uses the `<name>_id`
176+
convention that `useUnderscoreReferenceColumns` makes the migrator emit. The
177+
associations on RefParent / RefChild pass no `foreignKey`, so they only resolve if the
178+
default derivation honours the underscore form as well as the legacy `<name>id` one.
179+
--->
180+
<cfquery name="local.query" datasource="#application.wheels.dataSourceName#">
181+
CREATE TABLE c_o_r_e_refparents
182+
(
183+
id #local.identityColumnType#
184+
,name varchar(50)
185+
,PRIMARY KEY(id)
186+
) #local.storageEngine#
187+
</cfquery>
188+
189+
<cfquery name="local.query" datasource="#application.wheels.dataSourceName#">
190+
CREATE TABLE c_o_r_e_refchildren
191+
(
192+
id #local.identityColumnType#
193+
,refparent_id #local.intColumnType#
194+
,PRIMARY KEY(id)
195+
) #local.storageEngine#
196+
</cfquery>
197+
174198
<cfquery name="local.query" datasource="#application.wheels.dataSourceName#">
175199
CREATE TABLE c_o_r_e_combikeys
176200
(

0 commit comments

Comments
 (0)