Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,11 @@ t.primaryKey(name="userId", autoIncrement=true);

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`.

`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.
`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`.

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.

**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"`.

## Wheels Conventions

Expand Down
2 changes: 2 additions & 0 deletions changelog.d/3337-association-foreignkey-underscore.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- 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)
- 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)
6 changes: 5 additions & 1 deletion vendor/wheels/migrator/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,11 @@ New code should pass `columnNames`. Both keep working.
| `false` (framework default) | `userid` | `userid`, `usertype` |
| `true` (new-app template default) | `user_id` | `user_id`, `user_type` |

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.
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`.

**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.

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.

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.

Expand Down
144 changes: 140 additions & 4 deletions vendor/wheels/model/sql.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -1388,13 +1388,39 @@ component {
lock name="wheelsJoinMemo#application.applicationName#" type="exclusive" timeout="10" {
if (!StructKeyExists(local.classAssociations[local.name], "expandedMetadataFilled")) {
if (!Len(local.classAssociations[local.name].foreignKey)) {
// cfformat-ignore-start
// The foreign key column lives on a different side depending on the association
// type: for `belongsTo` it is a column on THIS model's table, for `hasMany` /
// `hasOne` it is a column on the ASSOCIATED model's table. Resolve the default
// against whichever side actually owns it so both the legacy `<modelName><key>`
// form and the `<modelName>_<key>` form that `useUnderscoreReferenceColumns`
// makes the migrator emit are honoured (#3337).
if (local.classAssociations[local.name].type == "belongsTo") {
local.classAssociations[local.name].foreignKey = local.associatedClass.$classData().modelName & Replace(local.associatedClass.$classData().keys, ",", ",#local.associatedClass.$classData().modelName#", "all");
local.fkNameSource = local.associatedClass;
local.fkColumnOwner = local.class;
} else {
local.classAssociations[local.name].foreignKey = local.class.$classData().modelName & Replace(local.class.$classData().keys, ",", ",#local.class.$classData().modelName#", "all");
local.fkNameSource = local.class;
local.fkColumnOwner = local.associatedClass;
}
local.classAssociations[local.name].foreignKey = $deriveAssociationForeignKey(
columnOwner = local.fkColumnOwner,
modelName = local.fkNameSource.$classData().modelName,
keys = local.fkNameSource.$classData().keys
);
// A derived default matching no column on the owning side can only fail later,
// deep inside the join builder, as `key [xxx] doesn't exist` — a message naming
// neither the association nor the `foreignKey=` argument that fixes it. Report it
// here instead, while both candidate shapes are still in hand (#3337). Runs inside
// the memo so the success path costs one check per application lifetime, and only
// for defaults derived here — an explicit `foreignKey=` is the developer's call.
if (application.wheels.showErrorInformation) {
$assertDerivedForeignKeyResolves(
associationName = local.name,
foreignKey = local.classAssociations[local.name].foreignKey,
columnOwner = local.fkColumnOwner,
modelName = local.fkNameSource.$classData().modelName,
keys = local.fkNameSource.$classData().keys
);
}
// cfformat-ignore-end
}
if (!Len(local.classAssociations[local.name].joinKey)) {
if (local.classAssociations[local.name].type == "belongsTo") {
Expand Down Expand Up @@ -1551,6 +1577,116 @@ component {
return local.rv;
}

/**
* Internal function.
* Builds the conventional foreign key list for an association default: the model name
* prefixed onto each of the target primary keys, joined by `separator`.
*
* `keys` may be a comma list for composite primary keys, so every element gets the
* prefix — `user` + `a,b` yields `usera,userb`, or `user_a,user_b` with an underscore.
*/
public string function $buildForeignKeyList(
required string modelName,
required string keys,
string separator = ""
) {
local.rv = "";
local.keysArray = ListToArray(arguments.keys);
local.iEnd = ArrayLen(local.keysArray);
for (local.i = 1; local.i <= local.iEnd; local.i++) {
local.rv = ListAppend(local.rv, arguments.modelName & arguments.separator & Trim(local.keysArray[local.i]));
}
return local.rv;
}

/**
* Internal function.
* True when every element of a foreign key list is a property on the supplied class.
* Checks property names rather than column names because that is the lookup the join
* builder performs (`properties[foreignKey].column`).
*/
public boolean function $foreignKeyListResolves(required any columnOwner, required string foreignKey) {
local.properties = arguments.columnOwner.$classData().properties;
local.keysArray = ListToArray(arguments.foreignKey);
local.iEnd = ArrayLen(local.keysArray);
for (local.i = 1; local.i <= local.iEnd; local.i++) {
if (!StructKeyExists(local.properties, Trim(local.keysArray[local.i]))) {
return false;
}
}
return local.iEnd > 0;
}

/**
* Internal function.
* Derives the default foreign key for an association, preferring whichever conventional
* shape actually exists on the model that owns the column.
*
* Wheels has two conventions in play. The legacy `<modelName><key>` form is what this
* function has always produced, and `useUnderscoreReferenceColumns` (the `wheels new`
* default) makes the migrator emit `<modelName>_<key>` instead — leaving stock new apps
* with a schema the association default could never match (#3337).
*
* Resolving against the real columns rather than reading the setting fixes both
* conventions at once, including apps that flipped the flag mid-life and therefore hold
* a mix of both shapes. It is also strictly error-reducing: the underscore form is only
* consulted when the legacy form is absent, which is a case that throws today. The
* setting is deliberately NOT consulted — it is read per call by the migrator, whereas
* this result is memoized for the application lifetime, so honouring it here would make
* a runtime flip take effect for migrations but not for models.
*
* Falls back to the legacy shape when neither resolves, leaving the existing error path
* (and `$assertDerivedForeignKeyResolves`) to report it.
*/
public string function $deriveAssociationForeignKey(
required any columnOwner,
required string modelName,
required string keys
) {
local.legacy = $buildForeignKeyList(modelName = arguments.modelName, keys = arguments.keys);
if ($foreignKeyListResolves(columnOwner = arguments.columnOwner, foreignKey = local.legacy)) {
return local.legacy;
}
local.underscored = $buildForeignKeyList(modelName = arguments.modelName, keys = arguments.keys, separator = "_");
if ($foreignKeyListResolves(columnOwner = arguments.columnOwner, foreignKey = local.underscored)) {
return local.underscored;
}
return local.legacy;
}

/**
* Internal function.
* Throws a descriptive error when an association's DERIVED default foreign key matches no
* property on the model that owns the column. Without this the failure surfaces much later
* as `key [xxx] doesn't exist` from inside the join builder, which names neither the
* association nor the argument that fixes it (#3337).
*
* Skipped when the owner has no properties at all — an un-migrated or missing table would
* otherwise produce this error instead of the clearer one the query itself raises.
*/
public void function $assertDerivedForeignKeyResolves(
required string associationName,
required string foreignKey,
required any columnOwner,
required string modelName,
required string keys
) {
if (!StructCount(arguments.columnOwner.$classData().properties)) {
return;
}
if ($foreignKeyListResolves(columnOwner = arguments.columnOwner, foreignKey = arguments.foreignKey)) {
return;
}
local.ownerName = arguments.columnOwner.$classData().modelName;
local.legacy = $buildForeignKeyList(modelName = arguments.modelName, keys = arguments.keys);
local.underscored = $buildForeignKeyList(modelName = arguments.modelName, keys = arguments.keys, separator = "_");
Throw(
type = "Wheels.AssociationForeignKeyNotFound",
message = "The `#arguments.associationName#` association derives a default foreign key of `#arguments.foreignKey#`, which is not a property on the `#local.ownerName#` model.",
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."
);
}

/**
* Internal function.
*/
Expand Down
16 changes: 16 additions & 0 deletions vendor/wheels/tests/_assets/models/RefChild.cfc
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/**
* Fixture for #3337: the child side of an association whose foreign key column uses the
* `<name>_id` convention that `useUnderscoreReferenceColumns` makes the migrator emit.
*
* The association passes no `foreignKey`, so the default derivation has to resolve
* `refparent_id` on this model. Before #3337 it derived `refparentid` unconditionally and
* any `include=` threw `key [refparentid] doesn't exist`.
*/
component extends="Model" {

function config() {
table("c_o_r_e_refchildren");
belongsTo("refParent");
}

}
15 changes: 15 additions & 0 deletions vendor/wheels/tests/_assets/models/RefParent.cfc
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/**
* Fixture for #3337: the parent side of an association whose foreign key column uses the
* `<name>_id` convention that `useUnderscoreReferenceColumns` makes the migrator emit.
*
* Exercises the hasMany branch of the association foreign-key default, where the column
* lives on the ASSOCIATED model (`c_o_r_e_refchildren.refparent_id`).
*/
component extends="Model" {

function config() {
table("c_o_r_e_refparents");
hasMany("refChildren");
}

}
26 changes: 25 additions & 1 deletion vendor/wheels/tests/populate.cfm
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@
</cfloop>

<!--- list of tables to delete --->
<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">
<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">
<!---
On Oracle, append CASCADE CONSTRAINTS so the drop removes incoming FK
references along with the table. PURGE skips the recycle bin so the
Expand Down Expand Up @@ -171,6 +171,30 @@ CREATE TABLE c_o_r_e_collisiontests
) #local.storageEngine#
</cfquery>

<!---
#3337 fixtures: a parent/child pair whose foreign key column uses the `<name>_id`
convention that `useUnderscoreReferenceColumns` makes the migrator emit. The
associations on RefParent / RefChild pass no `foreignKey`, so they only resolve if the
default derivation honours the underscore form as well as the legacy `<name>id` one.
--->
<cfquery name="local.query" datasource="#application.wheels.dataSourceName#">
CREATE TABLE c_o_r_e_refparents
(
id #local.identityColumnType#
,name varchar(50)
,PRIMARY KEY(id)
) #local.storageEngine#
</cfquery>

<cfquery name="local.query" datasource="#application.wheels.dataSourceName#">
CREATE TABLE c_o_r_e_refchildren
(
id #local.identityColumnType#
,refparent_id #local.intColumnType#
,PRIMARY KEY(id)
) #local.storageEngine#
</cfquery>

<cfquery name="local.query" datasource="#application.wheels.dataSourceName#">
CREATE TABLE c_o_r_e_combikeys
(
Expand Down
Loading
Loading