Skip to content

Commit 4742098

Browse files
authored
refactor: No normalized path interning (#375)
1 parent a623f2d commit 4742098

12 files changed

Lines changed: 199 additions & 161 deletions

File tree

‎packages/relic/test/router/relic_app_test.dart‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,34 @@ void main() {
3939
);
4040
});
4141

42+
test('Given a RelicApp with a route, '
43+
'when calling lookupUri, '
44+
'then it delegates to the underlying router', () {
45+
Response handler(final Request req) => Response.ok();
46+
final app = RelicApp()..get('/a/b', handler);
47+
48+
// The query and fragment are not part of the route path.
49+
final hit = app.lookupUri(
50+
Method.get,
51+
Uri.parse('http://example.com/a/b?q=1#frag'),
52+
);
53+
expect(hit, isA<RouterMatch<Handler>>());
54+
expect((hit as RouterMatch<Handler>).value, same(handler));
55+
56+
expect(
57+
app.lookupUri(Method.post, Uri.parse('http://example.com/a/b')),
58+
isA<MethodMiss<Handler>>(),
59+
);
60+
expect(
61+
app.lookupUri(
62+
Method.get,
63+
Uri.parse('http://example.com/nope'),
64+
backtrack: false,
65+
),
66+
isA<PathMiss<Handler>>(),
67+
);
68+
});
69+
4270
test('Given a RelicApp, '
4371
'when calling run with adapter factory, '
4472
'then it creates a RelicServer and mounts the handler', () async {

‎packages/relic_core/lib/src/middleware/routing_middleware.dart‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -110,11 +110,15 @@ class _RoutingMiddlewareBuilder<T extends Object> {
110110

111111
Handler call(final Handler next) {
112112
return (final req) async {
113-
final path = NormalizedPath.fromUri(req.url);
114-
final routingKey = useHostWhenRouting
115-
? NormalizedPath.fromSegments([req.url.host, ...path.segments])
116-
: path;
117-
final result = _router.lookupPath(req.method, routingKey);
113+
final result = useHostWhenRouting
114+
? _router.lookupPath(
115+
req.method,
116+
NormalizedPath.fromSegments([
117+
req.url.host,
118+
...NormalizedPath.fromUri(req.url).segments,
119+
]),
120+
)
121+
: _router.lookupUri(req.method, req.url);
118122
switch (result) {
119123
case MethodMiss():
120124
return Response(

‎packages/relic_core/lib/src/router/no_cache.dart‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,7 @@
11
import 'cache.dart';
22

3-
/// A no-op [Cache] implementation that never stores or retrieves values.
4-
///
5-
/// Useful for high-cardinality workloads where caching causes more overhead
6-
/// than it saves (e.g., many unique dynamic paths like `/users/:id`).
7-
///
8-
/// Example:
9-
/// ```dart
10-
/// NormalizedPath.interned = NoCache();
11-
/// ```
3+
/// A no-op [Cache] that never stores or retrieves values, for opting out of
4+
/// caching in high-cardinality workloads where it costs more than it saves.
125
final class NoCache<K, V> implements Cache<K, V> {
136
/// Creates a no-op cache.
147
const NoCache();

‎packages/relic_core/lib/src/router/normalized_path.dart‎

Lines changed: 21 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,17 @@
11
import 'package:meta/meta.dart';
22

3-
import 'cache.dart';
4-
import 'lru_cache.dart';
5-
63
/// Represents a URL path that has been normalized.
74
///
85
/// Normalization includes:
96
/// - Resolving `.` and `..` segments.
107
/// - Removing empty segments caused by multiple consecutive slashes.
118
/// - Ensuring the path starts with a `/`.
129
///
13-
/// Instances created from a path string are interned using an LRU cache for
14-
/// efficiency, so identical paths will often share the same object instance.
15-
/// Segment-built instances ([fromSegments], [fromUri]) are never interned: a
16-
/// string cache key cannot tell a separator inside a segment from a real one.
17-
/// Equality compares segments, so this affects allocation only.
10+
/// Equality and [hashCode] are derived from [segments], so paths with the same
11+
/// segments are equal regardless of how they were built. Construction does no
12+
/// caching.
1813
@immutable
1914
class NormalizedPath {
20-
/// Cache of interned instances.
21-
///
22-
/// Defaults to an [LruCache] with 10,000 entries. Can be replaced with any
23-
/// [Cache] implementation to tune caching behavior:
24-
///
25-
/// ```dart
26-
/// // Disable caching for high-cardinality workloads
27-
/// NormalizedPath.interned = NoCache();
28-
///
29-
/// // Use a larger cache
30-
/// NormalizedPath.interned = LruCache(50000);
31-
/// ```
32-
static Cache<String, NormalizedPath> interned =
33-
LruCache<String, NormalizedPath>(10000);
34-
3515
/// The individual segments of the normalized path.
3616
/// For example, the path `/a/b/c` would have segments `['a', 'b', 'c']`.
3717
final List<String> segments;
@@ -44,20 +24,12 @@ class NormalizedPath {
4424

4525
/// Creates a [NormalizedPath] from a given [path] string.
4626
///
47-
/// The provided [path] will be normalized by resolving `.` and `..` segments
48-
/// and removing empty segments. The resulting [NormalizedPath] instance may be
49-
/// retrieved from a cache if an identical normalized path has been created
50-
/// recently.
51-
factory NormalizedPath(final String path) {
52-
var result = interned[path];
53-
if (result == null) {
54-
result = NormalizedPath._(_normalizeSegments(path.split('/')));
55-
// intern for both normalized path and path
56-
result = interned[result.path] ??= result;
57-
interned[path] = result; // cache for original path as well
58-
}
59-
return result;
60-
}
27+
/// The provided [path] is split on `/` and normalized by resolving `.` and
28+
/// `..` segments and removing empty ones. The path is not percent-decoded,
29+
/// so an encoded separator such as `%2F` stays literal within its segment;
30+
/// use [NormalizedPath.fromUri] to derive a path from a request.
31+
factory NormalizedPath(final String path) =>
32+
NormalizedPath._(_normalizeSegments(path.split('/')));
6133

6234
/// Creates a [NormalizedPath] from segments that have already been split.
6335
///
@@ -71,14 +43,18 @@ class NormalizedPath {
7143

7244
/// Creates a [NormalizedPath] from the path of [url].
7345
///
74-
/// This is the correct way to derive a path from a request. It reads
75-
/// [Uri.pathSegments], which splits on the separator and only then decodes
76-
/// each segment, so an encoded separator such as `%2F` stays inside its
77-
/// segment. Building from [Uri.path] instead would decode first and then
78-
/// split, introducing separators that no proxy in front of the server ever
79-
/// saw.
80-
factory NormalizedPath.fromUri(final Uri url) =>
81-
NormalizedPath.fromSegments(url.pathSegments);
46+
/// [Uri.pathSegments] splits on the separator before decoding, so an encoded
47+
/// separator (`%2F`) stays within its segment.
48+
factory NormalizedPath.fromUri(final Uri url) {
49+
final segments = url.pathSegments;
50+
// Reuse the unmodifiable pathSegments if clean
51+
for (final segment in segments) {
52+
if (segment.isEmpty || segment == '.' || segment == '..') {
53+
return NormalizedPath._(_normalizeSegments(segments));
54+
}
55+
}
56+
return NormalizedPath._(segments);
57+
}
8258

8359
/// Normalizes [segments] by resolving `.` and `..` and dropping empty ones.
8460
static List<String> _normalizeSegments(final Iterable<String> segments) {

‎packages/relic_core/lib/src/router/path_trie.dart‎

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,10 @@ sealed class _DynamicSegment<T> {
6060
final class _Parameter<T> extends _DynamicSegment<T> {
6161
final String name;
6262

63-
_Parameter(this.name);
63+
/// The parameter [name] as a [Symbol], precomputed for use during lookup.
64+
final Symbol symbol;
65+
66+
_Parameter(this.name) : symbol = Symbol(name);
6467
}
6568

6669
final class _Wildcard<T> extends _DynamicSegment<T> {}
@@ -392,7 +395,13 @@ final class PathTrie<T extends Object> {
392395
final bool backtrack = true,
393396
}) {
394397
return backtrack
395-
? _lookupRecursive(_root, normalizedPath, 0, _root.map, const {})
398+
? _lookupRecursive(
399+
_root,
400+
normalizedPath,
401+
0,
402+
_root.map,
403+
<Symbol, String>{},
404+
)
396405
: _lookupIterative(normalizedPath);
397406
}
398407

@@ -432,17 +441,14 @@ final class PathTrie<T extends Object> {
432441

433442
final segment = segments[index];
434443

444+
TrieMatch<T>? next(_TrieNode<T> node, final T Function(T)? map) =>
445+
_lookupRecursive(node, normalizedPath, index + 1, map, parameters);
446+
435447
// Try literal match first
436448
final child = node.children[segment];
437449
if (child != null) {
438450
final newMap = _composeMap(currentMap, child.map);
439-
final result = _lookupRecursive(
440-
child,
441-
normalizedPath,
442-
index + 1,
443-
newMap,
444-
parameters,
445-
);
451+
final result = next(child, newMap);
446452
if (result != null) return result;
447453
// Fall through to try dynamic segment
448454
}
@@ -452,9 +458,6 @@ final class PathTrie<T extends Object> {
452458
if (dynamicSegment != null) {
453459
final dynamicNode = dynamicSegment.node;
454460
final newMap = _composeMap(currentMap, dynamicNode.map);
455-
final newParams = dynamicSegment is _Parameter<T>
456-
? {...parameters, Symbol(dynamicSegment.name): segment}
457-
: parameters;
458461

459462
if (dynamicSegment is _Tail<T>) {
460463
// Tail matches: check for value at this position
@@ -463,19 +466,23 @@ final class PathTrie<T extends Object> {
463466
value = newMap?.call(value) ?? value;
464467
return TrieMatch(
465468
value,
466-
newParams,
469+
parameters,
467470
normalizedPath.subPath(0, index),
468471
normalizedPath.subPath(index),
469472
);
470473
}
474+
} else if (dynamicSegment is _Parameter<T>) {
475+
final prev = parameters[dynamicSegment.symbol];
476+
parameters[dynamicSegment.symbol] = segment;
477+
final result = next(dynamicNode, newMap);
478+
if (result != null) return result;
479+
if (prev == null) {
480+
parameters.remove(dynamicSegment.symbol);
481+
} else {
482+
parameters[dynamicSegment.symbol] = prev;
483+
}
471484
} else {
472-
return _lookupRecursive(
473-
dynamicNode,
474-
normalizedPath,
475-
index + 1,
476-
newMap,
477-
newParams,
478-
);
485+
return next(dynamicNode, newMap);
479486
}
480487
}
481488

@@ -524,7 +531,7 @@ final class PathTrie<T extends Object> {
524531
currentNode = dynamicSegment.node;
525532
updateMap();
526533
if (dynamicSegment case final _Parameter<T> parameter) {
527-
parameters[Symbol(parameter.name)] = segment;
534+
parameters[parameter.symbol] = segment;
528535
}
529536
if (dynamicSegment is _Tail<T>) break; // possible early match
530537
}

‎packages/relic_core/lib/src/router/relic_app.dart‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,13 @@ final class RelicApp implements RelicRouter, _Reloadable {
192192
final NormalizedPath normalizedPath, {
193193
final bool backtrack = true,
194194
}) => delegate.lookupPath(method, normalizedPath, backtrack: backtrack);
195+
196+
@override
197+
LookupResult<Handler> lookupUri(
198+
final Method method,
199+
final Uri url, {
200+
final bool backtrack = true,
201+
}) => delegate.lookupUri(method, url, backtrack: backtrack);
195202
}
196203

197204
/// Developer tools for inspecting and debugging a [RelicApp].

‎packages/relic_core/lib/src/router/router.dart‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -165,9 +165,9 @@ final class Router<T extends Object> {
165165

166166
/// Looks up a route matching an already normalized [normalizedPath].
167167
///
168-
/// Use this when the caller has built the path from parts and must not have
169-
/// them re-split, such as when a routing key is assembled from a host and a
170-
/// request path.
168+
/// Use when the caller already holds a normalized path (e.g. a routing key
169+
/// assembled from host and path). Walks the trie directly; results are not
170+
/// cached.
171171
LookupResult<T> lookupPath(
172172
final Method method,
173173
final NormalizedPath normalizedPath, {
@@ -182,6 +182,17 @@ final class Router<T extends Object> {
182182
return RouterMatch(route, entry.parameters, entry.matched, entry.remaining);
183183
}
184184

185+
/// Looks up a route for the [Uri] of a request.
186+
///
187+
/// The entry point for request routing: derives the path via
188+
/// [NormalizedPath.fromUri] (splitting before decoding, so an encoded
189+
/// separator cannot alter routing), then looks it up with [lookupPath].
190+
LookupResult<T> lookupUri(
191+
final Method method,
192+
final Uri url, {
193+
final bool backtrack = true,
194+
}) => lookupPath(method, NormalizedPath.fromUri(url), backtrack: backtrack);
195+
185196
/// Returns true if the router has no routes.
186197
bool get isEmpty => _allRoutes.isEmpty;
187198

‎packages/relic_core/test/router/no_cache_test.dart‎

Lines changed: 0 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -27,69 +27,4 @@ void main() {
2727
expect(cache.length, equals(0));
2828
});
2929
});
30-
31-
group('Given NormalizedPath with NoCache', () {
32-
late Cache<String, NormalizedPath> originalCache;
33-
34-
setUp(() {
35-
originalCache = NormalizedPath.interned;
36-
NormalizedPath.interned = const NoCache();
37-
});
38-
39-
tearDown(() {
40-
NormalizedPath.interned = originalCache;
41-
});
42-
43-
test('when creating NormalizedPath '
44-
'then normalization still works correctly', () {
45-
final path = NormalizedPath('/a/b/c');
46-
expect(path.segments, equals(['a', 'b', 'c']));
47-
expect(path.toString(), equals('/a/b/c'));
48-
});
49-
50-
test('when creating equivalent paths '
51-
'then they are equal but not identical', () {
52-
final path1 = NormalizedPath('/a/b');
53-
final path2 = NormalizedPath('/a/b');
54-
expect(path1, equals(path2));
55-
expect(identical(path1, path2), isFalse);
56-
});
57-
58-
test('when normalizing complex paths '
59-
'then normalization is correct', () {
60-
final path = NormalizedPath('/a/./b/../c');
61-
expect(path.segments, equals(['a', 'c']));
62-
expect(path.toString(), equals('/a/c'));
63-
});
64-
});
65-
66-
group('Given NormalizedPath with custom-sized LruCache', () {
67-
late Cache<String, NormalizedPath> originalCache;
68-
69-
setUp(() {
70-
originalCache = NormalizedPath.interned;
71-
NormalizedPath.interned = LruCache<String, NormalizedPath>(2);
72-
});
73-
74-
tearDown(() {
75-
NormalizedPath.interned = originalCache;
76-
});
77-
78-
test('when cache capacity is exceeded '
79-
'then old entries are evicted', () {
80-
final path1 = NormalizedPath('/a');
81-
NormalizedPath('/b'); // fill cache
82-
final path3 = NormalizedPath('/c');
83-
84-
// path1 should have been evicted from the small cache
85-
final path1Again = NormalizedPath('/a');
86-
expect(path1, equals(path1Again));
87-
// With a cache of size 2, after /a, /b, /c, /a is evicted
88-
// so creating /a again produces a new (non-identical) instance
89-
expect(identical(path1, path1Again), isFalse);
90-
91-
// path3 and path2 should still be cached (or path3 and path1Again)
92-
expect(identical(path3, NormalizedPath('/c')), isTrue);
93-
});
94-
});
9530
}

0 commit comments

Comments
 (0)