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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ All historical references to "CFWheels" in this changelog have been preserved fo

### Fixed

- Model layer SELECT clause builder now routes column identifiers through the adapter's `$quoteIdentifier`, so reserved-word column names (e.g. `key`, `order`, `group`) survive on every supported dialect instead of breaking `findAll` / `findOne` / dynamic finders with cryptic SQL syntax errors. The WHERE / ORDER BY paths already quoted columns; `$createSQLFieldList` and the empty-pagination column-list extraction in `read.cfc` now match.
- `wheels packages install <name>` is now a transparent alias for `wheels packages add <name>` on every caller path that actually reaches `Module.cfc` (stdio MCP server, scripted in-process clients, spec suite). Previously the dispatch layer's `case "install":` branch printed a yellow warning to stdout and returned `""` without installing anything — so even though `PackagesMainCli.install()` itself had been a true alias for `add()` since #2729, any caller routing through the CLI module's verb dispatch silently no-op'd. The shell-facing `wheels packages install <name>` form is still intercepted by LuCLI's built-in extension installer upstream of module dispatch and remains broken on that path (and is documented as such in the module-owned help text), but MCP tool calls and programmatic callers now behave identically to `add`. Both branches now share a single fall-through body so the validation, error shape, and install behavior cannot drift apart again.
- `wheels.wheelstest.BrowserTest` now throws a clear `Wheels.BrowserTest.NotWired` error — naming `browserDescribe()` as the fix — when a spec calls a DSL method on `this.browser` from a plain `describe()` block. Previously the uninitialized `this.browser` was an empty string, producing the misleading `function [visitUrl] does not exist in the String` on every newcomer's first BrowserTest spec. A sentinel `UnwiredBrowserGuard` is now installed at `this.browser` before `browserDescribe()` wires the real `BrowserClient` and after `$endBrowserContext()` tears it down
- `BrowserTest`'s default base URL is no longer hardcoded to `http://localhost:8080`. `$resolveBaseUrl()` now consults a layered lookup at instance time: `this.baseUrl` (per-spec override, set in the component pseudo-constructor) → `get("browserTestBaseUrl")` (Wheels setting) → `-Dwheels.browserTest.baseUrl=...` (JVM system property) → `WHEELS_BROWSER_TEST_BASE_URL` env var → `cgi.server_name`/`cgi.server_port` auto-detect → `http://localhost:8080` default. Specs running against a non-8080 server (Titan on 60050, `wheels new` scaffolds on 60080) can set `this.baseUrl` in the pseudo-constructor or rely on the CGI auto-detect instead of comparing `getBaseUrl()` against a sentinel string. The bare-env-var approach still works for CI but is no longer the only escape hatch (the JVM caches env vars at process start, so post-launch `export` had no effect). Regression spec at `vendor/wheels/tests/specs/wheelstest/BrowserTestBaseUrlResolutionSpec.cfc` (#2779)
Expand Down
4 changes: 3 additions & 1 deletion vendor/wheels/model/read.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,9 @@ component {
list = arguments.select,
returnAs = arguments.returnAs
);
local.columns = ReReplace(local.columns, "[`""\[\]\w]*?\.([\w\s]*?)(,|$)", "\1\2", "all");
// Strip dialect quotes: $createSQLFieldList now quotes identifiers; the bare-identifier regex below requires unquoted input.
local.columns = variables.wheels.class.adapter.$stripIdentifierQuotes(local.columns);
local.columns = ReReplace(local.columns, "[\w]*?\.([\w\s]*?)(,|$)", "\1\2", "all");
local.columns = ReReplace(local.columns, "\(.*?\)\sAS\s([\w\s]*?)(,|$)", "\1\2", "all");
local.columns = ReReplace(local.columns, "\w*?\sAS\s([\w\s]*?)(,|$)", "\1\2", "all");
local.rv = QueryNew(local.columns);
Expand Down
9 changes: 6 additions & 3 deletions vendor/wheels/model/sql.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -567,9 +567,9 @@ component {
if (StructKeyExists(local.classData.propertyStruct, local.iItem)) {
local.toAppend &= variables.wheels.class.adapter.$quoteIdentifier(local.classData.tableName) & ".";
if (StructKeyExists(local.classData.columnStruct, local.iItem)) {
local.toAppend &= local.iItem;
local.toAppend &= variables.wheels.class.adapter.$quoteIdentifier(local.iItem);
} else {
local.toAppend &= local.classData.properties[local.iItem].column;
local.toAppend &= variables.wheels.class.adapter.$quoteIdentifier(local.classData.properties[local.iItem].column);
if (arguments.clause == "select") {
local.toAppend &= " AS " & local.iItem;
}
Expand Down Expand Up @@ -631,6 +631,9 @@ component {
// get the property part, done by taking everything from the end of the string to a . or a space (which would be found when using " AS ")
local.property = Reverse(SpanExcluding(Reverse(local.iItem), ". "));

// Strip dialect quotes added above so alias matching and downstream concatenation work on bare identifiers.
local.property = variables.wheels.class.adapter.$stripIdentifierQuotes(local.property);

// check if this one has been flagged as a duplicate, we get the number of classes to skip and also remove the flagged info from the item
local.duplicateCount = 0;
local.matches = ReFind("^\[\[duplicate\]\](\d+)(.+)$", local.iItem, 1, true);
Expand Down Expand Up @@ -668,7 +671,7 @@ component {
local.newItem = ReplaceNoCase(local.iItem, " AS " & local.property, " AS " & local.newProperty);
} else {
if (local.aliasFound) {
local.newItem = local.alias & "." & local.property & " AS " & local.newProperty;
local.newItem = local.alias & "." & variables.wheels.class.adapter.$quoteIdentifier(local.property) & " AS " & local.newProperty;
} else {
local.newItem = local.iItem & " AS " & local.newProperty;
}
Expand Down
6 changes: 3 additions & 3 deletions vendor/wheels/tests/specs/model/crudSpec.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ component extends="wheels.WheelsTest" {
// trim extra whitespace
actual = Trim(actual)

expected = "SELECT #qi('c_o_r_e_authors')#.id FROM #qi('c_o_r_e_authors')#"
expected = "SELECT #qi('c_o_r_e_authors')#.#qi('id')# FROM #qi('c_o_r_e_authors')#"

expect(actual).toBe(expected)
})
Expand Down Expand Up @@ -1671,7 +1671,7 @@ component extends="wheels.WheelsTest" {

local.a = qi("c_o_r_e_authors");
local.p = qi("c_o_r_e_posts");
expect(columnList).toBe("#local.a#.firstname,#local.a#.id,#local.a#.id AS Authorid,#local.a#.lastname,#local.p#.averagerating AS postaveragerating,#local.p#.body AS postbody,#local.p#.createdat AS postcreatedat,#local.p#.deletedat AS postdeletedat,#local.p#.id AS postid,#local.p#.status AS poststatus,#local.p#.title AS posttitle,#local.p#.updatedat AS postupdatedat,#local.p#.views AS postviews")
expect(columnList).toBe("#local.a#.#qi('firstname')#,#local.a#.#qi('id')#,#local.a#.#qi('id')# AS Authorid,#local.a#.#qi('lastname')#,#local.p#.#qi('averagerating')# AS postaveragerating,#local.p#.#qi('body')# AS postbody,#local.p#.#qi('createdat')# AS postcreatedat,#local.p#.#qi('deletedat')# AS postdeletedat,#local.p#.#qi('id')# AS postid,#local.p#.#qi('status')# AS poststatus,#local.p#.#qi('title')# AS posttitle,#local.p#.#qi('updatedat')# AS postupdatedat,#local.p#.#qi('views')# AS postviews")
})

it("works with association with expanded aliases disabled", () => {
Expand All @@ -1688,7 +1688,7 @@ component extends="wheels.WheelsTest" {

local.a = qi("c_o_r_e_authors");
local.p = qi("c_o_r_e_posts");
expect(columnList).toBe("#local.a#.firstname,#local.a#.id,#local.a#.id AS Authorid,#local.a#.lastname,#local.p#.averagerating,#local.p#.body,#local.p#.createdat,#local.p#.deletedat,#local.p#.id AS postid,#local.p#.status,#local.p#.title,#local.p#.updatedat,#local.p#.views")
expect(columnList).toBe("#local.a#.#qi('firstname')#,#local.a#.#qi('id')#,#local.a#.#qi('id')# AS Authorid,#local.a#.#qi('lastname')#,#local.p#.#qi('averagerating')#,#local.p#.#qi('body')#,#local.p#.#qi('createdat')#,#local.p#.#qi('deletedat')#,#local.p#.#qi('id')# AS postid,#local.p#.#qi('status')#,#local.p#.#qi('title')#,#local.p#.#qi('updatedat')#,#local.p#.#qi('views')#")
})

it("works on calculated property", () => {
Expand Down
51 changes: 51 additions & 0 deletions vendor/wheels/tests/specs/model/reservedColumnQuotingSpec.cfc
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
component extends="wheels.WheelsTest" {

function run() {

g = application.wo;

describe("$createSQLFieldList column identifier quoting", () => {

it("quotes the underlying column when a property aliases a different column name", () => {
// City aliases id -> countyid, so the SELECT column must be quoted in case it is a reserved word.
var m = g.model("city");
var actual = m.$selectClause(select = "id", include = "", returnAs = "objects");

expect(actual).toInclude(m.$quotedTableName() & "." & m.$quoteColumn("countyid"));
});

it("quotes the column when the property name matches the column name", () => {
// Author.firstName has no column mapping (property == column); SELECT must still quote the identifier.
var m = g.model("author");
var actual = m.$selectClause(select = "firstName", include = "", returnAs = "objects");

expect(actual).toInclude(m.$quotedTableName() & "." & m.$quoteColumn("firstName"));
});

it("quotes column identifiers in the GROUP BY clause too", () => {
// $groupByClause routes through the same $createSQLFieldList builder.
var m = g.model("city");
var actual = m.$groupByClause(
select = "id",
include = "",
group = "id",
distinct = false,
returnAs = "objects"
);

expect(actual).toInclude(m.$quotedTableName() & "." & m.$quoteColumn("countyid"));
});

it("returns a well-formed empty query when paginated findAll matches zero rows on an aliased column", () => {
// Without $stripIdentifierQuotes, dialect quote chars from $createSQLFieldList leak into QueryNew column names.
var m = g.model("city");
var rv = m.findAll(select = "id", where = "id = -1", page = 1, perPage = 25);

expect(IsQuery(rv)).toBeTrue();
expect(rv.recordCount).toBe(0);
expect(ListFindNoCase(rv.columnList, "id")).toBeGT(0);
});

});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ When the table, key, or column names don't match the defaults, override them in
- `tableName("old_blog_entries")` — non-convention table name.
- `setPrimaryKey("entryId")` — non-`id` primary key.
- `dataSource("legacy_db")` — non-default datasource. See [Database and Multiple Datasources](/v4-0-1-snapshot/basics/database-and-multiple-datasources/) when this lands for the wider multi-database story.
- `property(name="publishedAt", column="pub_date")` — the property on the model is `publishedAt`, the column in the table is `pub_date`. The view and controller never see the column name.
- `property(name="publishedAt", column="pub_date")` — the property on the model is `publishedAt`, the column in the table is `pub_date`. The view and controller never see the column name. This is also the correct pattern when a column name is a SQL reserved word (`key`, `order`, `group`, etc.) — Wheels quotes all column identifiers in SELECT, GROUP BY, and ORDER BY clauses, so the alias stays clean while the generated SQL brackets the underlying column name for the target dialect.

```cfm {test:compile}
component extends="Model" {
Expand Down
Loading