Router methods - #41
Conversation
|
Warning Rate limit exceeded@j03-dev has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 22 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThe updates refactor authentication, password hashing, and logging in the API example by moving all related logic into Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_Server
participant JWT_Middleware
participant Logger
participant Router
participant Handler
Client->>API_Server: HTTP Request (e.g., /me)
API_Server->>Logger: Log method, URI, timestamp
Logger->>JWT_Middleware: (if secured route) Check Authorization header
JWT_Middleware-->>API_Server: Set request.user_id or return UNAUTHORIZED
API_Server->>Router: Route request
Router->>Handler: Call endpoint handler
Handler-->>API_Server: Return response
API_Server-->>Client: HTTP Response
Possibly related PRs
Poem
✨ Finishing Touches
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
src/routing.rs (3)
71-93: Consider the mutability requirements for Decorator.The
Decoratorstruct's__call__method requires&mut self, but the router methods that create decorators (lines 132-138) use&self. This works because theRouteris cloned and its internalroutesfield usesArc<RwLock>for shared mutability. However, this design might be confusing as it's not immediately clear why__call__needs mutable access when it's only modifying shared state.Consider making
__call__take&selfinstead of&mut selffor clarity:- fn __call__(&mut self, handler: Py<PyAny>) -> PyResult<Route> { + fn __call__(&self, handler: Py<PyAny>) -> PyResult<Route> {
97-97: Good thread-safety implementation!The use of
Arc<RwLock<HashMap>>for theroutesfield enables safe concurrent access. This is a solid improvement for multi-threaded environments.Consider monitoring for lock contention under high load scenarios, as write locks on the entire HashMap could become a bottleneck if routes are frequently modified at runtime.
124-129: Use more idiomatic iteration pattern.Consider using the more conventional iteration pattern:
- for ref route in routes { + for route in &routes { self.route(route)?; }examples/api/main.py (1)
117-122: Avoid modifying validated data directly.Instead of modifying
validate_datain place, create a new user instance:- new_user.validate_data.update( - { - "id": str(uuid.uuid4()), - "password": hash_password(new_user.validate_data["password"]), - } - ) + user_data = { + **new_user.validate_data, + "id": str(uuid.uuid4()), + "password": hash_password(new_user.validate_data["password"]), + }Then use
user_datawhen saving the user.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
examples/api/main.py(4 hunks)examples/api/middlewares.py(0 hunks)examples/api/utils.py(0 hunks)examples/htmx/main.py(2 hunks)src/handling/request_handler.rs(3 hunks)src/handling/response_handler.rs(2 hunks)src/lib.rs(3 hunks)src/routing.rs(3 hunks)
💤 Files with no reviewable changes (2)
- examples/api/utils.py
- examples/api/middlewares.py
🧰 Additional context used
🪛 Gitleaks (8.26.0)
examples/api/main.py
22-22: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 Ruff (0.11.9)
examples/api/main.py
86-87: Use a single if statement instead of nested if statements
Combine if statements using and
(SIM102)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: macos (macos-13, x86_64)
- GitHub Check: linux (ubuntu-22.04, aarch64)
- GitHub Check: windows (windows-latest, x86)
- GitHub Check: windows (windows-latest, x64)
- GitHub Check: linux (ubuntu-22.04, s390x)
- GitHub Check: musllinux (ubuntu-22.04, x86)
- GitHub Check: linux (ubuntu-22.04, armv7)
- GitHub Check: linux (ubuntu-22.04, x86)
- GitHub Check: musllinux (ubuntu-22.04, armv7)
- GitHub Check: linux (ubuntu-22.04, ppc64le)
- GitHub Check: musllinux (ubuntu-22.04, x86_64)
- GitHub Check: musllinux (ubuntu-22.04, aarch64)
- GitHub Check: linux (ubuntu-22.04, x86_64)
🔇 Additional comments (12)
src/handling/response_handler.rs (1)
10-10: LGTM! Type alias rename is consistent.The rename from
MatchitRoutetoMatchRouteis consistent with the broader refactoring across the codebase. The change simplifies the type name while maintaining the same functionality.Also applies to: 53-53
examples/htmx/main.py (3)
2-2: Good cleanup of unused imports.Removing the unused
getandpostimports makes sense since the code now uses router-bound decorators instead.
5-6: Excellent refactoring to explicit router instance.Creating an explicit
Routerinstance and registering static files early improves code organization and makes the routing structure more explicit and maintainable.
9-9: Router-bound decorators improve modularity.The change from global decorators (
@get,@post) to router-bound decorators (@router.get,@router.post) is a good architectural improvement that makes routing more explicit and modular.Also applies to: 17-17, 22-22, 34-34
src/lib.rs (4)
49-49: Type alias rename improves clarity.The rename from
MatchitRoutetoMatchRouteprovides a cleaner, more concise type name while maintaining the same functionality.
87-87: Consistent type usage in ProcessRequest.The
MatchRoutetype is correctly used in theProcessRequeststruct, maintaining consistency with the type alias rename.
92-101: Excellent RequestContext consolidation.The new
RequestContextstruct effectively consolidates multiple shared fields into a single context object. This significantly improves code maintainability by:
- Reducing parameter passing complexity
- Providing a clear ownership model with
Arcfor thread-safe sharing- Making the request handling context more explicit
194-202: Clean RequestContext usage in async handling.The
RequestContextis properly created, cloned, and passed through the async task spawning chain. TheArccloning ensures thread-safe access to the shared context data across concurrent request handlers.Also applies to: 209-209, 217-218
src/handling/request_handler.rs (3)
9-9: Good import cleanup.Removing unused imports (
Sender,Router,Cors) and updating the type import (MatchitRoutetoMatchRoute) keeps the imports clean and consistent with the refactoring.Also applies to: 18-18
31-43: Excellent function signature simplification.The refactoring from multiple individual parameters to a single
Arc<RequestContext>parameter significantly improves the function signature's maintainability. The destructuring approach provides clear access to all context fields while maintaining the same functionality.
58-58: Type cast updated consistently.The unsafe transmute now uses the updated
MatchRoutetype, maintaining consistency with the type alias rename. While unsafe code should generally be avoided, this appears to be existing functionality that was just updated for the new type.examples/api/main.py (1)
75-81: Secure password handling implementation!Good use of bcrypt with proper salt generation and constant-time comparison. This follows security best practices for password storage.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: windows (windows-latest, x86)
- GitHub Check: windows (windows-latest, x64)
🔇 Additional comments (5)
README.md (5)
17-17: Import updated to Router-based API
The import line now correctly brings inHttpServer,Router,Status, andResponseinstead of individual route decorators, aligning with the new routing module design.
19-23: Instantiate and bind routes via Router instance
Creating aRouter()instance and using its.get()decorator for the root endpoint ensures thread-safe, decorator-based registration as intended.
25-28: Use parameterized path with router.get
The/hello/{name}route leverages the new placeholder syntax with therouter.getdecorator correctly. This matches the updated API.
45-47: Register middleware on Router instance
Movingrouter.middleware(auth_middleware)before route definitions aligns with the new middleware attachment approach on router instances.
72-79: Instantiate Router for application state example
Initializing a freshRouter()here keeps the examples consistent: each section demonstrates a standalone router usage pattern.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 participant
Summary by CodeRabbit
New Features
Refactor
Chores