Repository navigation
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
bdfb7a2 to
915875f
Compare
|
@coderabbitai full review |
✅ Action performedFull review finished. |
|
@rain-marvin review |
|
🔎 Reviewing |
There was a problem hiding this comment.
This PR adds a default method, Reactor::react_committed(event, committed), that gives a reactor the stored sequence of each committed event. With the sequence, a consumer such as st0x.liquidity can build a stable event id <aggregate type>:<aggregate id>:<sequence> for log deduplication. The default forwards to react, so existing reactors keep working. ReactorBridge::dispatch calls the new method. Arc<R> and RetryOnBusy<R> forward it, and ReactorHarness::receive_committed lets tests reach an override.
Overall read: correct and additive. The only path that sends committed events to reactors is the cqrs-es query dispatch after commit, which goes through ReactorBridge. Loading an entity, snapshot rebuild, projection catch-up and projection rebuild never reach a reactor, so the new method runs once per commit and never on replay. envelope.sequence is the value that goes into the events table, and numbering continues correctly after snapshots and compaction. Committed is Copy, so a RetryOnBusy retry gets the same sequence. The crate has no other forwarding wrapper that could drop an override. The tests cover the bridge, the full store commit, the restart and replay paths, and the wrappers. The docs and SPEC.md describe the wrapper-forwarding caveat. One minor API-shape note below. No blockers.
claude-opus-5-5 · high · 13 min
|
@JuaniRios 🔔 Follow-up: a day after my review @JuaniRios, #64 needs a human approval and nothing else. CodeRabbit and I both approved Next step: get one of @agryaznov, @ueco-jb, @rouzwelt or @findolor to approve it. After it merges, tag a release so st0x.liquidity#1696 can drop its git-rev pin. The follow-up to tighten the claude-opus-5-5 · high · 1 h 9 min |

Part of RAI-3105.
Reactors can now read the committed event's sequence, through a new default method
Reactor::react_committed. Nothing changes for existing reactors.Why
st0x.liquidity wants structured log lines (
liq_trade,liq_transfer,liq_event) with a stableevent_id, so the log pipeline can drop duplicates. The natural id is<aggregate type>:<aggregate id>:<sequence>.ReactorBridge::dispatchalready gets each committedEventEnvelopewith itssequence, but it only passes(id, event)to the reactor. So a reactor can't build that id today. Writing the lines fromevolve()isn't an option either, cause it also runs on every replay.API
ReactorBridge::dispatchnow callsreact_committedwith each envelope's sequence. The default forwards toreact, so a reactor that doesn't override it behaves exactly as before.Committedis#[non_exhaustive](build it withCommitted::new), so we can add more commit facts later without a breaking change.Arc<R>andRetryOnBusy<R>forwardreact_committed.RetryOnBusyretries it with the samecommitted. TheIdempotentReactorcontract now covers areact_committedoverride too.ReactorHarness::receive_committed(id, event, sequence)to test an override.receivestill callsreact.react_committedonly runs on commit dispatch. Loading an entity, rebuilding snapshots, and projection catch-up or rebuild never reach a reactor, same as before.send_command()still bypasses reactors.I picked a default method over a new trait or an adapter type: it's the smallest change, it needs no new wiring in
StoreBuilder, and a reactor opts in by overriding one method.Compatibility
impl Reactorblocks compile unchanged. I checked st0x.liquidity against this branch with no code changes:cargo check --workspace --all-targets --all-featuresis clean.Reactorby forwardingreactto an inner reactor must also forwardreact_committed. If not, the default calls the innerreactand the inner override never runs. Nothing breaks for wrappers of reactors that don't override. It's in the rustdoc anddocs/cqrs.md.Tests
dispatch_hands_each_envelope_sequence_to_react_committed: a multi-event commit through the bridge gives each event its own sequence.store_hands_each_commit_to_react_committed_with_its_stored_sequence: through a realStoreBuilderand SQLite, the reactor gets the same sequences as theeventstable, per aggregate.restart_replay_and_hydration_do_not_reach_react_committed: 11 events (past the snapshot size), snapshot deleted, then a new store over the same DB. Building it rebuilds the snapshot, thenrebuild_all,catch_up, andloadrun. The reactor gets nothing. The next command arrives with sequence 12.default_react_committed_forwards_to_react,arc_forwards_react_committed_to_the_override,retry_on_busy_retries_react_committed_with_the_same_sequence,retry_on_busy_react_still_calls_inner_react, andreactor_harness_receive_committed_reaches_the_override.IdempotentReactorcontract did not mentionreact_committed, and a misplaced heading indocs/domain.md). All fixed. Pass 2 was clean on every lane.Release
This needs a release before st0x.liquidity can drop its temporary git-rev pin (ST0x-Technology/st0x.liquidity#1696). I did not tag one.