Refactor movie review logging and resolution strategies

- Introduced `MovieResolver` and associated strategies for resolving movie data based on external metadata ID, manual title, or manual entry.
- Updated `log_review` use case to utilize the new `MovieResolver` for fetching movie details.
- Simplified the `LogReviewData` structure and its conversion to `LogReviewCommand`.
- Enhanced error handling for date parsing in review forms and requests.
- Updated dependencies in `Cargo.toml` and `Cargo.lock` to include necessary crates for async operations.
- Added tests for new functionality in `movie_resolver.rs` to ensure correct behavior of resolution strategies.

Cargo.lock

@@ -36,8 +36,10 @@ checksum = "7f202df86484c868dbad7eaa557ef785d5c66295e41b460ef922eca0723b842c"
 name = "application"
 version = "0.1.0"
 dependencies = [
+ "async-trait",
  "chrono",
  "domain",
+ "tokio",
  "tracing",
  "uuid",
 ]

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<Vec<DiaryRow>, DomainError> {
-        // sqlx macros require literal ORDER BY values; separate branches also let the
+        match sort {
-        // query planner use the movie_id index instead of falling back to a filtered scan.
+            SortDirection::Descending => sqlx::query_as!(
-        match (movie_id, sort) {
-            (None, 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<Vec<DiaryRow>, 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<Paginated<DiaryEntry>, DomainError> {
-        let movie_id: Option<String> = 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!(
+        let (total, rows) = match &filter.movie_id {
-            self.count_diary_entries(movie_id.as_deref()),
+            None => tokio::try_join!(
-            self.fetch_diary_rows(movie_id.as_deref(), &filter.sort_by, limit, offset)
+                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()

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 }

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;

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<Option<(Movie, bool)>, DomainError>;
+}
+
+pub struct ExternalIdStrategy;
+pub struct TitleSearchStrategy;
+pub struct ManualMovieStrategy;
+
+pub struct MovieResolver {
+    strategies: Vec<Box<dyn ResolutionStrategy>>,
+}
+
+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<Option<(Movie, bool)>, 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<Option<(Movie, bool)>, 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<Option<(Movie, bool)>, 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<u16>) -> 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<Option<Movie>, DomainError> {
+            Ok(Some(self.0.clone()))
+        }
+        async fn get_movie_by_id(&self, _: &MovieId) -> Result<Option<Movie>, DomainError> {
+            panic!("unexpected")
+        }
+        async fn get_movies_by_title_and_year(
+            &self,
+            _: &MovieTitle,
+            _: &ReleaseYear,
+        ) -> Result<Vec<Movie>, DomainError> {
+            panic!("unexpected")
+        }
+        async fn upsert_movie(&self, _: &Movie) -> Result<(), DomainError> {
+            panic!("unexpected")
+        }
+        async fn save_review(&self, _: &Review) -> Result<DomainEvent, DomainError> {
+            panic!("unexpected")
+        }
+        async fn query_diary(
+            &self,
+            _: &DiaryFilter,
+        ) -> Result<Paginated<DiaryEntry>, DomainError> {
+            panic!("unexpected")
+        }
+        async fn get_review_history(&self, _: &MovieId) -> Result<ReviewHistory, DomainError> {
+            panic!("unexpected")
+        }
+        async fn get_review_by_id(
+            &self,
+            _: &ReviewId,
+        ) -> Result<Option<Review>, 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<Option<Movie>, DomainError> {
+            Ok(None)
+        }
+        async fn get_movie_by_id(&self, _: &MovieId) -> Result<Option<Movie>, DomainError> {
+            panic!("unexpected")
+        }
+        async fn get_movies_by_title_and_year(
+            &self,
+            _: &MovieTitle,
+            _: &ReleaseYear,
+        ) -> Result<Vec<Movie>, DomainError> {
+            Ok(vec![])
+        }
+        async fn upsert_movie(&self, _: &Movie) -> Result<(), DomainError> {
+            panic!("unexpected")
+        }
+        async fn save_review(&self, _: &Review) -> Result<DomainEvent, DomainError> {
+            panic!("unexpected")
+        }
+        async fn query_diary(
+            &self,
+            _: &DiaryFilter,
+        ) -> Result<Paginated<DiaryEntry>, DomainError> {
+            panic!("unexpected")
+        }
+        async fn get_review_history(&self, _: &MovieId) -> Result<ReviewHistory, DomainError> {
+            panic!("unexpected")
+        }
+        async fn get_review_by_id(
+            &self,
+            _: &ReviewId,
+        ) -> Result<Option<Review>, 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<Option<Movie>, DomainError> {
+            panic!("unexpected")
+        }
+        async fn get_movie_by_id(&self, _: &MovieId) -> Result<Option<Movie>, DomainError> {
+            panic!("unexpected")
+        }
+        async fn get_movies_by_title_and_year(
+            &self,
+            _: &MovieTitle,
+            _: &ReleaseYear,
+        ) -> Result<Vec<Movie>, DomainError> {
+            Ok(vec![self.0.clone()])
+        }
+        async fn upsert_movie(&self, _: &Movie) -> Result<(), DomainError> {
+            panic!("unexpected")
+        }
+        async fn save_review(&self, _: &Review) -> Result<DomainEvent, DomainError> {
+            panic!("unexpected")
+        }
+        async fn query_diary(
+            &self,
+            _: &DiaryFilter,
+        ) -> Result<Paginated<DiaryEntry>, DomainError> {
+            panic!("unexpected")
+        }
+        async fn get_review_history(&self, _: &MovieId) -> Result<ReviewHistory, DomainError> {
+            panic!("unexpected")
+        }
+        async fn get_review_by_id(
+            &self,
+            _: &ReviewId,
+        ) -> Result<Option<Review>, 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<Movie, DomainError> {
+            Ok(self.0.clone())
+        }
+        async fn get_poster_url(
+            &self,
+            _: &ExternalMetadataId,
+        ) -> Result<Option<PosterUrl>, DomainError> {
+            panic!("unexpected")
+        }
+    }
+
+    #[async_trait]
+    impl MetadataClient for MetaErrors {
+        async fn fetch_movie_metadata(
+            &self,
+            _: &MetadataSearchCriteria,
+        ) -> Result<Movie, DomainError> {
+            Err(DomainError::InfrastructureError("metadata unavailable".into()))
+        }
+        async fn get_poster_url(
+            &self,
+            _: &ExternalMetadataId,
+        ) -> Result<Option<PosterUrl>, 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);
+    }
+}

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, Rating, UserId},
-    value_objects::{Comment, ExternalMetadataId, MovieTitle, Rating, ReleaseYear, 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<Option<(Movie, bool)>, 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<u16>,
-) -> Result<Option<(Movie, bool)>, 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,

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<Option<T>, 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<String>,
+    pub manual_title: Option<String>,
+    pub manual_release_year: Option<u16>,
+    pub manual_director: Option<String>,
+    pub rating: u8,
+    pub comment: Option<String>,
+    pub watched_at: NaiveDateTime,
+}
+
+#[derive(Debug)]
+pub struct ParseReviewError {
+    pub field: &'static str,
+    pub message: String,
+}
+
+impl TryFrom<LogReviewForm> for LogReviewData {
+    type Error = ParseReviewError;
+
+    fn try_from(form: LogReviewForm) -> Result<Self, Self::Error> {
+        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<LogReviewRequest> for LogReviewData {
+    type Error = DomainError;
+
+    fn try_from(req: LogReviewRequest) -> Result<Self, Self::Error> {
+        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<DiaryQueryParams> 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 {

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<AppState>,
         Query(params): Query<DiaryQueryParams>,
     ) -> Result<impl IntoResponse, ApiError> {
-        let query = GetDiaryQuery {
+        let query = params.into();
-            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 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<LogReviewForm>,
     ) -> impl IntoResponse {
-        let watched_at = NaiveDateTime::parse_from_str(&form.watched_at, "%Y-%m-%dT%H:%M:%S")
+        let data = match LogReviewData::try_from(form) {
-            .or_else(|_| NaiveDateTime::parse_from_str(&form.watched_at, "%Y-%m-%dT%H:%M"));
+            Ok(d) => d,
-
-        let watched_at = match watched_at {
-            Ok(dt) => dt,
             Err(_) => {
                 return Redirect::to("/reviews/new?error=Invalid+date+format").into_response()
             }
         };
 
-        let cmd = LogReviewCommand {
+        match log_review::execute(&state.app_ctx, data.into_command(user_id.value())).await {
-            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 {
             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},
+        commands::{DeleteReviewCommand, LoginCommand, RegisterCommand, SyncPosterCommand},
-        queries::{GetDiaryQuery, GetReviewHistoryQuery},
+        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<AppState>,
         Query(params): Query<DiaryQueryParams>,
     ) -> Result<Json<DiaryResponse>, ApiError> {
-        let query = GetDiaryQuery {
+        let page = get_diary::execute(&state.app_ctx, params.into()).await?;
-            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?;
 
         Ok(Json(DiaryResponse {
             items: page.items.iter().map(entry_to_dto).collect(),
@@ -408,26 +369,8 @@ pub mod api {
         user: AuthenticatedUser,
         Json(req): Json<LogReviewRequest>,
     ) -> Result<impl IntoResponse, ApiError> {
-        let watched_at = NaiveDateTime::parse_from_str(&req.watched_at, "%Y-%m-%dT%H:%M:%S")
+        let data = LogReviewData::try_from(req).map_err(ApiError)?;
-            .map_err(|_| {
+        log_review::execute(&state.app_ctx, data.into_command(user.0.value())).await?;
-                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?;
-
         Ok(StatusCode::CREATED)
     }