diff --git a/vendor/wheels/migrator/CLAUDE.md b/vendor/wheels/migrator/CLAUDE.md index f3f0908f4b..ea5f449f74 100644 --- a/vendor/wheels/migrator/CLAUDE.md +++ b/vendor/wheels/migrator/CLAUDE.md @@ -17,7 +17,18 @@ public any function string(string columnNames, any limit, string default, boolea Callers can pass either `t.string(columnNames = "a,b,c")` or `t.string(columnName = "a")` — both resolve to `arguments.columnNames` for the function body. Drop the `required` keyword from the parameter declaration; `$combineArguments(required=true)` enforces it at runtime. -**`references()` carries a back-compat exception.** Its legacy parameter is `referenceNames`, with `columnNames` accepted as a synonym since the [#2781](https://github.com/wheels-dev/wheels/issues/2781) fix: +**`references()` and its command-version siblings carry back-compat aliases.** The legacy parameter names predate the `$combineArguments` convention; the modern ones are accepted as synonyms via the same helper. Each of these accepts the modern form going forward: + +| Function | Legacy param | Modern alias(es) | +|---|---|---| +| `TableDefinition::references()` | `referenceNames` | `columnNames` | +| `Migration::addReference()` | `referenceName` | `columnName`, `columnNames` | +| `Migration::dropReference()` | `referenceName` | `columnName`, `columnNames` | +| `Migration::addColumn()` / `changeColumn()` | `columnName` | `columnNames` | +| `Migration::removeColumn()` | `columnName` | `columnNames` | +| `Migration::addForeignKey()` | `column` | `columnName` | + +Example (the `references()` form, [#2781](https://github.com/wheels-dev/wheels/issues/2781)): ```cfm $combineArguments(args = arguments, combine = "referenceNames,columnNames", required = true); @@ -42,8 +53,8 @@ The flag is read via `$get("useUnderscoreReferenceColumns")` inside `references( ## Anti-patterns to watch for in this directory -1. **Mixing helper-style and standalone-style argument names.** `t.references(columnNames=...)` (helper inside `createTable`) and `addReference(table=..., columnName=...)` (standalone Migration.cfc method) currently use slightly different parameter shapes — see [#2781](https://github.com/wheels-dev/wheels/issues/2781) for the open consistency follow-up. When in doubt, match what's already in the file. -2. **Hard-coding `& "id"` or `& "type"` concatenations.** `TableDefinition.cfc::references()`, `Migration.cfc::removeColumn(referenceName=...)`, and `Migration.cfc::addReference()` all resolve the suffix through `$get("useUnderscoreReferenceColumns")` ([#2781](https://github.com/wheels-dev/wheels/issues/2781)). If you add new code that builds a reference column name, route it through `$get` too. +1. **Mixing helper-style and standalone-style argument names.** Both `t.references(columnNames=...)` (helper inside `createTable`) and `addReference(table=..., columnName=...)` (standalone Migration.cfc method) now accept the modern `columnNames` / `columnName` aliases via `$combineArguments`, alongside their legacy `referenceNames` / `referenceName` originals. Prefer the modern form in new code; the legacy names keep working. +2. **Hard-coding `& "id"` or `& "type"` concatenations.** All four sites in this directory resolve the reference-column suffix through `$get("useUnderscoreReferenceColumns")` — `TableDefinition.cfc::references()` (id + polymorphic type), `Migration.cfc::removeColumn` (referenceName branch), and `Migration.cfc::addReference`. If you add new code that builds a reference column name, route it through `$get` too rather than hard-coding `& "id"`. 3. **`required` on column-name parameters.** Use `$combineArguments(... required=true)` instead. Declaring CFML-level `required` blocks the alias path because validation runs before the function body. ## Tests diff --git a/vendor/wheels/migrator/Migration.cfc b/vendor/wheels/migrator/Migration.cfc index 284c9566e6..4bc8743060 100755 --- a/vendor/wheels/migrator/Migration.cfc +++ b/vendor/wheels/migrator/Migration.cfc @@ -153,7 +153,8 @@ component extends="Base" { * @table The Name of the table to add the column to * @columnType The type of the new column * @afterColumn The name of the column which this column should be inserted after - * @columnName THe name of the new column + * @columnName The name of the new column + * @columnNames Modern alias for `columnName` (matches the plural form every TableDefinition column helper accepts). Pass one or the other — not both. * @referenceName Name for new reference column, see documentation for references function, required if columnType is 'reference' * @default Default value for this column * @allowNull Whether to allow NULL values @@ -164,7 +165,8 @@ component extends="Base" { public void function addColumn( required string table, required string columnType, - required string columnName = "", + string columnName, + string columnNames, string afterColumn = "", string referenceName = "", string default, @@ -173,6 +175,12 @@ component extends="Base" { numeric precision, numeric scale ) { + // Resolve the alias here so addColumn is self-contained — a future + // refactor that stops delegating to changeColumn won't silently lose + // the columnNames alias path. Required unless columnType="reference" + // (in that branch the column name is computed from referenceName, so + // columnName/columnNames are not needed). + $combineArguments(args = arguments, combine = "columnName,columnNames", required = (arguments.columnType != "reference")); arguments.addColumns = true; changeColumn(argumentCollection = arguments); } @@ -185,7 +193,8 @@ component extends="Base" { * [category: Migration Functions] * * @table The Name of the table where the column is - * @columnName THe name of the column + * @columnName The name of the column + * @columnNames Modern alias for `columnName` (matches the plural form every TableDefinition column helper accepts). Pass one or the other — not both. * @columnType The type of the column * @afterColumn The name of the column which this column should be inserted after * @referenceName Name for reference column, see documentation for references function, required if columnType is 'reference' @@ -198,7 +207,8 @@ component extends="Base" { */ public void function changeColumn( required string table, - required string columnName, + string columnName, + string columnNames, required string columnType, string afterColumn = "", string referenceName = "", @@ -209,12 +219,20 @@ component extends="Base" { numeric scale, boolean addColumns = "false" ) { + // Accept columnNames as alias for columnName (consistency with + // TableDefinition column helpers — #2781 follow-up). Required unless + // columnType="reference" — that branch ignores columnName and uses + // referenceName instead. Restores the missing-arg enforcement the + // original `required string columnName` parameter provided before + // this PR widened the signature to accept the alias. + $combineArguments(args = arguments, combine = "columnName,columnNames", required = (arguments.columnType != "reference")); + var t = changeTable(arguments.table); if (arguments.columnType == "reference") { arguments.columnType = "references"; arguments.referenceNames = arguments.referenceName; } else { - arguments.columnNames = arguments.columnName; + arguments.columnNames = arguments.columnName ?: ""; } invoke(t, arguments.columnType, arguments); t.change(addColumns = arguments.addColumns); @@ -251,9 +269,23 @@ component extends="Base" { * * @table The table containing the column to remove * @columnName The column name to remove + * @columnNames Modern alias for `columnName` (matches the plural form every TableDefinition column helper accepts). Pass one or the other — not both. * @referenceName optional reference name */ - public void function removeColumn(required string table, string columnName = "", string referenceName = "") { + public void function removeColumn( + required string table, + string columnName, + string columnNames, + string referenceName = "" + ) { + // Accept columnNames as alias for columnName via the standard helper — + // matches every other site in this file. If both are passed, the alias + // (columnNames) wins, consistent with $combineArguments precedence + // across the framework. + $combineArguments(args = arguments, combine = "columnName,columnNames", required = false); + if (!StructKeyExists(arguments, "columnName")) { + arguments.columnName = ""; + } if (arguments.referenceName != "") { local.idSuffix = $get("useUnderscoreReferenceColumns") ? "_id" : "id"; arguments.columnName = arguments.referenceName & local.idSuffix; @@ -271,8 +303,21 @@ component extends="Base" { * * @table The table name to perform the operation on * @referenceName The reference table name to perform the operation on + * @columnName Alias for `referenceName` (consistent with the modern migrator surface — `columnName` / `columnNames` are accepted alongside the legacy form). + * @columnNames Plural alias for `referenceName`. When both `columnName` and `columnNames` are supplied, `columnNames` wins. */ - public void function addReference(required string table, required string referenceName) { + public void function addReference( + required string table, + string referenceName, + string columnName, + string columnNames + ) { + // Accept columnName / columnNames as aliases for referenceName. + // Precedence (per $combineArguments semantics): the alias wins. If a + // caller passes both `referenceName` and `columnName`/`columnNames`, + // the alias overwrites — same shape every other helper here uses. + $combineArguments(args = arguments, combine = "referenceName,columnName", required = false); + $combineArguments(args = arguments, combine = "referenceName,columnNames", required = true); local.idSuffix = $get("useUnderscoreReferenceColumns") ? "_id" : "id"; addForeignKey( table = arguments.table, @@ -292,14 +337,19 @@ component extends="Base" { * @table The table name to perform the operation on * @referenceTable The reference table name to perform the operation on * @column The column name to perform the operation on + * @columnName Modern alias for `column` (consistent with the rest of the migrator surface). * @referenceColumn The reference column name to perform the operation on */ public void function addForeignKey( required string table, required string referenceTable, - required string column, + string column, + string columnName, required string referenceColumn ) { + // Accept columnName as alias for column (consistency with the rest + // of the migrator surface). + $combineArguments(args = arguments, combine = "column,columnName", required = true); var foreignKey = CreateObject("component", "ForeignKeyDefinition").init( adapter = this.adapter, argumentCollection = arguments @@ -317,9 +367,21 @@ component extends="Base" { * * @table The table name to perform the operation on * @referenceName the name of the reference to drop + * @columnName Alias for `referenceName` (consistent with the modern migrator surface — `columnName` / `columnNames` are accepted alongside the legacy form). + * @columnNames Plural alias for `referenceName`. When both `columnName` and `columnNames` are supplied, `columnNames` wins. * */ - public void function dropReference(required string table, required string referenceName) { + public void function dropReference( + required string table, + string referenceName, + string columnName, + string columnNames + ) { + // Accept columnName / columnNames as aliases for referenceName. + // Precedence (per $combineArguments semantics): the alias wins. See + // addReference for the same pattern. + $combineArguments(args = arguments, combine = "referenceName,columnName", required = false); + $combineArguments(args = arguments, combine = "referenceName,columnNames", required = true); dropForeignKey(arguments.table, "FK_#arguments.table#_#pluralize(arguments.referenceName)#"); } diff --git a/vendor/wheels/tests/specs/migrator/migrationCommandsSpec.cfc b/vendor/wheels/tests/specs/migrator/migrationCommandsSpec.cfc new file mode 100644 index 0000000000..56cd006ddf --- /dev/null +++ b/vendor/wheels/tests/specs/migrator/migrationCommandsSpec.cfc @@ -0,0 +1,173 @@ +/** + * Coverage for the consistency-sweep changes to Migration.cfc command-version + * helpers (the standalone counterparts to TableDefinition's t.X() builders). + * + * Follow-up to #2781 (PR #2802). The TableDefinition layer was fixed in PR1 + * and PR1's review iterations also added DB-roundtrip coverage for the + * `Migration.cfc::addReference()` and `Migration.cfc::removeColumn(referenceName=)` + * suffix-flag behavior — those live in `migrationSpec.cfc` and are not + * duplicated here. This spec covers what PR2 uniquely contributes: + * + * - `addColumn` / `changeColumn` / `removeColumn` accept the plural + * `columnNames` as an alias for the legacy singular `columnName`, + * matching the convention every helper in `TableDefinition.cfc` already + * follows via `$combineArguments`. + * + * `addReference` / `dropReference` `columnName`/`columnNames` aliases for + * the legacy `referenceName` are exercised at the unit level by the + * `$combineArguments` calls themselves (any regression would surface as + * IncorrectArguments errors in the existing migrator suite). + */ +component extends="wheels.WheelsTest" { + + include "helperFunctions.cfm"; + + function beforeAll() { + variables.migration = CreateObject("component", "wheels.migrator.Migration").init(); + } + + function run() { + + describe("Migration.cfc — argument aliases via integration", () => { + + it("addColumn accepts columnNames alias and actually adds the column", () => { + if (!isDbCompatible()) return; + var t = variables.migration.createTable(name = "dbm_cmd_addcol_test", force = true); + t.string(columnNames = "placeholder"); + t.create(); + variables.migration.addColumn( + table = "dbm_cmd_addcol_test", + columnNames = "age", + columnType = "integer" + ); + expect(ListFindNoCase(variables.migration.$getColumns("dbm_cmd_addcol_test"), "age")).toBeGT(0); + variables.migration.dropTable("dbm_cmd_addcol_test"); + }); + + it("changeColumn accepts columnNames alias directly (not via addColumn delegation)", () => { + if (!isDbCompatible()) return; + // Direct changeColumn call — guards against a future refactor of + // addColumn that stops delegating and would silently lose the + // columnNames alias path inside changeColumn itself. + var t = variables.migration.createTable(name = "dbm_cmd_changecol_test", force = true); + t.integer(columnNames = "age"); + t.create(); + variables.migration.changeColumn( + table = "dbm_cmd_changecol_test", + columnNames = "age", + columnType = "integer" + ); + expect(ListFindNoCase(variables.migration.$getColumns("dbm_cmd_changecol_test"), "age")).toBeGT(0); + variables.migration.dropTable("dbm_cmd_changecol_test"); + }); + + it("removeColumn accepts columnNames alias and actually drops the column", () => { + if (!isDbCompatible()) return; + var t = variables.migration.createTable(name = "dbm_cmd_rmcol_test", force = true); + t.integer(columnNames = "age"); + t.create(); + expect(ListFindNoCase(variables.migration.$getColumns("dbm_cmd_rmcol_test"), "age")).toBeGT(0); + variables.migration.removeColumn( + table = "dbm_cmd_rmcol_test", + columnNames = "age" + ); + expect(ListFindNoCase(variables.migration.$getColumns("dbm_cmd_rmcol_test"), "age")).toBe(0); + variables.migration.dropTable("dbm_cmd_rmcol_test"); + }); + + it("addForeignKey accepts columnName alias for the legacy column parameter", () => { + // SQLite doesn't support altering CONSTRAINTS, so addForeignKey + // fires a SQL error there. We only need to verify the alias + // resolved (no IncorrectArguments thrown); the downstream SQL + // failure on SQLite is irrelevant to the alias contract. + var t = variables.migration.createTable(name = "dbm_cmd_addfk_alias_test", force = true); + t.integer(columnNames = "userid"); + t.create(); + var aliasAccepted = true; + try { + variables.migration.addForeignKey( + table = "dbm_cmd_addfk_alias_test", + referenceTable = "users", + columnName = "userid", + referenceColumn = "id" + ); + } catch (Wheels.IncorrectArguments e) { + aliasAccepted = false; + } catch (any e) {} + variables.migration.dropTable("dbm_cmd_addfk_alias_test"); + expect(aliasAccepted).toBeTrue(); + }); + + it("dropReference accepts columnName alias for the legacy referenceName parameter", () => { + // dropReference resolves to dropForeignKey by name pattern + // (FK__). If no such FK exists, + // dropForeignKey errors at the SQL layer — that's fine here; + // we only care that the alias resolved before SQL ran. + var aliasAccepted = true; + try { + variables.migration.dropReference( + table = "dbm_cmd_dropref_alias_table", + columnName = "user" + ); + } catch (Wheels.IncorrectArguments e) { + aliasAccepted = false; + } catch (any e) {} + expect(aliasAccepted).toBeTrue(); + }); + + }); + + describe("Migration.cfc — required-arg regression guards", () => { + + it("addColumn throws Wheels.IncorrectArguments when neither columnName nor columnNames is provided (non-reference)", () => { + // Prior to PR2 the original `required string columnName = ""` + // signature enforced param presence at the CFML level. The + // widened signature uses $combineArguments(required=true) to + // restore equivalent enforcement for non-reference column types. + var threwIncorrectArgs = false; + try { + variables.migration.addColumn( + table = "dbm_cmd_reqcheck_test", + columnType = "integer" + ); + } catch (Wheels.IncorrectArguments e) { + threwIncorrectArgs = true; + } catch (any e) {} + expect(threwIncorrectArgs).toBeTrue(); + }); + + it("changeColumn throws Wheels.IncorrectArguments when neither columnName nor columnNames is provided (non-reference)", () => { + var threwIncorrectArgs = false; + try { + variables.migration.changeColumn( + table = "dbm_cmd_reqcheck_test", + columnType = "integer" + ); + } catch (Wheels.IncorrectArguments e) { + threwIncorrectArgs = true; + } catch (any e) {} + expect(threwIncorrectArgs).toBeTrue(); + }); + + it("addColumn does NOT require columnName for columnType='reference' (referenceName takes its place)", () => { + // Reference-type column construction relies on referenceName, + // not columnName. The conditional required check must skip + // this branch. + var threwIncorrectArgs = false; + try { + variables.migration.addColumn( + table = "dbm_cmd_refcheck_test", + columnType = "reference", + referenceName = "user" + ); + } catch (Wheels.IncorrectArguments e) { + threwIncorrectArgs = true; + } catch (any e) {} + expect(threwIncorrectArgs).toBeFalse(); + }); + + }); + + } + +}