Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
31 changes: 29 additions & 2 deletions vendor/wheels/model/sql.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -136,16 +136,23 @@ component {
// to every OUTER join, which only looked correct because issues #449 and #3245 both
// exercise a single OUTER join. A root-level INNER join (`parentPosition` 0) has no
// enclosing group and stays flat, keeping the root FROM table in scope for its ON.
//
// Join type comes from the association's `joinType`, not from scanning the
// generated SQL for "INNER". The text scan the pre-fix code used misreads any
// table whose name contains the substring — `winners`, `spinners`, `beginners`
// — as an inner join, which would emit its nested child flat and silently drop
// the parent rows this fix exists to preserve. `joinType` is the authoritative
// source: it is what the join string is built from a few hundred lines below.
local.nestedJoins = {};
local.isNested = {};
for (local.i = 1; local.i <= local.iEnd; local.i++) {
local.parentPosition = StructKeyExists(local.associations[local.i], "parentPosition")
? local.associations[local.i].parentPosition
: 0;
if (
FindNoCase("INNER", local.joins[local.i])
$associationJoinsInner(local.associations[local.i], local.joins[local.i])
&& local.parentPosition > 0
&& !FindNoCase("INNER", local.joins[local.parentPosition])
&& !$associationJoinsInner(local.associations[local.parentPosition], local.joins[local.parentPosition])
) {
if (!StructKeyExists(local.nestedJoins, local.parentPosition)) {
local.nestedJoins[local.parentPosition] = [];
Expand Down Expand Up @@ -1284,6 +1291,26 @@ component {
/**
* Internal function.
*/
/**
* Internal function.
* Whether an association contributes an INNER JOIN.
*
* Reads the association's declared `joinType` — the same value `$expandedAssociations`
* turns into the leading `INNER JOIN` / `LEFT OUTER JOIN` text — rather than searching the
* built SQL for "INNER". A substring search misclassifies every table whose name contains
* it (`winners`, `spinners`, `beginners`), and in `$fromClause` that would demote a nested
* group to a flat join and silently drop parent rows.
*
* Falls back to the text scan only if an entry somehow carries no `joinType`, which keeps
* this total for any caller assembling association structs by hand.
*/
public boolean function $associationJoinsInner(required struct association, required string join) {
if (StructKeyExists(arguments.association, "joinType") && Len(arguments.association.joinType)) {
return arguments.association.joinType == "inner";
}
return FindNoCase("INNER", arguments.join) > 0;
}

public array function $expandedAssociations(required string include, boolean includeSoftDeletes = "false") {
local.rv = [];

Expand Down
58 changes: 29 additions & 29 deletions vendor/wheels/tests/specs/jobs/JobClassRoundTripSpec.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ component extends="wheels.WheelsTest" {
describe("Tests that the persisted jobClass round-trips", () => {

it("reports a metadata name whose last segment matches the .cfc file name exactly", () => {
job = CreateObject("component", "wheels.tests._assets.jobs.ProbeJob")
meta = GetMetadata(job)
local.job = CreateObject("component", "wheels.tests._assets.jobs.ProbeJob")
local.meta = GetMetadata(local.job)

fileName = ListFirst(ListLast(Replace(meta.path, "\", "/", "all"), "/"), ".")
local.fileName = ListFirst(ListLast(Replace(local.meta.path, "\", "/", "all"), "/"), ".")

// case-sensitive comparison — Compare(), not CompareNoCase()
expect(Compare(ListLast(meta.name, "."), fileName)).toBe(0)
expect(Compare(ListLast(local.meta.name, "."), local.fileName)).toBe(0)
})

it("never persists a caller's miscased path", () => {
Expand All @@ -43,74 +43,74 @@ component extends="wheels.WheelsTest" {
//
// Asserting only the first would fail on Adobe for a reason that is *safer*
// than the one being tested, so assert the property both satisfy.
canonical = GetMetadata(CreateObject("component", "wheels.tests._assets.jobs.ProbeJob")).name
resolved = {miscasedConstructed = false, name = ""}
local.canonical = GetMetadata(CreateObject("component", "wheels.tests._assets.jobs.ProbeJob")).name
local.resolved = {miscasedConstructed = false, name = ""}

try {
resolved.name = GetMetadata(CreateObject("component", "wheels.tests._assets.jobs.probejob")).name
resolved.miscasedConstructed = true
local.resolved.name = GetMetadata(CreateObject("component", "wheels.tests._assets.jobs.probejob")).name
local.resolved.miscasedConstructed = true
} catch (any e) {
// case-sensitive resolver — the stronger guarantee
}

if (resolved.miscasedConstructed) {
expect(Compare(resolved.name, canonical)).toBe(0)
if (local.resolved.miscasedConstructed) {
expect(Compare(local.resolved.name, local.canonical)).toBe(0)
} else {
expect(resolved.name).toBe("")
expect(local.resolved.name).toBe("")
}
})

it("re-instantiates from its own persisted metadata name", () => {
// the actual enqueue -> drain round trip, without touching the queue table
original = CreateObject("component", "wheels.tests._assets.jobs.ProbeJob")
persisted = GetMetadata(original).name
local.original = CreateObject("component", "wheels.tests._assets.jobs.ProbeJob")
local.persisted = GetMetadata(local.original).name

// Hoisted receiver. A parenthesized `new` in receiver position — `(new X()).m()`
// — is rejected by Adobe's parser with `Invalid construct: Either argument or
// name is missing`, the same MissingNameException family as cross-engine
// invariant 16. Adobe blames the enclosing describe() line and the whole engine
// leg reports tests=0. Caught by the compat matrix; Lucee and BoxLang accept it.
bridge = new wheels.Job()
revived = bridge.$instantiateJobClass(jobClass = persisted)
local.bridge = new wheels.Job()
local.revived = local.bridge.$instantiateJobClass(jobClass = local.persisted)

expect(Compare(GetMetadata(revived).name, persisted)).toBe(0)
expect(Compare(GetMetadata(local.revived).name, local.persisted)).toBe(0)
})
})

describe("Tests that an unresolvable jobClass", () => {

it("throws Wheels.JobClassNotFound naming the row and the class", () => {
thrown = {type: "", message: ""}
local.thrown = {type: "", message: ""}

bridge = new wheels.Job()
local.bridge = new wheels.Job()

try {
bridge.$instantiateJobClass(jobClass = "app.jobs.NoSuchJob", jobId = "abc-123")
local.bridge.$instantiateJobClass(jobClass = "app.jobs.NoSuchJob", jobId = "abc-123")
} catch (any e) {
thrown.type = e.type
thrown.message = e.message
local.thrown.type = e.type
local.thrown.message = e.message
}

// the raw engine error is "component not found" for a class that plainly
// exists, which points investigators at mappings and deployment
expect(thrown.type).toBe("Wheels.JobClassNotFound")
expect(thrown.message).toInclude("app.jobs.NoSuchJob")
expect(thrown.message).toInclude("abc-123")
expect(local.thrown.type).toBe("Wheels.JobClassNotFound")
expect(local.thrown.message).toInclude("app.jobs.NoSuchJob")
expect(local.thrown.message).toInclude("abc-123")
})

it("throws Wheels.InvalidJobClass when the path resolves to something that is not a job", () => {
thrown = {type: ""}
local.thrown = {type: ""}

bridge = new wheels.Job()
local.bridge = new wheels.Job()

try {
// a real component with no perform()
bridge.$instantiateJobClass(jobClass = "wheels.tests._assets.models.Post")
local.bridge.$instantiateJobClass(jobClass = "wheels.tests._assets.models.Post")
} catch (any e) {
thrown.type = e.type
local.thrown.type = e.type
}

expect(thrown.type).toBe("Wheels.InvalidJobClass")
expect(local.thrown.type).toBe("Wheels.InvalidJobClass")
})
})
}
Expand Down
25 changes: 25 additions & 0 deletions vendor/wheels/tests/specs/model/crudSpec.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -1335,6 +1335,31 @@ component extends="wheels.WheelsTest" {
)
})

// The grouping decision used to read the join TYPE by searching the generated SQL
// for "INNER". That misclassifies every table whose name contains the substring —
// `winners`, `spinners`, `beginners` — and in $fromClause a parent misread as
// INNER emits its nested child flat instead of grouped, silently dropping the
// parent rows this fix exists to preserve. `joinType` is the authoritative source:
// it is what the join text is built from. Unit-tested here rather than through a
// fixture because adding a `c_o_r_e_winners` table would mean DDL on all seven
// databases to pin one boolean.
it("reads the join type from metadata, not from the table name (issue ##3334)", () => {
m = g.model("post")

// the trap: an OUTER join whose table name contains "inner"
outerWithInnerInName = {joinType = "outer"}
outerSql = "LEFT OUTER JOIN #qi('c_o_r_e_winners')# ON #qi('c_o_r_e_posts')#.#qi('id')# = #qi('c_o_r_e_winners')#.#qi('postid')#"
expect(FindNoCase("INNER", outerSql)).toBeGT(0)
expect(m.$associationJoinsInner(outerWithInnerInName, outerSql)).toBeFalse()

// and it still recognises a genuine inner join
expect(m.$associationJoinsInner({joinType = "inner"}, "INNER JOIN #qi('c_o_r_e_tags')# ON 1=1")).toBeTrue()

// no joinType at all (a hand-built struct) falls back to the text scan
expect(m.$associationJoinsInner({}, "INNER JOIN #qi('c_o_r_e_tags')# ON 1=1")).toBeTrue()
expect(m.$associationJoinsInner({}, "LEFT OUTER JOIN #qi('c_o_r_e_tags')# ON 1=1")).toBeFalse()
})

// An unbalanced include reaches the level-tracking loop with an empty parent
// stack. `ListDeleteAt` on a one-element list tolerates that; `ArrayDeleteAt(x, 0)`
// throws. Malformed includes resolved to a plain join before the issue #3334
Expand Down
Loading