diff --git a/changelog.d/3082-cli-seed-generate.fixed.md b/changelog.d/3082-cli-seed-generate.fixed.md new file mode 100644 index 0000000000..2f58ec3f60 --- /dev/null +++ b/changelog.d/3082-cli-seed-generate.fixed.md @@ -0,0 +1 @@ +- `wheels seed --generate` now actually creates rows and reports honest success/failure. The CLI seed bridge's generate loop iterated `$classData().properties` (a struct keyed by property name) as if it were an array of property structs — so `prop.name` threw `there is no property with name [NAME] found in [string]`, every model errored, zero rows were created, yet the run still returned `success=true` and the CLI printed "Seeding completed." with exit 0. The generate path is now a dedicated, unit-tested `wheels.Seeder.generateSeeds(models, count)` method that iterates the property struct correctly and forces overall `success=false` when any model fails or no rows are created, so the CLI surfaces a non-zero exit instead of silently lying (the #2987 honesty fix had only covered convention mode) (#3082) diff --git a/vendor/wheels/Seeder.cfc b/vendor/wheels/Seeder.cfc index 90deff7f66..afbc1d4ccc 100644 --- a/vendor/wheels/Seeder.cfc +++ b/vendor/wheels/Seeder.cfc @@ -227,6 +227,251 @@ component output="false" extends="wheels.Global" { return local.result; } + /** + * Generate fake records for one or more models — the legacy + * `wheels seed --generate` path. Unlike convention seeding this does not + * use seed files; it introspects each model's persisted properties and + * inserts `count` rows of plausible test data per model. + * + * Honesty contract (issue #3082): a model that throws, or whose generated + * rows do not all save, is recorded as a failed entry AND forces overall + * success=false. The previous CLI-view implementation iterated the + * $classData().properties STRUCT as if it were an array of property structs + * — so `prop.name` threw "there is no property with name [NAME] found in + * [string]" — created zero rows, yet still returned success=true and the + * CLI printed "Seeding completed." with exit 0. + * + * @models Comma-delimited list of model names. When blank, every *.cfc under + * /app/models (excluding _-prefixed files and the framework's + * parent Model.cfc base class) is used. + * @count Number of rows to generate per model. + */ + public struct function generateSeeds(string models = "", numeric count = 10) { + var result = { + success = false, + mode = "generate", + seeded = [], + totalCreated = 0, + // Generate mode never skips rows, but the CLI bridge contract + // requires the key: Module.cfc::runSeed() prints + // `#result.totalSkipped# skipped` whenever totalCreated exists. + totalSkipped = 0, + totalFailed = 0, + message = "" + }; + + var modelList = $resolveGenerateModels(arguments.models); + + if (!ArrayLen(modelList)) { + result.message = "No models found to generate seed data for. Pass models=... or add models under /app/models."; + return result; + } + + for (var modelName in modelList) { + try { + var modelInstance = model(modelName); + // $classData().properties is a STRUCT keyed by property name — + // each value is the property's metadata struct. Iterate the keys. + var properties = modelInstance.$classData().properties; + var seededCount = 0; + + for (var i = 1; i <= arguments.count; i++) { + var record = {}; + for (var propName in properties) { + if (propName != "id" && !ListFindNoCase("createdAt,updatedAt,deletedAt", propName)) { + var propType = StructKeyExists(properties[propName], "type") ? properties[propName].type : "string"; + record[propName] = $generateTestData(propName, propType, i); + } + } + var newRecord = modelInstance.new(record); + if (newRecord.save()) { + seededCount++; + } + } + + var entrySuccess = (seededCount == arguments.count); + ArrayAppend(result.seeded, { + model = modelName, + count = seededCount, + success = entrySuccess + }); + result.totalCreated += seededCount; + if (!entrySuccess) { + result.totalFailed++; + } + } catch (any modelError) { + ArrayAppend(result.seeded, { + model = modelName, + count = 0, + success = false, + error = modelError.message + }); + result.totalFailed++; + } + } + + result.success = (result.totalFailed == 0 && result.totalCreated > 0); + if (result.success) { + result.message = "Database seeding completed. Created #result.totalCreated# records across #ArrayLen(result.seeded)# #ArrayLen(result.seeded) == 1 ? 'model' : 'models'#."; + } else { + result.message = "Database seeding failed. Created #result.totalCreated# records; #result.totalFailed# of #ArrayLen(result.seeded)# #result.totalFailed == 1 ? 'model' : 'models'# failed (#$failedGenerateSummary(result.seeded)#)."; + } + return result; + } + + /** + * Internal function. Resolves the model list for generateSeeds(): an + * explicit comma-delimited list when provided (blank entries trimmed away), + * otherwise every *.cfc model file under /app/models except _-prefixed + * files and the framework's parent Model.cfc base class. + */ + public array function $resolveGenerateModels(string models = "") { + var list = []; + if (Len(Trim(arguments.models))) { + for (var name in ListToArray(arguments.models)) { + if (Len(Trim(name))) { + ArrayAppend(list, Trim(name)); + } + } + return list; + } + var modelPath = ExpandPath("/app/models"); + if (DirectoryExists(modelPath)) { + var modelFiles = DirectoryList(modelPath, false, "name", "*.cfc"); + for (var file in modelFiles) { + // Skip the framework's parent Model.cfc — every scaffolded app + // ships it as the base class for its models, it has no backing + // table, and model("Model") throws Wheels.TableNotFound. Same + // exclusion as the CLI's model enumeration (Analysis.cfc and + // Module.cfc both skip it). + if (Left(file, 1) != "_" && file != "Model.cfc") { + ArrayAppend(list, ListFirst(file, ".")); + } + } + } + return list; + } + + /** + * Internal function. Builds a "model: reason" list for every failed entry + * recorded by generateSeeds(), used in its failure message. + */ + public string function $failedGenerateSummary(required array seeded) { + var parts = []; + for (var entry in arguments.seeded) { + if (!entry.success) { + var reason = StructKeyExists(entry, "error") ? entry.error : "only #entry.count# of the requested rows saved"; + ArrayAppend(parts, "#entry.model#: #reason#"); + } + } + return ArrayToList(parts, "; "); + } + + /** + * Internal function. Produces a plausible fake value for a property based on + * its name and type — used by generateSeeds(). + */ + public any function $generateTestData(required string propertyName, string propertyType = "string", numeric index = 1) { + local.name = LCase(arguments.propertyName); + + // Email fields + if (FindNoCase("email", local.name)) { + return "test#arguments.index#@example.com"; + } + + // Name fields + if (FindNoCase("firstname", local.name) || local.name == "fname") { + local.firstNames = ["John", "Jane", "Bob", "Alice", "Charlie", "Diana", "Edward", "Fiona", "George", "Helen"]; + return local.firstNames[(arguments.index - 1) mod ArrayLen(local.firstNames) + 1]; + } + + if (FindNoCase("lastname", local.name) || local.name == "lname") { + local.lastNames = ["Smith", "Johnson", "Williams", "Brown", "Jones", "Garcia", "Miller", "Davis", "Rodriguez", "Martinez"]; + return local.lastNames[(arguments.index - 1) mod ArrayLen(local.lastNames) + 1]; + } + + if (local.name == "name" || FindNoCase("username", local.name)) { + return "TestUser#arguments.index#"; + } + + // Phone fields + if (FindNoCase("phone", local.name) || FindNoCase("mobile", local.name)) { + return "555-#NumberFormat(1000 + arguments.index, '0000')#"; + } + + // Address fields + if (FindNoCase("address", local.name) || FindNoCase("street", local.name)) { + return "#arguments.index# Test Street"; + } + + if (FindNoCase("city", local.name)) { + local.cities = ["New York", "Los Angeles", "Chicago", "Houston", "Phoenix", "Philadelphia", "San Antonio", "San Diego"]; + return local.cities[(arguments.index - 1) mod ArrayLen(local.cities) + 1]; + } + + if (FindNoCase("state", local.name) || FindNoCase("province", local.name)) { + local.states = ["CA", "TX", "FL", "NY", "PA", "IL", "OH", "GA"]; + return local.states[(arguments.index - 1) mod ArrayLen(local.states) + 1]; + } + + if (FindNoCase("zip", local.name) || FindNoCase("postal", local.name)) { + return NumberFormat(10000 + arguments.index, "00000"); + } + + // URL fields + if (FindNoCase("url", local.name) || FindNoCase("website", local.name)) { + return "https://example#arguments.index#.com"; + } + + // Password fields + if (FindNoCase("password", local.name)) { + return "TestPass#arguments.index#!"; + } + + // Boolean fields + if (arguments.propertyType == "boolean" || FindNoCase("active", local.name) || FindNoCase("enabled", local.name) || FindNoCase("published", local.name)) { + return (arguments.index mod 2) == 1; + } + + // Numeric fields + if (arguments.propertyType == "integer" || arguments.propertyType == "numeric") { + if (FindNoCase("age", local.name)) { + return 20 + (arguments.index mod 50); + } + if (FindNoCase("price", local.name) || FindNoCase("cost", local.name) || FindNoCase("amount", local.name)) { + return (arguments.index * 10) + 0.99; + } + if (FindNoCase("quantity", local.name) || FindNoCase("count", local.name)) { + return arguments.index * 5; + } + return arguments.index; + } + + // Date fields + if (arguments.propertyType == "date" || arguments.propertyType == "datetime" || FindNoCase("date", local.name) || FindNoCase("birthday", local.name) || FindNoCase("dob", local.name)) { + return DateAdd("d", -arguments.index, Now()); + } + + // Text/description fields + if (arguments.propertyType == "text" || FindNoCase("description", local.name) || FindNoCase("content", local.name) || FindNoCase("body", local.name)) { + return "This is test content #arguments.index#. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Sed do eiusmod tempor incididunt ut labore et dolore magna aliqua."; + } + + // Title fields + if (FindNoCase("title", local.name) || FindNoCase("subject", local.name)) { + return "Test Title #arguments.index#"; + } + + // Status fields + if (FindNoCase("status", local.name)) { + local.statuses = ["pending", "active", "completed", "cancelled"]; + return local.statuses[(arguments.index - 1) mod ArrayLen(local.statuses) + 1]; + } + + // Default string value + return "#arguments.propertyName# Test #arguments.index#"; + } + /** * Internal function. Builds a "model: first error message" list for every * failed entry recorded in this run, used in the runSeeds() failure message. diff --git a/vendor/wheels/public/views/cli.cfm b/vendor/wheels/public/views/cli.cfm index e53fb41ea9..2b063c909e 100644 --- a/vendor/wheels/public/views/cli.cfm +++ b/vendor/wheels/public/views/cli.cfm @@ -473,9 +473,10 @@ try { case "dbSeed": // The seed orchestration lives in the page-level - // runDbSeed() UDF (defined alongside generateTestData - // below). Extracted so `dbSetup` can compose seeding - // without re-entering the dispatcher (issue ##2959). + // runDbSeed() UDF below. Generate mode delegates to + // wheels.Seeder.generateSeeds(). Extracted so `dbSetup` + // can compose seeding without re-entering the dispatcher + // (issue ##2959). local.seedResult = runDbSeed(request.wheels.params); StructAppend(data, local.seedResult, true); break; @@ -907,68 +908,19 @@ function runDbSeed(struct seedParams = {}) { result.detail = conventionResult.detail; } } else { - result.mode = "generate"; + // Generate mode delegates to Seeder.generateSeeds(), which fixes + // both #3082 defects: it iterates $classData().properties as the + // STRUCT it is (the old inline loop treated it as an array of + // property structs and threw on every model), and it reports + // overall success=false when any model fails — so the CLI surfaces + // a non-zero exit instead of printing "Seeding completed." (#3082). var count = structKeyExists(sp, "count") ? val(sp.count) : 10; var modelsArg = structKeyExists(sp, "models") ? sp.models : ""; - result.seeded = []; - - var modelList = []; - if (len(modelsArg)) { - modelList = listToArray(modelsArg); - } else { - var modelPath = expandPath("/app/models"); - if (directoryExists(modelPath)) { - var modelFiles = directoryList(modelPath, false, "name", "*.cfc"); - for (var file in modelFiles) { - if (left(file, 1) != "_") { - arrayAppend(modelList, listFirst(file, ".")); - } - } - } - } - - for (var modelName in modelList) { - try { - var modelInstance = model(modelName); - var seededCount = 0; - var properties = []; - if (structKeyExists(modelInstance, "$classData") && structKeyExists(modelInstance.$classData(), "properties")) { - properties = modelInstance.$classData().properties; - } - for (var i = 1; i <= count; i++) { - var record = {}; - for (var prop in properties) { - if (prop.name != "id" && !listFindNoCase("createdAt,updatedAt,deletedAt", prop.name)) { - record[prop.name] = generateTestData(prop.name, prop.type, i); - } - } - var newRecord = modelInstance.new(record); - if (newRecord.save()) { - seededCount++; - } - } - arrayAppend(result.seeded, { - model = modelName, - count = seededCount, - success = true - }); - } catch (any modelError) { - arrayAppend(result.seeded, { - model = modelName, - count = 0, - success = false, - error = modelError.message - }); - } - } - - var totalSeeded = 0; - for (var seedEntry in result.seeded) { - if (seedEntry.success) { - totalSeeded += seedEntry.count; - } - } - result.message = "Database seeding completed. Created #totalSeeded# records across #arrayLen(result.seeded)# models."; + var generateSeeder = structKeyExists(application.wheels, "seeder") + ? application.wheels.seeder + : CreateObject("component", "wheels.Seeder").init(); + var generateResult = generateSeeder.generateSeeds(models = modelsArg, count = count); + StructAppend(result, generateResult, true); } } catch (any e) { result.success = false; @@ -978,108 +930,6 @@ function runDbSeed(struct seedParams = {}) { return result; } -// Helper function to generate test data based on property name and type -function generateTestData(required string propertyName, string propertyType = "string", numeric index = 1) { - // Common patterns for property names - local.name = lCase(arguments.propertyName); - - // Email fields - if (findNoCase("email", local.name)) { - return "test#arguments.index#@example.com"; - } - - // Name fields - if (findNoCase("firstname", local.name) || local.name == "fname") { - local.firstNames = ["John", "Jane", "Bob", "Alice", "Charlie", "Diana", "Edward", "Fiona", "George", "Helen"]; - return local.firstNames[(arguments.index - 1) mod arrayLen(local.firstNames) + 1]; - } - - if (findNoCase("lastname", local.name) || local.name == "lname") { - local.lastNames = ["Smith", "Johnson", "Williams", "Brown", "Jones", "Garcia", "Miller", "Davis", "Rodriguez", "Martinez"]; - return local.lastNames[(arguments.index - 1) mod arrayLen(local.lastNames) + 1]; - } - - if (local.name == "name" || findNoCase("username", local.name)) { - return "TestUser#arguments.index#"; - } - - // Phone fields - if (findNoCase("phone", local.name) || findNoCase("mobile", local.name)) { - return "555-#numberFormat(1000 + arguments.index, '0000')#"; - } - - // Address fields - if (findNoCase("address", local.name) || findNoCase("street", local.name)) { - return "#arguments.index# Test Street"; - } - - if (findNoCase("city", local.name)) { - local.cities = ["New York", "Los Angeles", "Chicago", "Houston", "Phoenix", "Philadelphia", "San Antonio", "San Diego"]; - return local.cities[(arguments.index - 1) mod arrayLen(local.cities) + 1]; - } - - if (findNoCase("state", local.name) || findNoCase("province", local.name)) { - local.states = ["CA", "TX", "FL", "NY", "PA", "IL", "OH", "GA"]; - return local.states[(arguments.index - 1) mod arrayLen(local.states) + 1]; - } - - if (findNoCase("zip", local.name) || findNoCase("postal", local.name)) { - return numberFormat(10000 + arguments.index, "00000"); - } - - // URL fields - if (findNoCase("url", local.name) || findNoCase("website", local.name)) { - return "https://example#arguments.index#.com"; - } - - // Password fields - if (findNoCase("password", local.name)) { - return "TestPass#arguments.index#!"; - } - - // Boolean fields - if (arguments.propertyType == "boolean" || findNoCase("active", local.name) || findNoCase("enabled", local.name) || findNoCase("published", local.name)) { - return (arguments.index mod 2) == 1; - } - - // Numeric fields - if (arguments.propertyType == "integer" || arguments.propertyType == "numeric") { - if (findNoCase("age", local.name)) { - return 20 + (arguments.index mod 50); - } - if (findNoCase("price", local.name) || findNoCase("cost", local.name) || findNoCase("amount", local.name)) { - return (arguments.index * 10) + 0.99; - } - if (findNoCase("quantity", local.name) || findNoCase("count", local.name)) { - return arguments.index * 5; - } - return arguments.index; - } - - // Date fields - if (arguments.propertyType == "date" || arguments.propertyType == "datetime" || findNoCase("date", local.name) || findNoCase("birthday", local.name) || findNoCase("dob", local.name)) { - return dateAdd("d", -arguments.index, now()); - } - - // Text/description fields - if (arguments.propertyType == "text" || findNoCase("description", local.name) || findNoCase("content", local.name) || findNoCase("body", local.name)) { - return "This is test content #arguments.index#. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Sed do eiusmod tempor incididunt ut labore et dolore magna aliqua."; - } - - // Title fields - if (findNoCase("title", local.name) || findNoCase("subject", local.name)) { - return "Test Title #arguments.index#"; - } - - // Status fields - if (findNoCase("status", local.name)) { - local.statuses = ["pending", "active", "completed", "cancelled"]; - return local.statuses[(arguments.index - 1) mod arrayLen(local.statuses) + 1]; - } - - // Default string value - return "#arguments.propertyName# Test #arguments.index#"; -} #SerializeJSON(data)# diff --git a/vendor/wheels/tests/specs/seederSpec.cfc b/vendor/wheels/tests/specs/seederSpec.cfc index 06898100a7..405e8b4220 100644 --- a/vendor/wheels/tests/specs/seederSpec.cfc +++ b/vendor/wheels/tests/specs/seederSpec.cfc @@ -220,6 +220,99 @@ component extends="wheels.WheelsTest" { }); + describe("generateSeeds()", () => { + + it("creates fake records for the requested model and reports honest success", () => { + // Capture existing Author ids so cleanup only removes our rows. + local.beforeIds = ValueList(model("Author").findAll(select = "id").id); + + local.gen = CreateObject("component", "wheels.Seeder").init(); + local.result = local.gen.generateSeeds(models = "Author", count = 2); + + expect(local.result.success).toBeTrue(); + expect(local.result.mode).toBe("generate"); + expect(local.result.totalCreated).toBe(2); + // CLI bridge contract: Module.cfc::runSeed() prints + // `#result.totalSkipped# skipped` whenever totalCreated exists, + // so generate results MUST carry the key (always 0 — generate + // mode never skips) or a successful run throws in the CLI. + expect(StructKeyExists(local.result, "totalSkipped")).toBeTrue(); + expect(local.result.totalSkipped).toBe(0); + expect(local.result.totalFailed).toBe(0); + expect(ArrayLen(local.result.seeded)).toBe(1); + expect(local.result.seeded[1].model).toBe("Author"); + expect(local.result.seeded[1].count).toBe(2); + expect(local.result.seeded[1].success).toBeTrue(); + + // The rows must really exist — the old generate loop iterated the + // $classData().properties STRUCT as if it were an array of property + // structs, threw on every model, and created zero rows while still + // reporting success (issue #3082). + local.afterIds = ValueList(model("Author").findAll(select = "id").id); + expect(ListLen(local.afterIds) - ListLen(local.beforeIds)).toBe(2); + + // Clean up only the rows we generated. + if (Len(local.beforeIds)) { + model("Author").deleteAll(where = "id NOT IN (#local.beforeIds#)", instantiate = false); + } else { + model("Author").deleteAll(instantiate = false); + } + }); + + it("reports overall failure when a model cannot be seeded", () => { + local.gen = CreateObject("component", "wheels.Seeder").init(); + local.result = local.gen.generateSeeds( + models = "NoSuchModel_#Replace(CreateUUID(), '-', '', 'all')#", + count = 2 + ); + + // Honesty contract: a model that errors must not be reported as + // success. Generate mode previously appended success=false entries + // while leaving the overall result success=true and the CLI printing + // "Seeding completed." with exit 0 (issue #3082). + expect(local.result.success).toBeFalse(); + expect(local.result.totalCreated).toBe(0); + expect(local.result.totalFailed).toBe(1); + expect(ArrayLen(local.result.seeded)).toBe(1); + expect(local.result.seeded[1].success).toBeFalse(); + expect(StructKeyExists(local.result.seeded[1], "error")).toBeTrue(); + }); + + it("reports failure (not silent success) when an explicit list resolves to no models", () => { + local.gen = CreateObject("component", "wheels.Seeder").init(); + // A delimiter-only list is a non-blank value (so it does NOT fall + // back to auto-scanning /app/models) that still resolves to zero + // usable model names — the run must report failure, not success. + local.result = local.gen.generateSeeds(models = ",", count = 2); + expect(local.result.success).toBeFalse(); + expect(local.result.totalCreated).toBe(0); + expect(local.result.message).toInclude("No models"); + }); + + it("auto-scan excludes the framework's parent Model.cfc base class", () => { + // Every scaffolded app ships app/models/Model.cfc as the base + // class for its models. It has no backing table, so including + // it in the auto-scan makes model('Model') throw + // Wheels.TableNotFound and — under the honesty rule — forces + // every blank-models `wheels seed --generate` run to fail on a + // conventional app. Mirrors the CLI's own enumeration, which + // skips Model.cfc (Analysis.cfc / Module.cfc). + local.gen = CreateObject("component", "wheels.Seeder").init(); + local.resolved = local.gen.$resolveGenerateModels(""); + expect(ArrayFindNoCase(local.resolved, "Model")).toBe(0); + }); + + it("keeps explicitly requested model names verbatim", () => { + // The Model.cfc exclusion applies only to the auto-scan; an + // explicit list is the caller's responsibility and passes + // through untouched. + local.gen = CreateObject("component", "wheels.Seeder").init(); + local.resolved = local.gen.$resolveGenerateModels(" Author , User "); + expect(local.resolved).toBe(["Author", "User"]); + }); + + }); + }); }