Skip to content

Commit 80773ab

Browse files
authored
fix(controller): register super<name> for app-level controller and view overrides (#3357)
Following the "Overriding Core Methods" guide — override `linkTo()`, delegate with `superLinkTo()` — returned a 500, because `superLinkTo` never existed in controller or view context. The convention was implemented asymmetrically. `Model.cfc`'s $integrateFunctions() aliases the framework original to `super<name>` whenever the mixin's name is ALREADY present on the target, which is precisely the app-override case. `Controller.cfc`'s had no such branch: it aliased only names a registered plugin/package mixin overrode. So model overrides got `superFindAll()` and controller/view overrides got nothing, with no indication why. Adding the same else-branch is the whole fix. Before writing it I measured the cost, because `Controller` mixes in a much larger surface than `Model` and this runs on every request: a controller that overrides nothing has 445 keys and gains ZERO `super*` entries, before and after. No two framework mixins contribute the same name, so the branch fires only on a genuine app override — where the extra reference is the point. `Mapper.cfc` has its own $integrateFunctions() variant and the issue flagged it as worth checking. It is not affected: it assigns unconditionally with no existence check and has no `super<name>` concept at all, so there is no asymmetry to correct. Left alone. Red-first, against the pristine Controller.cfc: the two controller specs fail — `Expected [false] to be true` and `No matching function [SUPERLINKTO] found` — while the model-side spec and the no-override spec pass. That split is the bug, stated as a test. 4 specs, using two new fixtures of the same name on each side: a controller overriding `linkTo()` and a model overriding `columnNames()`, both delegating through `super<name>`. The controller spec asserts a real anchor comes back and that the prefix appears exactly once, so it proves the framework original ran rather than the override recursing. The model spec pins behaviour that already worked, so the parity cannot regress from either direction. The fourth asserts the zero-extra-keys property above. Docs are NOT included here. The v3 guide presents the convention as general and the v4-0-0 tree has no "Overriding Core Methods" page at all; porting and correcting it is a separate change against the guides site. Verification, lucee7 + sqlite, full core suite: develop ab901cf 4732 pass / 0 fail / 0 error this branch 4736 pass / 0 fail / 0 error Exactly +4, the new specs. Closes #3325 Signed-off-by: Peter Amiri <peter@alurium.com>
1 parent 1987187 commit 80773ab

5 files changed

Lines changed: 107 additions & 4 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- Overriding a controller or view helper now registers the framework original as `super<name>`, matching the model layer. Following the "Overriding Core Methods" guide — override `linkTo()`, call `superLinkTo()` — produced a 500, because `Controller.cfc`'s `$integrateFunctions()` only aliased `super<name>` for names a registered plugin/package mixin overrode, while `Model.cfc`'s aliased it for any name already present on the target. App-level overrides of controller and view helpers silently got nothing. The manual `variables.coreLinkTo = CreateObject("component", "wheels.view.links").linkTo` workaround is no longer needed. Controllers that override nothing gain no extra keys — no two framework mixins contribute the same name, so the branch only fires on a genuine override (#3325, from discussion #3323)

vendor/wheels/Controller.cfc

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -391,11 +391,19 @@ component output="false" displayName="Controller" extends="wheels.Global"{
391391

392392
/**
393393
* Mix a component's pre-resolved public methods (each `{name, ref}`, see
394-
* $componentIntegrationPlan) into this instance. Preserves the original
395-
* semantics: a method that does not already exist (from inheritance or an
396-
* earlier-integrated component) is added, and any method a plugin/package
397-
* mixin will override is also aliased to `super<name>`. `overrideSet` is the
394+
* $componentIntegrationPlan) into this instance. A method that does not already
395+
* exist (from inheritance or an earlier-integrated component) is added; one that
396+
* DOES already exist is left alone and the framework original is exposed as
397+
* `super<name>`, so an app override can delegate to it. Any method a
398+
* plugin/package mixin will override is likewise aliased. `overrideSet` is the
398399
* precomputed mixin-override name set.
400+
*
401+
* The `super<name>` else-branch matches Model.cfc, which has always had it. Its
402+
* absence here meant an app that overrode a controller or view helper — exactly
403+
* as the "Overriding Core Methods" guide documents — got no `superLinkTo()` and a
404+
* 500 at render time (issue #3325, from discussion #3323). Only app overrides
405+
* reach the branch: no two framework mixins contribute the same name, so a
406+
* controller that overrides nothing gains zero extra keys.
399407
*/
400408
private function $integrateFunctions(required array publicMethods, required struct overrideSet) {
401409
local.iEnd = ArrayLen(arguments.publicMethods);
@@ -407,6 +415,10 @@ component output="false" displayName="Controller" extends="wheels.Global"{
407415
if (!(StructKeyExists(variables, local.name) || StructKeyExists(this, local.name))) {
408416
variables[local.name] = local.ref;
409417
this[local.name] = local.ref;
418+
} else {
419+
local.superName = "super" & local.name;
420+
variables[local.superName] = local.ref;
421+
this[local.superName] = local.ref;
410422
}
411423

412424
if (StructKeyExists(arguments.overrideSet, local.name)) {
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
component extends="wheels.Controller" {
2+
3+
/**
4+
* Overrides a framework view helper the way the "Overriding Core Methods" guide
5+
* describes, then delegates to the framework original via the `super<name>`
6+
* convention. Before issue #3325 `superLinkTo` was never registered in
7+
* controller/view context, so this threw at render time.
8+
*/
9+
public string function linkTo() {
10+
return "wrapped:" & superLinkTo(argumentCollection = arguments);
11+
}
12+
13+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
component extends="Model" {
2+
3+
function config() {
4+
table("c_o_r_e_posts");
5+
}
6+
7+
/**
8+
* Model-side counterpart to the controller fixture of the same name. The model
9+
* layer has always registered `super<name>`; this pins that so the parity the
10+
* issue #3325 fix establishes cannot regress from either side.
11+
*/
12+
public string function columnNames() {
13+
return "wrapped:" & superColumnNames();
14+
}
15+
16+
}
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
component extends="wheels.WheelsTest" {
2+
3+
function run() {
4+
g = application.wo
5+
6+
// Regression for issue #3325 (from discussion #3323).
7+
//
8+
// The `super<name>` convention was implemented asymmetrically. `Model.cfc`'s
9+
// $integrateFunctions() registered the framework original as `super<name>` whenever the
10+
// mixin's name already existed on the target — i.e. whenever the app had overridden it.
11+
// `Controller.cfc`'s did not: it only registered `super<name>` for names a registered
12+
// plugin/package mixin overrode. So an app that overrode a controller or view helper,
13+
// exactly as the "Overriding Core Methods" guide documents, got nothing — and calling
14+
// `superLinkTo()` was a 500.
15+
describe("Tests that the super<name> override convention", () => {
16+
17+
it("registers super<name> for an app-level controller/view helper override", () => {
18+
c = g.controller(name = "superOverride")
19+
20+
expect(StructKeyExists(c, "superLinkTo")).toBeTrue()
21+
})
22+
23+
it("lets the override delegate to the framework original", () => {
24+
c = g.controller(name = "superOverride")
25+
26+
// the fixture returns "wrapped:" & superLinkTo(...), so a real anchor
27+
// coming back proves the original ran rather than recursing into the override
28+
result = c.linkTo(text = "Home", route = "root")
29+
30+
expect(result).toStartWith("wrapped:")
31+
expect(result).toInclude("<a")
32+
expect(result).toInclude("Home")
33+
expect(result).notToInclude("wrapped:wrapped:")
34+
})
35+
36+
it("registers super<name> for a model override, unchanged", () => {
37+
// the model side already behaved this way; pinned so the parity cannot
38+
// regress from either direction
39+
m = g.model("superOverride")
40+
41+
expect(StructKeyExists(m, "superColumnNames")).toBeTrue()
42+
expect(m.columnNames()).toStartWith("wrapped:")
43+
})
44+
45+
it("adds no super<name> keys to a controller that overrides nothing", () => {
46+
// the else branch fires only on a genuine override, so the common case pays
47+
// nothing — this runs on every request
48+
c = g.controller(name = "test")
49+
supers = []
50+
for (key in StructKeyArray(c)) {
51+
if (Left(key, 5) == "super") {
52+
ArrayAppend(supers, key)
53+
}
54+
}
55+
56+
expect(ArrayLen(supers)).toBe(0)
57+
})
58+
})
59+
}
60+
61+
}

0 commit comments

Comments
 (0)