diff --git a/Cargo.lock b/Cargo.lock index 740c722..d51a349 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -36,8 +36,10 @@ checksum = "7f202df86484c868dbad7eaa557ef785d5c66295e41b460ef922eca0723b842c" name = "application" version = "0.1.0" dependencies = [ + "async-trait", "chrono", "domain", + "tokio", "tracing", "uuid", ] diff --git a/crates/adapters/sqlite/src/lib.rs b/crates/adapters/sqlite/src/lib.rs index 5a67a6d..41a1489 100644 --- a/crates/adapters/sqlite/src/lib.rs +++ b/crates/adapters/sqlite/src/lib.rs @@ -52,17 +52,14 @@ impl SqliteMovieRepository { } } - async fn fetch_diary_rows( + async fn fetch_all_diary_rows( &self, - movie_id: Option<&str>, sort: &SortDirection, limit: i64, offset: i64, ) -> Result, DomainError> { - // sqlx macros require literal ORDER BY values; separate branches also let the - // query planner use the movie_id index instead of falling back to a filtered scan. - match (movie_id, sort) { - (None, SortDirection::Descending) => sqlx::query_as!( + match sort { + SortDirection::Descending => sqlx::query_as!( DiaryRow, "SELECT m.id, m.external_metadata_id, m.title, m.release_year, m.director, m.poster_path, r.id AS review_id, r.movie_id, r.user_id, r.rating, r.comment, r.watched_at, r.created_at @@ -77,7 +74,7 @@ impl SqliteMovieRepository { .await .map_err(Self::map_err), - (None, SortDirection::Ascending) => sqlx::query_as!( + SortDirection::Ascending => sqlx::query_as!( DiaryRow, "SELECT m.id, m.external_metadata_id, m.title, m.release_year, m.director, m.poster_path, r.id AS review_id, r.movie_id, r.user_id, r.rating, r.comment, r.watched_at, r.created_at @@ -91,8 +88,18 @@ impl SqliteMovieRepository { .fetch_all(&self.pool) .await .map_err(Self::map_err), + } + } - (Some(id), SortDirection::Descending) => sqlx::query_as!( + async fn fetch_movie_diary_rows( + &self, + movie_id: &str, + sort: &SortDirection, + limit: i64, + offset: i64, + ) -> Result, DomainError> { + match sort { + SortDirection::Descending => sqlx::query_as!( DiaryRow, "SELECT m.id, m.external_metadata_id, m.title, m.release_year, m.director, m.poster_path, r.id AS review_id, r.movie_id, r.user_id, r.rating, r.comment, r.watched_at, r.created_at @@ -101,7 +108,7 @@ impl SqliteMovieRepository { WHERE r.movie_id = ? ORDER BY r.watched_at DESC LIMIT ? OFFSET ?", - id, + movie_id, limit, offset ) @@ -109,7 +116,7 @@ impl SqliteMovieRepository { .await .map_err(Self::map_err), - (Some(id), SortDirection::Ascending) => sqlx::query_as!( + SortDirection::Ascending => sqlx::query_as!( DiaryRow, "SELECT m.id, m.external_metadata_id, m.title, m.release_year, m.director, m.poster_path, r.id AS review_id, r.movie_id, r.user_id, r.rating, r.comment, r.watched_at, r.created_at @@ -118,7 +125,7 @@ impl SqliteMovieRepository { WHERE r.movie_id = ? ORDER BY r.watched_at ASC LIMIT ? OFFSET ?", - id, + movie_id, limit, offset ) @@ -251,14 +258,22 @@ impl MovieRepository for SqliteMovieRepository { } async fn query_diary(&self, filter: &DiaryFilter) -> Result, DomainError> { - let movie_id: Option = filter.movie_id.as_ref().map(|id| id.value().to_string()); let limit = filter.page.limit as i64; let offset = filter.page.offset as i64; - let (total, rows) = tokio::try_join!( - self.count_diary_entries(movie_id.as_deref()), - self.fetch_diary_rows(movie_id.as_deref(), &filter.sort_by, limit, offset) - )?; + let (total, rows) = match &filter.movie_id { + None => tokio::try_join!( + self.count_diary_entries(None), + self.fetch_all_diary_rows(&filter.sort_by, limit, offset) + )?, + Some(id) => { + let id_str = id.value().to_string(); + tokio::try_join!( + self.count_diary_entries(Some(id_str.as_str())), + self.fetch_movie_diary_rows(&id_str, &filter.sort_by, limit, offset) + )? + } + }; let items = rows .into_iter() diff --git a/crates/application/Cargo.toml b/crates/application/Cargo.toml index 90a2d62..28ed626 100644 --- a/crates/application/Cargo.toml +++ b/crates/application/Cargo.toml @@ -4,7 +4,11 @@ version = "0.1.0" edition = "2024" [dependencies] +async-trait = { workspace = true } domain = { workspace = true } uuid = { workspace = true } chrono = { workspace = true } tracing = { workspace = true } + +[dev-dependencies] +tokio = { workspace = true } diff --git a/crates/application/src/lib.rs b/crates/application/src/lib.rs index 50e5f19..9aed4a0 100644 --- a/crates/application/src/lib.rs +++ b/crates/application/src/lib.rs @@ -1,6 +1,7 @@ pub mod commands; pub mod config; pub mod context; +pub mod movie_resolver; pub mod ports; pub mod queries; pub mod use_cases; diff --git a/crates/application/src/movie_resolver.rs b/crates/application/src/movie_resolver.rs new file mode 100644 index 0000000..0391c05 --- /dev/null +++ b/crates/application/src/movie_resolver.rs @@ -0,0 +1,583 @@ +use async_trait::async_trait; +use domain::{ + errors::DomainError, + models::Movie, + ports::{MetadataClient, MetadataSearchCriteria, MovieRepository}, + value_objects::{ExternalMetadataId, MovieTitle, ReleaseYear}, +}; + +use crate::commands::LogReviewCommand; + +pub struct MovieResolverDeps<'a> { + pub repository: &'a dyn MovieRepository, + pub metadata_client: &'a dyn MetadataClient, +} + +#[async_trait] +pub trait ResolutionStrategy: Send + Sync { + fn can_handle(&self, cmd: &LogReviewCommand) -> bool; + async fn resolve( + &self, + cmd: &LogReviewCommand, + deps: &MovieResolverDeps<'_>, + ) -> Result, DomainError>; +} + +pub struct ExternalIdStrategy; +pub struct TitleSearchStrategy; +pub struct ManualMovieStrategy; + +pub struct MovieResolver { + strategies: Vec>, +} + +impl MovieResolver { + pub fn default_pipeline() -> Self { + Self { + strategies: vec![ + Box::new(ExternalIdStrategy), + Box::new(TitleSearchStrategy), + Box::new(ManualMovieStrategy), + ], + } + } + + pub async fn resolve( + &self, + cmd: &LogReviewCommand, + deps: &MovieResolverDeps<'_>, + ) -> Result<(Movie, bool), DomainError> { + for strategy in &self.strategies { + if strategy.can_handle(cmd) { + if let Some(result) = strategy.resolve(cmd, deps).await? { + return Ok(result); + } + } + } + Err(DomainError::ValidationError( + "Manual title required if TMDB fetch fails or is omitted".into(), + )) + } +} + +#[async_trait] +impl ResolutionStrategy for ExternalIdStrategy { + fn can_handle(&self, cmd: &LogReviewCommand) -> bool { + cmd.external_metadata_id.is_some() + } + + async fn resolve( + &self, + cmd: &LogReviewCommand, + deps: &MovieResolverDeps<'_>, + ) -> Result, DomainError> { + let ext_id_str = cmd.external_metadata_id.as_deref().unwrap(); + let tmdb_id = ExternalMetadataId::new(ext_id_str.to_string())?; + + if let Some(m) = deps.repository.get_movie_by_external_id(&tmdb_id).await? { + return Ok(Some((m, false))); + } + + match deps + .metadata_client + .fetch_movie_metadata(&MetadataSearchCriteria::ImdbId(tmdb_id)) + .await + { + Ok(m) => Ok(Some((m, true))), + Err(e) => { + tracing::warn!( + "Failed to fetch from TMDB, falling back to manual entry: {:?}", + e + ); + Ok(None) + } + } + } +} + +#[async_trait] +impl ResolutionStrategy for TitleSearchStrategy { + fn can_handle(&self, cmd: &LogReviewCommand) -> bool { + cmd.manual_title.is_some() + } + + async fn resolve( + &self, + cmd: &LogReviewCommand, + deps: &MovieResolverDeps<'_>, + ) -> Result, DomainError> { + let title = cmd.manual_title.as_deref().unwrap(); + let criteria = MetadataSearchCriteria::Title { + title: title.to_string(), + year: cmd.manual_release_year, + }; + match deps.metadata_client.fetch_movie_metadata(&criteria).await { + Ok(m) => Ok(Some((m, true))), + Err(e) => { + tracing::warn!("OMDb title search failed, falling back to manual: {:?}", e); + Ok(None) + } + } + } +} + +#[async_trait] +impl ResolutionStrategy for ManualMovieStrategy { + fn can_handle(&self, cmd: &LogReviewCommand) -> bool { + cmd.manual_title.is_some() + } + + async fn resolve( + &self, + cmd: &LogReviewCommand, + deps: &MovieResolverDeps<'_>, + ) -> Result, DomainError> { + let title_str = match &cmd.manual_title { + Some(t) => t, + None => return Ok(None), + }; + let year_val = cmd.manual_release_year.ok_or_else(|| { + DomainError::ValidationError( + "Manual release year required if TMDB fetch fails or is omitted".into(), + ) + })?; + + let title = MovieTitle::new(title_str.clone())?; + let release_year = ReleaseYear::new(year_val)?; + + let candidates = deps + .repository + .get_movies_by_title_and_year(&title, &release_year) + .await?; + + let matched = candidates + .into_iter() + .find(|m| m.is_manual_match(&title, &release_year, cmd.manual_director.as_deref())); + + if let Some(existing) = matched { + Ok(Some((existing, false))) + } else { + let new_movie = + Movie::new(None, title, release_year, cmd.manual_director.clone(), None); + Ok(Some((new_movie, true))) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use chrono::NaiveDate; + use domain::{ + errors::DomainError, + events::DomainEvent, + models::{DiaryEntry, DiaryFilter, Movie, Review, ReviewHistory, collections::Paginated}, + ports::{MetadataSearchCriteria, MovieRepository}, + value_objects::{ + ExternalMetadataId, MovieId, MovieTitle, PosterUrl, ReleaseYear, ReviewId, + }, + }; + + fn make_cmd(ext_id: Option<&str>, title: Option<&str>, year: Option) -> LogReviewCommand { + LogReviewCommand { + external_metadata_id: ext_id.map(String::from), + manual_title: title.map(String::from), + manual_release_year: year, + manual_director: None, + user_id: uuid::Uuid::new_v4(), + rating: 4, + comment: None, + watched_at: NaiveDate::from_ymd_opt(2024, 1, 1) + .unwrap() + .and_hms_opt(0, 0, 0) + .unwrap(), + } + } + + fn make_movie() -> Movie { + Movie::new( + None, + MovieTitle::new("Inception".to_string()).unwrap(), + ReleaseYear::new(2010).unwrap(), + None, + None, + ) + } + + struct RepoWithExternalMovie(Movie); + struct RepoEmpty; + struct RepoWithTitleMatch(Movie); + + #[async_trait] + impl MovieRepository for RepoWithExternalMovie { + async fn get_movie_by_external_id( + &self, + _: &ExternalMetadataId, + ) -> Result, DomainError> { + Ok(Some(self.0.clone())) + } + async fn get_movie_by_id(&self, _: &MovieId) -> Result, DomainError> { + panic!("unexpected") + } + async fn get_movies_by_title_and_year( + &self, + _: &MovieTitle, + _: &ReleaseYear, + ) -> Result, DomainError> { + panic!("unexpected") + } + async fn upsert_movie(&self, _: &Movie) -> Result<(), DomainError> { + panic!("unexpected") + } + async fn save_review(&self, _: &Review) -> Result { + panic!("unexpected") + } + async fn query_diary( + &self, + _: &DiaryFilter, + ) -> Result, DomainError> { + panic!("unexpected") + } + async fn get_review_history(&self, _: &MovieId) -> Result { + panic!("unexpected") + } + async fn get_review_by_id( + &self, + _: &ReviewId, + ) -> Result, DomainError> { + panic!("unexpected") + } + async fn delete_review(&self, _: &ReviewId) -> Result<(), DomainError> { + panic!("unexpected") + } + async fn delete_movie(&self, _: &MovieId) -> Result<(), DomainError> { + panic!("unexpected") + } + } + + #[async_trait] + impl MovieRepository for RepoEmpty { + async fn get_movie_by_external_id( + &self, + _: &ExternalMetadataId, + ) -> Result, DomainError> { + Ok(None) + } + async fn get_movie_by_id(&self, _: &MovieId) -> Result, DomainError> { + panic!("unexpected") + } + async fn get_movies_by_title_and_year( + &self, + _: &MovieTitle, + _: &ReleaseYear, + ) -> Result, DomainError> { + Ok(vec![]) + } + async fn upsert_movie(&self, _: &Movie) -> Result<(), DomainError> { + panic!("unexpected") + } + async fn save_review(&self, _: &Review) -> Result { + panic!("unexpected") + } + async fn query_diary( + &self, + _: &DiaryFilter, + ) -> Result, DomainError> { + panic!("unexpected") + } + async fn get_review_history(&self, _: &MovieId) -> Result { + panic!("unexpected") + } + async fn get_review_by_id( + &self, + _: &ReviewId, + ) -> Result, DomainError> { + panic!("unexpected") + } + async fn delete_review(&self, _: &ReviewId) -> Result<(), DomainError> { + panic!("unexpected") + } + async fn delete_movie(&self, _: &MovieId) -> Result<(), DomainError> { + panic!("unexpected") + } + } + + #[async_trait] + impl MovieRepository for RepoWithTitleMatch { + async fn get_movie_by_external_id( + &self, + _: &ExternalMetadataId, + ) -> Result, DomainError> { + panic!("unexpected") + } + async fn get_movie_by_id(&self, _: &MovieId) -> Result, DomainError> { + panic!("unexpected") + } + async fn get_movies_by_title_and_year( + &self, + _: &MovieTitle, + _: &ReleaseYear, + ) -> Result, DomainError> { + Ok(vec![self.0.clone()]) + } + async fn upsert_movie(&self, _: &Movie) -> Result<(), DomainError> { + panic!("unexpected") + } + async fn save_review(&self, _: &Review) -> Result { + panic!("unexpected") + } + async fn query_diary( + &self, + _: &DiaryFilter, + ) -> Result, DomainError> { + panic!("unexpected") + } + async fn get_review_history(&self, _: &MovieId) -> Result { + panic!("unexpected") + } + async fn get_review_by_id( + &self, + _: &ReviewId, + ) -> Result, DomainError> { + panic!("unexpected") + } + async fn delete_review(&self, _: &ReviewId) -> Result<(), DomainError> { + panic!("unexpected") + } + async fn delete_movie(&self, _: &MovieId) -> Result<(), DomainError> { + panic!("unexpected") + } + } + + struct MetaReturnsMovie(Movie); + struct MetaErrors; + + #[async_trait] + impl MetadataClient for MetaReturnsMovie { + async fn fetch_movie_metadata( + &self, + _: &MetadataSearchCriteria, + ) -> Result { + Ok(self.0.clone()) + } + async fn get_poster_url( + &self, + _: &ExternalMetadataId, + ) -> Result, DomainError> { + panic!("unexpected") + } + } + + #[async_trait] + impl MetadataClient for MetaErrors { + async fn fetch_movie_metadata( + &self, + _: &MetadataSearchCriteria, + ) -> Result { + Err(DomainError::InfrastructureError("metadata unavailable".into())) + } + async fn get_poster_url( + &self, + _: &ExternalMetadataId, + ) -> Result, DomainError> { + panic!("unexpected") + } + } + + // --- ExternalIdStrategy --- + + #[test] + fn external_id_strategy_can_handle_cmd_with_id() { + let cmd = make_cmd(Some("tt123"), None, None); + assert!(ExternalIdStrategy.can_handle(&cmd)); + } + + #[test] + fn external_id_strategy_cannot_handle_cmd_without_id() { + let cmd = make_cmd(None, Some("Inception"), Some(2010)); + assert!(!ExternalIdStrategy.can_handle(&cmd)); + } + + #[tokio::test] + async fn external_id_strategy_returns_cached_movie() { + let movie = make_movie(); + let repo = RepoWithExternalMovie(movie.clone()); + let meta = MetaErrors; + let deps = MovieResolverDeps { + repository: &repo, + metadata_client: &meta, + }; + let cmd = make_cmd(Some("tt123"), None, None); + let result = ExternalIdStrategy.resolve(&cmd, &deps).await.unwrap(); + assert!(matches!(result, Some((_, false)))); + } + + #[tokio::test] + async fn external_id_strategy_fetches_from_metadata_when_not_cached() { + let movie = make_movie(); + let repo = RepoEmpty; + let meta = MetaReturnsMovie(movie); + let deps = MovieResolverDeps { + repository: &repo, + metadata_client: &meta, + }; + let cmd = make_cmd(Some("tt123"), None, None); + let result = ExternalIdStrategy.resolve(&cmd, &deps).await.unwrap(); + assert!(matches!(result, Some((_, true)))); + } + + #[tokio::test] + async fn external_id_strategy_falls_through_on_metadata_error() { + let repo = RepoEmpty; + let meta = MetaErrors; + let deps = MovieResolverDeps { + repository: &repo, + metadata_client: &meta, + }; + let cmd = make_cmd(Some("tt123"), None, None); + let result = ExternalIdStrategy.resolve(&cmd, &deps).await.unwrap(); + assert!(result.is_none()); + } + + // --- TitleSearchStrategy --- + + #[test] + fn title_strategy_can_handle_cmd_with_title() { + let cmd = make_cmd(None, Some("Inception"), Some(2010)); + assert!(TitleSearchStrategy.can_handle(&cmd)); + } + + #[test] + fn title_strategy_cannot_handle_cmd_without_title() { + let cmd = make_cmd(Some("tt123"), None, None); + assert!(!TitleSearchStrategy.can_handle(&cmd)); + } + + #[tokio::test] + async fn title_strategy_fetches_from_metadata() { + let movie = make_movie(); + let repo = RepoEmpty; + let meta = MetaReturnsMovie(movie); + let deps = MovieResolverDeps { + repository: &repo, + metadata_client: &meta, + }; + let cmd = make_cmd(None, Some("Inception"), Some(2010)); + let result = TitleSearchStrategy.resolve(&cmd, &deps).await.unwrap(); + assert!(matches!(result, Some((_, true)))); + } + + #[tokio::test] + async fn title_strategy_falls_through_on_metadata_error() { + let repo = RepoEmpty; + let meta = MetaErrors; + let deps = MovieResolverDeps { + repository: &repo, + metadata_client: &meta, + }; + let cmd = make_cmd(None, Some("Inception"), Some(2010)); + let result = TitleSearchStrategy.resolve(&cmd, &deps).await.unwrap(); + assert!(result.is_none()); + } + + // --- ManualMovieStrategy --- + + #[test] + fn manual_strategy_can_handle_cmd_with_title() { + let cmd = make_cmd(None, Some("Inception"), Some(2010)); + assert!(ManualMovieStrategy.can_handle(&cmd)); + } + + #[test] + fn manual_strategy_cannot_handle_cmd_without_title() { + let cmd = make_cmd(Some("tt123"), None, None); + assert!(!ManualMovieStrategy.can_handle(&cmd)); + } + + #[tokio::test] + async fn manual_strategy_returns_existing_movie() { + let movie = make_movie(); + let repo = RepoWithTitleMatch(movie.clone()); + let meta = MetaErrors; + let deps = MovieResolverDeps { + repository: &repo, + metadata_client: &meta, + }; + let cmd = make_cmd(None, Some("Inception"), Some(2010)); + let result = ManualMovieStrategy.resolve(&cmd, &deps).await.unwrap(); + assert!(matches!(result, Some((_, false)))); + } + + #[tokio::test] + async fn manual_strategy_creates_new_movie_when_no_match() { + let repo = RepoEmpty; + let meta = MetaErrors; + let deps = MovieResolverDeps { + repository: &repo, + metadata_client: &meta, + }; + let cmd = make_cmd(None, Some("Inception"), Some(2010)); + let result = ManualMovieStrategy.resolve(&cmd, &deps).await.unwrap(); + assert!(matches!(result, Some((_, true)))); + } + + #[tokio::test] + async fn manual_strategy_errors_without_year() { + let repo = RepoEmpty; + let meta = MetaErrors; + let deps = MovieResolverDeps { + repository: &repo, + metadata_client: &meta, + }; + let cmd = make_cmd(None, Some("Inception"), None); + assert!(ManualMovieStrategy.resolve(&cmd, &deps).await.is_err()); + } + + // --- MovieResolver pipeline --- + + #[tokio::test] + async fn resolver_returns_error_when_no_strategy_matches() { + let repo = RepoEmpty; + let meta = MetaErrors; + let deps = MovieResolverDeps { + repository: &repo, + metadata_client: &meta, + }; + let cmd = make_cmd(None, None, None); + let result = MovieResolver::default_pipeline().resolve(&cmd, &deps).await; + assert!(result.is_err()); + } + + #[tokio::test] + async fn resolver_uses_cached_movie_when_external_id_matches() { + let movie = make_movie(); + let repo = RepoWithExternalMovie(movie.clone()); + let meta = MetaErrors; + let deps = MovieResolverDeps { + repository: &repo, + metadata_client: &meta, + }; + let cmd = make_cmd(Some("tt123"), None, None); + let (_, is_new) = MovieResolver::default_pipeline() + .resolve(&cmd, &deps) + .await + .unwrap(); + assert!(!is_new); + } + + #[tokio::test] + async fn resolver_falls_through_to_manual_when_external_and_title_both_fail() { + let repo = RepoEmpty; + let meta = MetaErrors; + let deps = MovieResolverDeps { + repository: &repo, + metadata_client: &meta, + }; + let cmd = make_cmd(Some("tt123"), Some("Inception"), Some(2010)); + let (_, is_new) = MovieResolver::default_pipeline() + .resolve(&cmd, &deps) + .await + .unwrap(); + assert!(is_new); + } +} diff --git a/crates/application/src/use_cases/log_review.rs b/crates/application/src/use_cases/log_review.rs index 29c1ebd..054d9ea 100644 --- a/crates/application/src/use_cases/log_review.rs +++ b/crates/application/src/use_cases/log_review.rs @@ -2,18 +2,25 @@ use domain::{ errors::DomainError, events::DomainEvent, models::{Movie, Review}, - ports::MetadataSearchCriteria, - value_objects::{Comment, ExternalMetadataId, MovieTitle, Rating, ReleaseYear, UserId}, + value_objects::{Comment, Rating, UserId}, }; -use crate::{commands::LogReviewCommand, context::AppContext}; +use crate::{ + commands::LogReviewCommand, + context::AppContext, + movie_resolver::{MovieResolver, MovieResolverDeps}, +}; pub async fn execute(ctx: &AppContext, cmd: LogReviewCommand) -> Result<(), DomainError> { let rating = Rating::new(cmd.rating)?; let user_id = UserId::from_uuid(cmd.user_id); let comment = cmd.comment.clone().map(Comment::new).transpose()?; - let (movie, is_new_movie) = resolve_movie(ctx, &cmd).await?; + let deps = MovieResolverDeps { + repository: ctx.repository.as_ref(), + metadata_client: ctx.metadata_client.as_ref(), + }; + let (movie, is_new_movie) = MovieResolver::default_pipeline().resolve(&cmd, &deps).await?; ctx.repository.upsert_movie(&movie).await?; @@ -25,101 +32,6 @@ pub async fn execute(ctx: &AppContext, cmd: LogReviewCommand) -> Result<(), Doma Ok(()) } -async fn resolve_movie( - ctx: &AppContext, - cmd: &LogReviewCommand, -) -> Result<(Movie, bool), DomainError> { - if let Some(ext_id_str) = &cmd.external_metadata_id { - if let Some(resolved) = resolve_external_movie(ctx, ext_id_str).await? { - return Ok(resolved); - } - } - - if let Some(title) = &cmd.manual_title { - if let Some(resolved) = resolve_by_title(ctx, title, cmd.manual_release_year).await? { - return Ok(resolved); - } - } - - resolve_manual_movie(ctx, cmd).await -} - -async fn resolve_external_movie( - ctx: &AppContext, - ext_id_str: &str, -) -> Result, DomainError> { - let tmdb_id = ExternalMetadataId::new(ext_id_str.to_string())?; - - if let Some(m) = ctx.repository.get_movie_by_external_id(&tmdb_id).await? { - return Ok(Some((m, false))); - } - - match ctx - .metadata_client - .fetch_movie_metadata(&MetadataSearchCriteria::ImdbId(tmdb_id)) - .await - { - Ok(m) => Ok(Some((m, true))), - Err(e) => { - tracing::warn!( - "Failed to fetch from TMDB, falling back to manual entry: {:?}", - e - ); - Ok(None) - } - } -} - -async fn resolve_by_title( - ctx: &AppContext, - title: &str, - year: Option, -) -> Result, DomainError> { - let criteria = MetadataSearchCriteria::Title { title: title.to_string(), year }; - match ctx.metadata_client.fetch_movie_metadata(&criteria).await { - Ok(m) => Ok(Some((m, true))), - Err(e) => { - tracing::warn!("OMDb title search failed, falling back to manual: {:?}", e); - Ok(None) - } - } -} - -async fn resolve_manual_movie( - ctx: &AppContext, - cmd: &LogReviewCommand, -) -> Result<(Movie, bool), DomainError> { - let title_str = cmd.manual_title.as_ref().ok_or_else(|| { - DomainError::ValidationError( - "Manual title required if TMDB fetch fails or is omitted".into(), - ) - })?; - let year_val = cmd.manual_release_year.ok_or_else(|| { - DomainError::ValidationError( - "Manual release year required if TMDB fetch fails or is omitted".into(), - ) - })?; - - let title = MovieTitle::new(title_str.clone())?; - let release_year = ReleaseYear::new(year_val)?; - - let candidates = ctx - .repository - .get_movies_by_title_and_year(&title, &release_year) - .await?; - - let matched_movie = candidates - .into_iter() - .find(|m| m.is_manual_match(&title, &release_year, cmd.manual_director.as_deref())); - - if let Some(existing_movie) = matched_movie { - Ok((existing_movie, false)) - } else { - let new_movie = Movie::new(None, title, release_year, cmd.manual_director.clone(), None); - Ok((new_movie, true)) - } -} - async fn publish_events( ctx: &AppContext, movie: &Movie, diff --git a/crates/presentation/src/dtos.rs b/crates/presentation/src/dtos.rs index 43ff60a..4503799 100644 --- a/crates/presentation/src/dtos.rs +++ b/crates/presentation/src/dtos.rs @@ -1,6 +1,10 @@ +use chrono::NaiveDateTime; use serde::{Deserialize, Serialize}; use uuid::Uuid; +use application::{commands::LogReviewCommand, queries::GetDiaryQuery}; +use domain::{errors::DomainError, models::SortDirection}; + fn empty_string_as_none<'de, D, T>(de: D) -> Result, D::Error> where D: serde::Deserializer<'de>, @@ -124,10 +128,210 @@ pub struct RegisterRequest { pub password: String, } +pub struct LogReviewData { + pub external_metadata_id: Option, + pub manual_title: Option, + pub manual_release_year: Option, + pub manual_director: Option, + pub rating: u8, + pub comment: Option, + pub watched_at: NaiveDateTime, +} + +#[derive(Debug)] +pub struct ParseReviewError { + pub field: &'static str, + pub message: String, +} + +impl TryFrom for LogReviewData { + type Error = ParseReviewError; + + fn try_from(form: LogReviewForm) -> Result { + let watched_at = NaiveDateTime::parse_from_str(&form.watched_at, "%Y-%m-%dT%H:%M:%S") + .or_else(|_| NaiveDateTime::parse_from_str(&form.watched_at, "%Y-%m-%dT%H:%M")) + .map_err(|_| ParseReviewError { + field: "watched_at", + message: format!( + "invalid date '{}'; expected YYYY-MM-DDTHH:MM[:SS]", + form.watched_at + ), + })?; + Ok(Self { + external_metadata_id: form.external_metadata_id.filter(|s| !s.trim().is_empty()), + manual_title: form.manual_title, + manual_release_year: form.manual_release_year, + manual_director: form.manual_director, + rating: form.rating, + comment: form.comment, + watched_at, + }) + } +} + +impl TryFrom for LogReviewData { + type Error = DomainError; + + fn try_from(req: LogReviewRequest) -> Result { + let watched_at = NaiveDateTime::parse_from_str(&req.watched_at, "%Y-%m-%dT%H:%M:%S") + .map_err(|_| { + DomainError::ValidationError( + "invalid watched_at; expected YYYY-MM-DDTHH:MM:SS".into(), + ) + })?; + Ok(Self { + external_metadata_id: req.external_metadata_id.filter(|s| !s.trim().is_empty()), + manual_title: req.manual_title, + manual_release_year: req.manual_release_year, + manual_director: req.manual_director, + rating: req.rating, + comment: req.comment, + watched_at, + }) + } +} + +impl LogReviewData { + pub fn into_command(self, user_id: Uuid) -> LogReviewCommand { + LogReviewCommand { + external_metadata_id: self.external_metadata_id, + manual_title: self.manual_title, + manual_release_year: self.manual_release_year, + manual_director: self.manual_director, + rating: self.rating, + comment: self.comment, + watched_at: self.watched_at, + user_id, + } + } +} + +impl From for GetDiaryQuery { + fn from(p: DiaryQueryParams) -> Self { + GetDiaryQuery { + limit: p.limit, + offset: p.offset, + sort_by: p.sort_by.as_deref().map(|s| { + if s == "asc" { + SortDirection::Ascending + } else { + SortDirection::Descending + } + }), + movie_id: p.movie_id, + } + } +} + #[cfg(test)] mod tests { use super::*; + fn make_form(watched_at: &str) -> LogReviewForm { + LogReviewForm { + external_metadata_id: None, + manual_title: None, + manual_release_year: None, + manual_director: None, + rating: 4, + comment: None, + watched_at: watched_at.to_string(), + } + } + + fn make_request(watched_at: &str) -> LogReviewRequest { + LogReviewRequest { + external_metadata_id: None, + manual_title: None, + manual_release_year: None, + manual_director: None, + rating: 4, + comment: None, + watched_at: watched_at.to_string(), + } + } + + #[test] + fn form_accepts_datetime_with_seconds() { + let data = LogReviewData::try_from(make_form("2024-03-15T20:30:00")).unwrap(); + assert_eq!(data.watched_at.format("%H:%M:%S").to_string(), "20:30:00"); + } + + #[test] + fn form_accepts_datetime_without_seconds() { + let data = LogReviewData::try_from(make_form("2024-03-15T20:30")).unwrap(); + assert_eq!(data.watched_at.format("%H:%M").to_string(), "20:30"); + } + + #[test] + fn form_rejects_invalid_datetime() { + assert!(LogReviewData::try_from(make_form("not-a-date")).is_err()); + } + + #[test] + fn api_accepts_datetime_with_seconds() { + let data = LogReviewData::try_from(make_request("2024-03-15T20:30:00")).unwrap(); + assert_eq!(data.watched_at.format("%H:%M:%S").to_string(), "20:30:00"); + } + + #[test] + fn api_rejects_datetime_without_seconds() { + assert!(LogReviewData::try_from(make_request("2024-03-15T20:30")).is_err()); + } + + #[test] + fn api_rejects_invalid_datetime() { + assert!(LogReviewData::try_from(make_request("garbage")).is_err()); + } + + #[test] + fn whitespace_external_id_becomes_none_in_form() { + let mut form = make_form("2024-03-15T20:30:00"); + form.external_metadata_id = Some(" ".to_string()); + let data = LogReviewData::try_from(form).unwrap(); + assert!(data.external_metadata_id.is_none()); + } + + #[test] + fn whitespace_external_id_becomes_none_in_request() { + let mut req = make_request("2024-03-15T20:30:00"); + req.external_metadata_id = Some(" ".to_string()); + let data = LogReviewData::try_from(req).unwrap(); + assert!(data.external_metadata_id.is_none()); + } + + #[test] + fn into_command_sets_user_id() { + let data = LogReviewData::try_from(make_form("2024-03-15T20:30:00")).unwrap(); + let user_id = Uuid::new_v4(); + let cmd = data.into_command(user_id); + assert_eq!(cmd.user_id, user_id); + } + + #[test] + fn sort_by_asc_string_becomes_ascending() { + let params = DiaryQueryParams { + sort_by: Some("asc".to_string()), + limit: None, + offset: None, + movie_id: None, + }; + let query = GetDiaryQuery::from(params); + assert!(matches!(query.sort_by, Some(domain::models::SortDirection::Ascending))); + } + + #[test] + fn sort_by_other_string_becomes_descending() { + let params = DiaryQueryParams { + sort_by: Some("desc".to_string()), + limit: None, + offset: None, + movie_id: None, + }; + let query = GetDiaryQuery::from(params); + assert!(matches!(query.sort_by, Some(domain::models::SortDirection::Descending))); + } + #[test] fn diary_response_serializes_correctly() { let resp = DiaryResponse { diff --git a/crates/presentation/src/handlers.rs b/crates/presentation/src/handlers.rs index 72d2ae5..2fd7ff3 100644 --- a/crates/presentation/src/handlers.rs +++ b/crates/presentation/src/handlers.rs @@ -5,19 +5,18 @@ pub mod html { response::{Html, IntoResponse, Redirect}, Form, }; - use chrono::{NaiveDateTime, Utc}; + use chrono::Utc; use uuid::Uuid; use application::{ - commands::{DeleteReviewCommand, LoginCommand, LogReviewCommand, RegisterCommand}, + commands::{DeleteReviewCommand, LoginCommand, RegisterCommand}, ports::{HtmlPageContext, LoginPageData, NewReviewPageData, RegisterPageData}, - queries::GetDiaryQuery, use_cases::{delete_review, get_diary, log_review, login as login_uc, register as register_uc}, }; - use domain::{errors::DomainError, models::SortDirection, value_objects::UserId}; + use domain::{errors::DomainError, value_objects::UserId}; use crate::{ - dtos::{DiaryQueryParams, ErrorQuery, LoginForm, LogReviewForm, RegisterForm}, + dtos::{DiaryQueryParams, ErrorQuery, LoginForm, LogReviewData, LogReviewForm, RegisterForm}, errors::ApiError, extractors::{OptionalCookieUser, RequiredCookieUser}, state::AppState, @@ -64,18 +63,7 @@ pub mod html { State(state): State, Query(params): Query, ) -> Result { - let query = GetDiaryQuery { - limit: params.limit, - offset: params.offset, - sort_by: params.sort_by.as_deref().map(|s| { - if s == "asc" { - SortDirection::Ascending - } else { - SortDirection::Descending - } - }), - movie_id: params.movie_id, - }; + let query = params.into(); let ctx = build_page_context(&state, user_id).await; let page = get_diary::execute(&state.app_ctx, query).await?; let html = state @@ -212,28 +200,14 @@ pub mod html { RequiredCookieUser(user_id): RequiredCookieUser, Form(form): Form, ) -> impl IntoResponse { - let watched_at = NaiveDateTime::parse_from_str(&form.watched_at, "%Y-%m-%dT%H:%M:%S") - .or_else(|_| NaiveDateTime::parse_from_str(&form.watched_at, "%Y-%m-%dT%H:%M")); - - let watched_at = match watched_at { - Ok(dt) => dt, + let data = match LogReviewData::try_from(form) { + Ok(d) => d, Err(_) => { return Redirect::to("/reviews/new?error=Invalid+date+format").into_response() } }; - let cmd = LogReviewCommand { - external_metadata_id: form.external_metadata_id.filter(|s| !s.trim().is_empty()), - manual_title: form.manual_title, - manual_release_year: form.manual_release_year, - manual_director: form.manual_director, - user_id: user_id.value(), - rating: form.rating, - comment: form.comment, - watched_at, - }; - - match log_review::execute(&state.app_ctx, cmd).await { + match log_review::execute(&state.app_ctx, data.into_command(user_id.value())).await { Ok(_) => Redirect::to("/").into_response(), Err(e) => { let msg = encode_error(&e.to_string()); @@ -329,17 +303,16 @@ pub mod api { http::StatusCode, response::IntoResponse, }; - use chrono::NaiveDateTime; use uuid::Uuid; use application::{ - commands::{DeleteReviewCommand, LoginCommand, LogReviewCommand, RegisterCommand, SyncPosterCommand}, - queries::{GetDiaryQuery, GetReviewHistoryQuery}, + commands::{DeleteReviewCommand, LoginCommand, RegisterCommand, SyncPosterCommand}, + queries::GetReviewHistoryQuery, use_cases::{delete_review, get_diary, get_review_history, log_review, login as login_uc, register as register_uc, sync_poster}, }; use domain::{ errors::DomainError, - models::{DiaryEntry, Movie, Review, SortDirection}, + models::{DiaryEntry, Movie, Review}, services::review_history::Trend, value_objects::MovieId, }; @@ -347,7 +320,8 @@ pub mod api { use crate::{ dtos::{ DiaryEntryDto, DiaryQueryParams, DiaryResponse, LoginRequest, LoginResponse, - LogReviewRequest, MovieDto, RegisterRequest, ReviewDto, ReviewHistoryResponse, + LogReviewData, LogReviewRequest, MovieDto, RegisterRequest, ReviewDto, + ReviewHistoryResponse, }, errors::ApiError, extractors::AuthenticatedUser, @@ -358,20 +332,7 @@ pub mod api { State(state): State, Query(params): Query, ) -> Result, ApiError> { - let query = GetDiaryQuery { - limit: params.limit, - offset: params.offset, - sort_by: params.sort_by.as_deref().map(|s| { - if s == "asc" { - SortDirection::Ascending - } else { - SortDirection::Descending - } - }), - movie_id: params.movie_id, - }; - - let page = get_diary::execute(&state.app_ctx, query).await?; + let page = get_diary::execute(&state.app_ctx, params.into()).await?; Ok(Json(DiaryResponse { items: page.items.iter().map(entry_to_dto).collect(), @@ -408,26 +369,8 @@ pub mod api { user: AuthenticatedUser, Json(req): Json, ) -> Result { - let watched_at = NaiveDateTime::parse_from_str(&req.watched_at, "%Y-%m-%dT%H:%M:%S") - .map_err(|_| { - ApiError(DomainError::ValidationError( - "Invalid watched_at format, expected YYYY-MM-DDTHH:MM:SS".into(), - )) - })?; - - let cmd = LogReviewCommand { - external_metadata_id: req.external_metadata_id.filter(|s| !s.trim().is_empty()), - manual_title: req.manual_title, - manual_release_year: req.manual_release_year, - manual_director: req.manual_director, - user_id: user.0.value(), - rating: req.rating, - comment: req.comment, - watched_at, - }; - - log_review::execute(&state.app_ctx, cmd).await?; - + let data = LogReviewData::try_from(req).map_err(ApiError)?; + log_review::execute(&state.app_ctx, data.into_command(user.0.value())).await?; Ok(StatusCode::CREATED) }