Skip to content

Commit ce445f0

Browse files
bpamiribpamiri
authored
Merge pull request #1894 from wheels-dev/claude/fix-pr-1891-workflow-pGc9Z
feat: modernize router with group(), typed constraints, API versioning, and performance indexes
2 parents 2a79fb3 + 8f9e7d8 commit ce445f0

File tree

14 files changed

+998
-86
lines changed

14 files changed

+998
-86
lines changed

‎.github/workflow-results/test-lucee7-mysql.md‎

Lines changed: 958 additions & 0 deletions
Large diffs are not rendered by default.

‎vendor/wheels/Dispatch.cfc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,19 +111,19 @@ component output="false" extends="wheels.Global"{
111111

112112
local.methodKey = UCase(arguments.requestMethod);
113113

114-
// --- Fast path 1: Static route O(1) lookup ---
114+
// --- Fast path: Static route O(1) lookup ---
115115
// Static routes (no variables in pattern) are indexed in a hash map at registration time.
116116
// This avoids regex matching entirely for common static paths like /login, /about, etc.
117117
if (StructKeyExists(application.wheels, "staticRoutes")) {
118118
local.staticKey = local.methodKey & ":/" & arguments.path;
119119
if (StructKeyExists(application.wheels.staticRoutes, local.staticKey)) {
120-
local.rv = Duplicate(application.wheels.staticRoutes[local.staticKey]);
120+
local.rv = StructCopy(application.wheels.staticRoutes[local.staticKey]);
121121
}
122122
// Also try the root path.
123123
if (!StructKeyExists(local, "rv") && !Len(arguments.path)) {
124124
local.staticKey = local.methodKey & ":/";
125125
if (StructKeyExists(application.wheels.staticRoutes, local.staticKey)) {
126-
local.rv = Duplicate(application.wheels.staticRoutes[local.staticKey]);
126+
local.rv = StructCopy(application.wheels.staticRoutes[local.staticKey]);
127127
}
128128
}
129129
}

‎vendor/wheels/Mapper.cfc‎

Lines changed: 19 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,8 @@ component output="false" {
3838
// placeholder for return value
3939
variables.routes = [];
4040

41-
// Performance indexes for faster route matching.
42-
// routeIndex: maps HTTP method -> array of routes for that method.
41+
// Performance index for faster route matching.
4342
// staticRoutes: maps "METHOD:/path" -> route struct for routes with no variables (O(1) lookup).
44-
variables.routeIndex = {};
4543
variables.staticRoutes = {};
4644

4745
return this;
@@ -132,9 +130,7 @@ component output="false" {
132130
/**
133131
* Private internal function.
134132
* Add route to Wheels, removing useless params.
135-
* Also builds performance indexes for faster route matching:
136-
* - routeIndex: Routes indexed by HTTP method (reduces search space)
137-
* - staticRoutes: Hash map for routes with no variables (O(1) lookup)
133+
* Also builds a static route index for O(1) lookup of routes with no variables.
138134
*/
139135
private void function $addRoute(required string pattern, required struct constraints) {
140136
// Remove controller and action if they are route variables.
@@ -159,40 +155,32 @@ component output="false" {
159155
// Static routes can be matched via O(1) hash lookup instead of regex scanning.
160156
arguments.isStatic = !Find("[", arguments.pattern);
161157

162-
// Add route to Wheels.
163-
ArrayAppend(variables.routes, arguments);
164-
ArrayAppend(application[$appKey()].routes, arguments);
165-
166-
// Build method-indexed lookup for faster matching.
167-
// This allows $findMatchingRoute to only scan routes for the requested HTTP method.
168-
if (StructKeyExists(arguments, "methods")) {
169-
local.methodList = ListToArray(arguments.methods);
170-
} else {
171-
// If no method restriction, index under all methods.
172-
local.methodList = ["get", "post", "put", "patch", "delete", "head"];
173-
}
158+
// Create a plain struct copy of the route data. On Adobe CF, the arguments
159+
// scope is a special struct type that can cause Duplicate() failures when
160+
// stored in shared scopes and later deep-copied. StructCopy() produces a
161+
// plain CFML struct that is safe to Duplicate() on all engines.
162+
local.routeStruct = StructCopy(arguments);
174163

175-
for (local.method in local.methodList) {
176-
local.methodKey = UCase(local.method);
164+
// Add route to Wheels.
165+
ArrayAppend(variables.routes, local.routeStruct);
166+
ArrayAppend(application[$appKey()].routes, local.routeStruct);
177167

178-
// Initialize route index and static route map in application scope if needed.
179-
if (!StructKeyExists(application[$appKey()], "routeIndex")) {
180-
application[$appKey()].routeIndex = {};
181-
}
168+
// Build static route index for O(1) lookup of routes with no variables.
169+
if (local.routeStruct.isStatic) {
182170
if (!StructKeyExists(application[$appKey()], "staticRoutes")) {
183171
application[$appKey()].staticRoutes = {};
184172
}
185173

186-
if (!StructKeyExists(application[$appKey()].routeIndex, local.methodKey)) {
187-
application[$appKey()].routeIndex[local.methodKey] = [];
174+
if (StructKeyExists(local.routeStruct, "methods")) {
175+
local.methodList = ListToArray(local.routeStruct.methods);
176+
} else {
177+
local.methodList = ["get", "post", "put", "patch", "delete", "head"];
188178
}
189-
ArrayAppend(application[$appKey()].routeIndex[local.methodKey], arguments);
190179

191-
// Add to static route fast-path index.
192-
if (arguments.isStatic) {
193-
local.staticKey = local.methodKey & ":" & arguments.pattern;
180+
for (local.method in local.methodList) {
181+
local.staticKey = UCase(local.method) & ":" & local.routeStruct.pattern;
194182
if (!StructKeyExists(application[$appKey()].staticRoutes, local.staticKey)) {
195-
application[$appKey()].staticRoutes[local.staticKey] = arguments;
183+
application[$appKey()].staticRoutes[local.staticKey] = local.routeStruct;
196184
}
197185
}
198186
}

‎vendor/wheels/mapper/matching.cfc‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -565,6 +565,11 @@ component {
565565

566566
// Recompile the regex with the new constraint.
567567
local.route.regex = $patternToRegex(local.route.pattern, local.route.constraints);
568+
569+
// Explicit write-back: on Adobe CF, modifying a local copy of an
570+
// array element does not always propagate back to the array. Write
571+
// the modified struct back to guarantee cross-engine safety.
572+
application[$appKey()].routes[local.i] = local.route;
568573
}
569574

570575
// If no name, only update the very last route.
@@ -589,6 +594,9 @@ component {
589594
}
590595
local.route.constraints[local.varName] = arguments.pattern;
591596
local.route.regex = $patternToRegex(local.route.pattern, local.route.constraints);
597+
598+
// Explicit write-back for Adobe CF safety (see comment above).
599+
variables.routes[local.i] = local.route;
592600
}
593601

594602
if (!Len(local.lastRouteName)) {

‎vendor/wheels/tests_testbox/specs/dispatch/findMatchingRouteMegaSpec.cfc‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@ component extends="wheels.WheelsTest" {
22

33
function beforeAll() {
44
_originalRoutes = Duplicate(application.wheels.routes)
5-
_originalRouteIndex = StructKeyExists(application.wheels, "routeIndex") ? Duplicate(application.wheels.routeIndex) : {}
6-
_originalStaticRoutes = StructKeyExists(application.wheels, "staticRoutes") ? Duplicate(application.wheels.staticRoutes) : {}
5+
_originalStaticRoutes = StructKeyExists(application.wheels, "staticRoutes") ? StructCopy(application.wheels.staticRoutes) : {}
76
nounPlurals = [
87
"people",
98
"dogs",
@@ -83,7 +82,6 @@ component extends="wheels.WheelsTest" {
8382

8483
function afterAll() {
8584
application.wheels.routes = _originalRoutes
86-
application.wheels.routeIndex = _originalRouteIndex
8785
application.wheels.staticRoutes = _originalStaticRoutes
8886
}
8987

@@ -125,7 +123,6 @@ component extends="wheels.WheelsTest" {
125123

126124
public void function $clearRoutes() {
127125
application.wheels.routes = []
128-
application.wheels.routeIndex = {}
129126
application.wheels.staticRoutes = {}
130127
}
131128
}

‎vendor/wheels/tests_testbox/specs/dispatch/findMatchingRouteSpec.cfc‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,8 @@ component extends="wheels.WheelsTest" {
22

33
function beforeAll() {
44
_originalRoutes = Duplicate(application.wheels.routes)
5-
_originalRouteIndex = StructKeyExists(application.wheels, "routeIndex") ? Duplicate(application.wheels.routeIndex) : {}
6-
_originalStaticRoutes = StructKeyExists(application.wheels, "staticRoutes") ? Duplicate(application.wheels.staticRoutes) : {}
5+
_originalStaticRoutes = StructKeyExists(application.wheels, "staticRoutes") ? StructCopy(application.wheels.staticRoutes) : {}
76
application.wheels.routes = []
8-
application.wheels.routeIndex = {}
97
application.wheels.staticRoutes = {}
108

119
application.wo.mapper()
@@ -23,7 +21,6 @@ component extends="wheels.WheelsTest" {
2321

2422
function afterAll() {
2523
application.wheels.routes = _originalRoutes
26-
application.wheels.routeIndex = _originalRouteIndex
2724
application.wheels.staticRoutes = _originalStaticRoutes
2825
}
2926

‎vendor/wheels/tests_testbox/specs/dispatch/requestSpec.cfc‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,14 @@ component extends="wheels.WheelsTest" {
77
beforeEach(() => {
88
_params = {controller = "test", action = "index"}
99
_originalRoutes = Duplicate(application.wheels.routes)
10-
_originalRouteIndex = StructKeyExists(application.wheels, "routeIndex") ? Duplicate(application.wheels.routeIndex) : {}
11-
_originalStaticRoutes = StructKeyExists(application.wheels, "staticRoutes") ? Duplicate(application.wheels.staticRoutes) : {}
10+
_originalStaticRoutes = StructKeyExists(application.wheels, "staticRoutes") ? StructCopy(application.wheels.staticRoutes) : {}
1211
application.wheels.routes = []
13-
application.wheels.routeIndex = {}
1412
application.wheels.staticRoutes = {}
1513
dispatch = CreateObject("component", "wheels.Dispatch")
1614
})
1715

1816
afterEach(() => {
1917
application.wheels.routes = _originalRoutes
20-
application.wheels.routeIndex = _originalRouteIndex
2118
application.wheels.staticRoutes = _originalStaticRoutes
2219
})
2320

‎vendor/wheels/tests_testbox/specs/dispatch/setCorsHeadersSpec.cfc‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,11 @@ component extends="wheels.WheelsTest" {
1515
$$oldCGIScope = request.cgi;
1616
$$oldHeaders = request.wheels.httprequestdata.headers;
1717
$$originalRoutes = application.wheels.routes;
18-
$$originalRouteIndex = StructKeyExists(application.wheels, "routeIndex") ? Duplicate(application.wheels.routeIndex) : {};
19-
$$originalStaticRoutes = StructKeyExists(application.wheels, "staticRoutes") ? Duplicate(application.wheels.staticRoutes) : {};
18+
$$originalStaticRoutes = StructKeyExists(application.wheels, "staticRoutes") ? StructCopy(application.wheels.staticRoutes) : {};
2019
application.wheels.allowCorsRequests = true;
2120
d = $createObjectFromRoot(path = "wheels", fileName = "Dispatch", method = "$init");
2221
$resetHeaders();
2322
application.wheels.routes = [];
24-
application.wheels.routeIndex = {};
2523
application.wheels.staticRoutes = {};
2624
config = {path = "wheels", fileName = "Mapper", method = "$init"};
2725
}
@@ -32,7 +30,6 @@ component extends="wheels.WheelsTest" {
3230
request.cgi = $$oldCGIScope;
3331
request.wheels.httprequestdata.headers = $$oldHeaders;
3432
application.wheels.routes = $$originalRoutes;
35-
application.wheels.routeIndex = $$originalRouteIndex;
3633
application.wheels.staticRoutes = $$originalStaticRoutes;
3734
application[$appKey()].allowCorsRequests = false;
3835
d = "";

‎vendor/wheels/tests_testbox/specs/global/urlforSpec.cfc‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,13 @@ component extends="wheels.WheelsTest" {
1010
config = {path = "wheels", fileName = "Mapper", method = "$init"}
1111
_params = {controller = "test", action = "index"}
1212
_originalRoutes = Duplicate(application.wheels.routes)
13-
_originalRouteIndex = StructKeyExists(application.wheels, "routeIndex") ? Duplicate(application.wheels.routeIndex) : {}
14-
_originalStaticRoutes = StructKeyExists(application.wheels, "staticRoutes") ? Duplicate(application.wheels.staticRoutes) : {}
13+
_originalStaticRoutes = StructKeyExists(application.wheels, "staticRoutes") ? StructCopy(application.wheels.staticRoutes) : {}
1514
_originalUrlRewriting = application.wheels.URLRewriting
1615
_originalObfuscateUrls = application.wheels.obfuscateUrls
1716
})
1817

1918
afterEach(() => {
2019
application.wheels.routes = _originalRoutes
21-
application.wheels.routeIndex = _originalRouteIndex
2220
application.wheels.staticRoutes = _originalStaticRoutes
2321
application.wheels.URLRewriting = _originalUrlRewriting
2422
application.wheels.obfuscateUrls = _originalObfuscateUrls
@@ -192,7 +190,6 @@ component extends="wheels.WheelsTest" {
192190

193191
public void function $clearRoutes() {
194192
application.wheels.routes = []
195-
application.wheels.routeIndex = {}
196193
application.wheels.staticRoutes = {}
197194
}
198195
}

‎vendor/wheels/tests_testbox/specs/mapper/MapperSpec.cfc‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,11 @@ component extends="wheels.WheelsTest" {
3131
config = {path = "wheels", fileName = "Mapper", method = "$init"}
3232
_params = {controller = "test", action = "index"}
3333
_originalRoutes = Duplicate(application.wheels.routes)
34-
_originalRouteIndex = StructKeyExists(application.wheels, "routeIndex") ? Duplicate(application.wheels.routeIndex) : {}
35-
_originalStaticRoutes = StructKeyExists(application.wheels, "staticRoutes") ? Duplicate(application.wheels.staticRoutes) : {}
34+
_originalStaticRoutes = StructKeyExists(application.wheels, "staticRoutes") ? StructCopy(application.wheels.staticRoutes) : {}
3635
})
3736

3837
afterEach(() => {
3938
application.wheels.routes = _originalRoutes
40-
application.wheels.routeIndex = _originalRouteIndex
4139
application.wheels.staticRoutes = _originalStaticRoutes
4240
})
4341

@@ -88,7 +86,6 @@ component extends="wheels.WheelsTest" {
8886

8987
public void function $clearRoutes() {
9088
application.wheels.routes = []
91-
application.wheels.routeIndex = {}
9289
application.wheels.staticRoutes = {}
9390
}
9491

0 commit comments

Comments
 (0)