chore: bump up cqrs-es (0.5)#10
Conversation
|
Warning Review limit reached
More reviews will be available in 59 minutes and 59 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (13)
WalkthroughThis PR upgrades event-sorcery and sqlite-es from cqrs-es 0.4.12 to 0.5.0. The main library change replaces the event-returning Changescqrs-es 0.5.0 migration: EventSink and TYPE constant
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
0xgleb
force-pushed
the
chore/bump-up-cqrs-es
branch
13 times, most recently
from
9f72eb6 to
7217caf
Compare
0xgleb
force-pushed
the
chore/bump-up-deps
branch
from
6bb147e to
1bca65b
Compare
0xgleb
force-pushed
the
chore/bump-up-cqrs-es
branch
from
7217caf to
efcba38
Compare
graphite-app
Bot
changed the base branch from
chore/bump-up-deps
to
graphite-base/10
0xgleb
force-pushed
the
chore/bump-up-deps
branch
from
1bca65b to
5edb8fc
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/event-sorcery/src/lifecycle.rs (1)
523-552: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAssert sink stays empty when
handlereturns an error.Add empty-event assertions in both error-path tests so no-events-on-error is explicitly enforced.
🤖 Prompt for 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. In `@crates/event-sorcery/src/lifecycle.rs` around lines 523 - 552, In both tests handle_maps_domain_error_to_lifecycle_apply and handle_failed_returns_stored_error, after calling lifecycle.handle(...) and unwrapping the error, add an assertion that the EventSink instance sink contains no emitted events (i.e., assert the sink is empty) to ensure no events are produced on error; locate the sink variable (EventSink::default()) and use its appropriate emptiness check (for example sink.is_empty() or inspecting sink.events.len() == 0) to enforce the no-events-on-error invariant.
🤖 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 `@crates/event-sorcery/src/lifecycle.rs`:
- Around line 493-520: Both tests currently assert emitted events but don't
verify that the Lifecycle value itself is unchanged by handle; clone the initial
lifecycle state before calling Lifecycle::handle (e.g., let original =
lifecycle.clone()) and after the await unwrap assert equality
(assert_eq!(lifecycle, original)) to lock in the scratch-copy contract for both
handle_uninitialized_delegates_to_initialize (Lifecycle::<Counter>::default())
and handle_live_delegates_to_transition (Lifecycle::Live(Counter { value: 0 })).
Ensure Lifecycle implements Clone/PartialEq where needed and use the same
variable names (lifecycle, original) so it's clear which state is being
compared.
- Around line 151-153: The match arm that returns Err(error.clone()) and the
subsequent .map_err(LifecycleError::Apply) silently propagate command/aggregate
errors; update the aggregate's handle(...) implementation to log those error
paths before returning or converting: add a structured log (warn/error) inside
the Self::Failed { error, .. } => branch immediately before return
Err(error.clone()), and also log the error inside the Result.map_err(...)
closure (i.e., before converting to LifecycleError::Apply) so both raw command
failures and mapped domain errors are recorded; reference the handle method, the
Self::Failed pattern, and LifecycleError::Apply when making these changes.
In `@crates/sqlite-es/src/event_repository.rs`:
- Around line 350-356: Reorder the imports into the required three-group layout
(external -> workspace -> crate-internal): move std::fmt and
serde::{Deserialize, Serialize} into the external group first, then place
cqrs_es::DomainEvent and cqrs_es::event_sink::EventSink into the workspace
group, and finally keep crate::testing::create_test_pool and use super::* in the
crate-internal group; also alphabetize imports within each group for consistency
and preserve existing symbols (DomainEvent, EventSink, create_test_pool,
super::*).
---
Outside diff comments:
In `@crates/event-sorcery/src/lifecycle.rs`:
- Around line 523-552: In both tests handle_maps_domain_error_to_lifecycle_apply
and handle_failed_returns_stored_error, after calling lifecycle.handle(...) and
unwrapping the error, add an assertion that the EventSink instance sink contains
no emitted events (i.e., assert the sink is empty) to ensure no events are
produced on error; locate the sink variable (EventSink::default()) and use its
appropriate emptiness check (for example sink.is_empty() or inspecting
sink.events.len() == 0) to enforce the no-events-on-error invariant.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 69eababc-6abf-43ce-9d4d-57d229f8eb5a
⛔ Files ignored due to path filters (3)
Cargo.lockis excluded by!**/*.lockexamples/complex/Cargo.lockis excluded by!**/*.lockexamples/simple/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
Cargo.tomlcrates/event-sorcery/src/lifecycle.rscrates/event-sorcery/src/projection.rscrates/event-sorcery/src/schema_registry.rscrates/event-sorcery/src/sqlite_event_repository.rscrates/event-sorcery/src/testing.rscrates/sqlite-es/src/event_repository.rscrates/sqlite-es/src/view_repository.rsexamples/complex/Cargo.tomlexamples/simple/Cargo.toml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Graphite / mergeability_check
- GitHub Check: examples
- GitHub Check: test
🧰 Additional context used
📓 Path-based instructions (5)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
When handling clippy errors about function lengths or cognitive complexity, don't split up functions more than necessary. Instead ask the user if we can add a clippy allow.
Files:
crates/event-sorcery/src/testing.rscrates/event-sorcery/src/schema_registry.rscrates/event-sorcery/src/projection.rscrates/event-sorcery/src/lifecycle.rscrates/sqlite-es/src/view_repository.rscrates/event-sorcery/src/sqlite_event_repository.rscrates/sqlite-es/src/event_repository.rs
crates/event-sorcery/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
The Lifecycle enum is pub(crate) deliberately—it's an implementation detail of the EventSourced ↔ cqrs-es bridge and must not leak through any public bound.
Files:
crates/event-sorcery/src/testing.rscrates/event-sorcery/src/schema_registry.rscrates/event-sorcery/src/projection.rscrates/event-sorcery/src/lifecycle.rscrates/event-sorcery/src/sqlite_event_repository.rs
crates/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
crates/**/*.rs: Package by Feature, Not by Layer. Organize by business domain, not technical layers. FORBIDDEN catch-all modules: types.rs, error.rs, models.rs, utils.rs, helpers.rs, http.rs, dto.rs, entities.rs, services.rs, domain.rs. CORRECT: lifecycle.rs, projection.rs, reactor.rs, view_backend.rs. Each feature module contains ALL related code.
CRITICAL: NEVER write directly to the events table — no direct INSERTs, no manual sequence numbers, no bypassing CqrsFramework. Always use CqrsFramework::execute() or execute_with_metadata() to emit events through aggregate commands.
CRITICAL: Single CQRS Framework Instance Per Aggregate. In any consuming application, each aggregate must have exactly ONE SqliteCqrs, constructed once at startup. Never call sqlite_cqrs() or CqrsFramework::new() per request. Direct construction is fine in test/CLI/migration code.
Use cqrs-es Services for side-effects in handle() to ensure atomicity with events. Naming: {Action}er trait -> {Domain}Service implements -> {Domain}Manager orchestrates.
Log in command handlers, not callers. All logging for command execution belongs in the aggregate's handle() method. The handler has full aggregate state making log messages rich without the caller needing extra context.
Make invalid states unrepresentable through the type system. Use ADTs and enums to encode business rules and state transitions directly in types rather than runtime validation.
Accept domain newtypes and convert to SDK primitives inside the callee. Exception: cross-crate boundaries where the callee can't depend on caller's domain types — destructure at the call site.
Favor FP/ADT patterns over OOP. Use pattern matching, combinators, type-driven design. Prefer iterators over imperative loops unless it increases complexity. Use itertools for richer iterator chains.
Follow comprehensive commenting guidelines. Comment complex business logic, algorithm rationale, external system behavior, non-obvious constraints, test data context, ...
Files:
crates/event-sorcery/src/testing.rscrates/event-sorcery/src/schema_registry.rscrates/event-sorcery/src/projection.rscrates/event-sorcery/src/lifecycle.rscrates/sqlite-es/src/view_repository.rscrates/event-sorcery/src/sqlite_event_repository.rscrates/sqlite-es/src/event_repository.rs
Cargo.toml
📄 CodeRabbit inference engine (AGENTS.md)
Workspace deps live in [workspace.dependencies]; per-crate Cargo.toml files use .workspace = true (with optional per-crate features added on top).
Files:
Cargo.toml
*
⚙️ CodeRabbit configuration file
Focus on providing constructive criticism. Whenever you see a suboptimal approach, suggest more idiomatic or robust alternative(s). Flag potential footguns. Suggest FP alternatives to mutable/imperative code. Point out architectural flaws like leaky abstractions, tight coupling, wrong level of abstraction, poor type modeling, over-abstraction, unclear domain boundaries. Code should generally be organized based on business concerns rather than technical aspects - suggest improvements if you find violations. Point out gaps in test coverage but suggest tests that are not too coupled to the implementation and actually test domain invariants and business logic
Files:
Cargo.toml
🔇 Additional comments (15)
examples/complex/Cargo.toml (1)
12-12: LGTM!examples/simple/Cargo.toml (1)
13-13: LGTM!crates/event-sorcery/src/sqlite_event_repository.rs (5)
60-60: LGTM!
81-81: LGTM!
99-99: LGTM!
152-152: LGTM!
176-176: LGTM!Also applies to: 189-189, 202-202
crates/event-sorcery/src/projection.rs (3)
226-226: LGTM!Also applies to: 243-243, 258-258
706-737: LGTM!
793-839: LGTM!Cargo.toml (1)
38-38: LGTM!crates/event-sorcery/src/testing.rs (1)
12-12: LGTM!Also applies to: 87-91
crates/event-sorcery/src/schema_registry.rs (1)
266-267: LGTM!Also applies to: 272-288, 308-323, 336-351
crates/sqlite-es/src/event_repository.rs (1)
150-182: LGTM!Also applies to: 237-237, 256-256, 263-263, 270-270, 393-405, 419-419, 446-446, 494-494, 503-503, 512-512
crates/sqlite-es/src/view_repository.rs (1)
200-200: LGTM!Also applies to: 242-255
0xgleb
force-pushed
the
chore/bump-up-deps
branch
from
1bca65b to
bfb4203
Compare
0xgleb
force-pushed
the
chore/bump-up-cqrs-es
branch
from
efcba38 to
99bbb51
Compare
0xgleb
force-pushed
the
chore/bump-up-deps
branch
from
bfb4203 to
c151c42
Compare
0xgleb
force-pushed
the
chore/bump-up-cqrs-es
branch
from
99bbb51 to
6228296
Compare
0xgleb
force-pushed
the
chore/bump-up-cqrs-es
branch
from
6228296 to
7550476
Compare
Merge activity
|
3 participants
Motivation
Track the latest cqrs-es release and keep consumers on a single version. The
public
EventSourcedAPI is preserved; the migration is internal to theLifecycleadapter. Closes RAI-865.Solution
Migrates the workspace from cqrs-es 0.4.12 to 0.5.0. cqrs-es 0.5 reworks the
aggregate command model and drops
async-traitfrom its core traits.Aggregate::handleis now&mut self, writes events through anEventSink,and returns
();fn aggregate_type()becameconst TYPE.Aggregate,PersistedEventRepository, andViewRepositoryare nativeasync fntraits, so#[async_trait]is removed from those impls (it stays onQueryand on ourEventSourced/Reactor).Lifecycle::handleadapts the new signature internally, so consumers' impls ofEventSourcedare untouched.commitby re-applying handled events;Lifecyclerejects a re-applied genesis event, sohandleleavesselfatits pre-command state (driving the sink through a throwaway copy) and events
apply exactly once.
cargo nextest run --workspace(96 pass), clippy, fmt, and bothexample crates on cqrs-es 0.5 + sqlx 0.9. Stacked on chore: bump up deps (sqlx 0.9) #9.