Skip to content

Commit 509cce7

Browse files
authored
🐛 Fix CQRS update version mismatch with hybrid LWW/rehydrate (#11)
* 🐛 Fix CQRS update version mismatch with hybrid LWW/rehydrate ReadModel.version lagged behind EventStore.MAX(version) because update operations use async projection. Passing ReadModel.version() as prev_version caused KernelError::Concurrency on rapid successive updates. Hybrid approach: - Profile.update: LWW (prev_version=None) - patch semantics, no state machine - Account.update / Metadata.update / state transitions: rehydrate aggregate from EventStore for authoritative current_version and domain checks (prevents bypassing suspend/ban and update-after-delete) E2E regression: PUT profile and PUT account twice in rapid succession both return 204 NO_CONTENT (previously second call failed). * 🚨 Fix trailing blank line in e2e test * 🐛 Start rikka-mq workers to make projection appliers consume signals The four applier wrappers constructed RedisMessageQueue but never called MessageQueue::start_workers(), so no consumer task was spawned for any of account_applier, auth_account_applier, profile_applier, or metadata_applier streams. Signals were enqueued to Redis and accumulated as pending entries with consumers=0. This was masked for Account/AuthAccount/Profile because their CommandProcessor.create paths write the ReadModel synchronously (documented asymmetry in AGENTS.md), so create-flow E2E coverage appeared to work. Metadata has no synchronous read-model create, so its projection never landed and update_metadata never executed applier-side, also blocking bug 2 (Updated-after-Deleted) coverage. Add queue.start_workers() inside each applier constructor and extend the E2E flow with a full metadata lifecycle assertion: create -> projection visible -> delete -> projection gone -> update returns 404. The 404 verifies the rehydrate-based OCC fix from the previous commit on this branch (Bug 2).
1 parent bf98a80 commit 509cce7

13 files changed

Lines changed: 306 additions & 49 deletions

File tree

adapter/src/processor/account.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use kernel::interfaces::read_model::{AccountReadModel, DependOnAccountReadModel}
66
use kernel::interfaces::signal::Signal;
77
use kernel::prelude::entity::{
88
Account, AccountId, AccountIsBot, AccountName, AccountPrivateKey, AccountPublicKey,
9-
AuthAccountId, Nanoid,
9+
AuthAccountId, EventVersion, Nanoid,
1010
};
1111
use kernel::KernelError;
1212
use std::future::Future;
@@ -25,7 +25,7 @@ pub struct CreateAccountParam {
2525
pub struct UpdateAccountParam {
2626
pub account_id: AccountId,
2727
pub is_bot: AccountIsBot,
28-
pub current_version: kernel::prelude::entity::EventVersion<Account>,
28+
pub current_version: EventVersion<Account>,
2929
}
3030

3131
// --- Signal DI trait (adapter-specific) ---

adapter/src/processor/profile.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,7 @@ use kernel::interfaces::event_store::{DependOnProfileEventStore, ProfileEventSto
55
use kernel::interfaces::read_model::{DependOnProfileReadModel, ProfileReadModel};
66
use kernel::interfaces::signal::Signal;
77
use kernel::prelude::entity::{
8-
AccountId, EventVersion, FieldAction, ImageId, Nanoid, Profile, ProfileDisplayName, ProfileId,
9-
ProfileSummary,
8+
AccountId, FieldAction, ImageId, Nanoid, Profile, ProfileDisplayName, ProfileId, ProfileSummary,
109
};
1110
use kernel::KernelError;
1211
use std::future::Future;
@@ -37,7 +36,6 @@ pub struct UpdateProfileParam {
3736
pub summary: FieldAction<ProfileSummary>,
3837
pub icon: FieldAction<ImageId>,
3938
pub banner: FieldAction<ImageId>,
40-
pub current_version: EventVersion<Profile>,
4139
}
4240

4341
// --- ProfileCommandProcessor ---
@@ -125,7 +123,6 @@ where
125123
param.summary,
126124
param.icon,
127125
param.banner,
128-
param.current_version,
129126
);
130127

131128
self.profile_event_store()

application/src/service/account.rs

Lines changed: 80 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,15 @@ use adapter::processor::profile::{
1414
use error_stack::Report;
1515
use kernel::interfaces::crypto::{DependOnPasswordProvider, PasswordProvider};
1616
use kernel::interfaces::database::DatabaseConnection;
17+
use kernel::interfaces::event::EventApplier;
18+
use kernel::interfaces::event_store::{AccountEventStore, DependOnAccountEventStore};
1719
use kernel::interfaces::permission::{
1820
AccountRelation, DependOnPermissionChecker, DependOnPermissionWriter, PermissionWriter,
1921
RelationTarget,
2022
};
2123
use kernel::prelude::entity::{
22-
Account, AccountIsBot, AccountName, AccountPrivateKey, AccountPublicKey, AuthAccountId, Nanoid,
23-
Profile, ProfileDisplayName,
24+
Account, AccountId, AccountIsBot, AccountName, AccountPrivateKey, AccountPublicKey,
25+
AuthAccountId, EventId, EventVersion, Nanoid, Profile, ProfileDisplayName,
2426
};
2527
use kernel::KernelError;
2628
use serde_json;
@@ -189,6 +191,7 @@ pub trait UpdateAccountUseCase:
189191
+ Send
190192
+ DependOnAccountCommandProcessor
191193
+ DependOnAccountQueryProcessor
194+
+ DependOnAccountEventStore
192195
+ DependOnPermissionChecker
193196
{
194197
fn update_account(
@@ -200,7 +203,7 @@ pub trait UpdateAccountUseCase:
200203
let mut transaction = self.database_connection().get_executor().await?;
201204

202205
let nanoid = Nanoid::<Account>::new(dto.account_nanoid);
203-
let account = self
206+
let projection = self
204207
.account_query_processor()
205208
.find_by_nanoid_unfiltered(&mut transaction, &nanoid)
206209
.await?
@@ -211,20 +214,29 @@ pub trait UpdateAccountUseCase:
211214
))
212215
})?;
213216

214-
check_permission(self, auth_account_id, &account_edit(account.id())).await?;
217+
check_permission(self, auth_account_id, &account_edit(projection.id())).await?;
218+
219+
let account_id = projection.id().clone();
220+
let (account, current_version) =
221+
rehydrate_account(self, &mut transaction, &account_id).await?;
215222

216223
if !account.status().is_active() {
217224
return Err(Report::new(KernelError::Rejected)
218225
.attach_printable("Cannot modify a suspended or banned account"));
219226
}
227+
if account.deleted_at().is_some() {
228+
return Err(
229+
Report::new(KernelError::Rejected).attach_printable("Account is deactivated")
230+
);
231+
}
220232

221233
self.account_command_processor()
222234
.update(
223235
&mut transaction,
224236
UpdateAccountParam {
225-
account_id: account.id().clone(),
237+
account_id,
226238
is_bot: AccountIsBot::new(dto.is_bot),
227-
current_version: account.version().clone(),
239+
current_version,
228240
},
229241
)
230242
.await?;
@@ -238,6 +250,7 @@ impl<T> UpdateAccountUseCase for T where
238250
T: 'static
239251
+ DependOnAccountCommandProcessor
240252
+ DependOnAccountQueryProcessor
253+
+ DependOnAccountEventStore
241254
+ DependOnPermissionChecker
242255
{
243256
}
@@ -248,6 +261,7 @@ pub trait DeactivateAccountUseCase:
248261
+ Send
249262
+ DependOnAccountCommandProcessor
250263
+ DependOnAccountQueryProcessor
264+
+ DependOnAccountEventStore
251265
+ DependOnPermissionChecker
252266
+ DependOnPermissionWriter
253267
{
@@ -260,7 +274,7 @@ pub trait DeactivateAccountUseCase:
260274
let mut transaction = self.database_connection().get_executor().await?;
261275

262276
let nanoid = Nanoid::<Account>::new(account_id);
263-
let account = self
277+
let projection = self
264278
.account_query_processor()
265279
.find_by_nanoid(&mut transaction, &nanoid)
266280
.await?
@@ -271,10 +285,11 @@ pub trait DeactivateAccountUseCase:
271285
))
272286
})?;
273287

274-
check_permission(self, auth_account_id, &account_deactivate(account.id())).await?;
288+
check_permission(self, auth_account_id, &account_deactivate(projection.id())).await?;
275289

276-
let account_id = account.id().clone();
277-
let current_version = account.version().clone();
290+
let account_id = projection.id().clone();
291+
let (_account, current_version) =
292+
rehydrate_account(self, &mut transaction, &account_id).await?;
278293
self.account_command_processor()
279294
.deactivate(&mut transaction, account_id.clone(), current_version)
280295
.await?;
@@ -304,6 +319,7 @@ impl<T> DeactivateAccountUseCase for T where
304319
T: 'static
305320
+ DependOnAccountCommandProcessor
306321
+ DependOnAccountQueryProcessor
322+
+ DependOnAccountEventStore
307323
+ DependOnPermissionChecker
308324
+ DependOnPermissionWriter
309325
{
@@ -315,6 +331,7 @@ pub trait SuspendAccountUseCase:
315331
+ Send
316332
+ DependOnAccountCommandProcessor
317333
+ DependOnAccountQueryProcessor
334+
+ DependOnAccountEventStore
318335
+ DependOnPermissionChecker
319336
{
320337
fn suspend_account(
@@ -328,7 +345,7 @@ pub trait SuspendAccountUseCase:
328345
let mut transaction = self.database_connection().get_executor().await?;
329346

330347
let nanoid = Nanoid::<Account>::new(account_id);
331-
let account = self
348+
let projection = self
332349
.account_query_processor()
333350
.find_by_nanoid_unfiltered(&mut transaction, &nanoid)
334351
.await?
@@ -348,6 +365,10 @@ pub trait SuspendAccountUseCase:
348365
}
349366
}
350367

368+
let account_id = projection.id().clone();
369+
let (account, current_version) =
370+
rehydrate_account(self, &mut transaction, &account_id).await?;
371+
351372
if !account.status().is_active() {
352373
return Err(
353374
Report::new(KernelError::Rejected).attach_printable("Account is not active")
@@ -359,8 +380,6 @@ pub trait SuspendAccountUseCase:
359380
);
360381
}
361382

362-
let account_id = account.id().clone();
363-
let current_version = account.version().clone();
364383
self.account_command_processor()
365384
.suspend(
366385
&mut transaction,
@@ -380,6 +399,7 @@ impl<T> SuspendAccountUseCase for T where
380399
T: 'static
381400
+ DependOnAccountCommandProcessor
382401
+ DependOnAccountQueryProcessor
402+
+ DependOnAccountEventStore
383403
+ DependOnPermissionChecker
384404
{
385405
}
@@ -390,6 +410,7 @@ pub trait UnsuspendAccountUseCase:
390410
+ Send
391411
+ DependOnAccountCommandProcessor
392412
+ DependOnAccountQueryProcessor
413+
+ DependOnAccountEventStore
393414
+ DependOnPermissionChecker
394415
{
395416
fn unsuspend_account(
@@ -401,7 +422,7 @@ pub trait UnsuspendAccountUseCase:
401422
let mut transaction = self.database_connection().get_executor().await?;
402423

403424
let nanoid = Nanoid::<Account>::new(account_id);
404-
let account = self
425+
let projection = self
405426
.account_query_processor()
406427
.find_by_nanoid_unfiltered(&mut transaction, &nanoid)
407428
.await?
@@ -414,14 +435,16 @@ pub trait UnsuspendAccountUseCase:
414435

415436
check_permission(self, auth_account_id, &instance_moderate()).await?;
416437

438+
let account_id = projection.id().clone();
439+
let (account, current_version) =
440+
rehydrate_account(self, &mut transaction, &account_id).await?;
441+
417442
if !account.status().is_suspended() {
418443
return Err(
419444
Report::new(KernelError::Rejected).attach_printable("Account is not suspended")
420445
);
421446
}
422447

423-
let account_id = account.id().clone();
424-
let current_version = account.version().clone();
425448
self.account_command_processor()
426449
.unsuspend(&mut transaction, account_id, current_version)
427450
.await?;
@@ -435,6 +458,7 @@ impl<T> UnsuspendAccountUseCase for T where
435458
T: 'static
436459
+ DependOnAccountCommandProcessor
437460
+ DependOnAccountQueryProcessor
461+
+ DependOnAccountEventStore
438462
+ DependOnPermissionChecker
439463
{
440464
}
@@ -445,6 +469,7 @@ pub trait BanAccountUseCase:
445469
+ Send
446470
+ DependOnAccountCommandProcessor
447471
+ DependOnAccountQueryProcessor
472+
+ DependOnAccountEventStore
448473
+ DependOnPermissionChecker
449474
{
450475
fn ban_account(
@@ -457,7 +482,7 @@ pub trait BanAccountUseCase:
457482
let mut transaction = self.database_connection().get_executor().await?;
458483

459484
let nanoid = Nanoid::<Account>::new(account_id);
460-
let account = self
485+
let projection = self
461486
.account_query_processor()
462487
.find_by_nanoid_unfiltered(&mut transaction, &nanoid)
463488
.await?
@@ -470,6 +495,10 @@ pub trait BanAccountUseCase:
470495

471496
check_permission(self, auth_account_id, &instance_moderate()).await?;
472497

498+
let account_id = projection.id().clone();
499+
let (account, current_version) =
500+
rehydrate_account(self, &mut transaction, &account_id).await?;
501+
473502
if account.status().is_banned() {
474503
return Err(Report::new(KernelError::Rejected)
475504
.attach_printable("Account is already banned"));
@@ -480,8 +509,6 @@ pub trait BanAccountUseCase:
480509
);
481510
}
482511

483-
let account_id = account.id().clone();
484-
let current_version = account.version().clone();
485512
self.account_command_processor()
486513
.ban(&mut transaction, account_id, reason, current_version)
487514
.await?;
@@ -495,6 +522,40 @@ impl<T> BanAccountUseCase for T where
495522
T: 'static
496523
+ DependOnAccountCommandProcessor
497524
+ DependOnAccountQueryProcessor
525+
+ DependOnAccountEventStore
498526
+ DependOnPermissionChecker
499527
{
500528
}
529+
530+
async fn rehydrate_account<T>(
531+
deps: &T,
532+
executor: &mut <<T as kernel::interfaces::database::DependOnDatabaseConnection>::DatabaseConnection as DatabaseConnection>::Executor,
533+
account_id: &AccountId,
534+
) -> error_stack::Result<(Account, EventVersion<Account>), KernelError>
535+
where
536+
T: DependOnAccountEventStore + ?Sized,
537+
{
538+
let event_id = EventId::from(account_id.clone());
539+
let events = deps
540+
.account_event_store()
541+
.find_by_id(executor, &event_id, None)
542+
.await?;
543+
if events.is_empty() {
544+
return Err(Report::new(KernelError::NotFound).attach_printable(format!(
545+
"No events found for account: {}",
546+
account_id.as_ref()
547+
)));
548+
}
549+
let mut account: Option<Account> = None;
550+
for event in events {
551+
Account::apply(&mut account, event)?;
552+
}
553+
let account = account.ok_or_else(|| {
554+
Report::new(KernelError::NotFound).attach_printable(format!(
555+
"Account aggregate could not be reconstructed for: {}",
556+
account_id.as_ref()
557+
))
558+
})?;
559+
let current_version = account.version().clone();
560+
Ok((account, current_version))
561+
}

0 commit comments

Comments
 (0)