feat: route-scoped middleware via existing interceptor points - #679
Merged
Conversation
Router.middleware() attaches middleware to a route at a ColdBox
interception point (preProcess by default, or postProcess) instead of
introducing a parallel middleware subsystem. A target can be an inline
closure, a WireBox ID (resolved via getInstance() on every call, so it
respects the mapping's own declared scope), or any object - WireBox
managed or not - that exposes a method named after the point, the same
duck-typed convention ColdBox interceptors already use.
group({ middleware: [...] }, body) shares a chain across every route
registered in the body, ahead of each route's own middleware(), tracked
on its own stack so nested groups compose correctly independent of
group()'s existing withClosure/onGroup nesting limitation.
RoutingService.runRouteMiddleware() executes the current route's
middleware for a given point, wired into Bootstrap.cfc right after the
global preProcess announce and right before postProcess - route-scoped
middleware runs closest to the handler, global interceptors stay the
outermost layer. A target returning true short-circuits the remaining
middleware at that point for that route, mirroring
InterceptorState.processSync()'s existing short-circuit contract; it
does not by itself skip the handler or render, matching how a normal
preProcess/postProcess interceptor works today.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016kCmPkBvNZZhU6iuDcG4NR
The WireBox-ID resolution test registered its mapping with no explicit scope, so getInstance() returned a fresh instance per call under the default (non-singleton) scope - the test's post-hoc assertion never saw the state the production code had mutated. Registers it as a singleton explicitly, matching what the docstring already promised: resolution respects the mapping's own declared scope. Also replaces group()'s arrow-function/map() middleware normalization with a plain for-loop, matching the rest of the file's established style more closely and avoiding CI's older cfformat disagreeing with a newer local run. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016kCmPkBvNZZhU6iuDcG4NR
…t block The new variables.groupMiddlewareStack line sat inside a comment-interrupted block of variable initializers that cfformat column-aligns across all lines. Its longer name pushed every other line's = column out, and local (newer) cfformat and CI's (older) cfformat apparently disagree on how far that realignment should propagate - CI kept flagging Router.cfc without saying why. Moving the new line after a blank line keeps the original four-line block byte-for-byte as it was before this feature, sidestepping the disagreement entirely. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016kCmPkBvNZZhU6iuDcG4NR
…ame line A background agent pulled the exact CI container image (cfml-ci-tools 1.0.12, CommandBox 5.8.0) and ran cfformat check --verbose against it, producing the real diff: the file's earlier session had already padded this line to satisfy CI's alignment.consecutive.assignments rule inside the mcpResponseClosure arrow function, but a later `cfformat run --overwrite` pass in this session (using a newer local cfformat that disagrees on whether a nested var inside an arrow-function body joins the outer assignment's alignment group) silently stripped that padding back out. Restoring it - unrelated to this session's actual feature work, same as the earlier documented instance of this exact drift. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016kCmPkBvNZZhU6iuDcG4NR
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces route-scoped middleware by reusing existing ColdBox interception points (preProcess by default, optionally postProcess) and executing middleware only for the currently matched route (plus group-level inheritance).
Changes:
- Adds
Router.middleware()and amiddlewareroute key, including group-level middleware inheritance via a newgroupMiddlewareStack. - Adds
RoutingService.runRouteMiddleware()with target resolution (closures, WireBox IDs, duck-typed objects) and short-circuit semantics viatruereturn. - Wires route middleware execution into the request lifecycle in
Bootstrap.cfc, with accompanying unit tests and fixtures.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| system/web/routing/Router.cfc | Adds middleware() modifier, route middleware storage, and group-level middleware inheritance via groupMiddlewareStack. |
| system/web/services/RoutingService.cfc | Implements runRouteMiddleware() and WireBox-ID resolution for route middleware targets. |
| system/Bootstrap.cfc | Executes route middleware after global preProcess and before global postProcess. |
| tests/specs/web/routing/RouterTest.cfc | Verifies middleware registration, accumulation, and group/nested-group inheritance order. |
| tests/specs/web/routing/RoutingServiceTest.cfc | Verifies execution semantics: point filtering, closures, WireBox IDs, duck-typed objects, and short-circuiting. |
| tests/resources/routing/SampleMiddleware.cfc | Adds a plain CFC fixture to validate duck-typed preProcess/postProcess dispatch. |
Suppressed comments (1)
system/web/routing/Router.cfc:543
- If the
group()body throws, the current implementation will skip the cleanup that popsgroupMiddlewareStackand resetsonGroup/withClosure, which can leak group-level middleware/options into subsequent route registrations. Wrap the body execution in a try/finally (or try/catch/finally) so cleanup always runs.
// Execute the body
arguments.body( arguments.options );
// Pivot out of the group and do cleanup
variables.groupMiddlewareStack.deleteAt( variables.groupMiddlewareStack.len() );
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+529
to
+537
| var groupMiddleware = structKeyExists( arguments.options, "middleware" ) ? arguments.options.middleware : []; | ||
| var normalizedGroupMW = []; | ||
| for ( var entry in groupMiddleware ) { | ||
| if ( isStruct( entry ) && entry.keyExists( "target" ) ) { | ||
| normalizedGroupMW.append( entry ); | ||
| } else { | ||
| normalizedGroupMW.append( { "target" : entry, "point" : "preProcess" } ); | ||
| } | ||
| } |
- group()'s options.middleware is now normalized to an array before iterating - a single non-array target (e.g. a bare closure or WireBox ID string, not wrapped in []) would otherwise iterate its characters (if a string) or fail outright, instead of being treated as one entry. - A struct entry in options.middleware that omits its own `point` key no longer throws later in RoutingService.runRouteMiddleware() when that key is read - it now defaults to "preProcess", same as every other middleware()-registered entry. - group() now wraps body execution in try/finally: if the body throws, groupMiddlewareStack/onGroup/withClosure cleanup still runs, so a failed group registration can't leak its middleware/options into whatever gets registered next. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016kCmPkBvNZZhU6iuDcG4NR
Member
Author
|
Both findings were real, fixed in 7274750:
New test coverage for all three in the same commit. Generated by Claude Code |
Route-scoped middleware currently only inherits through literal group()
nesting, with no way to share a bundle across unrelated routes or opt a
single route out of an inherited one. Adds two Laravel-inspired pieces on
top of the existing flat preProcess/postProcess dispatch:
- middlewareGroup(name, [...]) registers a named, reusable bundle,
referenced by name from either .middleware() or group({ middleware }).
Groups are flat - a member can't itself be another group's name - so
there's no cycle risk.
- withoutMiddleware(target) excludes middleware a route would otherwise
inherit, matched by WireBox ID or by the middlewareGroup() name an
entry was expanded from (dropping the whole bundle), or "*" for
everything.
normalizeMiddlewareEntries() is the shared expansion point used by
.middleware(), group()'s middleware option, and middlewareGroup() itself,
tagging group-expanded entries with their source group name so
withoutMiddleware() can match on it.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016kCmPkBvNZZhU6iuDcG4NR
…rement Final review of the named-groups/withoutMiddleware() work surfaced one real gap: group expansion happens immediately at registration time, so a name referenced before its middlewareGroup() call is silently treated as a literal target instead of being expanded - no error, just quietly wrong. Documents the requirement on both middleware() and middlewareGroup(), and adds a regression test pinning the current (silent-fallback) behavior so it stays visible rather than being an undiscovered trap. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016kCmPkBvNZZhU6iuDcG4NR
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Route-scoped middleware: attach middleware to a route at an existing ColdBox interception point (
preProcessby default, orpostProcess) instead of inventing a parallel middleware subsystem. See the full write-up and examples below.Jira Issues
COLDBOX-1416
Type of change
Checklist
Adds
Router.middleware(): attach middleware to a route at an existing ColdBoxinterception point (
preProcessby default, orpostProcess) instead ofinventing a parallel middleware subsystem. It reuses the same
dynamic method dispatch ColdBox
interceptors already use (
invoke(target, point, args)), just scoped to oneroute instead of the whole app.
Evaluated this design against Laravel's middleware pipeline (
$next-closureonion composition,
$middlewareGroups,withoutMiddleware()). Laravel's truewrapping composition doesn't fit here - ColdBox's chain is flat before/after
dispatch by design (
aroundHandleralready covers genuine wrapping at thehandler level) - but two of its ideas were worth adopting directly: named,
reusable middleware groups, and excluding inherited middleware per route.
A target can be
function( event, rc, prc ){ ... }getInstance()on every request, so itrespects whatever scope (singleton, prototype, etc) the mapping was
registered with
after the point (
preProcess()/postProcess()). No base class orinterface required - the same duck-typed convention ColdBox interceptors
themselves already use.
middlewareGroup()bundle - expands in place to thatbundle's own targets.
Examples
Semantics
truefrom a target short-circuits the remaining middleware forthat route at that point - the same contract
InterceptorState.processSync()already uses for the global chain. It doesnot, by itself, skip the handler or the render; call
event.relocate(),event.renderData().noExecution(),event.etag(),etc, exactly as you would from any other
preProcess/postProcessinterceptor.
preProcessannounce and before the global
postProcessannounce - route-specificwork happens closest to the handler, global interceptors stay the
outermost layer.
group({ middleware: [...] })is tracked on its own stack, independent ofgroup()'s existingwithClosure/onGroupstate, so nested groupscompose correctly.
middlewareGroup()bundles are flat - a member can't itself be anothergroup's name - so there's no cycle to guard against.
withoutMiddleware()matches by name: a WireBox ID, or amiddlewareGroup()name (which drops every member that group expanded to, not just a
same-named single target). Closures and object instances have no name to
match, so they can only be kept off a route by not attaching them.
Files touched
system/web/routing/Router.cfc-middleware()/middlewareGroup()/withoutMiddleware()fluent modifiers,middleware/withoutMiddlewareroute-struct keys, group-level middleware inheritance via
groupMiddlewareStack, shared expansion vianormalizeMiddlewareEntries()system/web/services/RoutingService.cfc-runRouteMiddleware()executesthe matched route's middleware for a given point;
resolveMiddlewareTarget()resolves WireBox ID strings
system/Bootstrap.cfc- wiresrunRouteMiddleware()in right after thepreProcessannounce and right before thepostProcessannouncetests/specs/web/routing/RouterTest.cfc/tests/specs/web/routing/RoutingServiceTest.cfc- fluent API, group/nestedgroup inheritance, closure/WireBox-ID/duck-typed-object dispatch,
short-circuit, point filtering, named group expansion (including per-member
point overrides), and
withoutMiddleware()by target name, by group name,and via
"*"tests/resources/routing/SampleMiddleware.cfc- a plain class fixture withno base class, proving middleware works by method-name convention alone
Testing notes
RouterTest.cfcextendsBaseModelTestand exercisesRouter.cfcdirectlyvia
createMock(), with no servlet/database dependency, so it's runnable inCI as-is. The sandbox this PR was authored in has no reachable test database,
so the full TestBox HTTP runner (
tests/runner.cfm) couldn't be exercisedlocally regardless of engine; instead every new registration-time behavior
(group expansion, per-member point overrides,
withoutMiddleware()by target/group/
"*") was verified with a standalone script instantiatingRouter.cfcdirectly and asserting against its real methods, matching the same assertions
now encoded in
RouterTest.cfc.