From 2355f89bed662bec98f7c8e898fad99f4b7317b0 Mon Sep 17 00:00:00 2001 From: Gabriel Kaszewski Date: Fri, 29 May 2026 11:19:02 +0200 Subject: [PATCH] refactor: fix all clippy warnings properly - UserProfile struct groups display_name/bio/avatar/banner/also_known_as/profile_fields - User::from_persistence takes UserProfile (6 args, was 11) - PersistedReview struct for Review::from_persistence (1 arg, was 8) - WatchlistApInput struct for watchlist_to_ap_object (1 arg, was 8) - ActivityPubDeps struct for activitypub::wire (1 arg, was 11) - FederationRepos type alias for wire() return types - FeedSortBy: impl std::str::FromStr instead of inherent from_str - postgres users.rs: row_to_user takes &PgRow like sqlite - collapse nested ifs in multipart handlers - type alias for complex return types (image-converter, worker) - tui: allow large_enum_variant at crate level (pre-existing, unrelated) --- .../adapters/activitypub/src/event_handler.rs | 16 +- crates/adapters/activitypub/src/lib.rs | 51 +++-- crates/adapters/activitypub/src/objects.rs | 32 ++- .../activitypub/src/review_handler.rs | 12 +- .../adapters/activitypub/src/tests/objects.rs | 68 ++++--- crates/adapters/image-converter/src/lib.rs | 4 +- .../adapters/postgres-federation/src/lib.rs | 12 +- crates/adapters/postgres/src/models.rs | 18 +- crates/adapters/postgres/src/users.rs | 186 +++++------------- crates/adapters/sqlite-federation/src/lib.rs | 12 +- crates/adapters/sqlite/src/models.rs | 17 +- crates/adapters/sqlite/src/tests/users.rs | 22 ++- crates/adapters/sqlite/src/users.rs | 30 ++- crates/application/src/movie_resolver.rs | 9 +- crates/application/src/tests/worker.rs | 1 + .../src/use_cases/update_profile.rs | 13 +- crates/domain/src/models/mod.rs | 107 ++++------ crates/domain/src/models/tests.rs | 30 ++- crates/domain/src/ports.rs | 15 +- crates/domain/src/testing.rs | 5 +- crates/presentation/src/handlers/api.rs | 20 +- crates/presentation/src/handlers/html.rs | 46 +++-- crates/presentation/src/main.rs | 16 +- crates/presentation/src/tests/extractors.rs | 5 +- crates/presentation/tests/api_test.rs | 5 +- crates/tui/src/lib.rs | 1 + crates/worker/src/main.rs | 65 +++--- 27 files changed, 363 insertions(+), 455 deletions(-) diff --git a/crates/adapters/activitypub/src/event_handler.rs b/crates/adapters/activitypub/src/event_handler.rs index a103ac9..3694db2 100644 --- a/crates/adapters/activitypub/src/event_handler.rs +++ b/crates/adapters/activitypub/src/event_handler.rs @@ -220,16 +220,16 @@ impl ActivityPubEventHandler { let added_at_utc = chrono::DateTime::::from_naive_utc_and_offset(*added_at, chrono::Utc); - let obj = crate::objects::watchlist_to_ap_object( - ap_id.clone(), - actor, - movie_title.to_string(), + let obj = crate::objects::watchlist_to_ap_object(crate::objects::WatchlistApInput { + ap_id: ap_id.clone(), + actor_url: actor, + movie_title: movie_title.to_string(), release_year, - external_metadata_id.clone(), + external_metadata_id: external_metadata_id.clone(), poster_url, - added_at_utc, - &self.base_url, - ); + added_at: added_at_utc, + base_url: self.base_url.clone(), + }); let json = serde_json::to_value(obj)?; self.ap_service diff --git a/crates/adapters/activitypub/src/lib.rs b/crates/adapters/activitypub/src/lib.rs index e011882..749aa65 100644 --- a/crates/adapters/activitypub/src/lib.rs +++ b/crates/adapters/activitypub/src/lib.rs @@ -22,25 +22,50 @@ pub use remote_review_repository::RemoteReviewRepository; pub use review_handler::ReviewObjectHandler; pub use user_adapter::DomainUserRepoAdapter; +pub type FederationRepos = ( + std::sync::Arc, + std::sync::Arc, + std::sync::Arc, + std::sync::Arc, + std::sync::Arc, + std::sync::Arc, + std::sync::Arc, +); + pub struct ActivityPubWire { pub service: std::sync::Arc, pub router: axum::Router, pub event_handler: std::sync::Arc, } -pub async fn wire( - activity_repo: std::sync::Arc, - follow_repo: std::sync::Arc, - actor_repo: std::sync::Arc, - blocklist_repo: std::sync::Arc, - review_store: std::sync::Arc, - remote_watchlist_repo: std::sync::Arc, - local_ap_content: std::sync::Arc, - user_repo: std::sync::Arc, - base_url: String, - allow_registration: bool, - event_publisher: std::sync::Arc, -) -> anyhow::Result { +pub struct ActivityPubDeps { + pub activity_repo: std::sync::Arc, + pub follow_repo: std::sync::Arc, + pub actor_repo: std::sync::Arc, + pub blocklist_repo: std::sync::Arc, + pub review_store: std::sync::Arc, + pub remote_watchlist_repo: std::sync::Arc, + pub local_ap_content: std::sync::Arc, + pub user_repo: std::sync::Arc, + pub base_url: String, + pub allow_registration: bool, + pub event_publisher: std::sync::Arc, +} + +pub async fn wire(deps: ActivityPubDeps) -> anyhow::Result { + let ActivityPubDeps { + activity_repo, + follow_repo, + actor_repo, + blocklist_repo, + review_store, + remote_watchlist_repo, + local_ap_content, + user_repo, + base_url, + allow_registration, + event_publisher, + } = deps; let review_handler = std::sync::Arc::new(ReviewObjectHandler { content_query: std::sync::Arc::clone(&local_ap_content), review_store, diff --git a/crates/adapters/activitypub/src/objects.rs b/crates/adapters/activitypub/src/objects.rs index b6f1889..ca20255 100644 --- a/crates/adapters/activitypub/src/objects.rs +++ b/crates/adapters/activitypub/src/objects.rs @@ -131,16 +131,28 @@ pub struct WatchlistObject { pub(crate) cc: Vec, } -pub fn watchlist_to_ap_object( - ap_id: Url, - actor_url: Url, - movie_title: String, - release_year: u16, - external_metadata_id: Option, - poster_url: Option, - added_at: chrono::DateTime, - base_url: &str, -) -> WatchlistObject { +pub struct WatchlistApInput { + pub ap_id: Url, + pub actor_url: Url, + pub movie_title: String, + pub release_year: u16, + pub external_metadata_id: Option, + pub poster_url: Option, + pub added_at: chrono::DateTime, + pub base_url: String, +} + +pub fn watchlist_to_ap_object(input: WatchlistApInput) -> WatchlistObject { + let WatchlistApInput { + ap_id, + actor_url, + movie_title, + release_year, + external_metadata_id, + poster_url, + added_at, + base_url, + } = input; let year_str = if release_year > 0 { format!(" ({})", release_year) } else { diff --git a/crates/adapters/activitypub/src/review_handler.rs b/crates/adapters/activitypub/src/review_handler.rs index f1142ca..18cdfe3 100644 --- a/crates/adapters/activitypub/src/review_handler.rs +++ b/crates/adapters/activitypub/src/review_handler.rs @@ -98,18 +98,18 @@ impl ApObjectHandler for ReviewObjectHandler { let rating = Rating::new(obj.rating.min(5))?; let comment = obj.comment.map(Comment::new).transpose()?; - let review = domain::models::Review::from_persistence( - review_id, + let review = domain::models::Review::from_persistence(domain::models::PersistedReview { + id: review_id, movie_id, user_id, rating, comment, - obj.watched_at.naive_utc(), - obj.published.naive_utc(), - ReviewSource::Remote { + watched_at: obj.watched_at.naive_utc(), + created_at: obj.published.naive_utc(), + source: ReviewSource::Remote { actor_url: actor_url_str, }, - ); + }); self.review_store .save_remote_review( diff --git a/crates/adapters/activitypub/src/tests/objects.rs b/crates/adapters/activitypub/src/tests/objects.rs index 1ebbecd..04bc635 100644 --- a/crates/adapters/activitypub/src/tests/objects.rs +++ b/crates/adapters/activitypub/src/tests/objects.rs @@ -14,20 +14,22 @@ fn normalize_hashtag_strips_non_alphanumeric() { fn review_to_ap_object_includes_two_hashtags() { use chrono::NaiveDateTime; use domain::{ - models::{Review, ReviewSource}, + models::{PersistedReview, Review, ReviewSource}, value_objects::{MovieId, Rating, ReviewId, UserId}, }; - let review = Review::from_persistence( - ReviewId::generate(), - MovieId::from_uuid(uuid::Uuid::new_v4()), - UserId::from_uuid(uuid::Uuid::new_v4()), - Rating::new(4).unwrap(), - None, - NaiveDateTime::parse_from_str("2024-01-01 00:00:00", "%Y-%m-%d %H:%M:%S").unwrap(), - NaiveDateTime::parse_from_str("2024-01-01 00:00:00", "%Y-%m-%d %H:%M:%S").unwrap(), - ReviewSource::Local, - ); + let review = Review::from_persistence(PersistedReview { + id: ReviewId::generate(), + movie_id: MovieId::from_uuid(uuid::Uuid::new_v4()), + user_id: UserId::from_uuid(uuid::Uuid::new_v4()), + rating: Rating::new(4).unwrap(), + comment: None, + watched_at: NaiveDateTime::parse_from_str("2024-01-01 00:00:00", "%Y-%m-%d %H:%M:%S") + .unwrap(), + created_at: NaiveDateTime::parse_from_str("2024-01-01 00:00:00", "%Y-%m-%d %H:%M:%S") + .unwrap(), + source: ReviewSource::Local, + }); let obj = review_to_ap_object( &review, "https://example.com/reviews/1".parse().unwrap(), @@ -47,20 +49,22 @@ fn review_to_ap_object_includes_two_hashtags() { fn review_to_ap_object_has_public_addressing() { use chrono::NaiveDateTime; use domain::{ - models::{Review, ReviewSource}, + models::{PersistedReview, Review, ReviewSource}, value_objects::{MovieId, Rating, ReviewId, UserId}, }; - let review = Review::from_persistence( - ReviewId::generate(), - MovieId::from_uuid(uuid::Uuid::new_v4()), - UserId::from_uuid(uuid::Uuid::new_v4()), - Rating::new(3).unwrap(), - None, - NaiveDateTime::parse_from_str("2024-06-01 00:00:00", "%Y-%m-%d %H:%M:%S").unwrap(), - NaiveDateTime::parse_from_str("2024-06-01 00:00:00", "%Y-%m-%d %H:%M:%S").unwrap(), - ReviewSource::Local, - ); + let review = Review::from_persistence(PersistedReview { + id: ReviewId::generate(), + movie_id: MovieId::from_uuid(uuid::Uuid::new_v4()), + user_id: UserId::from_uuid(uuid::Uuid::new_v4()), + rating: Rating::new(3).unwrap(), + comment: None, + watched_at: NaiveDateTime::parse_from_str("2024-06-01 00:00:00", "%Y-%m-%d %H:%M:%S") + .unwrap(), + created_at: NaiveDateTime::parse_from_str("2024-06-01 00:00:00", "%Y-%m-%d %H:%M:%S") + .unwrap(), + source: ReviewSource::Local, + }); let actor_url: url::Url = "https://example.com/users/abc".parse().unwrap(); let obj = review_to_ap_object( &review, @@ -78,16 +82,16 @@ fn review_to_ap_object_has_public_addressing() { #[test] fn watchlist_to_ap_object_has_public_addressing() { let actor_url: url::Url = "https://example.com/users/abc".parse().unwrap(); - let obj = watchlist_to_ap_object( - "https://example.com/watchlist/1".parse().unwrap(), - actor_url.clone(), - "Alien".to_string(), - 1979, - None, - None, - chrono::Utc::now(), - "https://example.com", - ); + let obj = watchlist_to_ap_object(WatchlistApInput { + ap_id: "https://example.com/watchlist/1".parse().unwrap(), + actor_url: actor_url.clone(), + movie_title: "Alien".to_string(), + release_year: 1979, + external_metadata_id: None, + poster_url: None, + added_at: chrono::Utc::now(), + base_url: "https://example.com".to_string(), + }); assert_eq!(obj.to, vec!["https://www.w3.org/ns/activitystreams#Public"]); assert_eq!(obj.cc, vec!["https://example.com/users/abc/followers"]); } diff --git a/crates/adapters/image-converter/src/lib.rs b/crates/adapters/image-converter/src/lib.rs index 5d8949c..3a2b7b0 100644 --- a/crates/adapters/image-converter/src/lib.rs +++ b/crates/adapters/image-converter/src/lib.rs @@ -11,12 +11,14 @@ use domain::ports::{ }; use std::sync::Arc; +type ConversionPair = (Arc, Arc); + pub fn build( image_storage: Arc, image_ref_command: Arc, image_ref_query: Arc, event_publisher: Arc, -) -> anyhow::Result, Arc)>> { +) -> anyhow::Result> { let config = match ConversionConfig::from_env()? { Some(c) => c, None => return Ok(None), diff --git a/crates/adapters/postgres-federation/src/lib.rs b/crates/adapters/postgres-federation/src/lib.rs index d237e5a..64d0d07 100644 --- a/crates/adapters/postgres-federation/src/lib.rs +++ b/crates/adapters/postgres-federation/src/lib.rs @@ -854,17 +854,7 @@ impl RemoteWatchlistRepository for PostgresFederationRepository { } } -pub fn wire( - pool: sqlx::PgPool, -) -> ( - std::sync::Arc, - std::sync::Arc, - std::sync::Arc, - std::sync::Arc, - std::sync::Arc, - std::sync::Arc, - std::sync::Arc, -) { +pub fn wire(pool: sqlx::PgPool) -> activitypub::FederationRepos { let fed = std::sync::Arc::new(PostgresFederationRepository::new(pool)); ( std::sync::Arc::clone(&fed) as _, diff --git a/crates/adapters/postgres/src/models.rs b/crates/adapters/postgres/src/models.rs index 84ff421..2c1a6dd 100644 --- a/crates/adapters/postgres/src/models.rs +++ b/crates/adapters/postgres/src/models.rs @@ -1,7 +1,10 @@ use chrono::NaiveDateTime; use domain::{ errors::DomainError, - models::{DiaryEntry, FeedEntry, Movie, MovieSummary, Review, ReviewSource, UserSummary}, + models::{ + DiaryEntry, FeedEntry, Movie, MovieSummary, PersistedReview, Review, ReviewSource, + UserSummary, + }, value_objects::{ Comment, Email, ExternalMetadataId, MovieId, MovieTitle, PosterPath, Rating, ReleaseYear, ReviewId, UserId, @@ -102,9 +105,16 @@ impl ReviewRow { None => ReviewSource::Local, Some(url) => ReviewSource::Remote { actor_url: url }, }; - Ok(Review::from_persistence( - id, movie_id, user_id, rating, comment, watched_at, created_at, source, - )) + Ok(Review::from_persistence(PersistedReview { + id, + movie_id, + user_id, + rating, + comment, + watched_at, + created_at, + source, + })) } } diff --git a/crates/adapters/postgres/src/users.rs b/crates/adapters/postgres/src/users.rs index 371be0e..8267426 100644 --- a/crates/adapters/postgres/src/users.rs +++ b/crates/adapters/postgres/src/users.rs @@ -1,6 +1,6 @@ use async_trait::async_trait; use chrono::Utc; -use sqlx::PgPool; +use sqlx::{PgPool, Row}; use domain::{ errors::DomainError, @@ -33,122 +33,67 @@ impl PostgresUserRepository { } fn row_to_user( - id_str: String, - email_str: String, - username_str: String, - hash_str: String, - role: UserRole, - display_name: Option, - bio: Option, - avatar_path: Option, - banner_path: Option, - also_known_as: Option, + row: &sqlx::postgres::PgRow, profile_fields: Vec, ) -> Result { + let id_str: String = row.get("id"); let id = uuid::Uuid::parse_str(&id_str) .map_err(|e| DomainError::InfrastructureError(e.to_string()))?; - let email = - Email::new(email_str).map_err(|e| DomainError::InfrastructureError(e.to_string()))?; - let username = Username::new(username_str) + let email = Email::new(row.get::("email")) .map_err(|e| DomainError::InfrastructureError(e.to_string()))?; - let hash = PasswordHash::new(hash_str) + let username = Username::new(row.get::("username")) .map_err(|e| DomainError::InfrastructureError(e.to_string()))?; + let hash = PasswordHash::new(row.get::("password_hash")) + .map_err(|e| DomainError::InfrastructureError(e.to_string()))?; + let role_str: String = row.get("role"); Ok(User::from_persistence( UserId::from_uuid(id), email, username, hash, - role, - display_name, - bio, - avatar_path, - banner_path, - also_known_as, - profile_fields, + Self::parse_role(&role_str), + domain::models::UserProfile { + display_name: row.try_get("display_name").ok().flatten(), + bio: row.try_get("bio").ok().flatten(), + avatar_path: row.try_get("avatar_path").ok().flatten(), + banner_path: row.try_get("banner_path").ok().flatten(), + also_known_as: row.try_get("also_known_as").ok().flatten(), + profile_fields, + }, )) } } +const PG_USER_COLS: &str = "id, email, username, password_hash, role, display_name, bio, avatar_path, banner_path, also_known_as"; + #[async_trait] impl UserRepository for PostgresUserRepository { async fn find_by_email(&self, email: &Email) -> Result, DomainError> { let email_str = email.value(); - #[derive(sqlx::FromRow)] - struct Row { - id: String, - email: String, - username: String, - password_hash: String, - role: String, - display_name: Option, - bio: Option, - avatar_path: Option, - banner_path: Option, - also_known_as: Option, - } - let row = sqlx::query_as::<_, Row>( - "SELECT id, email, username, password_hash, role, display_name, bio, avatar_path, banner_path, also_known_as FROM users WHERE email = $1", - ) + let row = sqlx::query(&format!( + "SELECT {PG_USER_COLS} FROM users WHERE email = $1" + )) .bind(email_str) .fetch_optional(&self.pool) .await .map_err(Self::map_err)?; - row.map(|r| { - Self::row_to_user( - r.id, - r.email, - r.username, - r.password_hash, - Self::parse_role(&r.role), - r.display_name, - r.bio, - r.avatar_path, - r.banner_path, - r.also_known_as, - vec![], - ) - }) - .transpose() + row.as_ref() + .map(|r| Self::row_to_user(r, vec![])) + .transpose() } async fn find_by_username(&self, username: &Username) -> Result, DomainError> { let username_str = username.value(); - #[derive(sqlx::FromRow)] - struct Row { - id: String, - email: String, - username: String, - password_hash: String, - role: String, - display_name: Option, - bio: Option, - avatar_path: Option, - banner_path: Option, - also_known_as: Option, - } - let row = sqlx::query_as::<_, Row>( - "SELECT id, email, username, password_hash, role, display_name, bio, avatar_path, banner_path, also_known_as FROM users WHERE username = $1", - ) + let row = sqlx::query(&format!( + "SELECT {PG_USER_COLS} FROM users WHERE username = $1" + )) .bind(username_str) .fetch_optional(&self.pool) .await .map_err(Self::map_err)?; - row.map(|r| { - Self::row_to_user( - r.id, - r.email, - r.username, - r.password_hash, - Self::parse_role(&r.role), - r.display_name, - r.bio, - r.avatar_path, - r.banner_path, - r.also_known_as, - vec![], - ) - }) - .transpose() + row.as_ref() + .map(|r| Self::row_to_user(r, vec![])) + .transpose() } async fn save(&self, user: &User) -> Result<(), DomainError> { @@ -191,35 +136,15 @@ impl UserRepository for PostgresUserRepository { async fn find_by_id(&self, id: &UserId) -> Result, DomainError> { let id_str = id.value().to_string(); - #[derive(sqlx::FromRow)] - struct Row { - id: String, - email: String, - username: String, - password_hash: String, - role: String, - display_name: Option, - bio: Option, - avatar_path: Option, - banner_path: Option, - also_known_as: Option, - } - let row = sqlx::query_as::<_, Row>( - "SELECT id, email, username, password_hash, role, display_name, bio, avatar_path, banner_path, also_known_as FROM users WHERE id = $1", - ) - .bind(&id_str) - .fetch_optional(&self.pool) - .await - .map_err(Self::map_err)?; + let row = sqlx::query(&format!("SELECT {PG_USER_COLS} FROM users WHERE id = $1")) + .bind(&id_str) + .fetch_optional(&self.pool) + .await + .map_err(Self::map_err)?; let Some(r) = row else { return Ok(None) }; - #[derive(sqlx::FromRow)] - struct FieldRow { - name: String, - value: String, - } - let field_rows = sqlx::query_as::<_, FieldRow>( + let field_rows = sqlx::query( "SELECT name, value FROM user_profile_fields WHERE user_id = $1 ORDER BY position ASC", ) .bind(&id_str) @@ -228,47 +153,30 @@ impl UserRepository for PostgresUserRepository { .map_err(|e| DomainError::InfrastructureError(e.to_string()))?; let profile_fields = field_rows - .into_iter() + .iter() .map(|f| ProfileField { - name: f.name, - value: f.value, + name: f.get("name"), + value: f.get("value"), }) .collect(); - Self::row_to_user( - r.id, - r.email, - r.username, - r.password_hash, - Self::parse_role(&r.role), - r.display_name, - r.bio, - r.avatar_path, - r.banner_path, - r.also_known_as, - profile_fields, - ) - .map(Some) + Self::row_to_user(&r, profile_fields).map(Some) } async fn update_profile( &self, user_id: &UserId, - display_name: Option, - bio: Option, - avatar_path: Option, - banner_path: Option, - also_known_as: Option, + profile: &domain::models::UserProfile, ) -> Result<(), DomainError> { let id_str = user_id.value().to_string(); sqlx::query( "UPDATE users SET display_name = $1, bio = $2, avatar_path = $3, banner_path = $4, also_known_as = $5 WHERE id = $6", ) - .bind(&display_name) - .bind(&bio) - .bind(&avatar_path) - .bind(&banner_path) - .bind(&also_known_as) + .bind(&profile.display_name) + .bind(&profile.bio) + .bind(&profile.avatar_path) + .bind(&profile.banner_path) + .bind(&profile.also_known_as) .bind(&id_str) .execute(&self.pool) .await diff --git a/crates/adapters/sqlite-federation/src/lib.rs b/crates/adapters/sqlite-federation/src/lib.rs index ccdd9b7..8be64c6 100644 --- a/crates/adapters/sqlite-federation/src/lib.rs +++ b/crates/adapters/sqlite-federation/src/lib.rs @@ -1038,17 +1038,7 @@ impl RemoteWatchlistRepository for SqliteFederationRepository { } } -pub fn wire( - pool: sqlx::SqlitePool, -) -> ( - std::sync::Arc, - std::sync::Arc, - std::sync::Arc, - std::sync::Arc, - std::sync::Arc, - std::sync::Arc, - std::sync::Arc, -) { +pub fn wire(pool: sqlx::SqlitePool) -> activitypub::FederationRepos { let fed = std::sync::Arc::new(SqliteFederationRepository::new(pool)); ( std::sync::Arc::clone(&fed) as _, diff --git a/crates/adapters/sqlite/src/models.rs b/crates/adapters/sqlite/src/models.rs index 5672590..3212f19 100644 --- a/crates/adapters/sqlite/src/models.rs +++ b/crates/adapters/sqlite/src/models.rs @@ -2,8 +2,8 @@ use chrono::NaiveDateTime; use domain::{ errors::DomainError, models::{ - DiaryEntry, FeedEntry, Movie, MovieSummary, Review, ReviewSource, UserSummary, - WatchlistEntry, WatchlistWithMovie, + DiaryEntry, FeedEntry, Movie, MovieSummary, PersistedReview, Review, ReviewSource, + UserSummary, WatchlistEntry, WatchlistWithMovie, }, value_objects::{ Comment, Email, ExternalMetadataId, MovieId, MovieTitle, PosterPath, Rating, ReleaseYear, @@ -109,9 +109,16 @@ impl ReviewRow { None => ReviewSource::Local, Some(url) => ReviewSource::Remote { actor_url: url }, }; - Ok(Review::from_persistence( - id, movie_id, user_id, rating, comment, watched_at, created_at, source, - )) + Ok(Review::from_persistence(PersistedReview { + id, + movie_id, + user_id, + rating, + comment, + watched_at, + created_at, + source, + })) } } diff --git a/crates/adapters/sqlite/src/tests/users.rs b/crates/adapters/sqlite/src/tests/users.rs index dd8d17b..c6fd2fa 100644 --- a/crates/adapters/sqlite/src/tests/users.rs +++ b/crates/adapters/sqlite/src/tests/users.rs @@ -6,7 +6,7 @@ use sqlx::SqlitePool; async fn setup() -> (SqlitePool, SqliteUserRepository) { let pool = SqlitePool::connect(":memory:").await.unwrap(); sqlx::query( - "CREATE TABLE users (id TEXT PRIMARY KEY, email TEXT NOT NULL UNIQUE, username TEXT NOT NULL UNIQUE, password_hash TEXT NOT NULL, created_at TEXT NOT NULL, role TEXT NOT NULL DEFAULT 'standard', bio TEXT, avatar_path TEXT, banner_path TEXT, also_known_as TEXT)" + "CREATE TABLE users (id TEXT PRIMARY KEY, email TEXT NOT NULL UNIQUE, username TEXT NOT NULL UNIQUE, password_hash TEXT NOT NULL, created_at TEXT NOT NULL, role TEXT NOT NULL DEFAULT 'standard', display_name TEXT, bio TEXT, avatar_path TEXT, banner_path TEXT, also_known_as TEXT)" ) .execute(&pool) .await @@ -65,10 +65,11 @@ async fn update_profile_persists_bio_and_avatar() { repo.update_profile( user.id(), - Some("My biography".to_string()), - Some("avatars/user1".to_string()), - None, - None, + &domain::models::UserProfile { + bio: Some("My biography".to_string()), + avatar_path: Some("avatars/user1".to_string()), + ..Default::default() + }, ) .await .unwrap(); @@ -90,14 +91,15 @@ async fn update_profile_clears_fields_with_none() { repo.save(&user).await.unwrap(); repo.update_profile( user.id(), - Some("bio".to_string()), - Some("path".to_string()), - None, - None, + &domain::models::UserProfile { + bio: Some("bio".to_string()), + avatar_path: Some("path".to_string()), + ..Default::default() + }, ) .await .unwrap(); - repo.update_profile(user.id(), None, None, None, None) + repo.update_profile(user.id(), &domain::models::UserProfile::default()) .await .unwrap(); diff --git a/crates/adapters/sqlite/src/users.rs b/crates/adapters/sqlite/src/users.rs index c445d75..cbc5527 100644 --- a/crates/adapters/sqlite/src/users.rs +++ b/crates/adapters/sqlite/src/users.rs @@ -51,12 +51,14 @@ impl SqliteUserRepository { username, hash, Self::parse_role(&role_str), - row.try_get("display_name").ok().flatten(), - row.try_get("bio").ok().flatten(), - row.try_get("avatar_path").ok().flatten(), - row.try_get("banner_path").ok().flatten(), - row.try_get("also_known_as").ok().flatten(), - profile_fields, + domain::models::UserProfile { + display_name: row.try_get("display_name").ok().flatten(), + bio: row.try_get("bio").ok().flatten(), + avatar_path: row.try_get("avatar_path").ok().flatten(), + banner_path: row.try_get("banner_path").ok().flatten(), + also_known_as: row.try_get("also_known_as").ok().flatten(), + profile_fields, + }, )) } } @@ -161,21 +163,17 @@ impl UserRepository for SqliteUserRepository { async fn update_profile( &self, user_id: &UserId, - display_name: Option, - bio: Option, - avatar_path: Option, - banner_path: Option, - also_known_as: Option, + profile: &domain::models::UserProfile, ) -> Result<(), DomainError> { let id_str = user_id.value().to_string(); sqlx::query( "UPDATE users SET display_name = ?, bio = ?, avatar_path = ?, banner_path = ?, also_known_as = ? WHERE id = ?", ) - .bind(&display_name) - .bind(&bio) - .bind(&avatar_path) - .bind(&banner_path) - .bind(&also_known_as) + .bind(&profile.display_name) + .bind(&profile.bio) + .bind(&profile.avatar_path) + .bind(&profile.banner_path) + .bind(&profile.also_known_as) .bind(&id_str) .execute(&self.pool) .await diff --git a/crates/application/src/movie_resolver.rs b/crates/application/src/movie_resolver.rs index 57d959e..733514a 100644 --- a/crates/application/src/movie_resolver.rs +++ b/crates/application/src/movie_resolver.rs @@ -117,11 +117,10 @@ impl ResolutionStrategy for TitleSearchStrategy { match deps.metadata_client.fetch_movie_metadata(&criteria).await { Ok(m) => { // Movie may already exist in DB under this external_metadata_id - if let Some(ext_id) = m.external_metadata_id() { - if let Some(existing) = deps.repository.get_movie_by_external_id(ext_id).await? - { - return Ok(Some((existing, false))); - } + if let Some(ext_id) = m.external_metadata_id() + && let Some(existing) = deps.repository.get_movie_by_external_id(ext_id).await? + { + return Ok(Some((existing, false))); } Ok(Some((m, true))) } diff --git a/crates/application/src/tests/worker.rs b/crates/application/src/tests/worker.rs index 3988e54..2900793 100644 --- a/crates/application/src/tests/worker.rs +++ b/crates/application/src/tests/worker.rs @@ -56,6 +56,7 @@ impl EventHandler for RecordingHandler { "watchlist" } DomainEvent::FollowAccepted { .. } => "follow_accepted", + DomainEvent::BackfillFollower { .. } => "backfill_follower", }; self.calls.lock().unwrap().push(label); Ok(()) diff --git a/crates/application/src/use_cases/update_profile.rs b/crates/application/src/use_cases/update_profile.rs index a898bc9..8b04029 100644 --- a/crates/application/src/use_cases/update_profile.rs +++ b/crates/application/src/use_cases/update_profile.rs @@ -68,11 +68,14 @@ pub async fn execute(ctx: &AppContext, cmd: UpdateProfileCommand) -> Result<(), ctx.user_repository .update_profile( &user_id, - cmd.display_name, - cmd.bio, - new_avatar_path, - new_banner_path, - cmd.also_known_as, + &domain::models::UserProfile { + display_name: cmd.display_name, + bio: cmd.bio, + avatar_path: new_avatar_path, + banner_path: new_banner_path, + also_known_as: cmd.also_known_as, + profile_fields: vec![], + }, ) .await?; diff --git a/crates/domain/src/models/mod.rs b/crates/domain/src/models/mod.rs index ed21d4d..48adadc 100644 --- a/crates/domain/src/models/mod.rs +++ b/crates/domain/src/models/mod.rs @@ -163,6 +163,17 @@ pub enum ReviewSource { }, } +pub struct PersistedReview { + pub id: ReviewId, + pub movie_id: MovieId, + pub user_id: UserId, + pub rating: Rating, + pub comment: Option, + pub watched_at: NaiveDateTime, + pub created_at: NaiveDateTime, + pub source: ReviewSource, +} + #[derive(Clone, Debug)] pub struct Review { id: ReviewId, @@ -195,25 +206,16 @@ impl Review { }) } - pub fn from_persistence( - id: ReviewId, - movie_id: MovieId, - user_id: UserId, - rating: Rating, - comment: Option, - watched_at: NaiveDateTime, - created_at: NaiveDateTime, - source: ReviewSource, - ) -> Self { + pub fn from_persistence(row: PersistedReview) -> Self { Self { - id, - movie_id, - user_id, - rating, - comment, - watched_at, - created_at, - source, + id: row.id, + movie_id: row.movie_id, + user_id: row.user_id, + rating: row.rating, + comment: row.comment, + watched_at: row.watched_at, + created_at: row.created_at, + source: row.source, } } @@ -314,6 +316,16 @@ pub struct ProfileField { pub value: String, } +#[derive(Clone, Debug, Default)] +pub struct UserProfile { + pub display_name: Option, + pub bio: Option, + pub avatar_path: Option, + pub banner_path: Option, + pub also_known_as: Option, + pub profile_fields: Vec, +} + #[derive(Clone, Debug)] pub struct User { id: UserId, @@ -321,12 +333,7 @@ pub struct User { username: Username, password_hash: PasswordHash, role: UserRole, - display_name: Option, - bio: Option, - avatar_path: Option, - banner_path: Option, - also_known_as: Option, - profile_fields: Vec, + profile: UserProfile, } impl User { @@ -342,12 +349,7 @@ impl User { username, password_hash, role, - display_name: None, - bio: None, - avatar_path: None, - banner_path: None, - also_known_as: None, - profile_fields: vec![], + profile: UserProfile::default(), } } @@ -357,12 +359,7 @@ impl User { username: Username, password_hash: PasswordHash, role: UserRole, - display_name: Option, - bio: Option, - avatar_path: Option, - banner_path: Option, - also_known_as: Option, - profile_fields: Vec, + profile: UserProfile, ) -> Self { Self { id, @@ -370,12 +367,7 @@ impl User { username, password_hash, role, - display_name, - bio, - avatar_path, - banner_path, - also_known_as, - profile_fields, + profile, } } @@ -383,19 +375,8 @@ impl User { self.password_hash = new_hash; } - pub fn update_profile( - &mut self, - display_name: Option, - bio: Option, - avatar_path: Option, - banner_path: Option, - also_known_as: Option, - ) { - self.display_name = display_name; - self.bio = bio; - self.avatar_path = avatar_path; - self.banner_path = banner_path; - self.also_known_as = also_known_as; + pub fn update_profile(&mut self, profile: UserProfile) { + self.profile = profile; } pub fn email(&self) -> &Email { @@ -414,26 +395,22 @@ impl User { &self.role } pub fn display_name(&self) -> Option<&str> { - self.display_name.as_deref() + self.profile.display_name.as_deref() } pub fn bio(&self) -> Option<&str> { - self.bio.as_deref() + self.profile.bio.as_deref() } - pub fn avatar_path(&self) -> Option<&str> { - self.avatar_path.as_deref() + self.profile.avatar_path.as_deref() } - pub fn banner_path(&self) -> Option<&str> { - self.banner_path.as_deref() + self.profile.banner_path.as_deref() } - pub fn also_known_as(&self) -> Option<&str> { - self.also_known_as.as_deref() + self.profile.also_known_as.as_deref() } - pub fn profile_fields(&self) -> &[ProfileField] { - &self.profile_fields + &self.profile.profile_fields } } diff --git a/crates/domain/src/models/tests.rs b/crates/domain/src/models/tests.rs index 27cd00d..d9025af 100644 --- a/crates/domain/src/models/tests.rs +++ b/crates/domain/src/models/tests.rs @@ -8,23 +8,18 @@ fn make_user() -> User { Username::new("alice".to_string()).unwrap(), PasswordHash::new("hash".to_string()).unwrap(), UserRole::Standard, - None, - None, - None, - None, - vec![], + UserProfile::default(), ) } #[test] fn update_profile_sets_fields() { let mut user = make_user(); - user.update_profile( - Some("My bio".to_string()), - Some("avatars/abc".to_string()), - None, - None, - ); + user.update_profile(UserProfile { + bio: Some("My bio".to_string()), + avatar_path: Some("avatars/abc".to_string()), + ..Default::default() + }); assert_eq!(user.bio(), Some("My bio")); assert_eq!(user.avatar_path(), Some("avatars/abc")); } @@ -32,13 +27,12 @@ fn update_profile_sets_fields() { #[test] fn update_profile_clears_with_none() { let mut user = make_user(); - user.update_profile( - Some("bio".to_string()), - Some("path".to_string()), - None, - None, - ); - user.update_profile(None, None, None, None); + user.update_profile(UserProfile { + bio: Some("bio".to_string()), + avatar_path: Some("path".to_string()), + ..Default::default() + }); + user.update_profile(UserProfile::default()); assert_eq!(user.bio(), None); assert_eq!(user.avatar_path(), None); } diff --git a/crates/domain/src/ports.rs b/crates/domain/src/ports.rs index cf02cae..190a1b8 100644 --- a/crates/domain/src/ports.rs +++ b/crates/domain/src/ports.rs @@ -33,14 +33,15 @@ pub enum FeedSortBy { RatingAsc, } -impl FeedSortBy { - pub fn from_str(s: &str) -> Self { - match s { +impl std::str::FromStr for FeedSortBy { + type Err = std::convert::Infallible; + fn from_str(s: &str) -> Result { + Ok(match s { "date_asc" => Self::DateAsc, "rating" => Self::Rating, "rating_asc" => Self::RatingAsc, _ => Self::Date, - } + }) } } @@ -185,11 +186,7 @@ pub trait UserRepository: Send + Sync { async fn update_profile( &self, user_id: &UserId, - display_name: Option, - bio: Option, - avatar_path: Option, - banner_path: Option, - also_known_as: Option, + profile: &crate::models::UserProfile, ) -> Result<(), DomainError>; } diff --git a/crates/domain/src/testing.rs b/crates/domain/src/testing.rs index 743e0b7..24d230d 100644 --- a/crates/domain/src/testing.rs +++ b/crates/domain/src/testing.rs @@ -219,10 +219,7 @@ impl UserRepository for InMemoryUserRepository { async fn update_profile( &self, _user_id: &UserId, - _bio: Option, - _avatar_path: Option, - _banner_path: Option, - _also_known_as: Option, + _profile: &crate::models::UserProfile, ) -> Result<(), DomainError> { Ok(()) } diff --git a/crates/presentation/src/handlers/api.rs b/crates/presentation/src/handlers/api.rs index 66f60a9..a6bf3a7 100644 --- a/crates/presentation/src/handlers/api.rs +++ b/crates/presentation/src/handlers/api.rs @@ -503,20 +503,20 @@ pub async fn update_profile_handler( } "avatar" => { let ct = field.content_type().map(|s| s.to_string()); - if let Ok(bytes) = field.bytes().await { - if !bytes.is_empty() { - avatar_bytes = Some(bytes.to_vec()); - avatar_content_type = ct; - } + if let Ok(bytes) = field.bytes().await + && !bytes.is_empty() + { + avatar_bytes = Some(bytes.to_vec()); + avatar_content_type = ct; } } "banner" => { let ct = field.content_type().map(|s| s.to_string()); - if let Ok(bytes) = field.bytes().await { - if !bytes.is_empty() { - banner_bytes = Some(bytes.to_vec()); - banner_content_type = ct; - } + if let Ok(bytes) = field.bytes().await + && !bytes.is_empty() + { + banner_bytes = Some(bytes.to_vec()); + banner_content_type = ct; } } _ => {} diff --git a/crates/presentation/src/handlers/html.rs b/crates/presentation/src/handlers/html.rs index f492aef..b4345e1 100644 --- a/crates/presentation/src/handlers/html.rs +++ b/crates/presentation/src/handlers/html.rs @@ -428,7 +428,7 @@ pub async fn get_activity_feed( let query = application::queries::GetActivityFeedQuery { limit, offset, - sort_by: domain::ports::FeedSortBy::from_str(sort_by_str), + sort_by: sort_by_str.parse().unwrap_or_default(), search: search_opt, following, }; @@ -661,7 +661,7 @@ pub async fn get_user_profile( view: profile_view, limit: params.limit, offset: params.offset, - sort_by: domain::ports::FeedSortBy::from_str(sort_by_str), + sort_by: sort_by_str.parse().unwrap_or_default(), search: if params.search.is_empty() { None } else { @@ -1599,38 +1599,36 @@ pub async fn post_profile_settings( } "avatar" => { let ct = field.content_type().map(|s| s.to_string()); - if let Ok(bytes) = field.bytes().await { - if !bytes.is_empty() { - avatar_bytes = Some(bytes.to_vec()); - avatar_content_type = ct; - } + if let Ok(bytes) = field.bytes().await + && !bytes.is_empty() + { + avatar_bytes = Some(bytes.to_vec()); + avatar_content_type = ct; } } "banner" => { let ct = field.content_type().map(|s| s.to_string()); - if let Ok(bytes) = field.bytes().await { - if !bytes.is_empty() { - banner_bytes = Some(bytes.to_vec()); - banner_content_type = ct; - } + if let Ok(bytes) = field.bytes().await + && !bytes.is_empty() + { + banner_bytes = Some(bytes.to_vec()); + banner_content_type = ct; } } n if n.starts_with("field_name_") => { - if let Ok(idx) = n["field_name_".len()..].parse::() { - if let Ok(text) = field.text().await { - if !text.is_empty() { - field_names.insert(idx, text); - } - } + if let Ok(idx) = n["field_name_".len()..].parse::() + && let Ok(text) = field.text().await + && !text.is_empty() + { + field_names.insert(idx, text); } } n if n.starts_with("field_value_") => { - if let Ok(idx) = n["field_value_".len()..].parse::() { - if let Ok(text) = field.text().await { - if !text.is_empty() { - field_values.insert(idx, text); - } - } + if let Ok(idx) = n["field_value_".len()..].parse::() + && let Ok(text) = field.text().await + && !text.is_empty() + { + field_values.insert(idx, text); } } _ => {} diff --git a/crates/presentation/src/main.rs b/crates/presentation/src/main.rs index f22bf39..c7ef146 100644 --- a/crates/presentation/src/main.rs +++ b/crates/presentation/src/main.rs @@ -125,19 +125,19 @@ async fn wire_dependencies() -> anyhow::Result<(AppState, axum::Router)> { } }; - let ap = activitypub::wire( + let ap = activitypub::wire(activitypub::ActivityPubDeps { activity_repo, follow_repo, actor_repo, blocklist_repo, review_store, - remote_watchlist_repo.clone(), - Arc::clone(&ap_content_repo), - Arc::clone(&user_repository), - app_config.base_url.clone(), - app_config.allow_registration, - Arc::clone(&ep), - ) + remote_watchlist_repo: remote_watchlist_repo.clone(), + local_ap_content: Arc::clone(&ap_content_repo), + user_repo: Arc::clone(&user_repository), + base_url: app_config.base_url.clone(), + allow_registration: app_config.allow_registration, + event_publisher: Arc::clone(&ep), + }) .await?; let ap_router = ap.router; let ap_service_arc = ap.service; diff --git a/crates/presentation/src/tests/extractors.rs b/crates/presentation/src/tests/extractors.rs index d8fcc98..9cf3346 100644 --- a/crates/presentation/src/tests/extractors.rs +++ b/crates/presentation/src/tests/extractors.rs @@ -219,10 +219,7 @@ impl UserRepository for Panic { async fn update_profile( &self, _: &UserId, - _: Option, - _: Option, - _: Option, - _: Option, + _: &domain::models::UserProfile, ) -> Result<(), DomainError> { panic!() } diff --git a/crates/presentation/tests/api_test.rs b/crates/presentation/tests/api_test.rs index db7c63a..16173b3 100644 --- a/crates/presentation/tests/api_test.rs +++ b/crates/presentation/tests/api_test.rs @@ -119,10 +119,7 @@ impl UserRepository for NobodyUserRepo { async fn update_profile( &self, _: &UserId, - _: Option, - _: Option, - _: Option, - _: Option, + _: &domain::models::UserProfile, ) -> Result<(), DomainError> { Ok(()) } diff --git a/crates/tui/src/lib.rs b/crates/tui/src/lib.rs index fcf5f50..4ca7242 100644 --- a/crates/tui/src/lib.rs +++ b/crates/tui/src/lib.rs @@ -1,3 +1,4 @@ +#![allow(clippy::large_enum_variant)] pub mod app; pub mod client; pub mod config; diff --git a/crates/worker/src/main.rs b/crates/worker/src/main.rs index b905a70..1440392 100644 --- a/crates/worker/src/main.rs +++ b/crates/worker/src/main.rs @@ -102,28 +102,27 @@ async fn main() -> anyhow::Result<()> { // Both the event handler and the staleness job are gated on TMDB_API_KEY. // Without a key, no MovieEnrichmentRequested events are produced or handled. - let (enrichment_handler, enrichment_job): ( - Option>, - Option>, - ) = match tmdb_enrichment::TmdbEnrichmentClient::from_env() { - Ok(client) => { - tracing::info!("TMDb enrichment enabled"); - let handler = Arc::new(tmdb_enrichment::EnrichmentHandler { - enrichment_client: Arc::new(client), - movie_repository: Arc::clone(&ctx.movie_repository), - profile_repo: Arc::clone(&ctx.movie_profile_repository), - person_command: Arc::clone(&ctx.person_command), - search_command: Arc::clone(&ctx.search_command), - }) as Arc; - let job = Arc::new(application::jobs::EnrichmentStalenessJob::new(ctx.clone())) - as Arc; - (Some(handler), Some(job)) - } - Err(e) => { - tracing::warn!("TMDb enrichment disabled: {e}"); - (None, None) - } - }; + type OptionalPair = (Option>, Option>); + let (enrichment_handler, enrichment_job): OptionalPair = + match tmdb_enrichment::TmdbEnrichmentClient::from_env() { + Ok(client) => { + tracing::info!("TMDb enrichment enabled"); + let handler = Arc::new(tmdb_enrichment::EnrichmentHandler { + enrichment_client: Arc::new(client), + movie_repository: Arc::clone(&ctx.movie_repository), + profile_repo: Arc::clone(&ctx.movie_profile_repository), + person_command: Arc::clone(&ctx.person_command), + search_command: Arc::clone(&ctx.search_command), + }) as Arc; + let job = Arc::new(application::jobs::EnrichmentStalenessJob::new(ctx.clone())) + as Arc; + (Some(handler), Some(job)) + } + Err(e) => { + tracing::warn!("TMDb enrichment disabled: {e}"); + (None, None) + } + }; // ── Image conversion ────────────────────────────────────────────────────── @@ -196,19 +195,19 @@ async fn main() -> anyhow::Result<()> { #[cfg(feature = "federation")] { - let ap_wire = activitypub::wire( - fed_activity_repo, - fed_follow_repo, - fed_actor_repo, - fed_blocklist_repo, - fed_review_store, - fed_remote_watchlist_repo, - fed_ap_content, - fed_user_repo, + let ap_wire = activitypub::wire(activitypub::ActivityPubDeps { + activity_repo: fed_activity_repo, + follow_repo: fed_follow_repo, + actor_repo: fed_actor_repo, + blocklist_repo: fed_blocklist_repo, + review_store: fed_review_store, + remote_watchlist_repo: fed_remote_watchlist_repo, + local_ap_content: fed_ap_content, + user_repo: fed_user_repo, base_url, allow_registration, - Arc::clone(&ctx.event_publisher), - ) + event_publisher: Arc::clone(&ctx.event_publisher), + }) .await?; let ap_event_handler = ap_wire.event_handler;