feat: PathTrie wildcard and tail matching support #70
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 Walkthrough""" WalkthroughThis change introduces support for wildcard ( Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Router
participant PathTrie
participant LookupResult
Client->>Router: lookup(method, path)
Router->>PathTrie: lookup(normalizedPath)
PathTrie->>PathTrie: Traverse segments (literal, parameter, *, **)
alt Tail segment (**)
PathTrie->>LookupResult: Return matched, parameters, remaining
else Wildcard or Parameter
PathTrie->>LookupResult: Return matched, parameters, remaining
end
Router->>Client: Return LookupResult (value, params, matched, remaining)
Assessment against linked issues
Possibly related PRs
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #70 +/- ##
==========================================
+ Coverage 80.75% 81.32% +0.56%
==========================================
Files 79 79
Lines 2370 2426 +56
Branches 1341 1361 +20
==========================================
+ Hits 1914 1973 +59
+ Misses 456 453 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
nielsenko
force-pushed
the
path-trie-wildcard-and-tail
branch
from
3da4a8f to
278b651
Compare
nielsenko
force-pushed
the
router-middleware
branch
3 times, most recently
from
6bc75ec to
ad9760f
Compare
nielsenko
force-pushed
the
path-trie-wildcard-and-tail
branch
from
6d2b796 to
924254f
Compare
nielsenko
force-pushed
the
path-trie-wildcard-and-tail
branch
from
924254f to
d7fe6cc
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
lib/src/router/path_trie.dart (3)
16-20: Doc-comment refers to obsolete “/::” tail syntaxThe comment still mentions
/::and/::name, which were replaced by**tail segments in this PR. This can confuse future readers.- /// This can only happen with path that ends with a tail-match /:: or /::name, + /// This can only happen with a path that ends with a tail segment `/**`,
223-229: Interpolating the generic type literal is not helpful
"$U"will interpolate to the (erased) type name which is the same for every conflicting segment case and does not add actionable information. Re-word the message to explicitly reference the requested segment kind instead.- 'New: "$U"'); + 'New: $U'); // or hard-code the literal `"wildcard" / "tail" / "parameter"`
265-272: Double-throw makes the code harder to read
raiseInvalidSegmentalready throws, so wrapping it into an additionalthrowis redundant and slightly obscures intent.- throw normalizedPath.raiseInvalidSegment( + normalizedPath.raiseInvalidSegment( i, 'Conflicting parameter names at the same level: ' 'Existing: ":${parameter.name}", ' 'New: ":$paramName"', );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
lib/src/router/normalized_path.dart(4 hunks)lib/src/router/path_trie.dart(9 hunks)lib/src/router/router.dart(2 hunks)pubspec.yaml(1 hunks)test/router/path_trie_crud_test.dart(2 hunks)test/router/path_trie_tail_test.dart(1 hunks)test/router/path_trie_wildcard_test.dart(1 hunks)
🔇 Additional comments (17)
pubspec.yaml (1)
14-14: Appropriate dependency addition.Moving the
metapackage from dev_dependencies to dependencies is necessary to support the@immutableannotation used in theNormalizedPathclass. The version constraint^1.16.0is appropriate.test/router/path_trie_crud_test.dart (8)
179-196: Great test coverage for wildcard path updates.This test thoroughly verifies that updating an existing wildcard path works correctly, with proper assertions checking the value, parameters, matched path, and remaining segments.
198-205: Good error case testing for wildcard paths.This test appropriately verifies that attempting to update a non-existent wildcard path throws an ArgumentError and doesn't inadvertently create the path.
207-223: Comprehensive tail path update test.This test thoroughly validates updating tail path values and verifies the matched/remaining path segments are correctly populated. The assertions for the remaining path segments are particularly valuable.
225-232: Good error handling test for tail paths.This test ensures that attempts to update non-existent tail paths fail appropriately with ArgumentError.
359-375: Well-structured wildcard path removal test.This test properly verifies that wildcard paths can be removed while sibling paths remain unaffected, validating both the removed value and subsequent lookup behavior.
377-389: Good negative test for wildcard path removal.This test confirms that attempting to remove a non-existent wildcard path returns null and doesn't affect other paths.
391-411: Thorough tail path removal test.This test effectively verifies tail path removal behavior while checking that more specific child paths remain accessible, which is important for maintaining route hierarchy.
413-423: Appropriate negative test for tail path removal.This test ensures that attempting to remove a non-existent tail path safely returns null without side effects on other paths.
lib/src/router/router.dart (2)
99-106: Enhanced lookup result for static routes.Good enhancement to the
lookupmethod to return a more detailedLookupResultfor static cache hits. Using the full normalized path as the matched path andNormalizedPath.emptyfor the remaining path makes sense for static routes.
120-125: Updated trie lookup result handling.The lookup result now properly includes the matched and remaining path information from the trie's lookup, providing consistent behavior between static and dynamic route resolutions.
lib/src/router/normalized_path.dart (4)
1-2: Appropriate package import.Adding the import for
package:meta/meta.dartsupports the@immutableannotation used below.
14-14: Good use of immutability annotation.The
@immutableannotation explicitly indicates that this class should not be mutated after construction, which helps prevent bugs and makes the code's intent clearer.
26-28: Efficient empty path singleton.Creating a static constant for an empty path is a good optimization that avoids unnecessary object creation when empty paths are needed.
69-76: Optimized subpath extraction.The improvements to the
subPathmethod are excellent:
- Handling optional end parameter
- Returning the empty path singleton for zero-length subpaths
- Returning the original instance for full-path subpaths
These optimizations reduce object creation and improve performance for router operations.
test/router/path_trie_wildcard_test.dart (1)
13-24: Great set of positive-path assertionsThe test verifies value, parameters, matched & remaining paths – exactly the fields most likely to regress. Nice work!
test/router/path_trie_tail_test.dart (1)
79-99: Solid precedence coverage for/vs/**These cases often trip implementations up; covering them here will prevent subtle routing bugs. 👍
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/src/router/path_trie.dart (1)
56-60: Update class documentation to reflect new capabilities.The class documentation for
PathTrieonly mentions "literal segments and parameterized segments" but now also supports wildcard and tail segments./// A Trie (prefix tree) data structure optimized for matching URL paths. /// -/// Supports literal segments and parameterized segments (e.g., `:id`). Allows +/// Supports literal segments, parameterized segments (e.g., `:id`), wildcard segments (`*`), +/// and tail segments (`**`). Allows /// associating a value of type [T] with each complete path.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/src/router/path_trie.dart(9 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Run Unit Tests (macos-latest, beta, upgrade)
- GitHub Check: Run Unit Tests (macos-latest, 3.3.0, upgrade)
- GitHub Check: Run Unit Tests (macos-latest, 3.3.0, downgrade)
🔇 Additional comments (16)
lib/src/router/path_trie.dart (16)
3-4: Good addition of a clear type alias.The
Parameterstype alias enhances code readability and makes the contract more explicit.
11-20: Well-documented new fields for enhancing route lookup capabilities.The addition of
matchedandremainingfields with clear documentation improves the API by providing clients with information about what part of the path was matched and what remains unmatched after a tail segment.
23-23: Constructor properly updated to support new fields.The
LookupResultconstructor has been correctly updated to include the newmatchedandremainingfields.
32-32: Refactored to support multiple dynamic segment types.Generalizing from
_Parameter<T>?to_DynamicSegment<T>?enables the trie to handle various types of dynamic segments.
40-54: Well-designed sealed class hierarchy for dynamic segments.Using a sealed class hierarchy for dynamic segments is an excellent choice, providing:
- Type safety through compile-time exhaustiveness checking
- Clear representation of different segment types
- Polymorphic behavior while maintaining strong typing
The implementation is clean and minimal.
174-182: Smart helper function for type-safe node resolution.The
nextIfhelper function elegantly handles type checking and node retrieval in a concise, reusable way. The@pragma('vm:prefer-inline')annotation is a good performance optimization.
186-200: Clear prioritization of segment matching types.The segment matching logic maintains a clear priority order (literal segments first, then dynamic segments) while properly handling the new wildcard and tail segment types.
213-224: Effective validation helper for segment type conflicts.The
isAhelper function provides a clean way to validate segment type consistency while generating precise error messages.
226-240: Robust validation for tail segments.The implementation properly enforces that:
- Tail segments must be exactly
**(no variations)- Tail segments must be the last segment in the path
- No conflicting segment types are allowed at the same level
These constraints help prevent ambiguous routing configurations.
241-248: Proper handling of wildcard segments.The wildcard segment handling ensures wildcards are exactly
*(no variations) and prevents conflicting segment types at the same level.
249-270: Refactored parameter segment handling.The parameter segment handling has been updated to work with the new class hierarchy while maintaining the existing validation for empty parameter names and conflicting parameter names.
303-305: Updated conflict check in attach.The conflict check in
attachhas been properly updated to use the generalizeddynamicSegment.
315-315: Updated field assignment in attach.The field assignment in
attachhas been correctly updated to use the generalizeddynamicSegment.
332-347: Enhanced lookup with pattern matching and early termination.The lookup method now:
- Uses Dart's pattern matching for cleaner parameter handling
- Supports early termination on tail matches
- Properly tracks the segment index for constructing matched and remaining paths
This enables the new wildcard and tail matching capabilities.
350-367: Smart handling of tail segments at path end.The additional logic for handling tail segments at the end of a path is important for cases like
/archive/**matching/archive(where the remaining path would be empty).
371-376: Excellent extension method for precise error messages.The
raiseInvalidSegmentextension method onNormalizedPathprovides more informative error messages by including the specific segment that caused the error.
nielsenko
force-pushed
the
path-trie-wildcard-and-tail
branch
from
08161e9 to
87420ac
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
tp
left a comment
tp
left a comment
There was a problem hiding this comment.
👏 Nice work!
Just a few questions for my understanding really 😀
Co-authored-by: Timm Preetz <52437+tp@users.noreply.github.com>
nielsenko
force-pushed
the
path-trie-wildcard-and-tail
branch
from
da468b6 to
e253ee5
Compare
List.unmodifiable is implemented in the runtime. I doubt it actually wraps the list object passed. I would expect a bigger cost on the benchmarks if it did.
|
Resolved all comments to merge |
Description
This change introduces support for wildcard (
*) and tail (**) path segments in thePathTriefor more flexible route matching, along with related improvements toNormalizedPathandRouter.Key Changes:
PathTrieEnhancements:_Wildcard<T>and_Tail<T>, alongside the existing_Parameter<T>.LookupResultnow includesmatched(the part of the path that matched up to and including dynamic segments) andremaining(the part of the path consumed by a**tail match)._buildmethod now handles the creation of nodes for*and**segments. It includes validation to ensure:*and**are standalone segments (e.g.,*fooor/**barare invalid).**(tail segment) must be the last segment in a path definition.lookupmethod is updated to:*and**segments.matchedpath, andremainingpath in theLookupResult./archive/**matching lookup path/archive).NormalizedPathImprovements:@immutableannotation.NormalizedPath.emptystatic instance for an empty path.subPathmethod to returnNormalizedPath.emptyfor zero-length subpaths orthisif the subpath covers the entire original path, improving efficiency by leveraging immutability.RouterUpdate:lookupmethod inRouterhas been updated to correctly constructLookupResultwith the newmatchedandremainingpath fields from thePathTrie's lookup result.Testing:
path_trie_tail_test.dart,path_trie_wildcard_test.dart)Related Issues
Benchmark results
2,4 GHz 8-Core Intel Core i9
Pre-Launch Checklist
Please ensure that your PR meets the following requirements before submitting:
///), ensuring consistency with existing project documentation.Breaking Changes
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores