feat: fix RouterGroup path matching and middleware inheritance #24
Conversation
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
- **Path Matching Improvements**:
- Fix boundary checking to prevent false matches (e.g., /api vs /apiv2)
- Add support for wildcard (*) parameter detection
- Improve dynamic parameter matching for :param, {param}, and *path patterns
- Handle root path and empty prefix edge cases correctly
- **Middleware Inheritance Fixes**:
- Fix nested group middleware collection in ServeHTTP
- Implement getAllMiddlewares() for recursive parent middleware gathering
- Add most specific group matching logic to prevent middleware duplication
- Ensure correct middleware execution order across nested groups
- **Regex Pattern Building**:
- Fix buildRegexPattern() to handle regexp.QuoteMeta() behavior correctly
- Improve dynamic route pattern matching for different parameter formats
- Add proper escaping handling for special characters
- **Path Joining Improvements**:
- Fix joinPath() method with proper slash normalization
- Handle root path special cases (/ + / = /)
- Improve URL path concatenation logic
- **Enhanced Dynamic Path Matching**:
- Add fallback exact matching for wildcard scenarios
- Improve regex matching reliability and error handling
- **Comprehensive Testing**:
- Add 16 comprehensive test cases covering all scenarios
- Include middleware inheritance, nested routing, and conflict resolution tests
- Add performance benchmarks for path matching and middleware collection
- All tests pass with good performance metrics
This fixes critical routing issues in nested RouterGroups and ensures proper
middleware execution order, dynamic parameter matching, and path boundary
checking for production use.
Closes: RouterGroup path matching and middleware inheritance issues
- Replace all Chinese comments with English equivalents in: - group.go: Path matching and middleware logic comments - application.go: ServeHTTP method comments - context.go: HTTP request body handling comments - group_test.go: Test case comments and descriptions - Fix test conflicts and unused imports - Maintain code functionality while improving international readability - All tests continue to pass after translation
…aching - Add pre-sorted groups by prefix length to avoid sorting on every request - Implement middleware chain caching to eliminate redundant collection and deduplication - Remove unnecessary fallback logic since all groups are guaranteed to be cached - Add reflection-based middleware deduplication for safer function comparison - Trigger cache recalculation when groups are created or middlewares are added Performance improvements: - Eliminates O(n²) sorting on every HTTP request - Avoids recursive middleware collection per request - Reduces memory allocations for temporary slices and maps - Provides significant performance boost for applications with complex group hierarchies
Pull Request Test Coverage Report for Build 16095904366Details
💛 - Coveralls |
- sortGroups() in New() was called too early when no user groups exist yet - This caused unnecessary computation and violated the principle of sorting only when the group structure actually changes - sortGroups() is already called appropriately in Group() and Use() methods - This improves initialization performance and follows better design patterns
- Remove inefficient sortGroups() calls from Group() and Use() methods - These were causing expensive sorting and middleware cache recalculation on every group/middleware registration during app setup - Add sortGroups to sync.Once struct for thread-safe single execution - Call sortGroups() in ServeHTTP() using sync.Once pattern - This ensures sorting happens exactly once before first request - Works for both Run() and direct ServeHTTP() usage (e.g., in tests) - Significant performance improvement during app initialization - Maintains all existing functionality and middleware inheritance
- Move sortGroups() call from ServeHTTP back to Run() method - This ensures sorting happens at startup, not on first request - Eliminates any delay impact on first request processing - Add ensureGroupsSorted() helper for test scenarios - Uses sync.Once pattern only for tests that call ServeHTTP directly - Maintains optimal performance for both production and test environments - First request now has zero sorting overhead
- Maintain sorted order during group creation instead of sorting at startup - Use insertion sort to keep groups sorted by prefix length (longest first) - Eliminate startup sorting overhead for better performance - Precompute middleware chains immediately when groups are added - Maintain backward compatibility with existing tests Performance improvements: - No startup sorting delay - Optimal request matching with maintained sort order - Immediate middleware chain computation - Zero impact on first request performance
- Remove redundant sortGroups calls from Run() and ServeHTTP() methods - Groups are now maintained in sorted order during insertion - Fix middleware caching issue where Use() method didn't update cache - Simplify collectGroupMiddlewares to avoid duplicate middleware collection - Remove unused sortedGroups field since groups array is now always sorted - Optimize ServeHTTP to only check middleware cache existence for fallback Performance improvements: - Zero redundant sorting operations at startup and runtime - Middleware cache updates only when needed (on Use() calls) - Cleaner separation of concerns between sorting and middleware management - Maintain optimal first-request performance without redundant operations
- Groups are now maintained in sorted order during insertion - sortGroups() function is no longer needed since groups array is always sorted - Simplified ensureMiddlewareChainsReady() to directly call precomputeMiddlewareChains() - Renamed sync.Once field from sortGroups to precomputeMiddleware for clarity - Reduced code complexity and improved maintainability
- Moved precomputeMiddlewareChains() call from ServeHTTP to Run() for better performance - Removed ensureMiddlewareChainsReady() function as it's no longer needed - Simplified ServeHTTP() by removing runtime ensure checks - Removed precomputeMiddleware sync.Once field as it's no longer needed - All middleware chains are now guaranteed to be ready before serving requests - Eliminates any potential performance overhead on first request
- Remove unnecessary matchedGroups array since we only need the first match - Use early break when first matching group is found - Reduce memory allocations and improve performance - Groups are already sorted by prefix length, so first match is always the most specific - Eliminates redundant array operations and improves request handling efficiency
- Move middleware cache from Application to RouterGroup for better encapsulation - Add middlewareCache and middlewareCacheValid fields to RouterGroup - Implement getMiddlewareCache() method with lazy computation - Add invalidateMiddlewareCache() method for cache invalidation - Automatically invalidate child group caches when parent middleware changes - Remove redundant app-level precomputeMiddlewareChains() method - Move insertGroupSorted() method to RouterGroup for better organization - Add comprehensive test for middleware cache functionality This change improves the architecture by: - Following single responsibility principle (each group manages its own cache) - Better encapsulation and data locality - Cleaner separation of concerns - More intuitive API design
- Remove middleware caching functionality from RouterGroup - Simplify ServeHTTP to use getAllMiddlewares() directly - Remove cache-related fields and methods from RouterGroup - Clean up test code to remove cache-related tests - Maintain insert-sorted group ordering for optimal performance - All tests pass with simpler, more maintainable code
- Remove complex dynamic path matching from RouterGroup - Remove insertGroupSorted and complex sorting logic - Simplify matchPath to use basic prefix matching - Update ServeHTTP to find longest matching prefix - Remove buildRegexPattern and matchDynamicPath methods - Reduce code complexity by ~100 lines - Maintain functionality while improving maintainability Performance: - matchPath: 5.970 ns/op (much faster than regex matching) - getAllMiddlewares: 449.9 ns/op (similar to before) - All tests pass with simplified logic
- Restore simplified dynamic path matching capability
- Support :param, {param}, and *wildcard patterns
- Use string splitting instead of regex for better performance
- Add comprehensive tests for dynamic routing groups
- Verify middleware inheritance works correctly with dynamic groups
- Performance: ~286.6 ns/op for dynamic matching vs ~5.970 ns/op for static
- All existing tests continue to pass
Features:
- Static prefix matching for simple cases (fast path)
- Dynamic pattern matching for :param, {param}, *wildcard
- Proper middleware inheritance in nested dynamic groups
- Longest prefix matching for conflict resolution
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.
Path Matching Improvements:
Middleware Inheritance Fixes:
Regex Pattern Building:
Path Joining Improvements:
Enhanced Dynamic Path Matching:
Comprehensive Testing:
This fixes critical routing issues in nested RouterGroups and ensures proper middleware execution order, dynamic parameter matching, and path boundary checking for production use.
Closes: RouterGroup path matching and middleware inheritance issues