From 47aeb9f9cc5a237a92f2423b5e88a831ecc76d8d Mon Sep 17 00:00:00 2001 From: Peter Amiri Date: Mon, 3 Aug 2026 22:20:14 -0700 Subject: [PATCH] fix(controller): register super for app-level controller and view overrides MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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` 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` 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`. 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 ab901cff7 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 --- .../3325-super-override-parity.fixed.md | 1 + vendor/wheels/Controller.cfc | 20 ++++-- .../_assets/controllers/SuperOverride.cfc | 13 ++++ .../tests/_assets/models/SuperOverride.cfc | 16 +++++ .../specs/controller/SuperOverrideSpec.cfc | 61 +++++++++++++++++++ 5 files changed, 107 insertions(+), 4 deletions(-) create mode 100644 changelog.d/3325-super-override-parity.fixed.md create mode 100644 vendor/wheels/tests/_assets/controllers/SuperOverride.cfc create mode 100644 vendor/wheels/tests/_assets/models/SuperOverride.cfc create mode 100644 vendor/wheels/tests/specs/controller/SuperOverrideSpec.cfc diff --git a/changelog.d/3325-super-override-parity.fixed.md b/changelog.d/3325-super-override-parity.fixed.md new file mode 100644 index 0000000000..8aaf5dff9e --- /dev/null +++ b/changelog.d/3325-super-override-parity.fixed.md @@ -0,0 +1 @@ +- Overriding a controller or view helper now registers the framework original as `super`, 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` 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) diff --git a/vendor/wheels/Controller.cfc b/vendor/wheels/Controller.cfc index f3fa5fd182..13afa6c0ea 100644 --- a/vendor/wheels/Controller.cfc +++ b/vendor/wheels/Controller.cfc @@ -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`. `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`, 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` 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); @@ -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)) { diff --git a/vendor/wheels/tests/_assets/controllers/SuperOverride.cfc b/vendor/wheels/tests/_assets/controllers/SuperOverride.cfc new file mode 100644 index 0000000000..6b6136731d --- /dev/null +++ b/vendor/wheels/tests/_assets/controllers/SuperOverride.cfc @@ -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` + * 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); + } + +} diff --git a/vendor/wheels/tests/_assets/models/SuperOverride.cfc b/vendor/wheels/tests/_assets/models/SuperOverride.cfc new file mode 100644 index 0000000000..52b91a3b51 --- /dev/null +++ b/vendor/wheels/tests/_assets/models/SuperOverride.cfc @@ -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`; this pins that so the parity the + * issue #3325 fix establishes cannot regress from either side. + */ + public string function columnNames() { + return "wrapped:" & superColumnNames(); + } + +} diff --git a/vendor/wheels/tests/specs/controller/SuperOverrideSpec.cfc b/vendor/wheels/tests/specs/controller/SuperOverrideSpec.cfc new file mode 100644 index 0000000000..8b2d03cf9b --- /dev/null +++ b/vendor/wheels/tests/specs/controller/SuperOverrideSpec.cfc @@ -0,0 +1,61 @@ +component extends="wheels.WheelsTest" { + + function run() { + g = application.wo + + // Regression for issue #3325 (from discussion #3323). + // + // The `super` convention was implemented asymmetrically. `Model.cfc`'s + // $integrateFunctions() registered the framework original as `super` 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` 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 override convention", () => { + + it("registers super 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(" 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 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) + }) + }) + } + +}