From e6f4a6256f05f23d4dc4c65febbd54653252bc1f Mon Sep 17 00:00:00 2001 From: Gabriel Kaszewski Date: Thu, 14 May 2026 16:27:03 +0200 Subject: [PATCH] =?UTF-8?q?refactor(application):=20fix=204=20code=20smell?= =?UTF-8?q?s=20=E2=80=94=20validate=20username=20input,=20extract=20owners?= =?UTF-8?q?hip=20guard=20and=20dedup=20helpers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/services/federation_event.rs | 17 ++++++++++------- .../src/services/notification_event.rs | 12 ++++++++---- crates/application/src/use_cases/profile.rs | 2 +- crates/application/src/use_cases/thoughts.rs | 11 +++++++++-- 4 files changed, 28 insertions(+), 14 deletions(-) diff --git a/crates/application/src/services/federation_event.rs b/crates/application/src/services/federation_event.rs index 31fe29b..4a0e3c6 100644 --- a/crates/application/src/services/federation_event.rs +++ b/crates/application/src/services/federation_event.rs @@ -2,8 +2,9 @@ use std::sync::Arc; use domain::{ errors::DomainError, events::DomainEvent, - models::thought::Visibility, + models::thought::{Thought, Visibility}, ports::{OutboundFederationPort, ThoughtRepository, UserRepository}, + value_objects::ThoughtId, }; pub struct FederationEventService { @@ -14,6 +15,12 @@ pub struct FederationEventService { } impl FederationEventService { + fn object_ap_id(&self, thought: &Thought, thought_id: &ThoughtId) -> String { + thought.ap_id.clone().unwrap_or_else(|| { + format!("{}/thoughts/{}", self.base_url, thought_id) + }) + } + pub async fn process(&self, event: &DomainEvent) -> Result<(), DomainError> { match event { DomainEvent::ThoughtCreated { thought_id, user_id, .. } => { @@ -52,9 +59,7 @@ impl FederationEventService { Some(t) => t, None => return Ok(()), }; - let object_ap_id = thought.ap_id.clone().unwrap_or_else(|| { - format!("{}/thoughts/{}", self.base_url, thought_id) - }); + let object_ap_id = self.object_ap_id(&thought, thought_id); self.ap.broadcast_announce(user_id, &object_ap_id).await } @@ -63,9 +68,7 @@ impl FederationEventService { Some(t) => t, None => return Ok(()), }; - let object_ap_id = thought.ap_id.clone().unwrap_or_else(|| { - format!("{}/thoughts/{}", self.base_url, thought_id) - }); + let object_ap_id = self.object_ap_id(&thought, thought_id); self.ap.broadcast_undo_announce(user_id, &object_ap_id).await } diff --git a/crates/application/src/services/notification_event.rs b/crates/application/src/services/notification_event.rs index de1f166..5844623 100644 --- a/crates/application/src/services/notification_event.rs +++ b/crates/application/src/services/notification_event.rs @@ -5,7 +5,7 @@ use domain::{ events::DomainEvent, models::notification::{Notification, NotificationType}, ports::{NotificationRepository, ThoughtRepository}, - value_objects::NotificationId, + value_objects::{NotificationId, UserId}, }; pub struct NotificationEventService { @@ -13,6 +13,10 @@ pub struct NotificationEventService { pub notifications: Arc, } +fn is_self_action(thought_author: &UserId, actor: &UserId) -> bool { + thought_author == actor +} + impl NotificationEventService { pub async fn process(&self, event: &DomainEvent) -> Result<(), DomainError> { match event { @@ -21,7 +25,7 @@ impl NotificationEventService { Some(t) => t, None => return Ok(()), }; - if thought.user_id == *user_id { return Ok(()); } + if is_self_action(&thought.user_id, user_id) { return Ok(()); } self.notifications.save(&Notification { id: NotificationId::new(), user_id: thought.user_id, @@ -37,7 +41,7 @@ impl NotificationEventService { Some(t) => t, None => return Ok(()), }; - if thought.user_id == *user_id { return Ok(()); } + if is_self_action(&thought.user_id, user_id) { return Ok(()); } self.notifications.save(&Notification { id: NotificationId::new(), user_id: thought.user_id, @@ -68,7 +72,7 @@ impl NotificationEventService { Some(t) => t, None => return Ok(()), }; - if original.user_id == *user_id { return Ok(()); } + if is_self_action(&original.user_id, user_id) { return Ok(()); } self.notifications.save(&Notification { id: NotificationId::new(), user_id: original.user_id, diff --git a/crates/application/src/use_cases/profile.rs b/crates/application/src/use_cases/profile.rs index 3f52803..fdb3be3 100644 --- a/crates/application/src/use_cases/profile.rs +++ b/crates/application/src/use_cases/profile.rs @@ -10,7 +10,7 @@ pub async fn get_user(users: &dyn UserRepository, user_id: &UserId) -> Result Result { - let username = Username::from_trusted(username.to_string()); + let username = Username::new(username).map_err(|_| DomainError::NotFound)?; users.find_by_username(&username).await?.ok_or(DomainError::NotFound) } diff --git a/crates/application/src/use_cases/thoughts.rs b/crates/application/src/use_cases/thoughts.rs index 2d7f098..c4105d6 100644 --- a/crates/application/src/use_cases/thoughts.rs +++ b/crates/application/src/use_cases/thoughts.rs @@ -6,6 +6,13 @@ use domain::{ value_objects::{Content, ThoughtId, UserId}, }; +fn require_owner(thought: &Thought, user_id: &UserId) -> Result<(), DomainError> { + if thought.user_id != *user_id { + return Err(DomainError::NotFound); + } + Ok(()) +} + pub struct CreateThoughtInput { pub user_id: UserId, pub content: String, @@ -45,7 +52,7 @@ pub async fn delete_thought( user_id: &UserId, ) -> Result<(), DomainError> { let thought = thoughts.find_by_id(id).await?.ok_or(DomainError::NotFound)?; - if thought.user_id != *user_id { return Err(DomainError::NotFound); } + require_owner(&thought, user_id)?; thoughts.delete(id, user_id).await?; events.publish(&DomainEvent::ThoughtDeleted { thought_id: id.clone(), user_id: user_id.clone() }).await?; Ok(()) @@ -59,7 +66,7 @@ pub async fn edit_thought( new_content: String, ) -> Result<(), DomainError> { let thought = thoughts.find_by_id(id).await?.ok_or(DomainError::NotFound)?; - if thought.user_id != *user_id { return Err(DomainError::NotFound); } + require_owner(&thought, user_id)?; let content = Content::new_local(new_content)?; thoughts.update_content(id, &content).await?; events.publish(&DomainEvent::ThoughtUpdated { thought_id: id.clone(), user_id: user_id.clone() }).await?;