-
Notifications
You must be signed in to change notification settings - Fork 0
Add generic envelope support #108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reviewer's GuideThis PR refactors core types and flows to support custom envelope types by introducing a Packet trait, parameterizing application and middleware layers over an arbitrary packet type, and updating constructors, routing, serialization, and tests accordingly. Sequence diagram for message processing with generic PacketsequenceDiagram
participant Client
participant WireframeApp
participant Middleware
participant HandlerService
participant Handler
Client->>WireframeApp: Send frame (bytes)
WireframeApp->>WireframeApp: Deserialize to E: Packet
WireframeApp->>Middleware: Pass envelope (E)
Middleware->>HandlerService: Pass envelope (E)
HandlerService->>Handler: Call handler with &E
Handler-->>HandlerService: Return response (bytes)
HandlerService->>WireframeApp: Construct response envelope (E::from_parts)
WireframeApp->>Client: Send response frame (bytes)
Class diagram for custom envelope implementationclassDiagram
class MyEnv {
+id: u32
+data: Vec<u8>
+id() u32
+into_parts() (u32, Vec<u8>)
+from_parts(id: u32, data: Vec<u8>) MyEnv
}
Packet <|.. MyEnv
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Warning Rate limit exceeded@leynos has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 2 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 (3)
WalkthroughThe codebase was refactored to allow the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant WireframeApp
participant Middleware
participant HandlerService
participant Handler
Client->>WireframeApp: Send frame (bytes)
WireframeApp->>WireframeApp: Deserialize to E: Packet
WireframeApp->>Middleware: Pass envelope (E)
Middleware->>HandlerService: Pass envelope (E)
HandlerService->>Handler: Call handler with &E
Handler-->>HandlerService: Return response (bytes)
HandlerService->>WireframeApp: Construct response envelope (E::from_parts)
WireframeApp->>Client: Send response frame (bytes)
Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate Unit Tests
🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @leynos - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/middleware.rs:220` </location>
<code_context>
/// Service that invokes a stored route handler and middleware chain.
-pub struct HandlerService {
+pub struct HandlerService<E: Packet> {
id: u32,
svc: Box<dyn Service<Error = Infallible> + Send + Sync>,
+ _marker: std::marker::PhantomData<E>,
}
</code_context>
<issue_to_address>
The addition of PhantomData<E> is necessary for type safety but could be documented for clarity.
Consider adding a comment explaining that PhantomData<E> is used to parameterize HandlerService by E for type safety.
</issue_to_address>
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
pub struct HandlerService<E: Packet> { | ||
id: u32, | ||
svc: Box<dyn Service<Error = Infallible> + Send + Sync>, | ||
_marker: std::marker::PhantomData<E>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: The addition of PhantomData is necessary for type safety but could be documented for clarity.
Consider adding a comment explaining that PhantomData is used to parameterize HandlerService by E for type safety.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
README.md
(1 hunks)src/app.rs
(15 hunks)src/middleware.rs
(3 hunks)tests/middleware_order.rs
(2 hunks)tests/routes.rs
(3 hunks)tests/util.rs
(5 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~91-~91: Since ownership is already implied, this phrasing may be redundant.
Context: ... payload bytes. Applications can supply their own envelope type by calling `WireframeApp:...
(PRP_OWN)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-test
🔇 Additional comments (12)
README.md (1)
88-115
: Well-documented feature addition!The new "Custom Envelopes" section provides clear documentation for the generic envelope support, including a complete example implementation of the
Packet
trait and its usage withWireframeApp
.tests/routes.rs (1)
25-31
: Test coverage for generic envelope support looks good!The
Packet
trait implementation forTestEnvelope
and the corresponding test updates correctly demonstrate the usage of custom envelope types.Also applies to: 41-46, 83-89
tests/middleware_order.rs (1)
34-37
: Middleware test correctly adapted for generic envelopes!The
Transform
implementation and handler type updates properly reflect the genericHandlerService<Envelope>
design.Also applies to: 51-51
src/middleware.rs (1)
217-274
: Excellent implementation of generic envelope support in middleware!The generalization of
HandlerService
andRouteService
overE: Packet
is well-implemented:
- Proper use of
PhantomData
for the unused type parameter- Consistent usage of
Packet
trait methods (from_parts
,into_parts
)- All service trait implementations correctly updated
tests/util.rs (1)
3-7
: Test utilities properly generalized!The addition of generic parameters
<S, C, E>
with appropriate trait bounds ensures the test utilities work correctly with the new genericWireframeApp
design.Also applies to: 34-42, 55-64, 74-82, 95-104
src/app.rs (7)
70-77
: Well-structured generic implementation with sensible defaults.The addition of the generic parameter
E: Packet = Envelope
maintains backward compatibility whilst enabling custom envelope types. The internal fields are correctly updated to use the generic type.
89-89
: Handler type correctly generalised.The
Handler
type alias properly accepts the generic envelope typeE
as a parameter reference.
92-101
: Middleware trait properly generalised with appropriate bounds.The trait definition and blanket implementation correctly constrain the middleware to work with
HandlerService<E>
for the envelope type.
140-154
: Excellent trait design for envelope abstraction.The
Packet
trait provides a minimal and clean interface with appropriate trait bounds. The three methods cover all necessary operations for message routing and handling.
175-182
: Correct Packet implementation for Envelope.The implementation properly delegates to existing
Envelope
methods, ensuring backward compatibility.
225-237
: Constructor for custom envelope types is well-designed.The
new_with_envelope()
method enables users to create aWireframeApp
with custom envelope types whilst maintaining API consistency by returningResult
.
547-566
: Core message handling correctly uses Packet trait methods.The implementation properly deserialises to the generic type
E
, usesinto_parts()
for decomposition, andE::from_parts()
for response construction. The routing logic remains intact.
@sourcery-ai review |
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary
Packet
traitnew_with_envelope
constructorTesting
make lint
make test
https://chatgpt.com/codex/tasks/task_e_6857449705c08322bd5082fc1489fb35
Summary by Sourcery
Enable custom envelope formats by defining a Packet trait and making the application, handlers, middleware, and utilities generic over that trait; provide a new_with_envelope constructor and document the feature with examples.
New Features:
Enhancements:
Documentation:
Tests:
Summary by CodeRabbit