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.d/3325-super-override-parity.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +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)
20 changes: 16 additions & 4 deletions vendor/wheels/Controller.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -391,11 +391,19 @@ component output="false" displayName="Controller" extends="wheels.Global"{

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

if (StructKeyExists(arguments.overrideSet, local.name)) {
Expand Down
13 changes: 13 additions & 0 deletions vendor/wheels/tests/_assets/controllers/SuperOverride.cfc
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
component extends="wheels.Controller" {

/**
* Overrides a framework view helper the way the "Overriding Core Methods" guide
* describes, then delegates to the framework original via the `super<name>`
* convention. Before issue #3325 `superLinkTo` was never registered in
* controller/view context, so this threw at render time.
*/
public string function linkTo() {
return "wrapped:" & superLinkTo(argumentCollection = arguments);
}

}
16 changes: 16 additions & 0 deletions vendor/wheels/tests/_assets/models/SuperOverride.cfc
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
component extends="Model" {

function config() {
table("c_o_r_e_posts");
}

/**
* Model-side counterpart to the controller fixture of the same name. The model
* layer has always registered `super<name>`; this pins that so the parity the
* issue #3325 fix establishes cannot regress from either side.
*/
public string function columnNames() {
return "wrapped:" & superColumnNames();
}

}
61 changes: 61 additions & 0 deletions vendor/wheels/tests/specs/controller/SuperOverrideSpec.cfc
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
component extends="wheels.WheelsTest" {

function run() {
g = application.wo

// Regression for issue #3325 (from discussion #3323).
//
// The `super<name>` convention was implemented asymmetrically. `Model.cfc`'s
// $integrateFunctions() registered the framework original as `super<name>` whenever the
// mixin's name already existed on the target — i.e. whenever the app had overridden it.
// `Controller.cfc`'s did not: it only registered `super<name>` for names a registered
// plugin/package mixin overrode. So an app that overrode a controller or view helper,
// exactly as the "Overriding Core Methods" guide documents, got nothing — and calling
// `superLinkTo()` was a 500.
describe("Tests that the super<name> override convention", () => {

it("registers super<name> for an app-level controller/view helper override", () => {
c = g.controller(name = "superOverride")

expect(StructKeyExists(c, "superLinkTo")).toBeTrue()
})

it("lets the override delegate to the framework original", () => {
c = g.controller(name = "superOverride")

// the fixture returns "wrapped:" & superLinkTo(...), so a real anchor
// coming back proves the original ran rather than recursing into the override
result = c.linkTo(text = "Home", route = "root")

expect(result).toStartWith("wrapped:")
expect(result).toInclude("<a")
expect(result).toInclude("Home")
expect(result).notToInclude("wrapped:wrapped:")
})

it("registers super<name> for a model override, unchanged", () => {
// the model side already behaved this way; pinned so the parity cannot
// regress from either direction
m = g.model("superOverride")

expect(StructKeyExists(m, "superColumnNames")).toBeTrue()
expect(m.columnNames()).toStartWith("wrapped:")
})

it("adds no super<name> keys to a controller that overrides nothing", () => {
// the else branch fires only on a genuine override, so the common case pays
// nothing — this runs on every request
c = g.controller(name = "test")
supers = []
for (key in StructKeyArray(c)) {
if (Left(key, 5) == "super") {
ArrayAppend(supers, key)
}
}

expect(ArrayLen(supers)).toBe(0)
})
})
}

}
Loading