GKaszewski/movies-diary
1
0
Fork 0
Code Issues 9 Pull Requests Actions Packages Projects Releases Wiki Activity

fix: NewRemoteReview param struct, atomic review+orphan movie delete

Browse Source
This commit is contained in:
Gabriel Kaszewski
2026-10-26 01:19:09 +01:00
parent 9a4d0c1714
commit 7f9c252d35
18 changed files with 261 additions and 116 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
crates/adapters/activitypub/src/lib.rs
View File

@@ -22,7 +22,7 @@ pub use k_ap::{
pub use event_handler::ActivityPubEventHandler;
pub use federation_ports::ApServiceAdapter;
pub use remote_review_repository::{RemoteReviewRepository, RemoteReviewUpdate};
pub use remote_review_repository::{NewRemoteReview, RemoteReviewRepository, RemoteReviewUpdate};
pub use review_handler::ReviewObjectHandler;
pub use social_adapter::CompositeSocialAdapter;
pub use user_adapter::DomainUserRepoAdapter;

23
crates/adapters/activitypub/src/remote_review_repository.rs
View File

@@ -14,19 +14,20 @@ pub struct RemoteReviewUpdate<'a> {
pub spoilers: bool,
}
/// A remote review to persist, with the events that ride the same write.
pub struct NewRemoteReview<'a> {
pub review: &'a Review,
pub ap_id: &'a str,
pub movie_title: &'a str,
pub release_year: u16,
pub external_metadata_id: Option<&'a str>,
pub poster_url: Option<&'a str>,
pub events: &'a [DomainEvent],
}
#[async_trait]
pub trait RemoteReviewRepository: Send + Sync {
#[allow(clippy::too_many_arguments)]
async fn save_remote_review(
&self,
review: &Review,
ap_id: &str,
movie_title: &str,
release_year: u16,
external_metadata_id: Option<&str>,
poster_url: Option<&str>,
events: &[DomainEvent],
) -> Result<()>;
async fn save_remote_review(&self, new: NewRemoteReview<'_>) -> Result<()>;
async fn delete_remote_review(&self, ap_id: &str, actor_url: &str) -> Result<()>;

20
crates/adapters/activitypub/src/review_handler.rs
View File

@@ -13,7 +13,7 @@ use k_ap::{AS_PUBLIC, ApContentReader, ApObjectHandler, LocalObject};
use url::Url;
use crate::objects::{ReviewApInput, ReviewObject, review_to_ap_object};
use crate::remote_review_repository::RemoteReviewRepository;
use crate::remote_review_repository::{NewRemoteReview, RemoteReviewRepository};
use crate::urls::{actor_url, review_url};
pub struct ReviewObjectHandler {
@@ -174,15 +174,15 @@ impl ApObjectHandler for ReviewObjectHandler {
.collect();
self.review_store
.save_remote_review(
&review,
obj.id.as_str(),
&obj.movie_title,
obj.release_year,
obj.external_metadata_id.as_deref(),
obj.poster_url.as_deref(),
&events,
)
.save_remote_review(NewRemoteReview {
review: &review,
ap_id: obj.id.as_str(),
movie_title: &obj.movie_title,
release_year: obj.release_year,
external_metadata_id: obj.external_metadata_id.as_deref(),
poster_url: obj.poster_url.as_deref(),
events: &events,
})
.await?;
Ok(())

28
crates/adapters/sqlite-federation/src/review.rs
View File

@@ -1,27 +1,23 @@
use activitypub::{RemoteReviewRepository, RemoteReviewUpdate};
use activitypub::{NewRemoteReview, RemoteReviewRepository, RemoteReviewUpdate};
use anyhow::{Result, anyhow};
use async_trait::async_trait;
use domain::{
events::DomainEvent,
models::{Review, ReviewSource},
};
use domain::models::ReviewSource;
use super::SqliteFederationRepository;
use sqlite::persistence::{datetime_to_str, enqueue_events};
#[async_trait]
impl RemoteReviewRepository for SqliteFederationRepository {
#[allow(clippy::too_many_arguments)]
async fn save_remote_review(
&self,
review: &Review,
ap_id: &str,
movie_title: &str,
release_year: u16,
external_metadata_id: Option<&str>,
poster_url: Option<&str>,
events: &[DomainEvent],
) -> Result<()> {
async fn save_remote_review(&self, new: NewRemoteReview<'_>) -> Result<()> {
let NewRemoteReview {
review,
ap_id,
movie_title,
release_year,
external_metadata_id,
poster_url,
events,
} = new;
let actor_url = match review.source() {
ReviewSource::Remote { actor_url } => actor_url.clone(),
ReviewSource::Local => {

16
crates/adapters/sqlite-federation/tests/federation/remote_review_test.rs
View File

@@ -1,6 +1,6 @@
//! A remote review and the enrichment request it triggers commit together.
use activitypub::RemoteReviewRepository;
use activitypub::{NewRemoteReview, RemoteReviewRepository};
use domain::{
events::DomainEvent,
models::{PersistedReview, Review, ReviewSource},
@@ -56,15 +56,15 @@ fn enrichment_requested(movie_id: &MovieId) -> DomainEvent {
async fn save(pool: &SqlitePool, review: &Review, events: &[DomainEvent]) -> anyhow::Result<()> {
SqliteFederationRepository::new(pool.clone())
.save_remote_review(
.save_remote_review(NewRemoteReview {
review,
AP_ID,
"The Matrix",
1999,
Some("tmdb:603"),
None,
ap_id: AP_ID,
movie_title: "The Matrix",
release_year: 1999,
external_metadata_id: Some("tmdb:603"),
poster_url: None,
events,
)
})
.await
}

23
crates/adapters/sqlite/src/repositories/review/mod.rs
View File

@@ -4,7 +4,7 @@ use domain::{
events::DomainEvent,
models::{Review, ReviewSource},
ports::ReviewRepository,
value_objects::{ReviewId, UserId},
value_objects::{MovieId, ReviewId, UserId},
};
use sqlx::SqlitePool;
@@ -114,12 +114,15 @@ impl ReviewRepository for SqliteReviewRepository {
tx.commit().await.map_err(crate::shared::map_sqlx_error)
}
async fn delete_review(
async fn delete_review_and_orphaned_movie(
&self,
review_id: &ReviewId,
events: &[DomainEvent],
movie_id: &MovieId,
review_events: &[DomainEvent],
orphan_events: &[DomainEvent],
) -> Result<(), DomainError> {
let id = review_id.value().to_string();
let movie = movie_id.value().to_string();
let mut tx = self
.pool
.begin()
@@ -130,7 +133,19 @@ impl ReviewRepository for SqliteReviewRepository {
.execute(&mut *tx)
.await
.map_err(crate::shared::map_sqlx_error)?;
enqueue_events(&mut tx, events).await?;
enqueue_events(&mut tx, review_events).await?;
let orphaned = sqlx::query(
"DELETE FROM movies WHERE id = ? \
AND NOT EXISTS (SELECT 1 FROM reviews WHERE movie_id = ?)",
)
.bind(&movie)
.bind(&movie)
.execute(&mut *tx)
.await
.map_err(crate::shared::map_sqlx_error)?;
if orphaned.rows_affected() > 0 {
enqueue_events(&mut tx, orphan_events).await?;
}
tx.commit().await.map_err(crate::shared::map_sqlx_error)
}

75
crates/adapters/sqlite/tests/repositories/review_repository_test.rs
View File

@@ -135,36 +135,85 @@ async fn a_failing_enqueue_after_the_review_update_rolls_the_update_back() {
assert_eq!(count(&pool, "event_queue").await, 0);
}
fn deleted(review: &domain::models::Review) -> DomainEvent {
DomainEvent::ReviewDeleted {
review_id: review.id().clone(),
user_id: review.user_id().clone(),
}
}
fn movie_deleted(review: &domain::models::Review) -> DomainEvent {
DomainEvent::MovieDeleted {
movie_id: review.movie_id().clone(),
poster_path: None,
}
}
#[tokio::test]
async fn deleting_a_review_writes_the_removal_and_its_event_in_one_commit() {
async fn deleting_the_last_review_removes_the_movie_and_enqueues_both_events_in_one_commit() {
let (pool, review) = seeded().await;
let repo = SqliteReviewRepository::new(pool.clone());
repo.save_review(&review, &[]).await.unwrap();
let event = DomainEvent::ReviewDeleted {
review_id: review.id().clone(),
user_id: review.user_id().clone(),
};
repo.delete_review(review.id(), &[event]).await.unwrap();
repo.delete_review_and_orphaned_movie(
review.id(),
review.movie_id(),
&[deleted(&review)],
&[movie_deleted(&review)],
)
.await
.unwrap();
assert_eq!(count(&pool, "reviews").await, 0);
assert_eq!(count(&pool, "movies").await, 0);
assert_eq!(count(&pool, "event_queue").await, 2);
}
#[tokio::test]
async fn deleting_a_review_while_another_remains_keeps_the_movie_and_enqueues_only_review_deleted()
{
let (pool, review) = seeded().await;
let repo = SqliteReviewRepository::new(pool.clone());
repo.save_review(&review, &[]).await.unwrap();
let other = test_review(review.movie_id(), review.user_id(), 3);
repo.save_review(&other, &[]).await.unwrap();
repo.delete_review_and_orphaned_movie(
review.id(),
review.movie_id(),
&[deleted(&review)],
&[movie_deleted(&review)],
)
.await
.unwrap();
assert_eq!(count(&pool, "reviews").await, 1);
assert_eq!(count(&pool, "movies").await, 1);
assert_eq!(count(&pool, "event_queue").await, 1);
}
#[tokio::test]
async fn a_failing_enqueue_after_the_review_delete_rolls_the_delete_back() {
async fn a_failing_enqueue_rolls_back_the_review_and_movie_delete() {
let (pool, review) = seeded().await;
let repo = SqliteReviewRepository::new(pool.clone());
repo.save_review(&review, &[]).await.unwrap();
fail_event_inserts(&pool).await;
let event = DomainEvent::ReviewDeleted {
review_id: review.id().clone(),
user_id: review.user_id().clone(),
};
let result = repo.delete_review(review.id(), &[event]).await;
let result = repo
.delete_review_and_orphaned_movie(
review.id(),
review.movie_id(),
&[deleted(&review)],
&[movie_deleted(&review)],
)
.await;
assert!(result.is_err());
assert_eq!(count(&pool, "reviews").await, 1, "delete rolled back");
assert_eq!(
count(&pool, "reviews").await,
1,
"review delete rolled back"
);
assert_eq!(count(&pool, "movies").await, 1, "movie delete rolled back");
assert_eq!(count(&pool, "event_queue").await, 0);
}

29
crates/application/src/diary/delete_review.rs
View File

@@ -20,29 +20,28 @@ pub async fn execute(deps: &DeleteReviewDeps, cmd: DeleteReviewCommand) -> Resul
}
let movie_id = review.movie_id().clone();
// Read before the write: the orphan event carries the poster to clean up.
let poster_path = deps
.diary
.get_review_history(&movie_id)
.await?
.movie()
.poster_path()
.cloned();
// One write: the review, plus its movie when this was the last review.
deps.review
.delete_review(
.delete_review_and_orphaned_movie(
&review_id,
&movie_id,
&[DomainEvent::ReviewDeleted {
review_id: review_id.clone(),
user_id: requesting_user_id.clone(),
user_id: requesting_user_id,
}],
)
.await?;
let history = deps.diary.get_review_history(&movie_id).await?;
if history.viewings().is_empty() {
let poster_path = history.movie().poster_path().cloned();
deps.movie_command
.delete_movie(
&movie_id,
&[DomainEvent::MovieDeleted {
movie_id: movie_id.clone(),
poster_path,
}],
)
.await?;
}
Ok(())
.await
}

3
crates/application/src/diary/deps.rs
View File

@@ -1,7 +1,7 @@
use std::sync::Arc;
use domain::ports::{
DiaryExporter, DiaryQuery, FollowGraphQuery, MovieCommand, MovieProfileRepository, MovieQuery,
DiaryExporter, DiaryQuery, FollowGraphQuery, MovieProfileRepository, MovieQuery,
ReviewRepository, UserRepository,
};
@@ -10,7 +10,6 @@ use crate::config::AppSettings;
pub struct DeleteReviewDeps {
pub review: Arc<dyn ReviewRepository>,
pub diary: Arc<dyn DiaryQuery>,
pub movie_command: Arc<dyn MovieCommand>,
}
pub struct EditReviewDeps {

29
crates/application/tests/diary/delete_review_test.rs
View File

@@ -3,7 +3,7 @@ use std::sync::Arc;
use domain::{
ports::{MovieCommand, MovieQuery, ReviewRepository},
testing::{FakeDiaryQuery, InMemoryStore, RefusingMovieCommand},
testing::{FakeDiaryQuery, InMemoryStore},
value_objects::{MovieId, UserId},
};
@@ -28,7 +28,6 @@ async fn deleting_a_review_removes_it() {
let deps = DeleteReviewDeps {
review: Arc::clone(&reviews) as _,
diary: diary.clone() as _,
movie_command: Arc::clone(&movies) as _,
};
delete_review::execute(
@@ -52,7 +51,6 @@ async fn deleting_a_review_removes_it() {
async fn deleting_another_users_review_is_unauthorized() {
let reviews = InMemoryStore::new();
let diary = FakeDiaryQuery::new();
let movies = Arc::clone(&reviews);
let movie_id = MovieId::from_uuid(uuid::Uuid::new_v4());
let owner_id = UserId::from_uuid(uuid::Uuid::new_v4());
@@ -64,7 +62,6 @@ async fn deleting_another_users_review_is_unauthorized() {
let deps = DeleteReviewDeps {
review: Arc::clone(&reviews) as _,
diary: diary as _,
movie_command: movies as _,
};
let result = delete_review::execute(
@@ -89,11 +86,12 @@ async fn deleting_a_review_removes_it_and_records_its_event_together() {
let review = test_review(movie.id(), &user_id, 4);
movies.upsert_movie(&movie, &[]).await.unwrap();
movies.save_review(&review, &[]).await.unwrap();
let other = test_review(movie.id(), &UserId::from_uuid(uuid::Uuid::new_v4()), 3);
movies.save_review(&other, &[]).await.unwrap();
diary.seed_history(movie.clone(), vec![review.clone()]);
let deps = DeleteReviewDeps {
review: Arc::clone(&movies) as _,
diary: diary.clone() as _,
movie_command: Arc::clone(&movies) as _,
};
delete_review::execute(
@@ -106,7 +104,7 @@ async fn deleting_a_review_removes_it_and_records_its_event_together() {
.await
.unwrap();
assert_eq!(movies.review_count(), 0);
assert_eq!(movies.review_count(), 1);
let recorded = movies.recorded_events();
assert_eq!(recorded.len(), 1);
assert!(matches!(
@@ -122,12 +120,12 @@ async fn a_refused_delete_propagates_and_records_nothing() {
let user_id = UserId::from_uuid(uuid::Uuid::new_v4());
let review = test_review(&MovieId::generate(), &user_id, 4);
movies.save_review(&review, &[]).await.unwrap();
diary.seed_history(test_movie_with_year("Terminator", 1984), vec![]);
let deps = DeleteReviewDeps {
review: Arc::new(crate::edit_review_test::WriteRefusingReviews(Arc::clone(
&movies,
))),
diary: diary as _,
movie_command: Arc::clone(&movies) as _,
};
let result = delete_review::execute(
@@ -157,7 +155,6 @@ async fn deleting_the_last_review_records_movie_deleted_with_the_movie_delete()
let deps = DeleteReviewDeps {
review: Arc::clone(&movies) as _,
diary: diary as _,
movie_command: Arc::clone(&movies) as _,
};
delete_review::execute(
@@ -198,7 +195,6 @@ async fn deleting_a_review_while_viewings_remain_keeps_the_movie_and_records_no_
let deps = DeleteReviewDeps {
review: Arc::clone(&movies) as _,
diary: diary as _,
movie_command: Arc::clone(&movies) as _,
};
delete_review::execute(
@@ -221,7 +217,7 @@ async fn deleting_a_review_while_viewings_remain_keeps_the_movie_and_records_no_
}
#[tokio::test]
async fn a_refused_movie_delete_propagates_and_records_no_movie_deleted() {
async fn a_refused_review_and_movie_delete_leaves_review_and_movie_and_records_nothing() {
let movies = InMemoryStore::new();
let diary = FakeDiaryQuery::new();
let movie = test_movie_with_year("Terminator", 1984);
@@ -231,9 +227,10 @@ async fn a_refused_movie_delete_propagates_and_records_no_movie_deleted() {
movies.save_review(&review, &[]).await.unwrap();
diary.seed_history(movie.clone(), vec![]);
let deps = DeleteReviewDeps {
review: Arc::clone(&movies) as _,
review: Arc::new(crate::edit_review_test::WriteRefusingReviews(Arc::clone(
&movies,
))),
diary: diary as _,
movie_command: Arc::new(RefusingMovieCommand),
};
let result = delete_review::execute(
@@ -246,11 +243,7 @@ async fn a_refused_movie_delete_propagates_and_records_no_movie_deleted() {
.await;
assert!(result.is_err());
assert_eq!(movies.review_count(), 1);
assert!(movies.get_movie_by_id(movie.id()).await.unwrap().is_some());
assert!(
!movies
.recorded_events()
.iter()
.any(|e| matches!(e, domain::events::DomainEvent::MovieDeleted { .. }))
);
assert!(movies.recorded_events().is_empty());
}

4
crates/application/tests/diary/edit_review_test.rs
View File

@@ -107,9 +107,11 @@ impl ReviewRepository for WriteRefusingReviews {
"refused".into(),
))
}
async fn delete_review(
async fn delete_review_and_orphaned_movie(
&self,
_: &ReviewId,
_: &domain::value_objects::MovieId,
_: &[domain::events::DomainEvent],
_: &[domain::events::DomainEvent],
) -> Result<(), domain::errors::DomainError> {
Err(domain::errors::DomainError::InfrastructureError(

1
crates/bootstrap/src/build.rs
View File

@@ -89,7 +89,6 @@ pub fn build_deps(
delete_review: DeleteReviewDeps {
review: repos.review.clone(),
diary: repos.diary.clone(),
movie_command: repos.movie_command.clone(),
},
edit_review: EditReviewDeps {
review: repos.review.clone(),

9
crates/domain/src/ports/diary.rs
View File

@@ -51,10 +51,15 @@ pub trait ReviewRepository: Send + Sync {
review: &Review,
events: &[DomainEvent],
) -> Result<(), DomainError>;
async fn delete_review(
/// Deletes the review and, in the same transaction, its movie when no
/// review of any kind references it any more. `review_events` are always
/// recorded; `orphan_events` only when the movie row was deleted.
async fn delete_review_and_orphaned_movie(
&self,
review_id: &ReviewId,
events: &[DomainEvent],
movie_id: &MovieId,
review_events: &[DomainEvent],
orphan_events: &[DomainEvent],
) -> Result<(), DomainError>;
async fn get_all_reviews_for_user(&self, user_id: &UserId) -> Result<Vec<Review>, DomainError>;
}

8
crates/domain/src/testing/fakes/panics.rs
View File

@@ -557,7 +557,13 @@ impl ReviewRepository for PanicReviewRepository {
async fn update_review(&self, _: &Review, _: &[DomainEvent]) -> Result<(), DomainError> {
panic!("PanicReviewRepository called")
}
async fn delete_review(&self, _: &ReviewId, _: &[DomainEvent]) -> Result<(), DomainError> {
async fn delete_review_and_orphaned_movie(
&self,
_: &ReviewId,
_: &MovieId,
_: &[DomainEvent],
_: &[DomainEvent],
) -> Result<(), DomainError> {
panic!("PanicReviewRepository called")
}
async fn get_all_reviews_for_user(&self, _: &UserId) -> Result<Vec<Review>, DomainError> {

8
crates/domain/src/testing/fakes/refusing.rs
View File

@@ -37,7 +37,13 @@ impl ReviewRepository for RefusingReviewRepository {
async fn update_review(&self, _: &Review, _: &[DomainEvent]) -> Result<(), DomainError> {
Err(refused())
}
async fn delete_review(&self, _: &ReviewId, _: &[DomainEvent]) -> Result<(), DomainError> {
async fn delete_review_and_orphaned_movie(
&self,
_: &ReviewId,
_: &MovieId,
_: &[DomainEvent],
_: &[DomainEvent],
) -> Result<(), DomainError> {
Err(refused())
}
async fn get_all_reviews_for_user(&self, _: &UserId) -> Result<Vec<Review>, DomainError> {

12
crates/domain/src/testing/fakes/store_movie.rs
View File

@@ -60,6 +60,15 @@ impl MovieCommand for InMemoryStore {
"Database operation failed".into(),
));
}
self.remove_movie_with_cascades(id);
self.record_events(events);
Ok(())
}
}
impl InMemoryStore {
/// Removes the movie row and applies the schema's cascades.
pub(super) fn remove_movie_with_cascades(&self, id: uuid::Uuid) {
self.movies.write().unwrap().remove(&id);
self.watchlist
.write()
@@ -72,9 +81,6 @@ impl MovieCommand for InMemoryStore {
*event = detached_from_movie(event);
}
}
drop(events_guard);
self.record_events(events);
Ok(())
}
}

21
crates/domain/src/testing/fakes/store_review.rs
View File

@@ -5,7 +5,7 @@ use crate::{
events::DomainEvent,
models::Review,
ports::ReviewRepository,
value_objects::{ReviewId, UserId},
value_objects::{MovieId, ReviewId, UserId},
};
use super::InMemoryStore;
@@ -47,13 +47,24 @@ impl ReviewRepository for InMemoryStore {
Ok(())
}
async fn delete_review(
async fn delete_review_and_orphaned_movie(
&self,
review_id: &ReviewId,
events: &[DomainEvent],
movie_id: &MovieId,
review_events: &[DomainEvent],
orphan_events: &[DomainEvent],
) -> Result<(), DomainError> {
self.reviews.write().unwrap().remove(&review_id.value());
self.record_events(events);
let movie = movie_id.value();
let still_referenced = {
let mut reviews = self.reviews.write().unwrap();
reviews.remove(&review_id.value());
reviews.values().any(|r| r.movie_id().value() == movie)
};
self.record_events(review_events);
if !still_referenced && self.movies.read().unwrap().contains_key(&movie) {
self.remove_movie_with_cascades(movie);
self.record_events(orphan_events);
}
Ok(())
}

58
crates/domain/tests/testing/in_memory_store_test.rs
View File

@@ -517,3 +517,61 @@ async fn saving_a_watch_event_records_its_events() {
assert_eq!(store.rows_in("watch_events"), 1);
assert_eq!(store.recorded_events().len(), 1);
}
#[tokio::test]
async fn deleting_the_last_review_removes_the_movie_and_records_both_events() {
let store = InMemoryStore::new();
let (movie, user_id) = seed_movie_with_dependents(&store, true).await;
let review = store
.get_all_reviews_for_user(&user_id)
.await
.unwrap()
.remove(0);
store
.delete_review_and_orphaned_movie(
review.id(),
movie.id(),
&[domain::events::DomainEvent::ReviewDeleted {
review_id: review.id().clone(),
user_id: user_id.clone(),
}],
&[movie_deleted(&movie)],
)
.await
.unwrap();
assert_eq!(store.review_count(), 0);
assert_eq!(store.movie_count(), 0);
assert_eq!(store.recorded_events().len(), 2);
}
#[tokio::test]
async fn deleting_a_review_while_another_remains_keeps_the_movie_and_records_one_event() {
let store = InMemoryStore::new();
let (movie, user_id) = seed_movie_with_dependents(&store, true).await;
let review = store
.get_all_reviews_for_user(&user_id)
.await
.unwrap()
.remove(0);
let other = test_review(movie.id(), &UserId::generate(), 3);
store.save_review(&other, &[]).await.unwrap();
store
.delete_review_and_orphaned_movie(
review.id(),
movie.id(),
&[domain::events::DomainEvent::ReviewDeleted {
review_id: review.id().clone(),
user_id: user_id.clone(),
}],
&[movie_deleted(&movie)],
)
.await
.unwrap();
assert_eq!(store.review_count(), 1);
assert_eq!(store.movie_count(), 1);
assert_eq!(store.recorded_events().len(), 1);
}