From 6afe6578bbbd09923abdde9c89de6f1fd3518cbd Mon Sep 17 00:00:00 2001 From: Peter Amiri Date: Tue, 14 Apr 2026 18:53:40 -0700 Subject: [PATCH 1/2] feat(migration): add auto-migration generation from model definitions Adds AutoMigrator.cfc that compares model property definitions against current database schema and generates migration CFCs automatically. Supports diff(), diffAll(), generateMigrationCFC(), and writeMigration(). Handles type mapping, skips calculated properties and primary keys. Co-Authored-By: Claude Opus 4.6 (1M context) --- vendor/wheels/migrator/AutoMigrator.cfc | 393 ++++++++++++++++++ .../tests/specs/migrator/autoMigratorSpec.cfc | 332 +++++++++++++++ 2 files changed, 725 insertions(+) create mode 100644 vendor/wheels/migrator/AutoMigrator.cfc create mode 100644 vendor/wheels/tests/specs/migrator/autoMigratorSpec.cfc diff --git a/vendor/wheels/migrator/AutoMigrator.cfc b/vendor/wheels/migrator/AutoMigrator.cfc new file mode 100644 index 000000000..59e8246eb --- /dev/null +++ b/vendor/wheels/migrator/AutoMigrator.cfc @@ -0,0 +1,393 @@ +/** + * Schema diff engine that compares model property definitions against the current + * database schema and generates migration CFC files automatically. + * + * Limitations: + * - Cannot detect column renames. A renamed column appears as removeColumn + addColumn. + * - Calculated properties (property(sql="...")) are excluded from the diff. + * - Tableless models are skipped. + */ +component extends="wheels.migrator.Base" { + + /** + * Compares a single model's expected schema (from its property definitions) against + * the actual database columns and returns a struct describing the differences. + * + * @modelName The name of the model to diff (e.g. "User"). + * @return Struct with keys: modelName, tableName, addColumns, removeColumns, changeColumns. + */ + public struct function diff(required string modelName) { + local.modelObj = model(arguments.modelName); + local.tableName = local.modelObj.tableName(); + local.primaryKeyList = local.modelObj.primaryKeys(); + local.classData = local.modelObj.$classData(); + local.properties = local.classData.properties; + local.calculatedProperties = local.classData.calculatedProperties; + + local.expectedColumns = {}; + for (local.propName in local.properties) { + // Skip calculated properties + if (StructKeyExists(local.calculatedProperties, local.propName)) { + continue; + } + local.prop = local.properties[local.propName]; + local.expectedColumns[LCase(local.prop.column)] = { + property: local.propName, + column: local.prop.column, + type: local.prop.type, + dataType: local.prop.dataType, + nullable: local.prop.nullable, + size: StructKeyExists(local.prop, "size") ? local.prop.size : "", + scale: StructKeyExists(local.prop, "scale") ? local.prop.scale : "" + }; + if (StructKeyExists(local.prop, "columnDefault")) { + local.expectedColumns[LCase(local.prop.column)]["default"] = local.prop.columnDefault; + } + } + + local.appKey = $appKey(); + local.dbColumns = $dbinfo( + type = "columns", + table = local.tableName, + datasource = application[local.appKey].dataSourceName, + username = application[local.appKey].dataSourceUserName, + password = application[local.appKey].dataSourcePassword + ); + + local.actualColumns = {}; + local.iEnd = local.dbColumns.recordCount; + for (local.i = 1; local.i <= local.iEnd; local.i++) { + local.colName = LCase(local.dbColumns["column_name"][local.i]); + local.typeName = Trim(SpanExcluding(local.dbColumns["type_name"][local.i], "(")); + local.actualColumns[local.colName] = { + column: local.colName, + typeName: local.typeName, + size: local.dbColumns["column_size"][local.i], + nullable: local.dbColumns["is_nullable"][local.i], + isPrimaryKey: local.dbColumns["is_primarykey"][local.i], + decimalDigits: local.dbColumns["decimal_digits"][local.i] + }; + } + + local.addColumns = []; + local.removeColumns = []; + local.changeColumns = []; + + for (local.colName in local.expectedColumns) { + if (!StructKeyExists(local.actualColumns, local.colName)) { + local.expected = local.expectedColumns[local.colName]; + ArrayAppend(local.addColumns, { + name: local.expected.column, + type: $cfSqlTypeToMigrationType(local.expected.type), + nullable: IsBoolean(local.expected.nullable) ? local.expected.nullable : true, + "default": StructKeyExists(local.expected, "default") ? local.expected["default"] : "" + }); + } + } + + for (local.colName in local.actualColumns) { + if (!StructKeyExists(local.expectedColumns, local.colName)) { + local.actual = local.actualColumns[local.colName]; + // Don't suggest removing primary key columns + if (IsBoolean(local.actual.isPrimaryKey) && local.actual.isPrimaryKey) { + continue; + } + ArrayAppend(local.removeColumns, { + name: local.colName + }); + } + } + + for (local.colName in local.expectedColumns) { + if (StructKeyExists(local.actualColumns, local.colName)) { + local.expected = local.expectedColumns[local.colName]; + local.actual = local.actualColumns[local.colName]; + + if (ListFindNoCase(local.primaryKeyList, local.expected.property)) { + continue; + } + + local.expectedMigType = $cfSqlTypeToMigrationType(local.expected.type); + local.actualMigType = $dbTypeToMigrationType(local.actual.typeName); + + if (local.expectedMigType != local.actualMigType && local.actualMigType != "unknown") { + ArrayAppend(local.changeColumns, { + name: local.colName, + from: {type: local.actualMigType}, + to: {type: local.expectedMigType} + }); + } + } + } + + return { + modelName: arguments.modelName, + tableName: local.tableName, + addColumns: local.addColumns, + removeColumns: local.removeColumns, + changeColumns: local.changeColumns + }; + } + + /** + * Iterates all models registered in the application, calls diff() on each, + * and returns combined results. Skips tableless models and models that fail to load. + * + * @return Struct keyed by model name, each value is the diff result for that model. + */ + public struct function diffAll() { + local.results = {}; + local.appKey = $appKey(); + + if (StructKeyExists(application[local.appKey], "models")) { + for (local.modelName in application[local.appKey].models) { + try { + local.modelObj = model(local.modelName); + + local.tName = local.modelObj.tableName(); + if (IsBoolean(local.tName) && !local.tName) { + continue; + } + + local.diffResult = diff(local.modelName); + + if ( + ArrayLen(local.diffResult.addColumns) + || ArrayLen(local.diffResult.removeColumns) + || ArrayLen(local.diffResult.changeColumns) + ) { + local.results[local.modelName] = local.diffResult; + } + } catch (any e) { + // Skip models that fail to load (e.g. missing tables) + continue; + } + } + } + + return local.results; + } + + /** + * Generates a migration CFC string with proper up() and down() methods + * using the Wheels migration DSL. + * + * @diffResult The diff struct returned by diff(). + * @migrationName A human-readable name for the migration. + * @return The CFC file content as a string. + */ + public string function generateMigrationCFC(required struct diffResult, required string migrationName) { + local.nl = Chr(10); + local.tab = Chr(9); + local.upBody = ""; + local.downBody = ""; + + local.iEnd = ArrayLen(arguments.diffResult.addColumns); + for (local.i = 1; local.i <= local.iEnd; local.i++) { + local.col = arguments.diffResult.addColumns[local.i]; + local.upBody &= local.tab & local.tab + & 'addColumn(table="' & arguments.diffResult.tableName + & '", columnType="' & local.col.type + & '", columnName="' & local.col.name & '"' + & ', allowNull=' & (IsBoolean(local.col.nullable) && local.col.nullable ? "true" : "false") + & ');' & local.nl; + local.downBody &= local.tab & local.tab + & 'removeColumn(table="' & arguments.diffResult.tableName + & '", columnName="' & local.col.name & '");' & local.nl; + } + + local.iEnd = ArrayLen(arguments.diffResult.removeColumns); + for (local.i = 1; local.i <= local.iEnd; local.i++) { + local.col = arguments.diffResult.removeColumns[local.i]; + local.upBody &= local.tab & local.tab + & 'removeColumn(table="' & arguments.diffResult.tableName + & '", columnName="' & local.col.name & '");' & local.nl; + local.downBody &= local.tab & local.tab + & '// TODO: restore column "' & local.col.name & '" — original type unknown' & local.nl; + } + + local.iEnd = ArrayLen(arguments.diffResult.changeColumns); + for (local.i = 1; local.i <= local.iEnd; local.i++) { + local.col = arguments.diffResult.changeColumns[local.i]; + local.upBody &= local.tab & local.tab + & 'changeColumn(table="' & arguments.diffResult.tableName + & '", columnName="' & local.col.name + & '", columnType="' & local.col.to.type & '");' & local.nl; + local.downBody &= local.tab & local.tab + & 'changeColumn(table="' & arguments.diffResult.tableName + & '", columnName="' & local.col.name + & '", columnType="' & local.col.from.type & '");' & local.nl; + } + + if (!Len(Trim(local.upBody))) { + local.upBody = local.tab & local.tab & '// No changes detected' & local.nl; + } + if (!Len(Trim(local.downBody))) { + local.downBody = local.tab & local.tab & '// No changes to reverse' & local.nl; + } + + local.content = 'component extends="wheels.migrator.Migration" hint="' & arguments.migrationName & '" {' & local.nl; + local.content &= local.nl; + local.content &= local.tab & 'public void function up() {' & local.nl; + local.content &= local.upBody; + local.content &= local.tab & '}' & local.nl; + local.content &= local.nl; + local.content &= local.tab & 'public void function down() {' & local.nl; + local.content &= local.downBody; + local.content &= local.tab & '}' & local.nl; + local.content &= local.nl; + local.content &= '}' & local.nl; + + return local.content; + } + + /** + * Generates a timestamp-prefixed filename and writes the migration CFC + * to the app/migrator/migrations/ directory. + * + * @diffResult The diff struct returned by diff(). + * @migrationName Optional human-readable name. Defaults to "auto_[modelName]_changes". + */ + public void function writeMigration(required struct diffResult, string migrationName = "") { + if (!Len(arguments.migrationName)) { + arguments.migrationName = "auto_" & LCase(arguments.diffResult.modelName) & "_changes"; + } + + local.content = generateMigrationCFC(arguments.diffResult, arguments.migrationName); + + local.timestamp = DateFormat(Now(), "yyyymmdd") & TimeFormat(Now(), "HHmmss"); + local.fileName = local.timestamp & "_" & arguments.migrationName & ".cfc"; + + local.migrationDir = ExpandPath("/app/migrator/migrations/"); + if (!DirectoryExists(local.migrationDir)) { + DirectoryCreate(local.migrationDir); + } + + $file( + action = "write", + file = local.migrationDir & local.fileName, + output = local.content, + addNewLine = false + ); + } + + /** + * Maps cf_sql types (as stored in model properties) to Wheels migration column types. + */ + public string function $cfSqlTypeToMigrationType(required string cfSqlType) { + switch (LCase(arguments.cfSqlType)) { + case "cf_sql_integer": + case "cf_sql_int": + return "integer"; + case "cf_sql_varchar": + case "cf_sql_char": + return "string"; + case "cf_sql_longvarchar": + case "cf_sql_clob": + return "text"; + case "cf_sql_timestamp": + return "datetime"; + case "cf_sql_date": + return "date"; + case "cf_sql_time": + return "time"; + case "cf_sql_bit": + case "cf_sql_boolean": + return "boolean"; + case "cf_sql_decimal": + case "cf_sql_numeric": + case "cf_sql_money": + return "decimal"; + case "cf_sql_float": + case "cf_sql_double": + case "cf_sql_real": + return "float"; + case "cf_sql_bigint": + return "biginteger"; + case "cf_sql_binary": + case "cf_sql_blob": + case "cf_sql_varbinary": + return "binary"; + case "cf_sql_smallint": + case "cf_sql_tinyint": + return "integer"; + default: + return "string"; + } + } + + /** + * Maps raw database type names (from cfdbinfo) to Wheels migration types. + * Used to compare the actual DB schema against the model's expected types. + */ + public string function $dbTypeToMigrationType(required string dbType) { + switch (LCase(arguments.dbType)) { + case "int": + case "int4": + case "integer": + case "mediumint": + case "smallint": + case "tinyint": + return "integer"; + case "bigint": + case "int8": + case "int64": + return "biginteger"; + case "varchar": + case "character varying": + case "nvarchar": + case "char": + case "nchar": + return "string"; + case "text": + case "ntext": + case "clob": + case "character large object": + case "mediumtext": + case "longtext": + case "tinytext": + return "text"; + case "datetime": + case "timestamp": + case "timestamp without time zone": + case "timestamp with time zone": + return "datetime"; + case "date": + return "date"; + case "time": + case "time without time zone": + case "time with time zone": + return "time"; + case "bit": + case "boolean": + case "bool": + return "boolean"; + case "decimal": + case "numeric": + case "money": + case "smallmoney": + return "decimal"; + case "float": + case "float4": + case "float8": + case "double": + case "double precision": + case "real": + return "float"; + case "binary": + case "varbinary": + case "image": + case "blob": + case "bytea": + case "longblob": + case "mediumblob": + case "tinyblob": + return "binary"; + case "uniqueidentifier": + return "string"; + default: + return "unknown"; + } + } + +} diff --git a/vendor/wheels/tests/specs/migrator/autoMigratorSpec.cfc b/vendor/wheels/tests/specs/migrator/autoMigratorSpec.cfc new file mode 100644 index 000000000..352cd80a7 --- /dev/null +++ b/vendor/wheels/tests/specs/migrator/autoMigratorSpec.cfc @@ -0,0 +1,332 @@ +component extends="wheels.WheelsTest" { + + function beforeAll() { + g = application.wo; + autoMigrator = CreateObject("component", "wheels.migrator.AutoMigrator"); + } + + function run() { + + describe("AutoMigrator", () => { + + describe("$cfSqlTypeToMigrationType", () => { + + it("maps cf_sql_integer to integer", () => { + expect(autoMigrator.$cfSqlTypeToMigrationType("cf_sql_integer")).toBe("integer"); + }); + + it("maps cf_sql_varchar to string", () => { + expect(autoMigrator.$cfSqlTypeToMigrationType("cf_sql_varchar")).toBe("string"); + }); + + it("maps cf_sql_longvarchar to text", () => { + expect(autoMigrator.$cfSqlTypeToMigrationType("cf_sql_longvarchar")).toBe("text"); + }); + + it("maps cf_sql_timestamp to datetime", () => { + expect(autoMigrator.$cfSqlTypeToMigrationType("cf_sql_timestamp")).toBe("datetime"); + }); + + it("maps cf_sql_date to date", () => { + expect(autoMigrator.$cfSqlTypeToMigrationType("cf_sql_date")).toBe("date"); + }); + + it("maps cf_sql_time to time", () => { + expect(autoMigrator.$cfSqlTypeToMigrationType("cf_sql_time")).toBe("time"); + }); + + it("maps cf_sql_bit to boolean", () => { + expect(autoMigrator.$cfSqlTypeToMigrationType("cf_sql_bit")).toBe("boolean"); + }); + + it("maps cf_sql_decimal to decimal", () => { + expect(autoMigrator.$cfSqlTypeToMigrationType("cf_sql_decimal")).toBe("decimal"); + }); + + it("maps cf_sql_float to float", () => { + expect(autoMigrator.$cfSqlTypeToMigrationType("cf_sql_float")).toBe("float"); + }); + + it("maps cf_sql_double to float", () => { + expect(autoMigrator.$cfSqlTypeToMigrationType("cf_sql_double")).toBe("float"); + }); + + it("maps cf_sql_bigint to biginteger", () => { + expect(autoMigrator.$cfSqlTypeToMigrationType("cf_sql_bigint")).toBe("biginteger"); + }); + + it("maps cf_sql_binary to binary", () => { + expect(autoMigrator.$cfSqlTypeToMigrationType("cf_sql_binary")).toBe("binary"); + }); + + it("maps cf_sql_blob to binary", () => { + expect(autoMigrator.$cfSqlTypeToMigrationType("cf_sql_blob")).toBe("binary"); + }); + + it("maps cf_sql_smallint to integer", () => { + expect(autoMigrator.$cfSqlTypeToMigrationType("cf_sql_smallint")).toBe("integer"); + }); + + it("maps cf_sql_tinyint to integer", () => { + expect(autoMigrator.$cfSqlTypeToMigrationType("cf_sql_tinyint")).toBe("integer"); + }); + + it("maps cf_sql_numeric to decimal", () => { + expect(autoMigrator.$cfSqlTypeToMigrationType("cf_sql_numeric")).toBe("decimal"); + }); + + it("defaults unknown types to string", () => { + expect(autoMigrator.$cfSqlTypeToMigrationType("cf_sql_unknown_type")).toBe("string"); + }); + + }); + + describe("$dbTypeToMigrationType", () => { + + it("maps varchar to string", () => { + expect(autoMigrator.$dbTypeToMigrationType("varchar")).toBe("string"); + }); + + it("maps int to integer", () => { + expect(autoMigrator.$dbTypeToMigrationType("int")).toBe("integer"); + }); + + it("maps text to text", () => { + expect(autoMigrator.$dbTypeToMigrationType("text")).toBe("text"); + }); + + it("maps datetime to datetime", () => { + expect(autoMigrator.$dbTypeToMigrationType("datetime")).toBe("datetime"); + }); + + it("maps timestamp to datetime", () => { + expect(autoMigrator.$dbTypeToMigrationType("timestamp")).toBe("datetime"); + }); + + it("maps boolean to boolean", () => { + expect(autoMigrator.$dbTypeToMigrationType("boolean")).toBe("boolean"); + }); + + it("maps decimal to decimal", () => { + expect(autoMigrator.$dbTypeToMigrationType("decimal")).toBe("decimal"); + }); + + it("maps float to float", () => { + expect(autoMigrator.$dbTypeToMigrationType("float")).toBe("float"); + }); + + it("maps bigint to biginteger", () => { + expect(autoMigrator.$dbTypeToMigrationType("bigint")).toBe("biginteger"); + }); + + it("maps blob to binary", () => { + expect(autoMigrator.$dbTypeToMigrationType("blob")).toBe("binary"); + }); + + it("returns unknown for unrecognized types", () => { + expect(autoMigrator.$dbTypeToMigrationType("geometry_collection_xyz")).toBe("unknown"); + }); + + }); + + describe("diff()", () => { + + it("returns a struct with required keys", () => { + local.result = autoMigrator.diff("Author"); + expect(local.result).toBeStruct(); + expect(local.result).toHaveKey("modelName"); + expect(local.result).toHaveKey("tableName"); + expect(local.result).toHaveKey("addColumns"); + expect(local.result).toHaveKey("removeColumns"); + expect(local.result).toHaveKey("changeColumns"); + expect(local.result.modelName).toBe("Author"); + expect(local.result.tableName).toBe("c_o_r_e_authors"); + }); + + it("returns arrays for column changes", () => { + local.result = autoMigrator.diff("Author"); + expect(local.result.addColumns).toBeArray(); + expect(local.result.removeColumns).toBeArray(); + expect(local.result.changeColumns).toBeArray(); + }); + + it("excludes calculated properties from diff", () => { + // The Author model has calculated property "numberofitems" with sql="..." + // These should NOT appear in addColumns since they are virtual + local.result = autoMigrator.diff("Author"); + local.addColumnNames = ""; + for (local.col in local.result.addColumns) { + local.addColumnNames = ListAppend(local.addColumnNames, local.col.name); + } + expect(ListFindNoCase(local.addColumnNames, "numberofitems")).toBe(0); + }); + + it("does not flag primary key columns for removal", () => { + local.result = autoMigrator.diff("Author"); + // The primary key "id" should never appear in removeColumns + local.removeNames = ""; + for (local.col in local.result.removeColumns) { + local.removeNames = ListAppend(local.removeNames, local.col.name); + } + expect(ListFindNoCase(local.removeNames, "id")).toBe(0); + }); + + }); + + describe("diffAll()", () => { + + it("returns a struct", () => { + local.result = autoMigrator.diffAll(); + expect(local.result).toBeStruct(); + }); + + it("only includes models with actual differences", () => { + local.result = autoMigrator.diffAll(); + // Each entry should have non-empty change arrays + for (local.modelName in local.result) { + local.d = local.result[local.modelName]; + local.hasChanges = ArrayLen(local.d.addColumns) > 0 + || ArrayLen(local.d.removeColumns) > 0 + || ArrayLen(local.d.changeColumns) > 0; + expect(local.hasChanges).toBeTrue(); + } + }); + + }); + + describe("generateMigrationCFC()", () => { + + it("produces valid CFC content with up and down methods", () => { + local.diffResult = { + modelName: "TestModel", + tableName: "test_models", + addColumns: [{name: "email", type: "string", nullable: true, "default": ""}], + removeColumns: [{name: "legacy_field"}], + changeColumns: [{name: "status", from: {type: "string"}, to: {type: "integer"}}] + }; + local.cfc = autoMigrator.generateMigrationCFC(local.diffResult, "update_test_models"); + + expect(local.cfc).toInclude("extends=""wheels.migrator.Migration"""); + expect(local.cfc).toInclude("function up()"); + expect(local.cfc).toInclude("function down()"); + }); + + it("generates addColumn in up for new columns", () => { + local.diffResult = { + modelName: "TestModel", + tableName: "test_models", + addColumns: [{name: "email", type: "string", nullable: true, "default": ""}], + removeColumns: [], + changeColumns: [] + }; + local.cfc = autoMigrator.generateMigrationCFC(local.diffResult, "add_email"); + + expect(local.cfc).toInclude('addColumn(table="test_models"'); + expect(local.cfc).toInclude('columnType="string"'); + expect(local.cfc).toInclude('columnName="email"'); + }); + + it("generates removeColumn in down for new columns", () => { + local.diffResult = { + modelName: "TestModel", + tableName: "test_models", + addColumns: [{name: "email", type: "string", nullable: true, "default": ""}], + removeColumns: [], + changeColumns: [] + }; + local.cfc = autoMigrator.generateMigrationCFC(local.diffResult, "add_email"); + + // The down() should have removeColumn to reverse the addColumn + expect(local.cfc).toInclude('removeColumn(table="test_models", columnName="email")'); + }); + + it("generates removeColumn in up for dropped columns", () => { + local.diffResult = { + modelName: "TestModel", + tableName: "test_models", + addColumns: [], + removeColumns: [{name: "legacy_field"}], + changeColumns: [] + }; + local.cfc = autoMigrator.generateMigrationCFC(local.diffResult, "remove_legacy"); + + expect(local.cfc).toInclude('removeColumn(table="test_models", columnName="legacy_field")'); + }); + + it("generates changeColumn in up for type changes", () => { + local.diffResult = { + modelName: "TestModel", + tableName: "test_models", + addColumns: [], + removeColumns: [], + changeColumns: [{name: "status", from: {type: "string"}, to: {type: "integer"}}] + }; + local.cfc = autoMigrator.generateMigrationCFC(local.diffResult, "change_status"); + + expect(local.cfc).toInclude('changeColumn(table="test_models", columnName="status", columnType="integer")'); + }); + + it("generates reverse changeColumn in down", () => { + local.diffResult = { + modelName: "TestModel", + tableName: "test_models", + addColumns: [], + removeColumns: [], + changeColumns: [{name: "status", from: {type: "string"}, to: {type: "integer"}}] + }; + local.cfc = autoMigrator.generateMigrationCFC(local.diffResult, "change_status"); + + // down() should reverse: change back from integer to string + expect(local.cfc).toInclude('changeColumn(table="test_models", columnName="status", columnType="string")'); + }); + + it("handles empty diff with no-op comments", () => { + local.diffResult = { + modelName: "TestModel", + tableName: "test_models", + addColumns: [], + removeColumns: [], + changeColumns: [] + }; + local.cfc = autoMigrator.generateMigrationCFC(local.diffResult, "no_changes"); + + expect(local.cfc).toInclude("No changes detected"); + }); + + it("includes migration name in the hint", () => { + local.diffResult = { + modelName: "TestModel", + tableName: "test_models", + addColumns: [], + removeColumns: [], + changeColumns: [] + }; + local.cfc = autoMigrator.generateMigrationCFC(local.diffResult, "my_migration_name"); + + expect(local.cfc).toInclude('hint="my_migration_name"'); + }); + + it("generates allowNull attribute for addColumn", () => { + local.diffResult = { + modelName: "TestModel", + tableName: "test_models", + addColumns: [ + {name: "required_field", type: "string", nullable: false, "default": ""}, + {name: "optional_field", type: "string", nullable: true, "default": ""} + ], + removeColumns: [], + changeColumns: [] + }; + local.cfc = autoMigrator.generateMigrationCFC(local.diffResult, "add_fields"); + + expect(local.cfc).toInclude("allowNull=false"); + expect(local.cfc).toInclude("allowNull=true"); + }); + + }); + + }); + + } + +} From 13aa009c4759d50bc632e6651ce46ab246d85aa5 Mon Sep 17 00:00:00 2001 From: Peter Amiri Date: Tue, 14 Apr 2026 22:52:43 -0700 Subject: [PATCH 2/2] fix(migration): sanitize migration filenames and add millisecond precision Sanitize migration name before using it as a filename to prevent filesystem errors with special characters. Add millisecond precision to the timestamp prefix to reduce collision risk on rapid successive writeMigration() calls. Includes 8 tests for the sanitizer. Co-Authored-By: Claude Opus 4.6 (1M context) --- vendor/wheels/migrator/AutoMigrator.cfc | 18 ++++++++-- .../tests/specs/migrator/autoMigratorSpec.cfc | 36 +++++++++++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/vendor/wheels/migrator/AutoMigrator.cfc b/vendor/wheels/migrator/AutoMigrator.cfc index 59e8246eb..390fd9e30 100644 --- a/vendor/wheels/migrator/AutoMigrator.cfc +++ b/vendor/wheels/migrator/AutoMigrator.cfc @@ -255,8 +255,10 @@ component extends="wheels.migrator.Base" { local.content = generateMigrationCFC(arguments.diffResult, arguments.migrationName); - local.timestamp = DateFormat(Now(), "yyyymmdd") & TimeFormat(Now(), "HHmmss"); - local.fileName = local.timestamp & "_" & arguments.migrationName & ".cfc"; + // Use millisecond precision to reduce filename collision risk on rapid successive calls. + local.now = Now(); + local.timestamp = DateFormat(local.now, "yyyymmdd") & TimeFormat(local.now, "HHmmssL"); + local.fileName = local.timestamp & "_" & $sanitizeFileName(arguments.migrationName) & ".cfc"; local.migrationDir = ExpandPath("/app/migrator/migrations/"); if (!DirectoryExists(local.migrationDir)) { @@ -271,6 +273,18 @@ component extends="wheels.migrator.Base" { ); } + /** + * Sanitizes a string for use as a filename component. + * Lowercases, collapses non-alphanumeric chars to underscores, and trims edge underscores. + */ + public string function $sanitizeFileName(required string name) { + local.safe = LCase(arguments.name); + local.safe = ReReplace(local.safe, "[^a-z0-9_]+", "_", "all"); + local.safe = ReReplace(local.safe, "_+", "_", "all"); + local.safe = ReReplace(local.safe, "^_|_$", "", "all"); + return Len(local.safe) ? local.safe : "migration"; + } + /** * Maps cf_sql types (as stored in model properties) to Wheels migration column types. */ diff --git a/vendor/wheels/tests/specs/migrator/autoMigratorSpec.cfc b/vendor/wheels/tests/specs/migrator/autoMigratorSpec.cfc index 352cd80a7..b82092361 100644 --- a/vendor/wheels/tests/specs/migrator/autoMigratorSpec.cfc +++ b/vendor/wheels/tests/specs/migrator/autoMigratorSpec.cfc @@ -325,6 +325,42 @@ component extends="wheels.WheelsTest" { }); + describe("$sanitizeFileName", () => { + + it("lowercases input", () => { + expect(autoMigrator.$sanitizeFileName("AddUserEmail")).toBe("adduseremail"); + }); + + it("replaces spaces with underscores", () => { + expect(autoMigrator.$sanitizeFileName("add user email")).toBe("add_user_email"); + }); + + it("replaces special chars with underscores", () => { + expect(autoMigrator.$sanitizeFileName("add;user/email")).toBe("add_user_email"); + }); + + it("collapses consecutive underscores", () => { + expect(autoMigrator.$sanitizeFileName("add___user")).toBe("add_user"); + }); + + it("trims leading and trailing underscores", () => { + expect(autoMigrator.$sanitizeFileName("__add_user__")).toBe("add_user"); + }); + + it("returns 'migration' for empty input", () => { + expect(autoMigrator.$sanitizeFileName("")).toBe("migration"); + }); + + it("returns 'migration' for input that sanitizes to empty", () => { + expect(autoMigrator.$sanitizeFileName("///")).toBe("migration"); + }); + + it("preserves alphanumerics and underscores", () => { + expect(autoMigrator.$sanitizeFileName("add_field_v2")).toBe("add_field_v2"); + }); + + }); + }); }