Implement typed microsvc route bundles - #95
Conversation
📝 WalkthroughWalkthroughIntroduces ChangesRoutes<D> bundle model and de-genericized Service
Sequence Diagram(s)sequenceDiagram
participant Client
participant Service
participant RouteIndex
participant RouteBundle
participant Handler
Client->>Service: dispatch message
Service->>RouteIndex: lookup by kind and name
RouteIndex-->>Service: selected bundle
Service->>RouteBundle: invoke message
RouteBundle->>Handler: run guard and handler
Handler-->>RouteBundle: HandlerResult
RouteBundle-->>Service: HandlerResult
Service-->>Client: response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/microsvc/mod.rs`:
- Around line 196-206: The register_handlers! macro is documented as a
backward-compatibility alias but currently just forwards directly to routes! on
both match arms, which breaks legacy service-based calls like
register_handlers!(Service::new(), command handlers::x). You need to modify the
register_handlers! macro to properly handle the legacy pattern where the first
argument is a Service instance followed by a handler type and handlers, and
internally convert that to the appropriate routes! call instead of directly
forwarding all arguments to routes!.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 35142d56-2dcc-4691-87c0-ebaf6039dcce
📒 Files selected for processing (33)
README.mdsrc/bus/handlers.rssrc/bus/router.rssrc/microsvc/grpc.rssrc/microsvc/http.rssrc/microsvc/knative_ingress.rssrc/microsvc/message_router.rssrc/microsvc/mod.rssrc/microsvc/runtime.rssrc/microsvc/service.rssrc/outbox_worker/outbox_source.rstests/distributed_read_model/checkout_saga_service/service.rstests/distributed_read_model/main.rstests/distributed_read_model/projection_service/service.rstests/distributed_read_model/seat_inventory_service/service.rstests/distributed_read_model_board/board_service/service.rstests/distributed_read_model_board/main.rstests/distributed_read_model_board/projections_service/mod.rstests/durable_enqueue_sqlite/main.rstests/kafka_transport/main.rstests/knative_cloudevents/main.rstests/microsvc/basic.rstests/microsvc/convention.rstests/microsvc/session.rstests/microsvc/transport_grpc.rstests/microsvc/transport_http.rstests/microsvc/transport_listen.rstests/microsvc/transport_subscribe.rstests/nats_transport/main.rstests/postgres_transport/main.rstests/rabbitmq_transport/main.rstests/sagas/microsvc_saga.rstests/transport_conformance/mod.rs
Summary
Routes<D>bundles and makeServicea non-generic deployment router that indexes multiple route bundles.routes!macro and remove the legacyregister_handlers!macro; backwards compatibility with service-owned registration is intentionally not retained.Verification
cargo fmt --checkgit diff --checkcargo check --tests --all-featurescargo test service_dispatches_all_route_dependency_builder_combinations --all-featurescargo test microsvc::service --all-featurescargo test --all-features --test microsvc --test sagas --test durable_enqueue_sqlite --test distributed_read_model --test distributed_read_model_boardcargo test microsvc::runtime --all-featurescargo test --all-featuresReview follow-up
r3462873453by removingregister_handlers!entirely, per the decision that backwards compatibility is not required.service_dispatches_all_route_dependency_builder_combinationsafter review discussion about covering all route-builder dependency shapes.Notes
cargo clippy --all-targets --all-features -- -D warningsis currently blocked by pre-existing lints in untouched files:tests/todos/main.rs(await_holding_lock) andsrc/outbox_worker/worker.rs(manual_async_fn).